[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2025061740-banter-acclaim-2006@gregkh>
Date: Tue, 17 Jun 2025 17:49:49 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Sean Anderson <sean.anderson@...ux.dev>
Cc: Saravana Kannan <saravanak@...gle.com>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Danilo Krummrich <dakr@...nel.org>, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, Christoph Hellwig <hch@....de>,
Rob Herring <robh+dt@...nel.org>,
Grant Likely <grant.likely@...aro.org>
Subject: Re: [PATCH] driver core: Prevent deferred probe loops
On Tue, Jun 17, 2025 at 11:35:04AM -0400, Sean Anderson wrote:
> On 6/17/25 04:50, Greg Kroah-Hartman wrote:
> > On Thu, Jun 12, 2025 at 04:40:48PM -0400, Sean Anderson wrote:
> >> On 6/12/25 13:56, Saravana Kannan wrote:
> >> > On Thu, Jun 12, 2025 at 8:53 AM Sean Anderson <sean.anderson@...ux.dev> wrote:
> >> >>
> >> >> On 6/11/25 08:23, Greg Kroah-Hartman wrote:
> >> >> > On Tue, Jun 10, 2025 at 07:44:27PM -0400, Sean Anderson wrote:
> >> >> >> On 6/10/25 19:32, Saravana Kannan wrote:
> >> >> >> > On Tue, Jun 10, 2025 at 11:35 AM Sean Anderson <sean.anderson@...ux.dev> wrote:
> >> >> >> >>
> >> >> >> >> A deferred probe loop can occur when a device returns EPROBE_DEFER after
> >> >> >> >> registering a bus with children:
> >> >> >> >
> >> >> >> > This is a broken driver. A parent device shouldn't register child
> >> >> >> > devices unless it is fully read itself. It's not logical to say the
> >> >> >> > child devices are available, if the parent itself isn't fully ready.
> >> >> >> > So, adding child devices/the bus should be the last thing done in the
> >> >> >> > parent's probe function.
> >> >> >> >
> >> >> >> > I know there are odd exceptions where the parent depends on the child,
> >> >> >> > so they might add the child a bit earlier in the probe
> >> >> >>
> >> >> >> This is exactly the case here. So the bus probing cannot happen any
> >> >> >> later than it already does.
> >> >> >
> >> >> > Please fix the driver not to do this.
> >> >>
> >> >> How? The driver needs the PCS to work. And the PCS can live on the MDIO
> >> >> bus.
> >> >
> >> > Obviously I don't know the full details, but you could implement it as
> >> > MFD. So the bus part would not get removed even if the PCS fails to
> >> > probe. Then the PCS can probe when whatever it needs ends up probing.
> >>
> >> I was thinking about making the MDIO bus a separate device. But I think
> >> it will be tricky to get suspend/resume working correctly. And this
> >> makes conversions more difficult because you cannot just add some
> >> pcs_get/pcs_put calls, you have to split out the MDIO bus too (which is
> >> invariably created as a child of the MAC).
> >>
> >> And what happens if a developer doesn't realize they have to split off
> >> the MDIO bus before converting? Everything works fine, except if there
> >> is some problem loading the PCS driver, which they may not test. Is this
> >> prohibition against failing after creating a bus documented anywhere? I
> >> don't recall seeing it...
> >
> > What do you mean "failing after creating a bus"? If a bus is failed to
> > be created, you fail like normal, no difference here.
>
> Creating the bus is successful, but there's an EPROBE_DEFER failure after
> that. Which induces the probe loop as described in my initial email.
Then don't allow a defer to happen :)
Or better yet, just succeed and spin up a new thread for the new bus to
attach it's devices to. That's what many other busses do today.
thanks,
greg k-h
Powered by blists - more mailing lists