2022-11-11 09:46:48

by Anand Moon

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

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

Signed-off-by: Anand Moon <[email protected]>
---
.../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

2022-11-11 12:20:12

by Michael Riesch

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

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 <[email protected]>
> ---
> .../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>;

2022-11-11 13:31:48

by Peter Geis

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

On Fri, Nov 11, 2022 at 4:44 AM Anand Moon <[email protected]> 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 <[email protected]>
> ---
> .../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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

2022-11-16 18:47:35

by Anand Moon

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

Hi Peter / Michael

On Fri, 11 Nov 2022 at 18:32, Peter Geis <[email protected]> wrote:
>
> On Fri, Nov 11, 2022 at 4:44 AM Anand Moon <[email protected]> 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 <[email protected]>
> > ---
> > .../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
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip