linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] add ethernet support to rk3568 dts
@ 2021-07-28 16:10 Michael Riesch
  2021-07-28 16:10 ` [PATCH 1/2] arm64: dts: rockchip: add gmac0 node to rk3568 Michael Riesch
  2021-07-28 16:10 ` [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support Michael Riesch
  0 siblings, 2 replies; 10+ messages in thread
From: Michael Riesch @ 2021-07-28 16:10 UTC (permalink / raw)
  To: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel
  Cc: Rob Herring, Heiko Stuebner, Liang Chen, Peter Geis,
	Michael Riesch, Simon Xue

Hi all,

these patches should be orthogonal to the ongoing work of Peter Geis
that aims to introduce the GMAC1 node to the common RK356x dts.
The GMAC0 node, which is exclusive to the RK3568, and the Ethernet
phy nodes in the RK3568 EVB1 are introduced in this series.
The second patch bases on the dts in barebox by Sascha Hauer.

Best regards,
Michael

Michael Riesch (2):
  arm64: dts: rockchip: add gmac0 node to rk3568
  arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support

 .../boot/dts/rockchip/rk3568-evb1-v10.dts     | 69 +++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3568.dtsi      | 51 ++++++++++++++
 2 files changed, 120 insertions(+)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] arm64: dts: rockchip: add gmac0 node to rk3568
  2021-07-28 16:10 [PATCH 0/2] add ethernet support to rk3568 dts Michael Riesch
@ 2021-07-28 16:10 ` Michael Riesch
  2021-07-28 16:43   ` Heiko Stübner
  2021-07-28 16:10 ` [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support Michael Riesch
  1 sibling, 1 reply; 10+ messages in thread
From: Michael Riesch @ 2021-07-28 16:10 UTC (permalink / raw)
  To: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel
  Cc: Rob Herring, Heiko Stuebner, Liang Chen, Peter Geis,
	Michael Riesch, Simon Xue

While both RK3566 and RK3568 feature the gmac1 node, the gmac0
node is exclusive to the RK3568.

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 arch/arm64/boot/dts/rockchip/rk3568.dtsi | 51 ++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index da01a59f6f26..ec39a2c593b6 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -22,6 +22,57 @@
 		compatible = "rockchip,rk3568-qos", "syscon";
 		reg = <0x0 0xfe190200 0x0 0x20>;
 	};
+
+	gmac0: ethernet@fe2a0000 {
+		compatible = "rockchip,rk3568-gmac", "snps,dwmac-4.20a";
+		reg = <0x0 0xfe2a0000 0x0 0x10000>;
+		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "macirq", "eth_wake_irq";
+		rockchip,grf = <&grf>;
+		clocks = <&cru SCLK_GMAC0>, <&cru SCLK_GMAC0_RX_TX>,
+			 <&cru SCLK_GMAC0_RX_TX>, <&cru CLK_MAC0_REFOUT>,
+			 <&cru ACLK_GMAC0>, <&cru PCLK_GMAC0>,
+			 <&cru SCLK_GMAC0_RX_TX>, <&cru CLK_GMAC0_PTP_REF>,
+			 <&cru PCLK_XPCS>;
+		clock-names = "stmmaceth", "mac_clk_rx",
+			      "mac_clk_tx", "clk_mac_refout",
+			      "aclk_mac", "pclk_mac",
+			      "clk_mac_speed", "ptp_ref",
+			      "pclk_xpcs";
+		resets = <&cru SRST_A_GMAC0>;
+		reset-names = "stmmaceth";
+
+		snps,mixed-burst;
+		snps,tso;
+
+		snps,axi-config = <&gmac0_stmmac_axi_setup>;
+		snps,mtl-rx-config = <&gmac0_mtl_rx_setup>;
+		snps,mtl-tx-config = <&gmac0_mtl_tx_setup>;
+		status = "disabled";
+
+		mdio0: mdio {
+			compatible = "snps,dwmac-mdio";
+			#address-cells = <0x1>;
+			#size-cells = <0x0>;
+		};
+
+		gmac0_stmmac_axi_setup: stmmac-axi-config {
+			snps,wr_osr_lmt = <4>;
+			snps,rd_osr_lmt = <8>;
+			snps,blen = <0 0 0 0 16 8 4>;
+		};
+
+		gmac0_mtl_rx_setup: rx-queues-config {
+			snps,rx-queues-to-use = <1>;
+			queue0 {};
+		};
+
+		gmac0_mtl_tx_setup: tx-queues-config {
+			snps,tx-queues-to-use = <1>;
+			queue0 {};
+		};
+	};
 };
 
 &cpu0_opp_table {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support
  2021-07-28 16:10 [PATCH 0/2] add ethernet support to rk3568 dts Michael Riesch
  2021-07-28 16:10 ` [PATCH 1/2] arm64: dts: rockchip: add gmac0 node to rk3568 Michael Riesch
