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

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

[Marek Vasut]

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.

Because it is the controller driver which can determine whether or not the
controller is in HSC domain based on its compatible string, add the missing
delay into the controller driver.

This 1ms delay is documented on R-Car V4H and V4M, it is currently unclear
whether S4 is affected as well. This patch does apply the extra delay on
R-Car S4 as well.

Fixes: 0d0c551011df ("PCI: rcar-gen4: Add R-Car Gen4 PCIe controller support for host mode")
Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: Use fsleep() instead of usleep_range()
---
 drivers/pci/controller/dwc/pcie-rcar-gen4.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index de9fe2ed2423d..db2d30fb5d904 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -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);
+		fsleep(1000);
+	}
 
 	val = readl(rcar->base + PCIEMSR0);
 	if (rcar->drvdata->mode == DW_PCIE_RC_TYPE) {
-- 
2.51.0

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

[Krzysztof Wilczyński]

Hello,

[...]
> Because it is the controller driver which can determine whether or not the
> controller is in HSC domain based on its compatible string, add the missing
> delay into the controller driver.
> 
> This 1ms delay is documented on R-Car V4H and V4M, it is currently unclear
> whether S4 is affected as well. This patch does apply the extra delay on
> R-Car S4 as well.

[...]
> -	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);
> +		fsleep(1000);
> +	}

Would it be a prudent thing to do here to add a comment over the fsleep()
or over the if-statement to document briefly (and for posterity) why we
sleep here?

Otherwise, looks good! Thank you for fixing this potential issue.

Reviewed-by: Krzysztof Wilczyński <kwilczynski@kernel.org>

Thank you,

	Krzysztof

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

[Manivannan Sadhasivam]

On Fri, Sep 19, 2025 at 07:00:38PM +0900, Krzysztof Wilczyński wrote:
> Hello,
> 
> [...]
> > Because it is the controller driver which can determine whether or not the
> > controller is in HSC domain based on its compatible string, add the missing
> > delay into the controller driver.
> > 
> > This 1ms delay is documented on R-Car V4H and V4M, it is currently unclear
> > whether S4 is affected as well. This patch does apply the extra delay on
> > R-Car S4 as well.
> 

What are the implications of not having the delay? Just asking to determine if
this patch is a stable candidate or not.

> [...]
> > -	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);
> > +		fsleep(1000);
> > +	}
> 
> Would it be a prudent thing to do here to add a comment over the fsleep()
> or over the if-statement to document briefly (and for posterity) why we
> sleep here?
> 

I can add the quote to the reference manual while applying.

> Otherwise, looks good! Thank you for fixing this potential issue.
> 
> Reviewed-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
> 

Thanks!

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

[Marek Vasut]

On 9/19/25 2:33 PM, Manivannan Sadhasivam wrote:

Hello Manivannan,

>>> Because it is the controller driver which can determine whether or not the
>>> controller is in HSC domain based on its compatible string, add the missing
>>> delay into the controller driver.
>>>
>>> This 1ms delay is documented on R-Car V4H and V4M, it is currently unclear
>>> whether S4 is affected as well. This patch does apply the extra delay on
>>> R-Car S4 as well.
>>
> 
> What are the implications of not having the delay? Just asking to determine if
> this patch is a stable candidate or not.

Observable, thus far, none. However, the current behavior does not 
follow the reference manual operations chart, so I think this should be 
in stable, else the hardware is operated out of specification.

>> [...]
>>> -	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);
>>> +		fsleep(1000);
>>> +	}
>>
>> Would it be a prudent thing to do here to add a comment over the fsleep()
>> or over the if-statement to document briefly (and for posterity) why we
>> sleep here?
>>
> 
> I can add the quote to the reference manual while applying.
I'll send a V3 with a comment shortly.