linux-kernel - Re: [PATCH 2/6] arm64: dts: rockchip: move reset to dedicated eth-phy node on ringneck

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a23bbc22-6bfd-46e0-b114-fc14ee66e74a@cherry.de>
Date: Mon, 12 May 2025 20:30:12 +0200
From: Quentin Schulz <quentin.schulz@...rry.de>
To: Heiko Stuebner <heiko@...ech.de>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Heiko Stuebner <heiko.stuebner@...rry.de>
Subject: Re: [PATCH 2/6] arm64: dts: rockchip: move reset to dedicated eth-phy
 node on ringneck

Hi Heiko,

On 5/8/25 5:09 PM, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@...rry.de>
> 
> Using snps,reset-* properties to handle the ethernet-phy resets is
> deprecated and instead a real phy node should be used.
> 
> Move the Ringneck phy-reset properties to such a node
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@...rry.de>

Reviewed-by: Quentin Schulz <quentin.schulz@...rry.de>

PHY is still detected after a cold reset and a reboot, iperf3 works in 
both scenario, therefore:
Tested-by: Quentin Schulz <quentin.schulz@...rry.de>

We'll need to do some work in U-Boot to prepare for this change I 
believe, will try to figure things out ahead of merging the device tree 
in U-Boot :)

Some nitpicks below.

> ---
>   .../boot/dts/rockchip/px30-ringneck.dtsi      | 22 ++++++++++++++++---
>   1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> index 142244d52706..ab232e5c7ad6 100644
> --- a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> @@ -83,9 +83,7 @@ &emmc {
>   
>   /* On-module TI DP83825I PHY but no connector, enable in carrierboard */
>   &gmac {
> -	snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> -	snps,reset-active-low;
> -	snps,reset-delays-us = <0 50000 50000>;
> +	phy-handle = <&dp83825>;
>   	phy-supply = <&vcc_3v3>;
>   	clock_in_out = "output";
>   };
> @@ -344,6 +342,18 @@ &io_domains {
>   	status = "okay";
>   };
>   
> +&mdio {
> +	dp83825: ethernet-phy@0 {

Not sure we would ever need a label? We don't expose the MDIO bus on the 
Q7 connector, so it's either the internal PHY for Ethernet or no 
Ethernet (or USB Ethernet or whatnot), so if you want to disable 
Ethernet you only need to NOT have &gmac { status= "okay";} in the final 
DTS?

> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0x0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&phy_rst>;
> +		reset-assert-us = <50000>;
> +		reset-deassert-us = <50000>;
> +		reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> +	};
> +};
> +
>   &pinctrl {
>   	emmc {
>   		emmc_reset: emmc-reset {
> @@ -351,6 +361,12 @@ emmc_reset: emmc-reset {
>   		};
>   	};
>   
> +	ethernet {
> +		phy_rst: phy-rst {

Maybe have eth somewhere in the name, there are many PHYs from different 
controllers available :)

Thanks!
Quentin