[PATCH] arm64: dts: rockchip: rock-3a: make ethernet work

Mon Jul 17 10:11:46 PDT 2023

On 2023-07-17 09:40, Michael Riesch wrote:
> Hi all,
> 
> In addition to what has been already said:
> 
> On 7/16/23 15:50, Jonas Karlman wrote:
>> On 2023-07-15 06:49, FUKAUMI Naoki wrote:
>>> hi,
>>>
>>> On 7/15/23 01:24, Jonas Karlman wrote:
>>>> On 2023-07-14 17:46, Heiko Stuebner wrote:
>>>>> Am Freitag, 14. Juli 2023, 08:30:27 CEST schrieb FUKAUMI Naoki:
>>>>>> ethernet on Radxa ROCK 3A is not working by following error:
>>>>>>
>>>>>>   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)
>>>>>>
>>>>>> to fix this problem, align related properties with vendor kernel
>>>>>>   https://github.com/radxa/kernel/blob/linux-5.10-gen-rkr4.1/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>>>>>
>>>>>> Fixes: 22a442e6586c ("arm64: dts: rockchip: add basic dts for the radxa rock3 model a")
>>>>>> Signed-off-by: FUKAUMI Naoki <naoki at radxa.com>
>>>>>
>>>>> There also is a second patch in the mix adding the gmac1_clkin
>>>>> ef9f4b4a5020 ("arm64: dts: rockchip: Add support of external clock to ethernet node on Rock 3A SBC")
>>>>>
>>>>> And this patch does the exact opposite as the original nodes.
>>>>> Can someone please mention board versions? Or did the gmac1 never
>>>>> really work in the first place?
> 
> As far as I know all schematics versions state that the PHY produces the
> clock (see the most recent schematics here:
> https://dl.radxa.com/rock3/docs/hw/3a/ROCK-3A-V1.3-SCH.pdf>> 
> Although using the internal MAC clock works as well, using the
> gmac1_clkin is most probably the correct way.
> 
>>>> Ethernet have worked and probably still works booting with vendor U-Boot.
>>>>
>>>> However, when booting using mainline U-Boot the ethernet PHY is never
>>>> initialized or reset due to lack of a ethernet gmac driver for rk35xx.
>>>
>>> surely I'm using mainline u-boot.
> 
> Ah, this might explain why I never experienced this issue: I am using
> mainline barebox with GMAC support.
> 
>>>> With an early work-in-progress gmac driver the ethernet PHY is working
>>>> same as with vendor U-Boot, and the ethernet PHY is identified.
>>>>
>>>> This revert from using reset-gpios to using the deprecated
>>>> snps,reset-gpio is probably the wrong way forward.
>>>>
>>>> I suspect there is a bug in net/phy/mdio_device.c on how reset-gpios
>>>> is asserted/deasserted, it works differently than how I would expect it
>>>> to work, and also differs in how U-Boot handles reset-gpios.
>>>>
>>>> Some earlier findings regarding this reset issue:
>>>> https://github.com/Kwiboo/linux-rockchip/compare/rk3568-eth-phy-reset~2...rk3568-eth-phy-reset
>>>>
>>>> Will try to get a proper patch/rfc out later this weekend or early next
>>>> week after re-testing that on latest kernel.
>>>
>>> thank you so much for your awesome work!
>>
>> Thanks, made some progress on tracking down the root cause.
>> From what I have discovered so far there is a chicken-and-egg problem:
>>
>> - 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
>>
>> Possible workarounds so far:
>> - phy is reset in U-Boot
>>   - very early work-in-progress at https://github.com/Kwiboo/u-boot-rockchip/commit/6ee80f9a895a32551cf30dd4252a4960ed80dfc9
>> - phy is reset using mdio bus reset
>>   - similar to old https://github.com/Kwiboo/linux-rockchip/commit/8597fcfa0c5c792dabb44a2db7b283c56c99ec6a
>> - phy is reset using deprecated snps,reset-gpio
>>   - similar to mdio bus reset
> 
> There was a similar discussion here:
> https://lore.kernel.org/all/CAMdYzYo_DGiO0UxJEb3xues7Um=X9AgPvz+Xp_YWb9pp9HaScg@mail.gmail.com/
> 
> The approach that moves the reset to the MDIO bus has been mentioned
> there as well. On the first glance this approach looks reasonable to me.

It does not look like U-Boot support mdio bus reset-gpios, so changing
to that would require adding even more code into U-Boot to get ethernet
working in U-Boot.

Moving to mdio bus would make it behave like with old snps,reset-gpio,
however phy reset-gpios still describe the hw more accurately, a
assert/deassert cycle of reset-gpios triggers a phy hard reset.

Do not know if it is possible to have both mdio bus reset and phy reset.

There also looks like there is a bug in dwmac-rk rk3568_set_to_rgmii,
tx/rx delay is always enabled. It should be disabled in some phy modes.
Can send a patch once I finish with the U-Boot ethernet driver.

Will try to complete a U-Boot driver that supports both phy reset-gpios
and the deprecated snps,reset-gpio as a first step.

Regards,
Jonas

> 
> Best regards,
> Michael
> 
>> [...]
>>>>>> ---
>>>>>>   .../boot/dts/rockchip/rk3568-rock-3a.dts      | 32 ++++++-------------
>>>>>>   1 file changed, 10 insertions(+), 22 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>>>>> index 917f5b2b8aab..f9381ab9629b 100644
>>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>>>>> @@ -32,13 +32,6 @@ hdmi_con_in: endpoint {
>>>>>>   		};
>>>>>>   	};
>>>>>>   
>>>>>> -	gmac1_clkin: external-gmac1-clock {
>>>>>> -		compatible = "fixed-clock";
>>>>>> -		clock-frequency = <125000000>;
>>>>>> -		clock-output-names = "gmac1_clkin";
>>>>>> -		#clock-cells = <0>;
>>>>>> -	};
>>>>>> -
>>>>>>   	leds {
>>>>>>   		compatible = "gpio-leds";
>>>>>>   
>>>>>> @@ -256,18 +249,24 @@ &cpu3 {
>>>>>>   
>>>>>>   &gmac1 {
>>>>>>   	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
>>>>>> -	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&gmac1_clkin>;
>>>>>> -	clock_in_out = "input";
>>>>>> +	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>;
>>>>>> +	assigned-clock-rates = <0>, <125000000>;
>>>>>> +	clock_in_out = "output";
>>>>>>   	phy-handle = <&rgmii_phy1>;
>>>>>> -	phy-mode = "rgmii-id";
>>>>>> +	phy-mode = "rgmii";
>>>>>>   	phy-supply = <&vcc_3v3>;
>>>>>>   	pinctrl-names = "default";
>>>>>>   	pinctrl-0 = <&gmac1m1_miim
>>>>>>   		     &gmac1m1_tx_bus2
>>>>>>   		     &gmac1m1_rx_bus2
>>>>>>   		     &gmac1m1_rgmii_clk
>>>>>> -		     &gmac1m1_clkinout
>>>>>>   		     &gmac1m1_rgmii_bus>;
>>>>>> +	snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
>>>>>> +	snps,reset-active-low;
>>>>>> +	/* Reset time is 20ms, 100ms for rtl8211f */
>>>>>> +	snps,reset-delays-us = <0 20000 100000>;
>>>>>> +	tx_delay = <0x42>;
>>>>>> +	rx_delay = <0x28>;
>>>>>>   	status = "okay";
>>>>>>   };
>>>>>>   
>>>>>> @@ -588,11 +587,6 @@ &mdio1 {
>>>>>>   	rgmii_phy1: ethernet-phy at 0 {
>>>>>>   		compatible = "ethernet-phy-ieee802.3-c22";
>>>>>>   		reg = <0x0>;
>>>>>> -		pinctrl-names = "default";
>>>>>> -		pinctrl-0 = <&eth_phy_rst>;
>>>>>> -		reset-assert-us = <20000>;
>>>>>> -		reset-deassert-us = <100000>;
>>>>>> -		reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
>>>>>>   	};
>>>>>>   };
>>>>>>   
>>>>>> @@ -630,12 +624,6 @@ vcc_mipi_en: vcc_mipi_en {
>>>>>>   		};
>>>>>>   	};
>>>>>>   
>>>>>> -	ethernet {
>>>>>> -		eth_phy_rst: eth_phy_rst {
>>>>>> -			rockchip,pins = <3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
>>>>>> -		};
>>>>>> -	};
>>>>>> -
>>>>>>   	hym8563 {
>>>>>>   		hym8563_int: hym8563-int {
>>>>>>   			rockchip,pins = <0 RK_PD3 RK_FUNC_GPIO &pcfg_pull_up>;
>>>>>>
>>