All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Joakim Zhang <qiangqing.zhang@nxp.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: (EXT) Re: [PATCH] net: fec: defer probe if PHY on external MDIO bus is not available
Date: Thu, 21 Oct 2021 09:08:19 +0200	[thread overview]
Message-ID: <c286107376a99ca2201db058e1973e2b2264e9fb.camel@ew.tq-group.com> (raw)
In-Reply-To: <YXBk8gwuCqrxDbVY@lunn.ch>

On Wed, 2021-10-20 at 20:50 +0200, Andrew Lunn wrote:
> > > I've not looked at the details yet, just back from vacation. But this
> > > seems wrong. I would of expected phylib to of returned -EPRODE_DEFER
> > > at some point, when asked for a PHY which does not exist yet. All the
> > > driver should need to do is make sure it returns the
> > > -EPRODE_DEFER.
> > 
> > This is what I expected as well, however there are a few complications:
> > 
> > - At the moment the first time the driver does anything with the PHY is
> >   in fec_enet_open(), not in fec_probe() - way too late to defer
> >   anything
> 
> O.K. Right. Are you using NFS root? For normal user space opening of
> the interface, this has all been sorted out by the time user space
> does anything. The NFS root changes the time in a big way.

NFS root is one of our usecases.

> 
> Anyway, i would say some bits of code need moving from open to probe
> so EPROBE_DEFER can be used.
> 
> We already have:
> 
>         phy_node = of_parse_phandle(np, "phy-handle", 0);
>         if (!phy_node && of_phy_is_fixed_link(np)) {
>                 ret = of_phy_register_fixed_link(np);
>                 if (ret < 0) {
>                         dev_err(&pdev->dev,
>                                 "broken fixed-link specification\n");
>                         goto failed_phy;
>                 }
>                 phy_node = of_node_get(np);
>         }
>         fep->phy_node = phy_node;
> 
> Go one step further. If fep->phy_node is not NULL, we know there
> should be a PHY. So call of_phy_find_device(). If it returns NULL,
> then -EPROBE_DEFER. Otherwise store the phydev into fep, and use it in
> open.
> 
> You will need to move the call to fec_enet_mii_init(pdev) earlier, so
> the MDIO bus is available.

I would love to do this, but driver-api/driver-model/driver.rst
contains the following warning:

      -EPROBE_DEFER must not be returned if probe() has already created
      child devices, even if those child devices are removed again
      in a cleanup path. If -EPROBE_DEFER is returned after a child
      device has been registered, it may result in an infinite loop of
      .probe() calls to the same driver.

My understanding of this is that there is simply no way to return
-EPROBE_DEFER after fec_enet_mii_init(pdev).



> 
>     Andrew


  reply	other threads:[~2021-10-21  7:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-14 11:30 [PATCH] net: fec: defer probe if PHY on external MDIO bus is not available Matthias Schiffer
2021-10-18 10:20 ` Joakim Zhang
2021-10-18 10:32   ` Matthias Schiffer
2021-10-19 14:12 ` Andrew Lunn
2021-10-20 12:03   ` Matthias Schiffer
2021-10-20 18:50     ` Andrew Lunn
2021-10-21  7:08       ` Matthias Schiffer [this message]
2021-10-21 12:50         ` (EXT) " Andrew Lunn
2021-10-22  7:57           ` Matthias Schiffer
2021-10-22 12:56             ` Andrew Lunn
2021-10-26 11:54               ` Matthias Schiffer
2021-11-03 11:59                 ` Matthias Schiffer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c286107376a99ca2201db058e1973e2b2264e9fb.camel@ew.tq-group.com \
    --to=matthias.schiffer@ew.tq-group.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=qiangqing.zhang@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.