linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC.
       [not found] <20221116200150.4657-1-linux.amoon@gmail.com>
@ 2022-11-16 20:01 ` Anand Moon
  2022-11-16 20:36   ` Peter Geis
                     ` (2 more replies)
  2022-11-16 20:01 ` [linux-next-v2 2/5] arm64: dts: rockchip: Add support of external clock to ethernet node " Anand Moon
                   ` (4 subsequent siblings)
  5 siblings, 3 replies; 27+ messages in thread
From: Anand Moon @ 2022-11-16 20:01 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Anand Moon, Chukun Pan, Michael Riesch, Peter Geis, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel

On rk356x ethernet phy support reduced media independent interface (RMII)
and reduced gigabit media independent interface (RGMII).
So set the phy mode to rgmii to support clock delay, also
add TX and RX delay for phy-mode.

Fix following warning

[    7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay.
[    7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30
[    7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay.
[    7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
V2: Fix commit message and added the RX and TX clock delay.
---
 arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 5 ++++-
 1 file changed, 4 insertions(+), 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..e1c75532dcee 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -253,13 +253,16 @@ &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
 		     &gmac1m1_rx_bus2
 		     &gmac1m1_rgmii_clk
 		     &gmac1m1_rgmii_bus>;
+
+	tx_delay = <0x4f>;
+	rx_delay = <0x26>;
 	status = "okay";
 };
 
-- 
2.38.1


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

* [linux-next-v2 2/5] arm64: dts: rockchip: Add support of external clock to ethernet node on Rock 3A SBC
       [not found] <20221116200150.4657-1-linux.amoon@gmail.com>
  2022-11-16 20:01 ` [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC Anand Moon
@ 2022-11-16 20:01 ` Anand Moon
  2022-11-22 11:18   ` Michael Riesch
  2022-11-16 20:01 ` [linux-next-v2 3/5] arm64: dts: rockchip: Add support of regulator for " Anand Moon
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: Anand Moon @ 2022-11-16 20:01 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Anand Moon, Chukun Pan, Michael Riesch, Peter Geis, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel

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>
---
V2: None
---
 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 e1c75532dcee..b848282ea005 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>;
 
 	tx_delay = <0x4f>;
-- 
2.38.1


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

* [linux-next-v2 3/5] arm64: dts: rockchip: Add support of regulator for ethernet node on Rock 3A SBC
       [not found] <20221116200150.4657-1-linux.amoon@gmail.com>
  2022-11-16 20:01 ` [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC Anand Moon
  2022-11-16 20:01 ` [linux-next-v2 2/5] arm64: dts: rockchip: Add support of external clock to ethernet node " Anand Moon
@ 2022-11-16 20:01 ` Anand Moon
  2022-11-22 11:23   ` Michael Riesch
  2022-11-16 20:01 ` [linux-next-v2 4/5] arm64: dts: rockchip: Add support of interrupt to " Anand Moon
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: Anand Moon @ 2022-11-16 20:01 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Anand Moon, Chukun Pan, Michael Riesch, Peter Geis, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel

Add regulator support for ethernet node

Fix following warning.
[    7.365199] rk_gmac-dwmac fe010000.ethernet: no regulator found

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

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index b848282ea005..5378254c57ca 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -260,6 +260,7 @@ &gmac1 {
 	clock_in_out = "input";
 	phy-handle = <&rgmii_phy1>;
 	phy-mode = "rgmii";
+	phy-supply = <&vcc_3v3>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&gmac1m1_miim
 		     &gmac1m1_tx_bus2
-- 
2.38.1


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

* [linux-next-v2 4/5] arm64: dts: rockchip: Add support of interrupt to ethernet node on Rock 3A SBC
       [not found] <20221116200150.4657-1-linux.amoon@gmail.com>
                   ` (2 preceding siblings ...)
  2022-11-16 20:01 ` [linux-next-v2 3/5] arm64: dts: rockchip: Add support of regulator for " Anand Moon
@ 2022-11-16 20:01 ` Anand Moon
  2022-11-22 16:10   ` Michael Riesch
  2022-11-16 20:01 ` Anand Moon
  2022-11-16 20:01 ` [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy " Anand Moon
  5 siblings, 1 reply; 27+ messages in thread
From: Anand Moon @ 2022-11-16 20:01 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Anand Moon, Chukun Pan, Michael Riesch, Peter Geis, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel

As per the shematic gmac1 support gpio interrupt controller 
GMAC1_INT/PMEB_GPIO3_A7 add the support for this.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v2: new patch added
---
 arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 10 +++++++++-
 1 file changed, 9 insertions(+), 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 5378254c57ca..9f84a23a8789 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -588,10 +588,14 @@ rgmii_phy1: ethernet-phy@0 {
 		compatible = "ethernet-phy-ieee802.3-c22";
 		reg = <0x0>;
 		pinctrl-names = "default";
-		pinctrl-0 = <&eth_phy_rst>;
+		pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
 		reset-assert-us = <20000>;
 		reset-deassert-us = <100000>;
 		reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
+		interrupt-parent = <&gpio3>;
+		/* GMAC1_INT/PMEB_GPIO3_A7 */
+		interrupts = <RK_PA7 IRQ_TYPE_LEVEL_LOW>;
+		#interrupt-cells = <1>;
 	};
 };
 
@@ -630,6 +634,10 @@ vcc_mipi_en: vcc_mipi_en {
 	};
 
 	ethernet {
+		eth_phy_int: eth-phy-int {
+			rockchip,pins = <3 RK_PA7 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
 		eth_phy_rst: eth_phy_rst {
 			rockchip,pins = <3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
-- 
2.38.1


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

* [linux-next-v2 4/5] arm64: dts: rockchip: Add support of interrupt to ethernet node on Rock 3A SBC
       [not found] <20221116200150.4657-1-linux.amoon@gmail.com>
                   ` (3 preceding siblings ...)
  2022-11-16 20:01 ` [linux-next-v2 4/5] arm64: dts: rockchip: Add support of interrupt to " Anand Moon
@ 2022-11-16 20:01 ` Anand Moon
  2022-11-16 20:01 ` [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy " Anand Moon
  5 siblings, 0 replies; 27+ messages in thread
From: Anand Moon @ 2022-11-16 20:01 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Anand Moon, Chukun Pan, Michael Riesch, Peter Geis, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel

As per the shematic gmac1 support gpio interrupt controller 
GMAC1_INT/PMEB_GPIO3_A7 add the support for this.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v2: new patch added
---
 arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 10 +++++++++-
 1 file changed, 9 insertions(+), 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 5378254c57ca..9f84a23a8789 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -588,10 +588,14 @@ rgmii_phy1: ethernet-phy@0 {
 		compatible = "ethernet-phy-ieee802.3-c22";
 		reg = <0x0>;
 		pinctrl-names = "default";
-		pinctrl-0 = <&eth_phy_rst>;
+		pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
 		reset-assert-us = <20000>;
 		reset-deassert-us = <100000>;
 		reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
+		interrupt-parent = <&gpio3>;
+		/* GMAC1_INT/PMEB_GPIO3_A7 */
+		interrupts = <RK_PA7 IRQ_TYPE_LEVEL_LOW>;
+		#interrupt-cells = <1>;
 	};
 };
 
@@ -630,6 +634,10 @@ vcc_mipi_en: vcc_mipi_en {
 	};
 
 	ethernet {
+		eth_phy_int: eth-phy-int {
+			rockchip,pins = <3 RK_PA7 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
 		eth_phy_rst: eth_phy_rst {
 			rockchip,pins = <3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
-- 
2.38.1


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

* [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
       [not found] <20221116200150.4657-1-linux.amoon@gmail.com>
                   ` (4 preceding siblings ...)
  2022-11-16 20:01 ` Anand Moon
@ 2022-11-16 20:01 ` Anand Moon
  2022-11-16 20:46   ` Peter Geis
  2022-11-22 16:46   ` Michael Riesch
  5 siblings, 2 replies; 27+ messages in thread
From: Anand Moon @ 2022-11-16 20:01 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Anand Moon, Chukun Pan, Michael Riesch, Peter Geis, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel

Add MDIO description with ethernet-phy-id compatible string
which enable calling reset of the phy. The PHY will then be probed,
independent of if it can be found on the bus or not,
and that probing will enable the GPIO.

ethernet-phy-id is read from ethenet register dump reg2 and reg3.

Fix following warning.
[   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
[   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
[   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
V2: new to the patch series.

alarm@rock-3a:~$ sudo ethtool -d eth0
[sudo] password for alarm:
ST GMAC Registers
GMAC Registers
Reg0  0x08072203
Reg1  0x00000000
Reg2  0x00000404
Reg3  0x00000000
Reg4  0x00000002
---
 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 9f84a23a8789..fe36156a5017 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -585,7 +585,7 @@ &i2s2_2ch {
 
 &mdio1 {
 	rgmii_phy1: ethernet-phy@0 {
-		compatible = "ethernet-phy-ieee802.3-c22";
+		compatible = "ethernet-phy-id0000.0404", "ethernet-phy-ieee802.3-c22";
 		reg = <0x0>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
-- 
2.38.1


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

* Re: [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC.
  2022-11-16 20:01 ` [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC Anand Moon
@ 2022-11-16 20:36   ` Peter Geis
  2022-11-17 14:44     ` Anand Moon
  2022-11-18  7:03   ` Michael Riesch
  2022-11-22 22:34   ` (subset) " Heiko Stuebner
  2 siblings, 1 reply; 27+ messages in thread
From: Peter Geis @ 2022-11-16 20:36 UTC (permalink / raw)
  To: Anand Moon
  Cc: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner, Chukun Pan,
	Michael Riesch, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On Wed, Nov 16, 2022 at 3:02 PM 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 to support clock delay, also
> add TX and RX delay for phy-mode.

Controller based clock delay, the various rgmii-id modes (rgmii-txid
and rgmii-rxid are also valid) apply the delays in the phy. They are
usually at a fixed amount, but some phys support variable delays.

You want your commit message to accurately describe the problem, such
as "In rgmii-id mode, the phy on the rock-3a is unreliable due to
incorrect delays. Switch to rgmii mode in order to handle the delays
in the controller."

>
> Fix following warning
>
> [    7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay.
> [    7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30
> [    7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay.
> [    7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10

I've been meaning to make this a dev_debug message, because in the
various rgmii-id modes it is feasible for these to be non-existent in
the device-tree. In rgmii-id mode these are disabled, no matter what
they are set to in the dt.

>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> V2: Fix commit message and added the RX and TX clock delay.
> ---
>  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 5 ++++-
>  1 file changed, 4 insertions(+), 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..e1c75532dcee 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -253,13 +253,16 @@ &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
>                      &gmac1m1_rx_bus2
>                      &gmac1m1_rgmii_clk
>                      &gmac1m1_rgmii_bus>;
> +
> +       tx_delay = <0x4f>;
> +       rx_delay = <0x26>;

These are pretty far off from the default, have you verified the upper
and lower bounds for the rock-3a? These should be roughly in the
middle of that range.

>         status = "okay";
>  };
>
> --
> 2.38.1
>

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

* Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
  2022-11-16 20:01 ` [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy " Anand Moon
@ 2022-11-16 20:46   ` Peter Geis
  2022-11-17  5:57     ` Anand Moon
  2022-11-22 16:46   ` Michael Riesch
  1 sibling, 1 reply; 27+ messages in thread
From: Peter Geis @ 2022-11-16 20:46 UTC (permalink / raw)
  To: Anand Moon
  Cc: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner, Chukun Pan,
	Michael Riesch, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On Wed, Nov 16, 2022 at 3:02 PM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Add MDIO description with ethernet-phy-id compatible string
> which enable calling reset of the phy. The PHY will then be probed,
> independent of if it can be found on the bus or not,
> and that probing will enable the GPIO.
>
> ethernet-phy-id is read from ethenet register dump reg2 and reg3.
>
> Fix following warning.
> [   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> [   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
> [   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)
>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> V2: new to the patch series.
>
> alarm@rock-3a:~$ sudo ethtool -d eth0
> [sudo] password for alarm:
> ST GMAC Registers
> GMAC Registers
> Reg0  0x08072203
> Reg1  0x00000000
> Reg2  0x00000404
> Reg3  0x00000000
> Reg4  0x00000002
> ---
>  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 9f84a23a8789..fe36156a5017 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -585,7 +585,7 @@ &i2s2_2ch {
>
>  &mdio1 {
>         rgmii_phy1: ethernet-phy@0 {
> -               compatible = "ethernet-phy-ieee802.3-c22";
> +               compatible = "ethernet-phy-id0000.0404", "ethernet-phy-ieee802.3-c22";
>                 reg = <0x0>;
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;

Have you tried instead moving the reset to the mdio bus? I've had
success with this, though you'll need to change the reset assert and
deassert timing handles, they are different for the bus.

> --
> 2.38.1
>

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

* Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
  2022-11-16 20:46   ` Peter Geis
@ 2022-11-17  5:57     ` Anand Moon
  2022-11-17 10:54       ` Robin Murphy
  0 siblings, 1 reply; 27+ messages in thread
From: Anand Moon @ 2022-11-17  5:57 UTC (permalink / raw)
  To: Peter Geis
  Cc: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner, Chukun Pan,
	Michael Riesch, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Hi Peter,

On Thu, 17 Nov 2022 at 02:16, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Wed, Nov 16, 2022 at 3:02 PM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Add MDIO description with ethernet-phy-id compatible string
> > which enable calling reset of the phy. The PHY will then be probed,
> > independent of if it can be found on the bus or not,
> > and that probing will enable the GPIO.
> >
> > ethernet-phy-id is read from ethenet register dump reg2 and reg3.
> >
> > Fix following warning.
> > [   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> > [   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
> > [   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > V2: new to the patch series.
> >
> > alarm@rock-3a:~$ sudo ethtool -d eth0
> > [sudo] password for alarm:
> > ST GMAC Registers
> > GMAC Registers
> > Reg0  0x08072203
> > Reg1  0x00000000
> > Reg2  0x00000404
> > Reg3  0x00000000
> > Reg4  0x00000002
> > ---
> >  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 9f84a23a8789..fe36156a5017 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > @@ -585,7 +585,7 @@ &i2s2_2ch {
> >
> >  &mdio1 {
> >         rgmii_phy1: ethernet-phy@0 {
> > -               compatible = "ethernet-phy-ieee802.3-c22";
> > +               compatible = "ethernet-phy-id0000.0404", "ethernet-phy-ieee802.3-c22";
> >                 reg = <0x0>;
> >                 pinctrl-names = "default";
> >                 pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
>
> Have you tried instead moving the reset to the mdio bus? I've had
> success with this, though you'll need to change the reset assert and
> deassert timing handles, they are different for the bus.
>
No can you share some examples?
If you got a better way to solve this issue please let me know.
I will give this a try.

Thanks
-Anand
> > --
> > 2.38.1
> >

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

* Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
  2022-11-17  5:57     ` Anand Moon
@ 2022-11-17 10:54       ` Robin Murphy
  2022-11-17 14:55         ` Anand Moon
  0 siblings, 1 reply; 27+ messages in thread
From: Robin Murphy @ 2022-11-17 10:54 UTC (permalink / raw)
  To: Anand Moon, Peter Geis
  Cc: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner, Chukun Pan,
	Michael Riesch, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On 2022-11-17 05:57, Anand Moon wrote:
> Hi Peter,
> 
> On Thu, 17 Nov 2022 at 02:16, Peter Geis <pgwipeout@gmail.com> wrote:
>>
>> On Wed, Nov 16, 2022 at 3:02 PM Anand Moon <linux.amoon@gmail.com> wrote:
>>>
>>> Add MDIO description with ethernet-phy-id compatible string
>>> which enable calling reset of the phy. The PHY will then be probed,
>>> independent of if it can be found on the bus or not,
>>> and that probing will enable the GPIO.
>>>
>>> ethernet-phy-id is read from ethenet register dump reg2 and reg3.
>>>
>>> Fix following warning.
>>> [   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
>>> [   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
>>> [   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)
>>>
>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>> ---
>>> V2: new to the patch series.
>>>
>>> alarm@rock-3a:~$ sudo ethtool -d eth0
>>> [sudo] password for alarm:
>>> ST GMAC Registers
>>> GMAC Registers
>>> Reg0  0x08072203
>>> Reg1  0x00000000
>>> Reg2  0x00000404
>>> Reg3  0x00000000
>>> Reg4  0x00000002
>>> ---
>>>   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 9f84a23a8789..fe36156a5017 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>> @@ -585,7 +585,7 @@ &i2s2_2ch {
>>>
>>>   &mdio1 {
>>>          rgmii_phy1: ethernet-phy@0 {
>>> -               compatible = "ethernet-phy-ieee802.3-c22";
>>> +               compatible = "ethernet-phy-id0000.0404", "ethernet-phy-ieee802.3-c22";
>>>                  reg = <0x0>;
>>>                  pinctrl-names = "default";
>>>                  pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
>>
>> Have you tried instead moving the reset to the mdio bus? I've had
>> success with this, though you'll need to change the reset assert and
>> deassert timing handles, they are different for the bus.
>>
> No can you share some examples?
> If you got a better way to solve this issue please let me know.
> I will give this a try.

Note that the Rock 3A schematic says the phy is configured for address 
1, not 0. From what I remember of adding the MDIO node for NanoiPi4, 
that didn't work if I got the address wrong, despite the fact that the 
auto-detection when the MDIO node is omitted claimed to find the same 
phy on both addresses 0 and 1.

Robin.

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

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

* Re: [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC.
  2022-11-16 20:36   ` Peter Geis
@ 2022-11-17 14:44     ` Anand Moon
  0 siblings, 0 replies; 27+ messages in thread
From: Anand Moon @ 2022-11-17 14:44 UTC (permalink / raw)
  To: Peter Geis
  Cc: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner, Chukun Pan,
	Michael Riesch, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Hi Peter

On Thu, 17 Nov 2022 at 02:06, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Wed, Nov 16, 2022 at 3:02 PM 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 to support clock delay, also
> > add TX and RX delay for phy-mode.
>
> Controller based clock delay, the various rgmii-id modes (rgmii-txid
> and rgmii-rxid are also valid) apply the delays in the phy. They are
> usually at a fixed amount, but some phys support variable delays.
>
> You want your commit message to accurately describe the problem, such
> as "In rgmii-id mode, the phy on the rock-3a is unreliable due to
> incorrect delays. Switch to rgmii mode in order to handle the delays
> in the controller."
>
> >
> > Fix following warning
> >
> > [    7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay.
> > [    7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30
> > [    7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay.
> > [    7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10
>
> I've been meaning to make this a dev_debug message, because in the
> various rgmii-id modes it is feasible for these to be non-existent in
> the device-tree. In rgmii-id mode these are disabled, no matter what
> they are set to in the dt.

As per the public datasheet share by Radxa below
[0] https://dl.radxa.com/rock3/docs/hw/datasheet/RTL8211F-CG-Datasheet.pdf

This Ethernet controller supports RGMII mode.

>
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > V2: Fix commit message and added the RX and TX clock delay.
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 5 ++++-
> >  1 file changed, 4 insertions(+), 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..e1c75532dcee 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > @@ -253,13 +253,16 @@ &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
> >                      &gmac1m1_rx_bus2
> >                      &gmac1m1_rgmii_clk
> >                      &gmac1m1_rgmii_bus>;
> > +
> > +       tx_delay = <0x4f>;
> > +       rx_delay = <0x26>;
>
> These are pretty far off from the default, have you verified the upper
> and lower bounds for the rock-3a? These should be roughly in the
> middle of that range.
>
No I have not tested this way but as per the datasheet, it supports

TXDLY   Add 2ns delay to RXC for RXD latching (via 4.7k-ohm to DVDD_RG)
RXDLY   Add 2ns delay to RXC for RXD latching (via 4.7k-ohm to DVDD_RG)

I feel I should ignore the above warning and add it below.

rx-internal-delay-ps = <2000>;
tx-internal-delay-ps = <2000>;

Thanks
-Anand
> >         status = "okay";
> >  };
> >
> > --
> > 2.38.1
> >

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

* Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
  2022-11-17 10:54       ` Robin Murphy
@ 2022-11-17 14:55         ` Anand Moon
  2022-11-17 19:10           ` Peter Geis
  0 siblings, 1 reply; 27+ messages in thread
From: Anand Moon @ 2022-11-17 14:55 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Peter Geis, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner,
	Chukun Pan, Michael Riesch, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

Hi Robin

On Thu, 17 Nov 2022 at 16:24, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2022-11-17 05:57, Anand Moon wrote:
> > Hi Peter,
> >
> > On Thu, 17 Nov 2022 at 02:16, Peter Geis <pgwipeout@gmail.com> wrote:
> >>
> >> On Wed, Nov 16, 2022 at 3:02 PM Anand Moon <linux.amoon@gmail.com> wrote:
> >>>
> >>> Add MDIO description with ethernet-phy-id compatible string
> >>> which enable calling reset of the phy. The PHY will then be probed,
> >>> independent of if it can be found on the bus or not,
> >>> and that probing will enable the GPIO.
> >>>
> >>> ethernet-phy-id is read from ethenet register dump reg2 and reg3.
> >>>
> >>> Fix following warning.
> >>> [   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> >>> [   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
> >>> [   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)
> >>>
> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> >>> ---
> >>> V2: new to the patch series.
> >>>
> >>> alarm@rock-3a:~$ sudo ethtool -d eth0
> >>> [sudo] password for alarm:
> >>> ST GMAC Registers
> >>> GMAC Registers
> >>> Reg0  0x08072203
> >>> Reg1  0x00000000
> >>> Reg2  0x00000404
> >>> Reg3  0x00000000
> >>> Reg4  0x00000002
> >>> ---
> >>>   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 9f84a23a8789..fe36156a5017 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> >>> @@ -585,7 +585,7 @@ &i2s2_2ch {
> >>>
> >>>   &mdio1 {
> >>>          rgmii_phy1: ethernet-phy@0 {
> >>> -               compatible = "ethernet-phy-ieee802.3-c22";
> >>> +               compatible = "ethernet-phy-id0000.0404", "ethernet-phy-ieee802.3-c22";
> >>>                  reg = <0x0>;
> >>>                  pinctrl-names = "default";
> >>>                  pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
> >>
> >> Have you tried instead moving the reset to the mdio bus? I've had
> >> success with this, though you'll need to change the reset assert and
> >> deassert timing handles, they are different for the bus.
> >>
> > No can you share some examples?
> > If you got a better way to solve this issue please let me know.
> > I will give this a try.
>
> Note that the Rock 3A schematic says the phy is configured for address
> 1, not 0. From what I remember of adding the MDIO node for NanoiPi4,
> that didn't work if I got the address wrong, despite the fact that the
> auto-detection when the MDIO node is omitted claimed to find the same
> phy on both addresses 0 and 1.
>

Yes, I have tested with these changes still this does not work, with
out this fix as of now..

We can check this comment in the below commit id
70f04e9a3358404367030493dc36718d4495a9a5  ARM: dts: imx6ul-14x14-evk:
Enable the GPIO expander

maybe something is still missing to be configured.

Thanks
-Anand





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

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

* Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
  2022-11-17 14:55         ` Anand Moon
@ 2022-11-17 19:10           ` Peter Geis
  2022-11-22 14:20             ` Anand Moon
  0 siblings, 1 reply; 27+ messages in thread
From: Peter Geis @ 2022-11-17 19:10 UTC (permalink / raw)
  To: Anand Moon
  Cc: Robin Murphy, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner,
	Chukun Pan, Michael Riesch, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

On Thu, Nov 17, 2022 at 9:56 AM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Robin
>
> On Thu, 17 Nov 2022 at 16:24, Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > On 2022-11-17 05:57, Anand Moon wrote:
> > > Hi Peter,
> > >
> > > On Thu, 17 Nov 2022 at 02:16, Peter Geis <pgwipeout@gmail.com> wrote:
> > >>
> > >> On Wed, Nov 16, 2022 at 3:02 PM Anand Moon <linux.amoon@gmail.com> wrote:
> > >>>
> > >>> Add MDIO description with ethernet-phy-id compatible string
> > >>> which enable calling reset of the phy. The PHY will then be probed,
> > >>> independent of if it can be found on the bus or not,
> > >>> and that probing will enable the GPIO.
> > >>>
> > >>> ethernet-phy-id is read from ethenet register dump reg2 and reg3.
> > >>>
> > >>> Fix following warning.
> > >>> [   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> > >>> [   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
> > >>> [   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)
> > >>>
> > >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > >>> ---
> > >>> V2: new to the patch series.
> > >>>
> > >>> alarm@rock-3a:~$ sudo ethtool -d eth0
> > >>> [sudo] password for alarm:
> > >>> ST GMAC Registers
> > >>> GMAC Registers
> > >>> Reg0  0x08072203
> > >>> Reg1  0x00000000
> > >>> Reg2  0x00000404
> > >>> Reg3  0x00000000
> > >>> Reg4  0x00000002
> > >>> ---
> > >>>   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 9f84a23a8789..fe36156a5017 100644
> > >>> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > >>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > >>> @@ -585,7 +585,7 @@ &i2s2_2ch {
> > >>>
> > >>>   &mdio1 {
> > >>>          rgmii_phy1: ethernet-phy@0 {
> > >>> -               compatible = "ethernet-phy-ieee802.3-c22";
> > >>> +               compatible = "ethernet-phy-id0000.0404", "ethernet-phy-ieee802.3-c22";
> > >>>                  reg = <0x0>;
> > >>>                  pinctrl-names = "default";
> > >>>                  pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
> > >>
> > >> Have you tried instead moving the reset to the mdio bus? I've had
> > >> success with this, though you'll need to change the reset assert and
> > >> deassert timing handles, they are different for the bus.
> > >>
> > > No can you share some examples?
> > > If you got a better way to solve this issue please let me know.
> > > I will give this a try.
> >
> > Note that the Rock 3A schematic says the phy is configured for address
> > 1, not 0. From what I remember of adding the MDIO node for NanoiPi4,
> > that didn't work if I got the address wrong, despite the fact that the
> > auto-detection when the MDIO node is omitted claimed to find the same
> > phy on both addresses 0 and 1.
> >

From the net-dev folk, mdio address 0 is a broadcast address. All
functional phys we have on the mdio bus should respond to it. The
problem I've run into is with the reset on the phy node the reset
triggers too late for the dwmac to detect the phy correctly. However
moving it to the mdio bus node makes the reset happen at roughly the
same time as the depreciated dwmac reset and the phy detects without
any weird hacks.

>
> Yes, I have tested with these changes still this does not work, with
> out this fix as of now..
>
> We can check this comment in the below commit id
> 70f04e9a3358404367030493dc36718d4495a9a5  ARM: dts: imx6ul-14x14-evk:
> Enable the GPIO expander
>
> maybe something is still missing to be configured.
>
> Thanks
> -Anand
>
>
>
>
>
> > Robin.
> >
> > >
> > > Thanks
> > > -Anand
> > >>> --
> > >>> 2.38.1
> > >>>
> > >
> > > _______________________________________________
> > > Linux-rockchip mailing list
> > > Linux-rockchip@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC.
  2022-11-16 20:01 ` [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC Anand Moon
  2022-11-16 20:36   ` Peter Geis
@ 2022-11-18  7:03   ` Michael Riesch
  2022-11-18  9:34     ` Anand Moon
  2022-11-22 22:34   ` (subset) " Heiko Stuebner
  2 siblings, 1 reply; 27+ messages in thread
From: Michael Riesch @ 2022-11-18  7:03 UTC (permalink / raw)
  To: Anand Moon, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Chukun Pan, Peter Geis, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

Hi Anand,

On 11/16/22 21:01, 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 to support clock delay, also
> add TX and RX delay for phy-mode.

Based on this commit message I still don't understand what you are
actually trying to fix here. If you encounter network problems/stability
issues, please let me know what test triggers the faulty behavior.
Please describe the problem you are facing in detail here or in the
cover letter.

> Fix following warning
> 
> [    7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay.
> [    7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30
> [    7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay.
> [    7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10

If the only purpose of this patch is to get rid of this warnings, it may
make sense to set them to dev_dbg as Peter pointed out.

Best regards,
Michael

> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> V2: Fix commit message and added the RX and TX clock delay.
> ---
>  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 5 ++++-
>  1 file changed, 4 insertions(+), 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..e1c75532dcee 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -253,13 +253,16 @@ &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
>  		     &gmac1m1_rx_bus2
>  		     &gmac1m1_rgmii_clk
>  		     &gmac1m1_rgmii_bus>;
> +
> +	tx_delay = <0x4f>;
> +	rx_delay = <0x26>;
>  	status = "okay";
>  };
>  

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

* Re: [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC.
  2022-11-18  7:03   ` Michael Riesch
@ 2022-11-18  9:34     ` Anand Moon
  2022-11-18 18:13       ` Peter Geis
  0 siblings, 1 reply; 27+ messages in thread
From: Anand Moon @ 2022-11-18  9:34 UTC (permalink / raw)
  To: Michael Riesch
  Cc: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner, Chukun Pan,
	Peter Geis, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Hi Michael,

On Fri, 18 Nov 2022 at 12:33, Michael Riesch
<michael.riesch@wolfvision.net> wrote:
>
> Hi Anand,
>
> On 11/16/22 21:01, 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 to support clock delay, also
> > add TX and RX delay for phy-mode.
>
> Based on this commit message I still don't understand what you are
> actually trying to fix here. If you encounter network problems/stability
> issues, please let me know what test triggers the faulty behavior.
> Please describe the problem you are facing in detail here or in the
> cover letter.
>

Ok, Ethernet does not work on my Radxa 3A see boot logs.

[0] https://gist.github.com/moonlinux/bb56c787031226fbb9f69121564e76a2

Please find this updated commit message.

As per the schematic and datasheet PHY mode is RGMII
Use 2ns clock delay to RXC for RXD and TXC for TXD latching.

> > Fix the following warning
> >
> > [    7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay.
> > [    7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30
> > [    7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay.
> > [    7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10
>
> If the only purpose of this patch is to get rid of this warnings, it may

No, the intent is to fix the PHY mode to RGMII and fix the delay.
[ 7.066357] rk_gmac-dwmac fe010000.ethernet: init for RGMII_ID

> make sense to set them to dev_dbg as Peter pointed out.
>
Ok, will update this in the next version.

> Best regards,
> Michael
>
Thanks
-Anand

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

* Re: [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC.
  2022-11-18  9:34     ` Anand Moon
@ 2022-11-18 18:13       ` Peter Geis
  2022-11-22 14:19         ` Anand Moon
  0 siblings, 1 reply; 27+ messages in thread
From: Peter Geis @ 2022-11-18 18:13 UTC (permalink / raw)
  To: Anand Moon
  Cc: Michael Riesch, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner,
	Chukun Pan, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On Fri, Nov 18, 2022 at 4:35 AM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Michael,
>
> On Fri, 18 Nov 2022 at 12:33, Michael Riesch
> <michael.riesch@wolfvision.net> wrote:
> >
> > Hi Anand,
> >
> > On 11/16/22 21:01, 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 to support clock delay, also
> > > add TX and RX delay for phy-mode.
> >
> > Based on this commit message I still don't understand what you are
> > actually trying to fix here. If you encounter network problems/stability
> > issues, please let me know what test triggers the faulty behavior.
> > Please describe the problem you are facing in detail here or in the
> > cover letter.
> >
>
> Ok, Ethernet does not work on my Radxa 3A see boot logs.
>
> [0] https://gist.github.com/moonlinux/bb56c787031226fbb9f69121564e76a2
>
> Please find this updated commit message.
>
> As per the schematic and datasheet PHY mode is RGMII
> Use 2ns clock delay to RXC for RXD and TXC for TXD latching.

rgmii-id mode does exactly this in the phy (your realtek chip). By
setting the mode to rgmii, you're telling the phy that delays are set
elsewhere, either in hardware or in the controller. You're then
handling them in the controller. While the delays aren't documented in
the TRM, I've long suspected that the defaults of 0x30 and 0x10 equate
to the standard 2ns delay. So you're setting the delays much higher
than the default means you need to add *more* than the standard 2ns
delay for your device to work.

This is why I've been asking if you have tested these. You need to set
each value and find the lowest and highest possible values that work,
then take the median value between those two.

>
> > > Fix the following warning
> > >
> > > [    7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay.
> > > [    7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30
> > > [    7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay.
> > > [    7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10
> >
> > If the only purpose of this patch is to get rid of this warnings, it may
>
> No, the intent is to fix the PHY mode to RGMII and fix the delay.
> [ 7.066357] rk_gmac-dwmac fe010000.ethernet: init for RGMII_ID
>
> > make sense to set them to dev_dbg as Peter pointed out.
> >
> Ok, will update this in the next version.
>
> > Best regards,
> > Michael
> >
> Thanks
> -Anand

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

* Re: [linux-next-v2 2/5] arm64: dts: rockchip: Add support of external clock to ethernet node on Rock 3A SBC
  2022-11-16 20:01 ` [linux-next-v2 2/5] arm64: dts: rockchip: Add support of external clock to ethernet node " Anand Moon
@ 2022-11-22 11:18   ` Michael Riesch
  2022-11-23 13:00     ` Anand Moon
  0 siblings, 1 reply; 27+ messages in thread
From: Michael Riesch @ 2022-11-22 11:18 UTC (permalink / raw)
  To: Anand Moon, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Chukun Pan, Peter Geis, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

Hi Anand,

On 11/16/22 21:01, Anand Moon wrote:
> Add support of external clock gmac1_clkin which is used as input clock
> to ethernet node.

Indeed this operating mode is defined in the schematics, thanks for the fix.

> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>

Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>
Tested-by: Michael Riesch <michael.riesch@wolfvision.net>

Best regards,
Michael

> ---
> V2: None
> ---
>  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 e1c75532dcee..b848282ea005 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>;
>  
>  	tx_delay = <0x4f>;

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

* Re: [linux-next-v2 3/5] arm64: dts: rockchip: Add support of regulator for ethernet node on Rock 3A SBC
  2022-11-16 20:01 ` [linux-next-v2 3/5] arm64: dts: rockchip: Add support of regulator for " Anand Moon
@ 2022-11-22 11:23   ` Michael Riesch
  0 siblings, 0 replies; 27+ messages in thread
From: Michael Riesch @ 2022-11-22 11:23 UTC (permalink / raw)
  To: Anand Moon, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Chukun Pan, Peter Geis, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

Hi Anand,

On 11/16/22 21:01, Anand Moon wrote:
> Add regulator support for ethernet node
> 
> Fix following warning.
> [    7.365199] rk_gmac-dwmac fe010000.ethernet: no regulator found
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>

Acked-by: Michael Riesch <michael.riesch@wolfvision.net>
Tested-by: Michael Riesch <michael.riesch@wolfvision.net>

Best regards,
Michael

> ---
> v2: new patch added
> ---
>  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index b848282ea005..5378254c57ca 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -260,6 +260,7 @@ &gmac1 {
>  	clock_in_out = "input";
>  	phy-handle = <&rgmii_phy1>;
>  	phy-mode = "rgmii";
> +	phy-supply = <&vcc_3v3>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&gmac1m1_miim
>  		     &gmac1m1_tx_bus2

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

* Re: [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC.
  2022-11-18 18:13       ` Peter Geis
@ 2022-11-22 14:19         ` Anand Moon
  0 siblings, 0 replies; 27+ messages in thread
From: Anand Moon @ 2022-11-22 14:19 UTC (permalink / raw)
  To: Peter Geis
  Cc: Michael Riesch, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner,
	Chukun Pan, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Hi Peter / Michael

On Fri, 18 Nov 2022 at 23:43, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Fri, Nov 18, 2022 at 4:35 AM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Hi Michael,
> >
> > On Fri, 18 Nov 2022 at 12:33, Michael Riesch
> > <michael.riesch@wolfvision.net> wrote:
> > >
> > > Hi Anand,
> > >
> > > On 11/16/22 21:01, 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 to support clock delay, also
> > > > add TX and RX delay for phy-mode.
> > >
> > > Based on this commit message I still don't understand what you are
> > > actually trying to fix here. If you encounter network problems/stability
> > > issues, please let me know what test triggers the faulty behavior.
> > > Please describe the problem you are facing in detail here or in the
> > > cover letter.
> > >
> >
> > Ok, Ethernet does not work on my Radxa 3A see boot logs.
> >
> > [0] https://gist.github.com/moonlinux/bb56c787031226fbb9f69121564e76a2
> >
> > Please find this updated commit message.
> >
> > As per the schematic and datasheet PHY mode is RGMII
> > Use 2ns clock delay to RXC for RXD and TXC for TXD latching.
>
> rgmii-id mode does exactly this in the phy (your realtek chip). By
> setting the mode to rgmii, you're telling the phy that delays are set
> elsewhere, either in hardware or in the controller. You're then
> handling them in the controller. While the delays aren't documented in
> the TRM, I've long suspected that the defaults of 0x30 and 0x10 equate
> to the standard 2ns delay. So you're setting the delays much higher
> than the default means you need to add *more* than the standard 2ns
> delay for your device to work.
>

TX and RX clock delay might have been updated for
     ID: 0x30, Synopsys ID: 0x51  DWMAC4/5

As per the schematic [0]
https://dl.radxa.com/rock3/docs/hw/3a/rock3a_v1.3_sch.pdf
   Pull-up for additional 2ns delay to RXC for data latching
   Pull-up for additional 2ns delay to TXC for data latching

As per the datasheet [1]
https://dl.radxa.com/rock3/docs/hw/datasheet/RTL8211F-CG-Datasheet.pdf
TXDLY   RGMII            Transmit Clock Timing Control.
    1: Add 2ns delay to RXC for RXD latching (via 4.7k-ohm to DVDD_RG)
    0: No delay (via 4.7k-ohm to GND)
RXDLY RGMII Receive Clock Timing Control.
    1: Add 2ns delay to RXC for RXD latching (via 4.7k-ohm to DVDD_RG)
    0: No delay (via 4.7k-ohm to GND)

tx_delay and rx_delay might be suitable for old DWMAC
hence it does not apply to this new Ethernet controller.
I have tested with remove these from the dts and
adding the following as this is required for rgmii mode.
       rx-internal-delay-ps = <2000>;
       tx-internal-delay-ps = <2000>;

I don't mind setting the defaults of 0x30 and 0x10 to equate
to the standard 2ns delay. but it does not have any effect
on the current ethernet controller.

> This is why I've been asking if you have tested these. You need to set
> each value and find the lowest and highest possible values that work,
> then take the median value between those two.
>

Yes, I have tested with and without those values.

Thanks
-Anand
> >
> > > > Fix the following warning
> > > >
> > > > [    7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay.
> > > > [    7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30
> > > > [    7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay.
> > > > [    7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10
> > >
> > > If the only purpose of this patch is to get rid of this warnings, it may
> >
> > No, the intent is to fix the PHY mode to RGMII and fix the delay.
> > [ 7.066357] rk_gmac-dwmac fe010000.ethernet: init for RGMII_ID
> >
> > > make sense to set them to dev_dbg as Peter pointed out.
> > >
> > Ok, will update this in the next version.
> >
> > > Best regards,
> > > Michael
> > >
> > Thanks
> > -Anand

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

* Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
  2022-11-17 19:10           ` Peter Geis
@ 2022-11-22 14:20             ` Anand Moon
  0 siblings, 0 replies; 27+ messages in thread
From: Anand Moon @ 2022-11-22 14:20 UTC (permalink / raw)
  To: Peter Geis
  Cc: Robin Murphy, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner,
	Chukun Pan, Michael Riesch, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

Hi Peter,

On Fri, 18 Nov 2022 at 00:40, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Thu, Nov 17, 2022 at 9:56 AM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Hi Robin
> >
> > On Thu, 17 Nov 2022 at 16:24, Robin Murphy <robin.murphy@arm.com> wrote:
> > >
> > > On 2022-11-17 05:57, Anand Moon wrote:
> > > > Hi Peter,
> > > >
> > > > On Thu, 17 Nov 2022 at 02:16, Peter Geis <pgwipeout@gmail.com> wrote:
> > > >>
> > > >> On Wed, Nov 16, 2022 at 3:02 PM Anand Moon <linux.amoon@gmail.com> wrote:
> > > >>>
> > > >>> Add MDIO description with ethernet-phy-id compatible string
> > > >>> which enable calling reset of the phy. The PHY will then be probed,
> > > >>> independent of if it can be found on the bus or not,
> > > >>> and that probing will enable the GPIO.
> > > >>>
> > > >>> ethernet-phy-id is read from ethenet register dump reg2 and reg3.
> > > >>>
> > > >>> Fix following warning.
> > > >>> [   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> > > >>> [   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
> > > >>> [   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)
> > > >>>
> > > >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > >>> ---
> > > >>> V2: new to the patch series.
> > > >>>
> > > >>> alarm@rock-3a:~$ sudo ethtool -d eth0
> > > >>> [sudo] password for alarm:
> > > >>> ST GMAC Registers
> > > >>> GMAC Registers
> > > >>> Reg0  0x08072203
> > > >>> Reg1  0x00000000
> > > >>> Reg2  0x00000404
> > > >>> Reg3  0x00000000
> > > >>> Reg4  0x00000002
> > > >>> ---
> > > >>>   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 9f84a23a8789..fe36156a5017 100644
> > > >>> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > > >>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > > >>> @@ -585,7 +585,7 @@ &i2s2_2ch {
> > > >>>
> > > >>>   &mdio1 {
> > > >>>          rgmii_phy1: ethernet-phy@0 {
> > > >>> -               compatible = "ethernet-phy-ieee802.3-c22";
> > > >>> +               compatible = "ethernet-phy-id0000.0404", "ethernet-phy-ieee802.3-c22";
> > > >>>                  reg = <0x0>;
> > > >>>                  pinctrl-names = "default";
> > > >>>                  pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
> > > >>
> > > >> Have you tried instead moving the reset to the mdio bus? I've had
> > > >> success with this, though you'll need to change the reset assert and
> > > >> deassert timing handles, they are different for the bus.
> > > >>
> > > > No can you share some examples?
> > > > If you got a better way to solve this issue please let me know.
> > > > I will give this a try.
> > >
> > > Note that the Rock 3A schematic says the phy is configured for address
> > > 1, not 0. From what I remember of adding the MDIO node for NanoiPi4,
> > > that didn't work if I got the address wrong, despite the fact that the
> > > auto-detection when the MDIO node is omitted claimed to find the same
> > > phy on both addresses 0 and 1.
> > >
>
> From the net-dev folk, mdio address 0 is a broadcast address. All
> functional phys we have on the mdio bus should respond to it. The
> problem I've run into is with the reset on the phy node the reset
> triggers too late for the dwmac to detect the phy correctly. However
> moving it to the mdio bus node makes the reset happen at roughly the
> same time as the depreciated dwmac reset and the phy detects without
> any weird hacks.
>

This is not a hack, we are already using mdio bus node.
&gma1 {
   phy-handle=ethernet-phy
}
mdio1 {
   ethernet-phy {
   }
}

Actually, *ethernet-phy-id* is one of the binding properties (see below).
we can find many examples in the device tree that used this property in
the linux kernel and u-boot.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ethernet-phy.yaml?h=v6.1-rc6#n31

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ethernet-phy.yaml?h=v6.1-rc6#n211

Please check this phy reset section below.

[2] https://wiki.st.com/stm32mpu/wiki/Ethernet_device_tree_configuration#RMII_with_25MHz_on_ETH_CLK_-28no_PHY_Crystal-29-2C_REF_CLK_from_PHY_-28Reference_clock_-28standard_RMII_clock_name-29_is_provided_by_a_PHY-29

If you have some other input please share I have tried but failed.

Thanks

-Anand

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

* Re: [linux-next-v2 4/5] arm64: dts: rockchip: Add support of interrupt to ethernet node on Rock 3A SBC
  2022-11-16 20:01 ` [linux-next-v2 4/5] arm64: dts: rockchip: Add support of interrupt to " Anand Moon
@ 2022-11-22 16:10   ` Michael Riesch
  2022-11-23 13:00     ` Anand Moon
  0 siblings, 1 reply; 27+ messages in thread
From: Michael Riesch @ 2022-11-22 16:10 UTC (permalink / raw)
  To: Anand Moon, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Chukun Pan, Peter Geis, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

Hi Anand,

On 11/16/22 21:01, Anand Moon wrote:
> As per the shematic gmac1 support gpio interrupt controller 

Typo "shematic" -> "schematic"

> GMAC1_INT/PMEB_GPIO3_A7 add the support for this.

Maybe split the commit message into two proper sentences.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> v2: new patch added
> ---
>  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 10 +++++++++-
>  1 file changed, 9 insertions(+), 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 5378254c57ca..9f84a23a8789 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -588,10 +588,14 @@ rgmii_phy1: ethernet-phy@0 {
>  		compatible = "ethernet-phy-ieee802.3-c22";
>  		reg = <0x0>;
>  		pinctrl-names = "default";
> -		pinctrl-0 = <&eth_phy_rst>;
> +		pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
>  		reset-assert-us = <20000>;
>  		reset-deassert-us = <100000>;
>  		reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> +		interrupt-parent = <&gpio3>;
> +		/* GMAC1_INT/PMEB_GPIO3_A7 */

This comment is pretty superfluous.

> +		interrupts = <RK_PA7 IRQ_TYPE_LEVEL_LOW>;
> +		#interrupt-cells = <1>;

I am not an expert here, but I believe #interrupt-cells = <1>; means
that the phy provides an array of interrupts. Are you sure this is
correct? I find it strange that the phy driver consumes one interrupt
and provides N interrupts?!

>  	};
>  };
>  
> @@ -630,6 +634,10 @@ vcc_mipi_en: vcc_mipi_en {
>  	};
>  
>  	ethernet {
> +		eth_phy_int: eth-phy-int {
> +			rockchip,pins = <3 RK_PA7 RK_FUNC_GPIO &pcfg_pull_down>;

Interrupt is active low and you pull down the line here? There is a pull
up resistor on sheet 11 of the schematic, so this does not seem right at
all.

Best regards,
Michael

> +		};
> +
>  		eth_phy_rst: eth_phy_rst {
>  			rockchip,pins = <3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
>  		};

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

* Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
  2022-11-16 20:01 ` [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy " Anand Moon
  2022-11-16 20:46   ` Peter Geis
@ 2022-11-22 16:46   ` Michael Riesch
  2022-12-07  4:29     ` Anand Moon
  1 sibling, 1 reply; 27+ messages in thread
From: Michael Riesch @ 2022-11-22 16:46 UTC (permalink / raw)
  To: Anand Moon, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner
  Cc: Chukun Pan, Peter Geis, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

Hi Anand,

On 11/16/22 21:01, Anand Moon wrote:
> Add MDIO description with ethernet-phy-id compatible string
> which enable calling reset of the phy. The PHY will then be probed,
> independent of if it can be found on the bus or not,
> and that probing will enable the GPIO.
> 
> ethernet-phy-id is read from ethenet register dump reg2 and reg3.
> 
> Fix following warning.
> [   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> [   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
> [   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)

Without this patch, the phy on my ROCK3A is properly detected:

[    1.494963] rk_gmac-dwmac fe010000.ethernet eth0: PHY [stmmac-0:00]
driver [RTL8211F Gigabit Ethernet)

but with the patch applied, only a generic phy is recognized:

[    1.398674] rk_gmac-dwmac fe010000.ethernet eth0: PHY [stmmac-0:00]
driver [Generic PHY] (irq=POLL)

This does not seem right at all. NACK to this patch!

Best regards,
Michael

> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> V2: new to the patch series.
> 
> alarm@rock-3a:~$ sudo ethtool -d eth0
> [sudo] password for alarm:
> ST GMAC Registers
> GMAC Registers
> Reg0  0x08072203
> Reg1  0x00000000
> Reg2  0x00000404
> Reg3  0x00000000
> Reg4  0x00000002
> ---
>  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 9f84a23a8789..fe36156a5017 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -585,7 +585,7 @@ &i2s2_2ch {
>  
>  &mdio1 {
>  	rgmii_phy1: ethernet-phy@0 {
> -		compatible = "ethernet-phy-ieee802.3-c22";
> +		compatible = "ethernet-phy-id0000.0404", "ethernet-phy-ieee802.3-c22";
>  		reg = <0x0>;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;

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

* Re: (subset) [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC.
  2022-11-16 20:01 ` [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC Anand Moon
  2022-11-16 20:36   ` Peter Geis
  2022-11-18  7:03   ` Michael Riesch
@ 2022-11-22 22:34   ` Heiko Stuebner
  2 siblings, 0 replies; 27+ messages in thread
From: Heiko Stuebner @ 2022-11-22 22:34 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Anand Moon, Rob Herring
  Cc: Heiko Stuebner, Michael Riesch, devicetree, Peter Geis,
	linux-arm-kernel, linux-kernel, Chukun Pan, linux-rockchip

On Wed, 16 Nov 2022 20:01:43 +0000, 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 to support clock delay, also
> add TX and RX delay for phy-mode.
> 
> Fix following warning
> 
> [...]

Applied, thanks!

[2/5] arm64: dts: rockchip: Add support of external clock to ethernet node on Rock 3A SBC
      commit: ef9f4b4a50206bedd931f45dd9fd57fd4c1714a6
[3/5] arm64: dts: rockchip: Add support of regulator for ethernet node on Rock 3A SBC
      commit: 79aa02ddc682558edb9bd56522ad841759c99201

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

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

* Re: [linux-next-v2 4/5] arm64: dts: rockchip: Add support of interrupt to ethernet node on Rock 3A SBC
  2022-11-22 16:10   ` Michael Riesch
@ 2022-11-23 13:00     ` Anand Moon
  0 siblings, 0 replies; 27+ messages in thread
From: Anand Moon @ 2022-11-23 13:00 UTC (permalink / raw)
  To: Michael Riesch
  Cc: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner, Chukun Pan,
	Peter Geis, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Hi Michael,

Thanks for your review comments.

On Tue, 22 Nov 2022 at 21:40, Michael Riesch
<michael.riesch@wolfvision.net> wrote:
>
> Hi Anand,
>
> On 11/16/22 21:01, Anand Moon wrote:
> > As per the shematic gmac1 support gpio interrupt controller
>
> Typo "shematic" -> "schematic"
Ok,
>
> > GMAC1_INT/PMEB_GPIO3_A7 add the support for this.
>
> Maybe split the commit message into two proper sentences.

Ok this Interrupt is used for Power Management Event (supports 3.3V pull up).
Set low if received a magic packet or wake up frame; active low.
so this interrupt is used for suspend / resume.

> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > v2: new patch added
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 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 5378254c57ca..9f84a23a8789 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > @@ -588,10 +588,14 @@ rgmii_phy1: ethernet-phy@0 {
> >               compatible = "ethernet-phy-ieee802.3-c22";
> >               reg = <0x0>;
> >               pinctrl-names = "default";
> > -             pinctrl-0 = <&eth_phy_rst>;
> > +             pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
> >               reset-assert-us = <20000>;
> >               reset-deassert-us = <100000>;
> >               reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> > +             interrupt-parent = <&gpio3>;
> > +             /* GMAC1_INT/PMEB_GPIO3_A7 */
>
> This comment is pretty superfluous.

Ok will drop this.

>
> > +             interrupts = <RK_PA7 IRQ_TYPE_LEVEL_LOW>;
> > +             #interrupt-cells = <1>;
>
> I am not an expert here, but I believe #interrupt-cells = <1>; means
> that the phy provides an array of interrupts. Are you sure this is
> correct? I find it strange that the phy driver consumes one interrupt
> and provides N interrupts?!
>
Ok will drop this.

> >       };
> >  };
> >
> > @@ -630,6 +634,10 @@ vcc_mipi_en: vcc_mipi_en {
> >       };
> >
> >       ethernet {
> > +             eth_phy_int: eth-phy-int {
> > +                     rockchip,pins = <3 RK_PA7 RK_FUNC_GPIO &pcfg_pull_down>;
>
> Interrupt is active low and you pull down the line here? There is a pull
> up resistor on sheet 11 of the schematic, so this does not seem right at
> all.
>
Actually this GPIO3_A7_d hence pull_down, this pin level triggered
So I will set this as &pcfg_pull_none.

I don't have PoE set up to verify these changes as of now.

> Best regards,
> Michael

Thanks

-Anand

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

* Re: [linux-next-v2 2/5] arm64: dts: rockchip: Add support of external clock to ethernet node on Rock 3A SBC
  2022-11-22 11:18   ` Michael Riesch
@ 2022-11-23 13:00     ` Anand Moon
  2022-11-23 13:47       ` Heiko Stuebner
  0 siblings, 1 reply; 27+ messages in thread
From: Anand Moon @ 2022-11-23 13:00 UTC (permalink / raw)
  To: Michael Riesch
  Cc: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner, Chukun Pan,
	Peter Geis, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Hi Michael, Heiko

On Tue, 22 Nov 2022 at 16:48, Michael Riesch
<michael.riesch@wolfvision.net> wrote:
>
> Hi Anand,
>
> On 11/16/22 21:01, Anand Moon wrote:
> > Add support of external clock gmac1_clkin which is used as input clock
> > to ethernet node.
>
> Indeed this operating mode is defined in the schematics, thanks for the fix.
>
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>
> Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>
> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
>

Actually, I wanted to drop these changes since looking into clk_summary
gmac1 CLK gets wrongly configured with PLL and the reference count is
not correct.

Plz do the patches, I will send the correct changes next version.

> Best regards,
> Michael
>
Thanks

-Anand

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

* Re: [linux-next-v2 2/5] arm64: dts: rockchip: Add support of external clock to ethernet node on Rock 3A SBC
  2022-11-23 13:00     ` Anand Moon
@ 2022-11-23 13:47       ` Heiko Stuebner
  0 siblings, 0 replies; 27+ messages in thread
From: Heiko Stuebner @ 2022-11-23 13:47 UTC (permalink / raw)
  To: Michael Riesch, Anand Moon
  Cc: Rob Herring, Krzysztof Kozlowski, Chukun Pan, Peter Geis,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

Hi,

Am Mittwoch, 23. November 2022, 14:00:50 CET schrieb Anand Moon:
> Hi Michael, Heiko
> 
> On Tue, 22 Nov 2022 at 16:48, Michael Riesch
> <michael.riesch@wolfvision.net> wrote:
> >
> > Hi Anand,
> >
> > On 11/16/22 21:01, Anand Moon wrote:
> > > Add support of external clock gmac1_clkin which is used as input clock
> > > to ethernet node.
> >
> > Indeed this operating mode is defined in the schematics, thanks for the fix.
> >
> > >
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> >
> > Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>
> > Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
> >
> 
> Actually, I wanted to drop these changes since looking into clk_summary
> gmac1 CLK gets wrongly configured with PLL and the reference count is
> not correct.
> 
> Plz do the patches, I will send the correct changes next version.

as you might've seen, I've applied this patch yesterday.

When doing patches also please always take into account, that devicetree
describes the hardware situation, not what the Linux-kernel does.

But assigned-clock* fixups in a next version are ok to do.

Thanks
Heiko



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

* Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
  2022-11-22 16:46   ` Michael Riesch
@ 2022-12-07  4:29     ` Anand Moon
  0 siblings, 0 replies; 27+ messages in thread
From: Anand Moon @ 2022-12-07  4:29 UTC (permalink / raw)
  To: Michael Riesch
  Cc: Rob Herring, Krzysztof Kozlowski, Heiko Stuebner, Chukun Pan,
	Peter Geis, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Hi Michael

On Tue, 22 Nov 2022 at 22:16, Michael Riesch
<michael.riesch@wolfvision.net> wrote:
>
> Hi Anand,
>
> On 11/16/22 21:01, Anand Moon wrote:
> > Add MDIO description with ethernet-phy-id compatible string
> > which enable calling reset of the phy. The PHY will then be probed,
> > independent of if it can be found on the bus or not,
> > and that probing will enable the GPIO.
> >
> > ethernet-phy-id is read from ethenet register dump reg2 and reg3.
> >
> > Fix following warning.
> > [   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> > [   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
> > [   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)
>
> Without this patch, the phy on my ROCK3A is properly detected:
>
> [    1.494963] rk_gmac-dwmac fe010000.ethernet eth0: PHY [stmmac-0:00]
> driver [RTL8211F Gigabit Ethernet)
>
> but with the patch applied, only a generic phy is recognized:
>
> [    1.398674] rk_gmac-dwmac fe010000.ethernet eth0: PHY [stmmac-0:00]
> driver [Generic PHY] (irq=POLL)
>
> This does not seem right at all. NACK to this patch!
>

Yep you are correct I found way to read the ethernet id

# there is kernel module witch help read the ethernet-id using netlink socket.
$ git clone https://github.com/wkz/mdio-tools

$ sudo mdio
fixed-0
stmmac-0
$ sudo mdio stmmac-0
 DEV      PHY-ID  LINK
0x00  0x001cc916  up
0x01  0x001cc916  up

with the above ethernet id I update the compatible string
compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22";

I could get the ethernet registered correctly.

[    9.865059] rk_gmac-dwmac fe010000.ethernet end0: Register
MEM_TYPE_PAGE_POOL RxQ-0
[   10.061904] rk_gmac-dwmac fe010000.ethernet end0: PHY [stmmac-0:00]
driver [RTL8211F Gigabit Ethernet] (irq=POLL)

Thanks
-Anand

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

end of thread, other threads:[~2022-12-07  4:29 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20221116200150.4657-1-linux.amoon@gmail.com>
2022-11-16 20:01 ` [linux-next-v2 1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC Anand Moon
2022-11-16 20:36   ` Peter Geis
2022-11-17 14:44     ` Anand Moon
2022-11-18  7:03   ` Michael Riesch
2022-11-18  9:34     ` Anand Moon
2022-11-18 18:13       ` Peter Geis
2022-11-22 14:19         ` Anand Moon
2022-11-22 22:34   ` (subset) " Heiko Stuebner
2022-11-16 20:01 ` [linux-next-v2 2/5] arm64: dts: rockchip: Add support of external clock to ethernet node " Anand Moon
2022-11-22 11:18   ` Michael Riesch
2022-11-23 13:00     ` Anand Moon
2022-11-23 13:47       ` Heiko Stuebner
2022-11-16 20:01 ` [linux-next-v2 3/5] arm64: dts: rockchip: Add support of regulator for " Anand Moon
2022-11-22 11:23   ` Michael Riesch
2022-11-16 20:01 ` [linux-next-v2 4/5] arm64: dts: rockchip: Add support of interrupt to " Anand Moon
2022-11-22 16:10   ` Michael Riesch
2022-11-23 13:00     ` Anand Moon
2022-11-16 20:01 ` Anand Moon
2022-11-16 20:01 ` [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy " Anand Moon
2022-11-16 20:46   ` Peter Geis
2022-11-17  5:57     ` Anand Moon
2022-11-17 10:54       ` Robin Murphy
2022-11-17 14:55         ` Anand Moon
2022-11-17 19:10           ` Peter Geis
2022-11-22 14:20             ` Anand Moon
2022-11-22 16:46   ` Michael Riesch
2022-12-07  4:29     ` Anand Moon

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