On Thu, Apr 25, 2024 at 05:51:57PM +0200, Stefan Eichenberger wrote: > On Thu, Apr 25, 2024 at 03:30:52PM +0100, Russell King (Oracle) wrote: > > On Thu, Apr 25, 2024 at 01:24:51PM +0200, Stefan Eichenberger wrote: > > > I think it should also work for mvneta, at least > > > the code looks almost the same. I get the following for the Port > > > Auto-Negotiation Configuration Register: > > > > > > For 1Gbit/s it switches to SGMII and enables inband AN: > > > Memory mapped at address 0xffffa0112000. > > > Value at address 0xF2132E0C (0xffffa0112e0c): 0xB0C6 > > > > So here the link is forced up which is wrong for inband, because then > > we have no way to detect the link going down. > > Yes I also saw this and didn't understand it. When I clear the force bit > it will be set back to 1 again when AN is enabled. I thought this might > be a bug of the controller. No it isn't, the hardware never changes the value in this register. The difference will be because of what I explained previously. Because the mvneta and mvpp2 hardware is "weird" in that these two registers provide both PCS and MAC functions, we have to deal with them in a way that is split between the phylink PCS and MAC operations. This code was written at a time when all we had was MLO_AN_* and none of the PHYLINK_PCS_NEG_* stuff. PCSes got passed the MLO_AN_* constants which were the same as what was passed via the MAC operations. This made the implementation entirely consistent. However, that lead PCS implementers to go off and do their own things, so we ended up with a range of different PCS behaviours depending on the implementation (because the way people interpreted MLO_AN_INBAND, interface mode, and the Autoneg bit in the advertising mask was all different. So to bring some consistency, I changed the PCS interface to what we have now, in the belief that it would later allow us to solve the 2500base-X problem. However, I'd forgotten about the mvneta/mvpp2 details, but that was fine at the time of this change because everything was still consistent - we would only ever use PHYLINK_PCS_NEG_OUTBAND or PHYLINK_PCS_NEG_NONE for non-MLO_AN_INBAND modes, and PHYLINK_PCS_NEG_INBAND_* for interface modes that support inband when using MLO_AN_INBAND. Now, when trying to solve the 2500base-X problem which needs us to use PHYLINK_PCS_NEG_OUTBAND in some situations, this means we can end up with MLO_AN_INBAND + PHYLINK_PCS_NEG_OUTBAND, and this is what is causing me problems (the link isn't forced up.) Conversely, I suspect you have the situation where you have MLO_AN_PHY or MLO_AN_FIXED being passed to the mac_config() and mac_link_up() operations, but the PCS is being configured for a different mode. I am wondering whether we should at the very least move the configuration of the control register 0 and 2 to the pcs_config() method so at least that's consistent with the PHYLINK_PCS_NEG_* value passed to the PCS and thus the values programmed into the autoneg config register. However, that still leaves a big hole in how we handle the link forcing - which is necessary if inband AN is disabled (in other words, if we need to read the link status from the PHY as is done in MLO_AN_PHY mode.) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
