Re: [PATCH v2 1/4] arm64: dts: meson: g12a: x96-max: fix the Ethernet PHY reset line

[Neil Armstrong]

On 12/06/2019 22:55, Martin Blumenstingl wrote:
> The Odroid-N2 schematics show that the following pins are used for the
> reset and interrupt lines:
> - GPIOZ_14 is the PHY interrupt line
> - GPIOZ_15 is the PHY reset line
> 
> The GPIOZ_14 and GPIOZ_15 pins are special. The datasheet describes that
> they are "3.3V input tolerant open drain (OD) output pins". This means
> the GPIO controller can drive the output LOW to reset the PHY. To
> release the reset it can only switch the pin to input mode. The output
> cannot be driven HIGH for these pins.
> This requires configuring the reset line as GPIO_OPEN_DRAIN because
> otherwise the PHY will be stuck in "reset" state (because driving the
> pin HIGH seems to result in the same signal as driving it LOW).
> 
> The reset line works together with a pull-up resistor (R143 in the
> Odroid-N2 schematics). The SoC can drive GPIOZ_14 LOW to assert the PHY
> reset. However, since the SoC can't drive the pin HIGH (to release the
> reset) we switch the mode to INPUT and let the pull-up resistor take
> care of driving the reset line HIGH.
> 
> Switch to GPIOZ_15 for the PHY reset line instead of using GPIOZ_14
> (which actually is the interrupt line).
> Move from the "snps" specific resets to the MDIO framework's
> reset-gpios because only the latter honors the GPIO flags.
> Use the GPIO flags (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN) to match with
> the pull-up resistor because this will:
> - drive the output LOW to reset the PHY (= active low)
> - switch the pin to INPUT mode so the pull-up will take the PHY out of
>   reset
> 
> Fixes: 51d116557b2044 ("arm64: dts: meson-g12a-x96-max: Add Gigabit Ethernet Support")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
> ---
>  arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
> index 98bc56e650a0..de58d7817836 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
> @@ -176,6 +176,10 @@
>  		reg = <0>;
>  		max-speed = <1000>;
>  		eee-broken-1000t;
> +
> +		reset-assert-us = <10000>;
> +		reset-deassert-us = <30000>;
> +		reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>  	};
>  };
>  
> @@ -186,9 +190,6 @@
>  	phy-mode = "rgmii";
>  	phy-handle = <&external_phy>;
>  	amlogic,tx-delay-ns = <2>;
> -	snps,reset-gpio = <&gpio GPIOZ_14 0>;
> -	snps,reset-delays-us = <0 10000 1000000>;
> -	snps,reset-active-low;
>  };
>  
>  &pwm_ef {
> 

Thanks for spotting this:
Reviewed-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>