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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx> > > > > 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