* [PATCH v2 0/5] PCI: dwc: Do not enumerate bus before endpoint devices are ready
@ 2025-06-12 11:49 Niklas Cassel
2025-06-12 11:49 ` [PATCH v2 1/5] PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ Niklas Cassel
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Niklas Cassel @ 2025-06-12 11:49 UTC (permalink / raw)
To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Heiko Stuebner, Shawn Lin, Kever Yang, Simon Xue,
Stanimir Varbanov
Cc: Wilfred Mallawa, Damien Le Moal, Laszlo Fiat, Niklas Cassel,
linux-pci, linux-arm-kernel, linux-rockchip, linux-arm-msm
Hello all,
The DWC PCIe controller driver currently does not follow the PCIe
specification with regards to the delays after link training, before
sending out configuration requests. This series fixes this.
At the same time, PATCH 1/4 addresses a regression where a Plextor
NVMe drive fails to be configured correctly. With this series, the
Plextor NVMe drive works once again.
Kind regards,
Niklas
Changes since v1:
-Put msleep() before the dev_dbg() that says "Enumerating bus" (Damien)
-Rewrite comment above LINK_WAIT_MAX_RETRIES / LINK_WAIT_SLEEP_MS (Damien)
-Remove comments above PCIE_RESET_CONFIG_DEVICE_WAIT_MS (Bjorn)
-Use different Fixes-tags (Bjorn)
-Shamelessly took Bjorn's commit message for patch 1 and 2 (Bjorn)
-Use PCIE_RESET_CONFIG_DEVICE_WAIT_MS rather than PCIE_T_RRS_READY_MS
-Add new patch (5/5) that drops PCIE_T_RRS_READY_MS
Niklas Cassel (5):
PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up
IRQ
PCI: qcom: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ
PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link
up
PCI: dwc: Reduce LINK_WAIT_SLEEP_MS
PCI: rockchip-host: Use macro PCIE_RESET_CONFIG_DEVICE_WAIT_MS
drivers/pci/controller/dwc/pcie-designware.c | 13 ++++++++++++-
drivers/pci/controller/dwc/pcie-designware.h | 13 +++++++++----
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 1 +
drivers/pci/controller/dwc/pcie-qcom.c | 1 +
drivers/pci/controller/pcie-rockchip-host.c | 2 +-
drivers/pci/pci.h | 7 -------
6 files changed, 24 insertions(+), 13 deletions(-)
--
2.49.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/5] PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ
2025-06-12 11:49 [PATCH v2 0/5] PCI: dwc: Do not enumerate bus before endpoint devices are ready Niklas Cassel
@ 2025-06-12 11:49 ` Niklas Cassel
2025-06-12 22:33 ` Wilfred Mallawa
2025-06-12 11:49 ` [PATCH v2 2/5] PCI: qcom: " Niklas Cassel
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Niklas Cassel @ 2025-06-12 11:49 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Simon Xue, Kever Yang, Shawn Lin
Cc: Wilfred Mallawa, Damien Le Moal, Laszlo Fiat, Niklas Cassel,
linux-pci, linux-arm-kernel, linux-rockchip
Per PCIe r6.0, sec 6.6.1, software must generally wait a minimum of
100ms (PCIE_RESET_CONFIG_DEVICE_WAIT_MS) after Link training completes
before sending a Configuration Request.
Prior to ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since
we can detect Link Up"), dw-rockchip used dw_pcie_wait_for_link(),
which waited between 0 and 90ms after the link came up before we
enumerate the bus, and this was apparently enough for most devices.
After ec9fd499b9c6, rockchip_pcie_rc_sys_irq_thread() started
enumeration immediately when handling the link-up IRQ, and devices
(e.g., Laszlo Fiat's PLEXTOR PX-256M8PeGN NVMe SSD) may not be ready
to handle config requests yet.
Delay PCIE_RESET_CONFIG_DEVICE_WAIT_MS after the link-up IRQ before
starting enumeration.
Cc: Laszlo Fiat <laszlo.fiat@proton.me>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Fixes: 0e898eb8df4e ("PCI: rockchip-dwc: Add Rockchip RK356X host controller driver")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
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 93171a392879..8a068fd4f867 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -458,6 +458,7 @@ static irqreturn_t rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
if (reg & PCIE_RDLH_LINK_UP_CHGED) {
if (rockchip_pcie_link_up(pci)) {
+ msleep(PCIE_RESET_CONFIG_DEVICE_WAIT_MS);
dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
/* Rescan the bus to enumerate endpoint devices */
pci_lock_rescan_remove();
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] PCI: qcom: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ
2025-06-12 11:49 [PATCH v2 0/5] PCI: dwc: Do not enumerate bus before endpoint devices are ready Niklas Cassel
2025-06-12 11:49 ` [PATCH v2 1/5] PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ Niklas Cassel
@ 2025-06-12 11:49 ` Niklas Cassel
2025-06-12 22:35 ` Wilfred Mallawa
2025-06-12 11:49 ` [PATCH v2 3/5] PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link up Niklas Cassel
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Niklas Cassel @ 2025-06-12 11:49 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Stanimir Varbanov
Cc: Wilfred Mallawa, Damien Le Moal, Laszlo Fiat, Niklas Cassel,
linux-arm-msm, linux-pci
Per PCIe r6.0, sec 6.6.1, software must generally wait a minimum of
100ms (PCIE_RESET_CONFIG_DEVICE_WAIT_MS) after Link training completes
before sending a Configuration Request.
Prior to 36971d6c5a9a ("PCI: qcom: Don't wait for link if we can detect
Link Up"), qcom used dw_pcie_wait_for_link(), which waited between 0
and 90ms after the link came up before we enumerate the bus, and this
was apparently enough for most devices.
After 36971d6c5a9a, qcom_pcie_global_irq_thread() started enumeration
immediately when handling the link-up IRQ, and devices (e.g., Laszlo
Fiat's PLEXTOR PX-256M8PeGN NVMe SSD) may not be ready to handle config
requests yet.
Delay PCIE_RESET_CONFIG_DEVICE_WAIT_MS after the link-up IRQ before
starting enumeration.
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
drivers/pci/controller/dwc/pcie-qcom.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index c789e3f85655..ff257fec6ad7 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1564,6 +1564,7 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
writel_relaxed(status, pcie->parf + PARF_INT_ALL_CLEAR);
if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
+ msleep(PCIE_RESET_CONFIG_DEVICE_WAIT_MS);
dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
/* Rescan the bus to enumerate endpoint devices */
pci_lock_rescan_remove();
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/5] PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link up
2025-06-12 11:49 [PATCH v2 0/5] PCI: dwc: Do not enumerate bus before endpoint devices are ready Niklas Cassel
2025-06-12 11:49 ` [PATCH v2 1/5] PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ Niklas Cassel
2025-06-12 11:49 ` [PATCH v2 2/5] PCI: qcom: " Niklas Cassel
@ 2025-06-12 11:49 ` Niklas Cassel
2025-06-12 22:36 ` Wilfred Mallawa
2025-06-12 11:49 ` [PATCH v2 4/5] PCI: dwc: Reduce LINK_WAIT_SLEEP_MS Niklas Cassel
2025-06-12 11:49 ` [PATCH v2 5/5] PCI: rockchip-host: Use macro PCIE_RESET_CONFIG_DEVICE_WAIT_MS Niklas Cassel
4 siblings, 1 reply; 11+ messages in thread
From: Niklas Cassel @ 2025-06-12 11:49 UTC (permalink / raw)
To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
Cc: Wilfred Mallawa, Damien Le Moal, Laszlo Fiat, Niklas Cassel,
linux-pci
As per PCIe r6.0, sec 6.6.1, a Downstream Port that supports Link speeds
greater than 5.0 GT/s, software must wait a minimum of 100 ms after Link
training completes before sending a Configuration Request.
Add this delay in dw_pcie_wait_for_link(), after the link is reported as
up. The delay will only be performed in the success case where the link
came up.
DWC glue drivers that have a link up IRQ (drivers that set
use_linkup_irq = true) do not call dw_pcie_wait_for_link(), instead they
perform this delay in their threaded link up IRQ handler.
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
drivers/pci/controller/dwc/pcie-designware.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 4d794964fa0f..7fd3e926c48d 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -714,6 +714,13 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
return -ETIMEDOUT;
}
+ /*
+ * As per PCIe r6.0, sec 6.6.1, a Downstream Port that supports Link
+ * speeds greater than 5.0 GT/s, software must wait a minimum of 100 ms
+ * after Link training completes before sending a Configuration Request.
+ */
+ msleep(PCIE_RESET_CONFIG_DEVICE_WAIT_MS);
+
offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/5] PCI: dwc: Reduce LINK_WAIT_SLEEP_MS
2025-06-12 11:49 [PATCH v2 0/5] PCI: dwc: Do not enumerate bus before endpoint devices are ready Niklas Cassel
` (2 preceding siblings ...)
2025-06-12 11:49 ` [PATCH v2 3/5] PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link up Niklas Cassel
@ 2025-06-12 11:49 ` Niklas Cassel
2025-06-12 22:37 ` Wilfred Mallawa
2025-06-12 11:49 ` [PATCH v2 5/5] PCI: rockchip-host: Use macro PCIE_RESET_CONFIG_DEVICE_WAIT_MS Niklas Cassel
4 siblings, 1 reply; 11+ messages in thread
From: Niklas Cassel @ 2025-06-12 11:49 UTC (permalink / raw)
To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
Cc: Wilfred Mallawa, Damien Le Moal, Laszlo Fiat, Niklas Cassel,
linux-pci
There is no reason for the delay, in each loop iteration, while polling for
link up (LINK_WAIT_SLEEP_MS), to be so long as 90 ms.
PCIe r6.0, sec 6.6.1, still require us to wait for up to 1.0 s for the link
to come up, thus the number of retries (LINK_WAIT_MAX_RETRIES) is increased
to keep the total timeout to 1.0 s.
PCIe r6.0, sec 6.6.1, also mandates that there is a 100 ms delay, after the
link has been established, before performing configuration requests (this
delay already exists in dw_pcie_wait_for_link() and is unchanged).
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
drivers/pci/controller/dwc/pcie-designware.c | 6 +++++-
drivers/pci/controller/dwc/pcie-designware.h | 13 +++++++++----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 7fd3e926c48d..5dab3d668ab3 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -701,7 +701,11 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
u32 offset, val;
int retries;
- /* Check if the link is up or not */
+ /*
+ * Check if the link is up or not. As per PCIe r6.0, sec 6.6.1, software
+ * must allow at least 1.0 s following exit from a Conventional Reset of
+ * a device, before determining that the device is broken.
+ */
for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
if (dw_pcie_link_up(pci))
break;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ce9e18554e42..b225c4f3d36a 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -62,11 +62,16 @@
#define dw_pcie_cap_set(_pci, _cap) \
set_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES 10
-#define LINK_WAIT_SLEEP_MS 90
+/*
+ * Parameters for waiting for a link to be established. As per PCIe r6.0,
+ * sec 6.6.1, software must allow at least 1.0 s following exit from a
+ * Conventional Reset of a device, before determining that the device is broken.
+ * Therefore LINK_WAIT_MAX_RETRIES * LINK_WAIT_SLEEP_MS should equal 1.0 s.
+ */
+#define LINK_WAIT_MAX_RETRIES 100
+#define LINK_WAIT_SLEEP_MS 10
-/* Parameters for the waiting for iATU enabled routine */
+/* Parameters for waiting for iATU enabled routine */
#define LINK_WAIT_MAX_IATU_RETRIES 5
#define LINK_WAIT_IATU 9
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] PCI: rockchip-host: Use macro PCIE_RESET_CONFIG_DEVICE_WAIT_MS
2025-06-12 11:49 [PATCH v2 0/5] PCI: dwc: Do not enumerate bus before endpoint devices are ready Niklas Cassel
` (3 preceding siblings ...)
2025-06-12 11:49 ` [PATCH v2 4/5] PCI: dwc: Reduce LINK_WAIT_SLEEP_MS Niklas Cassel
@ 2025-06-12 11:49 ` Niklas Cassel
2025-06-12 22:39 ` Wilfred Mallawa
4 siblings, 1 reply; 11+ messages in thread
From: Niklas Cassel @ 2025-06-12 11:49 UTC (permalink / raw)
To: Shawn Lin, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner
Cc: Wilfred Mallawa, Damien Le Moal, Laszlo Fiat, Niklas Cassel,
linux-pci, linux-rockchip, linux-arm-kernel
Macro PCIE_T_RRS_READY_MS was added to pci.h in commit 70a7bfb1e515 ("PCI:
rockchip-host: Wait 100ms after reset before starting configuration").
Later, in commit d5ceb9496c56 ("PCI: Add PCIE_RESET_CONFIG_DEVICE_WAIT_MS
waiting time value"), PCIE_RESET_CONFIG_DEVICE_WAIT_MS was added to pci.h.
These macros represent the same thing.
Since the comment above PCIE_RESET_CONFIG_DEVICE_WAIT_MS is strictly more
correct than the comment above PCIE_T_RRS_READY_MS, change rockchip-host
to use PCIE_RESET_CONFIG_DEVICE_WAIT_MS, and remove PCIE_T_RRS_READY_MS,
as rockchip-host is the only user of this macro.
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
drivers/pci/controller/pcie-rockchip-host.c | 2 +-
drivers/pci/pci.h | 7 -------
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index b9e7a8710cf0..3d40daff98bd 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -325,7 +325,7 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
msleep(PCIE_T_PVPERL_MS);
gpiod_set_value_cansleep(rockchip->perst_gpio, 1);
- msleep(PCIE_T_RRS_READY_MS);
+ msleep(PCIE_RESET_CONFIG_DEVICE_WAIT_MS);
/* 500ms timeout value should be enough for Gen1/2 training */
err = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_BASIC_STATUS1,
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 12215ee72afb..5a83338c8f99 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -35,13 +35,6 @@ struct pcie_tlp_log;
*/
#define PCIE_T_PERST_CLK_US 100
-/*
- * End of conventional reset (PERST# de-asserted) to first configuration
- * request (device able to respond with a "Request Retry Status" completion),
- * from PCIe r6.0, sec 6.6.1.
- */
-#define PCIE_T_RRS_READY_MS 100
-
/*
* PCIe r6.0, sec 5.3.3.2.1 <PME Synchronization>
* Recommends 1ms to 10ms timeout to check L2 ready.
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/5] PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ
2025-06-12 11:49 ` [PATCH v2 1/5] PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ Niklas Cassel
@ 2025-06-12 22:33 ` Wilfred Mallawa
0 siblings, 0 replies; 11+ messages in thread
From: Wilfred Mallawa @ 2025-06-12 22:33 UTC (permalink / raw)
To: shawn.lin@rock-chips.com, cassel@kernel.org, xxm@rock-chips.com,
robh@kernel.org, lpieralisi@kernel.org, kever.yang@rock-chips.com,
kwilczynski@kernel.org, heiko@sntech.de, mani@kernel.org,
bhelgaas@google.com
Cc: linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
laszlo.fiat@proton.me, dlemoal@kernel.org,
linux-rockchip@lists.infradead.org
On Thu, 2025-06-12 at 13:49 +0200, Niklas Cassel wrote:
> Per PCIe r6.0, sec 6.6.1, software must generally wait a minimum of
> 100ms (PCIE_RESET_CONFIG_DEVICE_WAIT_MS) after Link training
> completes
> before sending a Configuration Request.
>
> Prior to ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since
> we can detect Link Up"), dw-rockchip used dw_pcie_wait_for_link(),
> which waited between 0 and 90ms after the link came up before we
> enumerate the bus, and this was apparently enough for most devices.
>
> After ec9fd499b9c6, rockchip_pcie_rc_sys_irq_thread() started
> enumeration immediately when handling the link-up IRQ, and devices
> (e.g., Laszlo Fiat's PLEXTOR PX-256M8PeGN NVMe SSD) may not be ready
> to handle config requests yet.
>
> Delay PCIE_RESET_CONFIG_DEVICE_WAIT_MS after the link-up IRQ before
> starting enumeration.
>
> Cc: Laszlo Fiat <laszlo.fiat@proton.me>
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Fixes: 0e898eb8df4e ("PCI: rockchip-dwc: Add Rockchip RK356X host
> controller driver")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> 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 93171a392879..8a068fd4f867 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -458,6 +458,7 @@ static irqreturn_t
> rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
>
> if (reg & PCIE_RDLH_LINK_UP_CHGED) {
> if (rockchip_pcie_link_up(pci)) {
> + msleep(PCIE_RESET_CONFIG_DEVICE_WAIT_MS);
> dev_dbg(dev, "Received Link up event.
> Starting enumeration!\n");
> /* Rescan the bus to enumerate endpoint
> devices */
> pci_lock_rescan_remove();
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/5] PCI: qcom: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ
2025-06-12 11:49 ` [PATCH v2 2/5] PCI: qcom: " Niklas Cassel
@ 2025-06-12 22:35 ` Wilfred Mallawa
0 siblings, 0 replies; 11+ messages in thread
From: Wilfred Mallawa @ 2025-06-12 22:35 UTC (permalink / raw)
To: robh@kernel.org, mani@kernel.org, kwilczynski@kernel.org,
cassel@kernel.org, bhelgaas@google.com, lpieralisi@kernel.org,
svarbanov@mm-sol.com
Cc: linux-pci@vger.kernel.org, laszlo.fiat@proton.me,
linux-arm-msm@vger.kernel.org, dlemoal@kernel.org
On Thu, 2025-06-12 at 13:49 +0200, Niklas Cassel wrote:
> Per PCIe r6.0, sec 6.6.1, software must generally wait a minimum of
> 100ms (PCIE_RESET_CONFIG_DEVICE_WAIT_MS) after Link training
> completes
> before sending a Configuration Request.
>
> Prior to 36971d6c5a9a ("PCI: qcom: Don't wait for link if we can
> detect
> Link Up"), qcom used dw_pcie_wait_for_link(), which waited between 0
> and 90ms after the link came up before we enumerate the bus, and this
> was apparently enough for most devices.
>
> After 36971d6c5a9a, qcom_pcie_global_irq_thread() started enumeration
> immediately when handling the link-up IRQ, and devices (e.g., Laszlo
> Fiat's PLEXTOR PX-256M8PeGN NVMe SSD) may not be ready to handle
> config
> requests yet.
>
> Delay PCIE_RESET_CONFIG_DEVICE_WAIT_MS after the link-up IRQ before
> starting enumeration.
>
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller
> driver")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> index c789e3f85655..ff257fec6ad7 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1564,6 +1564,7 @@ static irqreturn_t
> qcom_pcie_global_irq_thread(int irq, void *data)
> writel_relaxed(status, pcie->parf + PARF_INT_ALL_CLEAR);
>
> if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
> + msleep(PCIE_RESET_CONFIG_DEVICE_WAIT_MS);
> dev_dbg(dev, "Received Link up event. Starting
> enumeration!\n");
> /* Rescan the bus to enumerate endpoint devices */
> pci_lock_rescan_remove();
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link up
2025-06-12 11:49 ` [PATCH v2 3/5] PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link up Niklas Cassel
@ 2025-06-12 22:36 ` Wilfred Mallawa
0 siblings, 0 replies; 11+ messages in thread
From: Wilfred Mallawa @ 2025-06-12 22:36 UTC (permalink / raw)
To: jingoohan1@gmail.com, mani@kernel.org, kwilczynski@kernel.org,
cassel@kernel.org, bhelgaas@google.com, lpieralisi@kernel.org,
robh@kernel.org
Cc: linux-pci@vger.kernel.org, laszlo.fiat@proton.me,
dlemoal@kernel.org
On Thu, 2025-06-12 at 13:49 +0200, Niklas Cassel wrote:
> As per PCIe r6.0, sec 6.6.1, a Downstream Port that supports Link
> speeds
> greater than 5.0 GT/s, software must wait a minimum of 100 ms after
> Link
> training completes before sending a Configuration Request.
>
> Add this delay in dw_pcie_wait_for_link(), after the link is reported
> as
> up. The delay will only be performed in the success case where the
> link
> came up.
>
> DWC glue drivers that have a link up IRQ (drivers that set
> use_linkup_irq = true) do not call dw_pcie_wait_for_link(), instead
> they
> perform this delay in their threaded link up IRQ handler.
>
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> b/drivers/pci/controller/dwc/pcie-designware.c
> index 4d794964fa0f..7fd3e926c48d 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -714,6 +714,13 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
> return -ETIMEDOUT;
> }
>
> + /*
> + * As per PCIe r6.0, sec 6.6.1, a Downstream Port that
> supports Link
> + * speeds greater than 5.0 GT/s, software must wait a
> minimum of 100 ms
> + * after Link training completes before sending a
> Configuration Request.
> + */
> + msleep(PCIE_RESET_CONFIG_DEVICE_WAIT_MS);
> +
> offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
>
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/5] PCI: dwc: Reduce LINK_WAIT_SLEEP_MS
2025-06-12 11:49 ` [PATCH v2 4/5] PCI: dwc: Reduce LINK_WAIT_SLEEP_MS Niklas Cassel
@ 2025-06-12 22:37 ` Wilfred Mallawa
0 siblings, 0 replies; 11+ messages in thread
From: Wilfred Mallawa @ 2025-06-12 22:37 UTC (permalink / raw)
To: jingoohan1@gmail.com, mani@kernel.org, kwilczynski@kernel.org,
cassel@kernel.org, bhelgaas@google.com, lpieralisi@kernel.org,
robh@kernel.org
Cc: linux-pci@vger.kernel.org, laszlo.fiat@proton.me,
dlemoal@kernel.org
On Thu, 2025-06-12 at 13:49 +0200, Niklas Cassel wrote:
> There is no reason for the delay, in each loop iteration, while
> polling for
> link up (LINK_WAIT_SLEEP_MS), to be so long as 90 ms.
>
> PCIe r6.0, sec 6.6.1, still require us to wait for up to 1.0 s for
> the link
> to come up, thus the number of retries (LINK_WAIT_MAX_RETRIES) is
> increased
> to keep the total timeout to 1.0 s.
>
> PCIe r6.0, sec 6.6.1, also mandates that there is a 100 ms delay,
> after the
> link has been established, before performing configuration requests
> (this
> delay already exists in dw_pcie_wait_for_link() and is unchanged).
>
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 6 +++++-
> drivers/pci/controller/dwc/pcie-designware.h | 13 +++++++++----
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> b/drivers/pci/controller/dwc/pcie-designware.c
> index 7fd3e926c48d..5dab3d668ab3 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -701,7 +701,11 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
> u32 offset, val;
> int retries;
>
> - /* Check if the link is up or not */
> + /*
> + * Check if the link is up or not. As per PCIe r6.0, sec
> 6.6.1, software
> + * must allow at least 1.0 s following exit from a
> Conventional Reset of
> + * a device, before determining that the device is broken.
> + */
> for (retries = 0; retries < LINK_WAIT_MAX_RETRIES;
> retries++) {
> if (dw_pcie_link_up(pci))
> break;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> b/drivers/pci/controller/dwc/pcie-designware.h
> index ce9e18554e42..b225c4f3d36a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -62,11 +62,16 @@
> #define dw_pcie_cap_set(_pci, _cap) \
> set_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
>
> -/* Parameters for the waiting for link up routine */
> -#define LINK_WAIT_MAX_RETRIES 10
> -#define LINK_WAIT_SLEEP_MS 90
> +/*
> + * Parameters for waiting for a link to be established. As per PCIe
> r6.0,
> + * sec 6.6.1, software must allow at least 1.0 s following exit from
> a
> + * Conventional Reset of a device, before determining that the
> device is broken.
> + * Therefore LINK_WAIT_MAX_RETRIES * LINK_WAIT_SLEEP_MS should equal
> 1.0 s.
> + */
> +#define LINK_WAIT_MAX_RETRIES 100
> +#define LINK_WAIT_SLEEP_MS 10
>
> -/* Parameters for the waiting for iATU enabled routine */
> +/* Parameters for waiting for iATU enabled routine */
> #define LINK_WAIT_MAX_IATU_RETRIES 5
> #define LINK_WAIT_IATU 9
>
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 5/5] PCI: rockchip-host: Use macro PCIE_RESET_CONFIG_DEVICE_WAIT_MS
2025-06-12 11:49 ` [PATCH v2 5/5] PCI: rockchip-host: Use macro PCIE_RESET_CONFIG_DEVICE_WAIT_MS Niklas Cassel
@ 2025-06-12 22:39 ` Wilfred Mallawa
0 siblings, 0 replies; 11+ messages in thread
From: Wilfred Mallawa @ 2025-06-12 22:39 UTC (permalink / raw)
To: cassel@kernel.org, robh@kernel.org, lpieralisi@kernel.org,
shawn.lin@rock-chips.com, heiko@sntech.de, mani@kernel.org,
kwilczynski@kernel.org, bhelgaas@google.com
Cc: linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
laszlo.fiat@proton.me, dlemoal@kernel.org,
linux-rockchip@lists.infradead.org
On Thu, 2025-06-12 at 13:49 +0200, Niklas Cassel wrote:
> Macro PCIE_T_RRS_READY_MS was added to pci.h in commit 70a7bfb1e515
> ("PCI:
> rockchip-host: Wait 100ms after reset before starting
> configuration").
>
> Later, in commit d5ceb9496c56 ("PCI: Add
> PCIE_RESET_CONFIG_DEVICE_WAIT_MS
> waiting time value"), PCIE_RESET_CONFIG_DEVICE_WAIT_MS was added to
> pci.h.
>
> These macros represent the same thing.
>
> Since the comment above PCIE_RESET_CONFIG_DEVICE_WAIT_MS is strictly
> more
> correct than the comment above PCIE_T_RRS_READY_MS, change rockchip-
> host
> to use PCIE_RESET_CONFIG_DEVICE_WAIT_MS, and remove
> PCIE_T_RRS_READY_MS,
> as rockchip-host is the only user of this macro.
>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> drivers/pci/controller/pcie-rockchip-host.c | 2 +-
> drivers/pci/pci.h | 7 -------
> 2 files changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c
> b/drivers/pci/controller/pcie-rockchip-host.c
> index b9e7a8710cf0..3d40daff98bd 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -325,7 +325,7 @@ static int rockchip_pcie_host_init_port(struct
> rockchip_pcie *rockchip)
> msleep(PCIE_T_PVPERL_MS);
> gpiod_set_value_cansleep(rockchip->perst_gpio, 1);
>
> - msleep(PCIE_T_RRS_READY_MS);
> + msleep(PCIE_RESET_CONFIG_DEVICE_WAIT_MS);
>
> /* 500ms timeout value should be enough for Gen1/2 training
> */
> err = readl_poll_timeout(rockchip->apb_base +
> PCIE_CLIENT_BASIC_STATUS1,
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 12215ee72afb..5a83338c8f99 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -35,13 +35,6 @@ struct pcie_tlp_log;
> */
> #define PCIE_T_PERST_CLK_US 100
>
> -/*
> - * End of conventional reset (PERST# de-asserted) to first
> configuration
> - * request (device able to respond with a "Request Retry Status"
> completion),
> - * from PCIe r6.0, sec 6.6.1.
> - */
> -#define PCIE_T_RRS_READY_MS 100
> -
> /*
> * PCIe r6.0, sec 5.3.3.2.1 <PME Synchronization>
> * Recommends 1ms to 10ms timeout to check L2 ready.
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-06-12 22:39 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-12 11:49 [PATCH v2 0/5] PCI: dwc: Do not enumerate bus before endpoint devices are ready Niklas Cassel
2025-06-12 11:49 ` [PATCH v2 1/5] PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_DEVICE_WAIT_MS after link-up IRQ Niklas Cassel
2025-06-12 22:33 ` Wilfred Mallawa
2025-06-12 11:49 ` [PATCH v2 2/5] PCI: qcom: " Niklas Cassel
2025-06-12 22:35 ` Wilfred Mallawa
2025-06-12 11:49 ` [PATCH v2 3/5] PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link up Niklas Cassel
2025-06-12 22:36 ` Wilfred Mallawa
2025-06-12 11:49 ` [PATCH v2 4/5] PCI: dwc: Reduce LINK_WAIT_SLEEP_MS Niklas Cassel
2025-06-12 22:37 ` Wilfred Mallawa
2025-06-12 11:49 ` [PATCH v2 5/5] PCI: rockchip-host: Use macro PCIE_RESET_CONFIG_DEVICE_WAIT_MS Niklas Cassel
2025-06-12 22:39 ` Wilfred Mallawa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).