Messages in this thread | | | Subject | Re: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet | From | Heiko Schocher <> | Date | Tue, 28 Nov 2023 05:41:43 +0100 |
| |
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
| |