[PATCH net-next v2 0/6] net: dsa: sja1105: phylink updates
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2022-02-25 11:59:31
Hi, This series updates the phylink implementation in sja1105 to use the supported_interfaces bitmap, convert to the mac_select_pcs() interface, mark as non-legacy, and get rid of the validation method. As a final step, enable switching between SGMII and 2500BASE-X as it is a feature that Vladimir desires. Specifically, the patches in this series: 1. Populates the supported_interfaces bitmap. 2. As a result of the supported_interfaces bitmap being populated, sja1105 no longer needs to check the interface mode as phylink will do this. 3. Switch away from using phylink_set_pcs(), using the mac_select_pcs() method instead. 4. Mark the driver as not-legacy 5. Fill in mac_capabilities using _exactly_ the same conditions as is currently used to decide which link modes to support, and convert to use phylink_generic_validate() 6. Add brand new support to permit switching between SGMII and 2500BASE-X modes of operation as per Vladimir's single patch that performs steps 1, 2, 5 and 6 in one go. There are some additional changes in Vladimir's single patch that I have not included: * validation of priv->phy_mode[] in sja1105_phylink_get_caps(). The driver has already validated the phy_mode for each port in sja1105_init_mii_settings(), and a failure here will prevent the driver reaching sja1105_phylink_get_caps(). * Changing the decisions on which mac_capabilities to set. Vladimir's patch always sets MAC_10FD | MAC_100FD | MAC_1000FD despite the current code clearly making the 1G speed conditional on the xmii_mode for the port. The change in decision making may be visible when in PHY_INTERFACE_MODE_INTERNAL mode, for which the phylink_generic_validate() will pass through all the MAC capabilities as ethtool link modes. Hence, if we have PHY_INTERFACE_MODE_INTERNAL but supports_rgmii[] or supports_sgmii[] is non-zero, currently we do not get 1G speeds. With Vladimir's additional change, we will get 1G speeds. While it is not clear whether that can happen, I feel changing the decision making should be a separate patch. * The decision for MAC_2500FD is made differently - sja1105_init_mii_settings() allows PHY_INTERFACE_MODE_2500BASEX when supports_2500basex[] is non-zero, and is not based on any other condition such as supports_sgmii[] or supports_rgmii[]. Vladimir's patch makes it additionally conditional on those supports_.gmii[] settings, which is a functional change that should be made in a separate patch - and if desired, then sja1105_init_mii_settings() should also be updated at the same time. Consequently, I believe that my previous objections to Vladimir's single patch approach are well founded and justified, even through Vladimir is the maintainer of this driver. I have no objection to the additional changes, I just don't think they should all be wrapped up into a single patch that converts the way validation is done _and_ also makes a bunch of other functional changes. RFC->non-RFC: added Vladimir's Reviewed-by's, fixed the typo in the commit message of patch 6, and removed the phrase at the end of a comment as requested. v2: fix the fscking vi fsckups when pasting in attributations. drivers/net/dsa/sja1105/sja1105_main.c | 100 ++++++++++++++------------------- 1 file changed, 42 insertions(+), 58 deletions(-) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!