[PATCH v2 0/7] PCI: Replace short msleep() calls with more precise delay functions

[PATCH v2 0/7] PCI: Replace short msleep() calls with more precise delay functions

[Hans Zhang]

This series replaces short msleep() calls (less than 20ms) with more
precise delay functions (fsleep() and usleep_range()) throughout the
PCI subsystem.

The msleep() function with small values can sleep longer than intended
due to timer granularity, which can cause unnecessary delays in PCI
operations such as link status checking, reset handling, and hotplug
operations.

These changes:
- Use fsleep() for delays that require precise timing (1-2ms).
- Use usleep_range() for delays that can benefit from a small range.
- Add #defines for all delay values with references to PCIe specifications.
- Update comments to reference the latest PCIe r7.0 specification.

This improves the responsiveness of PCI operations while maintaining
compliance with PCIe specifications.

---
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 (7):
  PCI: Replace msleep(2) with fsleep() for precise delay
  PCI: Replace msleep(1) with fsleep() for precise link status checking
  PCI: rcar-host: Replace msleep(1) with fsleep() for precise speed
    change monitoring
  PCI: brcmstb: Replace msleep(5) with usleep_range() for precise link
    up checking
  PCI: rcar: Replace msleep(5) with usleep_range() for precise PHY ready
    checking
  PCI: pciehp: Replace msleep(10) with usleep_range() for precise delays
  PCI/DPC: Replace msleep(10) with usleep_range() for precise RP busy
    checking

 drivers/pci/controller/pcie-brcmstb.c   | 5 ++++-
 drivers/pci/controller/pcie-rcar-host.c | 4 +++-
 drivers/pci/controller/pcie-rcar.c      | 4 +++-
 drivers/pci/hotplug/pciehp_hpc.c        | 6 +++++-
 drivers/pci/pci.c                       | 9 +++------
 drivers/pci/pci.h                       | 7 +++++++
 drivers/pci/pcie/dpc.c                  | 5 ++++-
 7 files changed, 29 insertions(+), 11 deletions(-)


base-commit: b19a97d57c15643494ac8bfaaa35e3ee472d41da
-- 
2.25.1

[PATCH v2 1/7] PCI: Replace msleep(2) with fsleep() for precise delay

[Hans Zhang]

The msleep(2) may sleep longer than intended due to timer granularity.
According to PCIe r7.0 spec, section 7.5.1.3.13, the minimum Trst is 1ms.
We double this to 2ms to ensure we meet the requirement. Using fsleep()
provides a more precise delay.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/pci.c | 7 ++-----
 drivers/pci/pci.h | 6 ++++++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b0f4d98036cd..fb4aff520f64 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);
