diff mbox series

[v3] ARM: dts: imx: Fix the AR803X phy-mode

Message ID 20190403221241.4753-1-festevam@gmail.com (mailing list archive)
State Mainlined, archived
Commit 0672d22a19244cdb0e5c753125c1a55a120db5d0
Headers show
Series [v3] ARM: dts: imx: Fix the AR803X phy-mode | expand

Commit Message

Fabio Estevam April 3, 2019, 10:12 p.m. UTC
Commit 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
exposed an issue on imx DTS files using AR8031/AR8035 PHYs.

The end result is that the boards can no longer obtain an IP address
via UDHCP, for example.

Quoting Andrew Lunn:

"The problem here is, all the DTs were broken since day 0. However,
because the PHY driver was also broken, nobody noticed and it
worked. Now that the PHY driver has been fixed, all the bugs in the
DTs now become an issue"

To fix this problem, the phy-mode property needs to be "rgmii-id",  which
has the following meaning as per 
Documentation/devicetree/bindings/net/ethernet.txt:

"RGMII with internal RX and TX delays provided by the PHY, the MAC should
not add the RX or TX delays in this case)"

Tested on imx6-sabresd, imx6sx-sdb and imx7d-pico boards with
successfully restored networking.

Based on the initial submission from Steve Twiss for the
imx6qdl-sabresd.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
Tested-by: Baruch Siach <baruch@tkos.co.il>
Tested-by: Soeren Moch <smoch@web.de>
Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
Tested-by: Adam Thomson <Adam.Thomson@diasemi.com>
Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Changes since v2:
- Also fixed imx6q-ba16
- Removed stable tag as it does not apply cleanly on older
stable trees. I can manually generate versions for stable
trees after this one hits mainline.

 arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi | 2 +-
 arch/arm/boot/dts/imx6dl-riotboard.dts        | 2 +-
 arch/arm/boot/dts/imx6q-ba16.dtsi             | 2 +-
 arch/arm/boot/dts/imx6q-marsboard.dts         | 2 +-
 arch/arm/boot/dts/imx6q-tbs2910.dts           | 2 +-
 arch/arm/boot/dts/imx6qdl-apf6.dtsi           | 2 +-
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi      | 2 +-
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi        | 2 +-
 arch/arm/boot/dts/imx6qdl-sr-som.dtsi         | 2 +-
 arch/arm/boot/dts/imx6qdl-wandboard.dtsi      | 2 +-
 arch/arm/boot/dts/imx6sx-sabreauto.dts        | 2 +-
 arch/arm/boot/dts/imx6sx-sdb.dtsi             | 2 +-
 arch/arm/boot/dts/imx7d-pico.dtsi             | 2 +-
 13 files changed, 13 insertions(+), 13 deletions(-)

Comments

Shawn Guo April 11, 2019, 2:58 a.m. UTC | #1
On Wed, Apr 03, 2019 at 07:12:41PM -0300, Fabio Estevam wrote:
> Commit 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
> exposed an issue on imx DTS files using AR8031/AR8035 PHYs.
> 
> The end result is that the boards can no longer obtain an IP address
> via UDHCP, for example.
> 
> Quoting Andrew Lunn:
> 
> "The problem here is, all the DTs were broken since day 0. However,
> because the PHY driver was also broken, nobody noticed and it
> worked. Now that the PHY driver has been fixed, all the bugs in the
> DTs now become an issue"
> 
> To fix this problem, the phy-mode property needs to be "rgmii-id",  which
> has the following meaning as per 
> Documentation/devicetree/bindings/net/ethernet.txt:
> 
> "RGMII with internal RX and TX delays provided by the PHY, the MAC should
> not add the RX or TX delays in this case)"
> 
> Tested on imx6-sabresd, imx6sx-sdb and imx7d-pico boards with
> successfully restored networking.
> 
> Based on the initial submission from Steve Twiss for the
> imx6qdl-sabresd.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> Tested-by: Baruch Siach <baruch@tkos.co.il>
> Tested-by: Soeren Moch <smoch@web.de>
> Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
> Tested-by: Adam Thomson <Adam.Thomson@diasemi.com>
> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

Applied, thanks.
George G. Davis May 13, 2019, 5:18 p.m. UTC | #2
Hello,

