lkml.org 
[lkml]   [2023]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] net: fec: fix probing of fec1 when fec0 is not probed yet
From
Date
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

\
 
 \ /
  Last update: 2023-11-28 05:43    [W:0.145 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site