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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <AM5PR04MB3139190E26F6948EA1D72CC28883A@AM5PR04MB3139.eurprd04.prod.outlook.com>
Date: Wed, 29 Nov 2023 02:34:14 +0000
From: Wei Fang <wei.fang@....com>
To: "hs@...x.de" <hs@...x.de>, Andrew Lunn <andrew@...n.ch>, Heiko Schocher
	<heiko.schocher@...il.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Clark Wang
	<xiaoning.wang@....com>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, dl-linux-imx
	<linux-imx@....com>, Paolo Abeni <pabeni@...hat.com>, Shenwei Wang
	<shenwei.wang@....com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Alexander Sverdlin
	<alexander.sverdlin@...mens.com>
Subject: RE: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet

> -----Original Message-----
> From: Heiko Schocher <hs@...x.de>
> Sent: 2023年11月28日 12:42
> To: Wei Fang <wei.fang@....com>; Andrew Lunn <andrew@...n.ch>; Heiko
> Schocher <heiko.schocher@...il.com>
> Cc: netdev@...r.kernel.org; Clark Wang <xiaoning.wang@....com>; David S.
> Miller <davem@...emloft.net>; Eric Dumazet <edumazet@...gle.com>;
> Jakub Kicinski <kuba@...nel.org>; dl-linux-imx <linux-imx@....com>; Paolo
> Abeni <pabeni@...hat.com>; Shenwei Wang <shenwei.wang@....com>;
> linux-kernel@...r.kernel.org; Alexander Sverdlin
> <alexander.sverdlin@...mens.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@...x.de