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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).