[PATCH net-next 0/15] Add and use helper for PCS negotiation modes

[Vladimir Oltean]

On Fri, Jun 16, 2023 at 04:52:21PM +0100, Russell King (Oracle) wrote:
> I should also add... yes, I did _then_ subsequently use the microchip
> driver as a justification for it. I probably should've explained it
> without using that as justification.
> 
> I could also have used the sja1105 driver as well, since:
> 
> 	MLO_AN_INBAND => PHYLINK_PCS_NEG_INBAND_ENABLED

Technically this should have been:

	MLO_AN_INBAND => neg_mode & PHYLINK_PCS_NEG_INBAND, which
	includes both INBAND_DISABLED and INBAND_ENABLED, right?

> 	MLO_AN_FIXED || MLO_AN_PHY => PHYLINK_PCS_NEG_OUTBAND
> 
> are the conversions done there, which fits with:
> 
> -               if (!phylink_autoneg_inband(mode)) {
> +               if (neg_mode == PHYLINK_PCS_NEG_OUTBAND) {
> 
> since the opposite of !inband is outband.

The conversion is correct - no doubt about it.

Maybe the SJA1105 and its use of the XPCS is also not the best example,
because it doesn't support 1000BASE-X. So it doesn't have to handle the
INBAND_DISABLED state. If it did, the !phylink_autoneg_inband(mode)
check would have been incorrect (insufficient to detect the xpcs state
that it's restoring).