| Message ID | 20201112172709.1817-1-phil@raspberrypi.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] PCI: brcmstb: Restore initial fundamental reset | expand |
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 */
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 >
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 > >
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
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
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 */
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(+)