Message-ID: <852071ad-24e8-40a8-9b10-623abf1dc4bf@kwiboo.se>
Date: Wed, 17 Jan 2024 09:15:19 +0100
From: Jonas Karlman <jonas@...boo.se>
To: Trevor Woerner <twoerner@...il.com>, "Chen-Yu Tsai" <wens@...e.org>
Cc: linux-kernel@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley
 <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: rock-pi-e: fix location of snps
 properties

On 2024-01-17 06:47, Trevor Woerner wrote:
> On Wed 2024-01-17 @ 12:38:39 PM, Chen-Yu Tsai wrote:
>> On Wed, Jan 17, 2024 at 4:41 AM Trevor Woerner <twoerner@...il.com> wrote:
>>>
>>> A number of snps (Synopsys) properties are not in their correct location.
>>
>> Nope. If you read the snps,dwmac.yaml binding file, you'll see that these
>> properties have been deprecated. They are properties pertaining to the PHY
>> and should be described under the PHY node. Support for reset GPIOs on PHY
>> devices in phylib has been there since v4.16. The snps prefixed properties
>> were deprecated in v5.3.
> 
> If that's the case, then the bindings and the drivers are out of sync in both
> U-Boot and Linux. I discovered this issue while working with older and newer
> revisions of the rock-pi-e board. The first three spins of the rock-pi-e have
> the rtl8211e PHY but the last one (v1.21) has the rtl8211f PHY.
> 
> With the existing layout nothing works in U-Boot and in Linux the rtl8211e
> works but not the rtl8211f. With this patch both the rtl8211e and the rtl8211f
> PHYs work using the exact same device trees on both older and newer rock-pi-e
> boards in both U-Boot and Linux.

For linux this is probably related to the same chicken-and-egg reset
issue outlined at [1]. The phy is not reset before it is probed and
cannot be probed because it is not reset.

As for U-Boot the designware/gmac_rockchip ethernet driver may need some
adjustments to properly integrate with eth-phy uclass to properly reset
the phy described in a ethernet-phy node before it can be probed.

Following config options is disabled for rock-pi-e-rk3328 defconfig:

# CONFIG_DM_ETH_PHY is not set
# CONFIG_PHY_REALTEK is not set

Also the driver needs to be updated to make use eth-phy uclass for it to
work correctly. More similar to how the dwc_eth_qos driver works related
to ethernet phy.

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

Regards,
Jonas

> 
> Comparing the rock-pi-e's dts file with the one from the roc-rk3328-cc board,
> which also uses the layout in this patch, and which also uses the rtl8211e
> external PHY, is what led me in this direction.
> 
>>
>> ChenYu
>>
>>> Fixes: b918e81f2145 ("arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E")
>>> Signed-off-by: Trevor Woerner <twoerner@...il.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
>>> index 096cfa19036e..0739b8fec86e 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
>>> @@ -150,8 +150,11 @@ &gmac2io {
>>>         phy-mode = "rgmii";
>>>         phy-supply = <&vcc_io>;
>>>         pinctrl-names = "default";
>>> -       pinctrl-0 = <&rgmiim1_pins>;
>>> +       pinctrl-0 = <&rgmiim1_pins>, <&eth_phy_reset_pin>;
>>>         snps,aal;
>>> +       snps,reset-gpio = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>;
>>> +       snps,reset-active-low;
>>> +       snps,reset-delays-us = <0 10000 50000>;
>>>         snps,rxpbl = <0x4>;
>>>         snps,txpbl = <0x4>;
>>>         tx_delay = <0x26>;
>>> @@ -165,13 +168,10 @@ mdio {
>>>
>>>                 rtl8211: ethernet-phy@1 {
>>>                         reg = <1>;
>>> -                       pinctrl-0 = <&eth_phy_int_pin>, <&eth_phy_reset_pin>;
>>> +                       pinctrl-0 = <&eth_phy_int_pin>;
>>>                         pinctrl-names = "default";
>>>                         interrupt-parent = <&gpio1>;
>>>                         interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
>>> -                       reset-assert-us = <10000>;
>>> -                       reset-deassert-us = <50000>;
>>> -                       reset-gpios = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>;
>>>                 };
>>>         };
>>>  };
>>> --
>>> 2.43.0.76.g1a87c842ece3
>>>
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Powered by blists - more mailing lists