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 for Android: free password hash cracker in your pocket

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

Date: Mon, 27 Nov 2023 06:55:02 +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月24日 13:19
> 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
> 
> Hello Wei,
> 
> On 24.11.23 03:33, Wei Fang wrote:
> >> -----Original Message-----
> >> From: Andrew Lunn <andrew@...n.ch>
> >> Sent: 2023年11月24日 4:09
> >> To: Heiko Schocher <heiko.schocher@...il.com>
> >> Cc: netdev@...r.kernel.org; Heiko Schocher <hs@...x.de>; 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>; Wei Fang
> <wei.fang@....com>;
> >> linux-kernel@...r.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@...x.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.