Re: [PATCH net-next v15 10/13] net: mdio: realtek-rtl9300: Open up C22 and C45 space in parallel

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] MDIO bus read and write operations incorrectly return non-standard error codes (-ENOENT for unmapped ports and -ENXIO on NACK), which causes the MDIO core to fatally abort bus scanning and registration.
--

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

net: mdio: realtek-rtl9300: Open up C22 and C45 space in parallel
    
Until now the MDIO driver supports either access to the C22 or to
the C45 address space of the PHYs. This is due to the fact that
the polling configuration favours one of the address spaces and
limits access to the other space. E.g. when polling is set to
C22 most of the C45 space can not be accessed.
    
There are however some exceptions from that. EEE register access
is allowed independently from the polling mode [1]. As the
downstream driver already allows parallel C22/C45 access [2] do
this in upstream too.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index ae7ba86e90b9a..b90972c1dc6a2 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -688,13 +688,10 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
>  		return -ENOMEM;
>  
>  	bus->name = "Realtek Switch MDIO Bus";
> -	if (priv->smi_bus_is_c45[mdio_bus]) {
> -		bus->read_c45 = otto_emdio_read_c45;
> -		bus->write_c45 = otto_emdio_write_c45;
> -	} else {
> -		bus->read = otto_emdio_read_c22;
> -		bus->write = otto_emdio_write_c22;
> -	}
> +	bus->read_c45 = otto_emdio_read_c45;
> +	bus->write_c45 = otto_emdio_write_c45;
> +	bus->read = otto_emdio_read_c22;
> +	bus->write = otto_emdio_write_c22;

[Severity: High]
This is a pre-existing issue, but I noticed that MDIO bus read and write
operations incorrectly return non-standard error codes (-ENOENT for unmapped
ports and -ENXIO on NACK), which causes the MDIO core to fatally abort bus
scanning and registration.

For example, when a PHY address is unmapped, -ENOENT is propagated to the MDIO
core during a C45 or C22 bus read:

drivers/net/mdio/mdio-realtek-rtl9300.c:otto_emdio_phy_to_port() {
    ...
    return -ENOENT;
}

drivers/net/mdio/mdio-realtek-rtl9300.c:otto_emdio_read_c45() {
    ...
    port = otto_emdio_phy_to_port(bus, phy_id);
    if (port < 0)
        return port;
    ...
}

And when the PHY NACKs the transaction, it returns -ENXIO instead of
successfully completing a read with 0xffff:

drivers/net/mdio/mdio-realtek-rtl9300.c:otto_emdio_run_cmd() {
    ...
    return cmdstate & info->cmd_fail ? -ENXIO : 0;
}

The get_phy_c22_id() and get_phy_c45_ids() core functions translate any unknown
error (like -ENOENT or -ENXIO) to -EIO. This -EIO error propagates to
__of_mdiobus_parse_phys() and mdiobus_scan_bus_c22()/mdiobus_scan_bus_c45(),
which treat it as a fatal bus error, aborting the scanning and registration
process. Consequently, no PHYs on that bus will be functional.

To comply with the MDIO subsystem and allow fallback scanning to continue, read
functions must return 0xffff (simulating line pull-ups) or explicitly return
-ENODEV (or -EIO, which is safely translated to -ENODEV by the core) when a
device is missing or unmapped.

Should these functions be updated to return compliant error codes to prevent the
MDIO core from fatally aborting bus scanning?

>  	bus->parent = dev;
>  	bus->notify_phy_attach = otto_emdio_notify_phy_attach;
>  	bus->notify_phy_detach = otto_emdio_notify_phy_detach;

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