* [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
* 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 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 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
* [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 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 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 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 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-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:49 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).