@ 2021-07-28 16:10 ` Michael Riesch
  2021-07-28 16:45   ` Johan Jonker
  2021-07-28 17:55   ` Andrew Lunn
  1 sibling, 2 replies; 10+ messages in thread
From: Michael Riesch @ 2021-07-28 16:10 UTC (permalink / raw)
  To: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel
  Cc: Rob Herring, Heiko Stuebner, Liang Chen, Peter Geis,
	Michael Riesch, Simon Xue

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 .../boot/dts/rockchip/rk3568-evb1-v10.dts     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
index 69786557093d..8f4c40d71914 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
@@ -13,6 +13,11 @@
 	model = "Rockchip RK3568 EVB1 DDR4 V10 Board";
 	compatible = "rockchip,rk3568-evb1-v10", "rockchip,rk3568";
 
+	aliases {
+		ethernet0 = &gmac0;
+		ethernet1 = &gmac1;
+	};
+
 	chosen: chosen {
 		stdout-path = "serial2:1500000n8";
 	};
@@ -67,6 +72,70 @@
 	};
 };
 
+&gmac0 {
+	phy-mode = "rgmii";
+	clock_in_out = "output";
+
+	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
+	assigned-clock-rates = <0>, <125000000>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac0_miim
+		     &gmac0_tx_bus2
+		     &gmac0_rx_bus2
+		     &gmac0_rgmii_clk
+		     &gmac0_rgmii_bus>;
+
+	tx_delay = <0x3c>;
+	rx_delay = <0x2f>;
+
+	phy-handle = <&rgmii_phy0>;
+	status = "okay";
+};
+
+&gmac1 {
+	phy-mode = "rgmii";
+	clock_in_out = "output";
+
+	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
+	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>;
+	assigned-clock-rates = <0>, <125000000>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac1m1_miim
+		     &gmac1m1_tx_bus2
+		     &gmac1m1_rx_bus2
+		     &gmac1m1_rgmii_clk
+		     &gmac1m1_rgmii_bus>;
+
+	tx_delay = <0x4f>;
+	rx_delay = <0x26>;
+
+	phy-handle = <&rgmii_phy1>;
+	status = "okay";
+};
+
+&mdio0 {
+	rgmii_phy0: phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reset-gpios = <&gpio2 RK_PD3 GPIO_ACTIVE_LOW>;
+		reset-assert-us = <20000>;
+		reset-deassert-us = <100000>;
+		reg = <0x0>;
+	};
+};
+
+&mdio1 {
+	rgmii_phy1: phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reset-gpios = <&gpio2 RK_PD1 GPIO_ACTIVE_LOW>;
+		reset-assert-us = <20000>;
+		reset-deassert-us = <100000>;
+		reg = <0x0>;
+	};
+};
+
 &sdhci {
 	bus-width = <8>;
 	max-frequency = <200000000>;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] arm64: dts: rockchip: add gmac0 node to rk3568
  2021-07-28 16:10 ` [PATCH 1/2] arm64: dts: rockchip: add gmac0 node to rk3568 Michael Riesch
