* [PATCH v4 0/4] PCI: Add delay macros for better code readability and maintainability
@ 2025-11-01 16:05 Hans Zhang
2025-11-01 16:05 ` [PATCH v4 1/4] PCI: Add macro for secondary bus reset delay Hans Zhang
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Hans Zhang @ 2025-11-01 16:05 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.
---
Hi Bjorn,
I wonder if there is still a need to advance this series? If it's not necessary,
please drop it. Thank you very much.
Best regards
Hans
Changes for v4:
https://patchwork.kernel.org/project/linux-pci/patch/20250826170315.721551-1-18255117159@163.com/
- According to Bjorn's feedback, the benefits of using fsleep are not significant.
drop the 0002 patch in v3. (Bjorn)
- For the controller drivers, the added macros do no good and provide no value.
So if you ever respin this series, you can drop them. (Mani)
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 (4):
PCI: Add macro for secondary bus reset delay
PCI: Add macro for link status check delay
PCI: pciehp: Add macros for hotplug operation delays
PCI/DPC: Add macro for RP busy check delay
drivers/pci/hotplug/pciehp_hpc.c | 7 +++++--
drivers/pci/pci.c | 11 +++++------
drivers/pci/pci.h | 3 +++
drivers/pci/pcie/dpc.c | 4 +++-
4 files changed, 16 insertions(+), 9 deletions(-)
base-commit: ba36dd5ee6fd4643ebbf6ee6eefcecf0b07e35c7
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 1/4] PCI: Add macro for secondary bus reset delay
2025-11-01 16:05 [PATCH v4 0/4] PCI: Add delay macros for better code readability and maintainability Hans Zhang
@ 2025-11-01 16:05 ` Hans Zhang
2025-11-05 15:56 ` Hans Zhang
2025-11-01 16:05 ` [PATCH v4 2/4] PCI: Add macro for link status check delay Hans Zhang
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Hans Zhang @ 2025-11-01 16:05 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 b14dd064006c..86449f2d627b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4912,11 +4912,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 4492b809094b..31f975619774 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -63,6 +63,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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 2/4] PCI: Add macro for link status check delay
2025-11-01 16:05 [PATCH v4 0/4] PCI: Add delay macros for better code readability and maintainability Hans Zhang
2025-11-01 16:05 ` [PATCH v4 1/4] PCI: Add macro for secondary bus reset delay Hans Zhang
@ 2025-11-01 16:05 ` Hans Zhang
2025-11-03 15:39 ` Bjorn Helgaas
2025-11-01 16:05 ` [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays Hans Zhang
2025-11-01 16:05 ` [PATCH v4 4/4] PCI/DPC: Add macro for RP busy check delay Hans Zhang
3 siblings, 1 reply; 13+ messages in thread
From: Hans Zhang @ 2025-11-01 16:05 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 86449f2d627b..b57d8e4c3a48 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);
@@ -4632,7 +4634,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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays
2025-11-01 16:05 [PATCH v4 0/4] PCI: Add delay macros for better code readability and maintainability Hans Zhang
2025-11-01 16:05 ` [PATCH v4 1/4] PCI: Add macro for secondary bus reset delay Hans Zhang
2025-11-01 16:05 ` [PATCH v4 2/4] PCI: Add macro for link status check delay Hans Zhang
@ 2025-11-01 16:05 ` Hans Zhang
2025-11-01 16:13 ` Christophe JAILLET
2025-11-03 15:37 ` Bjorn Helgaas
2025-11-01 16:05 ` [PATCH v4 4/4] PCI/DPC: Add macro for RP busy check delay Hans Zhang
3 siblings, 2 replies; 13+ messages in thread
From: Hans Zhang @ 2025-11-01 16:05 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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 4/4] PCI/DPC: Add macro for RP busy check delay
2025-11-01 16:05 [PATCH v4 0/4] PCI: Add delay macros for better code readability and maintainability Hans Zhang
` (2 preceding siblings ...)
2025-11-01 16:05 ` [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays Hans Zhang
@ 2025-11-01 16:05 ` Hans Zhang
3 siblings, 0 replies; 13+ messages in thread
From: Hans Zhang @ 2025-11-01 16:05 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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays
2025-11-01 16:05 ` [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays Hans Zhang
@ 2025-11-01 16:13 ` Christophe JAILLET
2025-11-01 16:16 ` Hans Zhang
2025-11-03 15:37 ` Bjorn Helgaas
1 sibling, 1 reply; 13+ messages in thread
From: Christophe JAILLET @ 2025-11-01 16:13 UTC (permalink / raw)
To: Hans Zhang, bhelgaas, helgaas, linux-pci, linux-kernel
Le 01/11/2025 à 17:05, Hans Zhang a écrit :
> 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;
Should we have: timeout -= POLL_CMD_TIMEOUT_MS;
?
> } 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);
Same with WAIT_PDS_TIMEOUT_MS.
CJ
> timeout -= 10;
> } while (timeout > 0);
> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays
2025-11-01 16:13 ` Christophe JAILLET
@ 2025-11-01 16:16 ` Hans Zhang
0 siblings, 0 replies; 13+ messages in thread
From: Hans Zhang @ 2025-11-01 16:16 UTC (permalink / raw)
To: Christophe JAILLET, bhelgaas, helgaas, linux-pci, linux-kernel
On 2025/11/2 00:13, Christophe JAILLET wrote:
> Le 01/11/2025 à 17:05, Hans Zhang a écrit :
>> 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;
>
> Should we have: timeout -= POLL_CMD_TIMEOUT_MS;
>
> ?
Hi Christophe,
Thank you very much for your reply and reminder.
If Bjorn finds it meaningful, I will fix it in the next version.
>
>> } 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);
>
> Same with WAIT_PDS_TIMEOUT_MS.
>
Will change.
Best regards,
Hans
> CJ
>
>> timeout -= 10;
>> } while (timeout > 0);
>> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays
2025-11-01 16:05 ` [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays Hans Zhang
2025-11-01 16:13 ` Christophe JAILLET
@ 2025-11-03 15:37 ` Bjorn Helgaas
2025-11-03 17:24 ` Lukas Wunner
1 sibling, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2025-11-03 15:37 UTC (permalink / raw)
To: Hans Zhang; +Cc: bhelgaas, linux-pci, linux-kernel, Lukas Wunner
[+cc Lukas]
On Sun, Nov 02, 2025 at 12:05:37AM +0800, Hans Zhang wrote:
> 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);
Lukas might have different opinions and I would defer to him here.
But IMO (a) these aren't timeouts, they are poll intervals, (b) the
values are arbitrary with no connection to a spec, so less reason for
a #define, and (c) the #defines don't improve readability because now
I have to look at two places to understand the poll loops.
> 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.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/4] PCI: Add macro for link status check delay
2025-11-01 16:05 ` [PATCH v4 2/4] PCI: Add macro for link status check delay Hans Zhang
@ 2025-11-03 15:39 ` Bjorn Helgaas
2025-11-05 15:54 ` Hans Zhang
0 siblings, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2025-11-03 15:39 UTC (permalink / raw)
To: Hans Zhang; +Cc: bhelgaas, linux-pci, linux-kernel
On Sun, Nov 02, 2025 at 12:05:36AM +0800, Hans Zhang wrote:
> 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 86449f2d627b..b57d8e4c3a48 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);
> @@ -4632,7 +4634,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);
Doesn't seem worth it to me.
> } while (time_before(jiffies, end_jiffies));
>
> return -ETIMEDOUT;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays
2025-11-03 15:37 ` Bjorn Helgaas
@ 2025-11-03 17:24 ` Lukas Wunner
2025-11-05 15:54 ` Hans Zhang
0 siblings, 1 reply; 13+ messages in thread
From: Lukas Wunner @ 2025-11-03 17:24 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Hans Zhang, bhelgaas, linux-pci, linux-kernel
On Mon, Nov 03, 2025 at 09:37:34AM -0600, Bjorn Helgaas wrote:
> On Sun, Nov 02, 2025 at 12:05:37AM +0800, Hans Zhang wrote:
> > Add WAIT_PDS_TIMEOUT_MS and POLL_CMD_TIMEOUT_MS macros for hotplug
> > operation delays to improve code readability.
[...]
> > +++ 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);
>
> Lukas might have different opinions and I would defer to him here.
>
> But IMO (a) these aren't timeouts, they are poll intervals, (b) the
> values are arbitrary with no connection to a spec, so less reason for
> a #define, and (c) the #defines don't improve readability because now
> I have to look at two places to understand the poll loops.
I agree on all counts.
Thanks,
Lukas
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays
2025-11-03 17:24 ` Lukas Wunner
@ 2025-11-05 15:54 ` Hans Zhang
0 siblings, 0 replies; 13+ messages in thread
From: Hans Zhang @ 2025-11-05 15:54 UTC (permalink / raw)
To: Lukas Wunner, Bjorn Helgaas; +Cc: bhelgaas, linux-pci, linux-kernel
On 2025/11/4 01:24, Lukas Wunner wrote:
> On Mon, Nov 03, 2025 at 09:37:34AM -0600, Bjorn Helgaas wrote:
>> On Sun, Nov 02, 2025 at 12:05:37AM +0800, Hans Zhang wrote:
>>> Add WAIT_PDS_TIMEOUT_MS and POLL_CMD_TIMEOUT_MS macros for hotplug
>>> operation delays to improve code readability.
> [...]
>>> +++ 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);
>>
>> Lukas might have different opinions and I would defer to him here.
>>
>> But IMO (a) these aren't timeouts, they are poll intervals, (b) the
>> values are arbitrary with no connection to a spec, so less reason for
>> a #define, and (c) the #defines don't improve readability because now
>> I have to look at two places to understand the poll loops.
>
> I agree on all counts.
>
Hi Bjorn,
Please drop this patch.
Best regards,
Hans
> Thanks,
>
> Lukas
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/4] PCI: Add macro for link status check delay
2025-11-03 15:39 ` Bjorn Helgaas
@ 2025-11-05 15:54 ` Hans Zhang
0 siblings, 0 replies; 13+ messages in thread
From: Hans Zhang @ 2025-11-05 15:54 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: bhelgaas, linux-pci, linux-kernel
On 2025/11/3 23:39, Bjorn Helgaas wrote:
> On Sun, Nov 02, 2025 at 12:05:36AM +0800, Hans Zhang wrote:
>> 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 86449f2d627b..b57d8e4c3a48 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);
>> @@ -4632,7 +4634,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);
>
> Doesn't seem worth it to me.
Hi Bjorn,
Please drop this patch.
Best regards,
Hans
>
>> } while (time_before(jiffies, end_jiffies));
>>
>> return -ETIMEDOUT;
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/4] PCI: Add macro for secondary bus reset delay
2025-11-01 16:05 ` [PATCH v4 1/4] PCI: Add macro for secondary bus reset delay Hans Zhang
@ 2025-11-05 15:56 ` Hans Zhang
0 siblings, 0 replies; 13+ messages in thread
From: Hans Zhang @ 2025-11-05 15:56 UTC (permalink / raw)
To: bhelgaas, helgaas, linux-pci, linux-kernel
Hi Bjorn,
I wonder if this is still necessary? If not, please drop it as well. Thanks.
Best regards,
Hans
On 2025/11/2 00:05, Hans Zhang wrote:
> 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 b14dd064006c..86449f2d627b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4912,11 +4912,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 4492b809094b..31f975619774 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -63,6 +63,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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-11-05 15:56 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-01 16:05 [PATCH v4 0/4] PCI: Add delay macros for better code readability and maintainability Hans Zhang
2025-11-01 16:05 ` [PATCH v4 1/4] PCI: Add macro for secondary bus reset delay Hans Zhang
2025-11-05 15:56 ` Hans Zhang
2025-11-01 16:05 ` [PATCH v4 2/4] PCI: Add macro for link status check delay Hans Zhang
2025-11-03 15:39 ` Bjorn Helgaas
2025-11-05 15:54 ` Hans Zhang
2025-11-01 16:05 ` [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays Hans Zhang
2025-11-01 16:13 ` Christophe JAILLET
2025-11-01 16:16 ` Hans Zhang
2025-11-03 15:37 ` Bjorn Helgaas
2025-11-03 17:24 ` Lukas Wunner
2025-11-05 15:54 ` Hans Zhang
2025-11-01 16:05 ` [PATCH v4 4/4] 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