linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] net: dsa: realtek: Add support for use of an optional mdio node
@ 2025-07-27 18:02 Jonas Karlman
  2025-07-27 18:02 ` [PATCH net-next 1/3] net: dsa: realtek: remove unused user_mii_bus from realtek_priv Jonas Karlman
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Jonas Karlman @ 2025-07-27 18:02 UTC (permalink / raw)
  To: Linus Walleij, Alvin Šipraga, Andrew Lunn, Vladimir Oltean
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, Heiko Stuebner, netdev, linux-rockchip,
	linux-arm-kernel, linux-kernel, Jonas Karlman

The Radxa E24C, a Rockchip RK3528A based device, uses a MDIO-connected
RTL8367RB-VB switch to expose four Ethernet ports on the device.

Trying to describe this switch in the device tree in a way that makes
it work for the driver results in dtschema complaining that use of an
mdio child OF node is not allowed.

This series relaxes the realtek dsa drivers requirement of having a mdio
child OF node to probe and instead have it register a user_mii_bus to
make it function when a mdio child OF node is missing.

Another option could also be to adjust the dt-bindings schema to allow
use of a mdio child OF node for MDIO-connected switches.

With this series dtschema is happy and the switch can work:

  rtl8365mb-mdio stmmac-0:1d: found an RTL8367RB-VB switch
  rtl8365mb-mdio stmmac-0:1d: configuring for fixed/rgmii-id link mode
  rtl8365mb-mdio stmmac-0:1d: Link is Up - 1Gbps/Full - flow control off
  rtl8365mb-mdio stmmac-0:1d wan (uninitialized): PHY [stmmac-0:1d:user_mii:00] driver [RTL8365MB-VC Gigabit Ethernet] (irq=74)
  rtl8365mb-mdio stmmac-0:1d lan1 (uninitialized): PHY [stmmac-0:1d:user_mii:01] driver [RTL8365MB-VC Gigabit Ethernet] (irq=75)
  rtl8365mb-mdio stmmac-0:1d lan2 (uninitialized): PHY [stmmac-0:1d:user_mii:02] driver [RTL8365MB-VC Gigabit Ethernet] (irq=76)
  rtl8365mb-mdio stmmac-0:1d lan3 (uninitialized): PHY [stmmac-0:1d:user_mii:03] driver [RTL8365MB-VC Gigabit Ethernet] (irq=77)
  rtl8365mb-mdio stmmac-0:1d wan: configuring for phy/gmii link mode
  rtl8365mb-mdio stmmac-0:1d wan: Link is Up - 1Gbps/Full - flow control off

The device tree changes builds on top of the "arm64: dts: rockchip: Add
Radxa E24C" series at [1].

[1] https://lore.kernel.org/r/20250727144409.327740-1-jonas@kwiboo.se

Jonas Karlman (3):
  net: dsa: realtek: remove unused user_mii_bus from realtek_priv
  net: dsa: realtek: Add support for use of an optional mdio node
  arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C

 .../boot/dts/rockchip/rk3528-radxa-e24c.dts   | 55 +++++++++++++++++++
 drivers/net/dsa/realtek/realtek.h             |  1 -
 drivers/net/dsa/realtek/rtl83xx.c             | 28 ++++++++--
 3 files changed, 77 insertions(+), 7 deletions(-)

-- 
2.50.1



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH net-next 1/3] net: dsa: realtek: remove unused user_mii_bus from realtek_priv
  2025-07-27 18:02 [PATCH net-next 0/3] net: dsa: realtek: Add support for use of an optional mdio node Jonas Karlman
@ 2025-07-27 18:02 ` Jonas Karlman
  2025-07-27 18:02 ` [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node Jonas Karlman
  2025-07-27 18:03 ` [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C Jonas Karlman
  2 siblings, 0 replies; 21+ messages in thread
From: Jonas Karlman @ 2025-07-27 18:02 UTC (permalink / raw)
  To: Linus Walleij, Alvin Šipraga, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Yao Zi, Chukun Pan, Heiko Stuebner, netdev, linux-rockchip,
	linux-arm-kernel, linux-kernel, Jonas Karlman

user_mii_bus of struct realtek_priv is not dereferenced anywhere and it
is easy to confuse it with user_mii_bus from struct dsa_switch, remove
it.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/net/dsa/realtek/realtek.h | 1 -
 drivers/net/dsa/realtek/rtl83xx.c | 2 --
 2 files changed, 3 deletions(-)

diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
index a1b2e0b529d5..7f652b245b2e 100644
--- a/drivers/net/dsa/realtek/realtek.h
+++ b/drivers/net/dsa/realtek/realtek.h
@@ -57,7 +57,6 @@ struct realtek_priv {
 	struct regmap		*map;
 	struct regmap		*map_nolock;
 	struct mutex		map_lock;
-	struct mii_bus		*user_mii_bus;
 	struct mii_bus		*bus;
 	int			mdio_addr;
 
diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c
index 2b9bd4462714..9a05616acea8 100644
--- a/drivers/net/dsa/realtek/rtl83xx.c
+++ b/drivers/net/dsa/realtek/rtl83xx.c
@@ -102,8 +102,6 @@ int rtl83xx_setup_user_mdio(struct dsa_switch *ds)
 		goto err_put_node;
 	}
 
-	priv->user_mii_bus = bus;
-
 err_put_node:
 	of_node_put(mdio_np);
 
-- 
2.50.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
  2025-07-27 18:02 [PATCH net-next 0/3] net: dsa: realtek: Add support for use of an optional mdio node Jonas Karlman
  2025-07-27 18:02 ` [PATCH net-next 1/3] net: dsa: realtek: remove unused user_mii_bus from realtek_priv Jonas Karlman
@ 2025-07-27 18:02 ` Jonas Karlman
  2025-07-27 19:09   ` Andrew Lunn
  2025-07-27 18:03 ` [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C Jonas Karlman
  2 siblings, 1 reply; 21+ messages in thread
From: Jonas Karlman @ 2025-07-27 18:02 UTC (permalink / raw)
  To: Linus Walleij, Alvin Šipraga, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Yao Zi, Chukun Pan, Heiko Stuebner, netdev, linux-rockchip,
	linux-arm-kernel, linux-kernel, Jonas Karlman

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



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-27 18:02 [PATCH net-next 0/3] net: dsa: realtek: Add support for use of an optional mdio node Jonas Karlman
  2025-07-27 18:02 ` [PATCH net-next 1/3] net: dsa: realtek: remove unused user_mii_bus from realtek_priv Jonas Karlman
  2025-07-27 18:02 ` [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node Jonas Karlman
@ 2025-07-27 18:03 ` Jonas Karlman
  2025-07-27 19:16   ` Andrew Lunn
                     ` (2 more replies)
  2 siblings, 3 replies; 21+ messages in thread
From: Jonas Karlman @ 2025-07-27 18:03 UTC (permalink / raw)
  To: Linus Walleij, Alvin Šipraga, Andrew Lunn, Vladimir Oltean,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, netdev, linux-rockchip, linux-arm-kernel,
	linux-kernel, Jonas Karlman, devicetree

The Radxa E24C has a Realtek RTL8367RB-VB switch with four usable ports
and is connected using a fixed-link to GMAC1 on the RK3528 SoC.

Add an ethernet-switch node to describe the RTL8367RB-VB switch.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
Initial testing with iperf3 showed ~930-940 Mbits/sec in one direction
and only around ~1-2 Mbits/sec in the other direction.

The RK3528 hardware design guide recommends that timing between TXCLK
and data is controlled by MAC, and timing between RXCLK and data is
controlled by PHY.

Any mix of MAC (rx/tx delay) and switch (rx/tx internal delay) did not
seem to resolve this speed issue, however dropping snps,tso seems to fix
that issue.

Unsure what is best here, should MAC or switch add the delays? Here I
just followed DT from vendor downstream tree and added rx/tx internal
delay to switch.

Vendor downstream DT also adds 'pause' to the fixed-link nodes, and this
may be something that should be added here. However, during testing flow
control always ended up being disabled so I skipped 'pause' here.

Schematics: https://dl.radxa.com/e/e24c/docs/radxa_e24c_v1200_schematic.pdf
---
 .../boot/dts/rockchip/rk3528-radxa-e24c.dts   | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e24c.dts b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e24c.dts
index 225f2b0c5339..26754ff7f4ef 100644
--- a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e24c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e24c.dts
@@ -196,6 +196,7 @@ &cpu3 {
 };
 
 &gmac1 {
+	/delete-property/ snps,tso;
 	clock_in_out = "output";
 	phy-mode = "rgmii-id";
 	phy-supply = <&avdd_rtl8367rb>;
@@ -368,6 +369,60 @@ &mdio1 {
 	reset-delay-us = <25000>;
 	reset-gpios = <&gpio4 RK_PC2 GPIO_ACTIVE_LOW>;
 	reset-post-delay-us = <100000>;
+
+	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";
+			};
+
+			ethernet-port@1 {
+				reg = <1>;
+				label = "lan1";
+			};
+
+			ethernet-port@2 {
+				reg = <2>;
+				label = "lan2";
+			};
+
+			ethernet-port@3 {
+				reg = <3>;
+				label = "lan3";
+			};
+
+			ethernet-port@6 {
+				reg = <6>;
+				ethernet = <&gmac1>;
+				label = "cpu";
+				phy-mode = "rgmii-id";
+				rx-internal-delay-ps = <2000>;
+				tx-internal-delay-ps = <2000>;
+
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+		};
+
+		interrupt-controller {
+			interrupt-parent = <&gpio1>;
+			interrupts = <RK_PC2 IRQ_TYPE_LEVEL_LOW>;
+			interrupt-controller;
+			#address-cells = <0>;
+			#interrupt-cells = <1>;
+		};
+	};
 };
 
 &pinctrl {
-- 
2.50.1



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
  2025-07-27 18:02 ` [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node Jonas Karlman
@ 2025-07-27 19:09   ` Andrew Lunn
  2025-07-27 21:52     ` Jonas Karlman
  0 siblings, 1 reply; 21+ messages in thread
From: Andrew Lunn @ 2025-07-27 19:09 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Linus Walleij, Alvin Šipraga, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, Heiko Stuebner, netdev, linux-rockchip,
	linux-arm-kernel, linux-kernel

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-27 18:03 ` [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C Jonas Karlman
@ 2025-07-27 19:16   ` Andrew Lunn
  2025-07-28 14:57     ` Jonas Karlman
  2025-07-27 19:57   ` Russell King (Oracle)
  2025-07-28 14:30   ` Chukun Pan
  2 siblings, 1 reply; 21+ messages in thread
From: Andrew Lunn @ 2025-07-27 19:16 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Linus Walleij, Alvin Šipraga, Vladimir Oltean, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, netdev, linux-rockchip, linux-arm-kernel,
	linux-kernel, devicetree

On Sun, Jul 27, 2025 at 06:03:00PM +0000, Jonas Karlman wrote:
> The Radxa E24C has a Realtek RTL8367RB-VB switch with four usable ports
> and is connected using a fixed-link to GMAC1 on the RK3528 SoC.
> 
> Add an ethernet-switch node to describe the RTL8367RB-VB switch.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> Initial testing with iperf3 showed ~930-940 Mbits/sec in one direction
> and only around ~1-2 Mbits/sec in the other direction.
> 
> The RK3528 hardware design guide recommends that timing between TXCLK
> and data is controlled by MAC, and timing between RXCLK and data is
> controlled by PHY.
> 
> Any mix of MAC (rx/tx delay) and switch (rx/tx internal delay) did not
> seem to resolve this speed issue, however dropping snps,tso seems to fix
> that issue.

It could well be that the Synopsis TSO code does not understand the
DSA headers. When it takes a big block to TCP data and segments it,
you need to have the DSA header on each segment. If it does not do
that, only the first segment has the DSA header, the switch is going
to be dropping all the other segments, causes TCP to do a lot of
retries.

> Unsure what is best here, should MAC or switch add the delays?

It should not matter. 2ns is 2ns...

	Andrew


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-27 18:03 ` [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C Jonas Karlman
  2025-07-27 19:16   ` Andrew Lunn
@ 2025-07-27 19:57   ` Russell King (Oracle)
  2025-07-28 14:30   ` Chukun Pan
  2 siblings, 0 replies; 21+ messages in thread
From: Russell King (Oracle) @ 2025-07-27 19:57 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Linus Walleij, Alvin Šipraga, Andrew Lunn, Vladimir Oltean,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, netdev, linux-rockchip, linux-arm-kernel,
	linux-kernel, devicetree

On Sun, Jul 27, 2025 at 06:03:00PM +0000, Jonas Karlman wrote:
> The Radxa E24C has a Realtek RTL8367RB-VB switch with four usable ports
> and is connected using a fixed-link to GMAC1 on the RK3528 SoC.
> 
> Add an ethernet-switch node to describe the RTL8367RB-VB switch.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> Initial testing with iperf3 showed ~930-940 Mbits/sec in one direction
> and only around ~1-2 Mbits/sec in the other direction.
> 
> The RK3528 hardware design guide recommends that timing between TXCLK
> and data is controlled by MAC, and timing between RXCLK and data is
> controlled by PHY.

Assuming RK3528 is the MAC side, then that makes sense - it's basically
suggesting that the _producer_ of the signals should appropriately skew
them.

> Any mix of MAC (rx/tx delay) and switch (rx/tx internal delay) did not
> seem to resolve this speed issue, however dropping snps,tso seems to fix
> that issue.
> 
> Unsure what is best here, should MAC or switch add the delays? Here I
> just followed DT from vendor downstream tree and added rx/tx internal
> delay to switch.

Okay. Heres'a an in-depth explanation, because I think many people need
this for MAC-to-MAC RGMII links. A MAC to PHY link:

	MAC1 ------- PHY1

The PHY mode in the MAC1 description controls the application of delays
at PHY1. This is relatively well undersood. Now, for a MAC to MAC link:

	MAC1 ------- MAC2 in a PHY

Let's say MAC2 is part of a PHY. Okay, so this is quite simple because
it's a PHY on the other end, and thus the situation above applies.

In both these cases, the MAC driver will pass the PHY interface to
phylib, which will in turn pass it to the PHY driver, which is expected
to configure the PHY appropriately.

There is a side-case, where a MAC driver is allowed to configure the
delays at its end _provided_ it then passes PHY_INTERFACE_MODE_RGMII
to phylib (telling the PHY not to add its own delays.)

Now let's look at something that isn't a PHY:

	MAC1 ------- MAC2 in a switch

In this case, MAC2 isn't in a PHY or part of a PHY that is driven by
phylib, so we don't have a way in the kernel of passing the PHY mode
from MAC1 to MAC2 in order for MAC2 to configure itself. It's tempting
to say that which RGMII mode is used doesn't matter, but consider the
side-case above - if we're talking about a MAC driver that interprets
the PHY mode and adds its own delays, then it *does* very much matter.

It also matters for MAC2. This could be a switch port that can be used
as a CPU facing port, or a switch port that is used as a PHY. In the
latter case, it becomes exactly as the first two cases above.

Let's take a theoretical case of two ethernet MACs on the same system
connected with a RGMII link:

	MAC1 ------- MAC2

They both use the same driver. What RGMII interface mode should be used
for each? Would it be correct to state "rgmii-id" for both MACs?
Or "rgmii" for one and "rgmii-id" for the other.

You may notice I'm not providing a solution - this is a thought
experiment, to provoke some thought into the proper use of the phy-mode
property at each end of a MAC to MAC link, and hopefully gain some
realisation that (probably) using "rgmii-id" for both MACs probably
isn't correct given the model that phy-mode _generally_ states how the
opposite end of the RGMII link to the MAC should be configured.

However, currently it doesn't matter as long as we don't end up with
two MACs that are back to back and the MAC drivers decide to insert
the RGMII delay (the side-case I mention above.)

Personally, I don't like that we have this side-case as a possibility
because it causes problems (if you go through the thought experiment
above, you'll trip over the problem if you consider the combinations
of MAC1 and MAC2 that do/do not use the side-case!)

So, I would expect a MAC to MAC link to have "rgmii" at one end, and
"rgmii-id" at the other end, rather than both having the same RGMII
mode.

> Vendor downstream DT also adds 'pause' to the fixed-link nodes, and this
> may be something that should be added here. However, during testing flow
> control always ended up being disabled so I skipped 'pause' here.

Does it get disabled at the switch, or the stmmac end?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
  2025-07-27 19:09   ` Andrew Lunn
@ 2025-07-27 21:52     ` Jonas Karlman
  2025-07-27 22:09       ` Andrew Lunn
  0 siblings, 1 reply; 21+ messages in thread
From: Jonas Karlman @ 2025-07-27 21:52 UTC (permalink / raw)
  To: Andrew Lunn, Linus Walleij
  Cc: Alvin Šipraga, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Yao Zi, Chukun Pan,
	Heiko Stuebner, netdev, linux-rockchip, linux-arm-kernel,
	linux-kernel

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



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
  2025-07-27 21:52     ` Jonas Karlman
@ 2025-07-27 22:09       ` Andrew Lunn
  2025-07-28 15:24         ` Jonas Karlman
  0 siblings, 1 reply; 21+ messages in thread
From: Andrew Lunn @ 2025-07-27 22:09 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Linus Walleij, Alvin Šipraga, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, Heiko Stuebner, netdev, linux-rockchip,
	linux-arm-kernel, linux-kernel

> 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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-27 18:03 ` [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C Jonas Karlman
  2025-07-27 19:16   ` Andrew Lunn
  2025-07-27 19:57   ` Russell King (Oracle)
@ 2025-07-28 14:30   ` Chukun Pan
  2025-07-28 17:47     ` Jonas Karlman
  2 siblings, 1 reply; 21+ messages in thread
From: Chukun Pan @ 2025-07-28 14:30 UTC (permalink / raw)
  To: jonas
  Cc: alsi, amadeus, andrew, conor+dt, davem, devicetree, edumazet,
	heiko, krzk+dt, kuba, linus.walleij, linux-arm-kernel,
	linux-kernel, linux-rockchip, netdev, olteanv, pabeni, robh,
	ziyao

Hi,

> Initial testing with iperf3 showed ~930-940 Mbits/sec in one direction
> and only around ~1-2 Mbits/sec in the other direction.

> Any mix of MAC (rx/tx delay) and switch (rx/tx internal delay) did not
> seem to resolve this speed issue, however dropping snps,tso seems to fix
> that issue.

Have you tried setting phy-mode to rgmii? (just for testing)
Usually this problem is caused by incorrect rx/tx delay.

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

Shouldn't this pinctrl be written in interrupts?

> +			ethernet-port@6 {
> +				reg = <6>;
> +				ethernet = <&gmac1>;
> +				label = "cpu";

No need for label = "cpu":
https://github.com/torvalds/linux/commit/567f38317054e66647fd59cfa4e261219a2a21db

> This series relaxes the realtek dsa drivers requirement of having a mdio
> child OF node to probe and instead have it register a user_mii_bus to
> make it function when a mdio child OF node is missing.

This is weird, the switch is connected to the gmac via mdio.
Can you try the following and see if it works? I tried it on
a rk3568 + rtl8367s board and it worked:

```
&mdio1 {
	switch@29 {
		compatible = "realtek,rtl8365mb";
		reg = <29>;
		reset-gpios = ...

		switch_intc: interrupt-controller {
			interrupt-parent = ...
			interrupts = ...
			interrupt-controller;
			#address-cells = <0>;
			#interrupt-cells = <1>;
		};

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

			phy0: ethernet-phy@0 {
				reg = <0>;
				interrupt-parent = <&switch_intc>;
				interrupts = <0>;
			};

			phy1: ethernet-phy@1 {
				reg = <1>;
				interrupt-parent = <&switch_intc>;
				interrupts = <1>;
			};

			phy2: ethernet-phy@2 {
				reg = <2>;
				interrupt-parent = <&switch_intc>;
				interrupts = <2>;
			};

			phy3: ethernet-phy@3 {
				reg = <3>;
				interrupt-parent = <&switch_intc>;
				interrupts = <3>;
			};
		};

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

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

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

			port@2 {
				reg = <2>;
				label = "lan2";
				phy-handle = <&phy2>;
			};

			port@3 {
				reg = <3>;
				label = "lan3";
				phy-handle = <&phy3>;
			};

			port@x {
				reg = <x>;
				ethernet = <&gmac1>;
				phy-mode = "rgmii";

				fixed-link {
					speed = <1000>;
					full-duplex;
				};
			};
		};
	};
};
```

Thanks,
Chukun

--
2.25.1




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-27 19:16   ` Andrew Lunn
@ 2025-07-28 14:57     ` Jonas Karlman
  0 siblings, 0 replies; 21+ messages in thread
From: Jonas Karlman @ 2025-07-28 14:57 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Linus Walleij, Alvin Šipraga, Vladimir Oltean, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, netdev, linux-rockchip, linux-arm-kernel,
	linux-kernel, devicetree

Hi Andrew,

On 7/27/2025 9:16 PM, Andrew Lunn wrote:
> On Sun, Jul 27, 2025 at 06:03:00PM +0000, Jonas Karlman wrote:
>> The Radxa E24C has a Realtek RTL8367RB-VB switch with four usable ports
>> and is connected using a fixed-link to GMAC1 on the RK3528 SoC.
>>
>> Add an ethernet-switch node to describe the RTL8367RB-VB switch.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> Initial testing with iperf3 showed ~930-940 Mbits/sec in one direction
>> and only around ~1-2 Mbits/sec in the other direction.
>>
>> The RK3528 hardware design guide recommends that timing between TXCLK
>> and data is controlled by MAC, and timing between RXCLK and data is
>> controlled by PHY.
>>
>> Any mix of MAC (rx/tx delay) and switch (rx/tx internal delay) did not
>> seem to resolve this speed issue, however dropping snps,tso seems to fix
>> that issue.
> 
> It could well be that the Synopsis TSO code does not understand the
> DSA headers. When it takes a big block to TCP data and segments it,
> you need to have the DSA header on each segment. If it does not do
> that, only the first segment has the DSA header, the switch is going
> to be dropping all the other segments, causes TCP to do a lot of
> retries.

Thanks for your insights!

I can confirm that disable of TSO and RX checksum offload on the conduit
interface help fix any TCP speed issue and reduced UDP packet loss to a
minimum.

Regards,
Jonas

> 
>> Unsure what is best here, should MAC or switch add the delays?
> 
> It should not matter. 2ns is 2ns...
> 
> 	Andrew



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
  2025-07-27 22:09       ` Andrew Lunn
@ 2025-07-28 15:24         ` Jonas Karlman
  2025-07-28 15:40           ` Andrew Lunn
  0 siblings, 1 reply; 21+ messages in thread
From: Jonas Karlman @ 2025-07-28 15:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Linus Walleij, Alvin Šipraga, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, Heiko Stuebner, netdev, linux-rockchip,
	linux-arm-kernel, linux-kernel

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



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
  2025-07-28 15:24         ` Jonas Karlman
@ 2025-07-28 15:40           ` Andrew Lunn
  2025-07-28 16:14             ` Jonas Karlman
  0 siblings, 1 reply; 21+ messages in thread
From: Andrew Lunn @ 2025-07-28 15:40 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Linus Walleij, Alvin Šipraga, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, Heiko Stuebner, netdev, linux-rockchip,
	linux-arm-kernel, linux-kernel

> 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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node
  2025-07-28 15:40           ` Andrew Lunn
@ 2025-07-28 16:14             ` Jonas Karlman
  0 siblings, 0 replies; 21+ messages in thread
From: Jonas Karlman @ 2025-07-28 16:14 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Linus Walleij, Alvin Šipraga, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Yao Zi, Chukun Pan, Heiko Stuebner, netdev, linux-rockchip,
	linux-arm-kernel, linux-kernel

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



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-28 14:30   ` Chukun Pan
@ 2025-07-28 17:47     ` Jonas Karlman
  2025-07-29 11:50       ` Chukun Pan
  0 siblings, 1 reply; 21+ messages in thread
From: Jonas Karlman @ 2025-07-28 17:47 UTC (permalink / raw)
  To: Chukun Pan
  Cc: alsi, andrew, conor+dt, davem, devicetree, edumazet, heiko,
	krzk+dt, kuba, linus.walleij, linux-arm-kernel, linux-kernel,
	linux-rockchip, netdev, olteanv, pabeni, robh, ziyao

Hi Chukun,

On 7/28/2025 4:30 PM, Chukun Pan wrote:
> Hi,
> 
>> Initial testing with iperf3 showed ~930-940 Mbits/sec in one direction
>> and only around ~1-2 Mbits/sec in the other direction.
> 
>> Any mix of MAC (rx/tx delay) and switch (rx/tx internal delay) did not
>> seem to resolve this speed issue, however dropping snps,tso seems to fix
>> that issue.
> 
> Have you tried setting phy-mode to rgmii? (just for testing)
> Usually this problem is caused by incorrect rx/tx delay.

The issue is with TSO and RX checksum offload, with those disabled on
the conduit interface (gmac1/eth0) my issues disappeared.

Use of rgmii-id "RX and TX delays are not provided by the PCB." as
defined by the dt-bindings seem to most correctly describe the HW.

Describing switches is new to me, so I could be wrong :-)

> 
>> +	ethernet-switch@1d {
>> +		compatible = "realtek,rtl8365mb";
>> +		reg = <0x1d>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&rtl8367rb_eint>;
> 
> Shouldn't this pinctrl be written in interrupts?

Not necessarily, in my mind the pinctrl is applied for the switch
interface to the SoC, not the internal workings of the switch.

> 
>> +			ethernet-port@6 {
>> +				reg = <6>;
>> +				ethernet = <&gmac1>;
>> +				label = "cpu";
> 
> No need for label = "cpu":
> https://github.com/torvalds/linux/commit/567f38317054e66647fd59cfa4e261219a2a21db

Thanks, will drop in v2.

> 
>> This series relaxes the realtek dsa drivers requirement of having a mdio
>> child OF node to probe and instead have it register a user_mii_bus to
>> make it function when a mdio child OF node is missing.
> 
> This is weird, the switch is connected to the gmac via mdio.
> Can you try the following and see if it works? I tried it on
> a rk3568 + rtl8367s board and it worked:

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#

With a mdio node the driver is happy and dtschema is sad, and without
the mdio node it was the other way around.

The plan is to drop this patch and instead modify the dt-binding to
allow describing a mdio node when the switch node has a reg prop in v2.

Regards,
Jonas

> 
> ```
> &mdio1 {
> 	switch@29 {
> 		compatible = "realtek,rtl8365mb";
> 		reg = <29>;
> 		reset-gpios = ...
> 
> 		switch_intc: interrupt-controller {
> 			interrupt-parent = ...
> 			interrupts = ...
> 			interrupt-controller;
> 			#address-cells = <0>;
> 			#interrupt-cells = <1>;
> 		};
> 
> 		mdio {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			phy0: ethernet-phy@0 {
> 				reg = <0>;
> 				interrupt-parent = <&switch_intc>;
> 				interrupts = <0>;
> 			};
> 
> 			phy1: ethernet-phy@1 {
> 				reg = <1>;
> 				interrupt-parent = <&switch_intc>;
> 				interrupts = <1>;
> 			};
> 
> 			phy2: ethernet-phy@2 {
> 				reg = <2>;
> 				interrupt-parent = <&switch_intc>;
> 				interrupts = <2>;
> 			};
> 
> 			phy3: ethernet-phy@3 {
> 				reg = <3>;
> 				interrupt-parent = <&switch_intc>;
> 				interrupts = <3>;
> 			};
> 		};
> 
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			port@0 {
> 				reg = <0>;
> 				label = "wan";
> 				phy-handle = <&phy0>;
> 			};
> 
> 			port@1 {
> 				reg = <1>;
> 				label = "lan1";
> 				phy-handle = <&phy1>;
> 			};
> 
> 			port@2 {
> 				reg = <2>;
> 				label = "lan2";
> 				phy-handle = <&phy2>;
> 			};
> 
> 			port@3 {
> 				reg = <3>;
> 				label = "lan3";
> 				phy-handle = <&phy3>;
> 			};
> 
> 			port@x {
> 				reg = <x>;
> 				ethernet = <&gmac1>;
> 				phy-mode = "rgmii";
> 
> 				fixed-link {
> 					speed = <1000>;
> 					full-duplex;
> 				};
> 			};
> 		};
> 	};
> };
> ```
> 
> Thanks,
> Chukun
> 
> --
> 2.25.1
> 
> 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-28 17:47     ` Jonas Karlman
@ 2025-07-29 11:50       ` Chukun Pan
  2025-07-29 20:55         ` Jonas Karlman
  0 siblings, 1 reply; 21+ messages in thread
From: Chukun Pan @ 2025-07-29 11:50 UTC (permalink / raw)
  To: jonas
  Cc: alsi, amadeus, andrew, conor+dt, davem, devicetree, edumazet,
	heiko, krzk+dt, kuba, linus.walleij, linux-arm-kernel,
	linux-kernel, linux-rockchip, netdev, olteanv, pabeni, robh,
	ziyao

Hi,

> The issue is with TSO and RX checksum offload, with those disabled on
> the conduit interface (gmac1/eth0) my issues disappeared.

I did a test today and the same problem occurred when running the new
kernel on my rk3568 + rtl8367s board. This problem does not exist on
older kernels (6.1 and 6.6). Not sure where the problem is.

> 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#
>
> With a mdio node the driver is happy and dtschema is sad, and without
> the mdio node it was the other way around.

On older kernels (6.1/6.6) only realtek-smi requires mdio child OF node.
Commit bba140a566ed ("net: dsa: realtek: use the same mii bus driver for both interfaces")
changed this behavior, both MDIO interface and SMI interface need it
(rtl83xx_setup_user_mdio), but the dt-bindings has not been updated.
I think this needs a Fixes tag.

Thanks,
Chukun

--
2.25.1




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-29 11:50       ` Chukun Pan
@ 2025-07-29 20:55         ` Jonas Karlman
  2025-07-29 21:44           ` Andrew Lunn
  2025-08-10 14:01           ` Chukun Pan
  0 siblings, 2 replies; 21+ messages in thread
From: Jonas Karlman @ 2025-07-29 20:55 UTC (permalink / raw)
  To: Chukun Pan
  Cc: alsi@bang-olufsen.dk, andrew@lunn.ch, conor+dt@kernel.org,
	davem@davemloft.net, devicetree@vger.kernel.org,
	edumazet@google.com, heiko@sntech.de, krzk+dt@kernel.org,
	kuba@kernel.org, linus.walleij@linaro.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	netdev@vger.kernel.org, olteanv@gmail.com, pabeni@redhat.com,
	robh@kernel.org, ziyao@disroot.org

Hi Chukun,

On 7/29/2025 1:50 PM, Chukun Pan wrote:
> Hi,
> 
>> The issue is with TSO and RX checksum offload, with those disabled on
>> the conduit interface (gmac1/eth0) my issues disappeared.
> 
> I did a test today and the same problem occurred when running the new
> kernel on my rk3568 + rtl8367s board. This problem does not exist on
> older kernels (6.1 and 6.6). Not sure where the problem is.

I had only tested on a next-20250722 based kernel and on a vendor 6.1
based kernel. And similar to your findings, on 6.1 based kernel there
was no issue only on the newer kernel.

I will probably drop the use of "/delete-property/ snps,tso" and include
a note in commit message about the TSO and RX checksum issue for v2.

> 
>> 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#
>>
>> With a mdio node the driver is happy and dtschema is sad, and without
>> the mdio node it was the other way around.
> 
> On older kernels (6.1/6.6) only realtek-smi requires mdio child OF node.
> Commit bba140a566ed ("net: dsa: realtek: use the same mii bus driver for both interfaces")
> changed this behavior, both MDIO interface and SMI interface need it
> (rtl83xx_setup_user_mdio), but the dt-bindings has not been updated.
> I think this needs a Fixes tag.

Thanks for finding this, and yes I can see that commit bba140a566ed
changed the behavior of the driver and probably broke out-of-tree users.

My current plan for a v2 is to:
- include a new dt-bindings patch to allow use of a mdio node
- include a mdio node in the switch node
- add a Fixes tag to the driver patch

Then leave up to maintainers to decide if they want to accept this patch
or not.

Regards,
Jonas

> 
> Thanks,
> Chukun
> 
> --
> 2.25.1
> 
> 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-29 20:55         ` Jonas Karlman
@ 2025-07-29 21:44           ` Andrew Lunn
  2025-08-10 14:01           ` Chukun Pan
  1 sibling, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2025-07-29 21:44 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Chukun Pan, alsi@bang-olufsen.dk, conor+dt@kernel.org,
	davem@davemloft.net, devicetree@vger.kernel.org,
	edumazet@google.com, heiko@sntech.de, krzk+dt@kernel.org,
	kuba@kernel.org, linus.walleij@linaro.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	netdev@vger.kernel.org, olteanv@gmail.com, pabeni@redhat.com,
	robh@kernel.org, ziyao@disroot.org

> > I did a test today and the same problem occurred when running the new
> > kernel on my rk3568 + rtl8367s board. This problem does not exist on
> > older kernels (6.1 and 6.6). Not sure where the problem is.
> 
> I had only tested on a next-20250722 based kernel and on a vendor 6.1
> based kernel. And similar to your findings, on 6.1 based kernel there
> was no issue only on the newer kernel.
> 
> I will probably drop the use of "/delete-property/ snps,tso" and include
> a note in commit message about the TSO and RX checksum issue for v2.

You are submitting a patch for todays kernel, not a historic
kernel. If todays kernel needs this property to work, please include
it.

You can always remove it when you have done a git bisect and find what
changed, and submit a fix.

	Andrew


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-07-29 20:55         ` Jonas Karlman
  2025-07-29 21:44           ` Andrew Lunn
@ 2025-08-10 14:01           ` Chukun Pan
  2025-08-10 15:15             ` Andrew Lunn
  1 sibling, 1 reply; 21+ messages in thread
From: Chukun Pan @ 2025-08-10 14:01 UTC (permalink / raw)
  To: jonas
  Cc: alsi, amadeus, andrew, conor+dt, davem, devicetree, edumazet,
	heiko, krzk+dt, kuba, linus.walleij, linux-arm-kernel,
	linux-kernel, linux-rockchip, netdev, olteanv, pabeni, robh,
	ziyao

Hi,

> I had only tested on a next-20250722 based kernel and on a vendor 6.1
> based kernel. And similar to your findings, on 6.1 based kernel there
> was no issue only on the newer kernel.
>
> I will probably drop the use of "/delete-property/ snps,tso" and include
> a note in commit message about the TSO and RX checksum issue for v2.

After my test, this problem is caused by commit 041cc86 ("net: stmmac: Enable TSO on VLANs")
https://github.com/torvalds/linux/commit/041cc86b3653cbcdf6ab96c2f2ae34f3d0a99b0a

It seems that this commit just exposed the TSO problem (with VLANs).

Thanks,
Chukun

--
2.25.1




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-08-10 14:01           ` Chukun Pan
@ 2025-08-10 15:15             ` Andrew Lunn
  2025-08-10 16:49               ` Vladimir Oltean
  0 siblings, 1 reply; 21+ messages in thread
From: Andrew Lunn @ 2025-08-10 15:15 UTC (permalink / raw)
  To: Chukun Pan
  Cc: jonas, alsi, conor+dt, davem, devicetree, edumazet, heiko,
	krzk+dt, kuba, linus.walleij, linux-arm-kernel, linux-kernel,
	linux-rockchip, netdev, olteanv, pabeni, robh, ziyao

On Sun, Aug 10, 2025 at 10:01:15PM +0800, Chukun Pan wrote:
> Hi,
> 
> > I had only tested on a next-20250722 based kernel and on a vendor 6.1
> > based kernel. And similar to your findings, on 6.1 based kernel there
> > was no issue only on the newer kernel.
> >
> > I will probably drop the use of "/delete-property/ snps,tso" and include
> > a note in commit message about the TSO and RX checksum issue for v2.
> 
> After my test, this problem is caused by commit 041cc86 ("net: stmmac: Enable TSO on VLANs")
> https://github.com/torvalds/linux/commit/041cc86b3653cbcdf6ab96c2f2ae34f3d0a99b0a
> 
> It seems that this commit just exposed the TSO problem (with VLANs).

I'm not sure that is correct. What this patch does is enable TSO for
VLANs by adding the VLAN header to the packet in software before
transmitting it, rather than asking the hardware to insert the VLAN
header as it transmits.

What i don't understand yet, is what has VLANs got to do with DSA?
Does the DSA tagger being used not actually insert a switch specific
header, but is using VLAN overlays? Why is the VLAN path in the stmmac
transmit function being used?

Just a guess, but maybe it is a DSA tagger bug? Maybe the user frame
is a VLAN frame. The tagger is placing the VLAN tag into the DSA
header, so in effect, the frame is no longer a VLAN frame. But it is
not calling __vlan_hwaccel_clear_tag() to indicate the skbuf no longer
needs VLAN processing?

	Andrew



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C
  2025-08-10 15:15             ` Andrew Lunn
@ 2025-08-10 16:49               ` Vladimir Oltean
  0 siblings, 0 replies; 21+ messages in thread
From: Vladimir Oltean @ 2025-08-10 16:49 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Chukun Pan, jonas, alsi, conor+dt, davem, devicetree, edumazet,
	heiko, krzk+dt, kuba, linus.walleij, linux-arm-kernel,
	linux-kernel, linux-rockchip, netdev, pabeni, robh, ziyao

On Sun, Aug 10, 2025 at 05:15:59PM +0200, Andrew Lunn wrote:
> Just a guess, but maybe it is a DSA tagger bug? Maybe the user frame
> is a VLAN frame. The tagger is placing the VLAN tag into the DSA
> header, so in effect, the frame is no longer a VLAN frame. But it is
> not calling __vlan_hwaccel_clear_tag() to indicate the skbuf no longer
> needs VLAN processing?

For the original skb to have had a VLAN hwaccel tag, validate_xmit_vlan()
would have had to not push it inside, so vlan_hw_offload_capable() must
have been true for DSA user ports. But we advertise neither the
NETIF_F_HW_VLAN_CTAG_TX nor the NETIF_F_HW_VLAN_STAG_TX netdev feature.
So the VLAN tags in skbs transmitted through DSA user ports should all
be in the skb head.


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2025-08-10 16:52 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-27 18:02 [PATCH net-next 0/3] net: dsa: realtek: Add support for use of an optional mdio node Jonas Karlman
2025-07-27 18:02 ` [PATCH net-next 1/3] net: dsa: realtek: remove unused user_mii_bus from realtek_priv Jonas Karlman
2025-07-27 18:02 ` [PATCH net-next 2/3] net: dsa: realtek: Add support for use of an optional mdio node Jonas Karlman
2025-07-27 19:09   ` Andrew Lunn
2025-07-27 21:52     ` Jonas Karlman
2025-07-27 22:09       ` Andrew Lunn
2025-07-28 15:24         ` Jonas Karlman
2025-07-28 15:40           ` Andrew Lunn
2025-07-28 16:14             ` Jonas Karlman
2025-07-27 18:03 ` [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C Jonas Karlman
2025-07-27 19:16   ` Andrew Lunn
2025-07-28 14:57     ` Jonas Karlman
2025-07-27 19:57   ` Russell King (Oracle)
2025-07-28 14:30   ` Chukun Pan
2025-07-28 17:47     ` Jonas Karlman
2025-07-29 11:50       ` Chukun Pan
2025-07-29 20:55         ` Jonas Karlman
2025-07-29 21:44           ` Andrew Lunn
2025-08-10 14:01           ` Chukun Pan
2025-08-10 15:15             ` Andrew Lunn
2025-08-10 16:49               ` Vladimir Oltean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).