Re: [PATCH v4 net-next 5/8] net: phylink: explicitly configure in-band autoneg for on-board PHYs

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

On Fri, Nov 18, 2022 at 02:01:21AM +0200, Vladimir Oltean wrote:
> +	if (pl->config->sync_an_inband && !phy_on_sfp(phy)) {

Hmm, this phy_on_sfp() is new to me, and looking at the git history, I
really don't think this does what it claims to do. This returns the
status of phydev->is_on_sfp_module, which is set by this code:

        phydev->phy_link_change = phy_link_change;
        if (dev) {
                phydev->attached_dev = dev;
                dev->phydev = phydev;

                if (phydev->sfp_bus_attached)
                        dev->sfp_bus = phydev->sfp_bus;
                else if (dev->sfp_bus)
                        phydev->is_on_sfp_module = true;
        }

... which is very wrong. "dev" here is the net_device, and a net_device
will have its sfp_bus member set when there is a SFP cage present,
which may be behind a off-SFP PHY.

This means that when a PHY is attached by the network driver in their
ndo_open, if there is a SFP bus on the interface (such as on the
Macchiatobin board), the above will set is_on_sfp_module true for the
on-board PHY even though it is not in the SFP module.

Essentially, commit b834489bcecc is incorrect, and needs to be fixed
before use is made of phy_on_sfp() outside of the broadcom driver.

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