diff mbox series

[13/14] arm64: dts: renesas: rzg3s-smarc-som: Enable Ethernet interfaces

Message ID 20231120070024.4079344-14-claudiu.beznea.uj@bp.renesas.com (mailing list archive)
State Not Applicable
Headers show
Series renesas: rzg3s: Add support for Ethernet | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Claudiu Nov. 20, 2023, 7 a.m. UTC 
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

RZ/G3S Smarc Module has Ethernet PHYs (KSZ9131) connected to each Ethernet
IP. For this add proper DT bindings to enable the Ethernet communication
though these PHYs.

The interface b/w PHYs and MACs is RGMII. The skew settings were set to
zero as based on phy-mode (rgmii-id) the KSZ9131 driver enables internal
DLL which adds 2ns delay b/w clocks (TX/RX) and data signals.

Different pin settings were applied to TXC, TX_CTL compared with the rest
of the RGMII pins to comply with requirements for these pins imposed by
HW manual of RZ/G3S (see chapters "Ether Ch0 Voltage Mode Control
Register (ETH0_POC)", "Ether Ch1 Voltage Mode Control Register (ETH1_POC)",
for power source selection, "Ether MII/RGMII Mode Control Register
(ETH_MODE)" for output-enable and "Input Enable Control Register (IEN_m)"
for input-enable configurations).

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Hi, Geert,

Please note that at the moment (on top of linux-next) the PHYs are probed
in poll mode as the interrupt controller support is not included yet.
It is work in progress (see [1]).

Thank you,
Claudiu Beznea

[1] https://lore.kernel.org/all/20231115142749.853106-1-claudiu.beznea.uj@bp.renesas.com/

 .../boot/dts/renesas/rzg3s-smarc-som.dtsi     | 145 +++++++++++++++++-
 1 file changed, 144 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven Dec. 6, 2023, 11:22 a.m. UTC | #1 
Hi Claudiu,

Thanks for your patch!

On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> RZ/G3S Smarc Module has Ethernet PHYs (KSZ9131) connected to each Ethernet
> IP. For this add proper DT bindings to enable the Ethernet communication
> though these PHYs.
>
> The interface b/w PHYs and MACs is RGMII. The skew settings were set to
> zero as based on phy-mode (rgmii-id) the KSZ9131 driver enables internal
> DLL which adds 2ns delay b/w clocks (TX/RX) and data signals.

So shouldn't you just use phy-mode "rgmii" instead?

> Different pin settings were applied to TXC, TX_CTL compared with the rest
> of the RGMII pins to comply with requirements for these pins imposed by
> HW manual of RZ/G3S (see chapters "Ether Ch0 Voltage Mode Control
> Register (ETH0_POC)", "Ether Ch1 Voltage Mode Control Register (ETH1_POC)",
> for power source selection, "Ether MII/RGMII Mode Control Register
> (ETH_MODE)" for output-enable and "Input Enable Control Register (IEN_m)"
> for input-enable configurations).
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
> @@ -25,7 +25,10 @@ / {
>
>         aliases {
>                 mmc0 = &sdhi0;
> -#if !SW_SD2_EN
> +#if SW_SD2_EN

Cfr. my comment on [PATCH 11/14], this looks odd...

> +               eth0 = &eth0;
> +               eth1 = &eth1;
> +#else
>                 mmc2 = &sdhi2;
>  #endif
>         };
> @@ -81,6 +84,64 @@ vcc_sdhi2: regulator2 {
>         };
>  };
>
> +#if SW_SD2_EN

Likewise.