On Wed, Apr 03, 2019 at 07:12:41PM -0300, Fabio Estevam wrote:
> Commit 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
> exposed an issue on imx DTS files using AR8031/AR8035 PHYs.
> 
> The end result is that the boards can no longer obtain an IP address
> via UDHCP, for example.
> 
> Quoting Andrew Lunn:
> 
> "The problem here is, all the DTs were broken since day 0. However,
> because the PHY driver was also broken, nobody noticed and it
> worked. Now that the PHY driver has been fixed, all the bugs in the
> DTs now become an issue"
> 
> To fix this problem, the phy-mode property needs to be "rgmii-id",  which
> has the following meaning as per 
> Documentation/devicetree/bindings/net/ethernet.txt:
> 
> "RGMII with internal RX and TX delays provided by the PHY, the MAC should
> not add the RX or TX delays in this case)"
> 
> Tested on imx6-sabresd, imx6sx-sdb and imx7d-pico boards with
> successfully restored networking.
> 
> Based on the initial submission from Steve Twiss for the
> imx6qdl-sabresd.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> Tested-by: Baruch Siach <baruch@tkos.co.il>
> Tested-by: Soeren Moch <smoch@web.de>
> Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
> Tested-by: Adam Thomson <Adam.Thomson@diasemi.com>
> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> Changes since v2:
> - Also fixed imx6q-ba16
> - Removed stable tag as it does not apply cleanly on older
> stable trees. I can manually generate versions for stable
> trees after this one hits mainline.

Please add this commit to the v5.1.x stable queue to resolve NFS root breakage
in v5.1. I can confirm that it applies cleanly to v5.1.1 and resolves NFS root
breakage that occurs on i.MX6 boards in v5.1.x, tested on imx6q-sabreauto.dts
and imx6q-sabresd.dts. Although the fix should be backported to pre-v5.1.x
stable series as well, it does not cause problems for pre-v5.1 but results in
NFS root breakage for v5.1.x.

TIA!

