it is possible that fec1 is probed before fec0. On SoCs
with FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii
from fec0) init of mii fails for fec1 when fec0 is not yet
probed, as fec0 setups mii bus. In this case fec_enet_mii_init
for fec1 returns with -ENODEV, and so fec1 never comes up.
Return here with -EPROBE_DEFER so interface gets later
probed again.
Found this on imx8qxp based board, using 2 ethernet interfaces,
and from time to time, fec1 interface came not up.
Signed-off-by: Heiko Schocher <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index c3b7694a7485..d956f95e7a65 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
mii_cnt++;
return 0;
}
- return -ENOENT;
+ return -EPROBE_DEFER;
}
bus_freq = 2500000; /* 2.5MHz by default */
--
2.20.1
Hello Heiko,
Am Donnerstag, 23. November 2023, 14:27:43 CET schrieb Heiko Schocher:
> it is possible that fec1 is probed before fec0. On SoCs
> with FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii
> from fec0) init of mii fails for fec1 when fec0 is not yet
> probed, as fec0 setups mii bus. In this case fec_enet_mii_init
> for fec1 returns with -ENODEV, and so fec1 never comes up.
>
> Return here with -EPROBE_DEFER so interface gets later
> probed again.
>
> Found this on imx8qxp based board, using 2 ethernet interfaces,
> and from time to time, fec1 interface came not up.
But FEC_QUIRK_SINGLE_MDIO is only set for imx28. How is this related to
imx8qxp?
Will this also help for imx6ul when fec1 is almost always probed before fec0
due to order of DT nodes?
Best regards,
Alexander
> Signed-off-by: Heiko Schocher <[email protected]>
> ---
>
> drivers/net/ethernet/freescale/fec_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c index
> c3b7694a7485..d956f95e7a65 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct platform_device
> *pdev) mii_cnt++;
> return 0;
> }
> - return -ENOENT;
> + return -EPROBE_DEFER;
> }
>
> bus_freq = 2500000; /* 2.5MHz by default */
--
TQ-Systems GmbH | M?hlstra?e 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht M?nchen, HRB 105018
Gesch?ftsf?hrer: Detlef Schneider, R?diger Stahl, Stefan Schneider
http://www.tq-group.com/
Hello Alexander,
On 23.11.23 16:11, Alexander Stein wrote:
> Hello Heiko,
>
> Am Donnerstag, 23. November 2023, 14:27:43 CET schrieb Heiko Schocher:
>> it is possible that fec1 is probed before fec0. On SoCs
>> with FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii
>> from fec0) init of mii fails for fec1 when fec0 is not yet
>> probed, as fec0 setups mii bus. In this case fec_enet_mii_init
>> for fec1 returns with -ENODEV, and so fec1 never comes up.
>>
>> Return here with -EPROBE_DEFER so interface gets later
>> probed again.
>>
>> Found this on imx8qxp based board, using 2 ethernet interfaces,
>> and from time to time, fec1 interface came not up.
>
> But FEC_QUIRK_SINGLE_MDIO is only set for imx28. How is this related to
> imx8qxp?
Ah, yes ... customer uses NXP based kernel there is:
/* board only enable one mii bus in default */
if (!of_get_property(np, "fsl,mii-exclusive", NULL))
fep->quirks |= FEC_QUIRK_SINGLE_MDIO;
which is missing in mainline... nevertheless patch fixes a problem
with boards having quirk FEC_QUIRK_SINGLE_MDIO set.
> Will this also help for imx6ul when fec1 is almost always probed before fec0
> due to order of DT nodes?
Yep, I think so... do you have the chance to test such a setup?
bye,
Heiko
>
> Best regards,
> Alexander
>
>> Signed-off-by: Heiko Schocher <[email protected]>
>> ---
>>
>> drivers/net/ethernet/freescale/fec_main.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>> b/drivers/net/ethernet/freescale/fec_main.c index
>> c3b7694a7485..d956f95e7a65 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct platform_device
>> *pdev) mii_cnt++;
>> return 0;
>> }
>> - return -ENOENT;
>> + return -EPROBE_DEFER;
>> }
>>
>> bus_freq = 2500000; /* 2.5MHz by default */
>
>
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: [email protected]
Hello Heiko,
Am Donnerstag, 23. November 2023, 16:26:18 CET schrieb Heiko Schocher:
> Hello Alexander,
>
> On 23.11.23 16:11, Alexander Stein wrote:
> > Hello Heiko,
> >
> > Am Donnerstag, 23. November 2023, 14:27:43 CET schrieb Heiko Schocher:
> >> it is possible that fec1 is probed before fec0. On SoCs
> >> with FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii
> >> from fec0) init of mii fails for fec1 when fec0 is not yet
> >> probed, as fec0 setups mii bus. In this case fec_enet_mii_init
> >> for fec1 returns with -ENODEV, and so fec1 never comes up.
> >>
> >> Return here with -EPROBE_DEFER so interface gets later
> >> probed again.
> >>
> >> Found this on imx8qxp based board, using 2 ethernet interfaces,
> >> and from time to time, fec1 interface came not up.
> >
> > But FEC_QUIRK_SINGLE_MDIO is only set for imx28. How is this related to
> > imx8qxp?
>
> Ah, yes ... customer uses NXP based kernel there is:
>
> /* board only enable one mii bus in default */
> if (!of_get_property(np, "fsl,mii-exclusive", NULL))
> fep->quirks |= FEC_QUIRK_SINGLE_MDIO;
>
> which is missing in mainline... nevertheless patch fixes a problem
> with boards having quirk FEC_QUIRK_SINGLE_MDIO set.
But this seems wrong. Apparently fec driver fails if MDIO bus is not (yet)
available. But 'fsl,mii-exclusive' + FEC_QUIRK_SINGLE_MDIO assumes both
interfaces use fec, no? Will this work e.g. on imx8mp if the FEC PHY is
attached to STMMAC (EQOS) PHY?
> > Will this also help for imx6ul when fec1 is almost always probed before
> > fec0 due to order of DT nodes?
>
> Yep, I think so... do you have the chance to test such a setup?
I have a board for that. But I'm not able to test at the moment.
Best regards,
Alexander
> bye,
> Heiko
>
> > Best regards,
> > Alexander
> >
> >> Signed-off-by: Heiko Schocher <[email protected]>
> >> ---
> >>
> >> drivers/net/ethernet/freescale/fec_main.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> >> b/drivers/net/ethernet/freescale/fec_main.c index
> >> c3b7694a7485..d956f95e7a65 100644
> >> --- a/drivers/net/ethernet/freescale/fec_main.c
> >> +++ b/drivers/net/ethernet/freescale/fec_main.c
> >> @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct platform_device
> >> *pdev) mii_cnt++;
> >>
> >> return 0;
> >>
> >> }
> >>
> >> - return -ENOENT;
> >> + return -EPROBE_DEFER;
> >>
> >> }
> >>
> >> bus_freq = 2500000; /* 2.5MHz by default */
--
TQ-Systems GmbH | M?hlstra?e 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht M?nchen, HRB 105018
Gesch?ftsf?hrer: Detlef Schneider, R?diger Stahl, Stefan Schneider
http://www.tq-group.com/
On Thu, Nov 23, 2023 at 02:27:43PM +0100, Heiko Schocher wrote:
> it is possible that fec1 is probed before fec0. On SoCs
> with FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii
> from fec0) init of mii fails for fec1 when fec0 is not yet
> probed, as fec0 setups mii bus. In this case fec_enet_mii_init
> for fec1 returns with -ENODEV, and so fec1 never comes up.
>
> Return here with -EPROBE_DEFER so interface gets later
> probed again.
>
> Found this on imx8qxp based board, using 2 ethernet interfaces,
> and from time to time, fec1 interface came not up.
>
> Signed-off-by: Heiko Schocher <[email protected]>
> ---
>
> drivers/net/ethernet/freescale/fec_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index c3b7694a7485..d956f95e7a65 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
> mii_cnt++;
> return 0;
> }
> - return -ENOENT;
> + return -EPROBE_DEFER;
I think this has been tried before.
Are there any issues with the mii_cnt++; I thought the previous
attempt as this had problems with the wrong mdio bus being assigned to
fep->mii_bus ? But i could be remembering wrongly.
Andrew
> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: 2023??11??24?? 4:09
> To: Heiko Schocher <[email protected]>
> Cc: [email protected]; Heiko Schocher <[email protected]>; Clark Wang
> <[email protected]>; David S. Miller <[email protected]>; Eric
> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
> dl-linux-imx <[email protected]>; Paolo Abeni <[email protected]>;
> Shenwei Wang <[email protected]>; Wei Fang <[email protected]>;
> [email protected]
> Subject: Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet
>
> On Thu, Nov 23, 2023 at 02:27:43PM +0100, Heiko Schocher wrote:
> > it is possible that fec1 is probed before fec0. On SoCs with
> > FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii from fec0) init
> > of mii fails for fec1 when fec0 is not yet probed, as fec0 setups mii
> > bus. In this case fec_enet_mii_init for fec1 returns with -ENODEV, and
> > so fec1 never comes up.
> >
> > Return here with -EPROBE_DEFER so interface gets later probed again.
> >
> > Found this on imx8qxp based board, using 2 ethernet interfaces, and
> > from time to time, fec1 interface came not up.
> >
> > Signed-off-by: Heiko Schocher <[email protected]>
> > ---
> >
> > drivers/net/ethernet/freescale/fec_main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index c3b7694a7485..d956f95e7a65 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct
> platform_device *pdev)
> > mii_cnt++;
> > return 0;
> > }
> > - return -ENOENT;
> > + return -EPROBE_DEFER;
>
> I think this has been tried before.
>
Yes, there was indeed a similar patch [1] before. but this issue seems to only exist
in downstream tree, because there is a local patch which is not in the upstream.
/* board only enable one mii bus in default */
if (!of_get_property(np, "fsl,mii-exclusive", NULL))
fep->quirks |= FEC_QUIRK_SINGLE_MDIO;
I will think about how to solve this issue in downstream if I'm free later.
[1] https://lore.kernel.org/lkml/[email protected]/T/#r6313852054bf12baabe21418584fbb6d58343197
> Are there any issues with the mii_cnt++; I thought the previous attempt as
> this had problems with the wrong mdio bus being assigned to
> fep->mii_bus ? But i could be remembering wrongly.
>
Hello Andrew,
On 23.11.23 21:09, Andrew Lunn wrote:
> On Thu, Nov 23, 2023 at 02:27:43PM +0100, Heiko Schocher wrote:
>> it is possible that fec1 is probed before fec0. On SoCs
>> with FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii
>> from fec0) init of mii fails for fec1 when fec0 is not yet
>> probed, as fec0 setups mii bus. In this case fec_enet_mii_init
>> for fec1 returns with -ENODEV, and so fec1 never comes up.
>>
>> Return here with -EPROBE_DEFER so interface gets later
>> probed again.
>>
>> Found this on imx8qxp based board, using 2 ethernet interfaces,
>> and from time to time, fec1 interface came not up.
>>
>> Signed-off-by: Heiko Schocher <[email protected]>
>> ---
>>
>> drivers/net/ethernet/freescale/fec_main.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
>> index c3b7694a7485..d956f95e7a65 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
>> mii_cnt++;
>> return 0;
>> }
>> - return -ENOENT;
>> + return -EPROBE_DEFER;
>
> I think this has been tried before.
Oh, wasn;t aware of it...
> Are there any issues with the mii_cnt++; I thought the previous
> attempt as this had problems with the wrong mdio bus being assigned to
> fep->mii_bus ? But i could be remembering wrongly.
The problem with returning -ENOENT is, that the probe never called
again ... with returning -EPROBE_DEFER, the device gets probed
later again.
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: [email protected]
Hello Wei,
On 24.11.23 03:33, Wei Fang wrote:
>> -----Original Message-----
>> From: Andrew Lunn <[email protected]>
>> Sent: 2023??11??24?? 4:09
>> To: Heiko Schocher <[email protected]>
>> Cc: [email protected]; Heiko Schocher <[email protected]>; Clark Wang
>> <[email protected]>; David S. Miller <[email protected]>; Eric
>> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
>> dl-linux-imx <[email protected]>; Paolo Abeni <[email protected]>;
>> Shenwei Wang <[email protected]>; Wei Fang <[email protected]>;
>> [email protected]
>> Subject: Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet
>>
>> On Thu, Nov 23, 2023 at 02:27:43PM +0100, Heiko Schocher wrote:
>>> it is possible that fec1 is probed before fec0. On SoCs with
>>> FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii from fec0) init
>>> of mii fails for fec1 when fec0 is not yet probed, as fec0 setups mii
>>> bus. In this case fec_enet_mii_init for fec1 returns with -ENODEV, and
>>> so fec1 never comes up.
>>>
>>> Return here with -EPROBE_DEFER so interface gets later probed again.
>>>
>>> Found this on imx8qxp based board, using 2 ethernet interfaces, and
>>> from time to time, fec1 interface came not up.
>>>
>>> Signed-off-by: Heiko Schocher <[email protected]>
>>> ---
>>>
>>> drivers/net/ethernet/freescale/fec_main.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>>> b/drivers/net/ethernet/freescale/fec_main.c
>>> index c3b7694a7485..d956f95e7a65 100644
>>> --- a/drivers/net/ethernet/freescale/fec_main.c
>>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>>> @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct
>> platform_device *pdev)
>>> mii_cnt++;
>>> return 0;
>>> }
>>> - return -ENOENT;
>>> + return -EPROBE_DEFER;
>>
>> I think this has been tried before.
>>
> Yes, there was indeed a similar patch [1] before. but this issue seems to only exist
> in downstream tree, because there is a local patch which is not in the upstream.
>
> /* board only enable one mii bus in default */
> if (!of_get_property(np, "fsl,mii-exclusive", NULL))
> fep->quirks |= FEC_QUIRK_SINGLE_MDIO;
Yes, sorry for overseeing this ... but there are SoCs in upstream which
set FEC_QUIRK_SINGLE_MDIO and so mii bus from for example fec0 is used on
other fecX devices, correct?
And for them probing currently fail, when probed before fec0.
> I will think about how to solve this issue in downstream if I'm free later.
>
> [1] https://lore.kernel.org/lkml/[email protected]/T/#r6313852054bf12baabe21418584fbb6d58343197
Hmm.. simply deferring the probe to later point solves the problem here.
I added Alexander may he can comment...
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: [email protected]
> -----Original Message-----
> From: Heiko Schocher <[email protected]>
> Sent: 2023??11??24?? 13:19
> To: Wei Fang <[email protected]>; Andrew Lunn <[email protected]>; Heiko
> Schocher <[email protected]>
> Cc: [email protected]; Clark Wang <[email protected]>; David S.
> Miller <[email protected]>; Eric Dumazet <[email protected]>;
> Jakub Kicinski <[email protected]>; dl-linux-imx <[email protected]>; Paolo
> Abeni <[email protected]>; Shenwei Wang <[email protected]>;
> [email protected]; Alexander Sverdlin
> <[email protected]>
> Subject: Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet
>
> Hello Wei,
>
> On 24.11.23 03:33, Wei Fang wrote:
> >> -----Original Message-----
> >> From: Andrew Lunn <[email protected]>
> >> Sent: 2023??11??24?? 4:09
> >> To: Heiko Schocher <[email protected]>
> >> Cc: [email protected]; Heiko Schocher <[email protected]>; Clark Wang
> >> <[email protected]>; David S. Miller <[email protected]>; Eric
> >> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
> >> dl-linux-imx <[email protected]>; Paolo Abeni <[email protected]>;
> >> Shenwei Wang <[email protected]>; Wei Fang
> <[email protected]>;
> >> [email protected]
> >> Subject: Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not
> >> probed yet
> >>
> >> On Thu, Nov 23, 2023 at 02:27:43PM +0100, Heiko Schocher wrote:
> >>> it is possible that fec1 is probed before fec0. On SoCs with
> >>> FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii from fec0) init
> >>> of mii fails for fec1 when fec0 is not yet probed, as fec0 setups
> >>> mii bus. In this case fec_enet_mii_init for fec1 returns with
> >>> -ENODEV, and so fec1 never comes up.
> >>>
> >>> Return here with -EPROBE_DEFER so interface gets later probed again.
> >>>
> >>> Found this on imx8qxp based board, using 2 ethernet interfaces, and
> >>> from time to time, fec1 interface came not up.
> >>>
> >>> Signed-off-by: Heiko Schocher <[email protected]>
> >>> ---
> >>>
> >>> drivers/net/ethernet/freescale/fec_main.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> >>> b/drivers/net/ethernet/freescale/fec_main.c
> >>> index c3b7694a7485..d956f95e7a65 100644
> >>> --- a/drivers/net/ethernet/freescale/fec_main.c
> >>> +++ b/drivers/net/ethernet/freescale/fec_main.c
> >>> @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct
> >> platform_device *pdev)
> >>> mii_cnt++;
> >>> return 0;
> >>> }
> >>> - return -ENOENT;
> >>> + return -EPROBE_DEFER;
> >>
> >> I think this has been tried before.
> >>
> > Yes, there was indeed a similar patch [1] before. but this issue seems
> > to only exist in downstream tree, because there is a local patch which is not
> in the upstream.
> >
> > /* board only enable one mii bus in default */
> > if (!of_get_property(np, "fsl,mii-exclusive", NULL))
> > fep->quirks |= FEC_QUIRK_SINGLE_MDIO;
>
> Yes, sorry for overseeing this ... but there are SoCs in upstream which set
> FEC_QUIRK_SINGLE_MDIO and so mii bus from for example fec0 is used on
> other fecX devices, correct?
>
Yes, i.MX28 still uses FEC_QUIRK_SINGLE_MDIO flag in the upstream, But the
current patch doesn't seem to completely solve this issue.
If fec1 is probed before fec0, I think its fep->dev_id should be equal to 0, so it will
not go to the following statement.
if ((fep->quirks & FEC_QUIRK_SINGLE_MDIO) && fep->dev_id > 0) {
/* fec1 uses fec0 mii_bus */
if (mii_cnt && fec0_mii_bus) {
fep->mii_bus = fec0_mii_bus;
mii_cnt++;
return 0;
}
return -ENOENT;
}
I'm considering removing the FEC_QUIRK_SINGLE_MDIO flag from upstream
in the future as it's not necessary. But not now as I am busy with other projects.
Hello Wei,
On 27.11.23 07:55, Wei Fang wrote:
>> -----Original Message-----
>> From: Heiko Schocher <[email protected]>
>> Sent: 2023??11??24?? 13:19
>> To: Wei Fang <[email protected]>; Andrew Lunn <[email protected]>; Heiko
>> Schocher <[email protected]>
>> Cc: [email protected]; Clark Wang <[email protected]>; David S.
>> Miller <[email protected]>; Eric Dumazet <[email protected]>;
>> Jakub Kicinski <[email protected]>; dl-linux-imx <[email protected]>; Paolo
>> Abeni <[email protected]>; Shenwei Wang <[email protected]>;
>> [email protected]; Alexander Sverdlin
>> <[email protected]>
>> Subject: Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet
>>
>> Hello Wei,
>>
>> On 24.11.23 03:33, Wei Fang wrote:
>>>> -----Original Message-----
>>>> From: Andrew Lunn <[email protected]>
>>>> Sent: 2023??11??24?? 4:09
>>>> To: Heiko Schocher <[email protected]>
>>>> Cc: [email protected]; Heiko Schocher <[email protected]>; Clark Wang
>>>> <[email protected]>; David S. Miller <[email protected]>; Eric
>>>> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
>>>> dl-linux-imx <[email protected]>; Paolo Abeni <[email protected]>;
>>>> Shenwei Wang <[email protected]>; Wei Fang
>> <[email protected]>;
>>>> [email protected]
>>>> Subject: Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not
>>>> probed yet
>>>>
>>>> On Thu, Nov 23, 2023 at 02:27:43PM +0100, Heiko Schocher wrote:
>>>>> it is possible that fec1 is probed before fec0. On SoCs with
>>>>> FEC_QUIRK_SINGLE_MDIO set (which means fec1 uses mii from fec0) init
>>>>> of mii fails for fec1 when fec0 is not yet probed, as fec0 setups
>>>>> mii bus. In this case fec_enet_mii_init for fec1 returns with
>>>>> -ENODEV, and so fec1 never comes up.
>>>>>
>>>>> Return here with -EPROBE_DEFER so interface gets later probed again.
>>>>>
>>>>> Found this on imx8qxp based board, using 2 ethernet interfaces, and
>>>>> from time to time, fec1 interface came not up.
>>>>>
>>>>> Signed-off-by: Heiko Schocher <[email protected]>
>>>>> ---
>>>>>
>>>>> drivers/net/ethernet/freescale/fec_main.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>>>>> b/drivers/net/ethernet/freescale/fec_main.c
>>>>> index c3b7694a7485..d956f95e7a65 100644
>>>>> --- a/drivers/net/ethernet/freescale/fec_main.c
>>>>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>>>>> @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct
>>>> platform_device *pdev)
>>>>> mii_cnt++;
>>>>> return 0;
>>>>> }
>>>>> - return -ENOENT;
>>>>> + return -EPROBE_DEFER;
>>>>
>>>> I think this has been tried before.
>>>>
>>> Yes, there was indeed a similar patch [1] before. but this issue seems
>>> to only exist in downstream tree, because there is a local patch which is not
>> in the upstream.
>>>
>>> /* board only enable one mii bus in default */
>>> if (!of_get_property(np, "fsl,mii-exclusive", NULL))
>>> fep->quirks |= FEC_QUIRK_SINGLE_MDIO;
>>
>> Yes, sorry for overseeing this ... but there are SoCs in upstream which set
>> FEC_QUIRK_SINGLE_MDIO and so mii bus from for example fec0 is used on
>> other fecX devices, correct?
>>
> Yes, i.MX28 still uses FEC_QUIRK_SINGLE_MDIO flag in the upstream, But the
> current patch doesn't seem to completely solve this issue.
>
> If fec1 is probed before fec0, I think its fep->dev_id should be equal to 0, so it will
> not go to the following statement.
Indeed... and than fep->dev_id is interchanged too... I wonder why not using:
fep->dev_id = of_alias_get_id(np, "ethernet");
in fec_probe function for getting the dev_id. With that no problems with
"wrong" dev_id and also my change makes sense...
But you are correct, currently patch is useless.
> if ((fep->quirks & FEC_QUIRK_SINGLE_MDIO) && fep->dev_id > 0) {
> /* fec1 uses fec0 mii_bus */
> if (mii_cnt && fec0_mii_bus) {
> fep->mii_bus = fec0_mii_bus;
> mii_cnt++;
> return 0;
> }
> return -ENOENT;
> }
>
> I'm considering removing the FEC_QUIRK_SINGLE_MDIO flag from upstream
> in the future as it's not necessary. But not now as I am busy with other projects.
Hmm.. and what is on platforms which have this usecase?
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: [email protected]
> -----Original Message-----
> From: Heiko Schocher <[email protected]>
> Sent: 2023??11??28?? 12:42
> To: Wei Fang <[email protected]>; Andrew Lunn <[email protected]>; Heiko
> Schocher <[email protected]>
> Cc: [email protected]; Clark Wang <[email protected]>; David S.
> Miller <[email protected]>; Eric Dumazet <[email protected]>;
> Jakub Kicinski <[email protected]>; dl-linux-imx <[email protected]>; Paolo
> Abeni <[email protected]>; Shenwei Wang <[email protected]>;
> [email protected]; Alexander Sverdlin
> <[email protected]>
> Subject: Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet
>
> >>>>>
> >>>>> drivers/net/ethernet/freescale/fec_main.c | 2 +-
> >>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> >>>>> b/drivers/net/ethernet/freescale/fec_main.c
> >>>>> index c3b7694a7485..d956f95e7a65 100644
> >>>>> --- a/drivers/net/ethernet/freescale/fec_main.c
> >>>>> +++ b/drivers/net/ethernet/freescale/fec_main.c
> >>>>> @@ -2445,7 +2445,7 @@ static int fec_enet_mii_init(struct
> >>>> platform_device *pdev)
> >>>>> mii_cnt++;
> >>>>> return 0;
> >>>>> }
> >>>>> - return -ENOENT;
> >>>>> + return -EPROBE_DEFER;
> >>>>
> >>>> I think this has been tried before.
> >>>>
> >>> Yes, there was indeed a similar patch [1] before. but this issue
> >>> seems to only exist in downstream tree, because there is a local
> >>> patch which is not
> >> in the upstream.
> >>>
> >>> /* board only enable one mii bus in default */
> >>> if (!of_get_property(np, "fsl,mii-exclusive", NULL))
> >>> fep->quirks |= FEC_QUIRK_SINGLE_MDIO;
> >>
> >> Yes, sorry for overseeing this ... but there are SoCs in upstream
> >> which set FEC_QUIRK_SINGLE_MDIO and so mii bus from for example fec0
> >> is used on other fecX devices, correct?
> >>
> > Yes, i.MX28 still uses FEC_QUIRK_SINGLE_MDIO flag in the upstream, But
> > the current patch doesn't seem to completely solve this issue.
> >
> > If fec1 is probed before fec0, I think its fep->dev_id should be equal
> > to 0, so it will not go to the following statement.
>
> Indeed... and than fep->dev_id is interchanged too... I wonder why not using:
>
> fep->dev_id = of_alias_get_id(np, "ethernet");
>
Maybe not all dts have ethernet aliases in them. And there are some platforms
that may not use DT.
> in fec_probe function for getting the dev_id. With that no problems with
> "wrong" dev_id and also my change makes sense...
>
> But you are correct, currently patch is useless.
>
> > if ((fep->quirks & FEC_QUIRK_SINGLE_MDIO) && fep->dev_id > 0) {
> > /* fec1 uses fec0 mii_bus */
> > if (mii_cnt && fec0_mii_bus) {
> > fep->mii_bus = fec0_mii_bus;
> > mii_cnt++;
> > return 0;
> > }
> > return -ENOENT;
> > }
> >
> > I'm considering removing the FEC_QUIRK_SINGLE_MDIO flag from
> upstream
> > in the future as it's not necessary. But not now as I am busy with other
> projects.
>
> Hmm.. and what is on platforms which have this usecase?
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH, Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email:
> [email protected]