Re: [PATCH v2 2/4] ARM: dts: meson: switch to the generic Ethernet PHY reset bindings

From: Neil Armstrong <narmstrong@baylibre.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	linux-amlogic@lists.infradead.org, khilman@baylibre.com
Cc: andrew@lunn.ch, netdev@vger.kernel.org, linus.walleij@linaro.org,
	linux-kernel@vger.kernel.org, robin.murphy@arm.com,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/4] ARM: dts: meson: switch to the generic Ethernet PHY reset bindings
Date: Fri, 14 Jun 2019 11:02:49 +0200	[thread overview]
Message-ID: <cd35d212-ed30-d0f5-5da1-3b16fc814a28@baylibre.com> (raw)
In-Reply-To: <20190612205529.19834-3-martin.blumenstingl@googlemail.com>

On 12/06/2019 22:55, Martin Blumenstingl wrote:
> The snps,reset-gpio bindings are deprecated in favour of the generic
> "Ethernet PHY reset" bindings.
> 
> Replace snps,reset-gpio from the &ethmac node with reset-gpios in the
> ethernet-phy node. The old snps,reset-active-low property is now encoded
> directly as GPIO flag inside the reset-gpios property.
> 
> snps,reset-delays-us is converted to reset-assert-us and
> reset-deassert-us. reset-assert-us is the second cell from
> snps,reset-delays-us while reset-deassert-us was the third cell.
> Instead of blindly copying the old values (which seems strange since
> they gave the PHY one second to come out of reset) over this also
> updates the delays based on the datasheets:
> - RTL8211F PHY on the Odroid-C1 and MXIII-Plus needs a 10ms assert
>   delay (the datasheet mentions: "For a complete PHY reset, this pin
>   must be asserted low for at least 10ms") and a 30ms deassert delay
>   (the datasheet mentions: "Wait for a further 30ms (for internal
>   circuits settling time) before accessing the PHY register"). The
>   old settings used 10ms for assert and 1000ms for deassert.
> - IP101GR PHY on the EC-100 and MXQ needs a 10ms assert delay (the
>   datasheet mentions: "Trst | Reset period | 10ms") and a 10ms deassert
>   delay as well (the datasheet mentions: "Tclk_MII_rdy | MII/RMII clock
>   output ready after reset released | 10ms")). The old settings used
>   10ms for assert and 1000ms for deassert.
> 
> No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  arch/arm/boot/dts/meson8b-ec100.dts       | 9 +++++----
>  arch/arm/boot/dts/meson8b-mxq.dts         | 9 +++++----
>  arch/arm/boot/dts/meson8b-odroidc1.dts    | 9 +++++----
>  arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 8 ++++----
>  4 files changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/meson8b-ec100.dts b/arch/arm/boot/dts/meson8b-ec100.dts
> index 9bf4249cb60d..96d239d8334e 100644
> --- a/arch/arm/boot/dts/meson8b-ec100.dts
> +++ b/arch/arm/boot/dts/meson8b-ec100.dts
> @@ -234,10 +234,6 @@
>  	phy-handle = <&eth_phy0>;
>  	phy-mode = "rmii";
>  
> -	snps,reset-gpio = <&gpio GPIOH_4 0>;
> -	snps,reset-delays-us = <0 10000 1000000>;
> -	snps,reset-active-low;
> -
>  	mdio {
>  		compatible = "snps,dwmac-mdio";
>  		#address-cells = <1>;
> @@ -246,6 +242,11 @@
>  		eth_phy0: ethernet-phy@0 {
>  			/* IC Plus IP101A/G (0x02430c54) */
>  			reg = <0>;
> +
> +			reset-assert-us = <10000>;
> +			reset-deassert-us = <10000>;
> +			reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
> +
>  			icplus,select-interrupt;
>  			interrupt-parent = <&gpio_intc>;
>  			/* GPIOH_3 */
> diff --git a/arch/arm/boot/dts/meson8b-mxq.dts b/arch/arm/boot/dts/meson8b-mxq.dts
> index ef602ab45efd..bb27b34eb346 100644
> --- a/arch/arm/boot/dts/meson8b-mxq.dts
> +++ b/arch/arm/boot/dts/meson8b-mxq.dts
> @@ -91,10 +91,6 @@
>  	phy-handle = <&eth_phy0>;
>  	phy-mode = "rmii";
>  
> -	snps,reset-gpio = <&gpio GPIOH_4 0>;
> -	snps,reset-delays-us = <0 10000 1000000>;
> -	snps,reset-active-low;
> -
>  	mdio {
>  		compatible = "snps,dwmac-mdio";
>  		#address-cells = <1>;
> @@ -103,6 +99,11 @@
>  		eth_phy0: ethernet-phy@0 {
>  			/* IC Plus IP101A/G (0x02430c54) */
>  			reg = <0>;
> +
> +			reset-assert-us = <10000>;
> +			reset-deassert-us = <10000>;
> +			reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
> +
>  			icplus,select-interrupt;
>  			interrupt-parent = <&gpio_intc>;
>  			/* GPIOH_3 */
> diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts
> index 018695b2b83a..86c4614e0a38 100644
> --- a/arch/arm/boot/dts/meson8b-odroidc1.dts
> +++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
> @@ -176,10 +176,6 @@
>  &ethmac {
>  	status = "okay";
>  
> -	snps,reset-gpio = <&gpio GPIOH_4 GPIO_ACTIVE_HIGH>;
> -	snps,reset-active-low;
> -	snps,reset-delays-us = <0 10000 30000>;
> -
>  	pinctrl-0 = <&eth_rgmii_pins>;
>  	pinctrl-names = "default";
>  
> @@ -195,6 +191,11 @@
>  		/* Realtek RTL8211F (0x001cc916) */
>  		eth_phy: ethernet-phy@0 {
>  			reg = <0>;
> +
> +			reset-assert-us = <10000>;
> +			reset-deassert-us = <30000>;
> +			reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
> +
>  			interrupt-parent = <&gpio_intc>;
>  			/* GPIOH_3 */
>  			interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> diff --git a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
> index 59b07a55e461..d54477b1001c 100644
> --- a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
> +++ b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
> @@ -73,10 +73,6 @@
>  
>  	amlogic,tx-delay-ns = <4>;
>  
> -	snps,reset-gpio = <&gpio GPIOH_4 0>;
> -	snps,reset-delays-us = <0 10000 1000000>;
> -	snps,reset-active-low;
> -
>  	mdio {
>  		compatible = "snps,dwmac-mdio";
>  		#address-cells = <1>;
> @@ -85,6 +81,10 @@
>  		eth_phy0: ethernet-phy@0 {
>  			/* Realtek RTL8211F (0x001cc916) */
>  			reg = <0>;
> +
> +			reset-assert-us = <10000>;
> +			reset-deassert-us = <30000>;
> +			reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>  		};
>  	};
>  };
> 

Nice !

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>