[PATCH 0/2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar -- cover

[PATCH 0/2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar -- cover

[Quentin Schulz]

This adds support for the Ethernet Switch adapter connected through the
mezzanine connector on RK3588 Jaguar.

This adapter has a KSZ9896 Ethernet Switch with 4 1GbE Ethernet
connectors, two user controllable LEDs, and an M12 12-pin connector
which exposes the following signals:
 - RS232/RS485 (max 250Kbps/500Kbps, RX pin1, TX pin2)
 - two digital inputs (pin4 routed to GPIO3_C5 on SoC, pin5 to GPIO4_B4)
 - two digital outputs (pin7 routed to GPIO3_D3 on SoC, pin8 to
   GPIO3_D1)
 - two analog inputs (pin10 to channel1 of ADS1015, pin11 to channel2)

The whole SBC can be powered through the M8 3-pin 12-24V connector on
the adapter.

This also adds the ethernet1 alias to the Jaguar main DTS as the GMAC1
is exposed on the proprietary Mezzanine connector so anyone using GMAC
on the Mezzanine connector would likely need it (actually need it until
https://lore.kernel.org/netdev/20250521-stmmac-mdio-bus_id-v1-1-918a3c11bf2c@cherry.de/T/#u
is merged).

Note that for this to work, you need this commit:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=ba54bce747fa9e07896c1abd9b48545f7b4b31d2
otherwise most packets are ignored by the DSA switch driver.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
Quentin Schulz (2):
      arm64: dts: rockchip: add ethernet1 alias to RK3588 Jaguar
      arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar

 arch/arm64/boot/dts/rockchip/Makefile              |   5 +
 .../rockchip/rk3588-jaguar-ethernet-switch.dtso    | 189 +++++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts     |   1 +
 3 files changed, 195 insertions(+)
---
base-commit: 4a95bc121ccdaee04c4d72f84dbfa6b880a514b6
change-id: 20250521-jaguar-mezz-eth-switch-75445fd1c725

Best regards,
-- 
Quentin Schulz <quentin.schulz@cherry.de>

[PATCH 1/2] arm64: dts: rockchip: add ethernet1 alias to RK3588 Jaguar

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

The RK3588 Jaguar exposes pins that can be muxed for GMAC1 functions to
the Mezzanine proprietary connector, so let's add the alias to prepare
for adapters using those signals in that function.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
index 9fceea6c1398e92114dcb735cf2babb7d05d67a5..70a2569478f6165f067befb6cdfb4f58f00dd17d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
@@ -33,6 +33,7 @@ button-bios-disable {
 
 	aliases {
 		ethernet0 = &gmac0;
+		ethernet1 = &gmac1;
 		i2c10 = &i2c10;
 		mmc0 = &sdhci;
 		mmc1 = &sdmmc;

-- 
2.49.0

Re: [PATCH 1/2] arm64: dts: rockchip: add ethernet1 alias to RK3588 Jaguar

[Heiko Stübner]

Am Mittwoch, 21. Mai 2025, 17:44:19 Mitteleuropäische Sommerzeit schrieb Quentin Schulz:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> The RK3588 Jaguar exposes pins that can be muxed for GMAC1 functions to
> the Mezzanine proprietary connector, so let's add the alias to prepare
> for adapters using those signals in that function.

In light of the discussion about unused aliases in [0], always adding an
ethernet1 alias for something that may never be used is probably not
the way to go.


Heiko

[0] https://lore.kernel.org/linux-rockchip/df6003e3-7fc3-4e50-a702-f0aa8d663dff@app.fastmail.com/

> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> index 9fceea6c1398e92114dcb735cf2babb7d05d67a5..70a2569478f6165f067befb6cdfb4f58f00dd17d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> @@ -33,6 +33,7 @@ button-bios-disable {
>  
>  	aliases {
>  		ethernet0 = &gmac0;
> +		ethernet1 = &gmac1;
>  		i2c10 = &i2c10;
>  		mmc0 = &sdhci;
>  		mmc1 = &sdmmc;
> 
> 

[PATCH 2/2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

This adds support for the Ethernet Switch adapter connected through the
mezzanine connector on RK3588 Jaguar.

This adapter has a KSZ9896 Ethernet Switch with 4 1GbE Ethernet
connectors, two user controllable LEDs, and an M12 12-pin connector
which exposes the following signals:
 - RS232/RS485 (max 250Kbps/500Kbps, RX pin1, TX pin2)
 - two digital inputs (pin4 routed to GPIO3_C5 on SoC, pin5 to GPIO4_B4)
 - two digital outputs (pin7 routed to GPIO3_D3 on SoC, pin8 to
   GPIO3_D1)
 - two analog inputs (pin10 to channel1 of ADS1015, pin11 to channel2)

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 arch/arm64/boot/dts/rockchip/Makefile              |   5 +
 .../rockchip/rk3588-jaguar-ethernet-switch.dtso    | 189 +++++++++++++++++++++
 2 files changed, 194 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index 3e8771ef69ba1c1428117cc2ae29b84e13523e21..6d5ad354b77de1c3f995b119f97541f9c2cc9dbd 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -151,6 +151,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-firefly-itx-3588j.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-friendlyelec-cm3588-nas.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-h96-max-v58.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-jaguar.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-jaguar-ethernet-switch.dtbo
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-jaguar-pre-ict-tester.dtbo
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-mnt-reform2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-nanopc-t6.dtb
@@ -222,6 +223,10 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6b-wifi.dtb
 rk3588-edgeble-neu6b-wifi-dtbs := rk3588-edgeble-neu6b-io.dtb \
 	rk3588-edgeble-neu6a-wifi.dtbo
 
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-jaguar-ethernet-switch.dtb
+rk3588-jaguar-ethernet-switch-dtbs := rk3588-jaguar.dtb \
+	rk3588-jaguar-ethernet-switch.dtbo
+
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-jaguar-pre-ict-tester.dtb
 rk3588-jaguar-pre-ict-tester-dtbs := rk3588-jaguar.dtb \
 	rk3588-jaguar-pre-ict-tester.dtbo
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar-ethernet-switch.dtso b/arch/arm64/boot/dts/rockchip/rk3588-jaguar-ethernet-switch.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..cff7655a21007a934019590f3836344b9851e537
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar-ethernet-switch.dtso
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2025 Cherry Embedded Solutions GmbH
+ *
+ * Device Tree Overlay for the Ethernet Switch adapter for the Mezzanine
+ * connector on RK3588 Jaguar.
+ *
+ * This adapter has a KSZ9896 Ethernet Switch with 4 1GbE Ethernet connectors,
+ * two user controllable LEDs, and an M12 12-pin connector which exposes the
+ * following signals:
+ *  - RS232/RS485 (max 250Kbps/500Kbps, RX pin1, TX pin2)
+ *  - two digital inputs (pin4 routed to GPIO3_C5 on SoC, pin5 to GPIO4_B4)
+ *  - two digital outputs (pin7 routed to GPIO3_D3 on SoC, pin8 to GPIO3_D1)
+ *  - two analog inputs (pin10 to channel1 of ADS1015, pin11 to channel2)
+ *
+ * RK3588 Jaguar can be powered entirely through the adapter via the M8 3-pin
+ * connector (12-24V).
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/rockchip,rk3588-cru.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/rockchip.h>
+
+&{/} {
+	mezzanine-leds {
+		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&led_usr1_pin &led_usr2_pin>;
+
+		led-1 {
+			gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_HIGH>;
+			label = "USR1";
+		};
+
+		led-2 {
+			gpios = <&gpio3 RK_PC4 GPIO_ACTIVE_HIGH>;
+			label = "USR2";
+		};
+	};
+};
+
+&gmac1 {
+	clock_in_out = "output";
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac1_rx_bus2
+		     &gmac1_tx_bus2
+		     &gmac1_rgmii_clk
+		     &gmac1_rgmii_bus
+		     &eth1_pins>;
+	rx_delay = <0x30>;
+	tx_delay = <0x30>;
+	status = "okay";
+
+	fixed-link {
+		speed = <1000>;
+		full-duplex;
+	};
+};
+
+&i2c1 {
+	#address-cells = <1>;
+	/* ADS1015 can handle high-speed (HS) mode (up to 3.4MHz) on I2C bus,
+	   but SOC can handle only up to 400kHz. */
+	clock-frequency = <400000>;
+	#size-cells = <0>;
+	status = "okay";
+
+	adc@48 {
+		compatible = "ti,ads1015";
+		reg = <0x48>;
+		#address-cells = <1>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <RK_PC7 IRQ_TYPE_EDGE_FALLING>;
+		pinctrl-0 = <&adc_alert>;
+		pinctrl-names = "default";
+		#io-channel-cells = <1>;
+		#size-cells = <0>;
+
+		channel@1 {
+			reg = <5>; /* Single-ended between AIN1 and GND */
+			ti,datarate = <0>;
+			ti,gain = <5>;
+		};
+
+		channel@2 {
+			reg = <6>; /* Single-ended between AIN2 and GND */
+			ti,datarate = <0>;
+			ti,gain = <5>;
+		};
+	};
+
+	switch@5f {
+		compatible = "microchip,ksz9896";
+		reg = <0x5f>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <RK_PB7 IRQ_TYPE_EDGE_FALLING>; /* ETH_INTRP_N */
+		pinctrl-0 = <&eth_reset_n &eth_intrp_n>;
+		pinctrl-names = "default";
+		reset-gpios = <&gpio3 RK_PB6 GPIO_ACTIVE_LOW>; /* ETH_RESET */
+		microchip,synclko-disable; /* CLKO_25_125 only routed to TP1 */
+
+		ethernet-ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			lan1: port@0 {
+				reg = <0>;
+				label = "ETH1";
+			};
+
+			lan2: port@1 {
+				reg = <1>;
+				label = "ETH2";
+			};
+
+			lan3: port@2 {
+				reg = <2>;
+				label = "ETH3";
+			};
+
+			lan4: port@3 {
+				reg = <3>;
+				label = "ETH4";
+			};
+
+			port@5 {
+				reg = <5>;
+				ethernet = <&gmac1>;
+				label = "CPU";
+				phy-mode = "rgmii";
+				/* Delay done by the MAC via rx_delay/tx_delay */
+				rx-internal-delay-ps = <0>;
+				tx-internal-delay-ps = <0>;
+
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+		};
+	};
+};
+
+&pinctrl {
+	adc {
+		adc_alert: adc-alert-irq {
+			rockchip,pins =
+				<3 RK_PC7 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	ethernet {
+		eth_intrp_n: eth-intrp-n {
+			rockchip,pins =
+				<3 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		eth_reset_n: eth-reset-n {
+			rockchip,pins =
+				<3 RK_PB6 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	leds {
+		led_usr1_pin: led-usr1-pin {
+			rockchip,pins =
+				<1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
+		led_usr2_pin: led-usr2-pin {
+			rockchip,pins =
+				<3 RK_PC4 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+};
+
+&uart9 {
+	/* GPIO3_D0/EN_RS485_MODE for switching between RS232 and RS485 */
+	pinctrl-0 = <&uart9m2_xfer &uart9m2_rtsn>;
+	pinctrl-names = "default";
+	linux,rs485-enabled-at-boot-time;
+	status = "okay";
+};

-- 
2.49.0

Re: [PATCH 2/2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar

[Andrew Lunn]

> +&gmac1 {
> +	clock_in_out = "output";
> +	phy-mode = "rgmii";

Does the PCB have extra long clock lines to implement the 2ns delays?

> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gmac1_rx_bus2
> +		     &gmac1_tx_bus2
> +		     &gmac1_rgmii_clk
> +		     &gmac1_rgmii_bus
> +		     &eth1_pins>;
> +	rx_delay = <0x30>;
> +	tx_delay = <0x30>;

Since this has a switch on the other end, its a bit more complicated
with RGMII delays. Normally, the MAC does nothing and passed rgmii-id
to the PHY, and the PHY then does the delays. However, here you don't
have a PHY. So you have the MAC add the delays. This looks O.K. I
would prefer that the driver used the standardized
rx-internal-delay-ps & tx-internal-delay-ps rather than these vendor
properties. But that is probably out of scope for this patchset.

> +			port@5 {
> +				reg = <5>;
> +				ethernet = <&gmac1>;
> +				label = "CPU";
> +				phy-mode = "rgmii";

Again, this probably should be rgmii-id to correctly describe the PCB,
but i don't know if the switch takes any notice of it.

	Andrew

Re: [PATCH 2/2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar

[Quentin Schulz]

Hi Andrew,

On 5/21/25 6:25 PM, Andrew Lunn wrote:
>> +&gmac1 {
>> +	clock_in_out = "output";
>> +	phy-mode = "rgmii";
> 
> Does the PCB have extra long clock lines to implement the 2ns delays?
> 

Not that I am aware no.

The issue here is that I believe the Linux driver actually got the whole 
phy-mode thing wrong?

drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c

First, tx_delay defaults to 0x30 if absent, rx_delay to 0x10 if absent, 
which seems a bit odd but why not.

Then you have rk_gmac_powerup() handling the delays.

If RGMII, then use rx_delay and tx_delay. If RGMII-ID, use neither. If 
RGMII-RXID use tx_delay. If RGMII-TXID use rx_delay.

This is the complete opposite of what I was expecting? I would like to 
use rgmii-id because this should better fit the HW matching the 
documentation in the dt-binding here 
(https://elixir.bootlin.com/linux/v6.15-rc6/source/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L287), 
but this would actually disable the delays on the MAC if I read that 
correctly.

>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&gmac1_rx_bus2
>> +		     &gmac1_tx_bus2
>> +		     &gmac1_rgmii_clk
>> +		     &gmac1_rgmii_bus
>> +		     &eth1_pins>;
>> +	rx_delay = <0x30>;
>> +	tx_delay = <0x30>;
> 
> Since this has a switch on the other end, its a bit more complicated
> with RGMII delays. Normally, the MAC does nothing and passed rgmii-id
> to the PHY, and the PHY then does the delays. However, here you don't
> have a PHY. So you have the MAC add the delays. This looks O.K. I

The switch actually supports adding delays on the port used for DSA conduit.

https://eu.mouser.com/datasheet/2/268/KSZ9896C_Data_Sheet_DS00002390C-3443638.pdf 
5.2.3.2 XMII Port Control 1 Register bits 3 and 4. But the granularity 
is essentially: 0 delay or at least 1.5ns...

With the SoC MAC we actually have a much (assumed) more precise granularity.

https://github.com/FanX-Tek/rk3588-TRM-and-Datasheet/blob/master/Rockchip%20RK3588%20TRM%20V1.0-Part1-20220309.pdf

25.6.11 Clock Architecture

"""
In order to dynamically adjust the timing between TX/RX clocks with data,
delayline is integrated in TX and RX clock path. Register 
SYS_GRF_SOC_CON7[5:2] can
enable the delayline and SYS_GRF_SOC_CON8[15:0]/SYS_GRF_SOC_CON9[15:0] 
is used to
determine the delay length. There are 200 delay elements in each delayline.
"""

No information on the meaning of the number we put in rx_delay/tx_delay. 
Then you'll ask how we figure out the value to put there :) We initially 
port on the downstream kernel where they have a debugfs entry to 
dynamically modify the delay and return an eye of which you get the center.

> would prefer that the driver used the standardized
> rx-internal-delay-ps & tx-internal-delay-ps rather than these vendor

I would prefer too. We would need to know what the value we put in 
rx_delay/tx_delay actually mean in terms of picoseconds? (adding Kever 
in Cc) The worry I have is whether this is stable across all SoCs using 
that IP and/or if the delay is based off the MAC clock or something like 
that?

Then, in order to NOT break new kernel Image with old DT, I assume we 
need something like:

If RGMII, then use rx_delay and tx_delay, no 
rx-internal-delay-ps/tx-internal-delay-ps.

If RGMII-ID, no rx_delay/tx_delay, both 
rx-internal-delay-ps/tx-internal-delay-ps.
If RGMII-RXID use tx_delay and rx-internal-delay-ps.
If RGMII-TXID use rx_delay and tx-internal-delay-ps.

Fail if there's a mix of tx_delay/rx_delay and 
tx-internal-delay-ps/rx-internal-delay-ps properties? (possibly even on 
dt-binding level).

> properties. But that is probably out of scope for this patchset.
> 
>> +			port@5 {
>> +				reg = <5>;
>> +				ethernet = <&gmac1>;
>> +				label = "CPU";
>> +				phy-mode = "rgmii";
> 
> Again, this probably should be rgmii-id to correctly describe the PCB,
> but i don't know if the switch takes any notice of it.
> 

It does something based on DT:

drivers/net/dsa/microchip/ksz_common.c:ksz_parse_rgmii_delay()

If neither rx-internal-delay-ps/tx-internal-delay-ps properties are 
present, set rx_delay to enabled (2000 is arbitrary there) when in RGMII 
or RGMII_RXID mode, set tx_delay to enabled (2000 is arbitrary there) 
when in RGMII or RGMII_TXID mode. If one property is missing, it is set 
to disabled.

Now I do have a question. Shouldn't phy-mode from the MAC match the one 
from the PHY (here I assume the switch contains a PHY on the CPU port)?

I'm a bit confused by the following sentence:

"""
Normally, the MAC does nothing and passed rgmii-id
"""

is this something that the MAC driver is supposed to do or is the 
subsystem handling that somehow? How do I know how/when to rewrite the 
phy-mode passed to the PHY?

Cheers,
Quentin

Re: [PATCH 2/2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar

[Andrew Lunn]

On Thu, May 22, 2025 at 10:18:12AM +0200, Quentin Schulz wrote:
> Hi Andrew,
> 
> On 5/21/25 6:25 PM, Andrew Lunn wrote:
> > > +&gmac1 {
> > > +	clock_in_out = "output";
> > > +	phy-mode = "rgmii";
> > 
> > Does the PCB have extra long clock lines to implement the 2ns delays?
> > 
> 
> Not that I am aware no.

So 'rgmii-id' describes the hardware.

> 
> The issue here is that I believe the Linux driver actually got the whole
> phy-mode thing wrong?

Quite possible, a few drivers do.

> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> 
> First, tx_delay defaults to 0x30 if absent, rx_delay to 0x10 if absent,
> which seems a bit odd but why not.
> 
> Then you have rk_gmac_powerup() handling the delays.
> 
> If RGMII, then use rx_delay and tx_delay. If RGMII-ID, use neither. If
> RGMII-RXID use tx_delay. If RGMII-TXID use rx_delay.
> 
> This is the complete opposite of what I was expecting?

This driver, and the aspeed driver cause a lot of problems....

> > Since this has a switch on the other end, its a bit more complicated
> > with RGMII delays. Normally, the MAC does nothing and passed rgmii-id
> > to the PHY, and the PHY then does the delays. However, here you don't
> > have a PHY. So you have the MAC add the delays. This looks O.K. I
> 
> The switch actually supports adding delays on the port used for DSA conduit.

That actually looks to be the simplest and correct solution. Set the
MAC to 'rgmii-id', rx_delay and tx_delay to 0, even if they are
ignored. And in the switch, also 'rgmii-id' and let it insert the 2ns
delay. You can use rx-internal-delay-ps and tx-internal-delay-ps if
you want, but it seems to default to sensible values.

> I'm a bit confused by the following sentence:
> 
> """
> Normally, the MAC does nothing and passed rgmii-id
> """
> 
> is this something that the MAC driver is supposed to do or is the subsystem
> handling that somehow? How do I know how/when to rewrite the phy-mode passed
> to the PHY?

A small number of MACs have hard coded delays. You cannot turn the
delay off. So the MAC has no choice but to do the delay. 'rgmii' is
simply not possible, so -EINVAL. For 'rgmii-id', if you pass that to
the PHY, it will also add a delay, and 4ns in total does not work. So
when the MAC is adding delays, it needs to mask out the delays it is
adding before calling phy_attach() passing an rgmii mode.

	Andrew

Re: [PATCH 2/2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar

[Quentin Schulz]

Hi Andrew,

On 5/22/25 2:50 PM, Andrew Lunn wrote:
> On Thu, May 22, 2025 at 10:18:12AM +0200, Quentin Schulz wrote:
>> Hi Andrew,
>>
>> On 5/21/25 6:25 PM, Andrew Lunn wrote:
>>>> +&gmac1 {
>>>> +	clock_in_out = "output";
>>>> +	phy-mode = "rgmii";
>>>
>>> Does the PCB have extra long clock lines to implement the 2ns delays?
>>>
>>
>> Not that I am aware no.
> 
> So 'rgmii-id' describes the hardware.
> 
>>
>> The issue here is that I believe the Linux driver actually got the whole
>> phy-mode thing wrong?
> 
> Quite possible, a few drivers do.
> 
>> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>
>> First, tx_delay defaults to 0x30 if absent, rx_delay to 0x10 if absent,
>> which seems a bit odd but why not.
>>
>> Then you have rk_gmac_powerup() handling the delays.
>>
>> If RGMII, then use rx_delay and tx_delay. If RGMII-ID, use neither. If
>> RGMII-RXID use tx_delay. If RGMII-TXID use rx_delay.
>>
>> This is the complete opposite of what I was expecting?
> 
> This driver, and the aspeed driver cause a lot of problems....
> 

Would the suggested change in the previous answer be acceptable to you?

@Kever, is there any way to know what the register values for 
rx_delay/tx_delay actually mean in terms of picoseconds delay added by 
the MAC maybe?

>>> Since this has a switch on the other end, its a bit more complicated
>>> with RGMII delays. Normally, the MAC does nothing and passed rgmii-id
>>> to the PHY, and the PHY then does the delays. However, here you don't
>>> have a PHY. So you have the MAC add the delays. This looks O.K. I
>>
>> The switch actually supports adding delays on the port used for DSA conduit.
> 
> That actually looks to be the simplest and correct solution. Set the
> MAC to 'rgmii-id', rx_delay and tx_delay to 0, even if they are
> ignored. And in the switch, also 'rgmii-id' and let it insert the 2ns
> delay. You can use rx-internal-delay-ps and tx-internal-delay-ps if
> you want, but it seems to default to sensible values.
> 

I don't have control on how much is inserted by the PHY as opposed to 
the MAC, so I'm wary of using a much less precise (on paper) delay. I 
have no clue if doing this in the PHY is going to put us in the center 
of the eye or not. Thanks to Rockchip's kernel tool, we are expecting to 
be in the center of the eye right now.

>> I'm a bit confused by the following sentence:
>>
>> """
>> Normally, the MAC does nothing and passed rgmii-id
>> """
>>
>> is this something that the MAC driver is supposed to do or is the subsystem
>> handling that somehow? How do I know how/when to rewrite the phy-mode passed
>> to the PHY?
> 
> A small number of MACs have hard coded delays. You cannot turn the
> delay off. So the MAC has no choice but to do the delay. 'rgmii' is
> simply not possible, so -EINVAL. For 'rgmii-id', if you pass that to
> the PHY, it will also add a delay, and 4ns in total does not work. So
> when the MAC is adding delays, it needs to mask out the delays it is
> adding before calling phy_attach() passing an rgmii mode.
> 

If I understand correctly, if phy-mode is rgmii-id in DT and the MAC is 
adding the delay, I need to force PHY_INTERFACE_MODE_RGMII phy-mode when 
attaching the PHY in the MAC device. This seems to indicate that the 
meaning of phy-mode = "rgmii" in the DT differs from phy_mode = 
PHY_INTERFACE_MODE_RGMII. The former represents the link, so rgmii means 
no delay added by either IC (MAC or PHY), but the latter is for the 
specific device with this phy_mode set and means "no delay for this 
particular device, but maybe the other end of the link adds it".

Does this also mean you cannot have mixed addition of delay? E.g. having 
1ns from MAC and 1ns from PHY? It has to be only on one of the IC?

In the comment at the bottom of the dt binding there's this following 
sentence:

# When the PCB does not implement the delays, the MAC or PHY must.  As
# such, this is software configuration, and so not described in Device
# Tree.

Why do we have two possible locations for rx-internal-delay-ps: PHY and 
MAC? This means to me the location is in some way software configuration 
since the MAC won't read the property from the PHY and vice-versa. Why 
isn't the MAC responsible for providing the delay to add to the PHY when 
attaching as well? Or are we supposed to have the same values and 
presence of the properties in the MAC and PHY? The KSZ DSA switch seems 
to be handling the delay based on the phy-mode from the DT for the DSA 
conduit and not the one it gets when being attached to the MAC, c.f. 
ksz_parse_rgmii_delay() called in ksz_switch_register() for each port in 
DT with port mode gotten with of_get_phy_mode().

Cheers,
Quentin

Re: [PATCH 2/2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar

[Andrew Lunn]

> Would the suggested change in the previous answer be acceptable to you?
> 
> @Kever, is there any way to know what the register values for
> rx_delay/tx_delay actually mean in terms of picoseconds delay added by the
> MAC maybe?

The problem is, what exactly do these values mean? Is it documented
somewhere?

> > > > Since this has a switch on the other end, its a bit more complicated
> > > > with RGMII delays. Normally, the MAC does nothing and passed rgmii-id
> > > > to the PHY, and the PHY then does the delays. However, here you don't
> > > > have a PHY. So you have the MAC add the delays. This looks O.K. I
> > > 
> > > The switch actually supports adding delays on the port used for DSA conduit.
> > 
> > That actually looks to be the simplest and correct solution. Set the
> > MAC to 'rgmii-id', rx_delay and tx_delay to 0, even if they are
> > ignored. And in the switch, also 'rgmii-id' and let it insert the 2ns
> > delay. You can use rx-internal-delay-ps and tx-internal-delay-ps if
> > you want, but it seems to default to sensible values.
> > 
> 
> I don't have control on how much is inserted by the PHY as opposed to the
> MAC, so I'm wary of using a much less precise (on paper) delay.

Experience i've had with this is that you don't need to be too
accurate. Devices generally work with 2ns.

> I have no
> clue if doing this in the PHY is going to put us in the center of the eye or
> not. Thanks to Rockchip's kernel tool, we are expecting to be in the center
> of the eye right now.

What exactly does the tool do? Can you run it when the 'PHY' is adding
the delay and see how good the eye alignment is?

arch$ grep -hr "tx_delay =" | sort | uniq -c
      1 	tx_delay = <0x0>;
      4 	tx_delay = <0x10>;
      1 	tx_delay = <0x1a>;
      1 	tx_delay = <0x20>;
      3 	tx_delay = <0x21>;
      2 	tx_delay = <0x22>;
      5 	tx_delay = <0x24>;
      2 	tx_delay = <0x26>;
     15 	tx_delay = <0x28>;
      2 	tx_delay = <0x2a>;
     17 	tx_delay = <0x30>;
      1 	tx_delay = <0x3a>;
      3 	tx_delay = <0x3c>;
      3 	tx_delay = <0x42>;
      5 	tx_delay = <0x43>;
      2 	tx_delay = <0x44>;
      1 	tx_delay = <0x45>;
      1 	tx_delay = <0x46>;
      6 	tx_delay = <0x4f>;

So 0x30 is the most popular, and i expect it maps to 2ns. The 0x28
value is interesting, given that 0x2a is not used much. That makes me
think there might be a common PHY used with these boards which has a
small built in delay when it should not.
 
> > > I'm a bit confused by the following sentence:
> > > 
> > > """
> > > Normally, the MAC does nothing and passed rgmii-id
> > > """
> > > 
> > > is this something that the MAC driver is supposed to do or is the subsystem
> > > handling that somehow? How do I know how/when to rewrite the phy-mode passed
> > > to the PHY?
> > 
> > A small number of MACs have hard coded delays. You cannot turn the
> > delay off. So the MAC has no choice but to do the delay. 'rgmii' is
> > simply not possible, so -EINVAL. For 'rgmii-id', if you pass that to
> > the PHY, it will also add a delay, and 4ns in total does not work. So
> > when the MAC is adding delays, it needs to mask out the delays it is
> > adding before calling phy_attach() passing an rgmii mode.
> > 
> 
> If I understand correctly, if phy-mode is rgmii-id in DT and the MAC is
> adding the delay, I need to force PHY_INTERFACE_MODE_RGMII phy-mode when
> attaching the PHY in the MAC device.

Correct. DT phy-mode describes the PCB. Does the PCB add the 2ns
delay.

Once you get to the MAC/PHY pair, what the MAC passes to the PHY is
about what remaining delays need adding. It could be nothing, because
the PCB adds the delay. It could be all of it, because neither the PCB
nor the MAC add the delays. It can also be nothing because the MAC has
already added the delays.

For linux, we have the policy that the PHY adds the delay, in an
attempt to try to make all systems the same. And most boards follow
this. And then we have a few systems that totally mess up delays, have
odd configuration knobs nobody knows what the do exactly, and put the
delays in the MAC.

> Does this also mean you cannot have mixed addition of delay? E.g. having 1ns
> from MAC and 1ns from PHY? It has to be only on one of the IC?

It is not recommended, because of the policy that the PHY does the
delay... You can, if you make use of the rx-internal-delay-ps
properties, assuming both the MAC and PHY support them.

> In the comment at the bottom of the dt binding there's this following
> sentence:
> 
> # When the PCB does not implement the delays, the MAC or PHY must.  As
> # such, this is software configuration, and so not described in Device
> # Tree.
> 
> Why do we have two possible locations for rx-internal-delay-ps: PHY and MAC?

Sometimes both can add variable delays. Sometimes it is fixed 2ns.

I would actually prefer that these properties were not used, because
they indicate somebody messed up. If you read the RGMII standard, it
says a 2ns delay is required between the clock and the data. It is as
simple as that. If you need to fine tune it, it means one of:

The PCB is badly designed, care was not taken to ensure the PCB tracks
are the same length.

The MAC is badly designed, it does not add 0ns/2ns, but some other
delay.

The PHY is badly designed, it does not add 0ns/2ns, but some other
delay.

I don't have empirical data, but i get the feeling designs are getting
worse, there is more need to use fine tuning. So these
{rx|tx}-internal-delay-ps properties are needed. Maybe the board you
are using is as a whole badly designed and needs fine tuning. If it
does, you have to decide where to put the fine tuning. But i would
prefer the 'PHY' adds the basic 2ns delay, and if need be, the MAC can
add/remove some picoseconds.

	Andrew

Re: [PATCH 2/2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar

[Quentin Schulz]

Hi Andrew,

On 5/22/25 6:59 PM, Andrew Lunn wrote:
>> Would the suggested change in the previous answer be acceptable to you?
>>
>> @Kever, is there any way to know what the register values for
>> rx_delay/tx_delay actually mean in terms of picoseconds delay added by the
>> MAC maybe?
> 
> The problem is, what exactly do these values mean? Is it documented
> somewhere?
> 

Those aren't documented in Rockchip's TRM (Technical Reference Manual) 
as far as I could tell, hence why I asked for Kever's input on that. We 
could empirically find direct correlation to a specific value but I 
cannot do this for every version of the IP we have in all Rockchip SoCs 
as I only have access to a handful :)

>>>>> Since this has a switch on the other end, its a bit more complicated
>>>>> with RGMII delays. Normally, the MAC does nothing and passed rgmii-id
>>>>> to the PHY, and the PHY then does the delays. However, here you don't
>>>>> have a PHY. So you have the MAC add the delays. This looks O.K. I
>>>>
>>>> The switch actually supports adding delays on the port used for DSA conduit.
>>>
>>> That actually looks to be the simplest and correct solution. Set the
>>> MAC to 'rgmii-id', rx_delay and tx_delay to 0, even if they are
>>> ignored. And in the switch, also 'rgmii-id' and let it insert the 2ns
>>> delay. You can use rx-internal-delay-ps and tx-internal-delay-ps if
>>> you want, but it seems to default to sensible values.
>>>
>>
>> I don't have control on how much is inserted by the PHY as opposed to the
>> MAC, so I'm wary of using a much less precise (on paper) delay.
> 
> Experience i've had with this is that you don't need to be too
> accurate. Devices generally work with 2ns.
> 

I see, I think I took this 2ns too critically. I thought we really were 
supposed to be close to it but thanks to the tip about the RGMII spec[1] 
we can see that the delay at transmitter needs to be between 1.2 and 2ns 
while at the receiver it needs to be between 1 and 2ns. So "at least 
1.5ns" granularity in the PHY is good enough for us :) (and we verified 
it works of course :) )

[1] https://etherealwake.com/2025/03/ethernet-rgmii/rgmii_2_0.pdf

>> I have no
>> clue if doing this in the PHY is going to put us in the center of the eye or
>> not. Thanks to Rockchip's kernel tool, we are expecting to be in the center
>> of the eye right now.
> 
> What exactly does the tool do? Can you run it when the 'PHY' is adding
> the delay and see how good the eye alignment is?
> 
> arch$ grep -hr "tx_delay =" | sort | uniq -c
>        1 	tx_delay = <0x0>;
>        4 	tx_delay = <0x10>;
>        1 	tx_delay = <0x1a>;
>        1 	tx_delay = <0x20>;
>        3 	tx_delay = <0x21>;
>        2 	tx_delay = <0x22>;
>        5 	tx_delay = <0x24>;
>        2 	tx_delay = <0x26>;
>       15 	tx_delay = <0x28>;
>        2 	tx_delay = <0x2a>;
>       17 	tx_delay = <0x30>;
>        1 	tx_delay = <0x3a>;
>        3 	tx_delay = <0x3c>;
>        3 	tx_delay = <0x42>;
>        5 	tx_delay = <0x43>;
>        2 	tx_delay = <0x44>;
>        1 	tx_delay = <0x45>;
>        1 	tx_delay = <0x46>;
>        6 	tx_delay = <0x4f>;
> 
> So 0x30 is the most popular, and i expect it maps to 2ns. The 0x28
> value is interesting, given that 0x2a is not used much. That makes me
> think there might be a common PHY used with these boards which has a
> small built in delay when it should not.
>   

Note that Rockchip SoCs typically have integrated PHYs as well (which we 
don't use at CHERRY but the option is there). The answer otherwise is 
usually of the kind of "we followed the reference design, it works with 
the DT from the evaluation board, this DT was accepted, we do the same". 
For contributors, I don't think it's feasible to figure out whether the 
PCB lane length matching is done properly or how much is actually 
required except by modifying the delay by hand and find both extremes 
where it stops working and empirically decide the eye is in the middle 
of those (for your board, which may differ from another board of the 
same kind).

>>>> I'm a bit confused by the following sentence:
>>>>
>>>> """
>>>> Normally, the MAC does nothing and passed rgmii-id
>>>> """
>>>>
>>>> is this something that the MAC driver is supposed to do or is the subsystem
>>>> handling that somehow? How do I know how/when to rewrite the phy-mode passed
>>>> to the PHY?
>>>
>>> A small number of MACs have hard coded delays. You cannot turn the
>>> delay off. So the MAC has no choice but to do the delay. 'rgmii' is
>>> simply not possible, so -EINVAL. For 'rgmii-id', if you pass that to
>>> the PHY, it will also add a delay, and 4ns in total does not work. So
>>> when the MAC is adding delays, it needs to mask out the delays it is
>>> adding before calling phy_attach() passing an rgmii mode.
>>>
>>
>> If I understand correctly, if phy-mode is rgmii-id in DT and the MAC is
>> adding the delay, I need to force PHY_INTERFACE_MODE_RGMII phy-mode when
>> attaching the PHY in the MAC device.
> 
> Correct. DT phy-mode describes the PCB. Does the PCB add the 2ns
> delay.
> 
> Once you get to the MAC/PHY pair, what the MAC passes to the PHY is
> about what remaining delays need adding. It could be nothing, because
> the PCB adds the delay. It could be all of it, because neither the PCB
> nor the MAC add the delays. It can also be nothing because the MAC has
> already added the delays.
> 

Clear thanks. I'm not sure whether Linux already has a way for the MAC 
to provide which delay the PHY is supposed to do? I (now) know we can 
override the phy_mode via one of phy_attach*/phy_connect* function which 
take a phy_interface_t as argument. I see a phy_get_internal_delay but 
not the setter part of it. Anyway, don't want to drag this too much as I 
don't need this anymore since the delay will be added by the PHY.

> For linux, we have the policy that the PHY adds the delay, in an
> attempt to try to make all systems the same. And most boards follow
> this. And then we have a few systems that totally mess up delays, have
> odd configuration knobs nobody knows what the do exactly, and put the
> delays in the MAC.
> 
>> Does this also mean you cannot have mixed addition of delay? E.g. having 1ns
>> from MAC and 1ns from PHY? It has to be only on one of the IC?
> 
> It is not recommended, because of the policy that the PHY does the
> delay... You can, if you make use of the rx-internal-delay-ps
> properties, assuming both the MAC and PHY support them.
> 

Thanks to your and Jakob (in private)'s explanations, I now understand 
that this is very likely not required as I would assume HW engineers for 
the IC would essentially allow to add some delay between 1.2ns and 2ns 
to satisfy the RGMII v2.0 spec but there's no need to be precise or have 
granularity. So this scenario is theoretical and could very likely be 
due to accumulated HW mistakes (either in ICs or routing)?

>> In the comment at the bottom of the dt binding there's this following
>> sentence:
>>
>> # When the PCB does not implement the delays, the MAC or PHY must.  As
>> # such, this is software configuration, and so not described in Device
>> # Tree.
>>
>> Why do we have two possible locations for rx-internal-delay-ps: PHY and MAC?
> 
> Sometimes both can add variable delays. Sometimes it is fixed 2ns.
> 
> I would actually prefer that these properties were not used, because

I assume we would therefore need some kind of negotiation between the 
PHY and the MAC at the subsystem level to figure out if the PHY can do 
the requested delay or if it should be handled by the MAC instead?

> they indicate somebody messed up. If you read the RGMII standard, it
> says a 2ns delay is required between the clock and the data. It is as
> simple as that. If you need to fine tune it, it means one of:
> 
> The PCB is badly designed, care was not taken to ensure the PCB tracks
> are the same length.
> 
> The MAC is badly designed, it does not add 0ns/2ns, but some other
> delay.
> 
> The PHY is badly designed, it does not add 0ns/2ns, but some other
> delay.
> 

Just need to be in the 1-2ns but otherwise I fully agree, thanks for the 
hint of the RGMII spec that helped!

Thanks for taking the time to write this all down and your patience, 
that was all very helpful!

Cheers,
Quentin