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

[Anand Moon]

Hi Robin,

On Thu, 12 Mar 2026 at 18:18, Robin Murphy <robin.murphy at arm.com> wrote:
>
> 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.
>

Thanks, these changes help fix the warning.
Please add my
Tested-by: Anand Moon <linux.amoon at gmail.com>

Thanks
-Anand