[PATCH v4 0/4] USB: pci-quirks: Add Raspberry Pi 4 quirk

[PATCH v4 0/4] USB: pci-quirks: Add Raspberry Pi 4 quirk

[Nicolas Saenz Julienne]

On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
loaded directly from an EEPROM or, if not present, by the SoC's
VideCore. This series adds support for the later.

Note that there are a set of constraints we have to consider (some of
them I missed on v1):
 - We need to make sure the VideoCore firmware interface is up and
   running before running the VL805 firmware load call.

 - There is no way to discern RPi4's VL805 chip from other platforms',
   so we need the firmware load to happen *before* running
   quirk_usb_handoff_xhci(). Failure to do so results in an unwarranted
   5 second wait while the fixup code polls xHC's unexisting state.

As per Stefan Wahren's comments I tested the behaviour on outdated
firmware. Boards dependent on this will not boot if firmware is not
up-to-date. Older boards with outdated firmware will fail to execute the
VideoCore firmware call, but xHCI will perform reliably. I added a
warning printout in case of failure.

I'm aware that Florian Fianelli noted on the previous revision that
waiting on the VideoCore firmware interface during PCIe's probe is far
from ideal. But this was before taking into account the second
constraint mentioned above. Impact on non RPi4 boards is minimal.

---

Changes since v3:
 - Addressed Greg's comments

There was no v2, my bad.

Changes since v1:
 - Addressed Floarians comments

Nicolas Saenz Julienne (4):
  soc: bcm2835: Sync xHCI reset firmware property with downstream
  firmware: raspberrypi: Introduce vl805 init routine
  PCI: brcmstb: Wait for Raspberry Pi's firmware when present
  USB: pci-quirks: Add Raspberry Pi 4 quirk

 drivers/firmware/Kconfig                   |  1 +
 drivers/firmware/raspberrypi.c             | 38 ++++++++++++++++++++++
 drivers/pci/controller/pcie-brcmstb.c      | 15 +++++++++
 drivers/usb/host/pci-quirks.c              | 16 +++++++++
 include/soc/bcm2835/raspberrypi-firmware.h |  9 ++++-
 5 files changed, 78 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v4 1/4] soc: bcm2835: Sync xHCI reset firmware property with downstream

[Nicolas Saenz Julienne]

The property is needed in order to trigger VL805's firmware load. Note
that there is a gap between the property introduced and the previous
one. This is also the case downstream.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/soc/bcm2835/raspberrypi-firmware.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index 7800e12ee042..cc9cdbc66403 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -90,7 +90,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_SET_PERIPH_REG =                         0x00038045,
 	RPI_FIRMWARE_GET_POE_HAT_VAL =                        0x00030049,
 	RPI_FIRMWARE_SET_POE_HAT_VAL =                        0x00030050,
-
+	RPI_FIRMWARE_NOTIFY_XHCI_RESET =                      0x00030058,
 
 	/* Dispmanx TAGS */
 	RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE =                   0x00040001,
-- 
2.25.1

[PATCH v4 2/4] firmware: raspberrypi: Introduce vl805 init routine

[Nicolas Saenz Julienne]

On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
loaded directly from an EEPROM or, if not present, by the SoC's
VideCore. The function informs VideCore that VL805 was just reset, or
requests for a probe defer.

Based on Tim Gover's downstream implementation.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

---
Changes since v1:
 - Move include into .c file and add forward declaration to .h

 drivers/firmware/raspberrypi.c             | 38 ++++++++++++++++++++++
 include/soc/bcm2835/raspberrypi-firmware.h |  7 ++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index da26a584dca0..cbb495aff6a0 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -12,6 +12,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/pci.h>
 #include <soc/bcm2835/raspberrypi-firmware.h>
 
 #define MBOX_MSG(chan, data28)		(((data28) & ~0xf) | ((chan) & 0xf))
@@ -286,6 +287,43 @@ struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node)
 }
 EXPORT_SYMBOL_GPL(rpi_firmware_get);
 
+/*
+ * On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
+ * loaded directly from an EEPROM or, if not present, by the SoC's VideCore.
+ * Inform VideCore that VL805 was just reset, or defer xhci's probe if not yet
+ * joinable trough the mailbox interface.
+ */
+int rpi_firmware_init_vl805(struct pci_dev *pdev)
+{
+	struct device_node *fw_np;
+	struct rpi_firmware *fw;
+	u32 dev_addr;
+	int ret;
+
+	fw_np = of_find_compatible_node(NULL, NULL,
+					"raspberrypi,bcm2835-firmware");
+	if (!fw_np)
+		return 0;
+
+	fw = rpi_firmware_get(fw_np);
+	of_node_put(fw_np);
+	if (!fw)
+		return -EPROBE_DEFER;
+
+	dev_addr = pdev->bus->number << 20 | PCI_SLOT(pdev->devfn) << 15 |
+		   PCI_FUNC(pdev->devfn) << 12;
+
+	ret = rpi_firmware_property(fw, RPI_FIRMWARE_NOTIFY_XHCI_RESET,
+				    &dev_addr, sizeof(dev_addr));
+	if (ret)
+		return ret;
+
+	dev_dbg(&pdev->dev, "loaded Raspberry Pi's VL805 firmware\n");
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_init_vl805);
+
 static const struct of_device_id rpi_firmware_of_match[] = {
 	{ .compatible = "raspberrypi,bcm2835-firmware", },
 	{},
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index cc9cdbc66403..c161bdab759d 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -10,6 +10,7 @@
 #include <linux/of_device.h>
 
 struct rpi_firmware;
+struct pci_dev;
 
 enum rpi_firmware_property_status {
 	RPI_FIRMWARE_STATUS_REQUEST = 0,
@@ -141,6 +142,7 @@ int rpi_firmware_property(struct rpi_firmware *fw,
 int rpi_firmware_property_list(struct rpi_firmware *fw,
 			       void *data, size_t tag_size);
 struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
+int rpi_firmware_init_vl805(struct pci_dev *pdev);
 #else
 static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag,
 					void *data, size_t len)
@@ -158,6 +160,11 @@ static inline struct rpi_firmware *rpi_firmware_get(struct device_node *firmware
 {
 	return NULL;
 }
+
+static int rpi_firmware_init_vl805(struct pci_dev *pdev)
+{
+	return 0;
+}
 #endif
 
 #endif /* __SOC_RASPBERRY_FIRMWARE_H__ */
-- 
2.25.1

[PATCH v4 3/4] PCI: brcmstb: Wait for Raspberry Pi's firmware when present

[Nicolas Saenz Julienne]

xHCI's PCI fixup, run at the end of pcie-brcmstb's probe, depends on
RPi4's VideoCore firmware interface to be up and running. It's possible
for both initializations to race, so make sure it's available prior
starting.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 3a10e678c7f4..a3d3070a5832 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -28,6 +28,8 @@
 #include <linux/string.h>
 #include <linux/types.h>
 
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
 #include "../pci.h"
 
 /* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
@@ -917,11 +919,24 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node, *msi_np;
 	struct pci_host_bridge *bridge;
+	struct device_node *fw_np;
 	struct brcm_pcie *pcie;
 	struct pci_bus *child;
 	struct resource *res;
 	int ret;
 
+	/*
+	 * We have to wait for the Raspberry Pi's firmware interface to be up
+	 * as some PCI fixups depend on it.
+	 */
+	fw_np = of_find_compatible_node(NULL, NULL,
+					"raspberrypi,bcm2835-firmware");
+	if (fw_np && !rpi_firmware_get(fw_np)) {
+		of_node_put(fw_np);
+		return -EPROBE_DEFER;
+	}
+	of_node_put(fw_np);
+
 	bridge = devm_pci_alloc_host_bridge(&pdev->dev, sizeof(*pcie));
 	if (!bridge)
 		return -ENOMEM;
-- 
2.25.1

Re: [PATCH v4 3/4] PCI: brcmstb: Wait for Raspberry Pi's firmware when present

[Sergei Shtylyov]

On 06.03.2020 14:43, Nicolas Saenz Julienne wrote:

> xHCI's PCI fixup, run at the end of pcie-brcmstb's probe, depends on
> RPi4's VideoCore firmware interface to be up and running. It's possible
> for both initializations to race, so make sure it's available prior
> starting.

     Prior to starting?

> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
[...]

MBR, Sergei

Re: [PATCH v4 3/4] PCI: brcmstb: Wait for Raspberry Pi's firmware when present

[Nicolas Saenz Julienne]

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

On Sat, 2020-03-07 at 12:55 +0300, Sergei Shtylyov wrote:
> On 06.03.2020 14:43, Nicolas Saenz Julienne wrote:
> 
> > xHCI's PCI fixup, run at the end of pcie-brcmstb's probe, depends on
> > RPi4's VideoCore firmware interface to be up and running. It's possible
> > for both initializations to race, so make sure it's available prior
> > starting.
> 
>      Prior to starting?

Of course, thanks!

Nicolas


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

[PATCH v4 4/4] USB: pci-quirks: Add Raspberry Pi 4 quirk

[Nicolas Saenz Julienne]

On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
loaded directly from an EEPROM or, if not present, by the SoC's
VideCore. Inform VideCore that VL805 was just reset.

Also, as this creates a dependency between XHCI_PCI and VideoCore's
firmware interface, reflect that on the firmware interface Kconfg.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---

Changes since v3:
 - Add more complete error message.
 - Add braces around if statement

Changes since v1:
 - Make RASPBERRYPI_FIRMWARE dependent on this quirk to make sure it
   gets compiled when needed.

 drivers/firmware/Kconfig      |  1 +
 drivers/usb/host/pci-quirks.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index ea869addc89b..40a468d712a5 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -180,6 +180,7 @@ config ISCSI_IBFT
 config RASPBERRYPI_FIRMWARE
 	tristate "Raspberry Pi Firmware Driver"
 	depends on BCM2835_MBOX
+	default XHCI_PCI
 	help
 	  This option enables support for communicating with the firmware on the
 	  Raspberry Pi.
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index beb2efa71341..452f5f12b042 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,6 +16,9 @@
 #include <linux/export.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
+
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"
 
@@ -1243,11 +1246,24 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
 
 static void quirk_usb_early_handoff(struct pci_dev *pdev)
 {
+	int ret;
+
 	/* Skip Netlogic mips SoC's internal PCI USB controller.
 	 * This device does not need/support EHCI/OHCI handoff
 	 */
 	if (pdev->vendor == 0x184e)	/* vendor Netlogic */
 		return;
+
+	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483) {
+		ret = rpi_firmware_init_vl805(pdev);
+		if (ret) {
+			/* Firmware might be outdated, or something failed */
+			dev_warn(&pdev->dev, "Failed to load VL805's firmware: %d\n", ret);
+			dev_warn(&pdev->dev, "Will continue to attempt to work, "
+				 "but bad things might happen. You should fix this...\n");
+		}
+	}
+
 	if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
 			pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
 			pdev->class != PCI_CLASS_SERIAL_USB_EHCI &&
-- 
2.25.1

Re: [PATCH v4 4/4] USB: pci-quirks: Add Raspberry Pi 4 quirk

[Sergei Shtylyov]

Hello!

On 06.03.2020 14:43, Nicolas Saenz Julienne wrote:

> On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
> loaded directly from an EEPROM or, if not present, by the SoC's
> VideCore. Inform VideCore that VL805 was just reset.
> 
> Also, as this creates a dependency between XHCI_PCI and VideoCore's
> firmware interface, reflect that on the firmware interface Kconfg.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
> 
> Changes since v3:
>   - Add more complete error message.
>   - Add braces around if statement
> 
> Changes since v1:
>   - Make RASPBERRYPI_FIRMWARE dependent on this quirk to make sure it
>     gets compiled when needed.
> 
>   drivers/firmware/Kconfig      |  1 +
>   drivers/usb/host/pci-quirks.c | 16 ++++++++++++++++
>   2 files changed, 17 insertions(+)
[...]
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index beb2efa71341..452f5f12b042 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -16,6 +16,9 @@
>   #include <linux/export.h>
>   #include <linux/acpi.h>
>   #include <linux/dmi.h>
> +
> +#include <soc/bcm2835/raspberrypi-firmware.h>
> +
>   #include "pci-quirks.h"
>   #include "xhci-ext-caps.h"
>   
> @@ -1243,11 +1246,24 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
>   
>   static void quirk_usb_early_handoff(struct pci_dev *pdev)
>   {
> +	int ret;
> +
>   	/* Skip Netlogic mips SoC's internal PCI USB controller.
>   	 * This device does not need/support EHCI/OHCI handoff
>   	 */
>   	if (pdev->vendor == 0x184e)	/* vendor Netlogic */
>   		return;
> +
> +	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483) {
> +		ret = rpi_firmware_init_vl805(pdev);
> +		if (ret) {
> +			/* Firmware might be outdated, or something failed */
> +			dev_warn(&pdev->dev, "Failed to load VL805's firmware: %d\n", ret);
> +			dev_warn(&pdev->dev, "Will continue to attempt to work, "
> +				 "but bad things might happen. You should fix this...\n");

    Don't break up the long kernel messages (checkpatch.pl should not complain 
about them).

[...]

MBR, Sergei

Re: [PATCH v4 4/4] USB: pci-quirks: Add Raspberry Pi 4 quirk

[Nicolas Saenz Julienne]

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

On Sat, 2020-03-07 at 12:54 +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 06.03.2020 14:43, Nicolas Saenz Julienne wrote:
> 
> > On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
> > loaded directly from an EEPROM or, if not present, by the SoC's
> > VideCore. Inform VideCore that VL805 was just reset.
> > 
> > Also, as this creates a dependency between XHCI_PCI and VideoCore's
> > firmware interface, reflect that on the firmware interface Kconfg.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> > 
> > Changes since v3:
> >   - Add more complete error message.
> >   - Add braces around if statement
> > 
> > Changes since v1:
> >   - Make RASPBERRYPI_FIRMWARE dependent on this quirk to make sure it
> >     gets compiled when needed.
> > 
> >   drivers/firmware/Kconfig      |  1 +
> >   drivers/usb/host/pci-quirks.c | 16 ++++++++++++++++
> >   2 files changed, 17 insertions(+)
> [...]
> > diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> > index beb2efa71341..452f5f12b042 100644
> > --- a/drivers/usb/host/pci-quirks.c
> > +++ b/drivers/usb/host/pci-quirks.c
> > @@ -16,6 +16,9 @@
> >   #include <linux/export.h>
> >   #include <linux/acpi.h>
> >   #include <linux/dmi.h>
> > +
> > +#include <soc/bcm2835/raspberrypi-firmware.h>
> > +
> >   #include "pci-quirks.h"
> >   #include "xhci-ext-caps.h"
> >   
> > @@ -1243,11 +1246,24 @@ static void quirk_usb_handoff_xhci(struct pci_dev
> > *pdev)
> >   
> >   static void quirk_usb_early_handoff(struct pci_dev *pdev)
> >   {
> > +	int ret;
> > +
> >   	/* Skip Netlogic mips SoC's internal PCI USB controller.
> >   	 * This device does not need/support EHCI/OHCI handoff
> >   	 */
> >   	if (pdev->vendor == 0x184e)	/* vendor Netlogic */
> >   		return;
> > +
> > +	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483) {
> > +		ret = rpi_firmware_init_vl805(pdev);
> > +		if (ret) {
> > +			/* Firmware might be outdated, or something failed */
> > +			dev_warn(&pdev->dev, "Failed to load VL805's firmware:
> > %d\n", ret);
> > +			dev_warn(&pdev->dev, "Will continue to attempt to work,
> > "
> > +				 "but bad things might happen. You should fix
> > this...\n");
> 
>     Don't break up the long kernel messages (checkpatch.pl should not
> complain 
> about them).

I generally don't, but in this specific case IMO it went way too long, as in
140 charaters long, so I figured it'd be better to split it. That said, I don't
really care, so I'll happily change it for v5.

Regards,
Nicolas


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