Re: [PATCH v2] PCI: brcmstb: Restore initial fundamental reset

From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Jim Quinlan <james.quinlan@broadcom.com>,
	phil <phil@raspberrypi.org>
Cc: Phil Elwell <phil@raspberrypi.com>, Rob Herring <robh@kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" 
	<bcm-kernel-feedback-list@broadcom.com>,
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
	<linux-rpi-kernel@lists.infradead.org>,
	"open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS" 
	<linux-pci@vger.kernel.org>
Subject: Re: [PATCH v2] PCI: brcmstb: Restore initial fundamental reset
Date: Mon, 18 Jan 2021 13:44:44 +0100	[thread overview]
Message-ID: <9c50176681925fa06a0c1c385a3ab7f88a3faec7.camel@suse.de> (raw)
In-Reply-To: <20210118124003.GA12967@e121166-lin.cambridge.arm.com>

[-- Attachment #1: Type: text/plain, Size: 2185 bytes --]

On Mon, 2021-01-18 at 12:40 +0000, Lorenzo Pieralisi wrote:
> On Thu, Nov 12, 2020 at 01:38:13PM -0500, Jim Quinlan wrote:
> > On Thu, Nov 12, 2020 at 12:27 PM Phil Elwell <phil@raspberrypi.com> 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@raspberrypi.com>
> > > Acked-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > ---
> > > Changes in v2:
> > >   - Exclude BCM7278 from the initial reset
> > >   - Ack from Nicolas
> > > ---
> > >  drivers/pci/controller/pcie-brcmstb.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > > index bea86899bd5d..83aa85bfe8e3 100644
> > > --- a/drivers/pci/controller/pcie-brcmstb.c
> > > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > > @@ -869,6 +869,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> > > 
> > >         /* Reset the bridge */
> > >         pcie->bridge_sw_init_set(pcie, 1);
> > > +
> > > +       /* Assert the fundemental reset, except on BCM7278 */
> > > +       if (pcie->type != BCM7278)
> > > +               pcie->perst_set(pcie, 1);
> > I'm okay with this although I  would rather it not be needed.
> 
> Can I merge this patch as is then ?

No. IIUC the consensus was to fix this in firmware. There is a u-boot fix
available in their mailing list, and I think RPi's firmware was also patched
accordingly (@Phil please confirm).

Regards,
Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]