THIS PATCH SET DOES NOT WORK, AND IT ISN'T INTENDED TO WORK. Changes in v2: - replace phy_check_cdr_lock() with phy_get_status(PHY_STATUS_CDR_LOCK) - rename PHY_MODE_ETHERNET_PHY to PHY_MODE_ETHTOOL - stop describing the AN/LT block in the device tree and make use of the fact that it is discoverable. Add phy_get_status(PHY_STATUS_PCVT_ADDR) to the generic PHY layer to discover it. - add the 25GBase-KR-S and 25GBase-CR-S link modes. Proper treatment of RS-FEC and BASE-R FEC is still TODO (will also require new API in the generic PHY layer). - rework the implementation from a phylib phy_device to a phylink_pcs component (library code). The phy-mode is still "internal". It may or may have not been the right thing to do. There are some things to say about that on patch 08/15. - support multi-lane link modes - tested with 40GBase-KR4 - solve the pre-configuration and register access problem for 1000Base-KX by having mtip_get_mdiodev() pre-configure the SerDes lane and protocol converter for the highest supported backplane link mode. The original cover letter for the RFC v1 at https://patchwork.kernel.org/project/netdevbpf/cover/20230817150644.3605105-1-vladimir.oltean@xxxxxxx/ is still attached below. ================================================= I've been tasked with maintaining the copper backplane support on Layerscape SoCs. There was a previous attempt to upstream that, which is located here: https://lore.kernel.org/lkml/1587732391-3374-1-git-send-email-florinel.iordache@xxxxxxx/ Nonetheless, I am presenting here a complete rewrite based on my understanding. The quality of implementation is not great, and there are probably bugs which I've yet to identify. The reason for this RFC is to collect feedback for the overall design from networking PHY and generic PHY maintainers. The newly proposed generic PHY API has the mtip_backplane.c driver as a user (a consumer), but its actual hardware implementation (which should go in drivers/phy/freescale/phy-fsl-lynx-28g.c), is not included in this patch set. That would just have just uselessly distracted the reviewers' attention. This is why the patch set, as posted, does not work. I recommend reviewing from the bottom up - dt-bindings first, those give an overall picture of the AN/LT block and its integration in the SoC. Future work consists of: - supporting multi-lane link modes - advertising more than a single link mode, and performing dynamic SerDes protocol switching towards that link mode. Strictly speaking, the hardware was intended to be used with a single link mode advertised over C73 (the link mode pre-configured through the RCW - Reset Configuration Word), and that is quite apparent in its design. With some inventive workarounds which I've yet to try out, it might be possible to circumvent the design limitations and advertise any link mode that the SerDes PLLs can sustain. This is in an exploratory stage and in the end it might not come to fruition, but I've identified some aspects which require forethought in the driver's design. Both these features hit a wall given the current driver design, which is "how do we access the AN/LT block's registers?". The hardware designers were inconsistent in the way that they've integrated this AN/LT blocks for 1G/10G ports, vs how they did it for 25G/40G/50G/100G ports. There is always one single AN/LT block per SerDes lane, but for 1G/10G modes, hardware design wanted to make it appear as if the AN/LT is part of the PCS. So it inherently responds to the same MDIO address as the PCS. Whereas in the >25G modes, it responds to an MDIO address defined by a different control register, and that address is also different from the PCS' MDIO address. In the current dt-bindings, I am requesting DT writers to put a phy-handle towards the MDIO address of the AN/LT block (which depends on a lot of things, see the dt-bindings document for details). As opposed to regular ports where the PCS and SerDes PHY are controlled by the MAC driver (drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c), with backplane, those same components are controlled by the mtip_backplane.c driver. The backplane AN/LT driver has a pcs-handle towards the PCS, and it treats it as a raw MDIO device when polling for link status. This gets obviously problematic when switching SerDes protocols, because the location of the backplane AN/LT block changes on the MDIO bus with the new SerDes protocol, and what's in the device tree becomes invalid (the phydev->mdio.addr would need to be updated). It's the same AN/LT block, and after a SerDes protocol change it has been reset, but it moved. The SerDes control registers for the MDIO addresses of the PCS and AN/LT blocks: - SGMIInCR1[MDEV_PORT] - SXGMIInCR1[MDEV_PORT] - ANLTnCR1[MDEV_PORT] - E25GnCR1[MDEV_PORT] - E40GnCR1[MDEV_PORT] - E50GnCR1[MDEV_PORT] - E100GnCR1[MDEV_PORT] are in premise all configurable, but it's an open question to me as to how Linux should configure them. Currently, we treat all those addresses as read-only and populate the device trees based on their default values. There's an additional (related) problem for 1000base-KX. The lane starts up at power-on reset in 1000base-X mode (non-backplane) and this means that the PCS which responds to MDIO commands is the one used for SGMII/1000Base-X by drivers/net/pcs/pcs-lynx.c. That responds to C22 MDIO commands. The PCS for 1000Base-KX is different, and it responds to C45 MDIO commands. It also incorporates the AN/LT block at this C45 MDIO address. In the current mtip_backplane.c driver design, the lane switches to backplane mode (thus enabling the respective PCS) once the link mode was resolved to 1000base-KX through C73 autoneg, using phy_set_mode_ext(). But that is too late with 1000base-KX, since the AN/LT block won't respond to C45 transactions either, if the port isn't pre-configured for 1000base-KX. So C73 autoneg won't work to begin with... Somebody needs to pre-configure the SerDes lane for backplane mode, so that the mtip_backplane.c driver can probe, but it's not clear who and based on what. Finally, I reckon that in the end, the PCS should be driven using a struct phylink_pcs, by the lynx_pcs driver, and not as an mdio_device using raw accesses by the mtip_backplane.c. In premise, this AN/LT IP core can be integrated, to my knowledge, with any PCS and any SerDes, so that should be also possible in the driver design. Yet, there's a problem: in the 1G/10G modes, there would be 2 drivers for the same mdio_device: one for the PCS and the other for the AN/LT block (PHY). True, they are at different MMDs, but bindings multiple Linux drivers to mdio_devices per MMD is not a thing, I guess? Interrupt support is also missing, and that's very far away on my TODO list. AFAIU, PCS/ANLT interrupts are routed by the SoC to the attached MAC's IEVENT register, which isn't enabled as an interrupt source on any Layerscape platform yet. I've structured the code using an irqpoll thread for now, so it is more-or-less just as event-driven as an IRQ based driver would be. As a side note, I have analyzed the feedback previously given to Florinel, especially the one from Russell: https://lore.kernel.org/lkml/20200425105210.GZ25745@xxxxxxxxxxxxxxxxxxxxx/ "This uses phylib, which is a problem ..." and I still haven't changed that here. Without going into a wall of text explanation, I'm not fully convinced that phylib isn't, in fact, the best place for a backplane AN/LT driver to live. At least in the initial implementation shown here, I'm not sure that going straight for a phylink implementation of the AN/LT block would have solved any of the problems that I described above. Vladimir Oltean (15): phy: introduce phy_get_status() and use it to report CDR lock phy: introduce the PHY_MODE_ETHTOOL mode for phy_set_mode_ext() phy: ethernet: add configuration interface for copper backplane Ethernet PHYs phy: allow querying the address of protocol converters through phy_get_status() net: add 25GBase-KR-S and 25GBase-CR-S to ethtool link mode UAPI net: mii: add C73 base page helpers net: phylink: centralize phy_interface_mode_is_8023z() && phylink_autoneg_inband() checks net: phylink: allow PCS to handle C73 autoneg for phy-mode = "internal" net: ethtool: introduce ethtool_link_mode_str() net: phylink: support all ethtool port modes with inband modes net: phylink: support the 25GBase-KR-S and 25GBase-CR-S link modes too net: phylink: add the 25G link modes to phylink_c73_priority_resolution[] dt-bindings: lynx-pcs: add properties for backplane mode net: pcs: mtip_backplane: add driver for MoreThanIP backplane AN/LT core net: pcs: lynx: use MTIP AN/LT block for copper backplanes .../bindings/net/pcs/fsl,lynx-pcs.yaml | 15 +- drivers/net/mii.c | 34 +- drivers/net/pcs/Kconfig | 8 + drivers/net/pcs/Makefile | 1 + drivers/net/pcs/mtip_backplane.c | 2022 +++++++++++++++++ drivers/net/pcs/mtip_backplane.h | 87 + drivers/net/pcs/pcs-lynx.c | 135 ++ drivers/net/phy/phy-core.c | 2 +- drivers/net/phy/phylink.c | 53 +- drivers/phy/phy-core.c | 31 + include/linux/ethtool.h | 6 + include/linux/mii.h | 105 + include/linux/phy/phy-ethernet.h | 292 +++ include/linux/phy/phy.h | 83 + include/linux/phylink.h | 1 + include/uapi/linux/ethtool.h | 2 + net/ethtool/common.c | 12 + 17 files changed, 2876 insertions(+), 13 deletions(-) create mode 100644 drivers/net/pcs/mtip_backplane.c create mode 100644 drivers/net/pcs/mtip_backplane.h create mode 100644 include/linux/phy/phy-ethernet.h -- 2.34.1
