[PATCH net V2] net: phy: motorcomm: read EEE abilities in yt8521_get_features()

[PATCH net V2] net: phy: motorcomm: read EEE abilities in yt8521_get_features()

[xiaoning.wang]

From: Clark Wang <xiaoning.wang@nxp.com>

In phy_probe(), genphy_c45_read_eee_abilities() is only called when a
driver uses phydrv->features. Drivers that implement .get_features are
responsible for reading the EEE abilities themselves.

yt8521_get_features() does not do this, so phydev->supported_eee stays
empty for YT8521/YT8531S and "ethtool --show-eee" reports "EEE status:
not supported", even though the PHY has the standard EEE capability
registers.

Call genphy_c45_read_eee_abilities() at the end of yt8521_get_features()
to populate supported_eee.

Fixes: 70479a40954c ("net: phy: Add driver for Motorcomm yt8521 gigabit ethernet phy")
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
V2 change:
 - Since both the YT8521 and YT8531s, which call the yt8521_get_features(),
   support EEE, if genphy_c45_read_eee_abilities() returns an error, it
   indicates a problem with MDIO bus, and the error should be returned
   accordingly.
---
 drivers/net/phy/motorcomm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 5071605a1a11..73b5da937d7f 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -2490,7 +2490,8 @@ static int yt8521_get_features(struct phy_device *phydev)
 		/* add fiber's features to phydev->supported */
 		yt8521_prepare_fiber_features(phydev, phydev->supported);
 	}
-	return ret;
+
+	return ret ? ret : genphy_c45_read_eee_abilities(phydev);
 }
 
 /**
-- 
2.34.1

Re: [PATCH net V2] net: phy: motorcomm: read EEE abilities in yt8521_get_features()

[Breno Leitao]

On Fri, Jul 10, 2026 at 04:00:18PM +0800, xiaoning.wang@oss.nxp.com wrote:
> From: Clark Wang <xiaoning.wang@nxp.com>
> 
> In phy_probe(), genphy_c45_read_eee_abilities() is only called when a
> driver uses phydrv->features. Drivers that implement .get_features are
> responsible for reading the EEE abilities themselves.
> 
> yt8521_get_features() does not do this, so phydev->supported_eee stays
> empty for YT8521/YT8531S and "ethtool --show-eee" reports "EEE status:
> not supported", even though the PHY has the standard EEE capability
> registers.
> 
> Call genphy_c45_read_eee_abilities() at the end of yt8521_get_features()
> to populate supported_eee.
> 
> Fixes: 70479a40954c ("net: phy: Add driver for Motorcomm yt8521 gigabit ethernet phy")
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

Re: [PATCH net V2] net: phy: motorcomm: read EEE abilities in yt8521_get_features()

[Andrew Lunn]

> @@ -2490,7 +2490,8 @@ static int yt8521_get_features(struct phy_device *phydev)
>  		/* add fiber's features to phydev->supported */
>  		yt8521_prepare_fiber_features(phydev, phydev->supported);
>  	}
> -	return ret;
> +
> +	return ret ? ret : genphy_c45_read_eee_abilities(phydev);

Using a ? like this is very uncommon.

Please change it to the usual

        if (ret < 0)
               return ret;

	return genphy_c45_read_eee_abilities(phydev);

It might also be better to put the test after
yt8521_get_features_paged() call, since that is the only place missing
the test.

    Andrew

---
pw-bot: cr

Re: [PATCH net V2] net: phy: motorcomm: read EEE abilities in yt8521_get_features()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The call to `genphy_c45_read_eee_abilities()` accesses MMD registers without ensuring the PHY is in the UTP register space, potentially executing accesses on the FIBER space.
--

--- Patch [1]: [PATCH net V2] net: phy: motorcomm: read EEE abilities in yt8521_get_features() ---
commit c7203820fd5ed8cfc23d1995e9a795cf1f8d31b3
Author: Clark Wang <xiaoning.wang@nxp.com>

net: phy: motorcomm: read EEE abilities in yt8521_get_features()

This commit calls genphy_c45_read_eee_abilities() at the end of
yt8521_get_features() to populate supported_eee for the YT8521/YT8531S PHYs.
Drivers implementing .get_features are responsible for reading EEE abilities
themselves.

> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
> index 5071605a1a117..818098a78d23d 100644
> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c
> @@ -2490,7 +2490,8 @@ static int yt8521_get_features(struct phy_device *phydev)
>  		/* add fiber's features to phydev->supported */
>  		yt8521_prepare_fiber_features(phydev, phydev->supported);
>  	}
> -	return ret;
> +
> +	return ret ? ret : genphy_c45_read_eee_abilities(phydev);
>  }

[Severity: High]
Does this code call genphy_c45_read_eee_abilities() while the PHY is
potentially left in the FIBER register space?

When yt8521_get_features() calls yt8521_get_features_paged(), that function
temporarily switches to the requested page (e.g. UTP) but restores the old
hardware page before returning. If the PHY was previously operating on a
fiber link, the restored page will be the FIBER space.

genphy_c45_read_eee_abilities() performs MMD indirect accesses using MII
registers 13 and 14.

If the PHY is currently in the FIBER page, won't these MMD accesses hit the
FIBER space instead of the UTP space where the MMD registers actually reside?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710080018.1070789-1-xiaoning.wang@oss.nxp.com?part=1