Message-ID: <6d14a0fb-2bf3-4ca5-9356-b94517ddfb14@linux.dev>
Date: Thu, 19 Jun 2025 12:19:23 -0400
From: Sean Anderson <sean.anderson@...ux.dev>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
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 6/19/25 04:21, Greg Kroah-Hartman wrote:
> On Tue, Jun 17, 2025 at 01:14:31PM -0400, Sean Anderson wrote:
>> On 6/17/25 11:49, Greg Kroah-Hartman wrote:
>> > 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 :)
>> 
>> Well, I could require all PCS drivers to be built-in I guess. But I suspect
>> users will want them to be modules to reduce kernel size.
> 
> True, then just auto-load them as needed like all other busses do.
> 
>> > 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.
>> 
>> Sorry, I'm not sure I follow. How can you attach a device to a thread? Do
>> you have an example for this?
> 
> Busses discover their devices in a thread, which then calls probe for
> them when needed.  A device isn't being attached to a thread, sorry for
> the confusion.

OK, just to clarify, the subsystem I linked above is not a bus, it's an
internal API. Think GPIO or PWM. The devices typically live on an MDIO
bus (although a platform bus wouldn't be out of the question). So it's
"not our job" to load the module; that should be done by the bus. From
our perspective, if we look up a device and it's not there we don't
really know if it's ever going to show up.

Regarding the auxiliary bus, I tried it out and it works. The conversion
is around +50 lines, which is not ideal. Ideally I would like to push
complexity into subsystem code rather than making drivers deal with it,
but I don't really see a good way to do this in the subsystem. There are
just a lot of assumptions along the line of "when you register the
device you must know what capabilities it supports." For example, fixed
links (MAC to MAC) are validated when the phylink is created and the
whole process fails if there's an incompatibility. Which can occur if
the late-binding component is the thing that adds support for the fixed
link.

--Sean

Powered by blists - more mailing lists