Re: [PATCH net-next] net: dsa: mt7530: register OF node for internal MDIO bus
From: Arınç ÜNAL
Date: Thu Dec 28 2023 - 11:59:18 EST
On 27.12.2023 23:02, Vladimir Oltean wrote:
On Wed, Dec 27, 2023 at 10:51:08PM +0300, Arınç ÜNAL wrote:
I didn't realise ds->user_mii_bus is also used to store irq mapping for
each PHY.
It needs to, if the MDIO bus does not have an OF description through
which PHYs can have an 'interrupts' property. But if there is an OF
description for the MDIO bus and the PHYs, I think it is strange to
expect PHYs to have interrupts if they aren't described in OF.
Should we agree that user_mii_bus is needed for all cases or make
another way to store the irq mappings?
I looked at the upstream device trees:
- users of arch/mips/boot/dts/ralink/mt7621.dtsi
- arch/arm/boot/dts/mediatek/mt7623n-bananapi-bpi-r2.dts
- arch/arm/boot/dts/mediatek/mt7623n-rfb-emmc.dts
- arch/arm/boot/dts/mediatek/mt7623a.dtsi
- arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
- arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
- arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
- arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
and without exception, none of these have the MDIO bus described in OF.
I'm not sure about other device trees. But it may well be that the
situation where "MDIO buses present in OF need an IRQ mapping for their
PHYs" does not need to be handled.
As Daniel stated on a previous submission of this patch, being able to
reference the PHYs on the switch MDIO bus is mandatory on MT7988 as
calibration data from NVMEM for each PHY is required, so defining the MDIO
bus is required to support MT7988. Therefore, we should support interrupts
on device trees with the switch MDIO bus defined.
The implementation below follows this logic:
No switch MDIO bus defined: Register the MDIO bus, set the interrupts for
PHYs if "interrupt-controller" is defined at the switch node.
Switch MDIO bus defined: Register the MDIO bus, set the interrupts for PHYs
if ["interrupt-controller" is defined at the switch node and "interrupts"
is defined at the PHY nodes under the switch MDIO bus node].
I think this approach fits your description so I'd like to agree that this
should be the way for all DSA subdrivers. Please let me know what you
think.
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 391c4dbdff42..bbd230a73ead 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2155,15 +2155,21 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
{
struct dsa_switch *ds = priv->ds;
struct device *dev = priv->dev;
+ struct device_node *np, *mnp;
struct mii_bus *bus;
static int idx;
int ret;
+ np = priv->dev->of_node;
+ mnp = of_get_child_by_name(np, "mdio");
+
bus = devm_mdiobus_alloc(dev);
if (!bus)
return -ENOMEM;
- ds->user_mii_bus = bus;
+ if (mnp == NULL)
+ ds->user_mii_bus = bus;
+
bus->priv = priv;
bus->name = KBUILD_MODNAME "-mii";
snprintf(bus->id, MII_BUS_ID_SIZE, KBUILD_MODNAME "-%d", idx++);
@@ -2174,10 +2180,11 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
bus->parent = dev;
bus->phy_mask = ~ds->phys_mii_mask;
- if (priv->irq)
+ if (priv->irq && mnp == NULL)
mt7530_setup_mdio_irq(priv);
- ret = devm_mdiobus_register(dev, bus);
+ ret = devm_of_mdiobus_register(dev, bus, mnp);
+ of_node_put(mnp);
if (ret) {
dev_err(dev, "failed to register MDIO bus: %d\n", ret);
if (priv->irq)
With this device tree:
switch {
interrupt-controller;
}
[ 1.420534] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=17)
[ 1.433224] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=18)
[ 1.445338] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=19)
[ 1.457472] mt7530-mdio mdio-bus:1f lan4 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=20)
[ 1.469587] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=21)
With this device tree:
switch {
interrupt-controller;
mdio {
phy {
reg = <0>;
}
}
}
[ 1.413101] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL)
[ 1.429954] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL)
[ 1.443704] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL)
[ 1.455876] mt7530-mdio mdio-bus:1f lan4 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL)
[ 1.468079] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL)
With this device tree:
switch {
interrupt-controller;
mdio {
phy {
reg = <0>;
interrupts = <0>;
}
}
}
[ 1.420534] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=17)
[ 1.433224] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=18)
[ 1.445338] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=19)
[ 1.457472] mt7530-mdio mdio-bus:1f lan4 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=20)
[ 1.469587] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=21)
Arınç