[PATCH] arm64: dts: rockchip: ROCK3B: Correct clock rates for Ethernet PHYs
Jonas Karlman
jonas at kwiboo.se
Fri Dec 20 12:42:57 PST 2024
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 at 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 at 0 {
>>>> + compatible = "ethernet-phy-ieee802.3-c22";
>>>> + reg = <0>;
>>> The phy addr used for these boards is 0x1.
>>>
>>>> + };
>>>> +};
>>>> +
>>>> +&mdio1 {
>>>> + rgmii_phy1: ethernet-phy at 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 at 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 at 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
>
More information about the Linux-rockchip
mailing list