netdev - Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to Radxa E24C

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5bdd0009-589f-49bc-914f-62e5dc4469e9@kwiboo.se>
Date: Mon, 28 Jul 2025 19:47:44 +0200
From: Jonas Karlman <jonas@...boo.se>
To: Chukun Pan <amadeus@....edu.cn>
Cc: alsi@...g-olufsen.dk, andrew@...n.ch, conor+dt@...nel.org,
 davem@...emloft.net, devicetree@...r.kernel.org, edumazet@...gle.com,
 heiko@...ech.de, krzk+dt@...nel.org, kuba@...nel.org,
 linus.walleij@...aro.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org,
 netdev@...r.kernel.org, olteanv@...il.com, pabeni@...hat.com,
 robh@...nel.org, ziyao@...root.org
Subject: Re: [PATCH 3/3] arm64: dts: rockchip: Add RTL8367RB-VB switch to
 Radxa E24C

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
> 
>