| Message ID | 20240430-b4-for-netnext-mt7530-use-switch-mdio-bus-for-phy-muxing-v2-1-9104d886d0db@arinc9.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d8dcf5bd6d0eace9f7c1daa14b63b3925b09d033 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] net: dsa: mt7530: detect PHY muxing when PHY is defined on switch MDIO bus | expand |
On Tue, Apr 30, 2024 at 08:01:33AM +0300, Arınç ÜNAL via B4 Relay wrote: > From: Arınç ÜNAL <arinc.unal@arinc9.com> > > 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@arinc9.com> Reviewed-by: Daniel Golle <daniel@makrotopia.org> > --- > 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@arinc9.com > --- > 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. > ret = of_get_phy_mode(mac_np, &interface); > if (ret && ret != -ENODEV) { > of_node_put(mac_np); > > --- > base-commit: 5c4c0edca68a5841a8d53ccd49596fe199c8334c > change-id: 20240429-b4-for-netnext-mt7530-use-switch-mdio-bus-for-phy-muxing-586269371c55 > > Best regards, > -- > Arınç ÜNAL <arinc.unal@arinc9.com> > > >
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@arinc9.com> >> >> 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@arinc9.com> > > Reviewed-by: Daniel Golle <daniel@makrotopia.org> > >> --- >> 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@arinc9.com >> --- >> 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ç
Hello: This patch was applied to netdev/net-next.git (main) by Paolo Abeni <pabeni@redhat.com>: On Tue, 30 Apr 2024 08:01:33 +0300 you wrote: > From: Arınç ÜNAL <arinc.unal@arinc9.com> > > 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. > > [...] Here is the summary with links: - [net-next,v2] net: dsa: mt7530: detect PHY muxing when PHY is defined on switch MDIO bus https://git.kernel.org/netdev/net-next/c/d8dcf5bd6d0e You are awesome, thank you!
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) { ret = of_get_phy_mode(mac_np, &interface); if (ret && ret != -ENODEV) { of_node_put(mac_np);