On Wed, Apr 24, 2024 at 05:56:47PM +0200, Stefan Eichenberger wrote: > On Wed, Apr 24, 2024 at 03:58:00PM +0100, Russell King (Oracle) wrote: > > On Thu, Apr 18, 2024 at 04:01:59PM +0100, Russell King (Oracle) wrote: > > > On Wed, Apr 17, 2024 at 09:22:50AM +0200, Stefan Eichenberger wrote: > > > > I also checked the datasheet and you are right about the 1000base-X mode > > > > and in-band AN. What worked for us so far was to use SGMII mode even for > > > > 2.5Gbps and disable in-band AN (which is possible for SGMII). I think > > > > this works because as you wrote, the genphy just multiplies the clock by > > > > 2.5 and doesn't care if it's 1000base-X or SGMII. With your patches we > > > > might even be able to use in-band autonegoation for 10,100 and 1000Mbps > > > > and then just disable it for 2.5Gbps. I need to test it, but I have hope > > > > that this should work. > > > > > > There is another way we could address this. If the querying support > > > had a means to identify that the endpoint supports bypass mode, we > > > could then have phylink identify that, and arrange to program the > > > mvpp2 end to be in 1000base-X + x2.5 clock + AN bypass, which would > > > mean it wouldn't require the inband 16-bit word to be present. > > > > > > I haven't fully thought it through yet - for example, I haven't > > > considered how we should indicate to the PCS that AN bypass mode > > > should be enabled or disabled via the pcs_config() method. > > > > Okay, I've been trying to put more effort into this, but it's been slow > > progress (sorry). > > > > My thoughts from a design point of view were that we could just switch > > to PHYLINK_PCS_NEG_OUTBAND instead of PHYLINK_PCS_NEG_INBAND_* and > > everything at the PCS layer should be able to cope, but this is not the > > case, especially with mvneta/mvpp2. > > > > The problem is that mvneta/mvpp2 (and probably more) expect that > > > > 1) MLO_AN_INBAND means that the PCS will be using inband, and that > > means the link up/down state won't be forced. This basically implies > > that only PHYLINK_PCS_NEG_INBAND_* can be used can be used for the > > PCS. > > > > 2) !MLO_AN_INBAND means that an out-of-band mechanism will be used and > > that means that the link needs to be forced (since there's no way > > for the hardware to know whether the link should be up or down.) > > It's therefore expected that only PHYLINK_PCS_NEG_OUTBAND will be > > used for the PCS. > > > > So, attempting to put a resolution of the PHY and PCS abilities into > > phylink_pcs_neg_mode() and select the appropriate PHYLINK_PCS_NEG_* > > mode alone just doesn't work. Yet... we need to do that in there when > > considering whether inband can be enabled or not for non-PHY links. > > > > Basically, it needs a re-think how to solve this... > > Today I was playing around with my combination of mxl-gpy and mvpp2 and > I got it working again with your patches applied. However, I hacked the > phylink driver to only rely on what the phy and pcs support. I know this > is not a proper solution, but it allowed me to verify the other changes. > My idea was if the phy and pcs support inband then use it, otherwise use > outband and ignore the rest. > > Here is how my minimal phylink_pcs_neg_mode test function looks like: > > static unsigned int phylink_pcs_neg_mode(struct phylink *pl, > struct phylink_pcs *pcs, > unsigned int mode, > phy_interface_t interface, > const unsigned long *advertising) > { > unsigned int phy_link_mode = 0; > unsigned int pcs_link_mode; > > pcs_link_mode = phylink_pcs_query_inband(pcs, interface); > if (pl->phydev) > phy_link_mode = phy_query_inband(pl->phydev, interface); > > /* If the PCS or PHY can not provide inband, then use > * outband. > */ > if (!(pcs_link_mode & LINK_INBAND_VALID) || > !(phy_link_mode & LINK_INBAND_VALID)) > return PHYLINK_PCS_NEG_OUTBAND; > > return PHYLINK_PCS_NEG_INBAND_ENABLED; > } Note that I've changed the flags that get reported to be disable (bit 0)/ enable (bit 1) rather than valid/possible/required because the former makes the resolution easier. The problem is that merely returning outband doesn't cause mvneta/mvpp2 to force the link up. So for example, here's a SFP module which doesn't support any inband for 2500base-X nor SGMII: mvneta f1034000.ethernet eno2: copper SFP: interfaces=[mac=4,9-12,19,22-23, sfp= 4,23,27] mvneta f1034000.ethernet eno2: copper SFP: chosen 2500base-x interface mvneta f1034000.ethernet eno2: PHY i2c:sfp:16 uses interfaces 4,23,27, validatin g 4,23 mvneta f1034000.ethernet eno2: interface 4 (sgmii) rate match none supports 2-3 ,5-6,13 mvneta f1034000.ethernet eno2: interface 23 (2500base-x) rate match none suppor ts 6,13,47 mvneta f1034000.ethernet eno2: PHY [i2c:sfp:16] driver [Broadcom BCM84881] (irq= POLL) mvneta f1034000.ethernet eno2: phy: 2500base-x setting supported 00,00000000,000 08000,0000206c advertising 00,00000000,00008000,0000206c mvneta f1034000.ethernet eno2: copper SFP: PHY link in-band modes 0x1 mvneta f1034000.ethernet eno2: major config 2500base-x mvneta f1034000.ethernet eno2: link modes: pcs=02 phy=01 mvneta f1034000.ethernet eno2: phylink_mac_config: mode=inband/2500base-x/none a dv=00,00000000,00008000,0000206c pause=04 mvneta f1034000.ethernet eno2: phylink_sfp_module_start() mvneta f1034000.ethernet eno2: phylink_sfp_link_up() mvneta f1034000.ethernet eno2: phy link down 2500base-x/Unknown/Unknown/none/off mvneta f1034000.ethernet eno2: phy link up sgmii/1Gbps/Full/none/off mvneta f1034000.ethernet eno2: major config sgmii mvneta f1034000.ethernet eno2: link modes: pcs=03 phy=01 mvneta f1034000.ethernet eno2: phylink_mac_config: mode=inband/sgmii/none adv=00,00000000,00008000,0000206c pause=00 mvneta f1034000.ethernet eno2: pcs link down mvneta f1034000.ethernet eno2: pcs link down mvneta f1034000.ethernet eno2: can LPI, EEE enabled, active mvneta f1034000.ethernet eno2: enabling tx_lpi, timer 250us mvneta f1034000.ethernet eno2: Link is Up - 1Gbps/Full - flow control off This looks like the link is up, but it isn't - note "pcs link down". If we look at the value of the GMAC AN status register: Value at address 0xf1036c10: 0x0000600a which indicates that the link is down, so no packets will pass. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!