On Tue, Nov 22, 2022 at 09:38:43AM +0000, Russell King (Oracle) wrote:

Also, if we get the Marvell driver implementing validate_an_inband()
then I believe we can get rid of other parts of this patch - 88E1111 is
the commonly used accessible PHY on gigabit SFPs, as this PHY implements
I2C access natively. As I mentioned, Marvell PHYs can be set to no
inband, requiring inband, or inband with bypass mode enabled. So we
need to decide how we deal with that - especially if we're going to be
changing the mode from 1000base-X to SGMII (which we do on some SFP
modules so they work at 10/100/1000.)

Not clear which parts of this patch we could ged rid of, if we
implemented in-band AN reporting/configuration for the 88E1111.
Based on your previous description, it sounds like it would be just more
functionality for software rather than less.

In that regard, I'm not entirely convinced that validate_an_inband()
covers the functionality we need - as reading the config register on
Marvell hardware doesn't guarantee that we're reading the right mode -
the PHY may be in 1000base-X, and we might change it to
SGMII-with-bypass - I'd need to go through the PHY datasheets to check
what we actually do.

Changing what the PHY driver does would be a recipe for regressions,
especially for drivers that do not use phylink.

I believe that currently, the "interface" passed to phy_validate_an_inband()
and phy_config_an_inband() is always also the phydev->interface.
We could strive to keep that being the case, and put a phydev_warn() in
the Marvell PHY driver if we get a query for interface != phydev->interface,
and report PHY_AN_INBAND_UNKNOWN.

It's also one of the reasons why I didn't yet want to jump right into
figuring out what should be done with PHYs that change SERDES protocols,
and when exactly we query the in-band capability for the new one. Right
now, phylink will assume that the in-band capability reported for the
first SERDES protocol will continue to be the same for all subsequent
protocols. Obviously this might not be the case.