* [PATCH] PCI: Replace msleep(2) with usleep_range() for precise delay
@ 2025-08-20 16:09 Hans Zhang
2025-08-20 18:06 ` Bjorn Helgaas
0 siblings, 1 reply; 3+ messages in thread
From: Hans Zhang @ 2025-08-20 16:09 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel; +Cc: Hans Zhang
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] PCI: Replace msleep(2) with usleep_range() for precise delay
2025-08-20 16:09 [PATCH] PCI: Replace msleep(2) with usleep_range() for precise delay Hans Zhang
@ 2025-08-20 18:06 ` Bjorn Helgaas
2025-08-22 15:53 ` Hans Zhang
0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2025-08-20 18:06 UTC (permalink / raw)
To: Hans Zhang; +Cc: bhelgaas, linux-pci, linux-kernel
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/
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PCI: Replace msleep(2) with usleep_range() for precise delay
2025-08-20 18:06 ` Bjorn Helgaas
@ 2025-08-22 15:53 ` Hans Zhang
0 siblings, 0 replies; 3+ messages in thread
From: Hans Zhang @ 2025-08-22 15:53 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: bhelgaas, linux-pci, linux-kernel
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/
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-22 15:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 16:09 [PATCH] PCI: Replace msleep(2) with usleep_range() for precise delay Hans Zhang
2025-08-20 18:06 ` Bjorn Helgaas
2025-08-22 15:53 ` 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).