Re: [PATCH net-next v3 1/1] net: phy: marvell: avoid bringing down fibre link when autoneg is bypassed

[Russell King (Oracle)]

On Thu, Oct 03, 2024 at 05:10:50PM +1000, Qingtao Cao wrote:
> On 88E151x the SGMII autoneg bypass mode defaults to be enabled. When it is
> activated, the device assumes a link-up status with existing configuration
> in BMCR, avoid bringing down the fibre link in this case
> 
> Test case:
> 1. Two 88E151x connected with SFP, both enable autoneg, link is up with
>    speed 1000M
> 2. Disable autoneg on one device and explicitly set its speed to 1000M
> 3. The fibre link can still up with this change, otherwise not.

As you're clearly using fibre, there's this chunk of code in the same
function that is (IMHO) wrong:

                if (phydev->duplex == DUPLEX_FULL) {
                        if (!(lpa & LPA_PAUSE_FIBER)) {
                                phydev->pause = 0;
                                phydev->asym_pause = 0;
                        } else if ((lpa & LPA_PAUSE_ASYM_FIBER)) {
                                phydev->pause = 1;
                                phydev->asym_pause = 1;
                        } else {
                                phydev->pause = 1;
                                phydev->asym_pause = 0;
                        }
                }

as ->pause and ->asym_pause are supposed to be the _resolved_
state, and this is only looking at the link partner's state.

fiber_lpa_mod_linkmode_lpa_t() also uses the baseT linkmodes for
Fibre.

IMHO, this should be:

		mii_lpa_mod_linkmode_x(phydev->lp_advertising, lpa,
				ETHTOOL_LINK_MODE_1000baseX_Full_BIT);
		phy_resolve_aneg_pause(phydev);

Please can you test whether that works correctly in addition to
your other fix? Thanks.

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