Re: [PATCH net-next 5/8] net: mdio: realtek-rtl9300: Add c45 over c22 mitigation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] State Machine Desynchronization and Hardware Polling Corruption on MDIO Error
--

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

net: mdio: realtek-rtl9300: Add c45 over c22 mitigation
    
Enhances the RTL9300 MDIO driver to detect the 4-step c45-over-c22 MMD
sequence. Disables hardware polling during the sequence to prevent concurrent
access from breaking the kernel's read/writes. Adds a state machine to track
sequence progress.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index a7fd075947b67..e206ee3e2b1cc 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
[ ... ]
> @@ -254,6 +257,43 @@ static int otto_emdio_set_port_polling(struct otto_emdio_priv *priv, int port, b
>  				  BIT(port % 32), active);
>  }
>  
> +static int otto_emdio_mmd_prefix(struct otto_emdio_priv *priv, int port, int regnum)
> +{
> +	u8 newstate, *state = &priv->mmd_state[port];
> +	int expected, ret = 0;
> +
[ ... ]
> +	expected = (*state & 1) ? MII_MMD_DATA : MII_MMD_CTRL;
> +	newstate = regnum == expected ? *state + 1 : 0;
> +
> +	if (newstate == 1 || newstate < *state)
> +		ret = otto_emdio_set_port_polling(priv, port, !newstate);
> +	*state = newstate;
> +
> +	return ret;
> +}
> +
> +static int otto_emdio_mmd_postfix(struct otto_emdio_priv *priv, int port, int regnum)
> +{
> +	if (priv->mmd_state[port] != 4)
> +		return 0;
> +
> +	priv->mmd_state[port] = 0;
> +
> +	return otto_emdio_set_port_polling(priv, port, true);
> +}
> +
[ ... ]
> @@ -463,7 +503,15 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
>  		if (regnum == 31)
>  			return priv->page[port];
>  
> +		ret = otto_emdio_mmd_prefix(priv, port, regnum);
> +		if (ret)
> +			return ret;
> +
>  		ret = priv->info->read_c22(bus, port, regnum, &value);
> +		if (ret)
> +			return ret;

[Severity: High]
If priv->info->read_c22() returns an error, the function returns early and
skips otto_emdio_mmd_postfix(), leaving priv->mmd_state[port] in an
intermediate state.

If an error occurs on the final step, the state is left at 4. A new MMD
sequence starting with register 13 will match the expected register logic
in otto_emdio_mmd_prefix() and increment the state to 5, then 6, etc.

Since otto_emdio_mmd_postfix() only re-enables polling when the state is
exactly 4, will hardware polling remain permanently disabled for this port
if an error occurs mid-sequence?

> +
> +		ret = otto_emdio_mmd_postfix(priv, port, regnum);
>  	}
>  
>  	return ret ? ret : value;
> @@ -472,7 +520,7 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
>  static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum, u16 value)
>  {
>  	struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
> -	int port;
> +	int port, ret;
>  
>  	port = otto_emdio_phy_to_port(bus, phy_id);
>  	if (port < 0)
> @@ -487,7 +535,15 @@ static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum, u16
>  			return 0;
>  		}
>  
> -		return priv->info->write_c22(bus, port, regnum, value);
> +		ret = otto_emdio_mmd_prefix(priv, port, regnum);
> +		if (ret)
> +			return ret;
> +
> +		ret = priv->info->write_c22(bus, port, regnum, value);
> +		if (ret)
> +			return ret;

[Severity: High]
Similarly, does returning early here on error skip otto_emdio_mmd_postfix()
and desynchronize the state machine?

> +
> +		return otto_emdio_mmd_postfix(priv, port, regnum);
>  	}
>  }

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