Re: [PATCH v3] PCI: dw-rockchip: Enable async probe by default

[Robin Murphy]

On 2026-03-11 9:09 pm, Danilo Krummrich wrote:
> On Wed Mar 11, 2026 at 1:28 PM CET, Manivannan Sadhasivam wrote:
> > On Wed, Mar 11, 2026 at 12:46:03PM +0100, Danilo Krummrich wrote:
> > > On Wed Mar 11, 2026 at 6:24 AM CET, Manivannan Sadhasivam wrote:
> > > > I have a contrary view here. If just a single driver or lib doesn't handle async
> > > > probe, it cannot just force other drivers to not take the advantage of async
> > > > probe. As I said above, enabling async probe easily saves a few hunderd ms or
> > > > even more if there are more than one Root Port or Root Complex in an SoC.
> > >
> > > Then the driver or lib has to be fixed / improved first or the driver core has
> > > to be enabled to deal with a case where PROBE_FORCE_SYNCHRONOUS is requested
> > > from an async path, etc.
> > >
> > > In any case, applying the patch and breaking things (knowingly?) doesn't seem
> > > like the correct approach.
> > >
> > > > I strongly agree with you here that the underlying issue should be fixed. But
> > > > the real impact to end users is not this splat, but not having the boot time
> > > > optimization that this patch brings in. As an end user, one would want their
> > > > systems to boot quickly and they wouldn't bother much about a harmless warning
> > > > splat appearing in the dmesg log.
> > >
> > > You mean quickly booting into a "harmless" potential deadlock condition the
> > > warning splat tries to make people aware of? :)
> >
> > Hmm, I overlooked the built-as-module part where the deadlock could be possible
> > as indicated by the comment about the WARN_ON_ONCE().
> >
> > But what is the path forward here? Do you want the phylib to fix the
> > request_module() call or fix the driver core instead?
>
> Here are a few thoughts.
>
> In general, I think the best would be to get rid of the (affected)
> PROBE_FORCE_SYNCHRONOUS cases.
>
> Now, I guess this can be pretty hard for a PCI controller driver, as you can't
> really predict what ends up being probed from you async context, i.e. it could
> even be some other bus controller and things could even propagate further.
>
> Not sure how big of a deal it is in practice though, there are not a lot of
> PROBE_FORCE_SYNCHRONOUS drivers (left), but note that specifying neither
> PROBE_FORCE_SYNCHRONOUS nor PROBE_PREFER_ASYNCHRONOUS currently results in
> synchronous by default.
>
> (Also, quite some other PCI controller drivers do set PROBE_PREFER_ASYNCHRONOUS
> and apparently got lucky with it.)
>
>  From a driver-core perspective I think we're rather limited on what we can do;
> we are already in async context at this point and can't magically go back to
> initcall context.
>
> So, the only thing I can think of is to kick off work on a workqueue, which in
> the end would be the same as the deferred probe handling.

Hmm, in fact, isn't the deferred probe mechanism itself actually quite
appropriate? A suitable calling context isn't the most obvious "resource
provider" to wait for, but ultimately it's still a case of "we don't
have everything we need right now, but it's worth trying again soon".
I may have missed some subtleties, but my instinct is that it could
perhaps be as simple as something like this (completely untested).

Cheers,
Robin.

----->8-----

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index bea8da5f8a3a..3c4a0207ae3f 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -954,6 +954,16 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
 	if (data->check_async && async_allowed != data->want_async)
 		return 0;
 
+	/*
+	 * Bus drivers may probe asynchronously, but be adding a child device
+	 * whose driver still wants a synchronous probe. In this case, just
+	 * defer it, to be triggered by the parent driver probe succeeding.
+	 */
+	if (!async_allowed && current_is_async()) {
+		driver_deferred_probe_add(dev);
+		return 0;
+	}
+
 	/*
 	 * Ignore errors returned by ->probe so that the next driver can try
 	 * its luck.