+	/* Wait for the reset to take effect */
+	fsleep(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 34f65d69662e..471dae45e46a 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -60,6 +60,12 @@ 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.
+ * Double this to 2ms to ensure we meet the minimum requirement.
+ */
+#define PCI_T_RST_SEC_BUS_DELAY_US	2000	/* 2ms */
+
 /* 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

[PATCH v2 2/7] PCI: Replace msleep(1) with fsleep() for precise link status checking

[Hans Zhang]

The msleep(1) in pcie_wait_for_link_status() may sleep longer than
intended due to timer granularity, which can cause unnecessary delays in
link status monitoring. Replace it with fsleep() for a more precise delay
of exactly 1ms.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/pci.c | 2 +-
 drivers/pci/pci.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index fb4aff520f64..fff49982b2a9 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4683,7 +4683,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);
+		fsleep(PCIE_LINK_STATUS_CHECK_US);
 	} while (time_before(jiffies, end_jiffies));
 
 	return -ETIMEDOUT;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 471dae45e46a..e9360d8ff81d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -16,6 +16,7 @@ struct pcie_tlp_log;
 #define PCI_VSEC_ID_INTEL_TBT	0x1234	/* Thunderbolt */
 
 #define PCIE_LINK_RETRAIN_TIMEOUT_MS	1000
+#define PCIE_LINK_STATUS_CHECK_US	1000
 
 /*
  * Power stable to PERST# inactive.
-- 
2.25.1

[PATCH v2 3/7] PCI: rcar-host: Replace msleep(1) with fsleep() for precise speed change monitoring

[Hans Zhang]

The msleep(1) in rcar_pcie_force_speedup() may sleep longer than intended
due to timer granularity, which can cause unnecessary delays in PCIe speed
change detection. Replace it with fsleep() for a more precise delay of
exactly 1ms.

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..5651285f1fb2 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_US	1000
+
 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);
+		fsleep(RCAR_SPEED_CHANGE_CHECK_US);
 	}
 
 	dev_err(dev, "Speed change timed out\n");
-- 
2.25.1

[PATCH v2 4/7] PCI: brcmstb: Replace msleep(5) with usleep_range() for precise link up checking

[Hans Zhang]

The msleep(5) in brcm_pcie_start_link() may sleep longer than intended
due to timer granularity, which can cause unnecessary delays in PCIe link
up detection. Replace it with usleep_range() for a more precise delay of
approximately 5ms.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 9afbd02ded35..6e34a052b02e 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 PCIE_LINK_UP_CHECK_US	5000
+
 /* Forward declarations */
 struct brcm_pcie;
 
@@ -1365,7 +1367,8 @@ 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);
+		usleep_range(PCIE_LINK_UP_CHECK_US,
+			     PCIE_LINK_UP_CHECK_US + 100);
 
 	if (!brcm_pcie_link_up(pcie)) {
 		dev_err(dev, "link down\n");
-- 
2.25.1

[PATCH v2 5/7] PCI: rcar: Replace msleep(5) with usleep_range() for precise PHY ready checking

[Hans Zhang]

The msleep(5) in rcar_pcie_wait_for_phyrdy() may sleep longer than
intended due to timer granularity, which can cause unnecessary delays
in PHY ready detection. Replace it with usleep_range() for a more precise
delay of approximately 5ms.

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..8d0f5a4a7fc0 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -11,6 +11,8 @@
 
 #include "pcie-rcar.h"
 
+#define PCIE_PHYRDY_CHECK_US	5000
+
 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);
+		usleep_range(PCIE_PHYRDY_CHECK_US, PCIE_PHYRDY_CHECK_US + 100);
 	}
 
 	return -ETIMEDOUT;
-- 
2.25.1

[PATCH v2 6/7] PCI: pciehp: Replace msleep(10) with usleep_range() for precise delays

[Hans Zhang]

The msleep(10) in pcie_poll_cmd() and pcie_wait_for_presence() may sleep
longer than intended due to timer granularity, which can cause unnecessary
delays in hotplug operations. Replace them with usleep_range() calls using
macros for more precise delays of approximately 10ms.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/hotplug/pciehp_hpc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index bcc51b26d03d..f9ae48b7b6ad 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_US	10000
+#define POLL_CMD_TIMEOUT_US	10000
+
 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);
+		usleep_range(POLL_CMD_TIMEOUT_US, POLL_CMD_TIMEOUT_US + 100);
 		timeout -= 10;
 	} while (timeout >= 0);
 	return 0;	/* timeout */
@@ -284,6 +287,7 @@ static void pcie_wait_for_presence(struct pci_dev *pdev)
 		if (slot_status & PCI_EXP_SLTSTA_PDS)
 			return;
 		msleep(10);
+		usleep_range(WAIT_PDS_TIMEOUT_US, WAIT_PDS_TIMEOUT_US + 100);
 		timeout -= 10;
 	} while (timeout > 0);
 }
-- 
2.25.1

[PATCH v2 7/7] PCI/DPC: Replace msleep(10) with usleep_range() for precise RP busy checking

[Hans Zhang]

The msleep(10) in dpc_wait_rp_inactive() may sleep longer than intended
due to timer granularity, which can cause unnecessary delays in downstream
port containment recovery. Replace it with usleep_range() for a more
precise delay of approximately 10ms.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/pcie/dpc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index fc18349614d7..ff0884d669b7 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_US	10000
+
 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,8 @@ 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);
