[PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node

Jonas Karlman posted 3 patches 2 months, 1 week ago
[PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
Posted by Jonas Karlman 2 months, 1 week ago
The dt-bindings schema for Realtek switches for unmanaged switches
contains a restriction on use of a mdio child OF node for MDIO-connected
switches, i.e.:

  if:
    required:
      - reg
  then:
    not:
      required:
        - mdio
    properties:
      mdio: false

However, the driver currently requires the existence of a mdio child OF
node to successfully probe and properly function.

Relax the requirement of a mdio child OF node and assign the dsa_switch
user_mii_bus to allow a MDIO-connected switch to probe and function
when a mdio child OF node is missing.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/net/dsa/realtek/rtl83xx.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c
index 9a05616acea8..47a09b27c4fa 100644
--- a/drivers/net/dsa/realtek/rtl83xx.c
+++ b/drivers/net/dsa/realtek/rtl83xx.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 
+#include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/of_mdio.h>
@@ -64,7 +65,7 @@ static int rtl83xx_user_mdio_write(struct mii_bus *bus, int addr, int regnum,
  * @ds: DSA switch associated with this user_mii_bus
  *
  * Registers the MDIO bus for built-in Ethernet PHYs, and associates it with
- * the mandatory 'mdio' child OF node of the switch.
+ * the optional 'mdio' child OF node of the switch.
  *
  * Context: Can sleep.
  * Return: 0 on success, negative value for failure.
@@ -75,11 +76,14 @@ int rtl83xx_setup_user_mdio(struct dsa_switch *ds)
 	struct device_node *mdio_np;
 	struct mii_bus *bus;
 	int ret = 0;
+	int irq;
+	int i;
 
 	mdio_np = of_get_child_by_name(priv->dev->of_node, "mdio");
-	if (!mdio_np) {
-		dev_err(priv->dev, "no MDIO bus node\n");
-		return -ENODEV;
+	if (mdio_np && !of_device_is_available(mdio_np)) {
+		dev_err(priv->dev, "no available MDIO bus node\n");
+		ret = -ENODEV;
+		goto err_put_node;
 	}
 
 	bus = devm_mdiobus_alloc(priv->dev);
@@ -95,6 +99,20 @@ int rtl83xx_setup_user_mdio(struct dsa_switch *ds)
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s:user_mii", dev_name(priv->dev));
 	bus->parent = priv->dev;
 
+	if (!mdio_np) {
+		ds->user_mii_bus = bus;
+		bus->phy_mask = ~ds->phys_mii_mask;
+
+		if (priv->irqdomain) {
+			for (i = 0; i < priv->num_ports; i++) {
+				irq = irq_find_mapping(priv->irqdomain, i);
+				if (irq < 0)
+					continue;
+				bus->irq[i] = irq;
+			}
+		}
+	}
+
 	ret = devm_of_mdiobus_register(priv->dev, bus, mdio_np);
 	if (ret) {
 		dev_err(priv->dev, "unable to register MDIO bus %s\n",
-- 
2.50.1
Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
Posted by Andrew Lunn 2 months, 1 week ago
On Sun, Jul 27, 2025 at 06:02:59PM +0000, Jonas Karlman wrote:
> The dt-bindings schema for Realtek switches for unmanaged switches
> contains a restriction on use of a mdio child OF node for MDIO-connected
> switches, i.e.:
> 
>   if:
>     required:
>       - reg
>   then:
>     not:
>       required:
>         - mdio
>     properties:
>       mdio: false
> 
> However, the driver currently requires the existence of a mdio child OF
> node to successfully probe and properly function.
> 
> Relax the requirement of a mdio child OF node and assign the dsa_switch
> user_mii_bus to allow a MDIO-connected switch to probe and function
> when a mdio child OF node is missing.
 
I could be getting this wrong.... Maybe Linus knows more.

It could be the switch does not have its own separate MDIO bus just
for its internal PHYs. They just appear on the parent mdio bus. So
you represent this with:

&mdio0 {
	reset-delay-us = <25000>;
	reset-gpios = <&gpio4 RK_PC2 GPIO_ACTIVE_LOW>;
	reset-post-delay-us = <100000>;

	phy0: ethernet-phy@0 {
		reg = <0>;
	};

	phy1: ethernet-phy@1 {
		reg = <0>;
	};


        ethernet-switch@1d {
               compatible = "realtek,rtl8365mb";
               reg = <0x1d>;
               pinctrl-names = "default";
               pinctrl-0 = <&rtl8367rb_eint>;

               ethernet-ports {
                       #address-cells = <1>;
                       #size-cells = <0>;

                       ethernet-port@0 {
                               reg = <0>;
                               label = "wan";
			       phy-handle = <phy0>;
                       };

                       ethernet-port@1 {
                               reg = <1>;
                               label = "lan1";
			       phy-handle = <phy1>;
                       };

If this is correct, you should not need any driver or DT binding
changes.

	Andrew
Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
Posted by Jonas Karlman 2 months, 1 week ago
Hi Andrew,

On 7/27/2025 9:09 PM, Andrew Lunn wrote:
> On Sun, Jul 27, 2025 at 06:02:59PM +0000, Jonas Karlman wrote:
>> The dt-bindings schema for Realtek switches for unmanaged switches
>> contains a restriction on use of a mdio child OF node for MDIO-connected
>> switches, i.e.:
>>
>>   if:
>>     required:
>>       - reg
>>   then:
>>     not:
>>       required:
>>         - mdio
>>     properties:
>>       mdio: false
>>
>> However, the driver currently requires the existence of a mdio child OF
>> node to successfully probe and properly function.
>>
>> Relax the requirement of a mdio child OF node and assign the dsa_switch
>> user_mii_bus to allow a MDIO-connected switch to probe and function
>> when a mdio child OF node is missing.
>  
> I could be getting this wrong.... Maybe Linus knows more.
> 
> It could be the switch does not have its own separate MDIO bus just
> for its internal PHYs. They just appear on the parent mdio bus. So
> you represent this with:
> 
> &mdio0 {
> 	reset-delay-us = <25000>;
> 	reset-gpios = <&gpio4 RK_PC2 GPIO_ACTIVE_LOW>;
> 	reset-post-delay-us = <100000>;
> 
> 	phy0: ethernet-phy@0 {
> 		reg = <0>;
> 	};
> 
> 	phy1: ethernet-phy@1 {
> 		reg = <0>;
> 	};
> 
> 
>         ethernet-switch@1d {
>                compatible = "realtek,rtl8365mb";
>                reg = <0x1d>;
>                pinctrl-names = "default";
>                pinctrl-0 = <&rtl8367rb_eint>;
> 
>                ethernet-ports {
>                        #address-cells = <1>;
>                        #size-cells = <0>;
> 
>                        ethernet-port@0 {
>                                reg = <0>;
>                                label = "wan";
> 			       phy-handle = <phy0>;
>                        };
> 
>                        ethernet-port@1 {
>                                reg = <1>;
>                                label = "lan1";
> 			       phy-handle = <phy1>;
>                        };
> 
> If this is correct, you should not need any driver or DT binding
> changes.

Something like above does not seem to work, I get following:

  mdio_bus stmmac-0: MDIO device at address 0 is missing.
  mdio_bus stmmac-0: MDIO device at address 1 is missing.
  mdio_bus stmmac-0: MDIO device at address 2 is missing.
  mdio_bus stmmac-0: MDIO device at address 3 is missing.

  rtl8365mb-mdio stmmac-0:1d: found an RTL8367RB-VB switch
  rtl8365mb-mdio stmmac-0:1d: configuring for fixed/rgmii link mode
  rtl8365mb-mdio stmmac-0:1d wan (uninitialized): failed to connect to PHY: -ENODEV
  rtl8365mb-mdio stmmac-0:1d: Link is Up - 1Gbps/Full - flow control off
  rtl8365mb-mdio stmmac-0:1d wan (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 0
  rtl8365mb-mdio stmmac-0:1d lan1 (uninitialized): failed to connect to PHY: -ENODEV
  rtl8365mb-mdio stmmac-0:1d lan1 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 1
  rtl8365mb-mdio stmmac-0:1d lan2 (uninitialized): failed to connect to PHY: -ENODEV
  rtl8365mb-mdio stmmac-0:1d lan2 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 2
  rtl8365mb-mdio stmmac-0:1d lan3 (uninitialized): failed to connect to PHY: -ENODEV
  rtl8365mb-mdio stmmac-0:1d lan3 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 3

And without an explicit 'mdio' child node the driver currently fails to
load and the switch is never registered:

  rtl8365mb-mdio stmmac-0:1d: found an RTL8367RB-VB switch
  rtl8365mb-mdio stmmac-0:1d: no MDIO bus node
  rtl8365mb-mdio stmmac-0:1d: could not set up MDIO bus
  rtl8365mb-mdio stmmac-0:1d: error -ENODEV: unable to register switch

With a 'mdio' child node 'make CHECK_DTBS=y' report something like:

  rockchip/rk3528-radxa-e24c.dtb: ethernet-switch@1d (realtek,rtl8365mb): mdio: False schema does not allow { [snip] }
        from schema $id: http://devicetree.org/schemas/net/dsa/realtek.yaml#

So something should probably be changed, the driver, dt-bindings or
possible both.

With this patch the last example in net/dsa/realtek.yaml can be made to
work. The example the ethernet-switch node in next patch is based on,
and something that closely matches how mediatek,mt7530 is described.

Please let me know if you want me to drop this and instead try to update
the dt-bindings and use a more verbose ethernet-switch node.

Regards,
Jonas

> 
> 	Andrew
Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
Posted by Andrew Lunn 2 months, 1 week ago
> Something like above does not seem to work, I get following:
> 
>   mdio_bus stmmac-0: MDIO device at address 0 is missing.
>   mdio_bus stmmac-0: MDIO device at address 1 is missing.
>   mdio_bus stmmac-0: MDIO device at address 2 is missing.
>   mdio_bus stmmac-0: MDIO device at address 3 is missing.

O.K, So i was wrong.

> And without an explicit 'mdio' child node the driver currently fails to
> load and the switch is never registered:
> 
>   rtl8365mb-mdio stmmac-0:1d: found an RTL8367RB-VB switch
>   rtl8365mb-mdio stmmac-0:1d: no MDIO bus node
>   rtl8365mb-mdio stmmac-0:1d: could not set up MDIO bus
>   rtl8365mb-mdio stmmac-0:1d: error -ENODEV: unable to register switch

So, not having an MDIO node was a long time ago seen as a short cut
for when there was a clear 1:1 mapping between port number and PHY bus
address. It still works, but it is somewhat deprecated. It also seems
like it is become more normal to need additional properties, like what
interrupt the PHY is using, the LEDs it has, etc.

> 
> With a 'mdio' child node 'make CHECK_DTBS=y' report something like:
> 
>   rockchip/rk3528-radxa-e24c.dtb: ethernet-switch@1d (realtek,rtl8365mb): mdio: False schema does not allow { [snip] }
>         from schema $id: http://devicetree.org/schemas/net/dsa/realtek.yaml#
> 
> So something should probably be changed, the driver, dt-bindings or
> possible both.

I think you should allow an MDIO node in DT. This is the only DSA
driver that i know of which does not allow it.

	Andrew
Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
Posted by Jonas Karlman 2 months, 1 week ago
Hi Andrew,

On 7/28/2025 12:09 AM, Andrew Lunn wrote:
>> Something like above does not seem to work, I get following:
>>
>>   mdio_bus stmmac-0: MDIO device at address 0 is missing.
>>   mdio_bus stmmac-0: MDIO device at address 1 is missing.
>>   mdio_bus stmmac-0: MDIO device at address 2 is missing.
>>   mdio_bus stmmac-0: MDIO device at address 3 is missing.
> 
> O.K, So i was wrong.
> 
>> And without an explicit 'mdio' child node the driver currently fails to
>> load and the switch is never registered:
>>
>>   rtl8365mb-mdio stmmac-0:1d: found an RTL8367RB-VB switch
>>   rtl8365mb-mdio stmmac-0:1d: no MDIO bus node
>>   rtl8365mb-mdio stmmac-0:1d: could not set up MDIO bus
>>   rtl8365mb-mdio stmmac-0:1d: error -ENODEV: unable to register switch
> 
> So, not having an MDIO node was a long time ago seen as a short cut
> for when there was a clear 1:1 mapping between port number and PHY bus
> address. It still works, but it is somewhat deprecated. It also seems
> like it is become more normal to need additional properties, like what
> interrupt the PHY is using, the LEDs it has, etc.

Sure, no problem to change to a more verbose DT even when there is a
clear 1:1 mapping between port, phy and interrupt here.

When it comes to having the switch being described as an interrupt
controller in the DT is also very wrong, the switch only consume a
single HW interrupt. The fact that the driver creates virtual irq for
each port is purely a software construct and is not something that
should be reflected in the DT.

Possible something for a future series to clean up.

> 
>>
>> With a 'mdio' child node 'make CHECK_DTBS=y' report something like:
>>
>>   rockchip/rk3528-radxa-e24c.dtb: ethernet-switch@1d (realtek,rtl8365mb): mdio: False schema does not allow { [snip] }
>>         from schema $id: http://devicetree.org/schemas/net/dsa/realtek.yaml#
>>
>> So something should probably be changed, the driver, dt-bindings or
>> possible both.
> 
> I think you should allow an MDIO node in DT. This is the only DSA
> driver that i know of which does not allow it.

Sure, I will drop this driver change and instead update the dt-binding
to allow use of a mdio node when the reg prop is defined.

Regards,
Jonas

> 
> 	Andrew
Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
Posted by Andrew Lunn 2 months, 1 week ago
> When it comes to having the switch being described as an interrupt
> controller in the DT is also very wrong, the switch only consume a
> single HW interrupt. The fact that the driver creates virtual irq for
> each port is purely a software construct and is not something that
> should be reflected in the DT.

I think that is not always clear cut. Switches can be considered SoC
of their own. They have multiple hardware blocks, which can be
described independent, just like a traditional SoC and its .dtsi
file. The switch blocks can then be connected together in the same way
SoCs are.

I've not looked at this particular switch driver, but the Marvell
switches have a similar single interrupt output pin connected to the
host SoC. Within the switch, there are at least two cascaded interrupt
controllers. We implement standard Linux interrupt controllers for
these. That allows us to use standard DT properties to link the
internal PHY interrupts to these interrupt controllers.

	 Andrew
Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
Posted by Jonas Karlman 2 months, 1 week ago
Hi Andrew,

On 7/28/2025 5:40 PM, Andrew Lunn wrote:
>> When it comes to having the switch being described as an interrupt
>> controller in the DT is also very wrong, the switch only consume a
>> single HW interrupt. The fact that the driver creates virtual irq for
>> each port is purely a software construct and is not something that
>> should be reflected in the DT.
> 
> I think that is not always clear cut. Switches can be considered SoC
> of their own. They have multiple hardware blocks, which can be
> described independent, just like a traditional SoC and its .dtsi
> file. The switch blocks can then be connected together in the same way
> SoCs are.

I guess you are correct, thanks for clarifying this :-)

> I've not looked at this particular switch driver, but the Marvell
> switches have a similar single interrupt output pin connected to the
> host SoC. Within the switch, there are at least two cascaded interrupt
> controllers. We implement standard Linux interrupt controllers for
> these. That allows us to use standard DT properties to link the
> internal PHY interrupts to these interrupt controllers.

Makes sense, I will describe the phy interrupts of the switch in a v2.

Regards,
Jonas

> 
> 	 Andrew