Re: [PATCH v7 phy-next 21/27] net: renesas: rswitch: include PHY provider header

Vladimir Oltean
2026-05-05T09:24:44Z

Sashiko says:
> commit 4e15b6920cd2b2dfce685835fa09c66664407579
> Author: Vladimir Oltean <vladimir.oltean@nxp.com>
> net: renesas: rswitch: include PHY provider header
>
> This commit includes the PHY provider header in the rswitch driver as a
> workaround to avoid breaking the build when struct phy becomes opaque.
> It allows the driver to continue accessing internal PHY fields while
> acknowledging that the driver's PHY API call sequence needs a complete
> redesign.
>
> > 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 */
>
> Does preserving the ability to check the provider's global reference counts
> (power_count and init_count) via this internal header leave the driver
> vulnerable to data races?
>
> Since these fields are protected by the PHY provider's internal phy->mutex,
> reading them locklessly can create a time-of-check to time-of-use
> vulnerability. The count could be altered by a concurrent operation before
> the mutex is acquired inside phy_power_off() or phy_exit().
>
> Additionally, if the PHY is shared across multiple ports (like in QSGMII
> mode), would checking the global reference count instead of tracking the
> consumer's local vote cause a port to observe a non-zero count from
> another port's vote? This could lead to an incorrect phy_power_off()
> call, prematurely decrementing the shared refcount and powering down the
> PHY while it is still in active use by other ports.
>
> While the commit message notes this is a known issue preserved as a
> workaround until a complete redesign, could retaining these unlocked
> reads cause a regression in power vote tracking in the interim?
>
> > #include "rswitch.h"
> > #include "rswitch_l2.h"

I will ignore this comment. Inaction will leave the driver in exactly
the same state as before. Not great, true, but I don't see how not
changing anything could cause a regression.