> 
>  arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi | 2 +-
>  arch/arm/boot/dts/imx6dl-riotboard.dts        | 2 +-
>  arch/arm/boot/dts/imx6q-ba16.dtsi             | 2 +-
>  arch/arm/boot/dts/imx6q-marsboard.dts         | 2 +-
>  arch/arm/boot/dts/imx6q-tbs2910.dts           | 2 +-
>  arch/arm/boot/dts/imx6qdl-apf6.dtsi           | 2 +-
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi      | 2 +-
>  arch/arm/boot/dts/imx6qdl-sabresd.dtsi        | 2 +-
>  arch/arm/boot/dts/imx6qdl-sr-som.dtsi         | 2 +-
>  arch/arm/boot/dts/imx6qdl-wandboard.dtsi      | 2 +-
>  arch/arm/boot/dts/imx6sx-sabreauto.dts        | 2 +-
>  arch/arm/boot/dts/imx6sx-sdb.dtsi             | 2 +-
>  arch/arm/boot/dts/imx7d-pico.dtsi             | 2 +-
>  13 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi b/arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi
> index fb01fa6e4224..3cae139e6396 100644
> --- a/arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi
> +++ b/arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi
> @@ -216,7 +216,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-reset-duration = <10>;
>  	phy-reset-gpios = <&gpio1 24 GPIO_ACTIVE_LOW>;
>  	phy-supply = <&reg_enet>;
> diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts b/arch/arm/boot/dts/imx6dl-riotboard.dts
> index 65c184bb8fb0..d9de49efa802 100644
> --- a/arch/arm/boot/dts/imx6dl-riotboard.dts
> +++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
> @@ -92,7 +92,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-reset-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>;
>  	interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
>  			      <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
> diff --git a/arch/arm/boot/dts/imx6q-ba16.dtsi b/arch/arm/boot/dts/imx6q-ba16.dtsi
> index adc9455e42c7..37c63402157b 100644
> --- a/arch/arm/boot/dts/imx6q-ba16.dtsi
> +++ b/arch/arm/boot/dts/imx6q-ba16.dtsi
> @@ -171,7 +171,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	status = "okay";
>  };
>  
> diff --git a/arch/arm/boot/dts/imx6q-marsboard.dts b/arch/arm/boot/dts/imx6q-marsboard.dts
> index d8ccb533b6b7..84b30bd6908f 100644
> --- a/arch/arm/boot/dts/imx6q-marsboard.dts
> +++ b/arch/arm/boot/dts/imx6q-marsboard.dts
> @@ -110,7 +110,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-reset-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>;
>  	status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/imx6q-tbs2910.dts b/arch/arm/boot/dts/imx6q-tbs2910.dts
> index 2ce8399a10ba..bfff87ce2e1f 100644
> --- a/arch/arm/boot/dts/imx6q-tbs2910.dts
> +++ b/arch/arm/boot/dts/imx6q-tbs2910.dts
> @@ -98,7 +98,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
>  	status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/imx6qdl-apf6.dtsi b/arch/arm/boot/dts/imx6qdl-apf6.dtsi
> index 1ebf29f43a24..4738c3c1ab50 100644
> --- a/arch/arm/boot/dts/imx6qdl-apf6.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-apf6.dtsi
> @@ -51,7 +51,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-reset-duration = <10>;
>  	phy-reset-gpios = <&gpio1 24 GPIO_ACTIVE_LOW>;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> index 1280de50a984..f3404dd10537 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> @@ -292,7 +292,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
>  			      <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
>  	fsl,err006687-workaround-present;
> diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> index a0705066ccba..185fb17a3500 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> @@ -202,7 +202,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
>  	status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/imx6qdl-sr-som.dtsi b/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
> index 4ccb7afc4b35..6d7f6b9035bc 100644
> --- a/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
> @@ -53,7 +53,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_microsom_enet_ar8035>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-reset-duration = <2>;
>  	phy-reset-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> index b7d5fb421404..50d9a989e06a 100644
> --- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> @@ -224,7 +224,7 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-reset-gpios = <&gpio3 29 GPIO_ACTIVE_LOW>;
>  	interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
>  			      <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
> diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> index b0ee324afe58..315044ccd65f 100644
> --- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
> +++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> @@ -75,7 +75,7 @@
>  &fec1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet1>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-handle = <&ethphy1>;
>  	fsl,magic-packet;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/imx6sx-sdb.dtsi b/arch/arm/boot/dts/imx6sx-sdb.dtsi
> index 08ede56c3f10..f6972deb5e39 100644
> --- a/arch/arm/boot/dts/imx6sx-sdb.dtsi
> +++ b/arch/arm/boot/dts/imx6sx-sdb.dtsi
> @@ -191,7 +191,7 @@
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet1>;
>  	phy-supply = <&reg_enet_3v3>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-handle = <&ethphy1>;
>  	phy-reset-gpios = <&gpio2 7 GPIO_ACTIVE_LOW>;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/imx7d-pico.dtsi b/arch/arm/boot/dts/imx7d-pico.dtsi
> index 3fd595a71202..6f50ebf31a0a 100644
> --- a/arch/arm/boot/dts/imx7d-pico.dtsi
> +++ b/arch/arm/boot/dts/imx7d-pico.dtsi
> @@ -92,7 +92,7 @@
>  			  <&clks IMX7D_ENET1_TIME_ROOT_CLK>;
>  	assigned-clock-parents = <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>;
>  	assigned-clock-rates = <0>, <100000000>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-handle = <&ethphy0>;
>  	fsl,magic-packet;
>  	phy-reset-gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
Sasha Levin May 14, 2019, 12:45 a.m. UTC | #3
On Mon, May 13, 2019 at 01:18:27PM -0400, George G. Davis wrote:
>Hello,
>
>On Wed, Apr 03, 2019 at 07:12:41PM -0300, Fabio Estevam wrote:
>> Commit 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
>> exposed an issue on imx DTS files using AR8031/AR8035 PHYs.
>>
>> The end result is that the boards can no longer obtain an IP address
>> via UDHCP, for example.
>>
>> Quoting Andrew Lunn:
>>
>> "The problem here is, all the DTs were broken since day 0. However,
>> because the PHY driver was also broken, nobody noticed and it
>> worked. Now that the PHY driver has been fixed, all the bugs in the
>> DTs now become an issue"
>>
>> To fix this problem, the phy-mode property needs to be "rgmii-id",  which
>> has the following meaning as per
>> Documentation/devicetree/bindings/net/ethernet.txt:
>>
>> "RGMII with internal RX and TX delays provided by the PHY, the MAC should
>> not add the RX or TX delays in this case)"
>>
>> Tested on imx6-sabresd, imx6sx-sdb and imx7d-pico boards with
>> successfully restored networking.
>>
>> Based on the initial submission from Steve Twiss for the
>> imx6qdl-sabresd.
>>
>> Signed-off-by: Fabio Estevam <festevam@gmail.com>
>> Tested-by: Baruch Siach <baruch@tkos.co.il>
>> Tested-by: Soeren Moch <smoch@web.de>
>> Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
>> Tested-by: Adam Thomson <Adam.Thomson@diasemi.com>
>> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
>> Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>> Changes since v2:
>> - Also fixed imx6q-ba16
>> - Removed stable tag as it does not apply cleanly on older
>> stable trees. I can manually generate versions for stable
>> trees after this one hits mainline.
>
>Please add this commit to the v5.1.x stable queue to resolve NFS root breakage
>in v5.1. I can confirm that it applies cleanly to v5.1.1 and resolves NFS root
>breakage that occurs on i.MX6 boards in v5.1.x, tested on imx6q-sabreauto.dts
>and imx6q-sabresd.dts. Although the fix should be backported to pre-v5.1.x
>stable series as well, it does not cause problems for pre-v5.1 but results in
>NFS root breakage for v5.1.x.

