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

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

[Anand Moon]

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.

Cc: Grimmauld <grimmauld@grimmauld.de>
Cc: Niklas Cassel <cassel@kernel.org>
Tested-by: Grimmauld <grimmauld@grimmauld.de>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v3: update the commit message to describe the changs.
    added tested by Grimmauld.
    https://lore.kernel.org/all/20240809073610.2517-1-linux.amoon@gmail.com/
v2: update the commit message to describe the changs.
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 5b17da63151d5..c31e0e9848327 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_PREFER_ASYNCHRONOUS,
 	},
 	.probe = rockchip_pcie_probe,
 };

base-commit: f4d0ec0aa20d49f09dc01d82894ce80d72de0560
-- 
2.50.1

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

[Niklas Cassel]

On Thu, Feb 26, 2026 at 03:40:23PM +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.
> 
> Cc: Grimmauld <grimmauld@grimmauld.de>
> Cc: Niklas Cassel <cassel@kernel.org>
> Tested-by: Grimmauld <grimmauld@grimmauld.de>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>

Reviewed-by: Niklas Cassel <cassel@kernel.org>

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

[Hans Zhang]

On 2026/2/26 18:10, 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.
> 
> Cc: Grimmauld <grimmauld@grimmauld.de>
> Cc: Niklas Cassel <cassel@kernel.org>
> Tested-by: Grimmauld <grimmauld@grimmauld.de>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>

Reviewed-by: Hans Zhang <18255117159@163.com>

> ---
> v3: update the commit message to describe the changs.
>      added tested by Grimmauld.
>      https://lore.kernel.org/all/20240809073610.2517-1-linux.amoon@gmail.com/
> v2: update the commit message to describe the changs.
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 5b17da63151d5..c31e0e9848327 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_PREFER_ASYNCHRONOUS,
>   	},
>   	.probe = rockchip_pcie_probe,
>   };
> 
> base-commit: f4d0ec0aa20d49f09dc01d82894ce80d72de0560

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

[Shawn Lin]

在 2026/02/26 星期四 18:10, Anand Moon 写道:
> 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.

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> 
> Cc: Grimmauld <grimmauld@grimmauld.de>
> Cc: Niklas Cassel <cassel@kernel.org>
> Tested-by: Grimmauld <grimmauld@grimmauld.de>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> v3: update the commit message to describe the changs.
>      added tested by Grimmauld.
>      https://lore.kernel.org/all/20240809073610.2517-1-linux.amoon@gmail.com/
> v2: update the commit message to describe the changs.
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 5b17da63151d5..c31e0e9848327 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_PREFER_ASYNCHRONOUS,
>   	},
>   	.probe = rockchip_pcie_probe,
>   };
> 
> base-commit: f4d0ec0aa20d49f09dc01d82894ce80d72de0560
> 

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

[Manivannan Sadhasivam]

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

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>

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

[Robin Murphy]

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.

