[PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[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.

Signed-off-by: Heiko Schocher <hs@denx.de>
---

 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

Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[Alexander Stein]

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 <hs@denx.de>
> ---
> 
>  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/

Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[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.

> 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 <hs@denx.de>
>> ---
>>
>>  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: hs@denx.de

Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[Alexander Stein]

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 <hs@denx.de>
> >> ---
> >> 
> >>  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/

Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[Andrew Lunn]

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 <hs@denx.de>
> ---
> 
>  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

RE: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[Wei Fang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: 2023年11月24日 4:09
> To: Heiko Schocher <heiko.schocher@gmail.com>
> Cc: netdev@vger.kernel.org; Heiko Schocher <hs@denx.de>; Clark Wang
> <xiaoning.wang@nxp.com>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> dl-linux-imx <linux-imx@nxp.com>; Paolo Abeni <pabeni@redhat.com>;
> Shenwei Wang <shenwei.wang@nxp.com>; Wei Fang <wei.fang@nxp.com>;
> linux-kernel@vger.kernel.org
> 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 <hs@denx.de>
> > ---
> >
> >  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/20230208101821.871269-1-alexander.sverdlin@siemens.com/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.
> 

Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[Heiko Schocher]

Hello Wei,

On 24.11.23 03:33, Wei Fang wrote:
>> -----Original Message-----
>> From: Andrew Lunn <andrew@lunn.ch>
>> Sent: 2023年11月24日 4:09
>> To: Heiko Schocher <heiko.schocher@gmail.com>
>> Cc: netdev@vger.kernel.org; Heiko Schocher <hs@denx.de>; Clark Wang
>> <xiaoning.wang@nxp.com>; David S. Miller <davem@davemloft.net>; Eric
>> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
>> dl-linux-imx <linux-imx@nxp.com>; Paolo Abeni <pabeni@redhat.com>;
>> Shenwei Wang <shenwei.wang@nxp.com>; Wei Fang <wei.fang@nxp.com>;
>> linux-kernel@vger.kernel.org
>> 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 <hs@denx.de>
>>> ---
>>>
>>>  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/20230208101821.871269-1-alexander.sverdlin@siemens.com/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: hs@denx.de

RE: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[Wei Fang]

> -----Original Message-----
> From: Heiko Schocher <hs@denx.de>
> Sent: 2023年11月24日 13:19
> To: Wei Fang <wei.fang@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Heiko
> Schocher <heiko.schocher@gmail.com>
> Cc: netdev@vger.kernel.org; Clark Wang <xiaoning.wang@nxp.com>; David S.
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
> Jakub Kicinski <kuba@kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Paolo
> Abeni <pabeni@redhat.com>; Shenwei Wang <shenwei.wang@nxp.com>;
> linux-kernel@vger.kernel.org; Alexander Sverdlin
> <alexander.sverdlin@siemens.com>
> 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 <andrew@lunn.ch>
> >> Sent: 2023年11月24日 4:09
> >> To: Heiko Schocher <heiko.schocher@gmail.com>
> >> Cc: netdev@vger.kernel.org; Heiko Schocher <hs@denx.de>; Clark Wang
> >> <xiaoning.wang@nxp.com>; David S. Miller <davem@davemloft.net>; Eric
> >> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> >> dl-linux-imx <linux-imx@nxp.com>; Paolo Abeni <pabeni@redhat.com>;
> >> Shenwei Wang <shenwei.wang@nxp.com>; Wei Fang
> <wei.fang@nxp.com>;
> >> linux-kernel@vger.kernel.org
> >> 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 <hs@denx.de>
> >>> ---
> >>>
> >>>  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.

Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[Heiko Schocher]

Hello Wei,

On 27.11.23 07:55, Wei Fang wrote:
>> -----Original Message-----
>> From: Heiko Schocher <hs@denx.de>
>> Sent: 2023年11月24日 13:19
>> To: Wei Fang <wei.fang@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Heiko
>> Schocher <heiko.schocher@gmail.com>
>> Cc: netdev@vger.kernel.org; Clark Wang <xiaoning.wang@nxp.com>; David S.
>> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
>> Jakub Kicinski <kuba@kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Paolo
>> Abeni <pabeni@redhat.com>; Shenwei Wang <shenwei.wang@nxp.com>;
>> linux-kernel@vger.kernel.org; Alexander Sverdlin
>> <alexander.sverdlin@siemens.com>
>> 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 <andrew@lunn.ch>
>>>> Sent: 2023年11月24日 4:09
>>>> To: Heiko Schocher <heiko.schocher@gmail.com>
>>>> Cc: netdev@vger.kernel.org; Heiko Schocher <hs@denx.de>; Clark Wang
>>>> <xiaoning.wang@nxp.com>; David S. Miller <davem@davemloft.net>; Eric
>>>> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
>>>> dl-linux-imx <linux-imx@nxp.com>; Paolo Abeni <pabeni@redhat.com>;
>>>> Shenwei Wang <shenwei.wang@nxp.com>; Wei Fang
>> <wei.fang@nxp.com>;
>>>> linux-kernel@vger.kernel.org
>>>> 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 <hs@denx.de>
>>>>> ---
>>>>>
>>>>>  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: hs@denx.de

RE: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[Wei Fang]

> -----Original Message-----
> From: Heiko Schocher <hs@denx.de>
> Sent: 2023年11月28日 12:42
> To: Wei Fang <wei.fang@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Heiko
> Schocher <heiko.schocher@gmail.com>
> Cc: netdev@vger.kernel.org; Clark Wang <xiaoning.wang@nxp.com>; David S.
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
> Jakub Kicinski <kuba@kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Paolo
> Abeni <pabeni@redhat.com>; Shenwei Wang <shenwei.wang@nxp.com>;
> linux-kernel@vger.kernel.org; Alexander Sverdlin
> <alexander.sverdlin@siemens.com>
> 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:
> hs@denx.de

Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

[Heiko Schocher]

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 <hs@denx.de>
>> ---
>>
>>  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: hs@denx.de