Skip to content

net: dsa: sja1105: phylink updates

Russell King (Oracle) netdev 2 versions Applied

Status from this message: This series was applied to netdev/net-next.git (master)

Get series with b4
b4 am YhjEm/[email protected]

Versions newest first · linked by author and title

  1. v2 Latest0 of 6 patches 0 repliesApplied
  2. v10 of 6 patches 0 repliesSuperseded

Patches in v2 Applied

None of the 6 patches the cover letter announces are in this archive.

Reviews trailers per patch

PatchVladimir Oltean
SeriesR-b R-b R-b R-b R-b R-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v2 thread

Cover letter [PATCH net-next v2 0/6] net: dsa: sja1105: phylink updates

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(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!