What's the commit id in Linus's tree? I don't see it.

--
Thanks,
Sasha
Florian Fainelli May 14, 2019, 1:06 a.m. UTC | #4
Le 5/13/19 à 5:45 PM, Sasha Levin a écrit :
> On Mon, May 13, 2019 at 01:18:27PM -0400, George G. Davis wrote:
>> Hello,
>>
>> On Wed, Apr 03, 2019 at 07:12:41PM -0300, Fabio Estevam wrote:
>>> Commit 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII
>>> mode")
>>> exposed an issue on imx DTS files using AR8031/AR8035 PHYs.
>>>
>>> The end result is that the boards can no longer obtain an IP address
>>> via UDHCP, for example.
>>>
>>> Quoting Andrew Lunn:
>>>
>>> "The problem here is, all the DTs were broken since day 0. However,
>>> because the PHY driver was also broken, nobody noticed and it
>>> worked. Now that the PHY driver has been fixed, all the bugs in the
>>> DTs now become an issue"
>>>
>>> To fix this problem, the phy-mode property needs to be "rgmii-id", 
>>> which
>>> has the following meaning as per
>>> Documentation/devicetree/bindings/net/ethernet.txt:
>>>
>>> "RGMII with internal RX and TX delays provided by the PHY, the MAC
>>> should
>>> not add the RX or TX delays in this case)"
>>>
>>> Tested on imx6-sabresd, imx6sx-sdb and imx7d-pico boards with
>>> successfully restored networking.
>>>
>>> Based on the initial submission from Steve Twiss for the
>>> imx6qdl-sabresd.
>>>
>>> Signed-off-by: Fabio Estevam <festevam@gmail.com>
>>> Tested-by: Baruch Siach <baruch@tkos.co.il>
>>> Tested-by: Soeren Moch <smoch@web.de>
>>> Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
>>> Tested-by: Adam Thomson <Adam.Thomson@diasemi.com>
>>> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
>>> Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>> ---
>>> Changes since v2:
>>> - Also fixed imx6q-ba16
>>> - Removed stable tag as it does not apply cleanly on older
>>> stable trees. I can manually generate versions for stable
>>> trees after this one hits mainline.
>>
>> Please add this commit to the v5.1.x stable queue to resolve NFS root
>> breakage
>> in v5.1. I can confirm that it applies cleanly to v5.1.1 and resolves
>> NFS root
>> breakage that occurs on i.MX6 boards in v5.1.x, tested on
>> imx6q-sabreauto.dts
>> and imx6q-sabresd.dts. Although the fix should be backported to
>> pre-v5.1.x
>> stable series as well, it does not cause problems for pre-v5.1 but
>> results in
>> NFS root breakage for v5.1.x.
> 
> What's the commit id in Linus's tree? I don't see it.

