On 10/1/2024 4:59 AM, Russell King (Oracle) wrote: > On Mon, Sep 30, 2024 at 03:33:40PM -0700, Abhishek Chauhan wrote: >> AQR115c reports incorrect PMA capabilities which includes >> 10G/5G and also incorrectly disables capabilities like autoneg >> and 10Mbps support. >> >> AQR115c as per the Marvell databook supports speeds up to 2.5Gbps >> with autonegotiation. > > Thanks for persisting with this. Just one further item: > No worries, Russell. Thank you for your guidance. >> +static int aqr115c_get_features(struct phy_device *phydev) >> +{ >> + /* PHY FIXUP */ >> + /* Phy supports Speeds up to 2.5G with Autoneg though the phy PMA says otherwise */ >> + linkmode_or(phydev->supported, phydev->supported, phy_gbit_features); >> + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported); > > I'd still prefer to see: > > unsigned long *supported = phydev->supported; > > /* PHY supports speeds up to 2.5G with autoneg. PMA capabilities > * are not useful. > */ > linkmode_or(supported, supported, phy_gbit_features); > linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported); > > because that avoids going over column 80, and networking prefers it that > way. > Noted! > Other than that, the patch looks the best solution. > > Thanks. >