The PHY used on the Freescale i.MX6Q/DL SABRE boards is qualified as
'rgmii' instead of 'rgmii-id'. Meaning the RX and TX delays that were
previously added by the MAC when required, but are now provided
internally by the PHY (and the MAC should no longer add the RX or TX
delays in this case).
This patch fixes the network problems seen on the Freescale i.MX6Q/DL
SABRE boards and makes a difference when correctly loading the NFS
rootfs on startup.
Link: https://lkml.kernel.org/r/1397569821-5530-4-git-send-email-thomas.petazzoni@free-electrons.com
Tested-by: Steve Twiss <[email protected]>
Tested-by: Adam Thomson <[email protected]>
Signed-off-by: Steve Twiss <[email protected]>
---
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index a070506..185fb17 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";
};
--
1.9.3
Hi Steve,
On Wed, Mar 20, 2019 at 9:06 AM Steve Twiss
<[email protected]> wrote:
>
> The PHY used on the Freescale i.MX6Q/DL SABRE boards is qualified as
> 'rgmii' instead of 'rgmii-id'. Meaning the RX and TX delays that were
This patch declares it as 'rgmii-id', which contradicts the commit log.
> previously added by the MAC when required, but are now provided
> internally by the PHY (and the MAC should no longer add the RX or TX
> delays in this case).
>
> This patch fixes the network problems seen on the Freescale i.MX6Q/DL
Please provide a Fixes tag. It would be good to know if this fix needs
to be applied to older kernels.
Hi Fabio,
On 20 March 2019 12:17, Fabio Estevam wrote:
> Subject: Re: [PATCH] ARM: dts: imx6qdl-sabresd: change phy-mode to use rgmii-id
>
> Hi Steve,
>
> On Wed, Mar 20, 2019 at 9:06 AM Steve Twiss
> <[email protected]> wrote:
> >
> > The PHY used on the Freescale i.MX6Q/DL SABRE boards is qualified as
> > 'rgmii' instead of 'rgmii-id'. Meaning the RX and TX delays that were
>
> This patch declares it as 'rgmii-id', which contradicts the commit log.
The PHY used on the Freescale i.MX6Q/DL SABRE boards is qualified as
'rgmii' instead of 'rgmii-id'.
Can that be read two ways?
I meant the phy-mode is currently qualified as 'rgmii' instead of what it
should be: 'rgmii-id'.
[...]
> > This patch fixes the network problems seen on the Freescale i.MX6Q/DL
>
> Please provide a Fixes tag. It would be good to know if this fix needs
> to be applied to older kernels.
I didn't put a Fixes tag in because I find this patch fixes v5.1-rc1 (which I see as broken).
But the patch is not needed in v5.0 because I see that as working.
So didn't see the need for a Cc: stable.
Regards,
Steve
Hi Steve,
[Adding Andrew]
On Wed, Mar 20, 2019 at 1:03 PM Steve Twiss
<[email protected]> wrote:
>
> Hi Fabio,
>
> On 20 March 2019 12:17, Fabio Estevam wrote:
> > Subject: Re: [PATCH] ARM: dts: imx6qdl-sabresd: change phy-mode to use rgmii-id
> >
> > Hi Steve,
> >
> > On Wed, Mar 20, 2019 at 9:06 AM Steve Twiss
> > <[email protected]> wrote:
> > >
> > > The PHY used on the Freescale i.MX6Q/DL SABRE boards is qualified as
> > > 'rgmii' instead of 'rgmii-id'. Meaning the RX and TX delays that were
> >
> > This patch declares it as 'rgmii-id', which contradicts the commit log.
>
> The PHY used on the Freescale i.MX6Q/DL SABRE boards is qualified as
> 'rgmii' instead of 'rgmii-id'.
>
> Can that be read two ways?
> I meant the phy-mode is currently qualified as 'rgmii' instead of what it
> should be: 'rgmii-id'.
Thanks for the clarification.
>
> [...]
>
> > > This patch fixes the network problems seen on the Freescale i.MX6Q/DL
> >
> > Please provide a Fixes tag. It would be good to know if this fix needs
> > to be applied to older kernels.
>
> I didn't put a Fixes tag in because I find this patch fixes v5.1-rc1 (which I see as broken).
> But the patch is not needed in v5.0 because I see that as working.
> So didn't see the need for a Cc: stable.
It seems we have other boards that need to be fixed and we can not
have an old dtb with functional Ethernet with a new kernel.
Does anyone know if this issue is AR8031 specific?
Thanks
On 19-03-20 14:04:42, Fabio Estevam wrote:
> Hi Steve,
>
> [Adding Andrew]
>
> On Wed, Mar 20, 2019 at 1:03 PM Steve Twiss
> <[email protected]> wrote:
> >
> > Hi Fabio,
> >
> > On 20 March 2019 12:17, Fabio Estevam wrote:
> > > Subject: Re: [PATCH] ARM: dts: imx6qdl-sabresd: change phy-mode to use rgmii-id
> > >
> > > Hi Steve,
> > >
> > > On Wed, Mar 20, 2019 at 9:06 AM Steve Twiss
> > > <[email protected]> wrote:
> > > >
> > > > The PHY used on the Freescale i.MX6Q/DL SABRE boards is qualified as
> > > > 'rgmii' instead of 'rgmii-id'. Meaning the RX and TX delays that were
> > >
> > > This patch declares it as 'rgmii-id', which contradicts the commit log.
> >
> > The PHY used on the Freescale i.MX6Q/DL SABRE boards is qualified as
> > 'rgmii' instead of 'rgmii-id'.
> >
> > Can that be read two ways?
> > I meant the phy-mode is currently qualified as 'rgmii' instead of what it
> > should be: 'rgmii-id'.
>
> Thanks for the clarification.
>
> >
> > [...]
> >
> > > > This patch fixes the network problems seen on the Freescale i.MX6Q/DL
> > >
> > > Please provide a Fixes tag. It would be good to know if this fix needs
> > > to be applied to older kernels.
> >
> > I didn't put a Fixes tag in because I find this patch fixes v5.1-rc1 (which I see as broken).
> > But the patch is not needed in v5.0 because I see that as working.
> > So didn't see the need for a Cc: stable.
>
> It seems we have other boards that need to be fixed and we can not
> have an old dtb with functional Ethernet with a new kernel.
>
> Does anyone know if this issue is AR8031 specific?
I can confirm the same fix is works on imx6sx too.
>
> Thanks
Hi Abel,
On Thu, Mar 21, 2019 at 5:42 AM Abel Vesa <[email protected]> wrote:
> > It seems we have other boards that need to be fixed and we can not
> > have an old dtb with functional Ethernet with a new kernel.
> >
> > Does anyone know if this issue is AR8031 specific?
>
> I can confirm the same fix is works on imx6sx too.
imx6sx-sabresd also uses an AR8031 Ethernet PHY.
I also tested that changing the phy-mode to "rgmii-id" fixes Ethernet
on pico-imx7d with AR8035.
So yes, we currently have lots of broken dtb's in mainline and I am
wondering what is the proper fix here.
Does anyone know what was the kernel commit that introduced such regressions?
Hi Fabio,
On 21 March 2019 11:17, Fabio Estevam,
> Subject: Re: [PATCH] ARM: dts: imx6qdl-sabresd: change phy-mode to use rgmii-id
>
> Hi Abel,
>
> On Thu, Mar 21, 2019 at 5:42 AM Abel Vesa <[email protected]> wrote:
>
> > > It seems we have other boards that need to be fixed and we can not
> > > have an old dtb with functional Ethernet with a new kernel.
> > >
> > > Does anyone know if this issue is AR8031 specific?
> >
> > I can confirm the same fix is works on imx6sx too.
>
> imx6sx-sabresd also uses an AR8031 Ethernet PHY.
>
> I also tested that changing the phy-mode to "rgmii-id" fixes Ethernet
> on pico-imx7d with AR8035.
>
> So yes, we currently have lots of broken dtb's in mainline and I am
> wondering what is the proper fix here.
Agreed!
The DT should be an ABI.
> Does anyone know what was the kernel commit that introduced such
> regressions?
I bisected to the original breakage (for the NFS rootfs) back to this commit:
commit 13d0ab6750b20957ac1466da4e44dc0af746ff28
Reverting this commit fixed the problem, but only for the kernel up to that
point: it was a long time ago, and several other fixes were added after that
which meant that by the time the kernel got to v5.0 it was working for me
again.
I bisected the breakage between v5.0 to v5.1-rc1 as this:
commit 3acca1dd17060332cfab15693733cdaf9fba1c90
... which doesn't make sense to me.
Regards,
Steve
---
commit 13d0ab6750b20957ac1466da4e44dc0af746ff28
Author: Heiner Kallweit <[email protected]>
Date: Wed Jan 16 08:07:38 2019 +0100
net: phy: check return code when requesting PHY driver module
When requesting the PHY driver module fails we'll bind the genphy
driver later. This isn't obvious to the user and may cause, depending
on the PHY, different types of issues. Therefore check the return code
of request_module(). Note that we only check for failures in loading
the module, not whether a module exists for the respective PHY ID.
v2:
- add comment explaining what is checked and what is not
- return error from phy_device_create() if loading module fails
Signed-off-by: Heiner Kallweit <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
---
commit 3acca1dd17060332cfab15693733cdaf9fba1c90
Author: Heiner Kallweit <[email protected]>
Date: Mon Mar 4 19:39:03 2019 +0100
net: dsa: mv88e6xxx: add call to mv88e6xxx_ports_cmode_init to probe for new DSA framework
In the original patch I missed to add mv88e6xxx_ports_cmode_init()
to the second probe function, the one for the new DSA framework.
Fixes: ed8fe20205ac ("net: dsa: mv88e6xxx: prevent interrupt storm caused by mv88e6390x_port_set_cmode")
Reported-by: Shaokun Zhang <[email protected]>
Suggested-by: Andrew Lunn <[email protected]>
Signed-off-by: Heiner Kallweit <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
On Thu, Mar 21, 2019 at 08:17:01AM -0300, Fabio Estevam wrote:
> Hi Abel,
>
> On Thu, Mar 21, 2019 at 5:42 AM Abel Vesa <[email protected]> wrote:
>
> > > It seems we have other boards that need to be fixed and we can not
> > > have an old dtb with functional Ethernet with a new kernel.
> > >
> > > Does anyone know if this issue is AR8031 specific?
> >
> > I can confirm the same fix is works on imx6sx too.
>
> imx6sx-sabresd also uses an AR8031 Ethernet PHY.
>
> I also tested that changing the phy-mode to "rgmii-id" fixes Ethernet
> on pico-imx7d with AR8035.
>
> So yes, we currently have lots of broken dtb's in mainline and I am
> wondering what is the proper fix here.
>
> Does anyone know what was the kernel commit that introduced such regressions?
Hi Fabio
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.
There is also a need to fix the PHY driver, because there is at least
one board which needs it fixed in order to work.
When we discussed fixing the PHY driver, we had no idea how many
boards would break. So we said, lets try it, and fix up whatever
breaks. We can however discuss this again. We can declare both the PHY
driver and the DTs terminally broken, and add a vendor proprietary
property for the phy-mode, which takes preference over the standard
phy-mode if present, and that does the correct thing.
Andrew
> I bisected to the original breakage (for the NFS rootfs) back to this commit:
> commit 13d0ab6750b20957ac1466da4e44dc0af746ff28
I don't think that is the correct patch. Yes, it broke here, but that
was a different problem.
I think the real commit is:
6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
That commit contains a fix for broken behaviour in the PHY driver,
when then also causes broken DTs to fail.
Andrew
Am Donnerstag, den 21.03.2019, 08:17 -0300 schrieb Fabio Estevam:
> Hi Abel,
>
> > On Thu, Mar 21, 2019 at 5:42 AM Abel Vesa <[email protected]> wrote:
>
> > > It seems we have other boards that need to be fixed and we can not
> > > have an old dtb with functional Ethernet with a new kernel.
> > >
> > > Does anyone know if this issue is AR8031 specific?
> >
> > I can confirm the same fix is works on imx6sx too.
>
> imx6sx-sabresd also uses an AR8031 Ethernet PHY.
>
> I also tested that changing the phy-mode to "rgmii-id" fixes Ethernet
> on pico-imx7d with AR8035.
>
> So yes, we currently have lots of broken dtb's in mainline and I am
> wondering what is the proper fix here.
Fix the DT. It's unfortunate the a kernel and DT bug canceled each
other out, but if the DT is clearly broken and fixing it in a backward
compatible way is not really feasible, I would argue that we should
treat a DT bug like any other bug.
Regards,
Lucas
On Thu, Mar 21, 2019 at 08:17:01AM -0300, Fabio Estevam wrote:
> Hi Abel,
>
> On Thu, Mar 21, 2019 at 5:42 AM Abel Vesa <[email protected]> wrote:
>
> > > It seems we have other boards that need to be fixed and we can not
> > > have an old dtb with functional Ethernet with a new kernel.
> > >
> > > Does anyone know if this issue is AR8031 specific?
> >
> > I can confirm the same fix is works on imx6sx too.
>
> imx6sx-sabresd also uses an AR8031 Ethernet PHY.
>
> I also tested that changing the phy-mode to "rgmii-id" fixes Ethernet
> on pico-imx7d with AR8035.
>
> So yes, we currently have lots of broken dtb's in mainline and I am
> wondering what is the proper fix here.
So can we have a single patch fixing all broken i.MX DTBs?
Shawn
Hi Shawn,
On Thu, Mar 21, 2019 at 10:12 PM Shawn Guo <[email protected]> wrote:
> > So yes, we currently have lots of broken dtb's in mainline and I am
> > wondering what is the proper fix here.
>
> So can we have a single patch fixing all broken i.MX DTBs?
Unfortunately, just by looking at the dts files we do not know if a
board uses an AR803x PHY or not, so I am afraid we can not do an
automatic conversion.
On Thu, Mar 21, 2019 at 11:00:18PM -0300, Fabio Estevam wrote:
> Hi Shawn,
>
> On Thu, Mar 21, 2019 at 10:12 PM Shawn Guo <[email protected]> wrote:
>
> > > So yes, we currently have lots of broken dtb's in mainline and I am
> > > wondering what is the proper fix here.
> >
> > So can we have a single patch fixing all broken i.MX DTBs?
>
> Unfortunately, just by looking at the dts files we do not know if a
> board uses an AR803x PHY or not, so I am afraid we can not do an
> automatic conversion.
At least for those we already know?
Shawn
On Thu, Mar 21, 2019 at 11:15 PM Shawn Guo <[email protected]> wrote:
> > Unfortunately, just by looking at the dts files we do not know if a
> > board uses an AR803x PHY or not, so I am afraid we can not do an
> > automatic conversion.
>
> At least for those we already know?
Yes, I can help preparing a patch that fixes the i.MX boards we know
use AR8031 PHY.
On 22. 03. 19 3:24, Fabio Estevam wrote:
> On Thu, Mar 21, 2019 at 11:15 PM Shawn Guo <[email protected]> wrote:
>
>>> Unfortunately, just by looking at the dts files we do not know if a
>>> board uses an AR803x PHY or not, so I am afraid we can not do an
>>> automatic conversion.
>>
>> At least for those we already know?
>
> Yes, I can help preparing a patch that fixes the i.MX boards we know
> use AR8031 PHY.
AFACT this issue is not AR803x specific. I was hit by the same problem
with QCA833x switch on imx6dl-yapp4. Though, we are currently the only
platform in tree using this chip and Shawn already has the fix in his tree.
I am not aware of any other PHY drivers that my cause problems.
$ git log --oneline v5.0..v5.1-rc1 --grep=RGMII --author=Vinod -- drivers/net/
6d4cd041f0af net: phy: at803x: disable delay only for RGMII mode
a968b5e9d587 net: dsa: qca8k: Enable delay for RGMII_ID mode
5ecdd77c61c8 net: dsa: qca8k: disable delay for RGMII mode
cd28d1d6e52e net: phy: at803x: Disable phy delay for RGMII mode
Michal
Here are my thoughts.
On 22 March 2019 10:21, Michal Vokáč wrote:
> Subject: Re: [PATCH] ARM: dts: imx6qdl-sabresd: change phy-mode to use rgmii-id
>
> On 22. 03. 19 3:24, Fabio Estevam wrote:
> > On Thu, Mar 21, 2019 at 11:15 PM Shawn Guo <[email protected]> wrote:
> >
> >>> Unfortunately, just by looking at the dts files we do not know if a
> >>> board uses an AR803x PHY or not, so I am afraid we can not do an
> >>> automatic conversion.
> >>
> >> At least for those we already know?
> >
> > Yes, I can help preparing a patch that fixes the i.MX boards we know
> > use AR8031 PHY.
>
> AFACT this issue is not AR803x specific. I was hit by the same problem
> with QCA833x switch on imx6dl-yapp4. Though, we are currently the only
> platform in tree using this chip and Shawn already has the fix in his tree.
>
> I am not aware of any other PHY drivers that my cause problems.
>
> $ git log --oneline v5.0..v5.1-rc1 --grep=RGMII --author=Vinod -- drivers/net/
> 6d4cd041f0af net: phy: at803x: disable delay only for RGMII mode
> a968b5e9d587 net: dsa: qca8k: Enable delay for RGMII_ID mode
> 5ecdd77c61c8 net: dsa: qca8k: disable delay for RGMII mode
> cd28d1d6e52e net: phy: at803x: Disable phy delay for RGMII mode
If this is more widespread, as Michal points out, this might affect the way
the fix is approach then?
On 21 March 2019 12:43 Lucas Stach wrote:
> > So yes, we currently have lots of broken dtb's in mainline and I am
> > wondering what is the proper fix here.
[...]
> but if the DT is clearly broken and fixing it in a backward
> compatible way is not really feasible, I would argue that we should
> treat a DT bug like any other bug.
There is a way of doing this to preserve backwards compatibility I think.
Maybe deprecating the previous DTS phy-mode and replacing it with a new one,
phy-mode-v2 instead?
At the moment, there is only a weak link between DTS and the driver in the kernel
which makes finding the error difficult. Also, it seems that partly, the direction of
change is caused by software which is pushing alterations in the DTS -- and not the
other way round.
Regards,
Steve
> There is a way of doing this to preserve backwards compatibility I think.
> Maybe deprecating the previous DTS phy-mode and replacing it with a new one,
> phy-mode-v2 instead?
Hi Steve
It needs to be a vendor specific property, atheros,phy-mode, for
example.
Andrew