Re: [PATCH net-next] net: phylink: Support disabling autonegotiation for PCS

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Wed, Jun 30, 2021 at 11:49:27AM -0600, Robert Hancock wrote:
> The auto-negotiation state in the PCS as set by
> phylink_mii_c22_pcs_config was previously always enabled when the driver is
> configured for in-band autonegotiation, even if autonegotiation was
> disabled on the interface with ethtool. Update the code to set the
> BMCR_ANENABLE bit based on the interface's autonegotiation enabled
> state.
> 
> Update phylink_mii_c22_pcs_get_state to not check
> autonegotiation-related fields when autonegotiation is disabled.
> 
> Update phylink_mac_pcs_get_state to initialize the state based on the
> interface's configured speed, duplex and pause parameters rather than to
> unknown when autonegotiation is disabled, before calling the driver's
> pcs_get_state functions, as they are not likely to provide meaningful data
> for these fields when autonegotiation is disabled. In this case the
> driver is really just filling in the link state field.
> 
> Note that in cases where there is a downstream PHY connected, such as
> with SGMII and a copper PHY, the configuration set by ethtool is handled by
> phy_ethtool_ksettings_set and not propagated to the PCS. This is correct
> since SGMII or 1000Base-X autonegotiation with the PCS should normally
> still be used even if the copper side has disabled it.

In theory, this seems to be correct, but...

We do have some cases where, if a port is in 1000Base-X mode, the
documentation explicitly states that AN must be enabled. So, I think
if we are introducing the possibility to disable the negotiation in
1000Base-X mode, we need to give an option to explicitly reject that
configuration attempt.

We also need this to be consistently applied over all the existing
phylink-using drivers that support 1000Base-X without AN - we shouldn't
end up in the situation where we have different behaviours with
different network drivers.

So, we need mvneta and mvpp2 to reject such a configuration - with
these ports in 1000Base-X mode, the documentation states:

"Bit 2 Field InBandAnEn In-band Auto-Negotiation enable. ...
When <PortType> = 1 (1000BASE-X) this field must be set to 1."

We should be aware that there may be other hardware out there which
may not support 1000BASE-X without inband.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!