[PATCH] PCI: brcmstb: Restore initial fundamental reset

[Nicolas Saenz Julienne]

Hi Jim,

On Thu, 2020-11-12 at 11:28 -0500, Jim Quinlan wrote:
> On Thu, Nov 12, 2020 at 10:44 AM Florian Fainelli <f.fainelli at gmail.com> wrote:
> > +JimQ,
> > 
> > On 11/12/2020 5:14 AM, Phil Elwell wrote:
> > > Commit 04356ac30771 ("PCI: brcmstb: Add bcm7278 PERST# support")
> > > replaced a single reset function with a pointer to one of two
> > > implementations, but also removed the call asserting the reset
> > > at the start of brcm_pcie_setup. Doing so breaks Raspberry Pis with
> > > VL805 XHCI controllers lacking dedicated SPI EEPROMs, which have been
> > > used for USB booting but then need to be reset so that the kernel
> > > can reconfigure them. The lack of a reset causes the firmware's loading
> > > of the EEPROM image to RAM to fail, breaking USB for the kernel.
> > > 
> > > Fixes: commit 04356ac30771 ("PCI: brcmstb: Add bcm7278 PERST# support")
> > > 
> > > Signed-off-by: Phil Elwell <phil at raspberrypi.com>
> > 
> > This does indeed seem to have been lost during that commit, I will let
> > JimQ comment on whether this was intentional or not. Please make sure
> > you copy him, always, he wrote the driver after all.
> Hello,
> 
> This wasn't accidentally lost; I intentionally removed it.  I was
> remiss in not mentioning this in comments, sorry.
> 
> The reason I took it out is because (a) it breaks certain STB SOCs and
> (b) I considered it superfluous (see reason below).  At any rate, if
> you must restore this line please add the following guard so
> everyone's board will work :-)
> 
>         if (pcie->type != BCM7278)
>                 brcm_pcie_perst_set(pcie, 1);

This seems reasonable to me.

> As for me considering that  this line is superfluous -- which
> apparently it is not : AFAIK PERST# is always asserted from cold start
> on all Brcm STB SOCs, and I expected the same on the RPi.  Asserting
> PERST# at this point in time should be a no-op.  Is this not the case?

I introduced this with 22e21e51ce7 ("PCI: brcmstb: Assert fundamental reset on
initialization"). IIRC The story is the following:

- RPI's XHCI chip, which is connected to pcie-brcmstb, depends on a firmware
  blob. The blob is copied into memory, then the chip can access it anytime
  through PCIe inbound transfers.

- At any boot stage the pcie-brcmstb might be reconfigured with different
  inbound windows. This could be in RPi's custom bootloader, U-boot or Linux
  (or anything else).

- The only way we have to reset the XHCI chip so as for it to catch the new
  firmware blob addresses is through this PERST# line. Hence the need to toggle
  it every time the controller has been reconfigured.

Hope it makes some sense. :)

Regards,
Nicolas