diff mbox series

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

Message ID 0d9fa5d730ac1cb91261b25b6809fcef3a12f03a.1709034476.git.ukleinek@debian.org (mailing list archive)
State New, archived
Headers show
Series arm64: Add basic support for QNAP TS-433 | expand

Commit Message

Uwe Kleine-König Feb. 27, 2024, 11:52 a.m. UTC
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 <ukleinek@debian.org>
---
 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

Comments

Andrew Lunn Feb. 27, 2024, 9 p.m. UTC | #1
> +&gmac0 {
> +	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
> +		     &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
Uwe Kleine-König Feb. 28, 2024, 7:23 a.m. UTC | #2
On Tue, Feb 27, 2024 at 10:00:48PM +0100, Andrew Lunn wrote:
> > +&gmac0 {
> > +	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
> > +		     &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:

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
index ba7137f80075..a8747d9f36da 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
@@ -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
Andrew Lunn Feb. 28, 2024, 1:53 p.m. UTC | #3
On Wed, Feb 28, 2024 at 08:23:33AM +0100, Uwe Kleine-König wrote:
> On Tue, Feb 27, 2024 at 10:00:48PM +0100, Andrew Lunn wrote:
> > > +&gmac0 {
> > > +	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
> > > +		     &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:
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
> index ba7137f80075..a8747d9f36da 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
> @@ -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.

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
Uwe Kleine-König Feb. 28, 2024, 5:05 p.m. UTC | #4
On 28.02.24 14:53, Andrew Lunn wrote:
> On Wed, Feb 28, 2024 at 08:23:33AM +0100, Uwe Kleine-König wrote:
>> On Tue, Feb 27, 2024 at 10:00:48PM +0100, Andrew Lunn wrote:
>>>> +&gmac0 {
>>>> +	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
>>>> +		     &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:
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
>> index ba7137f80075..a8747d9f36da 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
>> @@ -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.
> 
> 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.

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
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index a7b30e11beaf..a28a6d445929 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -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
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
new file mode 100644
index 000000000000..ba7137f80075
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
@@ -0,0 +1,86 @@ 
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
+ * Copyright (c) 2024 Uwe Kleine-König
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include "rk3568.dtsi"
+
+/ {
+	model = "Qnap TS-433-4G NAS System 4-Bay";
+	compatible = "qnap,ts433", "rockchip,rk3568";
+};
+
+&i2c0 {
+	pmic@20 {
+		compatible = "rockchip,rk809";
+		reg = <0x20>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+	};
+};
+
+&i2c1 {
+	status = "okay";
+
+	rtc@51 {
+		compatible = "microcrystal,rv8263";
+		reg = <0x51>;
+		wakeup-source;
+	};
+};
+
+&gmac0 {
+	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
+		     &gmac0_rx_bus2
+		     &gmac0_rgmii_clk
+		     &gmac0_rgmii_bus>;
+	rx_delay = <0x2f>;
+	tx_delay = <0x3c>;
+	status = "okay";
+};
+
+&mdio0 {
+	rgmii_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0x0>;
+	};
+};
+
+&pcie30phy {
+	status = "okay";
+};
+
+&pcie3x1 {
+	/* The downstream dts has: rockchip,bifurcation, XXX: find out what this is about */
+	reset-gpios = <&gpio0 RK_PC7 GPIO_ACTIVE_HIGH>;
+	status = "okay";
+};
+
+&sdhci {
+	bus-width = <8>;
+	max-frequency = <200000000>;
+	non-removable;
+	status = "okay";
+};
+
+/*
+ * Pins available on CN2 connector at TTL voltage level (3V3).
+ * ,_  _.
+ * |1234|  1=TX 2=VCC
+ * `----'  3=RX 4=GND
+ */
+&uart2 {
+	status = "okay";
+};