+		usleep_range(PCIE_EXP_DPC_BUSY_CHECK_US,
+			     PCIE_EXP_DPC_BUSY_CHECK_US + 100);
 		pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
 	}
 	if (status & PCI_EXP_DPC_RP_BUSY) {
-- 
2.25.1

Re: [PATCH v2 0/7] PCI: Replace short msleep() calls with more precise delay functions

[Bjorn Helgaas]

On Fri, Aug 22, 2025 at 11:59:01PM +0800, Hans Zhang wrote:
> This series replaces short msleep() calls (less than 20ms) with more
> precise delay functions (fsleep() and usleep_range()) throughout the
> PCI subsystem.
> 
> The msleep() function with small values can sleep longer than intended
> due to timer granularity, which can cause unnecessary delays in PCI
> operations such as link status checking, reset handling, and hotplug
> operations.
> 
> These changes:
> - Use fsleep() for delays that require precise timing (1-2ms).
> - Use usleep_range() for delays that can benefit from a small range.
> - Add #defines for all delay values with references to PCIe specifications.
> - Update comments to reference the latest PCIe r7.0 specification.
> 
> This improves the responsiveness of PCI operations while maintaining
> compliance with PCIe specifications.

I would split this a little differently:

  - Add #defines for values from PCIe base spec.  Make the #define
    value match the spec value.  If there's adjustment, e.g.,
    doubling, do it at the sleep site.  Adjustment like this seems a
    little paranoid since the spec should already have some margin
    built into it.

  - Change to fsleep() (or usleep_range()) in separate patch.  There
    might be discussion about these changes, but the #defines are
    desirable regardless.

I'm personally dubious about the places you used usleep_range().
These are low-frequency paths (rcar PHY ready, brcmstb link up,
hotplug command completion, DPC recover) that don't seem critical.  I
think they're all using made-up delays that don't come from any spec
or hardware requirement anyway.  I think it's hard to make an argument
for precision here.

Bjorn

Re: [PATCH v2 0/7] PCI: Replace short msleep() calls with more precise delay functions

[Hans Zhang]

On 2025/8/23 00:46, Bjorn Helgaas wrote:
> On Fri, Aug 22, 2025 at 11:59:01PM +0800, Hans Zhang wrote:
>> This series replaces short msleep() calls (less than 20ms) with more
>> precise delay functions (fsleep() and usleep_range()) throughout the
>> PCI subsystem.
>>
>> The msleep() function with small values can sleep longer than intended
>> due to timer granularity, which can cause unnecessary delays in PCI
>> operations such as link status checking, reset handling, and hotplug
>> operations.
>>
>> These changes:
>> - Use fsleep() for delays that require precise timing (1-2ms).
>> - Use usleep_range() for delays that can benefit from a small range.
>> - Add #defines for all delay values with references to PCIe specifications.
>> - Update comments to reference the latest PCIe r7.0 specification.
>>
>> This improves the responsiveness of PCI operations while maintaining
>> compliance with PCIe specifications.
> 

Dear Bjron,

Thank you very much for your reply.


> I would split this a little differently:
> 
>    - Add #defines for values from PCIe base spec.  Make the #define
>      value match the spec value.  If there's adjustment, e.g.,

Ok. For patch 0001, I will modify it again.

>      doubling, do it at the sleep site.  Adjustment like this seems a
>      little paranoid since the spec should already have some margin
>      built into it.

Can I understand that it's enough to just use fsleep(1000) here?

patch 0001 I intend to modify it as follows:

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b0f4d98036cd..fb4aff520f64 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);
+       /* Wait for the reset to take effect */
+       fsleep(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 34f65d69662e..9d38ef26c6a9 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_US     1000
+
  /* 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



> 
>    - Change to fsleep() (or usleep_range()) in separate patch.  There
>      might be discussion about these changes, but the #defines are
>      desirable regardless.
> 
> I'm personally dubious about the places you used usleep_range().
> These are low-frequency paths (rcar PHY ready, brcmstb link up,
> hotplug command completion, DPC recover) that don't seem critical.  I
> think they're all using made-up delays that don't come from any spec
> or hardware requirement anyway.  I think it's hard to make an argument
> for precision here.

My initial understanding was the same. There was no need for such 
precision here. Then msleep will be retained, but only modified to #defines?

If you think it's unnecessary, I will discard the remaining patches.

Best regards,
Hans

Re: [PATCH v2 0/7] PCI: Replace short msleep() calls with more precise delay functions

[Bjorn Helgaas]

On Mon, Aug 25, 2025 at 12:05:26AM +0800, Hans Zhang wrote:
> On 2025/8/23 00:46, Bjorn Helgaas wrote:
> > On Fri, Aug 22, 2025 at 11:59:01PM +0800, Hans Zhang wrote:
> > > This series replaces short msleep() calls (less than 20ms) with more
> > > precise delay functions (fsleep() and usleep_range()) throughout the
> > > PCI subsystem.
> > > 
> > > The msleep() function with small values can sleep longer than intended
> > > due to timer granularity, which can cause unnecessary delays in PCI
> > > operations such as link status checking, reset handling, and hotplug
> > > operations.

> > I would split this a little differently:
> > 
> >    - Add #defines for values from PCIe base spec.  Make the #define
> >      value match the spec value.  If there's adjustment, e.g.,
> >      doubling, do it at the sleep site.  Adjustment like this seems a
> >      little paranoid since the spec should already have some margin
> >      built into it.

> patch 0001 I intend to modify it as follows:
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b0f4d98036cd..fb4aff520f64 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);
> +       /* Wait for the reset to take effect */
> +       fsleep(PCI_T_RST_SEC_BUS_DELAY_US);

This mixes 3 changes:

  1) Add #define PCI_T_RST_SEC_BUS_DELAY_US

  2) Reduce overall delay from 2ms to 1ms

  3) Convert msleep() to fsleep()

