Re: [PATCH net-next v3 1/3] net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] The RTL8365MB-VC phy_driver entry (PHY ID 0x001cc942) is pointed
  at genphy_read_mmd_c45()/genphy_write_mmd_c45(), which go straight to…

--

net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access

This points the RTL8365MB-VC phy_driver entry (PHY ID 0x001cc942) at
genphy_read_mmd_c45()/genphy_write_mmd_c45() so that phylib MMD accesses
are issued as C45 transfers to the Realtek DSA driver, which maps them
onto the switch OCP registers.

> diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> index 97b0b67b9900c..b55691681df6e 100644
> --- a/drivers/net/phy/realtek/realtek_main.c
> +++ b/drivers/net/phy/realtek/realtek_main.c
> @@ -3338,6 +3338,8 @@ static struct phy_driver realtek_drvs[] = {
>  	}, {
>  		PHY_ID_MATCH_EXACT(0x001cc942),
>  		.name		= "RTL8365MB-VC Gigabit Ethernet",
> +		.read_mmd	= genphy_read_mmd_c45,
> +		.write_mmd	= genphy_write_mmd_c45,

[Severity: Low]
This isn't a bug, but the commit message describes the DSA-side mapping in
the present tense:

  "the realtek DSA driver maps those MMD requests onto OCP and exposes them
   over C45"

At this commit, rtl83xx_setup_user_mdio() in
drivers/net/dsa/realtek/rtl83xx.c only installs the C22 accessors:

	bus->read = rtl83xx_user_mdio_read;
	bus->write = rtl83xx_user_mdio_write;

The read_c45/write_c45 callbacks and the MMD to OCP mapping arrive later in
the series, in "net: dsa: realtek: rtl8365mb: add EEE support".

genphy_read_mmd_c45()/genphy_write_mmd_c45() go straight to
__mdiobus_c45_read()/__mdiobus_c45_write() with no C22 fallback, and those
return -EOPNOTSUPP when bus->read_c45 is NULL:

	if (bus->read_c45)
		retval = bus->read_c45(bus, addr, devad, regnum);
	else
		retval = -EOPNOTSUPP;

so every MMD access on these ports fails until the later patch lands.

The effect at this intermediate commit looks benign: the only MMD access
that actually runs is genphy_c45_read_eee_abilities() from
genphy_read_abilities(), whose error is explicitly discarded:

	/* This is optional functionality. If not supported, we may get an error
	 * which should be ignored.
	 */
	genphy_c45_read_eee_abilities(phydev);

and phy_check_link_status() only treats genphy_c45_eee_is_active() as a
boolean (err > 0), so a failure just leaves eee_active/enable_tx_lpi false.

Would it be worth wording that sentence as a forward reference to the later
patch, so the commit does not claim an in-tree mapping that only exists
after the following patch in the series?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916090102.349157-1-o.rempel%40pengutronix.de