Re: [PATCH net-next v2 5/5] net: mdio: realtek-rtl9300: Add support for RTL931x

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver returns non-standard error codes (-ENOENT and -ENXIO), causing the MDIO core to incorrectly abort bus registration
--

commit 75cd5960a2d7e0f2a11ca0ab524bcf1b9773e72c
Author: Markus Stockhausen <markus.stockhausen@gmx.de>

net: mdio: realtek-rtl9300: Add support for RTL931x

This commit prepares the MDIO driver for multiple device support and adds
all required bits for the RTL931x series, including expanding port limits
and supporting different controller layouts.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 33c00b6ba8f88..dbd52f9219099 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c

[ ... ]

> @@ -294,6 +319,60 @@ static int otto_emdio_9300_write_c45(struct mii_bus *bus, int port,
>  	return otto_emdio_write_cmd(bus, RTL9300_PHY_CTRL_TYPE_C45, &cmd_data);
>  }

[ ... ]

> +static int otto_emdio_9310_write_c45(struct mii_bus *bus, int port,
> +				     int dev_addr, int regnum, u16 value)
> +{
> +	struct otto_emdio_cmd_regs cmd_data = {
> +		.c45_data	= FIELD_PREP(PHY_CTRL_MMD_DEVAD, dev_addr) |
> +				  FIELD_PREP(PHY_CTRL_MMD_REG, regnum),
> +		.io_data	= FIELD_PREP(RTL9310_PHY_CTRL_INDATA, value),
> +		.port_mask_high	= (u32)(BIT_ULL(port) >> 32),
> +		.port_mask_low	= (u32)(BIT_ULL(port)),
> +	};
> +
> +	return otto_emdio_write_cmd(bus, RTL9310_PHY_CTRL_TYPE_C45, &cmd_data);
> +}
> +
>  static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
>  {
>  	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);

[Severity: High]
This is a pre-existing issue, but I noticed the driver returns non-standard
error codes that might break MDIO bus registration.

Looking at otto_emdio_phy_to_port(), it returns -ENOENT when a PHY ID isn't
found in the valid ports bitmap. Similarly, otto_emdio_run_cmd() returns
-ENXIO when the hardware indicates a command failure. Both of these
functions are called during MDIO read/write operations like
otto_emdio_read_c22().

When these non-standard errors bubble up, the MDIO core in get_phy_c22_id()
treats any error code that is not -EIO or -ENODEV as a fatal -EIO error:

get_phy_c22_id() {
        ...
        phy_reg = mdiobus_read(bus, addr, MII_PHYSID1);
        if (phy_reg < 0) {
                return (phy_reg == -EIO || phy_reg == -ENODEV) ? -ENODEV : -EIO;
        }
        ...
}

When get_phy_device() returns -EIO, of_mdiobus_register() treats it as a
fatal bus error and aborts the entire MDIO bus registration.

Could this cause the entire switch's network capabilities to fail if just a
single PHY is unpopulated or unresponsive? Would it be safer to return
standard codes like -ENODEV or 0xffff from the driver's read/write handlers
so the MDIO core can safely skip missing devices and continue scanning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609052856.3142922-1-markus.stockhausen@gmx.de?part=5