Re: [PATCH v5 2/4] PCI: Use standard wait times for PCIe link monitoring

[Thierry Reding <thierry.reding@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]