That would be 6d4cd041f0af5b4c8fc742b4a68eac22e420e28c ("net: phy:
at803x: disable delay only for RGMII mode").

I would use that as a Fixes: tag BTW.
George G. Davis May 14, 2019, 1:16 a.m. UTC | #5
On Mon, May 13, 2019 at 08:45:39PM -0400, Sasha Levin wrote:
> On Mon, May 13, 2019 at 01:18:27PM -0400, George G. Davis wrote:
> >Hello,
> >
> >On Wed, Apr 03, 2019 at 07:12:41PM -0300, Fabio Estevam wrote:
> >>Commit 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
> >>exposed an issue on imx DTS files using AR8031/AR8035 PHYs.
> >>
> >>The end result is that the boards can no longer obtain an IP address
> >>via UDHCP, for example.
> >>
> >>Quoting Andrew Lunn:
> >>
> >>"The problem here is, all the DTs were broken since day 0. However,
> >>because the PHY driver was also broken, nobody noticed and it
> >>worked. Now that the PHY driver has been fixed, all the bugs in the
> >>DTs now become an issue"
> >>
> >>To fix this problem, the phy-mode property needs to be "rgmii-id",  which
> >>has the following meaning as per
> >>Documentation/devicetree/bindings/net/ethernet.txt:
> >>
> >>"RGMII with internal RX and TX delays provided by the PHY, the MAC should
> >>not add the RX or TX delays in this case)"
> >>
> >>Tested on imx6-sabresd, imx6sx-sdb and imx7d-pico boards with
> >>successfully restored networking.
> >>
> >>Based on the initial submission from Steve Twiss for the
> >>imx6qdl-sabresd.
> >>
> >>Signed-off-by: Fabio Estevam <festevam@gmail.com>
> >>Tested-by: Baruch Siach <baruch@tkos.co.il>
> >>Tested-by: Soeren Moch <smoch@web.de>
> >>Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
> >>Tested-by: Adam Thomson <Adam.Thomson@diasemi.com>
> >>Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> >>Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >>Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >>---
> >>Changes since v2:
> >>- Also fixed imx6q-ba16
> >>- Removed stable tag as it does not apply cleanly on older
> >>stable trees. I can manually generate versions for stable
> >>trees after this one hits mainline.
> >
> >Please add this commit to the v5.1.x stable queue to resolve NFS root breakage
> >in v5.1. I can confirm that it applies cleanly to v5.1.1 and resolves NFS root
> >breakage that occurs on i.MX6 boards in v5.1.x, tested on imx6q-sabreauto.dts
> >and imx6q-sabresd.dts. Although the fix should be backported to pre-v5.1.x
> >stable series as well, it does not cause problems for pre-v5.1 but results in
> >NFS root breakage for v5.1.x.
> 
> What's the commit id in Linus's tree? I don't see it.

