linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] PCI: brcmstb: Restore initial fundamental reset
@ 2020-11-12 17:27 Phil Elwell
  2020-11-12 18:05 ` Stefan Wahren
  2020-11-12 18:38 ` Jim Quinlan
  0 siblings, 2 replies; 6+ messages in thread
From: Phil Elwell @ 2020-11-12 17:27 UTC (permalink / raw)
  To: Jim Quinlan, Nicolas Saenz Julienne, Rob Herring,
	Florian Fainelli, bcm-kernel-feedback-list, linux-rpi-kernel,
	linux-pci
  Cc: Phil Elwell

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);
+
 	usleep_range(100, 200);
 
 	/* Take the bridge out of reset */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] PCI: brcmstb: Restore initial fundamental reset
  2020-11-12 17:27 [PATCH v2] PCI: brcmstb: Restore initial fundamental reset Phil Elwell
@ 2020-11-12 18:05 ` Stefan Wahren
  2020-11-12 18:38 ` Jim Quinlan
  1 sibling, 0 replies; 6+ messages in thread
From: Stefan Wahren @ 2020-11-12 18:05 UTC (permalink / raw)
  To: Phil Elwell, Jim Quinlan, Nicolas Saenz Julienne, Rob Herring,
	Florian Fainelli, bcm-kernel-feedback-list, linux-rpi-kernel,
	linux-pci

Hi,

Am 12.11.20 um 18:27 schrieb Phil Elwell:
> 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);

since only the Raspberry Pi 4 requires the reset i had changed the logic
to pcie->type == BCM2711. But this isn't totally clean, since we need
this quirk only for the Raspberry Pi 4 and not for CM4 (which also has a
BCM2711 SoC). So maybe we should introduce BCM2711_RPI4 to be on the
safe side. Sorry for the bikeshedding.

Stefan

> +
>  	usleep_range(100, 200);
>  
>  	/* Take the bridge out of reset */


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] PCI: brcmstb: Restore initial fundamental reset
  2020-11-12 17:27 [PATCH v2] PCI: brcmstb: Restore initial fundamental reset Phil Elwell
  2020-11-12 18:05 ` Stefan Wahren
@ 2020-11-12 18:38 ` Jim Quinlan
  2021-01-18 12:40   ` Lorenzo Pieralisi
  1 sibling, 1 reply; 6+ messages in thread
From: Jim Quinlan @ 2020-11-12 18:38 UTC (permalink / raw)
  To: Phil Elwell
  Cc: Nicolas Saenz Julienne, Rob Herring, Florian Fainelli,
	maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS

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

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.
Regards,
Jim
> +
>         usleep_range(100, 200);
>
>         /* Take the bridge out of reset */
> --
> 2.25.1
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] PCI: brcmstb: Restore initial fundamental reset
  2020-11-12 18:38 ` Jim Quinlan
@ 2021-01-18 12:40   ` Lorenzo Pieralisi
  2021-01-18 12:44     ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 6+ messages in thread
From: Lorenzo Pieralisi @ 2021-01-18 12:40 UTC (permalink / raw)
  To: Jim Quinlan
  Cc: Phil Elwell, Nicolas Saenz Julienne, Rob Herring,
	Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS

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 ?

Thanks,
Lorenzo

> Regards,
> Jim
> > +
> >         usleep_range(100, 200);
> >
> >         /* Take the bridge out of reset */
> > --
> > 2.25.1
> >



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] PCI: brcmstb: Restore initial fundamental reset
  2021-01-18 12:40   ` Lorenzo Pieralisi
@ 2021-01-18 12:44     ` Nicolas Saenz Julienne
  2021-01-18 13:33       ` Phil Elwell
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Saenz Julienne @ 2021-01-18 12:44 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Jim Quinlan, phil
  Cc: Phil Elwell, Rob Herring, Florian Fainelli,
	maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS

[-- 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 --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] PCI: brcmstb: Restore initial fundamental reset
  2021-01-18 12:44     ` Nicolas Saenz Julienne
@ 2021-01-18 13:33       ` Phil Elwell
  0 siblings, 0 replies; 6+ messages in thread
From: Phil Elwell @ 2021-01-18 13:33 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: Lorenzo Pieralisi, Jim Quinlan, phil, Rob Herring,
	Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS

On Mon, 18 Jan 2021 at 12:44, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> 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).

Any users running firmware released since mid-November should not
require the precautionary reset in order to have functioning USB.
Having said that, we'll be keeping this patch in our repo for the
benefit of everybody else.

Phil

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-01-18 13:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-12 17:27 [PATCH v2] PCI: brcmstb: Restore initial fundamental reset Phil Elwell
2020-11-12 18:05 ` Stefan Wahren
2020-11-12 18:38 ` Jim Quinlan
2021-01-18 12:40   ` Lorenzo Pieralisi
2021-01-18 12:44     ` Nicolas Saenz Julienne
2021-01-18 13:33       ` Phil Elwell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).