| Message ID | 20201227130407.10991-5-wsa+renesas@sang-engineering.com |
|---|---|
| State | New |
| Headers | show |
| Series | v3u: add support for RAVB | expand |
Hi Wolfram, Thanks for your patch! On Sun, Dec 27, 2020 at 2:04 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > From: Tho Vu <tho.vu.wh@renesas.com> > > Define the Falcon board dependent part of the Ethernet-AVB device nodes. > Only AVB0 was tested because it was the only port with a PHY on current > hardware. I'm a bit confused: according to the schematics, AVB0 is wired by default to a KSZ9031 PHY connected to an RJ45 connector on the breakout-board, while AVB1-5 are wired to 88Q2110 PHYs connected to a 5port MATEnet connector on the Ethernet sub board. So all PHYs are present? (The alternative wiring for AVB0 is to a 88Q2110 PHY connected to a 1000Base-T1/TE MATEnet connector on the Ethernet sub board) > > Signed-off-by: Tho Vu <tho.vu.wh@renesas.com> > [wsa: rebased] > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts > +++ b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts > @@ -7,6 +7,7 @@ > > /dts-v1/; > #include "r8a779a0-falcon-cpu.dtsi" > +#include <dt-bindings/gpio/gpio.h> > > / { > model = "Renesas Falcon CPU and Breakout boards based on r8a779a0"; Missing ethernet0 alias, preventing U-Boot from finding the device-node and adding an appropriate "local-mac-address" property. > @@ -21,6 +22,97 @@ chosen { > }; > }; > > +&avb0 { > + pinctrl-0 = <&avb0_pins>; > + pinctrl-names = "default"; > + phy-handle = <&phy0>; > + phy-mode = "rgmii-txid"; As the default wiring of AVB0 is similar to Salvator-XS, I think the default phy-mode of "rgmii" in the base .dtsi should be fine, but "tx-internal-delay-ps" should be overridden to <2000>. > + status = "okay"; > + > + phy0: ethernet-phy@0 { > + rxc-skew-ps = <1500>; > + reg = <0>; > + interrupt-parent = <&gpio4>; > + interrupts = <16 IRQ_TYPE_LEVEL_LOW>; > + reset-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>; > + }; > +}; > + > +&avb1 { > + pinctrl-0 = <&avb1_pins>; > + pinctrl-names = "default"; > + phy-handle = <&phy1>; > + phy-mode = "rgmii-txid"; > + > + phy1: ethernet-phy@1 { Why not @0? As the PHYs are present, why not set "status" to "okay"? > + rxc-skew-ps = <1500>; This property is only supported by the Micrel PHY driver, not by the Marvell PHY driver. > + reg = <0>; > + interrupt-parent = <&gpio5>; > + interrupts = <16 IRQ_TYPE_LEVEL_LOW>; > + reset-gpios = <&gpio5 15 GPIO_ACTIVE_LOW>; > + }; > +}; Same questions and comments for all instances below. Perhaps we should postpone adding avb1-5 until they can be tested? > @@ -78,6 +170,109 @@ &i2c6 { > }; > > &pfc { > + avb0_pins: avb0 { > + mux { > + groups = "avb0_link", "avb0_mdio", "avb0_rgmii", "avb0_txcrefclk"; > + function = "avb0"; > + }; > + > + pins_mdio { > + groups = "avb0_mdio"; > + drive-strength = <21>; > + }; > + > + pins_mii_tx { Strange node name, as the "avb0_rgmii" group includes rx pins. > + groups = "avb0_rgmii"; > + drive-strength = <21>; I can't comment on the drive-strength values. 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, thank you for the reviews! > breakout-board, while AVB1-5 are wired to 88Q2110 PHYs connected to a > 5port MATEnet connector on the Ethernet sub board. So all PHYs are > present? I was under the assumption that we don't have the ethernet sub board in the lab. Sorry if I was wrong. Then I was probably just missing the correct Marvell driver for the phys when I tried to fire the interface up. I will retry (with your other suggestions, too).
Hi Wolfram, On Tue, Jan 5, 2021 at 5:27 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > breakout-board, while AVB1-5 are wired to 88Q2110 PHYs connected to a > > 5port MATEnet connector on the Ethernet sub board. So all PHYs are > > present? > > I was under the assumption that we don't have the ethernet sub board in > the lab. Sorry if I was wrong. Then I was probably just missing the > correct Marvell driver for the phys when I tried to fire the interface > up. I will retry (with your other suggestions, too). Actually I don't know if the Ethernet sub board is present or not :-) Which reminds me that the avb0 extensions should be added to r8a779a0-falcon-cpu.dtsi instead of r8a779a0-falcon.dts 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
diff --git a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts index f7f62fc40429..f5f27dece6ee 100644 --- a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts +++ b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts @@ -7,6 +7,7 @@ /dts-v1/; #include "r8a779a0-falcon-cpu.dtsi" +#include <dt-bindings/gpio/gpio.h> / { model = "Renesas Falcon CPU and Breakout boards based on r8a779a0"; @@ -21,6 +22,97 @@ chosen { }; }; +&avb0 { + pinctrl-0 = <&avb0_pins>; + pinctrl-names = "default"; + phy-handle = <&phy0>; + phy-mode = "rgmii-txid"; + status = "okay"; + + phy0: ethernet-phy@0 { + rxc-skew-ps = <1500>; + reg = <0>; + interrupt-parent = <&gpio4>; + interrupts = <16 IRQ_TYPE_LEVEL_LOW>; + reset-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>; + }; +}; + +&avb1 { + pinctrl-0 = <&avb1_pins>; + pinctrl-names = "default"; + phy-handle = <&phy1>; + phy-mode = "rgmii-txid"; + + phy1: ethernet-phy@1 { + rxc-skew-ps = <1500>; + reg = <0>; + interrupt-parent = <&gpio5>; + interrupts = <16 IRQ_TYPE_LEVEL_LOW>; + reset-gpios = <&gpio5 15 GPIO_ACTIVE_LOW>; + }; +}; + +&avb2 { + pinctrl-0 = <&avb2_pins>; + pinctrl-names = "default"; + phy-handle = <&phy2>; + phy-mode = "rgmii-txid"; + + phy2: ethernet-phy@2 { + rxc-skew-ps = <1500>; + reg = <0>; + interrupt-parent = <&gpio6>; + interrupts = <16 IRQ_TYPE_LEVEL_LOW>; + reset-gpios = <&gpio6 15 GPIO_ACTIVE_LOW>; + }; +}; + +&avb3 { + pinctrl-0 = <&avb3_pins>; + pinctrl-names = "default"; + phy-handle = <&phy3>; + phy-mode = "rgmii-txid"; + + phy3: ethernet-phy@3{ + rxc-skew-ps = <1500>; + reg = <0>; + interrupt-parent = <&gpio7>; + interrupts = <16 IRQ_TYPE_LEVEL_LOW>; + reset-gpios = <&gpio7 15 GPIO_ACTIVE_LOW>; + }; +}; + +&avb4 { + pinctrl-0 = <&avb4_pins>; + pinctrl-names = "default"; + phy-handle = <&phy4>; + phy-mode = "rgmii-txid"; + + phy4: ethernet-phy@4 { + rxc-skew-ps = <1500>; + reg = <0>; + interrupt-parent = <&gpio8>; + interrupts = <16 IRQ_TYPE_LEVEL_LOW>; + reset-gpios = <&gpio8 15 GPIO_ACTIVE_LOW>; + }; +}; + +&avb5 { + pinctrl-0 = <&avb5_pins>; + pinctrl-names = "default"; + phy-handle = <&phy5>; + phy-mode = "rgmii-txid"; + + phy5: ethernet-phy@5 { + rxc-skew-ps = <1500>; + reg = <0>; + interrupt-parent = <&gpio9>; + interrupts = <16 IRQ_TYPE_LEVEL_LOW>; + reset-gpios = <&gpio9 15 GPIO_ACTIVE_LOW>; + }; +}; + &i2c0 { pinctrl-0 = <&i2c0_pins>; pinctrl-names = "default"; @@ -78,6 +170,109 @@ &i2c6 { }; &pfc { + avb0_pins: avb0 { + mux { + groups = "avb0_link", "avb0_mdio", "avb0_rgmii", "avb0_txcrefclk"; + function = "avb0"; + }; + + pins_mdio { + groups = "avb0_mdio"; + drive-strength = <21>; + }; + + pins_mii_tx { + groups = "avb0_rgmii"; + drive-strength = <21>; + }; + + }; + + avb1_pins: avb1 { + mux { + groups = "avb1_link", "avb1_mdio", "avb1_rgmii", "avb1_txcrefclk"; + function = "avb1"; + }; + + pins_mdio { + groups = "avb1_mdio"; + drive-strength = <21>; + }; + + pins_mii_tx { + groups = "avb1_rgmii"; + drive-strength = <21>; + }; + }; + + avb2_pins: avb2 { + mux { + groups = "avb2_link", "avb2_mdio", "avb2_rgmii", "avb2_txcrefclk"; + function = "avb2"; + }; + + pins_mdio { + groups = "avb2_mdio"; + drive-strength = <21>; + }; + + pins_mii_tx { + groups = "avb2_rgmii"; + drive-strength = <21>; + }; + }; + + avb3_pins: avb3 { + mux { + groups = "avb3_link", "avb3_mdio", "avb3_rgmii", "avb3_txcrefclk"; + function = "avb3"; + }; + + pins_mdio { + groups = "avb3_mdio"; + drive-strength = <21>; + }; + + pins_mii_tx { + groups = "avb3_rgmii"; + drive-strength = <21>; + }; + }; + + avb4_pins: avb4 { + mux { + groups = "avb4_link", "avb4_mdio", "avb4_rgmii", "avb4_txcrefclk"; + function = "avb4"; + }; + + pins_mdio { + groups = "avb4_mdio"; + drive-strength = <21>; + }; + + pins_mii_tx { + groups = "avb4_rgmii"; + drive-strength = <21>; + }; + }; + + avb5_pins: avb5 { + mux { + groups = "avb5_link", "avb5_mdio", "avb5_rgmii", "avb5_txcrefclk"; + function = "avb5"; + }; + + pins_mdio { + groups = "avb5_mdio"; + drive-strength = <21>; + }; + + pins_mii_tx { + groups = "avb5_rgmii"; + drive-strength = <21>; + }; + }; + i2c0_pins: i2c0 { groups = "i2c0"; function = "i2c0";