Er, um, sorry, it was deferred to linux-next commit 68e9c97161b3 ("serial:
sh-sci: disable DMA for uart_console"), which quite frankly rather annoyed me
personally since linux-next commit 68e9c97161b3 is required to fix a regression
caused by v5.1 commit 6d4cd041f0af. In my opinion, linux-next commit
68e9c97161b3 really should have been pushed to the v5.1 release, earlier, due
to the noted regression, but I'm happy to wait for linux-next commit
68e9c97161b3 to make it to Linus's tree before propagating it to linux-stable.
Meanwhile, I wanted to let you know that v5.1.x is rather broken, in this
regard, on i.MX6 as-is for now.

> 
> --
> Thanks,
> Sasha
Sasha Levin May 14, 2019, 2:07 a.m. UTC | #6
On Mon, May 13, 2019 at 09:16:07PM -0400, George G. Davis wrote:
>On Mon, May 13, 2019 at 08:45:39PM -0400, Sasha Levin wrote:
>> On Mon, May 13, 2019 at 01:18:27PM -0400, George G. Davis wrote:
>> >Hello,
>> >
>> >On Wed, Apr 03, 2019 at 07:12:41PM -0300, Fabio Estevam wrote:
>> >>Commit 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
>> >>exposed an issue on imx DTS files using AR8031/AR8035 PHYs.
>> >>
>> >>The end result is that the boards can no longer obtain an IP address
>> >>via UDHCP, for example.
>> >>
>> >>Quoting Andrew Lunn:
>> >>
>> >>"The problem here is, all the DTs were broken since day 0. However,
>> >>because the PHY driver was also broken, nobody noticed and it
>> >>worked. Now that the PHY driver has been fixed, all the bugs in the
>> >>DTs now become an issue"
>> >>
>> >>To fix this problem, the phy-mode property needs to be "rgmii-id",  which
>> >>has the following meaning as per
>> >>Documentation/devicetree/bindings/net/ethernet.txt:
>> >>
>> >>"RGMII with internal RX and TX delays provided by the PHY, the MAC should
>> >>not add the RX or TX delays in this case)"
>> >>
>> >>Tested on imx6-sabresd, imx6sx-sdb and imx7d-pico boards with
>> >>successfully restored networking.
>> >>
>> >>Based on the initial submission from Steve Twiss for the
>> >>imx6qdl-sabresd.
>> >>
>> >>Signed-off-by: Fabio Estevam <festevam@gmail.com>
>> >>Tested-by: Baruch Siach <baruch@tkos.co.il>
>> >>Tested-by: Soeren Moch <smoch@web.de>
>> >>Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
>> >>Tested-by: Adam Thomson <Adam.Thomson@diasemi.com>
>> >>Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
>> >>Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> >>Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> >>---
>> >>Changes since v2:
>> >>- Also fixed imx6q-ba16
>> >>- Removed stable tag as it does not apply cleanly on older
>> >>stable trees. I can manually generate versions for stable
>> >>trees after this one hits mainline.
>> >
>> >Please add this commit to the v5.1.x stable queue to resolve NFS root breakage
>> >in v5.1. I can confirm that it applies cleanly to v5.1.1 and resolves NFS root
>> >breakage that occurs on i.MX6 boards in v5.1.x, tested on imx6q-sabreauto.dts
>> >and imx6q-sabresd.dts. Although the fix should be backported to pre-v5.1.x
>> >stable series as well, it does not cause problems for pre-v5.1 but results in
>> >NFS root breakage for v5.1.x.
>>
>> What's the commit id in Linus's tree? I don't see it.
>
>Er, um, sorry, it was deferred to linux-next commit 68e9c97161b3 ("serial:
>sh-sci: disable DMA for uart_console"), which quite frankly rather annoyed me
>personally since linux-next commit 68e9c97161b3 is required to fix a regression
>caused by v5.1 commit 6d4cd041f0af. In my opinion, linux-next commit
>68e9c97161b3 really should have been pushed to the v5.1 release, earlier, due
>to the noted regression, but I'm happy to wait for linux-next commit
>68e9c97161b3 to make it to Linus's tree before propagating it to linux-stable.

We'll happily take it once it makes it into Linus's tree and into a
release.

>Meanwhile, I wanted to let you know that v5.1.x is rather broken, in this
>regard, on i.MX6 as-is for now.

This would be something we can't do much about, but given it's an
important fix it should make it into Linus's tree very soon, right?

--
Sasha
George G. Davis May 14, 2019, 3:22 p.m. UTC | #7
Hello Sasha,

On Mon, May 13, 2019 at 10:07:42PM -0400, Sasha Levin wrote:
> On Mon, May 13, 2019 at 09:16:07PM -0400, George G. Davis wrote:
> >On Mon, May 13, 2019 at 08:45:39PM -0400, Sasha Levin wrote:
> >>On Mon, May 13, 2019 at 01:18:27PM -0400, George G. Davis wrote:
> >>>Hello,
> >>>
> >>>On Wed, Apr 03, 2019 at 07:12:41PM -0300, Fabio Estevam wrote:
> >>>>Commit 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
> >>>>exposed an issue on imx DTS files using AR8031/AR8035 PHYs.
> >>>>
> >>>>The end result is that the boards can no longer obtain an IP address
> >>>>via UDHCP, for example.
> >>>>
> >>>>Quoting Andrew Lunn:
> >>>>
> >>>>"The problem here is, all the DTs were broken since day 0. However,
> >>>>because the PHY driver was also broken, nobody noticed and it
> >>>>worked. Now that the PHY driver has been fixed, all the bugs in the
> >>>>DTs now become an issue"
> >>>>
> >>>>To fix this problem, the phy-mode property needs to be "rgmii-id",  which
> >>>>has the following meaning as per
> >>>>Documentation/devicetree/bindings/net/ethernet.txt:
> >>>>
> >>>>"RGMII with internal RX and TX delays provided by the PHY, the MAC should
> >>>>not add the RX or TX delays in this case)"
> >>>>
> >>>>Tested on imx6-sabresd, imx6sx-sdb and imx7d-pico boards with
> >>>>successfully restored networking.
> >>>>
> >>>>Based on the initial submission from Steve Twiss for the
> >>>>imx6qdl-sabresd.
> >>>>
> >>>>Signed-off-by: Fabio Estevam <festevam@gmail.com>
> >>>>Tested-by: Baruch Siach <baruch@tkos.co.il>
> >>>>Tested-by: Soeren Moch <smoch@web.de>
> >>>>Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
> >>>>Tested-by: Adam Thomson <Adam.Thomson@diasemi.com>
> >>>>Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> >>>>Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >>>>Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >>>>---
> >>>>Changes since v2:
> >>>>- Also fixed imx6q-ba16
> >>>>- Removed stable tag as it does not apply cleanly on older
> >>>>stable trees. I can manually generate versions for stable
> >>>>trees after this one hits mainline.
> >>>
> >>>Please add this commit to the v5.1.x stable queue to resolve NFS root breakage
> >>>in v5.1. I can confirm that it applies cleanly to v5.1.1 and resolves NFS root
> >>>breakage that occurs on i.MX6 boards in v5.1.x, tested on imx6q-sabreauto.dts
> >>>and imx6q-sabresd.dts. Although the fix should be backported to pre-v5.1.x
> >>>stable series as well, it does not cause problems for pre-v5.1 but results in
> >>>NFS root breakage for v5.1.x.
> >>
> >>What's the commit id in Linus's tree? I don't see it.
> >
> >Er, um, sorry, it was deferred to linux-next commit 68e9c97161b3 ("serial:
> >sh-sci: disable DMA for uart_console"), which quite frankly rather annoyed me
> >personally since linux-next commit 68e9c97161b3 is required to fix a regression
> >caused by v5.1 commit 6d4cd041f0af. In my opinion, linux-next commit
> >68e9c97161b3 really should have been pushed to the v5.1 release, earlier, due
> >to the noted regression, but I'm happy to wait for linux-next commit
> >68e9c97161b3 to make it to Linus's tree before propagating it to linux-stable.
> 
> We'll happily take it once it makes it into Linus's tree and into a
> release.
> 
> >Meanwhile, I wanted to let you know that v5.1.x is rather broken, in this
> >regard, on i.MX6 as-is for now.
> 
> This would be something we can't do much about, but given it's an
> important fix it should make it into Linus's tree very soon, right?

Yes, it will hopefully reach Linus's tree soon. Apologies for my impatience. :)

> 
> --
> Sasha
George G. Davis May 17, 2019, 10:25 p.m. UTC | #8
Hello Sasha,

On Tue, May 14, 2019 at 11:22:40AM -0400, George G. Davis wrote:
> On Mon, May 13, 2019 at 10:07:42PM -0400, Sasha Levin wrote:
> > On Mon, May 13, 2019 at 09:16:07PM -0400, George G. Davis wrote:
> > >On Mon, May 13, 2019 at 08:45:39PM -0400, Sasha Levin wrote:
> > >>On Mon, May 13, 2019 at 01:18:27PM -0400, George G. Davis wrote:
> > >>>On Wed, Apr 03, 2019 at 07:12:41PM -0300, Fabio Estevam wrote:
> > >>What's the commit id in Linus's tree? I don't see it.

Finally:

commit 0672d22a19244cdb0e5c753125c1a55a120db5d0 upstream.

> > We'll happily take it once it makes it into Linus's tree and into a
> > release.
> > 
> > >Meanwhile, I wanted to let you know that v5.1.x is rather broken, in this
> > >regard, on i.MX6 as-is for now.
> > 
> > This would be something we can't do much about, but given it's an
> > important fix it should make it into Linus's tree very soon, right?
> 
> Yes, it will hopefully reach Linus's tree soon. Apologies for my impatience. :)

Again, apologies for my impatience!

TIA!
Greg KH May 18, 2019, 6:38 a.m. UTC | #9
On Fri, May 17, 2019 at 06:25:04PM -0400, George G. Davis wrote:
> Hello Sasha,
> 
> On Tue, May 14, 2019 at 11:22:40AM -0400, George G. Davis wrote:
> > On Mon, May 13, 2019 at 10:07:42PM -0400, Sasha Levin wrote:
> > > On Mon, May 13, 2019 at 09:16:07PM -0400, George G. Davis wrote:
> > > >On Mon, May 13, 2019 at 08:45:39PM -0400, Sasha Levin wrote:
> > > >>On Mon, May 13, 2019 at 01:18:27PM -0400, George G. Davis wrote:
> > > >>>On Wed, Apr 03, 2019 at 07:12:41PM -0300, Fabio Estevam wrote:
> > > >>What's the commit id in Linus's tree? I don't see it.
> 
> Finally:
> 
> commit 0672d22a19244cdb0e5c753125c1a55a120db5d0 upstream.

Now queued up, thanks.

greg k-h
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi b/arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi
index fb01fa6e4224..3cae139e6396 100644
--- a/arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi
+++ b/arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi
@@ -216,7 +216,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-reset-duration = <10>;
 	phy-reset-gpios = <&gpio1 24 GPIO_ACTIVE_LOW>;
 	phy-supply = <&reg_enet>;
diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts b/arch/arm/boot/dts/imx6dl-riotboard.dts
index 65c184bb8fb0..d9de49efa802 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -92,7 +92,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-reset-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>;
 	interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
 			      <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/imx6q-ba16.dtsi b/arch/arm/boot/dts/imx6q-ba16.dtsi
index adc9455e42c7..37c63402157b 100644
--- a/arch/arm/boot/dts/imx6q-ba16.dtsi
+++ b/arch/arm/boot/dts/imx6q-ba16.dtsi
@@ -171,7 +171,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6q-marsboard.dts b/arch/arm/boot/dts/imx6q-marsboard.dts
index d8ccb533b6b7..84b30bd6908f 100644
--- a/arch/arm/boot/dts/imx6q-marsboard.dts
+++ b/arch/arm/boot/dts/imx6q-marsboard.dts
@@ -110,7 +110,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-reset-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx6q-tbs2910.dts b/arch/arm/boot/dts/imx6q-tbs2910.dts
index 2ce8399a10ba..bfff87ce2e1f 100644
--- a/arch/arm/boot/dts/imx6q-tbs2910.dts
+++ b/arch/arm/boot/dts/imx6q-tbs2910.dts
@@ -98,7 +98,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx6qdl-apf6.dtsi b/arch/arm/boot/dts/imx6qdl-apf6.dtsi
index 1ebf29f43a24..4738c3c1ab50 100644
--- a/arch/arm/boot/dts/imx6qdl-apf6.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apf6.dtsi
@@ -51,7 +51,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-reset-duration = <10>;
 	phy-reset-gpios = <&gpio1 24 GPIO_ACTIVE_LOW>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 1280de50a984..f3404dd10537 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -292,7 +292,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
 			      <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
 	fsl,err006687-workaround-present;
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index a0705066ccba..185fb17a3500 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -202,7 +202,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx6qdl-sr-som.dtsi b/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
index 4ccb7afc4b35..6d7f6b9035bc 100644
--- a/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
@@ -53,7 +53,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_microsom_enet_ar8035>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-reset-duration = <2>;
 	phy-reset-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
index b7d5fb421404..50d9a989e06a 100644
--- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
@@ -224,7 +224,7 @@ 
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-reset-gpios = <&gpio3 29 GPIO_ACTIVE_LOW>;
 	interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
 			      <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts b/arch/arm/boot/dts/imx6sx-sabreauto.dts
index b0ee324afe58..315044ccd65f 100644
--- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
+++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
@@ -75,7 +75,7 @@ 
 &fec1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet1>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy1>;
 	fsl,magic-packet;
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx6sx-sdb.dtsi b/arch/arm/boot/dts/imx6sx-sdb.dtsi
index 08ede56c3f10..f6972deb5e39 100644
--- a/arch/arm/boot/dts/imx6sx-sdb.dtsi
+++ b/arch/arm/boot/dts/imx6sx-sdb.dtsi
@@ -191,7 +191,7 @@ 
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet1>;
 	phy-supply = <&reg_enet_3v3>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy1>;
 	phy-reset-gpios = <&gpio2 7 GPIO_ACTIVE_LOW>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx7d-pico.dtsi b/arch/arm/boot/dts/imx7d-pico.dtsi
index 3fd595a71202..6f50ebf31a0a 100644
--- a/arch/arm/boot/dts/imx7d-pico.dtsi
+++ b/arch/arm/boot/dts/imx7d-pico.dtsi
@@ -92,7 +92,7 @@ 
 			  <&clks IMX7D_ENET1_TIME_ROOT_CLK>;
 	assigned-clock-parents = <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>;
 	assigned-clock-rates = <0>, <100000000>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy0>;
 	fsl,magic-packet;
 	phy-reset-gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;