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

[Marek Vasut <marek.vasut@mailbox.org>]

On 9/18/25 10:44 PM, Bjorn Helgaas wrote:
> 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.

Lemme do V2 with fsleep() then.

> 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.

You're right about this part.