Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Tue, Mar 17, 2020 at 04:04:28PM +0000, Jose Abreu wrote:
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Date: Mar/17/2020, 15:56:10 (UTC+00:00)
> 
> > > Please consider removing this condition and just rely on an_enabled field. 
> > > I have USXGMII support for Clause 73 Autoneg so this won't work with 
> > > that.

Do you really mean USXGMII or XLGMII that you recently contributed?
XLGMII makes more sense for clause 73.

> > That is actually incorrect.  SGMII can have an_enabled being true, but
> > SGMII is not an autonegotiation between the MAC and PHY - it is merely
> > a mechanism for the PHY to inform the MAC what the results of _its_
> > negotiation are.
> > 
> > I suspect USXGMII is the same since it is just an "upgraded" version of
> > SGMII.  Please can you check whether there really is any value in trying
> > (and likely failing) to restart the "handshake" with the PHY from the
> > MAC end, rather than requesting the PHY to restart negotiation on its
> > media side.
> 
> I think we are speaking of different things here. I'm speaking about 
> end-to-end Autoneg. Not PHY <-> PCS <-> MAC.

What do you mean end-to-end autoneg?  Let's take a simple example for
SGMII, here is the complete setup:

MAC <--> PCS <--SGMII--> PHY <--MEDIA--> PHY <--SGMII--> MAC

Generally, asking the PCS to "renegotiate" will either be ignored, or
might cause the PCS to restart the handshake with the PHY depending on
the implementation.  It will not cause the PHY to renegotiate with the
remote PHY.

Asking the PHY to renegotiate will cause the link to drop, which
updates the config_reg word sent to the PCS to indicate link down.
When the link is re-established, the config_reg word is updated again
with the new link parameters and that sent to the PCS.

So, just because something is closer to the MAC does not necessarily
mean that it causes more of the blocks to "renegotiate" when asked to
do so.

In SGMII, the PHY is the "master" and this is what needs negotiation
restarted on "ethtool -r" to have the effect that the user desires.

I believe (but I don't know for certain, because the USXGMII
documentation is not available to me) that USXGMII is SGMII extended
up to additionally include 10G, 5G and 2.5G speeds, and otherwise is
basically the same mechanism.

So, I would suggest that USXGMII and SGMII should be treated the same,
and for both of them, a renegotiation should always be performed at
the PHY and not the PCS.

There is another reason not to try triggering renegotiation at both
the PHY and PCS.  When the PHY is renegotiating, the state machines
at both the PHY and PCS end are in the process of changing - if we
hit the PCS with a request to renegotiate, and the hardware isn't
setup to ignore it, that could occur in an unexpected state - the risk
of triggering a hardware problem could be higher.

So, based on this, I don't think it's a good idea to restart
negotiation at the PCS for SGMII and USXGMII modes.

For the 802.3z based protocols, it makes complete sense to do so,
because the PCS are the blocks involved in the actual media negotiation
and there is no place else to restart negotiation:

MAC <---> PCS <----fiber 1000base-X----> PCS <---> MAC

> I'm so sorry but I'm not an expert in this field, I just deal mostly with 
> IP.
> 
> Anyway, I'm speaking about end-to-end Clause 73 Autoneg which involves 
> exchanging info with the peer. If peer for some reason is not available to 
> receive this info then AutoNeg will not succeed. Hence the reason to 
> restart it.

Clause 73 covers backplane and copper cable, and isn't USXGMII.
In the case of copper, I would expect the setup to be very similar
to what I've outlined above for the SGMII case, but using USXGMII
instead of SGMII, or automatically selecting something like
10GBASE-R, 5GBASE-R, 2500BASE-X, or SGMII depending on the result
from copper negotiation.  Depending on the Ethernet PHY being used,
it may or may not have the in-band control_reg word even for SGMII.
In any case, what I've said above applies: to provoke end-to-end
renegotiation, the PHY needs to be restarted, not the MAC's PCS.

For backplane, things are a little different, and that may indeed
require the MAC's PCS to be restarted, and for that case it may be
reasonable to expand the conditional there.

However, note that for the purposes of this patch, no change of
behaviour over the current behaviour is intended; a change to this
will need to be a separate patch.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up