There's no issue at all with 1), and I don't know if it's really worth
doing 2), so I would do this:

  - msleep(2);
  + msleep(2 * PCI_T_RST_SEC_BUS_DELAY_MS);

Then we can consider the question of whether "msleep(2)" is misleading
to the reader because the actual delay is always > 20ms.  If that's
the case, I would consider a separate patch like this:

  - msleep(2 * PCI_T_RST_SEC_BUS_DELAY_MS);
  + fsleep(2 * PCI_T_RST_SEC_BUS_DELAY_US);

to make the stated intent of the code closer to the actual behavior.
If we do this, the commit log should include concrete details about
why short msleep() doesn't work as advertised.

> > I'm personally dubious about the places you used usleep_range().
> > These are low-frequency paths (rcar PHY ready, brcmstb link up,
> > hotplug command completion, DPC recover) that don't seem critical.  I
> > think they're all using made-up delays that don't come from any spec
> > or hardware requirement anyway.  I think it's hard to make an argument
> > for precision here.
> 
> My initial understanding was the same. There was no need for such precision
> here. Then msleep will be retained, but only modified to #defines?

The #defines are useful when (1) the value comes from a spec or (2) we
want to use the same value several places.  Otherwise, the value is
minimal.

For rcar PHY ready, brcmstb link up, hotplug command completion, DPC
recover, I don't think either applies, so personally I would probably
leave them alone (or, if we think short msleep() is just misleading in
principle, convert them to fsleep()).

Bjorn

Re: [PATCH v2 0/7] PCI: Replace short msleep() calls with more precise delay functions

[Hans Zhang]

On 2025/8/23 00:46, Bjorn Helgaas wrote:
> On Fri, Aug 22, 2025 at 11:59:01PM +0800, Hans Zhang wrote:
>> This series replaces short msleep() calls (less than 20ms) with more
>> precise delay functions (fsleep() and usleep_range()) throughout the
>> PCI subsystem.
>>
>> The msleep() function with small values can sleep longer than intended
>> due to timer granularity, which can cause unnecessary delays in PCI
>> operations such as link status checking, reset handling, and hotplug
>> operations.
>>
>> These changes:
>> - Use fsleep() for delays that require precise timing (1-2ms).
>> - Use usleep_range() for delays that can benefit from a small range.
>> - Add #defines for all delay values with references to PCIe specifications.
>> - Update comments to reference the latest PCIe r7.0 specification.
>>
>> This improves the responsiveness of PCI operations while maintaining
>> compliance with PCIe specifications.
> 
> I would split this a little differently:
> 
>    - Add #defines for values from PCIe base spec.  Make the #define
>      value match the spec value.  If there's adjustment, e.g.,
>      doubling, do it at the sleep site.  Adjustment like this seems a
>      little paranoid since the spec should already have some margin
>      built into it.
> 
>    - Change to fsleep() (or usleep_range()) in separate patch.  There
>      might be discussion about these changes, but the #defines are
>      desirable regardless.
> 
> I'm personally dubious about the places you used usleep_range().
> These are low-frequency paths (rcar PHY ready, brcmstb link up,
> hotplug command completion, DPC recover) that don't seem critical.  I
> think they're all using made-up delays that don't come from any spec
> or hardware requirement anyway.  I think it's hard to make an argument
> for precision here.
> 

Dear Bjorn,

Thank you very much for your reply.

I have revised version v3 based on your review comments. Please continue 
your review. Thank you very much.

Best regards,
Hans

> Bjorn