| Message ID | 20241220163227.1501912-1-alicja.michalska@9elements.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: rockchip: ROCK3B: Correct clock rates for Ethernet PHYs | expand |
Hi Alicja, On 2024-12-20 17:32, Alicja Michalska wrote: > Built-in ethernet PHYs did not work on mainline kernel: > > fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0 > fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY Have you used mainline or vendor u-boot when testing? There is an existing issue with RTL8211F PHYs that they must be reset before Linux can identify them, see [1] for more details on the issue. Mainline U-Boot will reset the Ethernet PHYs on this board and that should allow for Linux to identify and use the Ethernet PHYs. [1] https://lore.kernel.org/r/47d55aca-bee6-810f-379f-9431649fefa6@kwiboo.se/ > > According to the board design, they need to be configured as output with > static TX/RX delay. This patch sets it accordingly. > > Signed-off-by: Alicja Michalska <alicja.michalska@9elements.com> > --- > .../boot/dts/rockchip/rk3568-rock-3b.dts | 68 +++++++++++-------- > 1 file changed, 38 insertions(+), 30 deletions(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts > index 3d0c1ccfaa79..5350158302e4 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts > @@ -183,37 +183,65 @@ &cpu3 { > &gmac0 { > assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>; > assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>; > - clock_in_out = "input"; > - phy-handle = <&rgmii_phy0>; > - phy-mode = "rgmii-id"; > + assigned-clock-rates = <0>, <125000000>; > + clock_in_out = "output"; > phy-supply = <&vcc_3v3>; > + phy-mode = "rgmii"; > + phy-handle = <&rgmii_phy0>; > pinctrl-names = "default"; > pinctrl-0 = <&gmac0_miim > &gmac0_tx_bus2 > &gmac0_rx_bus2 > &gmac0_rgmii_clk > - &gmac0_rgmii_bus > - &gmac0_clkinout>; > + &gmac0_rgmii_bus>; > + snps,reset-gpio = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>; > + snps,reset-active-low; > + snps,reset-delays-us = <0 20000 100000>; The snps,reset props is deprecated and reset- props should be used in the PHY node. This also changes to use 20/100 ms delay instead of current 20/50ms delay. If anything it could be changed to 20/80ms, I have only seen datasheets for rtl8211f mention minimum 10 ms and minimum 30-76 ms. > + > + tx_delay = <0x36>; > + rx_delay = <0x2d>; Are you having issue with use of rgmii-id on your board? > + > status = "okay"; > }; > > &gmac1 { > assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>; > assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>; > - clock_in_out = "input"; > - phy-handle = <&rgmii_phy1>; > - phy-mode = "rgmii-id"; > + assigned-clock-rates = <0>, <125000000>; > + clock_in_out = "output"; > phy-supply = <&vcc_3v3>; > + phy-mode = "rgmii"; > + phy-handle = <&rgmii_phy1>; > pinctrl-names = "default"; > pinctrl-0 = <&gmac1m1_miim > &gmac1m1_tx_bus2 > &gmac1m1_rx_bus2 > &gmac1m1_rgmii_clk > - &gmac1m1_rgmii_bus > - &gmac1m1_clkinout>; > + &gmac1m1_rgmii_bus>; > + snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>; > + snps,reset-active-low; > + snps,reset-delays-us = <0 50000 150000>; Same here, but now this is 50/150 ms? > + > + tx_delay = <0x47>; > + rx_delay = <0x28>; Same here, is use of rgmii-id causing issues on your boards? > + > status = "okay"; > }; > > +&mdio0 { > + rgmii_phy0: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; The phy addr used for these boards is 0x1. > + }; > +}; > + > +&mdio1 { > + rgmii_phy1: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; Same here. > + }; > +}; There should be no need to move these nodes, they should already be in alphabetical order? > + > &gpu { > mali-supply = <&vdd_gpu>; > status = "okay"; > @@ -512,26 +540,6 @@ &i2s1m0_sdi0 > status = "okay"; > }; > > -&mdio0 { > - rgmii_phy0: ethernet-phy@1 { > - compatible = "ethernet-phy-ieee802.3-c22"; If you want to use vendor u-boot you can probably change this to "ethernet-phy-id001c.c916". > - reg = <1>; > - reset-assert-us = <20000>; > - reset-deassert-us = <50000>; > - reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>; > - }; > -}; > - > -&mdio1 { > - rgmii_phy1: ethernet-phy@1 { > - compatible = "ethernet-phy-ieee802.3-c22"; Same here. Regards, Jonas > - reg = <1>; > - reset-assert-us = <20000>; > - reset-deassert-us = <50000>; > - reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>; > - }; > -}; > - > &pcie2x1 { > pinctrl-names = "default"; > pinctrl-0 = <&pcie20m1_pins>;
On Fri, Dec 20, 2024 at 05:32:27PM +0100, Alicja Michalska wrote: > Built-in ethernet PHYs did not work on mainline kernel: Builtin, as within the SoC? > > fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0 > fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY Not being able to find the PHY has nothing to do with RGMII delays. If the RGMII delays are wrong, packets will not be received/transmitted, or they will be corrupt and thrown away with checksum errors. But the PHY will be found and you can attach to it. So please leave everything to do with RGMII delays alone, at least until you have a patch which causes the PHY to be found and attached. It is likely not broken. > According to the board design, they need to be configured as output with > static TX/RX delay. This patch sets it accordingly. > > Signed-off-by: Alicja Michalska <alicja.michalska@9elements.com> > --- > .../boot/dts/rockchip/rk3568-rock-3b.dts | 68 +++++++++++-------- > 1 file changed, 38 insertions(+), 30 deletions(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts > index 3d0c1ccfaa79..5350158302e4 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts > @@ -183,37 +183,65 @@ &cpu3 { > &gmac0 { > assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>; > assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>; > - clock_in_out = "input"; > - phy-handle = <&rgmii_phy0>; > - phy-mode = "rgmii-id"; > + assigned-clock-rates = <0>, <125000000>; > + clock_in_out = "output"; > phy-supply = <&vcc_3v3>; > + phy-mode = "rgmii"; This is wrong. RGMII requires a 2ns delay between the data and clock lines. One way you can achieve this is by having extra long clock lines on the PCB. If you have that, you use phy-mode rgmii. If there are not extra long clock lines, you use rgmii-id, which indicates the MAC/PHY pair need to insert the delays. Now vendors very frequently get this wrong, monkeys typing Shakespeare until something works, with no idea why it works, or if it is correct. Using the wrong phy-mode, combined with wrong tx_delay and rx_delay settings cancel each other out and it works. But its just "Two wrongs make a right". We don't want mainline kernel code working like this. > + phy-handle = <&rgmii_phy0>; > pinctrl-names = "default"; > pinctrl-0 = <&gmac0_miim > &gmac0_tx_bus2 > &gmac0_rx_bus2 > &gmac0_rgmii_clk > - &gmac0_rgmii_bus > - &gmac0_clkinout>; > + &gmac0_rgmii_bus>; > + snps,reset-gpio = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>; > + snps,reset-active-low; > + snps,reset-delays-us = <0 20000 100000>; You are adding this here and removing the equivalent from the PHY node. Using the PHY node properties is the recommended way to do this, since it is the same for all PHYs, where as the vendor snps, properties only work for this device. So, now you have something that works, slowly undo all the changes, and find what actually made it work. But you need to undo stuff in pairs. Remove all the RGMII delay related changes and test. Remove the reset GPIO changes and test, etc. Andrew
Hello Jonas, On 20/12/2024 18:09, Jonas Karlman wrote: > Have you used mainline or vendor u-boot when testing? > > There is an existing issue with RTL8211F PHYs that they must be reset > before Linux can identify them, see [1] for more details on the issue. > > Mainline U-Boot will reset the Ethernet PHYs on this board and that > should allow for Linux to identify and use the Ethernet PHYs. > > [1] https://lore.kernel.org/r/47d55aca-bee6-810f-379f-9431649fefa6@kwiboo.se/ Thank you for the link. I've tested with vendor-provided U-Boot build: U-Boot latest-2023.10-11-cc60ff40-gcc60ff40 (Aug 14 2024 - 03:56:35 +0000) I can test with upstream in upcoming days. > The snps,reset props is deprecated and reset- props should be used in > the PHY node. > > This also changes to use 20/100 ms delay instead of current 20/50ms > delay. If anything it could be changed to 20/80ms, I have only seen > datasheets for rtl8211f mention minimum 10 ms and minimum 30-76 ms. > >> + >> + tx_delay = <0x36>; >> + rx_delay = <0x2d>; > Are you having issue with use of rgmii-id on your board? Interesting insight, thank you! I have to admit that I haven't read the current documentation, just seen that other RK356X boards in the tree used it. My apologies. I've ran into issues with rgmii-id, but that's with vendor U-Boot (as mentioned above), will try with upstream next and see a follow-up if I will still experience issues. >> + >> status = "okay"; >> }; >> >> &gmac1 { >> assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>; >> assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>; >> - clock_in_out = "input"; >> - phy-handle = <&rgmii_phy1>; >> - phy-mode = "rgmii-id"; >> + assigned-clock-rates = <0>, <125000000>; >> + clock_in_out = "output"; >> phy-supply = <&vcc_3v3>; >> + phy-mode = "rgmii"; >> + phy-handle = <&rgmii_phy1>; >> pinctrl-names = "default"; >> pinctrl-0 = <&gmac1m1_miim >> &gmac1m1_tx_bus2 >> &gmac1m1_rx_bus2 >> &gmac1m1_rgmii_clk >> - &gmac1m1_rgmii_bus >> - &gmac1m1_clkinout>; >> + &gmac1m1_rgmii_bus>; >> + snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>; >> + snps,reset-active-low; >> + snps,reset-delays-us = <0 50000 150000>; > Same here, but now this is 50/150 ms? I wasn't sure about those delays, as I've taken them from vendor's sources. They did seem awfully high, though. >> + >> + tx_delay = <0x47>; >> + rx_delay = <0x28>; > Same here, is use of rgmii-id causing issues on your boards? Answer above :) >> + >> status = "okay"; >> }; >> >> +&mdio0 { >> + rgmii_phy0: ethernet-phy@0 { >> + compatible = "ethernet-phy-ieee802.3-c22"; >> + reg = <0>; > The phy addr used for these boards is 0x1. > >> + }; >> +}; >> + >> +&mdio1 { >> + rgmii_phy1: ethernet-phy@0 { >> + compatible = "ethernet-phy-ieee802.3-c22"; >> + reg = <0>; > Same here. Makes sense, other boards in the tree had it configured the same way. >> + }; >> +}; > There should be no need to move these nodes, they should already be in > alphabetical order? There's no reason to, just thought moving them closer to PHYs would look cleaner and make a bit more sense. It's just a nitpick though. >> + >> &gpu { >> mali-supply = <&vdd_gpu>; >> status = "okay"; >> @@ -512,26 +540,6 @@ &i2s1m0_sdi0 >> status = "okay"; >> }; >> >> -&mdio0 { >> - rgmii_phy0: ethernet-phy@1 { >> - compatible = "ethernet-phy-ieee802.3-c22"; > If you want to use vendor u-boot you can probably change this to > "ethernet-phy-id001c.c916". > >> - reg = <1>; >> - reset-assert-us = <20000>; >> - reset-deassert-us = <50000>; >> - reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>; >> - }; >> -}; >> - >> -&mdio1 { >> - rgmii_phy1: ethernet-phy@1 { >> - compatible = "ethernet-phy-ieee802.3-c22"; > Same here. True, but in this case it likely wouldn't be accepted in upstream, right? I really would prefer devicetrees to be passed to the OS by bootloader, just like ACPI does on x86 and some (i.e Ampere) platforms. This way we wouldn't need to maintain per-board DTs in the tree, and would keep functionality between kernel versions/BSDs... but one can dream, huh? :) Thank you, Alicja
Good Afternoon, On Fri, Dec 20, 2024 at 12:54 PM Alicja Michalska <alicja.michalska@9elements.com> wrote: > > Hello Jonas, > > > On 20/12/2024 18:09, Jonas Karlman wrote: > > Have you used mainline or vendor u-boot when testing? > > > > There is an existing issue with RTL8211F PHYs that they must be reset > > before Linux can identify them, see [1] for more details on the issue. > > > > Mainline U-Boot will reset the Ethernet PHYs on this board and that > > should allow for Linux to identify and use the Ethernet PHYs. > > > > [1] https://lore.kernel.org/r/47d55aca-bee6-810f-379f-9431649fefa6@kwiboo.se/ > > Thank you for the link. I've tested with vendor-provided U-Boot build: > > U-Boot latest-2023.10-11-cc60ff40-gcc60ff40 (Aug 14 2024 - 03:56:35 +0000) > > I can test with upstream in upcoming days. > > > The snps,reset props is deprecated and reset- props should be used in > > the PHY node. > > > > This also changes to use 20/100 ms delay instead of current 20/50ms > > delay. If anything it could be changed to 20/80ms, I have only seen > > datasheets for rtl8211f mention minimum 10 ms and minimum 30-76 ms. > > > >> + > >> + tx_delay = <0x36>; > >> + rx_delay = <0x2d>; > > Are you having issue with use of rgmii-id on your board? > > Interesting insight, thank you! > > I have to admit that I haven't read the current documentation, just seen that other RK356X boards in the tree used it. My apologies. > > I've ran into issues with rgmii-id, but that's with vendor U-Boot (as mentioned above), will try with upstream next and see a follow-up if I will still experience issues. rgmii-id sets the phy to handle the tx and rx delays. All of the various rgmii settings require the board to be built to specific tolerances for the data lines. Rockchip based boards seem to forgo this design phase, instead using rgmii (no built in delays) and set the delays in the Rockchip gmac driver instead, using the tx_delay and rx_delay values in the device tree. The tx_delay and rx_delay value must be tuned correctly for your hardware. If you wish to use them you'll need to set rgmii, then find the lowest and highest values that work for all variations of hardware supported by the device tree then take the middle value. If you wish to use rgmii-id, these values should be omitted or set to zero as they are additive to the existing delays. > > >> + > >> status = "okay"; > >> }; > >> > >> &gmac1 { > >> assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>; > >> assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>; > >> - clock_in_out = "input"; > >> - phy-handle = <&rgmii_phy1>; > >> - phy-mode = "rgmii-id"; > >> + assigned-clock-rates = <0>, <125000000>; > >> + clock_in_out = "output"; Instead, I suspect the clocks here are your issue. Looking at the schematic at [1] your board is designed to feed the phy clock into the SoC. This isn't the current clock configuration in your device tree, you had it set to input, but the clock parents are wrong and will default to using the internal SoC clock source for the mac. I suspect setting the assigned clock rate is what fixed your issue. I wrote the rk3566-quartz64-a.dts to be an example for how the new soc behaves. I recommend looking at it for an example for how the clocks are supposed to be set up for proper input [2]. > >> phy-supply = <&vcc_3v3>; > >> + phy-mode = "rgmii"; > >> + phy-handle = <&rgmii_phy1>; > >> pinctrl-names = "default"; > >> pinctrl-0 = <&gmac1m1_miim > >> &gmac1m1_tx_bus2 > >> &gmac1m1_rx_bus2 > >> &gmac1m1_rgmii_clk > >> - &gmac1m1_rgmii_bus > >> - &gmac1m1_clkinout>; > >> + &gmac1m1_rgmii_bus>; > >> + snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>; > >> + snps,reset-active-low; > >> + snps,reset-delays-us = <0 50000 150000>; > > Same here, but now this is 50/150 ms? > I wasn't sure about those delays, as I've taken them from vendor's sources. They did seem awfully high, though. > >> + > >> + tx_delay = <0x47>; > >> + rx_delay = <0x28>; > > Same here, is use of rgmii-id causing issues on your boards? > Answer above :) > >> + > >> status = "okay"; > >> }; > >> > >> +&mdio0 { > >> + rgmii_phy0: ethernet-phy@0 { > >> + compatible = "ethernet-phy-ieee802.3-c22"; > >> + reg = <0>; > > The phy addr used for these boards is 0x1. > > > >> + }; > >> +}; > >> + > >> +&mdio1 { > >> + rgmii_phy1: ethernet-phy@0 { > >> + compatible = "ethernet-phy-ieee802.3-c22"; > >> + reg = <0>; > > Same here. > Makes sense, other boards in the tree had it configured the same way. > >> + }; > >> +}; > > There should be no need to move these nodes, they should already be in > > alphabetical order? > There's no reason to, just thought moving them closer to PHYs would look cleaner and make a bit more sense. It's just a nitpick though. > >> + > >> &gpu { > >> mali-supply = <&vdd_gpu>; > >> status = "okay"; > >> @@ -512,26 +540,6 @@ &i2s1m0_sdi0 > >> status = "okay"; > >> }; > >> > >> -&mdio0 { > >> - rgmii_phy0: ethernet-phy@1 { > >> - compatible = "ethernet-phy-ieee802.3-c22"; > > If you want to use vendor u-boot you can probably change this to > > "ethernet-phy-id001c.c916". > > > >> - reg = <1>; > >> - reset-assert-us = <20000>; > >> - reset-deassert-us = <50000>; > >> - reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>; > >> - }; > >> -}; > >> - > >> -&mdio1 { > >> - rgmii_phy1: ethernet-phy@1 { > >> - compatible = "ethernet-phy-ieee802.3-c22"; > > Same here. > > True, but in this case it likely wouldn't be accepted in upstream, right? > > I really would prefer devicetrees to be passed to the OS by bootloader, just like ACPI does on x86 and some (i.e Ampere) platforms. > > This way we wouldn't need to maintain per-board DTs in the tree, and would keep functionality between kernel versions/BSDs... but one can dream, huh? :) The device-tree is supposed to describe hardware without regards to a specific operating system. If written correctly, it will work no matter what software is running on top of it. While there are some specific tweaks to make the operating systems behave correctly around broken hardware, a lot of the deviations from this standard are due to legacy trees and are being cleaned up over time. > > Thank you, > > Alicja Very Respectfully, Peter Geis [1] https://dl.radxa.com/rock3/docs/hw/3b/Radxa_ROCK_3B_V1.51_SCH.pdf [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts?h=v6.13-rc3#n267 > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi, On 2024-12-20 20:52, Peter Geis wrote: > Good Afternoon, > > On Fri, Dec 20, 2024 at 12:54 PM Alicja Michalska > <alicja.michalska@9elements.com> wrote: >> >> Hello Jonas, >> >> >> On 20/12/2024 18:09, Jonas Karlman wrote: >>> Have you used mainline or vendor u-boot when testing? >>> >>> There is an existing issue with RTL8211F PHYs that they must be reset >>> before Linux can identify them, see [1] for more details on the issue. >>> >>> Mainline U-Boot will reset the Ethernet PHYs on this board and that >>> should allow for Linux to identify and use the Ethernet PHYs. >>> >>> [1] https://lore.kernel.org/r/47d55aca-bee6-810f-379f-9431649fefa6@kwiboo.se/ >> >> Thank you for the link. I've tested with vendor-provided U-Boot build: >> >> U-Boot latest-2023.10-11-cc60ff40-gcc60ff40 (Aug 14 2024 - 03:56:35 +0000) >> >> I can test with upstream in upcoming days. >> >>> The snps,reset props is deprecated and reset- props should be used in >>> the PHY node. >>> >>> This also changes to use 20/100 ms delay instead of current 20/50ms >>> delay. If anything it could be changed to 20/80ms, I have only seen >>> datasheets for rtl8211f mention minimum 10 ms and minimum 30-76 ms. >>> >>>> + >>>> + tx_delay = <0x36>; >>>> + rx_delay = <0x2d>; >>> Are you having issue with use of rgmii-id on your board? >> >> Interesting insight, thank you! >> >> I have to admit that I haven't read the current documentation, just seen that other RK356X boards in the tree used it. My apologies. >> >> I've ran into issues with rgmii-id, but that's with vendor U-Boot (as mentioned above), will try with upstream next and see a follow-up if I will still experience issues. > > rgmii-id sets the phy to handle the tx and rx delays. All of the > various rgmii settings require the board to be built to specific > tolerances for the data lines. Rockchip based boards seem to forgo > this design phase, instead using rgmii (no built in delays) and set > the delays in the Rockchip gmac driver instead, using the tx_delay and > rx_delay values in the device tree. The tx_delay and rx_delay value > must be tuned correctly for your hardware. If you wish to use them > you'll need to set rgmii, then find the lowest and highest values that > work for all variations of hardware supported by the device tree then > take the middle value. If you wish to use rgmii-id, these values > should be omitted or set to zero as they are additive to the existing > delays. > >> >>>> + >>>> status = "okay"; >>>> }; >>>> >>>> &gmac1 { >>>> assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>; >>>> assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>; >>>> - clock_in_out = "input"; >>>> - phy-handle = <&rgmii_phy1>; >>>> - phy-mode = "rgmii-id"; >>>> + assigned-clock-rates = <0>, <125000000>; >>>> + clock_in_out = "output"; > > Instead, I suspect the clocks here are your issue. Looking at the > schematic at [1] your board is designed to feed the phy clock into the > SoC. This isn't the current clock configuration in your device tree, > you had it set to input, but the clock parents are wrong and will > default to using the internal SoC clock source for the mac. I suspect > setting the assigned clock rate is what fixed your issue. Correct, I should probably have used the gmacX_clkin instead of clk_macX_2top in the initial ROCK 3B device tree. However, Ethernet was tested using iperf3 on both interfaces and was working at that time so did not pay too close attention ;-) The change to use snps,reset- props is probably what made the PHY work. As pointed out in the thread I linked, there is an chicken-and-egg issue with the PHY not being probed unless it has been reset and a proper phy_id can be read back over mdio bus: As I mentioned earlier there is a chicken-and-egg problem to be solved: - phy needs to be reset or phy_id read back as 0xffffffff on mdio bus - phy device is not created because a valid phy_id is not read back - phy device needs to be created before it can be reset A reset need to happen before/during this call chain: stmmac_mdio_register() -> of_mdiobus_register() -> of_mdiobus_register_phy() -> fwnode_mdiobus_register_phy() -> get_phy_device() -> get_phy_c22_id() -> mdiobus_read(bus, addr, MII_PHYSID1) Here mdiobus_read cannot read back the phy_id unless the PHY has been reset. This is why the deprecated props work, the reset is issued before/during stmmac_mdio_register(). Or at the mdiobus level, the reset is issued before/during of_mdiobus_register(). With reset at phy level the reset happens after device is created, after phy_id has been read, hence a chicken-and-egg problem. Currently I know of these workarounds: a) reset PHY before OS, i.e. use mainline U-Boot b) use a ethernet-phy-id001c.c916 compatible for the onboard RTL8211F PHY instead of the generic ethernet-phy-ieee802.3-c22 compatible c) use deprecated snps,reset- props d) use reset props on mdiobus level > > I wrote the rk3566-quartz64-a.dts to be an example for how the new soc > behaves. I recommend looking at it for an example for how the clocks > are supposed to be set up for proper input [2]. rk3566-quartz64-a.dts use the deprecated snps,reset- props and is not affected by same Ethernet PHY reset/probe issue. > >>>> phy-supply = <&vcc_3v3>; >>>> + phy-mode = "rgmii"; >>>> + phy-handle = <&rgmii_phy1>; >>>> pinctrl-names = "default"; >>>> pinctrl-0 = <&gmac1m1_miim >>>> &gmac1m1_tx_bus2 >>>> &gmac1m1_rx_bus2 >>>> &gmac1m1_rgmii_clk >>>> - &gmac1m1_rgmii_bus >>>> - &gmac1m1_clkinout>; >>>> + &gmac1m1_rgmii_bus>; >>>> + snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>; >>>> + snps,reset-active-low; >>>> + snps,reset-delays-us = <0 50000 150000>; >>> Same here, but now this is 50/150 ms? >> I wasn't sure about those delays, as I've taken them from vendor's sources. They did seem awfully high, though. >>>> + >>>> + tx_delay = <0x47>; >>>> + rx_delay = <0x28>; >>> Same here, is use of rgmii-id causing issues on your boards? >> Answer above :) >>>> + >>>> status = "okay"; >>>> }; >>>> >>>> +&mdio0 { >>>> + rgmii_phy0: ethernet-phy@0 { >>>> + compatible = "ethernet-phy-ieee802.3-c22"; >>>> + reg = <0>; >>> The phy addr used for these boards is 0x1. >>> >>>> + }; >>>> +}; >>>> + >>>> +&mdio1 { >>>> + rgmii_phy1: ethernet-phy@0 { >>>> + compatible = "ethernet-phy-ieee802.3-c22"; >>>> + reg = <0>; >>> Same here. >> Makes sense, other boards in the tree had it configured the same way. >>>> + }; >>>> +}; >>> There should be no need to move these nodes, they should already be in >>> alphabetical order? >> There's no reason to, just thought moving them closer to PHYs would look cleaner and make a bit more sense. It's just a nitpick though. >>>> + >>>> &gpu { >>>> mali-supply = <&vdd_gpu>; >>>> status = "okay"; >>>> @@ -512,26 +540,6 @@ &i2s1m0_sdi0 >>>> status = "okay"; >>>> }; >>>> >>>> -&mdio0 { >>>> - rgmii_phy0: ethernet-phy@1 { >>>> - compatible = "ethernet-phy-ieee802.3-c22"; >>> If you want to use vendor u-boot you can probably change this to >>> "ethernet-phy-id001c.c916". >>> >>>> - reg = <1>; >>>> - reset-assert-us = <20000>; >>>> - reset-deassert-us = <50000>; >>>> - reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>; >>>> - }; >>>> -}; >>>> - >>>> -&mdio1 { >>>> - rgmii_phy1: ethernet-phy@1 { >>>> - compatible = "ethernet-phy-ieee802.3-c22"; >>> Same here. >> >> True, but in this case it likely wouldn't be accepted in upstream, right? Should be fine, as it correctly define the Ethernet PHYs used. Regards, Jonas >> >> I really would prefer devicetrees to be passed to the OS by bootloader, just like ACPI does on x86 and some (i.e Ampere) platforms. >> >> This way we wouldn't need to maintain per-board DTs in the tree, and would keep functionality between kernel versions/BSDs... but one can dream, huh? :) > > The device-tree is supposed to describe hardware without regards to a > specific operating system. If written correctly, it will work no > matter what software is running on top of it. While there are some > specific tweaks to make the operating systems behave correctly around > broken hardware, a lot of the deviations from this standard are due to > legacy trees and are being cleaned up over time. > >> >> Thank you, >> >> Alicja > > Very Respectfully, > Peter Geis > > [1] https://dl.radxa.com/rock3/docs/hw/3b/Radxa_ROCK_3B_V1.51_SCH.pdf > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts?h=v6.13-rc3#n267 >
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts index 3d0c1ccfaa79..5350158302e4 100644 --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts @@ -183,37 +183,65 @@ &cpu3 { &gmac0 { assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>; assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>; - clock_in_out = "input"; - phy-handle = <&rgmii_phy0>; - phy-mode = "rgmii-id"; + assigned-clock-rates = <0>, <125000000>; + clock_in_out = "output"; phy-supply = <&vcc_3v3>; + phy-mode = "rgmii"; + phy-handle = <&rgmii_phy0>; pinctrl-names = "default"; pinctrl-0 = <&gmac0_miim &gmac0_tx_bus2 &gmac0_rx_bus2 &gmac0_rgmii_clk - &gmac0_rgmii_bus - &gmac0_clkinout>; + &gmac0_rgmii_bus>; + snps,reset-gpio = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>; + snps,reset-active-low; + snps,reset-delays-us = <0 20000 100000>; + + tx_delay = <0x36>; + rx_delay = <0x2d>; + status = "okay"; }; &gmac1 { assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>; assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>; - clock_in_out = "input"; - phy-handle = <&rgmii_phy1>; - phy-mode = "rgmii-id"; + assigned-clock-rates = <0>, <125000000>; + clock_in_out = "output"; phy-supply = <&vcc_3v3>; + phy-mode = "rgmii"; + phy-handle = <&rgmii_phy1>; pinctrl-names = "default"; pinctrl-0 = <&gmac1m1_miim &gmac1m1_tx_bus2 &gmac1m1_rx_bus2 &gmac1m1_rgmii_clk - &gmac1m1_rgmii_bus - &gmac1m1_clkinout>; + &gmac1m1_rgmii_bus>; + snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>; + snps,reset-active-low; + snps,reset-delays-us = <0 50000 150000>; + + tx_delay = <0x47>; + rx_delay = <0x28>; + status = "okay"; }; +&mdio0 { + rgmii_phy0: ethernet-phy@0 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <0>; + }; +}; + +&mdio1 { + rgmii_phy1: ethernet-phy@0 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <0>; + }; +}; + &gpu { mali-supply = <&vdd_gpu>; status = "okay"; @@ -512,26 +540,6 @@ &i2s1m0_sdi0 status = "okay"; }; -&mdio0 { - rgmii_phy0: ethernet-phy@1 { - compatible = "ethernet-phy-ieee802.3-c22"; - reg = <1>; - reset-assert-us = <20000>; - reset-deassert-us = <50000>; - reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>; - }; -}; - -&mdio1 { - rgmii_phy1: ethernet-phy@1 { - compatible = "ethernet-phy-ieee802.3-c22"; - reg = <1>; - reset-assert-us = <20000>; - reset-deassert-us = <50000>; - reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>; - }; -}; - &pcie2x1 { pinctrl-names = "default"; pinctrl-0 = <&pcie20m1_pins>;
Built-in ethernet PHYs did not work on mainline kernel: fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0 fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY According to the board design, they need to be configured as output with static TX/RX delay. This patch sets it accordingly. Signed-off-by: Alicja Michalska <alicja.michalska@9elements.com> --- .../boot/dts/rockchip/rk3568-rock-3b.dts | 68 +++++++++++-------- 1 file changed, 38 insertions(+), 30 deletions(-)