[linux-next-v1 1/3] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A sbc

[linux-next-v1 1/3] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A sbc

[Anand Moon]

On rk356x ethernet phy support reduced media independent interface (RMII)
and reduced gigabit media independent interface (RGMII).
So set the phy mode to rgmii.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index ea74ba32fbbd..72e410e3aca8 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -253,7 +253,7 @@ &gmac1 {
 	assigned-clock-rates = <0>, <125000000>;
 	clock_in_out = "output";
 	phy-handle = <&rgmii_phy1>;
-	phy-mode = "rgmii-id";
+	phy-mode = "rgmii";
 	pinctrl-names = "default";
 	pinctrl-0 = <&gmac1m1_miim
 		     &gmac1m1_tx_bus2
-- 
2.38.1

[linux-next-v1 2/3] arm64: dts: rockchip: Add support of external clock to ethernet node on Rock 3A sbc

[Anand Moon]

Add support of external clock gmac1_clkin which is used as input clock
to ethernet node.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index 72e410e3aca8..16fff1ada195 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -32,6 +32,13 @@ hdmi_con_in: endpoint {
 		};
 	};
 
+	gmac1_clkin: external-gmac1-clock {
+		compatible = "fixed-clock";
+		clock-frequency = <125000000>;
+		clock-output-names = "gmac1_clkin";
+		#clock-cells = <0>;
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
@@ -249,9 +256,8 @@ &cpu3 {
 
 &gmac1 {
 	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
-	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>;
-	assigned-clock-rates = <0>, <125000000>;
-	clock_in_out = "output";
+	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&gmac1_clkin>;
+	clock_in_out = "input";
 	phy-handle = <&rgmii_phy1>;
 	phy-mode = "rgmii";
 	pinctrl-names = "default";
@@ -259,6 +265,7 @@ &gmac1 {
 		     &gmac1m1_tx_bus2
 		     &gmac1m1_rx_bus2
 		     &gmac1m1_rgmii_clk
+		     &gmac1m1_clkinout
 		     &gmac1m1_rgmii_bus>;
 	status = "okay";
 };
-- 
2.38.1

[linux-next-v1 3/3] arm64: dts: rockchip: Fix ethernet reset node Rock 3A sbc

[Anand Moon]

Add support for snps,reset-gpio reset ethernet gpio pins and
drop the mdio reset code.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 .../boot/dts/rockchip/rk3568-rock-3a.dts      | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index 16fff1ada195..9172cf60b867 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -258,6 +258,11 @@ &gmac1 {
 	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
 	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&gmac1_clkin>;
 	clock_in_out = "input";
+	snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
+	snps,reset-active-low;
+	/* Reset time is 20ms, 100ms for rtl8211f */
+	snps,reset-delays-us = <0 20000 100000>;
+	phy-supply = <&vcc_3v3>;
 	phy-handle = <&rgmii_phy1>;
 	phy-mode = "rgmii";
 	pinctrl-names = "default";
@@ -267,6 +272,9 @@ &gmac1m1_rx_bus2
 		     &gmac1m1_rgmii_clk
 		     &gmac1m1_clkinout
 		     &gmac1m1_rgmii_bus>;
+
+	tx_delay = <0x4f>;
+	rx_delay = <0x26>;
 	status = "okay";
 };
 
@@ -583,11 +591,6 @@ &mdio1 {
 	rgmii_phy1: ethernet-phy@0 {
 		compatible = "ethernet-phy-ieee802.3-c22";
 		reg = <0x0>;
-		pinctrl-names = "default";
-		pinctrl-0 = <&eth_phy_rst>;
-		reset-assert-us = <20000>;
-		reset-deassert-us = <100000>;
-		reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
 	};
 };
 
@@ -625,12 +628,6 @@ vcc_mipi_en: vcc_mipi_en {
 		};
 	};
 
-	ethernet {
-		eth_phy_rst: eth_phy_rst {
-			rockchip,pins = <3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
-		};
-	};
-
 	hym8563 {
 		hym8563_int: hym8563-int {
 			rockchip,pins = <0 RK_PD3 RK_FUNC_GPIO &pcfg_pull_up>;
-- 
2.38.1

Re: [linux-next-v1 1/3] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A sbc

[Michael Riesch]

Hi Anand,

On 11/11/22 10:42, Anand Moon wrote:
> On rk356x ethernet phy support reduced media independent interface (RMII)
> and reduced gigabit media independent interface (RGMII).
> So set the phy mode to rgmii.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index ea74ba32fbbd..72e410e3aca8 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -253,7 +253,7 @@ &gmac1 {
>  	assigned-clock-rates = <0>, <125000000>;
>  	clock_in_out = "output";
>  	phy-handle = <&rgmii_phy1>;
> -	phy-mode = "rgmii-id";
> +	phy-mode = "rgmii";

What are you trying to fix here? If it ain't broken, don't fix it. I
would assume if you switch from rgmii-id to rgmii you need to adjust the
rx/tx-delays, otherwise this change does not make sense to me. Feel free
to point out what I am missing, though.

Best regards,
Michael

>  	pinctrl-names = "default";
>  	pinctrl-0 = <&gmac1m1_miim
>  		     &gmac1m1_tx_bus2

Re: [linux-next-v1 3/3] arm64: dts: rockchip: Fix ethernet reset node Rock 3A sbc

[Michael Riesch]

Hi Anand,

On 11/11/22 10:42, Anand Moon wrote:
> Add support for snps,reset-gpio reset ethernet gpio pins and
> drop the mdio reset code.

But why? Is it preferred that the MAC driver performs the reset? Or what
does this change fix?

> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  .../boot/dts/rockchip/rk3568-rock-3a.dts      | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index 16fff1ada195..9172cf60b867 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -258,6 +258,11 @@ &gmac1 {
>  	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
>  	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&gmac1_clkin>;
>  	clock_in_out = "input";
> +	snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> +	snps,reset-active-low;
> +	/* Reset time is 20ms, 100ms for rtl8211f */
> +	snps,reset-delays-us = <0 20000 100000>;
> +	phy-supply = <&vcc_3v3>;
>  	phy-handle = <&rgmii_phy1>;
>  	phy-mode = "rgmii";
>  	pinctrl-names = "default";
> @@ -267,6 +272,9 @@ &gmac1m1_rx_bus2
>  		     &gmac1m1_rgmii_clk
>  		     &gmac1m1_clkinout
>  		     &gmac1m1_rgmii_bus>;
> +
> +	tx_delay = <0x4f>;
> +	rx_delay = <0x26>;

Ah, here they are. I would say these delays should be added in patch 1/3
along a comment why the rgmii-id approach does not work.

>  	status = "okay";
>  };
>  
> @@ -583,11 +591,6 @@ &mdio1 {
>  	rgmii_phy1: ethernet-phy@0 {
>  		compatible = "ethernet-phy-ieee802.3-c22";
>  		reg = <0x0>;
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&eth_phy_rst>;
> -		reset-assert-us = <20000>;
> -		reset-deassert-us = <100000>;
> -		reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
>  	};
>  };
>  
> @@ -625,12 +628,6 @@ vcc_mipi_en: vcc_mipi_en {
>  		};
>  	};
>  
> -	ethernet {
> -		eth_phy_rst: eth_phy_rst {
> -			rockchip,pins = <3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
> -		};
> -	};

I would assume the pinctrl is still required no matter whether the MAC
driver or the PHY driver asserts the GPIO reset.

Best regards,
Michael

> -
>  	hym8563 {
>  		hym8563_int: hym8563-int {
>  			rockchip,pins = <0 RK_PD3 RK_FUNC_GPIO &pcfg_pull_up>;

Re: [linux-next-v1 1/3] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A sbc

[Peter Geis]

On Fri, Nov 11, 2022 at 4:43 AM Anand Moon <linux.amoon@gmail.com> wrote:
>
> On rk356x ethernet phy support reduced media independent interface (RMII)
> and reduced gigabit media independent interface (RGMII).
> So set the phy mode to rgmii.

The RTL8211 supports rgmii-id just fine, you only need to go to rgmii
on these boards if you need to tune the delays manually.

>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index ea74ba32fbbd..72e410e3aca8 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -253,7 +253,7 @@ &gmac1 {
>         assigned-clock-rates = <0>, <125000000>;
>         clock_in_out = "output";
>         phy-handle = <&rgmii_phy1>;
> -       phy-mode = "rgmii-id";
> +       phy-mode = "rgmii";

You should be setting the delay tuning in this patch as well.

>         pinctrl-names = "default";
>         pinctrl-0 = <&gmac1m1_miim
>                      &gmac1m1_tx_bus2
> --
> 2.38.1
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [linux-next-v1 3/3] arm64: dts: rockchip: Fix ethernet reset node Rock 3A sbc

[Peter Geis]

On Fri, Nov 11, 2022 at 4:44 AM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Add support for snps,reset-gpio reset ethernet gpio pins and
> drop the mdio reset code.

Nack, the snps,reset-gpio is depreciated in favor of the mdio-reset
code. See [0].

>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  .../boot/dts/rockchip/rk3568-rock-3a.dts      | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index 16fff1ada195..9172cf60b867 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -258,6 +258,11 @@ &gmac1 {
>         assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
>         assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&gmac1_clkin>;
>         clock_in_out = "input";
> +       snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> +       snps,reset-active-low;
> +       /* Reset time is 20ms, 100ms for rtl8211f */
> +       snps,reset-delays-us = <0 20000 100000>;
> +       phy-supply = <&vcc_3v3>;

The phy supply shouldn't be part of this patch.

>         phy-handle = <&rgmii_phy1>;
>         phy-mode = "rgmii";
>         pinctrl-names = "default";
> @@ -267,6 +272,9 @@ &gmac1m1_rx_bus2
>                      &gmac1m1_rgmii_clk
>                      &gmac1m1_clkinout
>                      &gmac1m1_rgmii_bus>;
> +
> +       tx_delay = <0x4f>;
> +       rx_delay = <0x26>;

Delays should move to your rgmii patch. Have you tested these to
ensure they are in the middle of the functional range for this board?

>         status = "okay";
>  };
>
> @@ -583,11 +591,6 @@ &mdio1 {
>         rgmii_phy1: ethernet-phy@0 {
>                 compatible = "ethernet-phy-ieee802.3-c22";
>                 reg = <0x0>;
> -               pinctrl-names = "default";
> -               pinctrl-0 = <&eth_phy_rst>;
> -               reset-assert-us = <20000>;
> -               reset-deassert-us = <100000>;
> -               reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
>         };
>  };
>
> @@ -625,12 +628,6 @@ vcc_mipi_en: vcc_mipi_en {
>                 };
>         };
>
> -       ethernet {
> -               eth_phy_rst: eth_phy_rst {
> -                       rockchip,pins = <3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
> -               };
> -       };
> -
>         hym8563 {
>                 hym8563_int: hym8563-int {
>                         rockchip,pins = <0 RK_PD3 RK_FUNC_GPIO &pcfg_pull_up>;
> --
> 2.38.1
>
[0] https://elixir.bootlin.com/linux/v6.1-rc4/source/Documentation/devicetree/bindings/net/snps,dwmac.yaml#L222
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [linux-next-v1 1/3] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A sbc

[Anand Moon]

 Hi Petter / Michael.

Thanks for your review comments.

On Fri, 11 Nov 2022 at 18:26, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Fri, Nov 11, 2022 at 4:43 AM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > On rk356x ethernet phy support reduced media independent interface (RMII)
> > and reduced gigabit media independent interface (RGMII).
> > So set the phy mode to rgmii.
>
> The RTL8211 supports rgmii-id just fine, you only need to go to rgmii
> on these boards if you need to tune the delays manually.

Ok rgmii supports internal rx and tx clock delay
whereas rgmii-id does not supports internal clock delay.

>
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > index ea74ba32fbbd..72e410e3aca8 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > @@ -253,7 +253,7 @@ &gmac1 {
> >         assigned-clock-rates = <0>, <125000000>;
> >         clock_in_out = "output";
> >         phy-handle = <&rgmii_phy1>;
> > -       phy-mode = "rgmii-id";
> > +       phy-mode = "rgmii";
>
> You should be setting the delay tuning in this patch as well.

I will try to fix this in the next version.

>
> >         pinctrl-names = "default";
> >         pinctrl-0 = <&gmac1m1_miim
> >                      &gmac1m1_tx_bus2
> > --
> > 2.38.1
Thanks


-Anand

Re: [linux-next-v1 3/3] arm64: dts: rockchip: Fix ethernet reset node Rock 3A sbc

[Anand Moon]

Hi Peter / Michael

On Fri, 11 Nov 2022 at 18:32, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Fri, Nov 11, 2022 at 4:44 AM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Add support for snps,reset-gpio reset ethernet gpio pins and
> > drop the mdio reset code.
>
> Nack, the snps,reset-gpio is depreciated in favor of the mdio-reset
> code. See [0].
>

Ok coming to the ethernet reset controller with gpio-reset with MDIO.
I got this working by adding ethernet-phy-id to MDIO.

> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  .../boot/dts/rockchip/rk3568-rock-3a.dts      | 19 ++++++++-----------
> >  1 file changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > index 16fff1ada195..9172cf60b867 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > @@ -258,6 +258,11 @@ &gmac1 {
> >         assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> >         assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&gmac1_clkin>;
> >         clock_in_out = "input";
> > +       snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> > +       snps,reset-active-low;
> > +       /* Reset time is 20ms, 100ms for rtl8211f */
> > +       snps,reset-delays-us = <0 20000 100000>;
> > +       phy-supply = <&vcc_3v3>;
>
> The phy supply shouldn't be part of this patch.
>

Ok will add this in a separate patch.

> >         phy-handle = <&rgmii_phy1>;
> >         phy-mode = "rgmii";
> >         pinctrl-names = "default";
> > @@ -267,6 +272,9 @@ &gmac1m1_rx_bus2
> >                      &gmac1m1_rgmii_clk
> >                      &gmac1m1_clkinout
> >                      &gmac1m1_rgmii_bus>;
> > +
> > +       tx_delay = <0x4f>;
> > +       rx_delay = <0x26>;
>
> Delays should move to your rgmii patch. Have you tested these to
> ensure they are in the middle of the functional range for this board?

Ok but this delay is different from the actual requirements see below.
rx-internal-delay-ps and tx-internal-delay-ps
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ethernet-controller.yaml?h=v6.1-rc5#n222

tx_delay and rx_dealy are used by rockchip binding.

>
> >         status = "okay";
> >  };
> >
> > @@ -583,11 +591,6 @@ &mdio1 {
> >         rgmii_phy1: ethernet-phy@0 {
> >                 compatible = "ethernet-phy-ieee802.3-c22";
> >                 reg = <0x0>;
> > -               pinctrl-names = "default";
> > -               pinctrl-0 = <&eth_phy_rst>;
> > -               reset-assert-us = <20000>;
> > -               reset-deassert-us = <100000>;
> > -               reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> >         };
> >  };
> >
> > @@ -625,12 +628,6 @@ vcc_mipi_en: vcc_mipi_en {
> >                 };
> >         };
> >
> > -       ethernet {
> > -               eth_phy_rst: eth_phy_rst {
> > -                       rockchip,pins = <3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
> > -               };
> > -       };
> > -
> >         hym8563 {
> >                 hym8563_int: hym8563-int {
> >                         rockchip,pins = <0 RK_PD3 RK_FUNC_GPIO &pcfg_pull_up>;
> > --
> > 2.38.1
> >
> [0] https://elixir.bootlin.com/linux/v6.1-rc4/source/Documentation/devicetree/bindings/net/snps,dwmac.yaml#L222
> >
Thanks





-Anand
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip