Skip to contentSkip to conversation

arm64: Add basic support for QNAP TS-433

PATCH Applied Uwe Kleine-König 6 participants 14 messages Feb 27, 2024 – Feb 28, 2024

Subsystems (2)
Device tree bindings Arm Rockchip
lore
Share & tools
lore

Uwe Kleine-König

author
#0 lore raw

cover arm64: Add basic support for QNAP TS-433

Hello,

this is an initial dts to support QNAP's TS-433 NAS. It's enough to start the debian installer with a custom built kernel (original Debian kernel fails to open a console, didn't debug that yet) and access HD, eMMC, RTC and network.

There are still some missing bits and pieces, but to make people aware there are efforts to support this machine and so prevent duplicate work, I think it makes sense to add the dts in its current form already.

Best regards
Uwe

Uwe Kleine-König (2):
dt-bindings: arm: rockchip: Add QNAP TS-433
arm64: dts: rockchip: Add basic support for QNAP TS-433

 .../devicetree/bindings/arm/rockchip.yaml     |  5 ++
 arch/arm64/boot/dts/rockchip/Makefile         |  1 +
 .../boot/dts/rockchip/rk3568-qnap-ts433.dts   | 86 +++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts

base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d

Signature · 1 line
2.43.0
author
#1 lore raw

PATCH 1/2 dt-bindings: arm: rockchip: Add QNAP TS-433

This is a SOHO NAS with 4 hd bays and 4 GB of RAM.

Signed-off-by Uwe Kleine-König <[email protected]>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/arm/rockchip.yaml +5 -0
index 5cf5cbef2cf5..389bf0c7d702 100644
@@ -697,6 +697,11 @@ properties:
               - powkiddy,x55
           - const: rockchip,rk3566
 
+      - description: QNAP TS-433-4G 4-Bay NAS
+        items:
+          - const: qnap,ts433
+          - const: rockchip,rk3568
+
       - description: Radxa Compute Module 3(CM3)
         items:
           - enum:
Signature · 1 line
2.43.0
author
#3 lore raw

PATCH 2/2 arm64: dts: rockchip: Add basic support for QNAP TS-433

In reply to Uwe Kleine-König

This is enough to make eMMC, networking, UART (console), RTC and a hard disk accessible. Still missing are (at least): USB, LEDs, regulators, fan.

Signed-off-by Uwe Kleine-König <[email protected]>
---
 arch/arm64/boot/dts/rockchip/Makefile         |  1 +
 .../boot/dts/rockchip/rk3568-qnap-ts433.dts   | 86 +++++++++++++++++++
 2 files changed, 87 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
arch/arm64/boot/dts/rockchip/Makefile +1 -0
index a7b30e11beaf..a28a6d445929 100644
@@ -98,6 +98,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-lubancat-2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-nanopi-r5c.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-nanopi-r5s.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-qnap-ts433.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-radxa-e25.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-roc-pc.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-rock-3a.dtb
arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts new file +86 -0

86 lines not on this page · raw message

Signature · 1 line
2.43.0
#4 lore raw

Re: [PATCH 2/2] arm64: dts: rockchip: Add basic support for QNAP TS-433

Quoted 5 lines

+&gmac0 {

arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts new file +4 -0
+	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>;
+	assigned-clock-rates = <0>, <125000000>;
+	clock_in_out = "output";

arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts line 50

Quoted 10 lines
arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts new file +10 -0
+	phy-handle = <&rgmii_phy0>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac0_miim
+		     &gmac0_tx_bus2
+		     &gmac0_rx_bus2
+		     &gmac0_rgmii_clk
+		     &gmac0_rgmii_bus>;
+	rx_delay = <0x2f>;
+	tx_delay = <0x3c>;

Have you tried phy-mode = "rgmii-id"; and not have these two _delay settings?

In general, we try to have the PHY do the delay, not the MAC, so that all devices work the same.

Andrew

author
#5 lore raw

Re: [PATCH 2/2] arm64: dts: rockchip: Add basic support for QNAP TS-433

Quoted 10 lines · Andrew Lunn
Quoted 10 lines

+&gmac0 {

(quoted diff) +9 -0
+	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>;
+	assigned-clock-rates = <0>, <125000000>;
+	clock_in_out = "output";
+	phy-handle = <&rgmii_phy0>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac0_miim
+		     &gmac0_tx_bus2
Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+		     &gmac0_rx_bus2
+		     &gmac0_rgmii_clk
+		     &gmac0_rgmii_bus>;
+	rx_delay = <0x2f>;
+	tx_delay = <0x3c>;

Have you tried phy-mode = "rgmii-id"; and not have these two _delay settings?

I didnt' up to now. I applied the following on top of my dts:

arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts +1 -3
index ba7137f80075..a8747d9f36da 100644
@@ -39,15 +39,13 @@ &gmac0 {
 	assigned-clock-rates = <0>, <125000000>;
 	clock_in_out = "output";
 	phy-handle = <&rgmii_phy0>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	pinctrl-names = "default";
 	pinctrl-0 = <&gmac0_miim
 		     &gmac0_tx_bus2
 		     &gmac0_rx_bus2
 		     &gmac0_rgmii_clk
 		     &gmac0_rgmii_bus>;
-	rx_delay = <0x2f>;
-	tx_delay = <0x3c>;
 	status = "okay";
 };

and this makes the machine unable to complete dhcp. I see the requests and replies on the dhcp server side, but the machine tells me not to receive a dhcp reply. So the above patch breaks the receive path.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
#6 lore raw

Re: [PATCH 2/2] arm64: dts: rockchip: Add basic support for QNAP TS-433

Quoted 43 lines · Uwe Kleine-König
Quoted 10 lines · Andrew Lunn
Quoted 10 lines

+&gmac0 {

(quoted diff) +9 -0
+	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>;
+	assigned-clock-rates = <0>, <125000000>;
+	clock_in_out = "output";
+	phy-handle = <&rgmii_phy0>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac0_miim
+		     &gmac0_tx_bus2
Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+		     &gmac0_rx_bus2
+		     &gmac0_rgmii_clk
+		     &gmac0_rgmii_bus>;
+	rx_delay = <0x2f>;
+	tx_delay = <0x3c>;

Have you tried phy-mode = "rgmii-id"; and not have these two _delay settings?

I didnt' up to now. I applied the following on top of my dts:

arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts +1 -3
index ba7137f80075..a8747d9f36da 100644
@@ -39,15 +39,13 @@ &gmac0 {
 	assigned-clock-rates = <0>, <125000000>;
 	clock_in_out = "output";
 	phy-handle = <&rgmii_phy0>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	pinctrl-names = "default";
 	pinctrl-0 = <&gmac0_miim
 		     &gmac0_tx_bus2
 		     &gmac0_rx_bus2
 		     &gmac0_rgmii_clk
 		     &gmac0_rgmii_bus>;
-	rx_delay = <0x2f>;
-	tx_delay = <0x3c>;
 	status = "okay";
 };
Quoted 3 lines

and this makes the machine unable to complete dhcp. I see the requests and replies on the dhcp server side, but the machine tells me not to receive a dhcp reply. So the above patch breaks the receive path.

O.K.

This binding is particularly bad. What does 0x3c mean? Generally, we use rx-internal-delay-ps making it clear what the value means.

RGMII requires a 2ns delay between the clock and the data. Generally, we have the MAC insert 0 delay, and request the PHY add the 2ns delay by specifying "rgmii-id". Sometimes you need to fine tune this, because of the length of the tracks etc. You can then do that fine tuning either in the PHY, or the MAC.

Looking at the binding:

tx_delay:

    description: Delay value for TXD timing.
    $ref: /schemas/types.yaml#/definitions/uint32
    minimum: 0
    maximum: 0x7F
    default: 0x30

rx_delay:

    description: Delay value for RXD timing.
    $ref: /schemas/types.yaml#/definitions/uint32
    minimum: 0
    maximum: 0x7F
    default: 0x10

For your board, you have increased both values from there default values. My guess is 0x30 tx_delay is 2ns, and 0x10 rx_delay is also 2ns.

So maybe try:

rx_delay = <0x1f>;
tx_delay = <0x0c>;

combined with rmgii-id.

Andrew

author
#7 lore raw

Re: [PATCH 2/2] arm64: dts: rockchip: Add basic support for QNAP TS-433

Quoted 79 lines · Andrew Lunn
Quoted 43 lines · Uwe Kleine-König
Quoted 10 lines · Andrew Lunn
Quoted 10 lines

+&gmac0 {

(quoted diff) +9 -0
+	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>;
+	assigned-clock-rates = <0>, <125000000>;
+	clock_in_out = "output";
+	phy-handle = <&rgmii_phy0>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac0_miim
+		     &gmac0_tx_bus2
Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+		     &gmac0_rx_bus2
+		     &gmac0_rgmii_clk
+		     &gmac0_rgmii_bus>;
+	rx_delay = <0x2f>;
+	tx_delay = <0x3c>;

Have you tried phy-mode = "rgmii-id"; and not have these two _delay settings?

I didnt' up to now. I applied the following on top of my dts:

arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts +1 -3
index ba7137f80075..a8747d9f36da 100644
@@ -39,15 +39,13 @@ &gmac0 {
  	assigned-clock-rates = <0>, <125000000>;
  	clock_in_out = "output";
  	phy-handle = <&rgmii_phy0>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
  	pinctrl-names = "default";
  	pinctrl-0 = <&gmac0_miim
  		     &gmac0_tx_bus2
  		     &gmac0_rx_bus2
  		     &gmac0_rgmii_clk
  		     &gmac0_rgmii_bus>;
-	rx_delay = <0x2f>;
-	tx_delay = <0x3c>;
  	status = "okay";
  };
Quoted 3 lines

and this makes the machine unable to complete dhcp. I see the requests and replies on the dhcp server side, but the machine tells me not to receive a dhcp reply. So the above patch breaks the receive path.

O.K.

This binding is particularly bad. What does 0x3c mean? Generally, we use rx-internal-delay-ps making it clear what the value means.

RGMII requires a 2ns delay between the clock and the data. Generally, we have the MAC insert 0 delay, and request the PHY add the 2ns delay by specifying "rgmii-id". Sometimes you need to fine tune this, because of the length of the tracks etc. You can then do that fine tuning either in the PHY, or the MAC.

Looking at the binding:

   tx_delay:
     description: Delay value for TXD timing.
     $ref: /schemas/types.yaml#/definitions/uint32
     minimum: 0
     maximum: 0x7F
     default: 0x30

   rx_delay:
     description: Delay value for RXD timing.
     $ref: /schemas/types.yaml#/definitions/uint32
     minimum: 0
     maximum: 0x7F
     default: 0x10

For your board, you have increased both values from there default values. My guess is 0x30 tx_delay is 2ns, and 0x10 rx_delay is also 2ns.

Quoted 6 lines

So maybe try:

rx_delay = <0x1f>;
tx_delay = <0x0c>;

combined with rmgii-id.

With the right phy driver (MOTORCOMM_PHY) enabled, it works also without specifying {rx,tx}_delay. Thanks to Oleksij for the relevant hint. I'll switch to that then.

Best regards
Uwe

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

maintainer
#8 lore raw
In reply to Uwe Kleine-König

Am Dienstag, 27. Februar 2024, 12:52:35 CET schrieb Uwe Kleine-König:

Quoted 10 lines

Hello,

this is an initial dts to support QNAP's TS-433 NAS. It's enough to start the debian installer with a custom built kernel (original Debian kernel fails to open a console, didn't debug that yet) and access HD, eMMC, RTC and network.

There are still some missing bits and pieces, but to make people aware there are efforts to support this machine and so prevent duplicate work, I think it makes sense to add the dts in its current form already.

woohoo and yes we're adding support for not 100% supported boards all the time, so adding basic support and extending it later is fully ok.

For people thinking about "following along at home", how much voodoo is involved to make the device boot said Debian installer? ;-)

Heiko

#9 lore raw

Hello Heiko,

Quoted 10 lines · Heiko Stübner

Am Dienstag, 27. Februar 2024, 12:52:35 CET schrieb Uwe Kleine-König:

Quoted 8 lines

this is an initial dts to support QNAP's TS-433 NAS. It's enough to start the debian installer with a custom built kernel (original Debian kernel fails to open a console, didn't debug that yet) and access HD, eMMC, RTC and network.

There are still some missing bits and pieces, but to make people aware there are efforts to support this machine and so prevent duplicate work, I think it makes sense to add the dts in its current form already.

Quoted 5 lines

woohoo and yes we're adding support for not 100% supported boards all the time, so adding basic support and extending it later is fully ok.

For people thinking about "following along at home", how much voodoo is involved to make the device boot said Debian installer? ;-)

I'd ask these people for some patience as I intend to document the details in the Debian wiki.

For those who are not that patient, here comes the short version:
I compiled v6.8-rc1 with this patch, put the resulting kernel image (Image) and device tree into /srv/tftp, together with the initrd[1] from Debian and then typed in U-boot (using the vendor version):

	setenv laptop 192.168.144.173
	setenv bootargs ${bootargs} clk_ignore_unused ignore_loglevel
	i2c dev 0; i2c mw 0x20 0xf4 0x10; i2c mw 0x40 0x00 0xa3; run phy_cmd
	dhcp ${fdt_addr_r} ${laptop}:rk3568-qnap-ts433.dtb
	dhcp ${kernel_addr_r} ${laptop}:Image
	dhcp 0x0b100000 ${laptop}:debian/20240222-02:09/initrd.gz
	booti ${kernel_addr_r} 0x0b100000:$filesize ${fdt_addr_r}

Feel free to ping me on irc if you have further questions.

Best regards
Uwe

[1] I used
https://d-i.debian.org/daily-images/arm64/20240222-02:09/netboot/debian-installer/arm64/initrd.gz

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
maintainer
#10 lore raw
In reply to Uwe Kleine-König
Quoted 24 lines · Uwe Kleine-König

Hello,

this is an initial dts to support QNAP's TS-433 NAS. It's enough to start the debian installer with a custom built kernel (original Debian kernel fails to open a console, didn't debug that yet) and access HD, eMMC, RTC and network.

There are still some missing bits and pieces, but to make people aware there are efforts to support this machine and so prevent duplicate work, I think it makes sense to add the dts in its current form already.

Best regards
Uwe

Uwe Kleine-König (2):
dt-bindings: arm: rockchip: Add QNAP TS-433
arm64: dts: rockchip: Add basic support for QNAP TS-433

 .../devicetree/bindings/arm/rockchip.yaml     |  5 ++
 arch/arm64/boot/dts/rockchip/Makefile         |  1 +
 .../boot/dts/rockchip/rk3568-qnap-ts433.dts   | 86 +++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
Quoted 5 lines

base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d --
2.43.0

My bot found new DT warnings on the .dts files added or changed in this series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not.

If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date:

  pip3 install dtschema --upgrade

New warnings running 'make CHECK_DTBS=y rockchip/rk3568-qnap-ts433.dtb' for [email protected]:

arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dtb: video-codec@fdea0400: 'interrupt-names' is a required property
from schema $id: http://devicetree.org/schemas/media/rockchip-vpu.yaml#
arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dtb: i2s@fe420000: reset-names:0: 'm' is not one of ['tx-m', 'rx-m']
from schema $id: http://devicetree.org/schemas/sound/rockchip,i2s-tdm.yaml#

#11 lore raw

Hello Rob,

Quoted 6 lines · Rob Herring

New warnings running 'make CHECK_DTBS=y rockchip/rk3568-qnap-ts433.dtb' for [email protected]:

arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dtb: video-codec@fdea0400: 'interrupt-names' is a required property
from schema $id: http://devicetree.org/schemas/media/rockchip-vpu.yaml#
arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dtb: i2s@fe420000: reset-names:0: 'm' is not one of ['tx-m', 'rx-m']
from schema $id: http://devicetree.org/schemas/sound/rockchip,i2s-tdm.yaml#

Yes, I saw these, too. But given that the added dts doesn't touch these two nodes, I thought this to be a problem of rk3568.dtsi that I can ignore for this series.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
maintainer
#12 lore raw

Am Dienstag, 27. Februar 2024, 14:55:57 CET schrieb Uwe Kleine-König:

Quoted 5 lines

Hello Rob,

Quoted 2 lines · Rob Herring

New warnings running 'make CHECK_DTBS=y rockchip/rk3568-qnap-ts433.dtb' for [email protected]:

Quoted 8 lines
Quoted 4 lines

arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dtb: video-codec@fdea0400: 'interrupt-names' is a required property
from schema $id: http://devicetree.org/schemas/media/rockchip-vpu.yaml#
arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dtb: i2s@fe420000: reset-names:0: 'm' is not one of ['tx-m', 'rx-m']
from schema $id: http://devicetree.org/schemas/sound/rockchip,i2s-tdm.yaml#

Yes, I saw these, too. But given that the added dts doesn't touch these two nodes, I thought this to be a problem of rk3568.dtsi that I can ignore for this series.

I think I found suitable fixes for those in [0].

The question would be, if the bot finding errors causes the patch to get removed from the review queue, because the binding addition still would profit from an Ack :-)

Heiko

[0] https://lore.kernel.org/linux-rockchip/[email protected]/

maintainer Applied
#13 lore raw
In reply to Uwe Kleine-König
Quoted 10 lines · Uwe Kleine-König

this is an initial dts to support QNAP's TS-433 NAS. It's enough to start the debian installer with a custom built kernel (original Debian kernel fails to open a console, didn't debug that yet) and access HD, eMMC, RTC and network.

There are still some missing bits and pieces, but to make people aware there are efforts to support this machine and so prevent duplicate work, I think it makes sense to add the dts in its current form already.

[...]

Applied, thanks!

[1/2] dt-bindings: arm: rockchip: Add QNAP TS-433

      commit: 0660dd951e1a09e155e31732fbdf735112b5d6c4
[2/2] arm64: dts: rockchip: Add basic support for QNAP TS-433
      commit: 9da1c0327d58e0cfc2d86799192585ddeb1ae4a0

Moved &gmac above &i2c* - alphabetical sorting and all

Best regards,

Signature · 1 line
Heiko Stuebner <[email protected]>