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月28日 12:42
> 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
> 
> >>>>>
> >>>>>  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@xxxxxxx