Re: [PATCH net-next v4 02/23] net: phy: add genphy_c45_read_eee_abilities() function

[Andrew Lunn]

On Wed, Feb 01, 2023 at 03:58:24PM +0100, Oleksij Rempel wrote:
> Add generic function for EEE abilities defined by IEEE 802.3
> specification. For now following registers are supported:
> - IEEE 802.3-2018 45.2.3.10 EEE control and capability 1 (Register 3.20)
> - IEEE 802.3cg-2019 45.2.1.186b 10BASE-T1L PMA status register
>   (Register 1.2295)
> 
> Since I was not able to find any flag signaling support of this
> registers, we should detect link mode abilities first and then based on
> this abilities doing EEE link modes detection.

Hi Oleksij

There was a discussion along these lines with Chris Healy
recently. The meson-gxl PHYs don't have these registers, and reads
return 0xffff. The 802.3 2018 standard says the top 2 bits are
reserved and should read as 0. Also, it seems unlikely anybody will
build a PHY which supports 100GBASE-R deep sleep all the way down to
100BASE-TX EEE. So i would suggest adding a check when reading
MDIO_PCS_EEE_ABLE and if it is 0xffff assume EEE is not supported.

> +		val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
> +		if (val < 0)
> +			return val;
> +
> +		mii_eee_100_10000_adv_mod_linkmode_t(phydev->supported_eee, val);
> +
> +		/* Some buggy devices claim not supported EEE link modes */
> +		linkmode_and(phydev->supported_eee, phydev->supported_eee,
> +			     phydev->supported);

That comment could be improved. What i think you mean is

/* Some buggy devices indicate EEE link modes in MDIO_PCS_EEE_ABLE
   which they don't support as indicated by BMSR, ESTATUS etc. */

   Andrew