Re: [PATCH v4 3/4] PCI: pciehp: Add macros for hotplug operation delays

[Hans Zhang]

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