Skip to contentSkip to conversation

PCI: dw-rockchip: Enable async probe by default

PATCH Superseded Anand Moon 3 participants 5 messages Jun 25, 2024 – Aug 8, 2024

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Anand Moon

author
#0 lore raw

PATCH 1/1 PCI: dw-rockchip: Enable async probe by default

Rockchip PCIe driver lets waits for the combo PHY link like PCIe 3.0, PCIe 2.0 and SATA 3.0 controller to be up during the probe this consumes several milliseconds during boot.

Establishing a PCIe link can take a while; allow asynchronous probing so that link establishment can happen in the background while other devices are being probed.

---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 1 +
 1 file changed, 1 insertion(+)
drivers/pci/controller/dwc/pcie-dw-rockchip.c +1 -0
index 61b1acba7182..74a3e9d172a0 100644
@@ -367,6 +367,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,
 };
Signature · 1 line
2.44.0
author
#1 lore raw

Hi All,

Quoted 10 lines · Anand Moon

Rockchip PCIe driver lets waits for the combo PHY link like PCIe 3.0, PCIe 2.0 and SATA 3.0 controller to be up during the probe this consumes several milliseconds during boot.

Establishing a PCIe link can take a while; allow asynchronous probing so that link establishment can happen in the background while other devices are being probed.

Gentle ping.

Thanks
-Anand

#2 lore raw
In reply to Anand Moon
Quoted 3 lines · Anand Moon

Rockchip PCIe driver lets waits for the combo PHY link like PCIe 3.0, PCIe 2.0 and SATA 3.0 controller to be up during the probe this consumes several milliseconds during boot.

This needs some wordsmithing. "driver lets waits" ... I guess "lets" is not supposed to be there? I'm not sure what the relevance of "PCIe 3.0, PCIe 2.0, SATA 3.0" is. I assume the host controller driver doesn't know what downstream devices might be present, and the async probing is desirable no matter what they might be?

Quoted 24 lines

Establishing a PCIe link can take a while; allow asynchronous probing so that link establishment can happen in the background while other devices are being probed.

---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 1 +
 1 file changed, 1 insertion(+)
drivers/pci/controller/dwc/pcie-dw-rockchip.c +1 -0
index 61b1acba7182..74a3e9d172a0 100644
@@ -367,6 +367,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,
 };
Signature · 1 line
2.44.0
#3 lore raw
Quoted 11 lines · Bjorn Helgaas
Quoted 3 lines · Anand Moon

Rockchip PCIe driver lets waits for the combo PHY link like PCIe 3.0, PCIe 2.0 and SATA 3.0 controller to be up during the probe this consumes several milliseconds during boot.

This needs some wordsmithing. "driver lets waits" ... I guess "lets" is not supposed to be there? I'm not sure what the relevance of "PCIe 3.0, PCIe 2.0, SATA 3.0" is. I assume the host controller driver doesn't know what downstream devices might be present, and the async probing is desirable no matter what they might be?

Since the DWC driver is enabling link training during boot, it also waits for the link to be 'up'. But if the device is 'up', then the wait time would be usually negligible (few ms). But if there is no device, then the wait time of 1s would be evident.

But here the patch is trying to avoid the few ms delay itself (which is fine). The type of endpoint might have some impact on the link training also. But async probe is always preferred.

- Mani

Quoted 25 lines
Quoted 24 lines

Establishing a PCIe link can take a while; allow asynchronous probing so that link establishment can happen in the background while other devices are being probed.

---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 1 +
 1 file changed, 1 insertion(+)
drivers/pci/controller/dwc/pcie-dw-rockchip.c +1 -0
index 61b1acba7182..74a3e9d172a0 100644
@@ -367,6 +367,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,
 };
Signature · 1 line
2.44.0
Signature · 1 line
மணிவண்ணன் சதாசிவம்
author
#4 lore raw

Hi Manivannan, Bjorn

Quoted 7 lines · Manivannan Sadhasivam
Quoted 5 lines · Bjorn Helgaas
Quoted 3 lines · Anand Moon

Rockchip PCIe driver lets waits for the combo PHY link like PCIe 3.0, PCIe 2.0 and SATA 3.0 controller to be up during the probe this consumes several milliseconds during boot.

Quoted 6 lines
Quoted 6 lines

This needs some wordsmithing. "driver lets waits" ... I guess "lets" is not supposed to be there? I'm not sure what the relevance of "PCIe 3.0, PCIe 2.0, SATA 3.0" is. I assume the host controller driver doesn't know what downstream devices might be present, and the async probing is desirable no matter what they might be?

Ok I will improve this commit message.

Rockchip DWC PCIe driver currently waits for the combo PHY link (PCIe 3.0, PCIe 2.0, and SATA 3.0) to be established during the probe, which could consume several milliseconds during boot.
To optimize boot time, this commit allows asynchronous probing. This change enables the PCIe link establishment to occur in the background while other devices are being probed.

Quoted 6 lines

Since the DWC driver is enabling link training during boot, it also waits for the link to be 'up'. But if the device is 'up', then the wait time would be usually negligible (few ms). But if there is no device, then the wait time of 1s would be evident.

Quoted 6 lines

But here the patch is trying to avoid the few ms delay itself (which is fine). The type of endpoint might have some impact on the link training also. But async probe is always preferred.

- Mani

Ok,

Thanks
-Anand