Re: [PATCH v2 2/2] arm64: dts: rockchip: Add Radxa ROCK 4D device tree

[Jonas Karlman <jonas@kwiboo.se>]

Hi Detlev,

On 2025-02-13 20:56, Detlev Casanova wrote:
> Hi Jonas,
> 
> On Thursday, 13 February 2025 10:48:10 EST Jonas Karlman wrote:
>> Hi Detlev,
>>
>> On 2025-02-13 15:57, Detlev Casanova wrote:
>>> From: Stephen Chen <stephen@radxa.com>
>>>
>>> The Radxa ROCK 4D board is based on the Rockchip rk3576 SoC.
>>>
>>> The device tree adds support for basic devices:
>>>  - UART
>>>  - SD Card
>>>  - Ethernet
>>>  - USB
>>>  - RTC
>>>
>>> It has 4 USB ports but only 3 are usable as the top left one is used
>>> for maskrom.
>>>
>>> It has a USB-C port that is only used for powering the board.
>>>
>>> Signed-off-by: Stephen Chen <stephen@radxa.com>
>>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
>>> ---
>>>
>>>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>>>  .../boot/dts/rockchip/rk3576-rock-4d.dts      | 651 ++++++++++++++++++
>>>  2 files changed, 652 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/Makefile
>>> b/arch/arm64/boot/dts/rockchip/Makefile index
>>> def1222c1907e..a112aeb37948a 100644
>>> --- a/arch/arm64/boot/dts/rockchip/Makefile
>>> +++ b/arch/arm64/boot/dts/rockchip/Makefile
>>> @@ -132,6 +132,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) +=
>>> rk3568-wolfvision-pf5-display-vz.dtbo> 
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-armsom-sige5.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-evb1-v10.dtb
>>>
>>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-rock-4d.dtb
>>>
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3582-radxa-e52c.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-sige7.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-w3.dtb
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
>>> b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts new file mode 100644
>>> index 0000000000000..f356742f9d643
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
>>> @@ -0,0 +1,651 @@
>>
>> [snip]
>>
>>> +&gmac0 {
>>> +	phy-mode = "rgmii-id";
>>> +	clock_in_out = "output";
>>> +
>>> +	snps,reset-gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_LOW>;
>>> +	snps,reset-active-low;
>>> +	snps,reset-delays-us = <0 20000 100000>;
>>
>> The snps,reset- props are deprecated and should be changed to reset-
>> props in the phy node.
> 
> Arg, second time I use deprectated props on new things. Are there plans or 
> ways to make dtbs_check warn about those ?

Agree, would be nice if dtbs_check could be a little bit more verbose
about use of deprecated props :-)

> 
>>> +
>>> +	pinctrl-names = "default";
>>> +	pinctrl-0 = <&eth0m0_miim
>>> +		     &eth0m0_tx_bus2
>>> +		     &eth0m0_rx_bus2
>>> +		     &eth0m0_rgmii_clk
>>> +		     &eth0m0_rgmii_bus
>>> +		     &ethm0_clk0_25m_out>;
>>> +
>>> +	phy-handle = <&rgmii_phy0>;
>>> +	status = "okay";
>>> +};
>>
>> [snip]
>>
>>> +&mdio0 {
>>> +	rgmii_phy0: phy@1 {
>>
>> Maybe ethernet-phy@1 ?
> 
> Indeed.
> 
>>> +		compatible = "ethernet-phy-ieee802.3-c22";
>>> +		reg = <0x1>;
>>> +		clocks = <&cru REFCLKO25M_GMAC0_OUT>;
>>
>> Please add reset- props here.
>>
>> Changing to use reset- props may cause issue if a RTL8211F PHY is used
>> on the board. Use a ethernet-phy-id compatible or mainline U-Boot to
>> ensure the Ethernet PHY can be discovered during probe.
> 
> Using downstream u-boot, with the RTL8211F PHY, linux can still detect the PHY 
> and use it correctly, even with reset-* props at the PHY level.
> 
> I guess I can keep those there then, unless the issues you mention are more 
> subtle than that ?
> 

Ohh, typically there has been issues for Linux to find the Ethernet PHY unless
it has first been reset by boot firmware. Something like:

  rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
  rk_gmac-dwmac fe010000.ethernet eth0: stmmac_open: Cannot attach to PHY (error: -19)

There was a chicken-and-egg issue related to detecting ethernet phy:
- 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

See [1] for more details on the ethernet phy issue that typically have
affected multiple Rockchip boards with RTL8211F PHYs. Maybe it has been
fixed in v6.13+.

[1] https://lore.kernel.org/all/47d55aca-bee6-810f-379f-9431649fefa6@kwiboo.se/

Regards,
Jonas

> 
> Detlev.
> 
> 
>