drivers/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
The msleep(2) may sleep up to 20ms due to timer granularity, which can
cause unnecessary delays. According to PCI spec v3.0 7.6.4.2, the minimum
Trst is 1ms and we doubled that to 2ms to meet the requirement. Using
usleep_range(2000, 2001) provides a more precise delay of exactly 2ms.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
Dear maintainers,
During the development process, I often check whether the file encoding meets the
basic rules. A warning appears when checking the following files:
./scripts/checkpatch.pl --no-tree --show-types --ignore EMAIL_SUBJECT,FILE_PATH_CHANGES,\
GERRIT_CHANGE_ID,UNDOCUMENTED_DT_STRING,TYPO_SPELLING -f drivers/pci/pci.c
WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
#4914: FILE: drivers/pci/pci.c:4914:
+ msleep(2);
In addition, I also found that the following documents all have similar problems.
Here, I'll first submit a patch to ask everyone. If it's necessary, I'll continue
to submit related patches later. If not, please ignore it.
WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
#4630: FILE: drivers/pci/pci.c:4630:
+ msleep(1);
WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
#342: FILE: drivers/pci/controller/pcie-rcar-host.c:342:
+ msleep(1);
WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
#1368: FILE: drivers/pci/controller/pcie-brcmstb.c:1368:
+ msleep(5);
WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
#42: FILE: drivers/pci/controller/pcie-rcar.c:42:
+ msleep(5);
WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
#106: FILE: drivers/pci/hotplug/pciehp_hpc.c:106:
+ msleep(10);
WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
#286: FILE: drivers/pci/hotplug/pciehp_hpc.c:286:
+ msleep(10);
WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
#138: FILE: drivers/pci/pcie/dpc.c:138:
+ msleep(10);
WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
#3970: FILE: drivers/pci/quirks.c:3970:
+ msleep(10);
---
drivers/pci/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b0f4d98036cd..ffe491635144 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4967,7 +4967,7 @@ void pci_reset_secondary_bus(struct pci_dev *dev)
* 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);
+ usleep_range(2000, 2001);
ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
base-commit: b19a97d57c15643494ac8bfaaa35e3ee472d41da
--
2.25.1
On Thu, Aug 21, 2025 at 12:09:44AM +0800, Hans Zhang wrote: > The msleep(2) may sleep up to 20ms due to timer granularity, which can > cause unnecessary delays. According to PCI spec v3.0 7.6.4.2, the minimum > Trst is 1ms and we doubled that to 2ms to meet the requirement. Using > usleep_range(2000, 2001) provides a more precise delay of exactly 2ms. > ... Please cite a recent spec version, i.e., r7.0. I see this probably came from the comment at the change; I wouldn't object to updating the comment, too. > WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep(). > #4630: FILE: drivers/pci/pci.c:4630: > + msleep(1); > ... > WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep(). > #3970: FILE: drivers/pci/quirks.c:3970: > + msleep(10); > +++ b/drivers/pci/pci.c > @@ -4967,7 +4967,7 @@ void pci_reset_secondary_bus(struct pci_dev *dev) > * 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); > + usleep_range(2000, 2001); IMO the most valuable thing here would be to replace the hard-coded "2" with some kind of #define explicitly tied to the spec. Similarly for the other cases. There is some concern [1] about places that say "msleep(1)" but actually rely on the current behavior of a longer sleep. Apart from that concern, I think fsleep() would be my first choice. usleep_range(x, x+1) defeats the purpose of the range, which is to reduce interrupts; see 5e7f5a178bba ("timer: Added usleep_range timer"). Bjorn [1] https://lore.kernel.org/all/20070809001640.ec2f3bfb.akpm@linux-foundation.org/
Dear Bjorn, Thank you very much for your reply. I'll try to understand your meaning and then submit the next version. If I haven't fully understood your meaning yet, please continue to help correct the mistakes. Best regards, Hans On 2025/8/21 02:06, Bjorn Helgaas wrote: > On Thu, Aug 21, 2025 at 12:09:44AM +0800, Hans Zhang wrote: >> The msleep(2) may sleep up to 20ms due to timer granularity, which can >> cause unnecessary delays. According to PCI spec v3.0 7.6.4.2, the minimum >> Trst is 1ms and we doubled that to 2ms to meet the requirement. Using >> usleep_range(2000, 2001) provides a more precise delay of exactly 2ms. >> ... > > Please cite a recent spec version, i.e., r7.0. I see this probably > came from the comment at the change; I wouldn't object to updating > the comment, too. > >> WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep(). >> #4630: FILE: drivers/pci/pci.c:4630: >> + msleep(1); >> ... >> WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep(). >> #3970: FILE: drivers/pci/quirks.c:3970: >> + msleep(10); > >> +++ b/drivers/pci/pci.c >> @@ -4967,7 +4967,7 @@ void pci_reset_secondary_bus(struct pci_dev *dev) >> * 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); >> + usleep_range(2000, 2001); > > IMO the most valuable thing here would be to replace the hard-coded > "2" with some kind of #define explicitly tied to the spec. Similarly > for the other cases. > > There is some concern [1] about places that say "msleep(1)" but > actually rely on the current behavior of a longer sleep. > > Apart from that concern, I think fsleep() would be my first choice. > usleep_range(x, x+1) defeats the purpose of the range, which is to > reduce interrupts; see 5e7f5a178bba ("timer: Added usleep_range > timer"). > > Bjorn > > [1] https://lore.kernel.org/all/20070809001640.ec2f3bfb.akpm@linux-foundation.org/
© 2016 - 2025 Red Hat, Inc.