From: Sean Anderson <sean.anderson@linux.dev>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Saravana Kannan <saravanak@google.com>,
"Rafael J . Wysocki" <rafael@kernel.org>,
Danilo Krummrich <dakr@kernel.org>,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Christoph Hellwig <hch@lst.de>, Rob Herring <robh+dt@kernel.org>,
Grant Likely <grant.likely@linaro.org>
Subject: Re: [PATCH] driver core: Prevent deferred probe loops
Date: Tue, 17 Jun 2025 11:35:04 -0400 [thread overview]
Message-ID: <0ee2f641-c3f3-4a3a-87b4-e1279a862d68@linux.dev> (raw)
In-Reply-To: <2025061700-unmapped-labrador-a8c9@gregkh>
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@linux.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@linux.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.
> And if MFD doesn't work, there's always the aux-bus code, perhaps that
> should be used here instead?
I will have a look. However, I expect both of these approaches to
require fairly invasive conversions for existing drivers. Ideally, I
would like to keep conversions simple.
--Sean
next prev parent reply other threads:[~2025-06-17 15:35 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-09 23:57 [BUG] Deferred probe loop with child devices Sean Anderson
2025-06-10 18:34 ` [PATCH] driver core: Prevent deferred probe loops Sean Anderson
2025-06-10 23:32 ` Saravana Kannan
2025-06-10 23:44 ` Sean Anderson
2025-06-11 12:23 ` Greg Kroah-Hartman
2025-06-12 15:53 ` Sean Anderson
2025-06-12 17:56 ` Saravana Kannan
2025-06-12 20:40 ` Sean Anderson
2025-06-17 8:50 ` Greg Kroah-Hartman
2025-06-17 15:35 ` Sean Anderson [this message]
2025-06-17 15:49 ` Greg Kroah-Hartman
2025-06-17 17:14 ` Sean Anderson
2025-06-19 8:21 ` Greg Kroah-Hartman
2025-06-19 16:19 ` Sean Anderson
2025-06-19 16:33 ` Greg Kroah-Hartman
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=0ee2f641-c3f3-4a3a-87b4-e1279a862d68@linux.dev \
--to=sean.anderson@linux.dev \
--cc=dakr@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=grant.likely@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=hch@lst.de \
--cc=linux-kernel@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=robh+dt@kernel.org \
--cc=saravanak@google.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.