* [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions
@ 2025-08-26 17:03 Hans Zhang
2025-08-26 17:03 ` [PATCH v3 1/8] PCI: Add macro for secondary bus reset delay Hans Zhang
` (7 more replies)
0 siblings, 8 replies; 11+ messages in thread
From: Hans Zhang @ 2025-08-26 17:03 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
This series improves code readability and maintainability in the PCI
subsystem by replacing hard-coded delay values with descriptive macros.
The changes include:
- Adding macros for various delay values used in PCI operations
- Replacing msleep(2) with fsleep(2000) for precise secondary bus reset
- Keeping the same delay values but using macros for better documentation
These changes make the code easier to understand and maintain, while
ensuring that the timing requirements specified in the PCIe r7.0
specification are met.
---
Changes for v3:
https://patchwork.kernel.org/project/linux-pci/cover/20250822155908.625553-1-18255117159@163.com/
- According to Bjorn's suggestion, split the first patch of v2 and add
macro definitions to the remaining patches.
Changes for v2:
https://patchwork.kernel.org/project/linux-pci/patch/20250820160944.489061-1-18255117159@163.com/
- According to the Maintainer's suggestion, it was modified to fsleep,
usleep_range, and macro definitions were used instead of hard code. (Bjorn)
---
Hans Zhang (8):
PCI: Add macro for secondary bus reset delay
PCI: Replace msleep with fsleep for precise secondary bus reset
PCI: Add macro for link status check delay
PCI: rcar-host: Add macro for speed change monitoring delay
PCI: brcmstb: Add macro for link up check delay
PCI: rcar: Add macro for PHY ready check delay
PCI: pciehp: Add macros for hotplug operation delays
PCI/DPC: Add macro for RP busy check delay
drivers/pci/controller/pcie-brcmstb.c | 4 +++-
drivers/pci/controller/pcie-rcar-host.c | 4 +++-
drivers/pci/controller/pcie-rcar.c | 4 +++-
drivers/pci/hotplug/pciehp_hpc.c | 7 +++++--
drivers/pci/pci.c | 11 +++++------
drivers/pci/pci.h | 3 +++
drivers/pci/pcie/dpc.c | 4 +++-
7 files changed, 25 insertions(+), 12 deletions(-)
base-commit: fab1beda7597fac1cecc01707d55eadb6bbe773c
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/8] PCI: Add macro for secondary bus reset delay
2025-08-26 17:03 [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions Hans Zhang
@ 2025-08-26 17:03 ` Hans Zhang
2025-08-26 17:03 ` [PATCH v3 2/8] PCI: Replace msleep with fsleep for precise secondary bus reset Hans Zhang
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Hans Zhang @ 2025-08-26 17:03 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
Add PCI_T_RST_SEC_BUS_DELAY_MS macro for the secondary bus reset
delay value according to PCIe r7.0 spec, section 7.5.1.3.13.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/pci.c | 7 ++-----
drivers/pci/pci.h | 3 +++
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b0f4d98036cd..c05a4c2fa643 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4963,11 +4963,8 @@ void pci_reset_secondary_bus(struct pci_dev *dev)
ctrl |= PCI_BRIDGE_CTL_BUS_RESET;
pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
- /*
- * PCI spec v3.0 7.6.4.2 requires minimum Trst of 1ms. Double
- * this to 2ms to ensure that we meet the minimum requirement.
- */
- msleep(2);
+ /* Double this to 2ms to ensure that we meet the minimum requirement */
+ msleep(2 * PCI_T_RST_SEC_BUS_DELAY_MS);
ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 34f65d69662e..4d7e9c3f3453 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -60,6 +60,9 @@ struct pcie_tlp_log;
#define PCIE_LINK_WAIT_MAX_RETRIES 10
#define PCIE_LINK_WAIT_SLEEP_MS 90
+/* PCIe r7.0, sec 7.5.1.3.13, requires minimum Trst of 1ms */
+#define PCI_T_RST_SEC_BUS_DELAY_MS 1
+
/* Message Routing (r[2:0]); PCIe r6.0, sec 2.2.8 */
#define PCIE_MSG_TYPE_R_RC 0
#define PCIE_MSG_TYPE_R_ADDR 1
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/8] PCI: Replace msleep with fsleep for precise secondary bus reset
2025-08-26 17:03 [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions Hans Zhang
2025-08-26 17:03 ` [PATCH v3 1/8] PCI: Add macro for secondary bus reset delay Hans Zhang
@ 2025-08-26 17:03 ` Hans Zhang
2025-08-26 23:05 ` Bjorn Helgaas
2025-08-26 17:03 ` [PATCH v3 3/8] PCI: Add macro for link status check delay Hans Zhang
` (5 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Hans Zhang @ 2025-08-26 17:03 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
The msleep() function with small values (less than 20ms) may not sleep
for the exact duration due to the kernel's timer wheel design. According
to the comment in kernel/time/sleep_timeout.c:
"The slack of timers which will end up in level 0 depends on sleep
duration (msecs) and HZ configuration. For example, with HZ=1000 and
a requested sleep of 2ms, the slack can be as high as 50% (1ms) because
the minimum slack is 12.5% but the actual calculation for level 0 timers
is slack = MSECS_PER_TICK / msecs. This means that msleep(2) can
actually take up to 3ms (2ms + 1ms) on a system with HZ=1000."
This unnecessary delay can impact system responsiveness during PCI
operations, especially since the PCIe r7.0 specification, section
7.5.1.3.13, requires only a minimum Trst of 1ms. We double this to 2ms
to ensure we meet the minimum requirement, but using msleep(2) may
actually wait longer than needed.
Using fsleep() provides a more precise delay that matches the stated
intent of the code. The fsleep() function uses high-resolution timers
where available to achieve microsecond precision.
Replace msleep(2 * PCI_T_RST_SEC_BUS_DELAY_MS) with
fsleep(2 * PCI_T_RST_SEC_BUS_DELAY_US) to ensure the actual delay is
closer to the intended 2ms delay.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/pci.c | 2 +-
drivers/pci/pci.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index c05a4c2fa643..81105dfc2f62 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4964,7 +4964,7 @@ void pci_reset_secondary_bus(struct pci_dev *dev)
pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
/* Double this to 2ms to ensure that we meet the minimum requirement */
- msleep(2 * PCI_T_RST_SEC_BUS_DELAY_MS);
+ fsleep(2 * PCI_T_RST_SEC_BUS_DELAY_US);
ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4d7e9c3f3453..9d38ef26c6a9 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -61,7 +61,7 @@ struct pcie_tlp_log;
#define PCIE_LINK_WAIT_SLEEP_MS 90
/* PCIe r7.0, sec 7.5.1.3.13, requires minimum Trst of 1ms */
-#define PCI_T_RST_SEC_BUS_DELAY_MS 1
+#define PCI_T_RST_SEC_BUS_DELAY_US 1000
/* Message Routing (r[2:0]); PCIe r6.0, sec 2.2.8 */
#define PCIE_MSG_TYPE_R_RC 0
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/8] PCI: Add macro for link status check delay
2025-08-26 17:03 [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions Hans Zhang
2025-08-26 17:03 ` [PATCH v3 1/8] PCI: Add macro for secondary bus reset delay Hans Zhang
2025-08-26 17:03 ` [PATCH v3 2/8] PCI: Replace msleep with fsleep for precise secondary bus reset Hans Zhang
@ 2025-08-26 17:03 ` Hans Zhang
2025-08-26 17:03 ` [PATCH v3 4/8] PCI: rcar-host: Add macro for speed change monitoring delay Hans Zhang
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Hans Zhang @ 2025-08-26 17:03 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
Add PCIE_LINK_STATUS_CHECK_MS macro for link status check delay.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 81105dfc2f62..f5f5474d9aba 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -77,6 +77,8 @@ struct pci_pme_device {
*/
#define PCIE_RESET_READY_POLL_MS 60000 /* msec */
+#define PCIE_LINK_STATUS_CHECK_MS 1
+
static void pci_dev_d3_sleep(struct pci_dev *dev)
{
unsigned int delay_ms = max(dev->d3hot_delay, pci_pm_d3hot_delay);
@@ -4683,7 +4685,7 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
if ((lnksta & lnksta_mask) == lnksta_match)
return 0;
- msleep(1);
+ msleep(PCIE_LINK_STATUS_CHECK_MS);
} while (time_before(jiffies, end_jiffies));
return -ETIMEDOUT;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 4/8] PCI: rcar-host: Add macro for speed change monitoring delay
2025-08-26 17:03 [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions Hans Zhang
` (2 preceding siblings ...)
2025-08-26 17:03 ` [PATCH v3 3/8] PCI: Add macro for link status check delay Hans Zhang
@ 2025-08-26 17:03 ` Hans Zhang
2025-08-26 17:03 ` [PATCH v3 5/8] PCI: brcmstb: Add macro for link up check delay Hans Zhang
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Hans Zhang @ 2025-08-26 17:03 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
Add RCAR_SPEED_CHANGE_CHECK_MS macro for speed change monitoring delay.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/pcie-rcar-host.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
index fe288fd770c4..77f45637b0fe 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -34,6 +34,8 @@
#include "pcie-rcar.h"
+#define RCAR_SPEED_CHANGE_CHECK_MS 1
+
struct rcar_msi {
DECLARE_BITMAP(used, INT_PCI_MSI_NR);
struct irq_domain *domain;
@@ -339,7 +341,7 @@ static void rcar_pcie_force_speedup(struct rcar_pcie *pcie)
goto done;
}
- msleep(1);
+ msleep(RCAR_SPEED_CHANGE_CHECK_MS);
}
dev_err(dev, "Speed change timed out\n");
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 5/8] PCI: brcmstb: Add macro for link up check delay
2025-08-26 17:03 [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions Hans Zhang
` (3 preceding siblings ...)
2025-08-26 17:03 ` [PATCH v3 4/8] PCI: rcar-host: Add macro for speed change monitoring delay Hans Zhang
@ 2025-08-26 17:03 ` Hans Zhang
2025-08-26 17:03 ` [PATCH v3 6/8] PCI: rcar: Add macro for PHY ready " Hans Zhang
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Hans Zhang @ 2025-08-26 17:03 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
Add BRCM_PCIE_LINK_UP_CHECK_MS macro for link up check delay.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/pcie-brcmstb.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 9afbd02ded35..dd682c5e5f49 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -214,6 +214,8 @@
#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK 0x1
#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT 0x0
+#define BRCM_PCIE_LINK_UP_CHECK_MS 5
+
/* Forward declarations */
struct brcm_pcie;
@@ -1365,7 +1367,7 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
* total of 100ms.
*/
for (i = 0; i < 100 && !brcm_pcie_link_up(pcie); i += 5)
- msleep(5);
+ msleep(BRCM_PCIE_LINK_UP_CHECK_MS);
if (!brcm_pcie_link_up(pcie)) {
dev_err(dev, "link down\n");
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 6/8] PCI: rcar: Add macro for PHY ready check delay
2025-08-26 17:03 [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions Hans Zhang
` (4 preceding siblings ...)
2025-08-26 17:03 ` [PATCH v3 5/8] PCI: brcmstb: Add macro for link up check delay Hans Zhang
@ 2025-08-26 17:03 ` Hans Zhang
2025-08-26 17:03 ` [PATCH v3 7/8] PCI: pciehp: Add macros for hotplug operation delays Hans Zhang
2025-08-26 17:03 ` [PATCH v3 8/8] PCI/DPC: Add macro for RP busy check delay Hans Zhang
7 siblings, 0 replies; 11+ messages in thread
From: Hans Zhang @ 2025-08-26 17:03 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
Add RCAR_PCIE_PHYRDY_CHECK_MS macro for PHY ready check delay.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/pcie-rcar.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
index 7583699ef7b6..653f13d82934 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -11,6 +11,8 @@
#include "pcie-rcar.h"
+#define RCAR_PCIE_PHYRDY_CHECK_MS 5
+
void rcar_pci_write_reg(struct rcar_pcie *pcie, u32 val, unsigned int reg)
{
writel(val, pcie->base + reg);
@@ -39,7 +41,7 @@ int rcar_pcie_wait_for_phyrdy(struct rcar_pcie *pcie)
if (rcar_pci_read_reg(pcie, PCIEPHYSR) & PHYRDY)
return 0;
- msleep(5);
+ msleep(RCAR_PCIE_PHYRDY_CHECK_MS);
}
return -ETIMEDOUT;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 7/8] PCI: pciehp: Add macros for hotplug operation delays
2025-08-26 17:03 [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions Hans Zhang
` (5 preceding siblings ...)
2025-08-26 17:03 ` [PATCH v3 6/8] PCI: rcar: Add macro for PHY ready " Hans Zhang
@ 2025-08-26 17:03 ` Hans Zhang
2025-08-26 17:03 ` [PATCH v3 8/8] PCI/DPC: Add macro for RP busy check delay Hans Zhang
7 siblings, 0 replies; 11+ messages in thread
From: Hans Zhang @ 2025-08-26 17:03 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
Add WAIT_PDS_TIMEOUT_MS and POLL_CMD_TIMEOUT_MS macros for hotplug
operation delays to improve code readability.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/hotplug/pciehp_hpc.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index bcc51b26d03d..15b09c6a8d6b 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -28,6 +28,9 @@
#include "../pci.h"
#include "pciehp.h"
+#define WAIT_PDS_TIMEOUT_MS 10
+#define POLL_CMD_TIMEOUT_MS 10
+
static const struct dmi_system_id inband_presence_disabled_dmi_table[] = {
/*
* Match all Dell systems, as some Dell systems have inband
@@ -103,7 +106,7 @@ static int pcie_poll_cmd(struct controller *ctrl, int timeout)
smp_mb();
return 1;
}
- msleep(10);
+ msleep(POLL_CMD_TIMEOUT_MS);
timeout -= 10;
} while (timeout >= 0);
return 0; /* timeout */
@@ -283,7 +286,7 @@ static void pcie_wait_for_presence(struct pci_dev *pdev)
pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
if (slot_status & PCI_EXP_SLTSTA_PDS)
return;
- msleep(10);
+ msleep(WAIT_PDS_TIMEOUT_MS);
timeout -= 10;
} while (timeout > 0);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 8/8] PCI/DPC: Add macro for RP busy check delay
2025-08-26 17:03 [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions Hans Zhang
` (6 preceding siblings ...)
2025-08-26 17:03 ` [PATCH v3 7/8] PCI: pciehp: Add macros for hotplug operation delays Hans Zhang
@ 2025-08-26 17:03 ` Hans Zhang
7 siblings, 0 replies; 11+ messages in thread
From: Hans Zhang @ 2025-08-26 17:03 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
Add PCIE_EXP_DPC_BUSY_CHECK_MS macro for RP busy check delay.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/pcie/dpc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index fc18349614d7..bd5e8cd9e43e 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -21,6 +21,8 @@
#define PCI_EXP_DPC_CTL_EN_MASK (PCI_EXP_DPC_CTL_EN_FATAL | \
PCI_EXP_DPC_CTL_EN_NONFATAL)
+#define PCIE_EXP_DPC_BUSY_CHECK_MS 10
+
static const char * const rp_pio_error_string[] = {
"Configuration Request received UR Completion", /* Bit Position 0 */
"Configuration Request received CA Completion", /* Bit Position 1 */
@@ -135,7 +137,7 @@ static int dpc_wait_rp_inactive(struct pci_dev *pdev)
pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
while (status & PCI_EXP_DPC_RP_BUSY &&
!time_after(jiffies, timeout)) {
- msleep(10);
+ msleep(PCIE_EXP_DPC_BUSY_CHECK_MS);
pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
}
if (status & PCI_EXP_DPC_RP_BUSY) {
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/8] PCI: Replace msleep with fsleep for precise secondary bus reset
2025-08-26 17:03 ` [PATCH v3 2/8] PCI: Replace msleep with fsleep for precise secondary bus reset Hans Zhang
@ 2025-08-26 23:05 ` Bjorn Helgaas
2025-08-28 14:25 ` Hans Zhang
0 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2025-08-26 23:05 UTC (permalink / raw)
To: Hans Zhang; +Cc: bhelgaas, linux-pci, linux-kernel
On Wed, Aug 27, 2025 at 01:03:09AM +0800, Hans Zhang wrote:
> The msleep() function with small values (less than 20ms) may not sleep
> for the exact duration due to the kernel's timer wheel design. According
> to the comment in kernel/time/sleep_timeout.c:
>
> "The slack of timers which will end up in level 0 depends on sleep
> duration (msecs) and HZ configuration. For example, with HZ=1000 and
> a requested sleep of 2ms, the slack can be as high as 50% (1ms) because
> the minimum slack is 12.5% but the actual calculation for level 0 timers
> is slack = MSECS_PER_TICK / msecs. This means that msleep(2) can
> actually take up to 3ms (2ms + 1ms) on a system with HZ=1000."
I thought I heard something about 20ms being the minimum actual delay
for small msleeps. I suppose the error is larger for HZ=100 systems.
The fsleep() would turn into something between 2ms and 2.5ms, so if
we're talking about reducing 3ms to 2.5ms, I have a hard time getting
worried about that.
And we're going to wait at least 100ms before touching the device
below the bridge anyway.
> This unnecessary delay can impact system responsiveness during PCI
> operations, especially since the PCIe r7.0 specification, section
> 7.5.1.3.13, requires only a minimum Trst of 1ms. We double this to 2ms
> to ensure we meet the minimum requirement, but using msleep(2) may
> actually wait longer than needed.
>
> Using fsleep() provides a more precise delay that matches the stated
> intent of the code. The fsleep() function uses high-resolution timers
> where available to achieve microsecond precision.
>
> Replace msleep(2 * PCI_T_RST_SEC_BUS_DELAY_MS) with
> fsleep(2 * PCI_T_RST_SEC_BUS_DELAY_US) to ensure the actual delay is
> closer to the intended 2ms delay.
>
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
> drivers/pci/pci.c | 2 +-
> drivers/pci/pci.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index c05a4c2fa643..81105dfc2f62 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4964,7 +4964,7 @@ void pci_reset_secondary_bus(struct pci_dev *dev)
> pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
>
> /* Double this to 2ms to ensure that we meet the minimum requirement */
> - msleep(2 * PCI_T_RST_SEC_BUS_DELAY_MS);
> + fsleep(2 * PCI_T_RST_SEC_BUS_DELAY_US);
>
> ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
> pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 4d7e9c3f3453..9d38ef26c6a9 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -61,7 +61,7 @@ struct pcie_tlp_log;
> #define PCIE_LINK_WAIT_SLEEP_MS 90
>
> /* PCIe r7.0, sec 7.5.1.3.13, requires minimum Trst of 1ms */
> -#define PCI_T_RST_SEC_BUS_DELAY_MS 1
> +#define PCI_T_RST_SEC_BUS_DELAY_US 1000
>
> /* Message Routing (r[2:0]); PCIe r6.0, sec 2.2.8 */
> #define PCIE_MSG_TYPE_R_RC 0
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/8] PCI: Replace msleep with fsleep for precise secondary bus reset
2025-08-26 23:05 ` Bjorn Helgaas
@ 2025-08-28 14:25 ` Hans Zhang
0 siblings, 0 replies; 11+ messages in thread
From: Hans Zhang @ 2025-08-28 14:25 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: bhelgaas, linux-pci, linux-kernel
On 2025/8/27 07:05, Bjorn Helgaas wrote:
> On Wed, Aug 27, 2025 at 01:03:09AM +0800, Hans Zhang wrote:
>> The msleep() function with small values (less than 20ms) may not sleep
>> for the exact duration due to the kernel's timer wheel design. According
>> to the comment in kernel/time/sleep_timeout.c:
>>
>> "The slack of timers which will end up in level 0 depends on sleep
>> duration (msecs) and HZ configuration. For example, with HZ=1000 and
>> a requested sleep of 2ms, the slack can be as high as 50% (1ms) because
>> the minimum slack is 12.5% but the actual calculation for level 0 timers
>> is slack = MSECS_PER_TICK / msecs. This means that msleep(2) can
>> actually take up to 3ms (2ms + 1ms) on a system with HZ=1000."
>
> I thought I heard something about 20ms being the minimum actual delay
> for small msleeps. I suppose the error is larger for HZ=100 systems.
>
Yes.
> The fsleep() would turn into something between 2ms and 2.5ms, so if
> we're talking about reducing 3ms to 2.5ms, I have a hard time getting
> worried about that.
>
> And we're going to wait at least 100ms before touching the device
> below the bridge anyway.
int pci_bridge_secondary_bus_reset(struct pci_dev *dev)
pcibios_reset_secondary_bus(dev);
pci_reset_secondary_bus(dev);
pci_read_config_word(dev, PCI_BRIDGE_CONTROL,
&ctrl);
ctrl |= PCI_BRIDGE_CTL_BUS_RESET;
pci_write_config_word(dev, PCI_BRIDGE_CONTROL,
ctrl);
/*
* PCI spec v3.0 7.6.4.2 requires minimum Trst
of 1ms. Double
* this to 2ms to ensure that we meet the
minimum requirement.
*/
msleep(2); // As you mentioned, is it
necessary to be very precise here? Please decide whether you want to
make the modification.
ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
pci_write_config_word(dev, PCI_BRIDGE_CONTROL,
ctrl);
pci_bridge_wait_for_secondary_bus(dev, "bus reset");
// There are also some delays in between.
// The delay here is also long enough.
pci_dev_wait(child, reset_type,
PCIE_RESET_READY_POLL_MS - delay);
If it's not necessary to do so, do I still need to resubmit the version?
Or do you choose a few acceptable ones or the first patch?
Best regards,
Hans
>
>> This unnecessary delay can impact system responsiveness during PCI
>> operations, especially since the PCIe r7.0 specification, section
>> 7.5.1.3.13, requires only a minimum Trst of 1ms. We double this to 2ms
>> to ensure we meet the minimum requirement, but using msleep(2) may
>> actually wait longer than needed.
>>
>> Using fsleep() provides a more precise delay that matches the stated
>> intent of the code. The fsleep() function uses high-resolution timers
>> where available to achieve microsecond precision.
>>
>> Replace msleep(2 * PCI_T_RST_SEC_BUS_DELAY_MS) with
>> fsleep(2 * PCI_T_RST_SEC_BUS_DELAY_US) to ensure the actual delay is
>> closer to the intended 2ms delay.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> drivers/pci/pci.c | 2 +-
>> drivers/pci/pci.h | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index c05a4c2fa643..81105dfc2f62 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -4964,7 +4964,7 @@ void pci_reset_secondary_bus(struct pci_dev *dev)
>> pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
>>
>> /* Double this to 2ms to ensure that we meet the minimum requirement */
>> - msleep(2 * PCI_T_RST_SEC_BUS_DELAY_MS);
>> + fsleep(2 * PCI_T_RST_SEC_BUS_DELAY_US);
>>
>> ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
>> pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 4d7e9c3f3453..9d38ef26c6a9 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -61,7 +61,7 @@ struct pcie_tlp_log;
>> #define PCIE_LINK_WAIT_SLEEP_MS 90
>>
>> /* PCIe r7.0, sec 7.5.1.3.13, requires minimum Trst of 1ms */
>> -#define PCI_T_RST_SEC_BUS_DELAY_MS 1
>> +#define PCI_T_RST_SEC_BUS_DELAY_US 1000
>>
>> /* Message Routing (r[2:0]); PCIe r6.0, sec 2.2.8 */
>> #define PCIE_MSG_TYPE_R_RC 0
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-08-28 14:25 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-26 17:03 [PATCH v3 0/8] PCI: Replace short msleep() calls with more precise delay functions Hans Zhang
2025-08-26 17:03 ` [PATCH v3 1/8] PCI: Add macro for secondary bus reset delay Hans Zhang
2025-08-26 17:03 ` [PATCH v3 2/8] PCI: Replace msleep with fsleep for precise secondary bus reset Hans Zhang
2025-08-26 23:05 ` Bjorn Helgaas
2025-08-28 14:25 ` Hans Zhang
2025-08-26 17:03 ` [PATCH v3 3/8] PCI: Add macro for link status check delay Hans Zhang
2025-08-26 17:03 ` [PATCH v3 4/8] PCI: rcar-host: Add macro for speed change monitoring delay Hans Zhang
2025-08-26 17:03 ` [PATCH v3 5/8] PCI: brcmstb: Add macro for link up check delay Hans Zhang
2025-08-26 17:03 ` [PATCH v3 6/8] PCI: rcar: Add macro for PHY ready " Hans Zhang
2025-08-26 17:03 ` [PATCH v3 7/8] PCI: pciehp: Add macros for hotplug operation delays Hans Zhang
2025-08-26 17:03 ` [PATCH v3 8/8] PCI/DPC: Add macro for RP busy check delay Hans Zhang
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).