Re: [PATCH] PCI: rcar-gen4: Add missing 1ms delay after PWR reset assertion

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Sep 18, 2025 at 10:35:08PM +0200, Marek Vasut wrote:
> On 9/18/25 10:04 PM, Bjorn Helgaas wrote:
> > On Thu, Sep 18, 2025 at 05:00:26AM +0200, Marek Vasut wrote:
> > > R-Car V4H Reference Manual R19UH0186EJ0130 Rev.1.30 Apr. 21, 2025 page 585
> > > Figure 9.3.2 Software Reset flow (B) indicates that for peripherals in HSC
> > > domain, after reset has been asserted by writing a matching reset bit into
> > > register SRCR, it is mandatory to wait 1ms.
> > 
> > > @@ -182,8 +182,10 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar)
> > >   		return ret;
> > >   	}
> > > -	if (!reset_control_status(dw->core_rsts[DW_PCIE_PWR_RST].rstc))
> > > +	if (!reset_control_status(dw->core_rsts[DW_PCIE_PWR_RST].rstc)) {
> > >   		reset_control_assert(dw->core_rsts[DW_PCIE_PWR_RST].rstc);
> > > +		usleep_range(1000, 2000);
> > 
> > What would you think of "fsleep(1000)"?
> > 
> > I know there's controvery about fsleep(), but while the 1000 usec
> > lower bound is important, I think the selection of the 2000 usec upper
> > bound is pretty arbitrary and doesn't really justify spelling it out.
>
> The upper bound is arbitrary.
> 
> My reasoning for picking up usleep_range() is to give the kernel
> sufficient space to pick the best fitting delay in that 1..2 ms
> range, without constraining the timers too much. In other words, let
> the kernel pick the next easy to use timer tick which guarantees at
> least 1ms delay.

Right, basically the same motivation as fsleep().

> As far as I can tell, fsleep() in this case would add a bit of
> auto-detection overhead, and then select equivalent of
> usleep_range(1000, 1250) , wouldn't it ?
> 
> So I think using fsleep() here would add overhead, but not yield any
> actual benefit. Is my understanding and conclusions correct ?

I think you're right that fsleep() will pick usleep_range(1000, 1250),
so it's less optimal in that sense, but I think optimization like that
would be better done inside fsleep() instead of everybody doing it ad
hoc at the call site.

I don't think fsleep() should add any overhead since it's inlined and
all the delays are constants.  But I haven't actually looked at the
generated code.