Re: [PATCH net-next v2] net: dsa: mt7530: detect PHY muxing when PHY is defined on switch MDIO bus

[Arınç ÜNAL]

On 06/05/2024 14:24, Daniel Golle wrote:
> On Tue, Apr 30, 2024 at 08:01:33AM +0300, Arınç ÜNAL via B4 Relay wrote:
> > From: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
> >
> > Currently, the MT7530 DSA subdriver configures the MT7530 switch to provide
> > direct access to switch PHYs, meaning, the switch PHYs listen on the MDIO
> > bus the switch listens on. The PHY muxing feature makes use of this.
> >
> > This is problematic as the PHY may be attached before the switch is
> > initialised, in which case, the PHY will fail to be attached.
> >
> > Since commit 91374ba537bd ("net: dsa: mt7530: support OF-based registration
> > of switch MDIO bus"), we can describe the switch PHYs on the MDIO bus of
> > the switch on the device tree. Extend the check to detect PHY muxing when
> > the PHY is defined on the MDIO bus of the switch on the device tree.
> >
> > When the PHY is described this way, the switch will be initialised first,
> > then the switch MDIO bus will be registered. Only after these steps, the
> > PHY will be attached.
> >
> > Signed-off-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
>
> Reviewed-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
>
> > ---
> > Changes in v2:
> > - Address the terminology on the patch log.
> > - Link to v1: https://lore.kernel.org/r/20240429-b4-for-netnext-mt7530-use-switch-mdio-bus-for-phy-muxing-v1-1-1f775983e155@xxxxxxxxxx
> > ---
> >   drivers/net/dsa/mt7530.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> > index 2b9f904a98f0..6cf21c9d523b 100644
> > --- a/drivers/net/dsa/mt7530.c
> > +++ b/drivers/net/dsa/mt7530.c
> > @@ -2483,7 +2483,8 @@ mt7530_setup(struct dsa_switch *ds)
> >   			if (!phy_node)
> >   				continue;
> >   
> > -			if (phy_node->parent == priv->dev->of_node->parent) {
> > +			if (phy_node->parent == priv->dev->of_node->parent ||
> > +			    phy_node->parent->parent == priv->dev->of_node) {
>
> I had some concerns about missing check for phy_node->parent != NULL,
> but it's impossible in practise. If phy_node exists, it will have a parent
> node as well.
>
> To be super extra safe, maybe doing
> phy_node->parent && phy_node->parent->parent == priv->dev->of_node
> would be better.

At the current state of this driver where the hardware probes on OF, I
don't see any benefit of doing this so I'm going to pass on this for now.

Arınç