Re: [PATCH] PCI: rcar-gen4: Fix inverted break condition in PHY initialization

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Sep 16, 2025 at 07:39:07PM +0200, Marek Vasut wrote:
> On 9/16/25 7:13 PM, Bjorn Helgaas wrote:
> > On Tue, Sep 16, 2025 at 06:31:31PM +0200, Marek Vasut wrote:
> > > On 9/16/25 3:57 PM, Geert Uytterhoeven wrote:
> > > > On Tue, 16 Sept 2025 at 15:39, Marek Vasut <marek.vasut@mailbox.org> wrote:
> > > > > On 9/16/25 11:59 AM, Geert Uytterhoeven wrote:
> > 
> > > > > > This change looks correct, and fixes the timeout seen on White Hawk
> > > > > > with CONFIG_DEBUG_LOCK_ALLOC=y.
> > > > > > However, it causes a crash when CONFIG_DEBUG_LOCK_ALLOC=n:
> > > > > > 
> > > > > >        SError Interrupt on CPU0, code 0x00000000be000011 -- SError
> > > > > 
> > > > > ...
> > > > > 
> > > > > >         el1h_64_error_handler+0x2c/0x40
> > > > > >         el1h_64_error+0x70/0x74
> > > > > >         dw_pcie_read+0x20/0x74 (P)
> > > > > >         rcar_gen4_pcie_additional_common_init+0x1c/0x6c
> > > > > 
> > > > > SError in rcar_gen4_pcie_additional_common_init , this is unrelated to
> > > > > this fix.
> > > > > 
> > > > > Does the following patch make this SError go away ?
> > > > 
> > > > > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > > > > @@ -204,6 +204,8 @@ static int rcar_gen4_pcie_common_init(struct
> > > > > rcar_gen4_pcie *rcar)
> > > > >            if (ret)
> > > > >                    goto err_unprepare;
> > > > > 
> > > > > +mdelay(1);
> > > > > +
> > > > >            if (rcar->drvdata->additional_common_init)
> > > > >                    rcar->drvdata->additional_common_init(rcar);
> > > > > 
> > > > 
> > > > Yes it does, thanks!
> > > 
> > > So with this one extra mdelay(1), the PCIe is fully good on your side, or is
> > > there still anything weird/flaky/malfunctioning ?
> > 
> > Do we have a theory about why this delay is needed?  I assume it's
> > something specific to the rcar hardware (not something required by the
> > PCIe base spec)?
> > 
> > I'm seeing SError interrupts and external aborts on several arm64
> > systems and I'd like to understand better why they happen and when/if
> > we can recover from them.
>
> The SError here happens when the PWR RST is released and DBI is
> accessed rapidly right after that. The current hypothesis is, that
> the controller core needs a bit of time to initialize itself after
> the reset is released, before DBI access can be performed ; if the
> DBI access happens too soon after the reset was released, the core
> reject the access and CPU interprets that as SError.

Sounds like this SError is for something on the CPU side of the host
bridge.

I've also seen errors that seem to be related to errors on the PCIe
side, e.g., a PCIe Completion Timeout or Unsupported Request.  On most
platforms, those result in writes being silently dropped or ~0 data
being synthesized for reads.

E.g., this SError that seems related to a BAR programming issue:
https://lore.kernel.org/linux-pci/86plf0lgit.fsf@scott-ph-mail.amperecomputing.com/

> The reference manual however does not list any such delay, which
> makes me concerned. I can send such a patch which adds the mdelay(1)
> as a temporary stopgap fix, until some better explanation maybe
> sometimes gets uncovered, if that would be OK ?

Yeah, if it's some rcar-specific thing, I don't see a better
alternative.

Bjorn