(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".)

Thanks,
Robin.


1467 00:47:33.141229  <4>[   18.243373] ------------[ cut here ]------------
1468 00:47:33.141490  <4>[   18.243803] WARNING: kernel/module/kmod.c:144 at __request_module+0x198/0x200, CPU#4: kworker/u32:5/101
1469 00:47:33.142056  <4>[   18.244653] Modules linked in: r8169 snd_soc_hdmi_codec rfkill_gpio rfkill pwm_fan display_connector snd_soc_simple_card rtc_hym8563 rk805_pwrkey snd_soc_es8316 industrialio_triggered_buffer typec phy_rockchip_samsung_hdptx rockchip_thermal kfifo_buf phy_rockchip_naneng_combphy spi_rockchip_sfc rockchip_dfi synopsys_hdmirx v4l2_dv_timings snd_soc_rockchip_i2s_tdm hantro_vpu v4l2_vp9 v4l2_h264 v4l2_jpeg videobuf2_dma_contig rockchip_rga videobuf2_dma_sg v4l2_mem2mem videobuf2_memops videobuf2_v4l2 videodev videobuf2_common mc pci_endpoint_test rockchipdrm dw_hdmi_qp analogix_dp inno_hdmi dw_dp dw_mipi_dsi drm_dp_aux_bus dw_hdmi panthor drm_display_helper drm_gpuvm drm_exec cec drm_shmem_helper drm_client_lib gpu_sched drm_dma_helper drm_kms_helper snd_soc_audio_graph_card snd_soc_simple_card_utils snd_soc_core snd_compress snd_pcm_dmaengine snd_pcm drm snd_timer snd backlight soundcore dm_mod ipv6
1470 00:47:33.142376  <4>[   18.251632] CPU: 4 UID: 0 PID: 101 Comm: kworker/u32:5 Not tainted 7.0.0-rc3-next-20260309 #1 PREEMPT
1471 00:47:33.142648  <4>[   18.252465] Hardware name: Radxa ROCK 5B (DT)
1472 00:47:33.142915  <4>[   18.252866] Workqueue: async async_run_entry_fn
1473 00:47:33.143182  <4>[   18.253291] pstate: 00400009 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
1474 00:47:33.143450  <4>[   18.253920] pc : __request_module+0x198/0x200
1475 00:47:33.143714  <4>[   18.254325] lr : __request_module+0x190/0x200
1476 00:47:33.143979  <4>[   18.254730] sp : ffff800084233420
1477 00:47:33.144239  <4>[   18.255042] x29: ffff800084233420 x28: ffff000116886000 x27: ffff000116887000
1478 00:47:33.144524  <4>[   18.255687] x26: ffff0001168870d0 x25: ffff80007b6c1738 x24: 0000000000000000
1479 00:47:33.172973  <4>[   18.256333] x23: ffff000116886000 x22: 0000000000000000 x21: ffff00011b80f800
1480 00:47:33.173352  <4>[   18.256979] x20: 0000000000000001 x19: ffff800081e2bfd8 x18: 0000000000000002
1481 00:47:33.173658  <4>[   18.257625] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
1482 00:47:33.173946  <4>[   18.258270] x14: 0000000000000001 x13: 0000000000000000 x12: 0000000000000000
1483 00:47:33.174228  <4>[   18.258916] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
1484 00:47:33.174507  <4>[   18.259561] x8 : ffff8000842334c8 x7 : 0000000000000000 x6 : 0000000000000000
1485 00:47:33.174779  <4>[   18.260206] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000030
1486 00:47:33.175050  <4>[   18.260851] x2 : 0000000000000008 x1 : ffff8000800df110 x0 : 0000000000000001
1487 00:47:33.175318  <4>[   18.261497] Call trace:
1488 00:47:33.175584  <4>[   18.261735]  __request_module+0x198/0x200 (P)
1489 00:47:33.175851  <4>[   18.262141]  phy_request_driver_module+0x11c/0x17c
1490 00:47:33.176116  <4>[   18.262584]  phy_device_create+0x234/0x260
1491 00:47:33.176381  <4>[   18.262967]  get_phy_device+0x74/0x150
1492 00:47:33.176674  <4>[   18.263319]  mdiobus_scan+0x34/0x114
1493 00:47:33.176938  <4>[   18.263657]  __mdiobus_register+0x1e4/0x430
1494 00:47:33.177201  <4>[   18.264047]  __devm_mdiobus_register+0x70/0xe8
1495 00:47:33.177465  <4>[   18.264460]  rtl_init_one+0x95c/0x1040 [r8169]
1496 00:47:33.177730  <4>[   18.264884]  local_pci_probe+0x40/0xa8
1497 00:47:33.177993  <4>[   18.265239]  pci_device_probe+0xd0/0x21c
1498 00:47:33.178255  <4>[   18.265609]  really_probe+0xbc/0x298
1499 00:47:33.178516  <4>[   18.265948]  __driver_probe_device+0x78/0x12c
1500 00:47:33.178778  <4>[   18.266353]  driver_probe_device+0x3c/0x15c
1501 00:47:33.179040  <4>[   18.266742]  __device_attach_driver+0xb8/0x134
1502 00:47:33.179302  <4>[   18.267155]  bus_for_each_drv+0x84/0xe0
1503 00:47:33.179561  <4>[   18.267514]  __device_attach+0x9c/0x188
1504 00:47:33.179821  <4>[   18.267874]  device_initial_probe+0x50/0x54
1505 00:47:33.180082  <4>[   18.268264]  pci_bus_add_device+0x74/0x100
1506 00:47:33.180343  <4>[   18.268647]  pci_bus_add_devices+0x38/0x84
1507 00:47:33.180623  <4>[   18.269030]  pci_bus_add_devices+0x64/0x84
1508 00:47:33.180883  <4>[   18.269412]  pci_host_probe+0x90/0x108
1509 00:47:33.181145  <4>[   18.269766]  dw_pcie_host_init+0x3fc/0x5bc
1510 00:47:33.181407  <4>[   18.270148]  rockchip_pcie_probe+0x354/0x380
1511 00:47:33.181668  <4>[   18.270546]  platform_probe+0x5c/0x98
1512 00:47:33.181933  <4>[   18.270892]  really_probe+0xbc/0x298
1513 00:47:33.182197  <4>[   18.271229]  __driver_probe_device+0x78/0x12c
1514 00:47:33.182460  <4>[   18.271634]  driver_probe_device+0x3c/0x15c
1515 00:47:33.182721  <4>[   18.272025]  __device_attach_driver+0xb8/0x134
1516 00:47:33.182981  <4>[   18.272438]  bus_for_each_drv+0x84/0xe0
1517 00:47:33.183241  <4>[   18.272798]  __device_attach_async_helper+0xac/0xd0
1518 00:47:33.183502  <4>[   18.273247]  async_run_entry_fn+0x34/0xe0
1519 00:47:33.183763  <4>[   18.273622]  process_one_work+0x158/0x29c
1520 00:47:33.184023  <4>[   18.273997]  worker_thread+0x184/0x300
1521 00:47:33.184284  <4>[   18.274348]  kthread+0x11c/0x128
1522 00:47:33.184560  <4>[   18.274656]  ret_from_fork+0x10/0x20
1523 00:47:33.184823  <4>[   18.274995] ---[ end trace 0000000000000000 ]---

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

[Manivannan Sadhasivam]

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.

- Mani

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

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

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

[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

[Manivannan Sadhasivam]

On Wed, Mar 04, 2026 at 12:18:59PM +0530, 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
> 

Dropped the patch from controller/dwc-rockchip branch due to concerns with the
potential deadlock in the phylib. When Bjorn updates the pci/next branch, this
will get dropped from linux-next as well.

- 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

[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

[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

[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-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

[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

[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

[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

[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

[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

[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

[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 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

[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