Re: [PATCH v1 1/1] arm64: dts: freescale: imx9-var-dart-sonata: fix Ethernet PHY configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Removing the explicitly linked `reset-gpios` property from the PHY nodes without adding a corresponding `-supply` reference breaks `fw_devlink` dependency ordering, creating a probe race condition.
- [Medium] Replacing the PHY reset configuration with a fixed regulator reduces the post-enable delay from 100ms to 10ms, which may violate the PHY's initialization timing requirements.
--

commit aa29283e1adc330c15b9cab63314f0eec59e0b65
Author: Stefano Radaelli <stefano.r@variscite.com>
arm64: dts: freescale: imx9-var-dart-sonata: fix Ethernet PHY configuration

This commit corrects the Ethernet PHY description on DART-MX9 Sonata boards by
modeling the PHY reset line as a fixed regulator instead. It removes the
reset-gpios property from the PHY nodes and asserts the enable signal during
boot to ensure initialization completes successfully.

> diff --git a/arch/arm64/boot/dts/freescale/imx91-var-dart-sonata.dts b/arch/arm64/boot/dts/freescale/imx91-var-dart-sonata.dts
> index 3f0fd321d95f0..f7e32f9375f50 100644
> --- a/arch/arm64/boot/dts/freescale/imx91-var-dart-sonata.dts
> +++ b/arch/arm64/boot/dts/freescale/imx91-var-dart-sonata.dts
[ ... ]
> +	reg_phy1_supply: regulator-phy1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "SUPPLY_PHY1";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pca6408_2 0 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		startup-delay-us = <10000>;

[Severity: Medium]
Could reducing the post-enable delay from 100ms (originally in
reset-deassert-us) to 10ms violate the PHY's initialization timing
requirements?

> +		regulator-always-on;
> +	};
[ ... ]
> @@ -139,9 +150,6 @@ mdio {
>  		ethphy1: ethernet-phy@1 {
>  			compatible = "ethernet-phy-ieee802.3-c22";
>  			reg = <1>;

[Severity: High]
Since the explicitly linked reset-gpios property was removed below, shouldn't
there be a corresponding phy-supply = <&reg_phy1_supply>; link here to maintain
the device link dependency?

Without this link, fw_devlink no longer sees any dependency between the
Ethernet subsystem and the GPIO expander. Could the MAC driver probe and scan
the MDIO bus before the I2C GPIO expander (pca6408_2) and the new always-on
regulator are initialized, resulting in get_phy_device() failing with -ENODEV?

> -			reset-gpios = <&pca6408_2 0 GPIO_ACTIVE_LOW>;
> -			reset-assert-us = <15000>;
> -			reset-deassert-us = <100000>;
>  
>  			leds {
>  				#address-cells = <1>;
[ ... ]
> diff --git a/arch/arm64/boot/dts/freescale/imx93-var-dart-sonata.dts b/arch/arm64/boot/dts/freescale/imx93-var-dart-sonata.dts
> index 5513d3b148a2f..1ca3ec8330d07 100644
> --- a/arch/arm64/boot/dts/freescale/imx93-var-dart-sonata.dts
> +++ b/arch/arm64/boot/dts/freescale/imx93-var-dart-sonata.dts
[ ... ]
> +	reg_phy1_supply: regulator-phy1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "SUPPLY_PHY1";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pca6408_2 0 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		startup-delay-us = <10000>;

[Severity: Medium]
Does this same reduction to a 10ms startup delay also risk initialization
failure here?

> +		regulator-always-on;
> +	};
[ ... ]
> @@ -180,9 +191,6 @@ mdio {
>  		ethphy1: ethernet-phy@1 {
>  			compatible = "ethernet-phy-ieee802.3-c22";
>  			reg = <1>;

[Severity: High]
Does this node also need a phy-supply = <&reg_phy1_supply>; property to prevent
the probe race condition described above?

> -			reset-assert-us = <15000>;
> -			reset-deassert-us = <100000>;
> -			reset-gpios = <&pca6408_2 0 GPIO_ACTIVE_LOW>;
>  
>  			leds {
>  				#address-cells = <1>;
[ ... ]
> diff --git a/arch/arm64/boot/dts/freescale/imx95-var-dart-sonata.dts b/arch/arm64/boot/dts/freescale/imx95-var-dart-sonata.dts
> index 7a58ad38ffb46..ccf7e77016e48 100644
> --- a/arch/arm64/boot/dts/freescale/imx95-var-dart-sonata.dts
> +++ b/arch/arm64/boot/dts/freescale/imx95-var-dart-sonata.dts
[ ... ]
> @@ -346,9 +346,6 @@ &netc_emdio {
>  	ethphy1: ethernet-phy@1 {
>  		compatible = "ethernet-phy-ieee802.3-c22";
>  		reg = <1>;

[Severity: High]
Does this PHY node also require a phy-supply property to maintain its
dependency ordering after the reset-gpios removal?

> -		reset-gpios = <&pca6408_2 0 GPIO_ACTIVE_LOW>;
> -		reset-assert-us = <10000>;
> -		reset-deassert-us = <100000>;
>  
>  		leds {
>  			#address-cells = <1>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703152731.236793-1-stefano.r@variscite.com?part=1