Re: [PATCH v5 2/4] PCI: Use standard wait times for PCIe link monitoring
From: Thierry Reding
Date: Thu May 28 2026 - 02:44:44 EST
On Wed, May 27, 2026 at 07:22:55PM +0200, Lukas Wunner wrote:
> On Wed, May 27, 2026 at 10:28:51AM +0200, Thierry Reding wrote:
> > On Tue, May 26, 2026 at 01:10:39PM +0200, Lukas Wunner wrote:
> > > On Tue, May 26, 2026 at 10:53:11AM +0200, Thierry Reding wrote:
> > > > Instead of defining the wait values for each driver, use common values
> > > > defined in the core pci.h header file. Note that most drivers don't use
> > > > the millisecond waits, but rather usleep_range(), so add these commonly
> > > > used values to the header so that all drivers can use them.
> > >
> > > Hm, why not just replace usleep_range() with msleep() and use the existing
> > > macro instead of defining new ones?
> >
> > I'm trying to somewhat unify the implementations across drivers without
> > changing behaviour. I don't have a way of testing any of these drivers,
> > so keeping the existing implementation and just switching out the symbol
> > seemed like a good compromise.
>
> It looks like the code was copy-pasted from one driver to the next
> and I applaud every effort to deduplicate, unify and simplify things.
>
> I don't quite see how using msleep() instead of usleep_range()
> should result in breakage. The drivers are just polling for
> link up for an amount of time and as long as you're not decreasing
> that amount, everything ought to be fine. Moving to msleep()
> should never decrease the amount.
>
> > Then again, I count 2 drivers (in linux-next) that use the existing
> > PCIE_LINK_WAIT_SLEEP_MS, one of which multiplies by MILLI to get at the
> > US version (so it could easily be converted to the US_MIN version). Only
> > pcie-designware.c uses msleep() with PCIE_LINK_WAIT_SLEEP_MS, so it is
> > clearly the outlier.
>
> Konrad Dybcio went ahead and replaced usleep_range() with msleep()
> in that driver, kudos to him. Cf. b262518262f5 ("PCI: dwc: Use msleep()
> in dw_pcie_wait_for_link()"). If you could just carry over that change
> to the other drivers that would be great.
Alright, I see you really don't like usleep_range(), so msleep() it'll
be.
Thierry
Attachment:
signature.asc
Description: PGP signature