> +&eth0 {
> +       pinctrl-0 = <&eth0_pins>;
> +       pinctrl-names = "default";
> +       phy-handle = <&phy0>;
> +       phy-mode = "rgmii-id";
> +       #address-cells = <1>;
> +       #size-cells = <0>;

#{address,size}-cells should be in the SoC-specific .dtsi.
Same for eth1.

> +       status = "okay";

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Claudiu Dec. 6, 2023, 11:48 a.m. UTC | #2 
Hi, Geert,

On 06.12.2023 13:22, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> Thanks for your patch!
> 
> On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> RZ/G3S Smarc Module has Ethernet PHYs (KSZ9131) connected to each Ethernet
>> IP. For this add proper DT bindings to enable the Ethernet communication
>> though these PHYs.
>>
>> The interface b/w PHYs and MACs is RGMII. The skew settings were set to
>> zero as based on phy-mode (rgmii-id) the KSZ9131 driver enables internal
>> DLL which adds 2ns delay b/w clocks (TX/RX) and data signals.
> 
> So shouldn't you just use phy-mode "rgmii" instead?

I chose it like this for simpler configuration of the skew settings. The
PHY supports fixed 2ns delays which is enough for RGMII. And this is
configured based on phy-mode="rgmii-id". As this delay depends also on
soldering length I consider it better this way.

The other variant would have been using phy-mode="rgmii" + skew settings.

Also, same phy-mode is used by rzg2ul-smarc-som.dtsi which is using the
same PHY.

>> Different pin settings were applied to TXC, TX_CTL compared with the rest
>> of the RGMII pins to comply with requirements for these pins imposed by
>> HW manual of RZ/G3S (see chapters "Ether Ch0 Voltage Mode Control
>> Register (ETH0_POC)", "Ether Ch1 Voltage Mode Control Register (ETH1_POC)",
>> for power source selection, "Ether MII/RGMII Mode Control Register
>> (ETH_MODE)" for output-enable and "Input Enable Control Register (IEN_m)"
>> for input-enable configurations).
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
>> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
>> @@ -25,7 +25,10 @@ / {
>>
>>         aliases {
>>                 mmc0 = &sdhi0;
>> -#if !SW_SD2_EN
>> +#if SW_SD2_EN
> 
> Cfr. my comment on [PATCH 11/14], this looks odd...
> 
>> +               eth0 = &eth0;
>> +               eth1 = &eth1;
>> +#else
>>                 mmc2 = &sdhi2;
>>  #endif
>>         };
>> @@ -81,6 +84,64 @@ vcc_sdhi2: regulator2 {
>>         };
>>  };
>>
>> +#if SW_SD2_EN
> 
> Likewise.
> 
>> +&eth0 {
>> +       pinctrl-0 = <&eth0_pins>;
>> +       pinctrl-names = "default";
>> +       phy-handle = <&phy0>;
>> +       phy-mode = "rgmii-id";
>> +       #address-cells = <1>;
>> +       #size-cells = <0>;
> 
> #{address,size}-cells should be in the SoC-specific .dtsi.
> Same for eth1.
> 
>> +       status = "okay";
> 
> The rest LGTM.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
index e090a4837468..571cade41647 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
@@ -25,7 +25,10 @@  / {
 
 	aliases {
 		mmc0 = &sdhi0;
-#if !SW_SD2_EN
+#if SW_SD2_EN
+		eth0 = &eth0;
+		eth1 = &eth1;
+#else
 		mmc2 = &sdhi2;
 #endif
 	};
@@ -81,6 +84,64 @@  vcc_sdhi2: regulator2 {
 	};
 };
 
+#if SW_SD2_EN
+&eth0 {
+	pinctrl-0 = <&eth0_pins>;
+	pinctrl-names = "default";
+	phy-handle = <&phy0>;
+	phy-mode = "rgmii-id";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "okay";
+
+	phy0: ethernet-phy@7 {
+		reg = <7>;
+		interrupt-parent = <&pinctrl>;
+		interrupts = <RZG2L_GPIO(12, 0) IRQ_TYPE_EDGE_FALLING>;
+		rxc-skew-psec = <0>;
+		txc-skew-psec = <0>;
+		rxdv-skew-psec = <0>;
+		txen-skew-psec = <0>;
+		rxd0-skew-psec = <0>;
+		rxd1-skew-psec = <0>;
+		rxd2-skew-psec = <0>;
+		rxd3-skew-psec = <0>;
+		txd0-skew-psec = <0>;
+		txd1-skew-psec = <0>;
+		txd2-skew-psec = <0>;
+		txd3-skew-psec = <0>;
+	};
+};
+
+&eth1 {
+	pinctrl-0 = <&eth1_pins>;
+	pinctrl-names = "default";
+	phy-handle = <&phy1>;
+	phy-mode = "rgmii-id";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "okay";
+
+	phy1: ethernet-phy@7 {
+		reg = <7>;
+		interrupt-parent = <&pinctrl>;
+		interrupts = <RZG2L_GPIO(12, 1) IRQ_TYPE_EDGE_FALLING>;
+		rxc-skew-psec = <0>;
+		txc-skew-psec = <0>;
+		rxdv-skew-psec = <0>;
+		txen-skew-psec = <0>;
+		rxd0-skew-psec = <0>;
+		rxd1-skew-psec = <0>;
+		rxd2-skew-psec = <0>;
+		rxd3-skew-psec = <0>;
+		txd0-skew-psec = <0>;
+		txd1-skew-psec = <0>;
+		txd2-skew-psec = <0>;
+		txd3-skew-psec = <0>;
+	};
+};
+#endif
+
 &extal_clk {
 	clock-frequency = <24000000>;
 };
@@ -128,6 +189,88 @@  &sdhi2 {
 #endif
 
 &pinctrl {
+	eth0-phy-irq-hog {
+		gpio-hog;
+		gpios = <RZG2L_GPIO(12, 0) GPIO_ACTIVE_LOW>;
+		input;
+		line-name = "eth0-phy-irq";
+	};
+
+	eth0_pins: eth0 {
+		txc {
+			pinmux = <RZG2L_PORT_PINMUX(1, 0, 1)>;  /* ET0_TXC */
+			power-source = <1800>;
+			output-enable;
+			input-enable;
+			drive-strength-microamp = <5200>;
+		};
+
+		tx_ctl {
+			pinmux = <RZG2L_PORT_PINMUX(1, 1, 1)>;  /* ET0_TX_CTL */
+			power-source = <1800>;
+			output-enable;
+			drive-strength-microamp = <5200>;
+		};
+
+		mux {
+			pinmux = <RZG2L_PORT_PINMUX(1, 2, 1)>,	/* ET0_TXD0 */
+				 <RZG2L_PORT_PINMUX(1, 3, 1)>,	/* ET0_TXD1 */
+				 <RZG2L_PORT_PINMUX(1, 4, 1)>,	/* ET0_TXD2 */
+				 <RZG2L_PORT_PINMUX(2, 0, 1)>,	/* ET0_TXD3 */
+				 <RZG2L_PORT_PINMUX(3, 0, 1)>,	/* ET0_RXC */
+				 <RZG2L_PORT_PINMUX(3, 1, 1)>,	/* ET0_RX_CTL */
+				 <RZG2L_PORT_PINMUX(3, 2, 1)>,	/* ET0_RXD0 */
+				 <RZG2L_PORT_PINMUX(3, 3, 1)>,	/* ET0_RXD1 */
+				 <RZG2L_PORT_PINMUX(4, 0, 1)>,	/* ET0_RXD2 */
+				 <RZG2L_PORT_PINMUX(4, 1, 1)>,	/* ET0_RXD3 */
+				 <RZG2L_PORT_PINMUX(4, 3, 1)>,	/* ET0_MDC */
+				 <RZG2L_PORT_PINMUX(4, 4, 1)>,	/* ET0_MDIO */
+				 <RZG2L_PORT_PINMUX(4, 5, 1)>;	/* ET0_LINKSTA */
+			power-source = <1800>;
+		};
+	};
+
+	eth1-phy-irq-hog {
+		gpio-hog;
+		gpios = <RZG2L_GPIO(12, 1) GPIO_ACTIVE_LOW>;
+		input;
+		line-name = "eth1-phy-irq";
+	};
+
+	eth1_pins: eth1 {
+		txc {
+			pinmux = <RZG2L_PORT_PINMUX(7, 0, 1)>;	/* ET1_TXC */
+			power-source = <1800>;
+			output-enable;
+			input-enable;
+			drive-strength-microamp = <5200>;
+		};
+
+		tx_ctl {
+			pinmux = <RZG2L_PORT_PINMUX(7, 1, 1)>;	/* ET1_TX_CTL */
+			power-source = <1800>;
+			output-enable;
+			drive-strength-microamp = <5200>;
+		};
+
+		mux {
+			pinmux = <RZG2L_PORT_PINMUX(7, 2, 1)>,	/* ET1_TXD0 */
+				 <RZG2L_PORT_PINMUX(7, 3, 1)>,	/* ET1_TXD1 */
+				 <RZG2L_PORT_PINMUX(7, 4, 1)>,	/* ET1_TXD2 */
+				 <RZG2L_PORT_PINMUX(8, 0, 1)>,	/* ET1_TXD3 */
+				 <RZG2L_PORT_PINMUX(8, 4, 1)>,	/* ET1_RXC */
+				 <RZG2L_PORT_PINMUX(9, 0, 1)>,	/* ET1_RX_CTL */
+				 <RZG2L_PORT_PINMUX(9, 1, 1)>,	/* ET1_RXD0 */
+				 <RZG2L_PORT_PINMUX(9, 2, 1)>,	/* ET1_RXD1 */
+				 <RZG2L_PORT_PINMUX(9, 3, 1)>,	/* ET1_RXD2 */
+				 <RZG2L_PORT_PINMUX(10, 0, 1)>,	/* ET1_RXD3 */
+				 <RZG2L_PORT_PINMUX(10, 2, 1)>,	/* ET1_MDC */
+				 <RZG2L_PORT_PINMUX(10, 3, 1)>,	/* ET1_MDIO */
+				 <RZG2L_PORT_PINMUX(10, 4, 1)>;	/* ET1_LINKSTA */
+			power-source = <1800>;
+		};
+	};
+
 	sdhi0_pins: sd0 {
 		data {
 			pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3";