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

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

[Robin Murphy]

[ +driver-core maintainers - async probe question below ]

On 2026-03-10 3:30 pm, Manivannan Sadhasivam wrote:
> Hi Robin,
> 
> On Tue, Mar 10, 2026 at 01:41:56PM +0000, Robin Murphy wrote:
>> Hi Mani,
>>
>> On 2026-03-04 6:48 am, Manivannan Sadhasivam wrote:
>>>
>>> On Thu, 26 Feb 2026 15:40:23 +0530, Anand Moon wrote:
>>>> Rockchip DWC PCIe driver currently performs synchronous link training for
>>>> combo PHYs (PCIe 3.0/2.0 and SATA 3.0) during boot. This process waits for
>>>> the link to be fully established, adding several milliseconds to the boot
>>>> sequence. To optimize boot time, this change enables asynchronous probing,
>>>> allowing link establishment to proceed in the background while the kernel
>>>> continues probing other devices.
>>>>
>>>> [...]
>>>
>>> Applied, thanks!
>>>
>>> [1/1] PCI: dw-rockchip: Enable async probe by default
>>>         commit: ec392abc95932838bf7e3d659d358f4df9ff5a0a
>>
>> This appears to have the side-effect that calling pci_host_probe() from
>> async context can effectively force async probe for the endpoint drivers
>> as well, but some drivers are not OK with that, as our CI has just
>> flagged up.
>>
> 
> Thanks for reporting!
> 
>> (And as for that particular warning, ISTR last time I looked into it
>> another context, the opinion of the MDIO/phy maintainers seemed to be
>> "don't force async probe".)
>>
> 
> This was discussed during v2 [1] and concluded that the async probe benefits
> outweigh the unharmful splat from phylib. I also agree with the above conclusion
> that this splat should not prevent us from enabling async probe for PCI
> controller drivers. It can easily save a few 100ms during boot.

The problem is not an "unharmful splat" from one driver on one board - 
that's *a* symptom, and the fact that it happens to be a relatively 
benign one does not dismiss the problem that forcing async probe upon 
drivers that are not designed to support async probe cannot in general 
be assumed to be safe, so is not OK. It's one thing if a user brings it 
upon themselves by explicitly using the "driver_async_probe=" option, 
but it's very different if some other driver starts doing it for them.

Looking closer, it seems like the fundamental issue might be when we've 
got this far (simplified for clarity):

- async_run_entry_fn
   - rockchip_pcie_probe
     - pci_host_probe
      - pci_bus_add_device
        - device_initial_probe
          - __device_attach_driver

wherein we then reach the "if (data->check_async && async_allowed != 
data->want_async)" condition, at which point check_async is true (from 
device_initial_probe()), while async_allowed and want_async are *both* 
false, but that leads us to actually go ahead and call 
driver_probe_device() for the child device despite being in async 
context. That doesn't seem right to me - I'm guessing it maybe wasn't 
anticipated to have bus drivers calling device_initial_probe() from 
within async in the first place?

It may not strictly be the fault of this patch - clearly 91703041697c 
("PCI: Allow built-in drivers to use async initial probing") is 
implicated in this too - but the fact is that it *has* exposed a bug 
that needs fixing one way or another, it can't just be left hanging and 
impacting end users.

Thanks,
Robin.

> 
> - Mani
> 
> [1] https://lore.kernel.org/linux-pci/aYHVlS1nbCMMyF04@ryzen
> 

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

[Danilo Krummrich]

(Cc: Dmitry)

On Tue Mar 10, 2026 at 10:03 PM CET, Robin Murphy wrote:
> [ +driver-core maintainers - async probe question below ]

<snip>

> I'm guessing it maybe wasn't anticipated to have bus drivers calling
> device_initial_probe() from within async in the first place?

I think this is not limited to device_initial_probe(), device_attach() or even
device_add() would have the same problem. I.e. the driver core simply does not
consider whether it is already running in an async handler when it is requested
to run probe() synchronously.

A simple workaround to this would be to check whether current_is_async() and in
case it returns true just defer probing in an PROBE_FORCE_SYNCHRONOUS case. This
would at least be compatible with the guarantees given by
PROBE_FORCE_SYNCHRONOUS, but it doesn't sound quite right to me -- guess I have
to think about it a bit more.

In any case, given that this is not a supported case, this commit seems to be
wrong and should probably be reverted.

I think a quick workaround in the driver core as mentioned above is not a good
idea, instead this should be properly thought through.

> It may not strictly be the fault of this patch - clearly 91703041697c 
> ("PCI: Allow built-in drivers to use async initial probing") is 
> implicated in this too - but the fact is that it *has* exposed a bug 
> that needs fixing one way or another, it can't just be left hanging and 
> impacting end users.

At a side note, I think device_initial_probe() was not meant to be exposed
outside of the driver core in the first place. As the name suggests it is only
meant to be called on the initial probe() path (i.e. the initial probe() path of
the driver core). It seems to me that it ended up in include/linux/device.h
instead of drivers/base/base.h by accident.

