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

[Wei Fang]

> -----Original Message-----
> From: Heiko Schocher <hs@xxxxxxx>
> Sent: 2023年11月24日 13:19
> To: Wei Fang <wei.fang@xxxxxxx>; Andrew Lunn <andrew@xxxxxxx>; Heiko
> Schocher <heiko.schocher@xxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; Clark Wang <xiaoning.wang@xxxxxxx>; David S.
> Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>;
> Jakub Kicinski <kuba@xxxxxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>; Paolo
> Abeni <pabeni@xxxxxxxxxx>; Shenwei Wang <shenwei.wang@xxxxxxx>;
> linux-kernel@xxxxxxxxxxxxxxx; Alexander Sverdlin
> <alexander.sverdlin@xxxxxxxxxxx>
> 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@xxxxxxx>
> >> Sent: 2023年11月24日 4:09
> >> To: Heiko Schocher <heiko.schocher@xxxxxxxxx>
> >> Cc: netdev@xxxxxxxxxxxxxxx; Heiko Schocher <hs@xxxxxxx>; Clark Wang
> >> <xiaoning.wang@xxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric
> >> Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>;
> >> dl-linux-imx <linux-imx@xxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>;
> >> Shenwei Wang <shenwei.wang@xxxxxxx>; Wei Fang
> <wei.fang@xxxxxxx>;
> >> linux-kernel@xxxxxxxxxxxxxxx
> >> 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@xxxxxxx>
> >>> ---
> >>>
> >>>  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.