Re: [PATCH v8 phy-next 24/31] net: renesas: rswitch: include PHY provider header

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Pre-existing data races and flawed power/initialization state tracking (API abuse)
--

commit 2d0a78469a71c1f0b6d4b9cc19ca73b858d63917
Author: Vladimir Oltean <vladimir.oltean@nxp.com>

net: renesas: rswitch: include PHY provider header

As a PHY consumer driver, the Renesas rswitch dereferences internal fields
of struct phy, which is being made an opaque pointer. This commit works
around the new boundary by explicitly including the internal PHY provider
header to keep the driver working as before.

Link: https://lore.kernel.org/linux-phy/20260211194541.cdmibrpfn6ej6e74@skbuf/

> diff --git a/drivers/net/ethernet/renesas/rswitch_main.c b/drivers/net/ethernet/renesas/rswitch_main.c
> index 6fe964816322a..132be5f15073c 100644
> --- a/drivers/net/ethernet/renesas/rswitch_main.c
> +++ b/drivers/net/ethernet/renesas/rswitch_main.c
> @@ -27,6 +27,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/sys_soc.h>
>  
> +#include "../../../phy/phy-provider.h" /* FIXME */

This isn't a new regression, but while the commit message notes this driver
needs a complete rethink, actively preserving this behavior by including the
provider header bypasses the boundary and leaves a race condition and logic
error intact.

Could the driver track its own power and initialization states locally instead
of relying on the internal global state of the generic PHY framework?

In rswitch_adjust_link(), the driver reads rdev->serdes->power_count
without holding phy->mutex:

rswitch_main.c:rswitch_adjust_link() {
...
		if (phydev->link)
			phy_power_on(rdev->serdes);
		else if (rdev->serdes->power_count)
			phy_power_off(rdev->serdes);
...
}

Can this lockless access create a TOCTOU (Time-Of-Check to Time-Of-Use)
data race?

Furthermore, because multiple ports can share the same PHY in a QSGMII setup,
does using the global power_count to deduce this port's state cause logic
errors?

If Port A's phy_power_on() fails, but Port B subsequently turns on the PHY,
Port A will read power_count > 0. When Port A's link goes down, won't it
incorrectly call phy_power_off(), stealing Port B's power vote and turning
off the shared PHY while it is still in use by Port B?

Similarly, in renesas_eth_sw_suspend(), init_count is read without a lock:

rswitch_main.c:renesas_eth_sw_suspend() {
...
		if (priv->rdev[i]->serdes->init_count)
			phy_exit(priv->rdev[i]->serdes);
...
}

Would it be safer to add local boolean flags (e.g., bool serdes_power_on or
bool serdes_init_done) directly to struct rswitch_device to track successful
calls?

>  #include "rswitch.h"
>  #include "rswitch_l2.h"
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505100523.1922388-1-vladimir.oltean@nxp.com?part=24