Re: [PATCH 4/5] arm64: dts: renesas: falcon: Add Ethernet-AVB support
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-01-05 16:21:50
Also in:
linux-renesas-soc, lkml
Hi Wolfram, Thanks for your patch! On Sun, Dec 27, 2020 at 2:04 PM Wolfram Sang [off-list ref] wrote:
From: Tho Vu <redacted> 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 <redacted> [wsa: rebased] Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
quoted hunk ↗ jump to hunk
--- 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.
quoted hunk ↗ jump to hunk
@@ -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?
quoted hunk ↗ jump to hunk
@@ -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