Re: [PATCH v1 1/4] arm64: dts: ti: var-som-am62p: fix Ethernet PHY configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Replacing PHY `reset-gpios` with an unhandled `vdd-supply` regulator without `regulator-always-on` will cause the kernel to erroneously reset the PHY, breaking Ethernet connectivity.
- [Medium] TI-specific PHY properties are erroneously applied to an Analog Devices PHY.
--

commit 3f60b20a54eb8dc897529e8f7d0ac14da1c7808f
Author: Stefano Radaelli <stefano.radaelli21@gmail.com>

arm64: dts: ti: var-som-am62p: fix Ethernet PHY configuration

Enable the CPSW Ethernet controller and correct the Ethernet PHY
description by modelling the PHY power supply and adding the required
board-specific PHY properties.

> diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-var-som.dtsi b/arch/arm64/boot/dts/ti/k3-am62p5-var-som.dtsi
> index fc5a3942cde00..be19cbaffc4b0 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62p5-var-som.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62p5-var-som.dtsi
> @@ -113,6 +113,15 @@ reg_3v3_phy: regulator-3v3-phy {
>  		regulator-always-on;
>  	};
>  
> +	reg_eth_phy_vdd: regulator-eth-vdd {
> +		compatible = "regulator-fixed";
> +		regulator-name = "reg_eth_phy_vdd";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&main_gpio0 46 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +

[Severity: High]
Does this code cause the PHY to be erroneously reset during late init, breaking
Ethernet connectivity?

Since the kernel PHY drivers do not handle the vdd-supply property, this
regulator will have no consumers. Without regulator-always-on, it will be
disabled by regulator_init_complete() during late init.

Disabling this regulator drives the active-high GPIO (main_gpio0 46) low,
which will assert the PHY's physical active-low reset pin.

Additionally, the required 100ms hardware initialization delay is lost since
this new regulator lacks a corresponding startup-delay-us property.

[ ... ]

> @@ -159,9 +169,10 @@ &cpsw3g_mdio {
>  	cpsw3g_phy0: ethernet-phy@4 {
>  		compatible = "ethernet-phy-id0283.bc30";
>  		reg = <4>;
> -		reset-gpios = <&main_gpio0 46 GPIO_ACTIVE_LOW>;
> -		reset-assert-us = <10000>;
> -		reset-deassert-us = <100000>;
> +		bootph-all;
> +		enet-phy-lane-no-swap;
> +		vdd-supply = <&reg_eth_phy_vdd>;
> +		ti,min-output-impedance;
>  	};
>  };

[Severity: Medium]
Are these TI-specific properties (ti,min-output-impedance and
enet-phy-lane-no-swap) correct for an Analog Devices ADIN1300 PHY
(ethernet-phy-id0283.bc30)?

If the hardware is actually an ADIN1300, these TI properties are ignored by
the adin.c driver. If it is a TI PHY, the kernel will bind the wrong driver
due to the incorrect compatible string.

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