| Message ID | 20231120070024.4079344-14-claudiu.beznea.uj@bp.renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | renesas: rzg3s: Add support for Ethernet | expand |
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 = ð0; > + eth1 = ð1; > +#else > mmc2 = &sdhi2; > #endif > }; > @@ -81,6 +84,64 @@ vcc_sdhi2: regulator2 { > }; > }; > > +#if SW_SD2_EN Likewise. > +ð0 { > + pinctrl-0 = <ð0_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
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 = ð0; >> + eth1 = ð1; >> +#else >> mmc2 = &sdhi2; >> #endif >> }; >> @@ -81,6 +84,64 @@ vcc_sdhi2: regulator2 { >> }; >> }; >> >> +#if SW_SD2_EN > > Likewise. > >> +ð0 { >> + pinctrl-0 = <ð0_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 --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 = ð0; + eth1 = ð1; +#else mmc2 = &sdhi2; #endif }; @@ -81,6 +84,64 @@ vcc_sdhi2: regulator2 { }; }; +#if SW_SD2_EN +ð0 { + pinctrl-0 = <ð0_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>; + }; +}; + +ð1 { + pinctrl-0 = <ð1_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";