On Thu, Apr 25, 2024 at 05:30:50PM +0100, Russell King (Oracle) wrote: > 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.) > Now I got it, thanks a lot for the explanation. So the issue is that MLO_AN_INBAND + PHYLINK_PCS_NEG_OUTBAND is happening in my use case and therefore the link is not forced up because for that MLO_AN_PHY would be needed. I will also try to think about it. Thanks, Stefan