@ 2021-07-28 16:43   ` Heiko Stübner
  2021-07-28 17:34     ` Heiko Stübner
  0 siblings, 1 reply; 10+ messages in thread
From: Heiko Stübner @ 2021-07-28 16:43 UTC (permalink / raw)
  To: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	Michael Riesch
  Cc: Rob Herring, Liang Chen, Peter Geis, Michael Riesch, Simon Xue

Hi,

Am Mittwoch, 28. Juli 2021, 18:10:19 CEST schrieb Michael Riesch:
> While both RK3566 and RK3568 feature the gmac1 node, the gmac0
> node is exclusive to the RK3568.
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  arch/arm64/boot/dts/rockchip/rk3568.dtsi | 51 ++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index da01a59f6f26..ec39a2c593b6 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -22,6 +22,57 @@
>  		compatible = "rockchip,rk3568-qos", "syscon";
>  		reg = <0x0 0xfe190200 0x0 0x20>;
>  	};
> +
> +	gmac0: ethernet@fe2a0000 {
> +		compatible = "rockchip,rk3568-gmac", "snps,dwmac-4.20a";
> +		reg = <0x0 0xfe2a0000 0x0 0x10000>;
> +		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "macirq", "eth_wake_irq";
> +		rockchip,grf = <&grf>;
> +		clocks = <&cru SCLK_GMAC0>, <&cru SCLK_GMAC0_RX_TX>,
> +			 <&cru SCLK_GMAC0_RX_TX>, <&cru CLK_MAC0_REFOUT>,
> +			 <&cru ACLK_GMAC0>, <&cru PCLK_GMAC0>,
> +			 <&cru SCLK_GMAC0_RX_TX>, <&cru CLK_GMAC0_PTP_REF>,
> +			 <&cru PCLK_XPCS>;
> +		clock-names = "stmmaceth", "mac_clk_rx",
> +			      "mac_clk_tx", "clk_mac_refout",
> +			      "aclk_mac", "pclk_mac",
> +			      "clk_mac_speed", "ptp_ref",
> +			      "pclk_xpcs";
> +		resets = <&cru SRST_A_GMAC0>;
> +		reset-names = "stmmaceth";
> +

is this missing a rockchip,grf phandle?

gmac1 has one and the driver side also does want to access the grf for both
controllers.


Heiko

> +		snps,mixed-burst;
> +		snps,tso;
> +
> +		snps,axi-config = <&gmac0_stmmac_axi_setup>;
> +		snps,mtl-rx-config = <&gmac0_mtl_rx_setup>;
> +		snps,mtl-tx-config = <&gmac0_mtl_tx_setup>;
> +		status = "disabled";
> +
> +		mdio0: mdio {
> +			compatible = "snps,dwmac-mdio";
> +			#address-cells = <0x1>;
> +			#size-cells = <0x0>;
> +		};
> +
> +		gmac0_stmmac_axi_setup: stmmac-axi-config {
> +			snps,wr_osr_lmt = <4>;
> +			snps,rd_osr_lmt = <8>;
> +			snps,blen = <0 0 0 0 16 8 4>;
> +		};
> +
> +		gmac0_mtl_rx_setup: rx-queues-config {
> +			snps,rx-queues-to-use = <1>;
> +			queue0 {};
> +		};
> +
> +		gmac0_mtl_tx_setup: tx-queues-config {
> +			snps,tx-queues-to-use = <1>;
> +			queue0 {};
> +		};
> +	};
>  };
>  
>  &cpu0_opp_table {
> 





^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support
  2021-07-28 16:10 ` [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support Michael Riesch
@ 2021-07-28 16:45   ` Johan Jonker
  2021-07-28 17:55   ` Andrew Lunn
  1 sibling, 0 replies; 10+ messages in thread
From: Johan Jonker @ 2021-07-28 16:45 UTC (permalink / raw)
  To: Michael Riesch, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel
  Cc: Rob Herring, Heiko Stuebner, Liang Chen, Peter Geis, Simon Xue

Hi Michael,

On 7/28/21 6:10 PM, Michael Riesch wrote:
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  .../boot/dts/rockchip/rk3568-evb1-v10.dts     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
> index 69786557093d..8f4c40d71914 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
> @@ -13,6 +13,11 @@
>  	model = "Rockchip RK3568 EVB1 DDR4 V10 Board";
>  	compatible = "rockchip,rk3568-evb1-v10", "rockchip,rk3568";
>  
> +	aliases {
> +		ethernet0 = &gmac0;
> +		ethernet1 = &gmac1;
> +	};
> +
>  	chosen: chosen {
>  		stdout-path = "serial2:1500000n8";
>  	};
> @@ -67,6 +72,70 @@
>  	};
>  };
>  
> +&gmac0 {
> +	phy-mode = "rgmii";
> +	clock_in_out = "output";
> +
> +	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> +	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> +	assigned-clock-rates = <0>, <125000000>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gmac0_miim
> +		     &gmac0_tx_bus2
> +		     &gmac0_rx_bus2
> +		     &gmac0_rgmii_clk
> +		     &gmac0_rgmii_bus>;
> +
> +	tx_delay = <0x3c>;
> +	rx_delay = <0x2f>;
> +
> +	phy-handle = <&rgmii_phy0>;
> +	status = "okay";
> +};
> +
> +&gmac1 {
> +	phy-mode = "rgmii";
> +	clock_in_out = "output";
> +
> +	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> +	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>;
> +	assigned-clock-rates = <0>, <125000000>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gmac1m1_miim
> +		     &gmac1m1_tx_bus2
> +		     &gmac1m1_rx_bus2
> +		     &gmac1m1_rgmii_clk
> +		     &gmac1m1_rgmii_bus>;
> +
> +	tx_delay = <0x4f>;
> +	rx_delay = <0x26>;
> +
> +	phy-handle = <&rgmii_phy1>;
> +	status = "okay";
> +};
> +
> +&mdio0 {

> +	rgmii_phy0: phy@0 {

Could you test with ethernet-phy.yaml?

  $nodename:
    pattern: "^ethernet-phy(@[a-f0-9]+)?$"

> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reset-gpios = <&gpio2 RK_PD3 GPIO_ACTIVE_LOW>;
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;
> +		reg = <0x0>;

Sort order is:
compatible
reg
(interrupts)
The rest in alphabetical order.

> +	};
> +};
> +
> +&mdio1 {

> +	rgmii_phy1: phy@0 {

dito

> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reset-gpios = <&gpio2 RK_PD1 GPIO_ACTIVE_LOW>;
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;
> +		reg = <0x0>;

dito

> +	};
> +};
> +
>  &sdhci {
>  	bus-width = <8>;
>  	max-frequency = <200000000>;
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] arm64: dts: rockchip: add gmac0 node to rk3568
  2021-07-28 16:43   ` Heiko Stübner
@ 2021-07-28 17:34     ` Heiko Stübner
  0 siblings, 0 replies; 10+ messages in thread
From: Heiko Stübner @ 2021-07-28 17:34 UTC (permalink / raw)
  To: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	Michael Riesch
  Cc: Rob Herring, Liang Chen, Peter Geis, Michael Riesch, Simon Xue

Am Mittwoch, 28. Juli 2021, 18:43:24 CEST schrieb Heiko Stübner:
> Hi,
> 
> Am Mittwoch, 28. Juli 2021, 18:10:19 CEST schrieb Michael Riesch:
> > While both RK3566 and RK3568 feature the gmac1 node, the gmac0
> > node is exclusive to the RK3568.
> > 
> > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3568.dtsi | 51 ++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > index da01a59f6f26..ec39a2c593b6 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > @@ -22,6 +22,57 @@
> >  		compatible = "rockchip,rk3568-qos", "syscon";
> >  		reg = <0x0 0xfe190200 0x0 0x20>;
> >  	};
> > +
> > +	gmac0: ethernet@fe2a0000 {
> > +		compatible = "rockchip,rk3568-gmac", "snps,dwmac-4.20a";
> > +		reg = <0x0 0xfe2a0000 0x0 0x10000>;
> > +		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> > +		interrupt-names = "macirq", "eth_wake_irq";
> > +		rockchip,grf = <&grf>;

Johan thankfully pointed out that the grf is hiding up here, so this should
move below reset-names ;-)


Heiko


> > +		clocks = <&cru SCLK_GMAC0>, <&cru SCLK_GMAC0_RX_TX>,
> > +			 <&cru SCLK_GMAC0_RX_TX>, <&cru CLK_MAC0_REFOUT>,
> > +			 <&cru ACLK_GMAC0>, <&cru PCLK_GMAC0>,
> > +			 <&cru SCLK_GMAC0_RX_TX>, <&cru CLK_GMAC0_PTP_REF>,
> > +			 <&cru PCLK_XPCS>;
> > +		clock-names = "stmmaceth", "mac_clk_rx",
> > +			      "mac_clk_tx", "clk_mac_refout",
> > +			      "aclk_mac", "pclk_mac",
> > +			      "clk_mac_speed", "ptp_ref",
> > +			      "pclk_xpcs";
> > +		resets = <&cru SRST_A_GMAC0>;
> > +		reset-names = "stmmaceth";
> > +
> 
> is this missing a rockchip,grf phandle?
> 
> gmac1 has one and the driver side also does want to access the grf for both
> controllers.
> 
> 
> Heiko
> 
> > +		snps,mixed-burst;
> > +		snps,tso;
> > +
> > +		snps,axi-config = <&gmac0_stmmac_axi_setup>;
> > +		snps,mtl-rx-config = <&gmac0_mtl_rx_setup>;
> > +		snps,mtl-tx-config = <&gmac0_mtl_tx_setup>;
> > +		status = "disabled";
> > +
> > +		mdio0: mdio {
> > +			compatible = "snps,dwmac-mdio";
> > +			#address-cells = <0x1>;
> > +			#size-cells = <0x0>;
> > +		};
> > +
> > +		gmac0_stmmac_axi_setup: stmmac-axi-config {
> > +			snps,wr_osr_lmt = <4>;
> > +			snps,rd_osr_lmt = <8>;
> > +			snps,blen = <0 0 0 0 16 8 4>;
> > +		};
> > +
> > +		gmac0_mtl_rx_setup: rx-queues-config {
> > +			snps,rx-queues-to-use = <1>;
> > +			queue0 {};
> > +		};
> > +
> > +		gmac0_mtl_tx_setup: tx-queues-config {
> > +			snps,tx-queues-to-use = <1>;
> > +			queue0 {};
> > +		};
> > +	};
> >  };
> >  
> >  &cpu0_opp_table {
> > 
> 
> 





^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support
  2021-07-28 16:10 ` [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support Michael Riesch
  2021-07-28 16:45   ` Johan Jonker
@ 2021-07-28 17:55   ` Andrew Lunn
  2021-07-28 18:29     ` Peter Geis
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2021-07-28 17:55 UTC (permalink / raw)
  To: Michael Riesch
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	Rob Herring, Heiko Stuebner, Liang Chen, Peter Geis, Simon Xue

On Wed, Jul 28, 2021 at 06:10:20PM +0200, Michael Riesch wrote:
> +&gmac0 {
> +	phy-mode = "rgmii";

...
> +
> +	tx_delay = <0x3c>;
> +	rx_delay = <0x2f>;

Hi Michael

In general, we try to have the PHY introduce the RGMII delays, not the
MAC. Did you try

phy-mode = "rgmii-id";

and remove these delay values? It is hard for me to say if that will
work because i've no idea what 0x3c and 0x2f means? Are they
equivalent to 2ns?

     Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support
  2021-07-28 17:55   ` Andrew Lunn
@ 2021-07-28 18:29     ` Peter Geis
  2021-07-28 20:37       ` Andrew Lunn
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Geis @ 2021-07-28 18:29 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Michael Riesch, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List, Rob Herring, Heiko Stuebner,
	Liang Chen, Simon Xue

On Wed, Jul 28, 2021 at 1:55 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Jul 28, 2021 at 06:10:20PM +0200, Michael Riesch wrote:
> > +&gmac0 {
> > +     phy-mode = "rgmii";
>
> ...
> > +
> > +     tx_delay = <0x3c>;
> > +     rx_delay = <0x2f>;
>
> Hi Michael
>
> In general, we try to have the PHY introduce the RGMII delays, not the
> MAC. Did you try
>
> phy-mode = "rgmii-id";
>
> and remove these delay values? It is hard for me to say if that will
> work because i've no idea what 0x3c and 0x2f means? Are they
> equivalent to 2ns?

Unfortunately the driver and TRM are both rather non-specific as to
how this works.
The driver sets the tx_delay to 0x30 and rx_delay to 0x10 if these
values are not defined, or sets them both to 0 in case of rgmii_id.

Generally all rockchip boards use this value instead of the rgmii_id,
I imagine because it's more consistent to tune here than the hit or
miss support of the phy drivers.
The usual course of action is to test to find the lowest and highest
working values and take the median value to plug in here.

>
>      Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support
  2021-07-28 18:29     ` Peter Geis
@ 2021-07-28 20:37       ` Andrew Lunn
  2021-07-29  9:07         ` Michael Riesch
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2021-07-28 20:37 UTC (permalink / raw)
  To: Peter Geis
  Cc: Michael Riesch, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List, Rob Herring, Heiko Stuebner,
	Liang Chen, Simon Xue

> Generally all rockchip boards use this value instead of the rgmii_id,
> I imagine because it's more consistent to tune here than the hit or
> miss support of the phy drivers.

Most PHY drivers actually implement it correctly, since by default,
most systems get the PHY to do the delays.

But if most Rockchip boards do it this way, there is a lot to be said
for consistence, so this is fine by me.

    Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support
  2021-07-28 20:37       ` Andrew Lunn
@ 2021-07-29  9:07         ` Michael Riesch
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Riesch @ 2021-07-29  9:07 UTC (permalink / raw)
  To: Andrew Lunn, Peter Geis
  Cc: devicetree, arm-mail-list, open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List, Rob Herring, Heiko Stuebner,
	Liang Chen, Simon Xue

Hello Andrew, Peter,

On 7/28/21 10:37 PM, Andrew Lunn wrote:
>> Generally all rockchip boards use this value instead of the rgmii_id,
>> I imagine because it's more consistent to tune here than the hit or
>> miss support of the phy drivers.
> 
> Most PHY drivers actually implement it correctly, since by default,
> most systems get the PHY to do the delays.
> 
> But if most Rockchip boards do it this way, there is a lot to be said
> for consistence, so this is fine by me.

I have tested a dts without the delays and with phy-mode = "rgmii-id"
and it seems to work just fine.

Although consistency with other Rockchip boards is something one should
consider, I think I'll go along the "rgmii-id" path since this seems to
be a more general convention.

Thanks for your comments, I'll submit a v2.

Regards, Michael

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-07-29  9:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-28 16:10 [PATCH 0/2] add ethernet support to rk3568 dts Michael Riesch
2021-07-28 16:10 ` [PATCH 1/2] arm64: dts: rockchip: add gmac0 node to rk3568 Michael Riesch
2021-07-28 16:43   ` Heiko Stübner
2021-07-28 17:34     ` Heiko Stübner
2021-07-28 16:10 ` [PATCH 2/2] arm64: dts: rockchip: rk3568-evb1-v10: add ethernet support Michael Riesch
2021-07-28 16:45   ` Johan Jonker
2021-07-28 17:55   ` Andrew Lunn
2021-07-28 18:29     ` Peter Geis
2021-07-28 20:37       ` Andrew Lunn
2021-07-29  9:07         ` Michael Riesch

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