From: Niklas Cassel <cassel@kernel.org>
To: "Lorenzo Pieralisi" <lpieralisi@kernel.org>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
"Rob Herring" <robh@kernel.org>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Heiko Stuebner" <heiko@sntech.de>,
"Niklas Cassel" <cassel@kernel.org>
Cc: "Wilfred Mallawa" <wilfred.mallawa@wdc.com>,
"Damien Le Moal" <dlemoal@kernel.org>,
"Hans Zhang" <18255117159@163.com>,
"Laszlo Fiat" <laszlo.fiat@proton.me>,
"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
Subject: [PATCH v2 1/4] PCI: dw-rockchip: Do not enumerate bus before endpoint devices are ready
Date: Tue, 6 May 2025 09:39:36 +0200 [thread overview]
Message-ID: <20250506073934.433176-7-cassel@kernel.org> (raw)
In-Reply-To: <20250506073934.433176-6-cassel@kernel.org>
Commit ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can
detect Link Up") changed so that we no longer call dw_pcie_wait_for_link(),
and instead enumerate the bus when receiving a Link Up IRQ.
Laszlo Fiat reported (off-list) that his PLEXTOR PX-256M8PeGN NVMe SSD is
no longer functional, and simply reverting commit ec9fd499b9c6 ("PCI:
dw-rockchip: Don't wait for link since we can detect Link Up") makes his
SSD functional again.
It seems that we are enumerating the bus before the endpoint is ready.
Adding a msleep(PCIE_T_RRS_READY_MS) before enumerating the bus in the
threaded IRQ handler makes the SSD functional once again.
What appears to happen is that before ec9fd499b9c6, we called
dw_pcie_wait_for_link(), and in the first iteration of the loop, the link
will never be up (because the link was just started),
dw_pcie_wait_for_link() will then sleep for LINK_WAIT_SLEEP_MS (90 ms),
before trying again.
This means that even if a driver was missing a msleep(PCIE_T_RRS_READY_MS)
(100 ms), because of the call to dw_pcie_wait_for_link(), enumerating the
bus would essentially be delayed by that time anyway (because of the sleep
LINK_WAIT_SLEEP_MS (90 ms)).
While we could add the msleep(PCIE_T_RRS_READY_MS) after deasserting PERST,
that would essentially bring back an unconditional delay during synchronous
probe (the whole reason to use a Link Up IRQ was to avoid an unconditional
delay during probe).
Thus, add the msleep(PCIE_T_RRS_READY_MS) before enumerating the bus in the
IRQ handler. This way, we will not have an unconditional delay during boot
for unpopulated PCIe slots.
Cc: Laszlo Fiat <laszlo.fiat@proton.me>
Fixes: ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can detect Link Up")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 3c6ab71c996e..6a7ec3545a7f 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -23,6 +23,7 @@
#include <linux/reset.h>
#include "pcie-designware.h"
+#include "../../pci.h"
/*
* The upper 16 bits of PCIE_CLIENT_CONFIG are a write
@@ -458,6 +459,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)) {
dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
+ msleep(PCIE_T_RRS_READY_MS);
/* Rescan the bus to enumerate endpoint devices */
pci_lock_rescan_remove();
pci_rescan_bus(pp->bridge->bus);
--
2.49.0
WARNING: multiple messages have this Message-ID (diff)
From: Niklas Cassel <cassel@kernel.org>
To: "Lorenzo Pieralisi" <lpieralisi@kernel.org>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
"Rob Herring" <robh@kernel.org>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Heiko Stuebner" <heiko@sntech.de>,
"Niklas Cassel" <cassel@kernel.org>
Cc: "Wilfred Mallawa" <wilfred.mallawa@wdc.com>,
"Damien Le Moal" <dlemoal@kernel.org>,
"Hans Zhang" <18255117159@163.com>,
"Laszlo Fiat" <laszlo.fiat@proton.me>,
"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
Subject: [PATCH v2 1/4] PCI: dw-rockchip: Do not enumerate bus before endpoint devices are ready
Date: Tue, 6 May 2025 09:39:36 +0200 [thread overview]
Message-ID: <20250506073934.433176-7-cassel@kernel.org> (raw)
In-Reply-To: <20250506073934.433176-6-cassel@kernel.org>
Commit ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can
detect Link Up") changed so that we no longer call dw_pcie_wait_for_link(),
and instead enumerate the bus when receiving a Link Up IRQ.
Laszlo Fiat reported (off-list) that his PLEXTOR PX-256M8PeGN NVMe SSD is
no longer functional, and simply reverting commit ec9fd499b9c6 ("PCI:
dw-rockchip: Don't wait for link since we can detect Link Up") makes his
SSD functional again.
It seems that we are enumerating the bus before the endpoint is ready.
Adding a msleep(PCIE_T_RRS_READY_MS) before enumerating the bus in the
threaded IRQ handler makes the SSD functional once again.
What appears to happen is that before ec9fd499b9c6, we called
dw_pcie_wait_for_link(), and in the first iteration of the loop, the link
will never be up (because the link was just started),
dw_pcie_wait_for_link() will then sleep for LINK_WAIT_SLEEP_MS (90 ms),
before trying again.
This means that even if a driver was missing a msleep(PCIE_T_RRS_READY_MS)
(100 ms), because of the call to dw_pcie_wait_for_link(), enumerating the
bus would essentially be delayed by that time anyway (because of the sleep
LINK_WAIT_SLEEP_MS (90 ms)).
While we could add the msleep(PCIE_T_RRS_READY_MS) after deasserting PERST,
that would essentially bring back an unconditional delay during synchronous
probe (the whole reason to use a Link Up IRQ was to avoid an unconditional
delay during probe).
Thus, add the msleep(PCIE_T_RRS_READY_MS) before enumerating the bus in the
IRQ handler. This way, we will not have an unconditional delay during boot
for unpopulated PCIe slots.
Cc: Laszlo Fiat <laszlo.fiat@proton.me>
Fixes: ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can detect Link Up")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 3c6ab71c996e..6a7ec3545a7f 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -23,6 +23,7 @@
#include <linux/reset.h>
#include "pcie-designware.h"
+#include "../../pci.h"
/*
* The upper 16 bits of PCIE_CLIENT_CONFIG are a write
@@ -458,6 +459,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)) {
dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
+ msleep(PCIE_T_RRS_READY_MS);
/* Rescan the bus to enumerate endpoint devices */
pci_lock_rescan_remove();
pci_rescan_bus(pp->bridge->bus);
--
2.49.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2025-05-06 7:44 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-06 7:39 [PATCH v2 0/4] PCI: dwc: Link Up IRQ fixes Niklas Cassel
2025-05-06 7:39 ` Niklas Cassel
2025-05-06 7:39 ` Niklas Cassel [this message]
2025-05-06 7:39 ` [PATCH v2 1/4] PCI: dw-rockchip: Do not enumerate bus before endpoint devices are ready Niklas Cassel
2025-05-06 11:32 ` Laszlo Fiat
2025-05-06 11:32 ` Laszlo Fiat
2025-05-06 22:23 ` Wilfred Mallawa
2025-05-06 22:23 ` Wilfred Mallawa
2025-05-28 22:42 ` Bjorn Helgaas
2025-05-28 22:42 ` Bjorn Helgaas
2025-05-30 13:57 ` Niklas Cassel
2025-05-30 13:57 ` Niklas Cassel
2025-05-30 15:21 ` Bjorn Helgaas
2025-05-30 15:21 ` Bjorn Helgaas
2025-05-30 15:59 ` Manivannan Sadhasivam
2025-05-30 15:59 ` Manivannan Sadhasivam
2025-05-30 17:19 ` Bjorn Helgaas
2025-05-30 17:19 ` Bjorn Helgaas
2025-05-30 17:24 ` Niklas Cassel
2025-05-30 17:24 ` Niklas Cassel
2025-05-30 19:43 ` Bjorn Helgaas
2025-05-30 19:43 ` Bjorn Helgaas
2025-05-31 6:47 ` Manivannan Sadhasivam
2025-05-31 6:47 ` Manivannan Sadhasivam
2025-06-03 14:08 ` Niklas Cassel
2025-06-03 14:08 ` Niklas Cassel
2025-06-03 18:12 ` Bjorn Helgaas
2025-06-03 18:12 ` Bjorn Helgaas
2025-06-04 11:40 ` Niklas Cassel
2025-06-04 11:40 ` Niklas Cassel
2025-06-04 17:10 ` Manivannan Sadhasivam
2025-06-04 17:10 ` Manivannan Sadhasivam
2025-06-04 18:44 ` Bjorn Helgaas
2025-06-04 18:44 ` Bjorn Helgaas
2025-06-05 12:28 ` Niklas Cassel
2025-06-05 12:28 ` Niklas Cassel
2025-06-05 13:22 ` Manivannan Sadhasivam
2025-06-05 13:22 ` Manivannan Sadhasivam
2025-06-05 19:27 ` Bjorn Helgaas
2025-06-05 19:27 ` Bjorn Helgaas
2025-05-06 7:39 ` [PATCH v2 2/4] PCI: qcom: " Niklas Cassel
2025-05-06 10:11 ` Krishna Chaitanya Chundru
2025-05-06 11:29 ` Laszlo Fiat
2025-05-06 22:24 ` Wilfred Mallawa
2025-05-06 7:39 ` [PATCH v2 3/4] PCI: dw-rockchip: Replace PERST sleep time with proper macro Niklas Cassel
2025-05-06 7:39 ` Niklas Cassel
2025-05-06 9:07 ` Hans Zhang
2025-05-06 9:07 ` Hans Zhang
2025-05-06 11:36 ` Laszlo Fiat
2025-05-06 11:36 ` Laszlo Fiat
2025-05-06 22:24 ` Wilfred Mallawa
2025-05-06 22:24 ` Wilfred Mallawa
2025-05-06 7:39 ` [PATCH v2 4/4] PCI: qcom: " Niklas Cassel
2025-05-06 9:07 ` Hans Zhang
2025-05-06 10:12 ` Krishna Chaitanya Chundru
2025-05-06 11:37 ` Laszlo Fiat
2025-05-06 22:25 ` Wilfred Mallawa
2025-05-06 14:33 ` [PATCH v2 0/4] PCI: dwc: Link Up IRQ fixes Niklas Cassel
2025-05-06 14:33 ` Niklas Cassel
2025-05-06 14:51 ` Laszlo Fiat
2025-05-06 14:51 ` Laszlo Fiat
2025-05-13 10:53 ` Manivannan Sadhasivam
2025-05-13 10:53 ` Manivannan Sadhasivam
2025-05-13 14:07 ` Niklas Cassel
2025-05-13 14:07 ` Niklas Cassel
2025-05-15 17:33 ` Laszlo Fiat
2025-05-15 17:33 ` Laszlo Fiat
2025-05-16 10:00 ` Niklas Cassel
2025-05-16 10:00 ` Niklas Cassel
2025-05-16 18:48 ` Laszlo Fiat
2025-05-16 18:48 ` Laszlo Fiat
2025-05-19 9:44 ` Niklas Cassel
2025-05-19 9:44 ` Niklas Cassel
2025-05-19 12:10 ` Manivannan Sadhasivam
2025-05-19 12:10 ` Manivannan Sadhasivam
2025-05-19 12:37 ` Manivannan Sadhasivam
2025-05-19 12:37 ` Manivannan Sadhasivam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250506073934.433176-7-cassel@kernel.org \
--to=cassel@kernel.org \
--cc=18255117159@163.com \
--cc=bhelgaas@google.com \
--cc=dlemoal@kernel.org \
--cc=heiko@sntech.de \
--cc=kw@linux.com \
--cc=kwilczynski@kernel.org \
--cc=laszlo.fiat@proton.me \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=lpieralisi@kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=robh@kernel.org \
--cc=wilfred.mallawa@wdc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.