The original commit - commit 765230b5f084 ("driver-core: add asynchronous
probing support for drivers") - introducing the feature even mentions "manual
binding is still synchronous" in its commit message and I think this has never
been changed.

So, it seems commit 91703041697c ("PCI: Allow built-in drivers to use async
initial probing") relies on something that might work by accident. :)

So, I wouldn't rule out any unexpected side effects entirely.

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

[Dmitry Torokhov]

On Wed, Mar 11, 2026 at 01:43:15AM +0100, Danilo Krummrich wrote:
> (Cc: Dmitry)
> 
> On Tue Mar 10, 2026 at 10:03 PM CET, Robin Murphy wrote:
> > [ +driver-core maintainers - async probe question below ]
> 
> <snip>
> 
> > I'm guessing it maybe wasn't anticipated to have bus drivers calling
> > device_initial_probe() from within async in the first place?
> 
> I think this is not limited to device_initial_probe(), device_attach() or even
> device_add() would have the same problem. I.e. the driver core simply does not
> consider whether it is already running in an async handler when it is requested
> to run probe() synchronously.
> 
> A simple workaround to this would be to check whether current_is_async() and in
> case it returns true just defer probing in an PROBE_FORCE_SYNCHRONOUS case. This
> would at least be compatible with the guarantees given by
> PROBE_FORCE_SYNCHRONOUS, but it doesn't sound quite right to me -- guess I have
> to think about it a bit more.
> 
> In any case, given that this is not a supported case, this commit seems to be
> wrong and should probably be reverted.
> 
> I think a quick workaround in the driver core as mentioned above is not a good
> idea, instead this should be properly thought through.

I think the bigger question is why PCI does something different from
every other bus? Why doesn't it rely on driver core to bind devices to
driver?

> 
> > It may not strictly be the fault of this patch - clearly 91703041697c 
> > ("PCI: Allow built-in drivers to use async initial probing") is 
> > implicated in this too - but the fact is that it *has* exposed a bug 
> > that needs fixing one way or another, it can't just be left hanging and 
> > impacting end users.
> 
> At a side note, I think device_initial_probe() was not meant to be exposed
> outside of the driver core in the first place. As the name suggests it is only
> meant to be called on the initial probe() path (i.e. the initial probe() path of
> the driver core). It seems to me that it ended up in include/linux/device.h
> instead of drivers/base/base.h by accident.

Yes, at the time when async probing was introduced driver core was the
sole caller of device_initial_probe(). 

> 
> The original commit - commit 765230b5f084 ("driver-core: add asynchronous
> probing support for drivers") - introducing the feature even mentions "manual
> binding is still synchronous" in its commit message and I think this has never
> been changed.

Yes, when users "echo" into bind/unbind sysfs attributes they expect
error codes to indicate whether operation has succeeded or not. That is
why even if driver is marked as "prefer asynchronous" in this particular
case the kernel still binds driver synchronously.

> 
> So, it seems commit 91703041697c ("PCI: Allow built-in drivers to use async
> initial probing") relies on something that might work by accident. :)
> 
> So, I wouldn't rule out any unexpected side effects entirely.

Thanks.

-- 
Dmitry

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

[Lukas Wunner]

On Tue, Mar 24, 2026 at 08:44:22PM -0700, Dmitry Torokhov wrote:
> I think the bigger question is why PCI does something different from
> every other bus? Why doesn't it rely on driver core to bind devices to
> driver?

This was introduced in 2013 with commit 4f535093cf8f ("PCI: Put pci_dev
in device tree as early as possible").  Corresponding patch submission:

https://lore.kernel.org/all/1358803252-20639-12-git-send-email-yinghai@kernel.org/

The rationale in the commit message for performing device registration
earlier is that devices would otherwise not be found by for_each_pci_dev().
I don't know why that was a problem at the time.  The patch is the last
one in the series, so there's no other patch succeeding and thus
depending on it, which could provide a clue why this was necessary.

Needless to say, changing the behavior now risks regressions.

The author of the patch is apparently no longer around.  Last activity
on the mailing list was in October 2017.  Access to kernel.org infra
persists though, so maybe he's still reachable:

https://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git/plain/keys/A7C92F7A39BD11DA.asc

I'm not defending the patch, merely trying to explain the history
as far as it is possible.

Thanks,

Lukas

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

[Manivannan Sadhasivam]

On Tue, Mar 10, 2026 at 09:03:32PM +0000, Robin Murphy wrote:
> [ +driver-core maintainers - async probe question below ]
> 
> On 2026-03-10 3:30 pm, Manivannan Sadhasivam wrote:
> > Hi Robin,
> > 
> > On Tue, Mar 10, 2026 at 01:41:56PM +0000, Robin Murphy wrote:
> > > Hi Mani,
> > > 
> > > On 2026-03-04 6:48 am, Manivannan Sadhasivam wrote:
> > > > 
> > > > On Thu, 26 Feb 2026 15:40:23 +0530, Anand Moon wrote:
> > > > > Rockchip DWC PCIe driver currently performs synchronous link training for
> > > > > combo PHYs (PCIe 3.0/2.0 and SATA 3.0) during boot. This process waits for
> > > > > the link to be fully established, adding several milliseconds to the boot
> > > > > sequence. To optimize boot time, this change enables asynchronous probing,
> > > > > allowing link establishment to proceed in the background while the kernel
> > > > > continues probing other devices.
> > > > > 
> > > > > [...]
> > > > 
> > > > Applied, thanks!
> > > > 
> > > > [1/1] PCI: dw-rockchip: Enable async probe by default
> > > >         commit: ec392abc95932838bf7e3d659d358f4df9ff5a0a
> > > 
> > > This appears to have the side-effect that calling pci_host_probe() from
> > > async context can effectively force async probe for the endpoint drivers
> > > as well, but some drivers are not OK with that, as our CI has just
> > > flagged up.
> > > 
> > 
> > Thanks for reporting!
> > 
> > > (And as for that particular warning, ISTR last time I looked into it
> > > another context, the opinion of the MDIO/phy maintainers seemed to be
> > > "don't force async probe".)
> > > 
> > 
> > This was discussed during v2 [1] and concluded that the async probe benefits
> > outweigh the unharmful splat from phylib. I also agree with the above conclusion
> > that this splat should not prevent us from enabling async probe for PCI
> > controller drivers. It can easily save a few 100ms during boot.
> 
> The problem is not an "unharmful splat" from one driver on one board -
> that's *a* symptom, and the fact that it happens to be a relatively benign
> one does not dismiss the problem that forcing async probe upon drivers that
> are not designed to support async probe cannot in general be assumed to be
> safe, so is not OK. It's one thing if a user brings it upon themselves by
> explicitly using the "driver_async_probe=" option, but it's very different
> if some other driver starts doing it for them.
> 

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.

Moreover, there are multiple ways this splat could be triggered as reported in:
https://lore.kernel.org/netdev/7103704.9J7NaK4W3v@fedora.fritz.box

> Looking closer, it seems like the fundamental issue might be when we've got
> this far (simplified for clarity):
> 
> - async_run_entry_fn
>   - rockchip_pcie_probe
>     - pci_host_probe
>      - pci_bus_add_device
>        - device_initial_probe
>          - __device_attach_driver
> 
> wherein we then reach the "if (data->check_async && async_allowed !=
> data->want_async)" condition, at which point check_async is true (from
> device_initial_probe()), while async_allowed and want_async are *both*
> false, but that leads us to actually go ahead and call driver_probe_device()
> for the child device despite being in async context. That doesn't seem right
> to me - I'm guessing it maybe wasn't anticipated to have bus drivers calling
> device_initial_probe() from within async in the first place?
> 

But isn't the underlying issue is with phylib calling request_module() while the
driver core performs async probing?

> It may not strictly be the fault of this patch - clearly 91703041697c ("PCI:
> Allow built-in drivers to use async initial probing") is implicated in this
> too - but the fact is that it *has* exposed a bug that needs fixing one way
> or another, it can't just be left hanging and impacting end users.
> 

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.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

[Lukas Wunner]

On Wed, Mar 11, 2026 at 10:54:59AM +0530, Manivannan Sadhasivam wrote:
> But isn't the underlying issue is with phylib calling request_module()
> while the driver core performs async probing?

I'd say the underlying issue is the call to async_synchronize_full()
in do_init_module().  That's very coarse-grained.  Maybe it can be
replaced with async_synchronize_full_domain() or async_synchronize_cookie()
to perform a more targeted wait?

Thanks,

Lukas

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

[Danilo Krummrich]

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? :)

(Or am I missing a subtle detail and we can never actually end up in a deadlock
for some reason?)

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

[Niklas Cassel]

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? :)
> 
> (Or am I missing a subtle detail and we can never actually end up in a deadlock
> for some reason?)

Right now it will print a warning even when trying to load a PHY driver that
is already loaded (e.g. if the PHY driver is built as built-in):
https://github.com/torvalds/linux/blob/v7.0-rc3/drivers/net/phy/phy_device.c#L806-L815

If the PHY driver is built as a module, then I assume that the deadlock
warning is legit.


Kind regards,
Niklas

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

[Manivannan Sadhasivam]

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?

I can drop this patch in the meantime. But holding this prolong wouldn't help.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

[Danilo Krummrich]

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.

Another alternative would be to let the subsystem handle such cases, which in
this case would probably mean to handle the current_is_async() case in
pci_host_probe() or pci_bus_add_devices().

On the other hand, this would probably end up to be a subsystem specific
implementation of "kick of work on a workqueue".

> I can drop this patch in the meantime. But holding this prolong wouldn't help.

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

[Shawn Lin]

Hi Mani and Danilo,

在 2026/03/12 星期四 5:09, Danilo Krummrich 写道:
> 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.
> 
> Another alternative would be to let the subsystem handle such cases, which in
> this case would probably mean to handle the current_is_async() case in
> pci_host_probe() or pci_bus_add_devices().
> 
> On the other hand, this would probably end up to be a subsystem specific
> implementation of "kick of work on a workqueue".

Actually, this is exactly what we've been doing in our downstream
code[1] for quite a long time. We've even pushed this optimization down
to the individual driver level.

To be honest, I'm not particularly fond of this approach either.
However, on our platform, we have more than 5 Root Port instances that
need to be initialized during boot. When all of them probe
synchronously, it significantly delays the execution of many other
critical drivers. Our customers have been complaining about the boot
time, and our downstream team had no real choice but to kick off a
kthread similar to your suggestion.

The code is very ugly and expose a lot of corner cases problem which
wasted quite lot of time to fix. That's why I was hesitant to bring
up the this approach upstream initially. But since you mentioned it, I
think it's worth evaluating whether we can implement this at the
subsystem level instead of forcing each driver to handle it
individually. This way, the complexity would be centralized, and drivers
that benefit from async probe (like multi-Root-Port SoCs) could opt-in
without worrying about the request_module() deadlock scenario.

What do you think?


[1] 
https://github.com/rockchip-linux/kernel/blob/develop-6.6/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L1764

> 
>> I can drop this patch in the meantime. But holding this prolong wouldn't help.
> 

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

[Anand Moon]

Hi Shawn, Mani, Niklas, Robin,,

On Thu, 12 Mar 2026 at 07:03, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> Hi Mani and Danilo,
>
> 在 2026/03/12 星期四 5:09, Danilo Krummrich 写道:
> > 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.
> >
> > Another alternative would be to let the subsystem handle such cases, which in
> > this case would probably mean to handle the current_is_async() case in
> > pci_host_probe() or pci_bus_add_devices().
> >
> > On the other hand, this would probably end up to be a subsystem specific
> > implementation of "kick of work on a workqueue".
>
> Actually, this is exactly what we've been doing in our downstream
> code[1] for quite a long time. We've even pushed this optimization down
> to the individual driver level.
>
> To be honest, I'm not particularly fond of this approach either.
> However, on our platform, we have more than 5 Root Port instances that
> need to be initialized during boot. When all of them probe
> synchronously, it significantly delays the execution of many other
> critical drivers. Our customers have been complaining about the boot
> time, and our downstream team had no real choice but to kick off a
> kthread similar to your suggestion.
>
> The code is very ugly and expose a lot of corner cases problem which
> wasted quite lot of time to fix. That's why I was hesitant to bring
> up the this approach upstream initially. But since you mentioned it, I
> think it's worth evaluating whether we can implement this at the
> subsystem level instead of forcing each driver to handle it
> individually. This way, the complexity would be centralized, and drivers
> that benefit from async probe (like multi-Root-Port SoCs) could opt-in
> without worrying about the request_module() deadlock scenario.
>
> What do you think?
>
>
> [1]
> https://github.com/rockchip-linux/kernel/blob/develop-6.6/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L1764
>
> >
> >> I can drop this patch in the meantime. But holding this prolong wouldn't help.
> >
I've reproduced the issue on my end. Following Danilo's suggestion,
I used PROBE_FORCE_SYNCHRONOUS to resolve the warning.
If you agree this is the correct fix, I’ll resubmit the patch.

 git diff drivers/pci/controller/dwc/pcie-dw-rockchip.c
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 5b17da63151d..ecaf1b446008 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -746,6 +746,7 @@ static struct platform_driver rockchip_pcie_driver = {
                .name   = "rockchip-dw-pcie",
                .of_match_table = rockchip_pcie_of_match,
                .suppress_bind_attrs = true,
+               .probe_type = PROBE_FORCE_SYNCHRONOUS,
        },
        .probe = rockchip_pcie_probe,
 };

Thanks
-Anand

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

[Danilo Krummrich]

On Thu Mar 12, 2026 at 12:40 PM CET, Anand Moon wrote:
> I've reproduced the issue on my end. Following Danilo's suggestion,
> I used PROBE_FORCE_SYNCHRONOUS to resolve the warning.

I did not propose to add PROBE_FORCE_SYNCHRONOUS, I just noted that we should
not force all subsequent drivers into async probe() by adding
PROBE_PREFER_ASYNCHRONOUS before the issue is resolved.

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

[Anand Moon]

Hi Danilo,

On Thu, 12 Mar 2026 at 17:24, Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Thu Mar 12, 2026 at 12:40 PM CET, Anand Moon wrote:
> > I've reproduced the issue on my end. Following Danilo's suggestion,
> > I used PROBE_FORCE_SYNCHRONOUS to resolve the warning.
>
> I did not propose to add PROBE_FORCE_SYNCHRONOUS, I just noted that we should
> not force all subsequent drivers into async probe() by adding
> PROBE_PREFER_ASYNCHRONOUS before the issue is resolved.

My apologies,Thanks for clarifying. I misunderstood your point.

Thanks
-Anand

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.

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

[Danilo Krummrich]

On Thu Mar 12, 2026 at 1:48 PM CET, Robin Murphy wrote:
> On 2026-03-11 9:09 pm, Danilo Krummrich wrote:
>>  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?

Yes, I've also mentioned this in [1], including the fact that it technically
even complies with the guarantees given by PROBE_FORCE_SYNCHRONOUS. I.e. the
documentation says:

	Use this to annotate drivers that need their probe routines to run
	synchronously with driver and device registration (with the exception of
	-EPROBE_DEFER handling - re-probing always ends up being done
	asynchronously).

However, I'm still not sure how I feel about this, since I consider this to be
more like a workaround that just moves things to a "more approprite" async
context.

On the other hand, eventually we want everything to work with
PROBE_PREFER_ASYNCHRONOUS, so maybe it's also good enough for the time being.

[1] https://lore.kernel.org/driver-core/DGZJBMG2Y738.2MU5LXVGEDD47@kernel.org/

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

[Robin Murphy]

On 2026-03-12 12:59 pm, Danilo Krummrich wrote:
> On Thu Mar 12, 2026 at 1:48 PM CET, Robin Murphy wrote:
>> On 2026-03-11 9:09 pm, Danilo Krummrich wrote:
>>>   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?
> 
> Yes, I've also mentioned this in [1], including the fact that it technically
> even complies with the guarantees given by PROBE_FORCE_SYNCHRONOUS. I.e. the
> documentation says:
> 
> 	Use this to annotate drivers that need their probe routines to run
> 	synchronously with driver and device registration (with the exception of
> 	-EPROBE_DEFER handling - re-probing always ends up being done
> 	asynchronously).
> 
> However, I'm still not sure how I feel about this, since I consider this to be
> more like a workaround that just moves things to a "more approprite" async
> context.

I guess the underlying problem there is that there are at least 3 
different significant aspects to what "synchronous" can mean:

- literally in the context or device/driver registration as documented. 
Off-hand I'm not really sure what useful property may be *specific* to 
those conditions that a driver might rely on, other than for 
super-special cases like platform_driver_probe().

- serialised, i.e. probes of multiple devices won't happen concurrently 
on multiple threads. This is probably the one hiding the most driver 
bugs, e.g. internal shared/global state without sufficient 
synchronisation. I guess this falls out as a side-effect of the first 
condition, but AFAICS it *can* also still provided by deferred probe 
(given that it's a single work item iterating a list one-by-one)

- in some regular thread context that isn't liable to have issues 
synchronising against other async_func workers (i.e. the request_module 
case). Again, deferred can't have a problem here, or it wouldn't have 
worked properly in general for the last decade.

So it's not that we'd be relying on some dubious "deferred is always 
synchronous" assumption - AFAICS deferred *can* launch async if the 
driver permits it - more just ratifying that deferred is still able to 
effectively honour all the useful properties of PROBE_FORCE_SYNCHRONOUS 
other than "during registration of the thing". And where people do want 
the semantics for a platform_driver_probe()-like thing, then I think 
it's reasonable to say that they should definitely never be invoking 
that from the probe routine of some other driver which permits async itself.

Thanks,
Robin.

> 
> On the other hand, eventually we want everything to work with
> PROBE_PREFER_ASYNCHRONOUS, so maybe it's also good enough for the time being.
> 
> [1] https://lore.kernel.org/driver-core/DGZJBMG2Y738.2MU5LXVGEDD47@kernel.org/

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

[Danilo Krummrich]

On Fri Mar 13, 2026 at 2:15 PM CET, Robin Murphy wrote:
> On 2026-03-12 12:59 pm, Danilo Krummrich wrote:
>> On Thu Mar 12, 2026 at 1:48 PM CET, Robin Murphy wrote:
>>> On 2026-03-11 9:09 pm, Danilo Krummrich wrote:
>>>>   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?
>> 
>> Yes, I've also mentioned this in [1], including the fact that it technically
>> even complies with the guarantees given by PROBE_FORCE_SYNCHRONOUS. I.e. the
>> documentation says:
>> 
>> 	Use this to annotate drivers that need their probe routines to run
>> 	synchronously with driver and device registration (with the exception of
>> 	-EPROBE_DEFER handling - re-probing always ends up being done
>> 	asynchronously).
>> 
>> However, I'm still not sure how I feel about this, since I consider this to be
>> more like a workaround that just moves things to a "more approprite" async
>> context.
>
> I guess the underlying problem there is that there are at least 3 
> different significant aspects to what "synchronous" can mean:
>
> - literally in the context or device/driver registration as documented. 
> Off-hand I'm not really sure what useful property may be *specific* to 
> those conditions that a driver might rely on, other than for 
> super-special cases like platform_driver_probe().

I'm not worried about this one, as it is already special by not being compatible
with deferred probe. I.e. the caller already has to promise that the
corresponding probe() call will never return -EPROBE_DEFER.

This is a much more error prone requirement than just "don't call this from
async" already.

> - serialised, i.e. probes of multiple devices won't happen concurrently 
> on multiple threads. This is probably the one hiding the most driver 
> bugs, e.g. internal shared/global state without sufficient 
> synchronisation. I guess this falls out as a side-effect of the first 
> condition, but AFAICS it *can* also still provided by deferred probe 
> (given that it's a single work item iterating a list one-by-one)
>
> - in some regular thread context that isn't liable to have issues 
> synchronising against other async_func workers (i.e. the request_module 
> case). Again, deferred can't have a problem here, or it wouldn't have 
> worked properly in general for the last decade.
>
> So it's not that we'd be relying on some dubious "deferred is always 
> synchronous" assumption - AFAICS deferred *can* launch async if the 
> driver permits it - more just ratifying that deferred is still able to 
> effectively honour all the useful properties of PROBE_FORCE_SYNCHRONOUS 
> other than "during registration of the thing".

Yes, and as mentioned above, EPROBE_DEFER has always been a valid path for
PROBE_FORCE_SYNCHRONOUS, which is why I brought it up as a a workaround in the
first place.

(The reason why I still say "workaround" is because nothing actually needs to be
deferred.)

Anyways, as I mentioned...

>> On the other hand, eventually we want everything to work with
>> PROBE_PREFER_ASYNCHRONOUS, so maybe it's also good enough for the time being. 

...plus there are not a lot of PROBE_FORCE_SYNCHRONOUS left anyways.

Do you want to send a patch?

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

[Robin Murphy]

On 2026-03-13 2:39 pm, Danilo Krummrich wrote:
> On Fri Mar 13, 2026 at 2:15 PM CET, Robin Murphy wrote:
>> On 2026-03-12 12:59 pm, Danilo Krummrich wrote:
>>> On Thu Mar 12, 2026 at 1:48 PM CET, Robin Murphy wrote:
>>>> On 2026-03-11 9:09 pm, Danilo Krummrich wrote:
>>>>>    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?
>>>
>>> Yes, I've also mentioned this in [1], including the fact that it technically
>>> even complies with the guarantees given by PROBE_FORCE_SYNCHRONOUS. I.e. the
>>> documentation says:
>>>
>>> 	Use this to annotate drivers that need their probe routines to run
>>> 	synchronously with driver and device registration (with the exception of
>>> 	-EPROBE_DEFER handling - re-probing always ends up being done
>>> 	asynchronously).
>>>
>>> However, I'm still not sure how I feel about this, since I consider this to be
>>> more like a workaround that just moves things to a "more approprite" async
>>> context.
>>
>> I guess the underlying problem there is that there are at least 3
>> different significant aspects to what "synchronous" can mean:
>>
>> - literally in the context or device/driver registration as documented.
>> Off-hand I'm not really sure what useful property may be *specific* to
>> those conditions that a driver might rely on, other than for
>> super-special cases like platform_driver_probe().
> 
> I'm not worried about this one, as it is already special by not being compatible
> with deferred probe. I.e. the caller already has to promise that the
> corresponding probe() call will never return -EPROBE_DEFER.
> 
> This is a much more error prone requirement than just "don't call this from
> async" already.
> 
>> - serialised, i.e. probes of multiple devices won't happen concurrently
>> on multiple threads. This is probably the one hiding the most driver
>> bugs, e.g. internal shared/global state without sufficient
>> synchronisation. I guess this falls out as a side-effect of the first
>> condition, but AFAICS it *can* also still provided by deferred probe
>> (given that it's a single work item iterating a list one-by-one)
>>
>> - in some regular thread context that isn't liable to have issues
>> synchronising against other async_func workers (i.e. the request_module
>> case). Again, deferred can't have a problem here, or it wouldn't have
>> worked properly in general for the last decade.
>>
>> So it's not that we'd be relying on some dubious "deferred is always
>> synchronous" assumption - AFAICS deferred *can* launch async if the
>> driver permits it - more just ratifying that deferred is still able to
>> effectively honour all the useful properties of PROBE_FORCE_SYNCHRONOUS
>> other than "during registration of the thing".
> 
> Yes, and as mentioned above, EPROBE_DEFER has always been a valid path for
> PROBE_FORCE_SYNCHRONOUS, which is why I brought it up as a a workaround in the
> first place.
> 
> (The reason why I still say "workaround" is because nothing actually needs to be
> deferred.)
> 
> Anyways, as I mentioned...
> 
>>> On the other hand, eventually we want everything to work with
>>> PROBE_PREFER_ASYNCHRONOUS, so maybe it's also good enough for the time being.
> 
> ...plus there are not a lot of PROBE_FORCE_SYNCHRONOUS left anyways.
> 
> Do you want to send a patch?

Sure, I just wanted to check that you didn't see anything obviously 
wrong with my reasoning-by-inspection - if you're sufficiently happy 
then I'll write this up as a proper commit message and post the patch 
(likely on Monday now) so we can all nit-pick the details :)

Cheers,
Robin.

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

[Anand Moon]

Hi Robin,

On Fri, 13 Mar 2026 at 23:07, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2026-03-13 2:39 pm, Danilo Krummrich wrote:
> > On Fri Mar 13, 2026 at 2:15 PM CET, Robin Murphy wrote:
> >> On 2026-03-12 12:59 pm, Danilo Krummrich wrote:
> >>> On Thu Mar 12, 2026 at 1:48 PM CET, Robin Murphy wrote:
> >>>> On 2026-03-11 9:09 pm, Danilo Krummrich wrote:
> >>>>>    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?
> >>>
> >>> Yes, I've also mentioned this in [1], including the fact that it technically
> >>> even complies with the guarantees given by PROBE_FORCE_SYNCHRONOUS. I.e. the
> >>> documentation says:
> >>>
> >>>     Use this to annotate drivers that need their probe routines to run
> >>>     synchronously with driver and device registration (with the exception of
> >>>     -EPROBE_DEFER handling - re-probing always ends up being done
> >>>     asynchronously).
> >>>
> >>> However, I'm still not sure how I feel about this, since I consider this to be
> >>> more like a workaround that just moves things to a "more approprite" async
> >>> context.
> >>
> >> I guess the underlying problem there is that there are at least 3
> >> different significant aspects to what "synchronous" can mean:
> >>
> >> - literally in the context or device/driver registration as documented.
> >> Off-hand I'm not really sure what useful property may be *specific* to
> >> those conditions that a driver might rely on, other than for
> >> super-special cases like platform_driver_probe().
> >
> > I'm not worried about this one, as it is already special by not being compatible
> > with deferred probe. I.e. the caller already has to promise that the
> > corresponding probe() call will never return -EPROBE_DEFER.
> >
> > This is a much more error prone requirement than just "don't call this from
> > async" already.
> >
> >> - serialised, i.e. probes of multiple devices won't happen concurrently
> >> on multiple threads. This is probably the one hiding the most driver
> >> bugs, e.g. internal shared/global state without sufficient
> >> synchronisation. I guess this falls out as a side-effect of the first
> >> condition, but AFAICS it *can* also still provided by deferred probe
> >> (given that it's a single work item iterating a list one-by-one)
> >>
> >> - in some regular thread context that isn't liable to have issues
> >> synchronising against other async_func workers (i.e. the request_module
> >> case). Again, deferred can't have a problem here, or it wouldn't have
> >> worked properly in general for the last decade.
> >>
> >> So it's not that we'd be relying on some dubious "deferred is always
> >> synchronous" assumption - AFAICS deferred *can* launch async if the
> >> driver permits it - more just ratifying that deferred is still able to
> >> effectively honour all the useful properties of PROBE_FORCE_SYNCHRONOUS
> >> other than "during registration of the thing".
> >
> > Yes, and as mentioned above, EPROBE_DEFER has always been a valid path for
> > PROBE_FORCE_SYNCHRONOUS, which is why I brought it up as a a workaround in the
> > first place.
> >
> > (The reason why I still say "workaround" is because nothing actually needs to be
> > deferred.)
> >
> > Anyways, as I mentioned...
> >
> >>> On the other hand, eventually we want everything to work with
> >>> PROBE_PREFER_ASYNCHRONOUS, so maybe it's also good enough for the time being.
> >
> > ...plus there are not a lot of PROBE_FORCE_SYNCHRONOUS left anyways.
> >
> > Do you want to send a patch?
>
> Sure, I just wanted to check that you didn't see anything obviously
> wrong with my reasoning-by-inspection - if you're sufficiently happy
> then I'll write this up as a proper commit message and post the patch
> (likely on Monday now) so we can all nit-pick the details :)
>
I believe the issue lies in the r8169 module, which currently has the
PHY address hardcoded to zero in mdiobus_get_phy. I have modified the
implementation to scan the MDIO bus for all possible addresses to improve
hardware compatibility.

Can you check this at your end.

----->8-----
$ git diff drivers/net/ethernet/realtek/r8169_main.c
diff --git a/drivers/net/ethernet/realtek/r8169_main.c
b/drivers/net/ethernet/realtek/r8169_main.c
index 58788d196c57..fa99e33eccfe 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5420,7 +5420,8 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
 {
        struct pci_dev *pdev = tp->pci_dev;
        struct mii_bus *new_bus;
-       int ret;
+       struct phy_device *phydev;
+       int ret, addr;

        /* On some boards with this chip version the BIOS is buggy and misses
         * to reset the PHY page selector. This results in the PHY ID read
@@ -5455,18 +5456,30 @@ static int r8169_mdio_register(struct
rtl8169_private *tp)
        if (ret)
                return ret;

-       tp->phydev = mdiobus_get_phy(new_bus, 0);
-       if (!tp->phydev) {
+       /* find the first (lowest address) PHY on the current MAC's MII bus */
+       for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
+               struct phy_device *tmp = mdiobus_get_phy(new_bus, addr);
+               if (tmp) {
+                       phydev = tmp;
+                       break;
+               }
+       }
+
+       if (!phydev) {
+               dev_err(&pdev->dev, "no PHY found on bus\n");
                return -ENODEV;
-       } else if (!tp->phydev->drv) {
-               /* Most chip versions fail with the genphy driver.
-                * Therefore ensure that the dedicated PHY driver is loaded.
-                */
+       }
+
+       /* Most chip versions fail with the genphy driver.
+        * Therefore ensure that the dedicated PHY driver is loaded.
+        */
+       if (!phydev->drv || phy_driver_is_genphy(phydev)) {
                dev_err(&pdev->dev, "no dedicated PHY driver found for
PHY ID 0x%08x, maybe realtek.ko needs to be added to initramfs?\n",
                        tp->phydev->phy_id);
                return -EUNATCH;
        }

+       tp->phydev = phydev;
        tp->phydev->mac_managed_pm = true;
        if (rtl_supports_eee(tp))
                phy_support_eee(tp->phydev);

> Cheers,
> Robin.

Thanks
-Anand

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

[Anand Moon]

Hi Robin,

On Sat, 14 Mar 2026 at 10:42, Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Robin,
>
> On Fri, 13 Mar 2026 at 23:07, Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > On 2026-03-13 2:39 pm, Danilo Krummrich wrote:
> > > On Fri Mar 13, 2026 at 2:15 PM CET, Robin Murphy wrote:
> > >> On 2026-03-12 12:59 pm, Danilo Krummrich wrote:
> > >>> On Thu Mar 12, 2026 at 1:48 PM CET, Robin Murphy wrote:
> > >>>> On 2026-03-11 9:09 pm, Danilo Krummrich wrote:
> > >>>>>    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?
> > >>>
> > >>> Yes, I've also mentioned this in [1], including the fact that it technically
> > >>> even complies with the guarantees given by PROBE_FORCE_SYNCHRONOUS. I.e. the
> > >>> documentation says:
> > >>>
> > >>>     Use this to annotate drivers that need their probe routines to run
> > >>>     synchronously with driver and device registration (with the exception of
> > >>>     -EPROBE_DEFER handling - re-probing always ends up being done
> > >>>     asynchronously).
> > >>>
> > >>> However, I'm still not sure how I feel about this, since I consider this to be
> > >>> more like a workaround that just moves things to a "more approprite" async
> > >>> context.
> > >>
> > >> I guess the underlying problem there is that there are at least 3
> > >> different significant aspects to what "synchronous" can mean:
> > >>
> > >> - literally in the context or device/driver registration as documented.
> > >> Off-hand I'm not really sure what useful property may be *specific* to
> > >> those conditions that a driver might rely on, other than for
> > >> super-special cases like platform_driver_probe().
> > >
> > > I'm not worried about this one, as it is already special by not being compatible
> > > with deferred probe. I.e. the caller already has to promise that the
> > > corresponding probe() call will never return -EPROBE_DEFER.
> > >
> > > This is a much more error prone requirement than just "don't call this from
> > > async" already.
> > >
> > >> - serialised, i.e. probes of multiple devices won't happen concurrently
> > >> on multiple threads. This is probably the one hiding the most driver
> > >> bugs, e.g. internal shared/global state without sufficient
> > >> synchronisation. I guess this falls out as a side-effect of the first
> > >> condition, but AFAICS it *can* also still provided by deferred probe
> > >> (given that it's a single work item iterating a list one-by-one)
> > >>
> > >> - in some regular thread context that isn't liable to have issues
> > >> synchronising against other async_func workers (i.e. the request_module
> > >> case). Again, deferred can't have a problem here, or it wouldn't have
> > >> worked properly in general for the last decade.
> > >>
> > >> So it's not that we'd be relying on some dubious "deferred is always
> > >> synchronous" assumption - AFAICS deferred *can* launch async if the
> > >> driver permits it - more just ratifying that deferred is still able to
> > >> effectively honour all the useful properties of PROBE_FORCE_SYNCHRONOUS
> > >> other than "during registration of the thing".
> > >
> > > Yes, and as mentioned above, EPROBE_DEFER has always been a valid path for
> > > PROBE_FORCE_SYNCHRONOUS, which is why I brought it up as a a workaround in the
> > > first place.
> > >
> > > (The reason why I still say "workaround" is because nothing actually needs to be
> > > deferred.)
> > >
> > > Anyways, as I mentioned...
> > >
> > >>> On the other hand, eventually we want everything to work with
> > >>> PROBE_PREFER_ASYNCHRONOUS, so maybe it's also good enough for the time being.
> > >
> > > ...plus there are not a lot of PROBE_FORCE_SYNCHRONOUS left anyways.
> > >
> > > Do you want to send a patch?
> >
> > Sure, I just wanted to check that you didn't see anything obviously
> > wrong with my reasoning-by-inspection - if you're sufficiently happy
> > then I'll write this up as a proper commit message and post the patch
> > (likely on Monday now) so we can all nit-pick the details :)
> >
> I believe the issue lies in the r8169 module, which currently has the
> PHY address hardcoded to zero in mdiobus_get_phy. I have modified the
> implementation to scan the MDIO bus for all possible addresses to improve
> hardware compatibility.
>
> Can you check this at your end.
>
> ----->8-----
> $ git diff drivers/net/ethernet/realtek/r8169_main.c
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c
> b/drivers/net/ethernet/realtek/r8169_main.c
> index 58788d196c57..fa99e33eccfe 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5420,7 +5420,8 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>  {
>         struct pci_dev *pdev = tp->pci_dev;
>         struct mii_bus *new_bus;
> -       int ret;
> +       struct phy_device *phydev;
> +       int ret, addr;
>
>         /* On some boards with this chip version the BIOS is buggy and misses
>          * to reset the PHY page selector. This results in the PHY ID read
> @@ -5455,18 +5456,30 @@ static int r8169_mdio_register(struct
> rtl8169_private *tp)
>         if (ret)
>                 return ret;
>
> -       tp->phydev = mdiobus_get_phy(new_bus, 0);
> -       if (!tp->phydev) {
> +       /* find the first (lowest address) PHY on the current MAC's MII bus */
> +       for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
> +               struct phy_device *tmp = mdiobus_get_phy(new_bus, addr);
> +               if (tmp) {
> +                       phydev = tmp;
> +                       break;
> +               }
> +       }
> +
> +       if (!phydev) {
> +               dev_err(&pdev->dev, "no PHY found on bus\n");
>                 return -ENODEV;
> -       } else if (!tp->phydev->drv) {
> -               /* Most chip versions fail with the genphy driver.
> -                * Therefore ensure that the dedicated PHY driver is loaded.
> -                */
> +       }
> +
> +       /* Most chip versions fail with the genphy driver.
> +        * Therefore ensure that the dedicated PHY driver is loaded.
> +        */
> +       if (!phydev->drv || phy_driver_is_genphy(phydev)) {
>                 dev_err(&pdev->dev, "no dedicated PHY driver found for
> PHY ID 0x%08x, maybe realtek.ko needs to be added to initramfs?\n",
>                         tp->phydev->phy_id);
>                 return -EUNATCH;
>         }
>
> +       tp->phydev = phydev;
>         tp->phydev->mac_managed_pm = true;
>         if (rtl_supports_eee(tp))
>                 phy_support_eee(tp->phydev);
>
> > Cheers,
> > Robin.
>
I've submitted a patch to address the issue you reported.
Could you please verify if this resolves it on your side?

[1] https://lore.kernel.org/all/20260317061700.7734-1-linux.amoon@gmail.com/

Thanks
-Anand

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

[Manivannan Sadhasivam]

On Thu, Mar 12, 2026 at 01:59:52PM +0100, Danilo Krummrich wrote:
> On Thu Mar 12, 2026 at 1:48 PM CET, Robin Murphy wrote:
> > On 2026-03-11 9:09 pm, Danilo Krummrich wrote:
> >>  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?
> 
> Yes, I've also mentioned this in [1], including the fact that it technically
> even complies with the guarantees given by PROBE_FORCE_SYNCHRONOUS. I.e. the
> documentation says:
> 
> 	Use this to annotate drivers that need their probe routines to run
> 	synchronously with driver and device registration (with the exception of
> 	-EPROBE_DEFER handling - re-probing always ends up being done
> 	asynchronously).
> 
> However, I'm still not sure how I feel about this, since I consider this to be
> more like a workaround that just moves things to a "more approprite" async
> context.
> 
> On the other hand, eventually we want everything to work with
> PROBE_PREFER_ASYNCHRONOUS, so maybe it's also good enough for the time being.
> 

FWIW, I'd love to handle this corner case in the driver core and not spawn a
workqueue/thread in the bus drivers as we would end up duplicating it all over
defeating the purpose of PROBE_PREFER_ASYNCHRONOUS flag.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [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@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@gmail.com>

Thanks
-Anand

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

[Dmitry Torokhov]

On Thu, Mar 12, 2026 at 12:48:36PM +0000, Robin Murphy 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;
> +	}

That means that you are kicking the majority devices (for now) into
deferral path. I do not think this is optimal.

Does phy really need to request modules synchronously (and on its own)?
Why can't it rely on udev to load the modules and signal when phy
devices are ready?

Seems like a deficiency on PHY subsystem that is stuck in times long
past.

Thanks.

-- 
Dmitry

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

[Robin Murphy]

On 25/03/2026 4:13 am, Dmitry Torokhov wrote:
> On Thu, Mar 12, 2026 at 12:48:36PM +0000, Robin Murphy 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;
>> +	}
> 
> That means that you are kicking the majority devices (for now) into
> deferral path. I do not think this is optimal.

And probing drivers under conditions where they may go wrong or deadlock 
is better? I've not yet had a chance to actually test this myself to see 
the effect on timings, but whatever it might be, I can't imagine any 
*other* method of re-serialising child driver probes could be 
significantly better (or if it could be, that might represent some 
improvement we could make to the deferred probe mechanism in general 
anyway).

I have finally got a bit of time this afternoon to pick this up again, 
so I'll have a play and try to finish the write-up capturing all the 
reasoning so far (it's a long one...)

> Does phy really need to request modules synchronously (and on its own)?
> Why can't it rely on udev to load the modules and signal when phy
> devices are ready?

Getting hung up on what phylib does in this one particular case is 
rather missing the point. There is a reason that we're still not forcing 
async_probe on for everything by default. Many drivers will still not 
have been tested and validated to handle it correctly, and while the 
majority of latent issues will likely just be concurrency bugs which can 
be fixed with better locking or whatever, even then we should be 
encouraging developers to actively test and look for such bugs to make 
their drivers "async probe clean", rather than knowingly enabling bugs 
to surface in the wild as weird and subtle breakage on end-user systems.

However, I imagine there will always remain some small minority of 
PROBE_FORCE_SYNCHRONOUS drivers - either for niche legitimate technical 
reasons, or just legacy drivers where updating them would take more 
effort than it's worth - so the driver core surely needs the ability to 
not do the wrong thing itself. That doesn't even need to be "optimal", 
it just needs to be functionally correct.

Thanks,
Robin.

> 
> Seems like a deficiency on PHY subsystem that is stuck in times long
> past.
> 
> Thanks.
> 

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

[Dmitry Torokhov]

On Wed, Mar 25, 2026 at 03:01:03PM +0000, Robin Murphy wrote:
> On 25/03/2026 4:13 am, Dmitry Torokhov wrote:
> > On Thu, Mar 12, 2026 at 12:48:36PM +0000, Robin Murphy 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;
> > > +	}
> > 
> > That means that you are kicking the majority devices (for now) into
> > deferral path. I do not think this is optimal.
> 
> And probing drivers under conditions where they may go wrong or deadlock is
> better? I've not yet had a chance to actually test this myself to see the
> effect on timings, but whatever it might be, I can't imagine any *other*
> method of re-serialising child driver probes could be significantly better
> (or if it could be, that might represent some improvement we could make to
> the deferred probe mechanism in general anyway).

There is serious disconnect as to what asynchronous and synchronous
probing means. You do not go "back" to synchronous probing.

	/*
	 * Indicates whether we are considering asynchronous probing or
	 * not. Only initial binding after device or driver registration
	 * (including deferral processing) may be done asynchronously, the
	 * rest is always synchronous, as we expect it is being done by
	 * request from userspace.
	 */
	bool check_async;

So synchronous means that when you register device or driver we dun
probe immediately on the same thread that registered said device or
driver, while with async we _may_ schedule on a workqueue (but if the
system has already scheduled too much async it will be probed
synchronously or immediately).

So in the case we are discussing the probing of phy device is indeed
synchronous/immediate relative to the thread where the phy device is
registered.

> 
> I have finally got a bit of time this afternoon to pick this up again, so
> I'll have a play and try to finish the write-up capturing all the reasoning
> so far (it's a long one...)
> 
> > Does phy really need to request modules synchronously (and on its own)?
> > Why can't it rely on udev to load the modules and signal when phy
> > devices are ready?
> 
> Getting hung up on what phylib does in this one particular case is rather
> missing the point. There is a reason that we're still not forcing
> async_probe on for everything by default. Many drivers will still not have
> been tested and validated to handle it correctly, and while the majority of
> latent issues will likely just be concurrency bugs which can be fixed with
> better locking or whatever, even then we should be encouraging developers to
> actively test and look for such bugs to make their drivers "async probe
> clean", rather than knowingly enabling bugs to surface in the wild as weird
> and subtle breakage on end-user systems.

No. We have identified (at least IMO) that what phylib does is wrong. I
am not advocating forcing this PCI host to be marked for asycn probing
right now, before the phy issue is fixed, but I object to adding
workarounds to driver core which hurt other cases and may lead to phylib
never be fixed.

> 
> However, I imagine there will always remain some small minority of
> PROBE_FORCE_SYNCHRONOUS drivers - either for niche legitimate technical
> reasons, or just legacy drivers where updating them would take more effort
> than it's worth - so the driver core surely needs the ability to not do the
> wrong thing itself. That doesn't even need to be "optimal", it just needs to
> be functionally correct.

Again, your expectation of what sync probing means differs from what I
had in mind when implementing it. PROBE_FORCE_SYNCHRONOUS is typically
for drivers/modules that register both a device (typically a singleton)
and a driver, and want to unregister both and free all resources when
initialization fails for some reason. Think cases like PC keyboard
controller.

Thanks.

-- 
Dmitry

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

[Danilo Krummrich]

On Wed Mar 25, 2026 at 5:13 AM CET, Dmitry Torokhov wrote:
> That means that you are kicking the majority devices (for now) into
> deferral path. I do not think this is optimal.

That's not necessary, we'd only need to kick those into the deferral path that
have PROBE_FORCE_SYNCHRONOUS, no?

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

[Dmitry Torokhov]

On Wed, Mar 25, 2026 at 04:13:03PM +0100, Danilo Krummrich wrote:
> On Wed Mar 25, 2026 at 5:13 AM CET, Dmitry Torokhov wrote:
> > That means that you are kicking the majority devices (for now) into
> > deferral path. I do not think this is optimal.
> 
> That's not necessary, we'd only need to kick those into the deferral path that
> have PROBE_FORCE_SYNCHRONOUS, no?

Yes, it may limit the fallout, but as I explained in my other email
there seem to be a serious disconnect on what synchronous and
asynchronous probing mean.

I wonder, don't we get issues with phylib in other cases? Don't we ever
have a module resulting in creating a phy device triggering loading
another module on the same thread? Won't that result in a warning also?

Thanks.

-- 
Dmitry

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

[Anand Moon]

Hi Dmitry/ Robin,

On Wed, 25 Mar 2026 at 20:57, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> On Wed, Mar 25, 2026 at 04:13:03PM +0100, Danilo Krummrich wrote:
> > On Wed Mar 25, 2026 at 5:13 AM CET, Dmitry Torokhov wrote:
> > > That means that you are kicking the majority devices (for now) into
> > > deferral path. I do not think this is optimal.
> >
> > That's not necessary, we'd only need to kick those into the deferral path that
> > have PROBE_FORCE_SYNCHRONOUS, no?
>
> Yes, it may limit the fallout, but as I explained in my other email
> there seem to be a serious disconnect on what synchronous and
> asynchronous probing mean.
>
> I wonder, don't we get issues with phylib in other cases? Don't we ever
> have a module resulting in creating a phy device triggering loading
> another module on the same thread? Won't that result in a warning also?
>
> Thanks.
>
> --
> Dmitry

In comparing the Orange Pi 5 Plus and Radxa Rock 5B schematics,
I noticed a key difference in regulator implementation for pcie2x1l2 node.
While the Orange Pi uses a dedicated GPIO (GPIO3_B4) for ethernet_power_en,

Reference: orangepi‑5‑plus schematics
[1] https://drive.google.com/file/d/1cVT7lSLlgtIvHh4PllijP5E4zz7pxCyh/view

On the Rock 5B ties the LAN_EXT_CORE regulator directly to the VCC_3V3_S3
PMIC rail without a control GPIO.

For LAN_EXT_CORE regulator POW_EXT_SWR/ENSWREG is defined
as enabled by this regulator, but with no GPIO pin linked to toggle.

Reference: Radxa Rock 5B schematics
[2] https://dl.radxa.com/rock5/5b+/docs/hw/radxa_rock5bp_v1.2_schematic.pdf

Because the Rock 5B lacks that GPIO toggle, the existing dummy regulator
definition is redundant.

By stripping out the dummy node and pointing the PCIe supply
directly to the PMIC rail,

If somebody could verify these DTS changes

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi
b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi
index 668b19c05f7e..cf7d9d516d0e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi
@@ -81,15 +81,6 @@ vcc3v3_pcie2x1l0: regulator-vcc3v3-pcie2x1l0 {
                status = "disabled";
        };

-       vcc3v3_pcie2x1l2: regulator-vcc3v3-pcie2x1l2 {
-               compatible = "regulator-fixed";
-               regulator-name = "vcc3v3_pcie2x1l2";
-               regulator-min-microvolt = <3300000>;
-               regulator-max-microvolt = <3300000>;
-               startup-delay-us = <5000>;
-               vin-supply = <&vcc_3v3_s3>;
-       };
-
        vcc3v3_pcie30: regulator-vcc3v3-pcie30 {
                compatible = "regulator-fixed";
                enable-active-high;
@@ -480,7 +471,7 @@ &pcie2x1l2 {
        pinctrl-0 = <&pcie2_2_rst>, <&pcie20x1m0_clkreqn>;
        supports-clkreq;
        reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_HIGH>;
-       vpcie3v3-supply = <&vcc3v3_pcie2x1l2>;
+       vpcie3v3-supply = <&vcc_3v3_s3>;
        status = "okay";
 };

Thanks
-Anand