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

[PATCH v8 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
co-processor, VideoCore. This series adds support for the later.

Note that there are a set of constraints we have to consider:
 - 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 non-existing state.

By Florian's suggestion I've been spending some time exploring the device
link[1] API in order to see if that could save us from explicitly creating
probe dependencies between pcie-brcmstb and firmware/raspberrypi (patch #3).
Technically these dependencies could be inferred from DT. It turns out Saravana
Kannan has been looking at this already. A new boot mechanism, activated with
fw_devlink=on takes care of the device probe ordering on devices with
consumer/supplier relationships. For now this relationship is created based on
the usage of generic DT properties, but has no support for vendor-specifc DT
properties, which we'd be forced to use in order to create a relationship
between our two devices since our setup is highly non generic. There will
probably be at some point support for such properties, and we will then be able
to revisit some of this code.

All this is based on the work by Tim Gover in RPi's downstream
kernel[2].

[1] https://www.kernel.org/doc/html/v4.13/driver-api/device_link.html
[2] https://github.com/raspberrypi/linux/commit/9935b4c7e360b4494b4cb6e3ce797238a1ab78bd

---

Changes since v7:
 - Address Stefan's comments

Changes since v6:
 - Make rpi_firmware_init_vl805() more robust
 - Rewrite comments and patch descriptions to be more accessible to non RPi
   fluent people
 - Removed Florian's Reviewed-by in patch #2 as function changed
   substantially
 - Tested with/witout u-boot

Changes since v5:
 - Fix issues reported by Kbuild test robot

Changes since v4:
 - Addressed Sergei's comments
 - Fix potential warning in patch #2

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: Add notify xHCI reset property
  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                   |  3 +-
 drivers/firmware/raspberrypi.c             | 61 ++++++++++++++++++++++
 drivers/pci/controller/pcie-brcmstb.c      | 17 ++++++
 drivers/usb/host/pci-quirks.c              | 16 ++++++
 include/soc/bcm2835/raspberrypi-firmware.h |  9 +++-
 5 files changed, 104 insertions(+), 2 deletions(-)

-- 
2.26.2

[PATCH v8 1/4] soc: bcm2835: Add notify xHCI reset property

[Nicolas Saenz Julienne]

The property is needed in order to trigger VL805's firmware load. Note
that gap between the property introduced and the previous one is due to
the properties not being defined.

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.26.2

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

[Nicolas Saenz Julienne]

The Raspberry Pi 4 gets its USB functionality from VL805, a PCIe chip
that implements xHCI. After a PCI reset, VL805's firmware may either be
loaded directly from an EEPROM or, if not present, by the SoC's
co-processor, VideoCore. RPi4's VideoCore OS contains both the non public
firmware load logic and the VL805 firmware blob. The function this patch
introduces triggers the aforementioned process.

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

---

Change since v7:
- Use usleep_delay()
- Add comment about PCI errors
- Don't wait on error
- Typos

Change since v6:
- Add test to avoid loading the firmware when not needed
- Since we have it around, print VL805's firmware version, it'll make
debugging easier in the future
- Correct typos
- Add a clearer view of HW topology in patch description

Changes since v4:
- Inline function definition when RASPBERRYPI_FIRMWARE is not defined

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

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

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index da26a584dca0..a166ad0cec2c 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -12,6 +12,8 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/pci.h>
+#include <linux/delay.h>
 #include <soc/bcm2835/raspberrypi-firmware.h>
 
 #define MBOX_MSG(chan, data28)		(((data28) & ~0xf) | ((chan) & 0xf))
@@ -19,6 +21,8 @@
 #define MBOX_DATA28(msg)		((msg) & ~0xf)
 #define MBOX_CHAN_PROPERTY		8
 
+#define VL805_PCI_CONFIG_VERSION_OFFSET		0x50
+
 static struct platform_device *rpi_hwmon;
 static struct platform_device *rpi_clk;
 
@@ -286,6 +290,63 @@ struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node)
 }
 EXPORT_SYMBOL_GPL(rpi_firmware_get);
 
+/*
+ * The Raspberry Pi 4 gets its USB functionality from VL805, a PCIe chip that
+ * implements xHCI. After a PCI reset, VL805's firmware may either be loaded
+ * directly from an EEPROM or, if not present, by the SoC's co-processor,
+ * VideoCore. RPi4's VideoCore OS contains both the non public firmware load
+ * logic and the VL805 firmware blob. This function triggers the aforementioned
+ * process.
+ */
+int rpi_firmware_init_vl805(struct pci_dev *pdev)
+{
+	struct device_node *fw_np;
+	struct rpi_firmware *fw;
+	u32 dev_addr, version;
+	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 -ENODEV;
+
+	/*
+	 * Make sure we don't trigger a firmware load unnecessarily.
+	 *
+	 * If something went wrong with PCI, this whole exercise would be
+	 * futile as VideoCore expects from us a configured PCI bus. Just take
+	 * the faulty version (likely ~0) and let xHCI's registration fail
+	 * further down the line.
+	 */
+	pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET, &version);
+	if (version)
+		goto exit;
+
+	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;
+
+	/* Wait for vl805 to startup */
+	usleep_range(200, 1000);
+
+	pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET,
+			      &version);
+exit:
+	pci_info(pdev, "VL805 firmware version %08x\n", version);
+
+	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..3025aca3c358 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 inline int rpi_firmware_init_vl805(struct pci_dev *pdev)
+{
+	return 0;
+}
 #endif
 
 #endif /* __SOC_RASPBERRY_FIRMWARE_H__ */
-- 
2.26.2

[PATCH v8 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 to
starting.

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

---

Changes since v6:
- Add more complete comment

 drivers/pci/controller/pcie-brcmstb.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 6d79d14527a6..0b97b94c4a9a 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,26 @@ 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 Raspberry Pi's firmware interface to be up as a
+	 * PCI fixup, rpi_firmware_init_vl805(), depends on it. This driver's
+	 * probe can race with the firmware interface's (see
+	 * drivers/firmware/raspberrypi.c) and potentially break the PCI fixup.
+	 */
+	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.26.2

[PATCH v8 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
VideoCore. Inform VideoCore that VL805 was just reset.

Also, as this creates a dependency between USB_PCI and VideoCore's
firmware interface, and since USB_PCI can't be set as a module neither
this can. Reflect that on the firmware interface Kconfg.

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

Changes since v5:
 - Fix Kconfig issue with allmodconfig

Changes since v4:
 - Do not split up error message

Changes since v3:
 - Add more complete error message

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

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

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 8007d4aa76dc..b42140cff8ac 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -178,8 +178,9 @@ config ISCSI_IBFT
 	  Otherwise, say N.
 
 config RASPBERRYPI_FIRMWARE
-	tristate "Raspberry Pi Firmware Driver"
+	bool "Raspberry Pi Firmware Driver"
 	depends on BCM2835_MBOX
+	default USB_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 92150ecdb036..0b949acfa258 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. Will continue to attempt to work, but bad things might happen. You should fix this...\n",
+				 ret);
+		}
+	}
+
 	if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
 			pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
 			pdev->class != PCI_CLASS_SERIAL_USB_EHCI &&
-- 
2.26.2

Re: [PATCH v8 1/4] soc: bcm2835: Add notify xHCI reset property

[Rob Herring]

On Tue,  5 May 2020 18:13:14 +0200, Nicolas Saenz Julienne wrote:
> The property is needed in order to trigger VL805's firmware load. Note
> that gap between the property introduced and the previous one is due to
> the properties not being defined.
> 
> 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(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Rob Herring]

On Tue,  5 May 2020 18:13:15 +0200, Nicolas Saenz Julienne wrote:
> The Raspberry Pi 4 gets its USB functionality from VL805, a PCIe chip
> that implements xHCI. After a PCI reset, VL805's firmware may either be
> loaded directly from an EEPROM or, if not present, by the SoC's
> co-processor, VideoCore. RPi4's VideoCore OS contains both the non public
> firmware load logic and the VL805 firmware blob. The function this patch
> introduces triggers the aforementioned process.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> 
> ---
> 
> Change since v7:
> - Use usleep_delay()
> - Add comment about PCI errors
> - Don't wait on error
> - Typos
> 
> Change since v6:
> - Add test to avoid loading the firmware when not needed
> - Since we have it around, print VL805's firmware version, it'll make
> debugging easier in the future
> - Correct typos
> - Add a clearer view of HW topology in patch description
> 
> Changes since v4:
> - Inline function definition when RASPBERRYPI_FIRMWARE is not defined
> 
> Changes since v1:
> - Move include into .c file and add forward declaration to .h
> 
>  drivers/firmware/raspberrypi.c             | 61 ++++++++++++++++++++++
>  include/soc/bcm2835/raspberrypi-firmware.h |  7 +++
>  2 files changed, 68 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Rob Herring]

On Tue,  5 May 2020 18:13:16 +0200, 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 to
> starting.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> ---
> 
> Changes since v6:
> - Add more complete comment
> 
>  drivers/pci/controller/pcie-brcmstb.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Rob Herring]

On Tue,  5 May 2020 18:13:17 +0200, 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
> VideoCore. Inform VideoCore that VL805 was just reset.
> 
> Also, as this creates a dependency between USB_PCI and VideoCore's
> firmware interface, and since USB_PCI can't be set as a module neither
> this can. Reflect that on the firmware interface Kconfg.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
> 
> Changes since v5:
>  - Fix Kconfig issue with allmodconfig
> 
> Changes since v4:
>  - Do not split up error message
> 
> Changes since v3:
>  - Add more complete error message
> 
> Changes since v1:
>  - Make RASPBERRYPI_FIRMWARE dependent on this quirk to make sure it
>    gets compiled when needed.
> 
>  drivers/firmware/Kconfig      |  3 ++-
>  drivers/usb/host/pci-quirks.c | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Stefan Wahren]

Hi Nicolas,

Am 07.05.20 um 23:48 schrieb Rob Herring:
> On Tue,  5 May 2020 18:13:15 +0200, Nicolas Saenz Julienne wrote:
>> The Raspberry Pi 4 gets its USB functionality from VL805, a PCIe chip
>> that implements xHCI. After a PCI reset, VL805's firmware may either be
>> loaded directly from an EEPROM or, if not present, by the SoC's
>> co-processor, VideoCore. RPi4's VideoCore OS contains both the non public
>> firmware load logic and the VL805 firmware blob. The function this patch
>> introduces triggers the aforementioned process.
>>
>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>
>> ---
>>
>> Change since v7:
>> - Use usleep_delay()
>> - Add comment about PCI errors
>> - Don't wait on error
>> - Typos
>>
>> Change since v6:
>> - Add test to avoid loading the firmware when not needed
>> - Since we have it around, print VL805's firmware version, it'll make
>> debugging easier in the future
>> - Correct typos
>> - Add a clearer view of HW topology in patch description
>>
>> Changes since v4:
>> - Inline function definition when RASPBERRYPI_FIRMWARE is not defined
>>
>> Changes since v1:
>> - Move include into .c file and add forward declaration to .h
>>
>>  drivers/firmware/raspberrypi.c             | 61 ++++++++++++++++++++++
>>  include/soc/bcm2835/raspberrypi-firmware.h |  7 +++
>>  2 files changed, 68 insertions(+)
>>
> Reviewed-by: Rob Herring <robh@kernel.org>

i modified the code a little bit for testing, but also successfully
tested it without my modifications:

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 0d1422b..f3f4c2d 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -337,8 +337,10 @@ int rpi_firmware_init_vl805(struct pci_dev *pdev)
         * further down the line.
         */
        pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET,
&version);
-       if (version)
-               goto exit;
+       if (version) {
+               pci_info(pdev, "VL805 EEPROM firmware version %08x\n",
version);
+               return 0;
+       }
 
        dev_addr = pdev->bus->number << 20 | PCI_SLOT(pdev->devfn) << 15 |
                   PCI_FUNC(pdev->devfn) << 12;
@@ -353,9 +355,8 @@ int rpi_firmware_init_vl805(struct pci_dev *pdev)
 
        pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET,
                              &version);
-exit:
-       pci_info(pdev, "VL805 firmware version %08x\n", version);
 
+       pci_info(pdev, "VL805 RAM firmware version %08x\n", version);
        return 0;
 }
 EXPORT_SYMBOL_GPL(rpi_firmware_init_vl805);

Here are the my results with 3x Raspberry Pi 4:

VL805 EEPROM firmware version 000137ad
VL805 EEPROM firmware version 00013701
VL805 RAM firmware version 000137ad

So the whole patch series is:

Tested-by: Stefan Wahren <stefan.wahren@i2se.com>

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

[Stefan Wahren]

Am 09.05.20 um 12:02 schrieb Stefan Wahren:
> Hi Nicolas,
>
> Am 07.05.20 um 23:48 schrieb Rob Herring:
>> On Tue,  5 May 2020 18:13:15 +0200, Nicolas Saenz Julienne wrote:
>>> The Raspberry Pi 4 gets its USB functionality from VL805, a PCIe chip
>>> that implements xHCI. After a PCI reset, VL805's firmware may either be
>>> loaded directly from an EEPROM or, if not present, by the SoC's
>>> co-processor, VideoCore. RPi4's VideoCore OS contains both the non public
>>> firmware load logic and the VL805 firmware blob. The function this patch
>>> introduces triggers the aforementioned process.
>>>
>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>
>>> ---
>>>
>>> Change since v7:
>>> - Use usleep_delay()
>>> - Add comment about PCI errors
>>> - Don't wait on error
>>> - Typos
>>>
>>> Change since v6:
>>> - Add test to avoid loading the firmware when not needed
>>> - Since we have it around, print VL805's firmware version, it'll make
>>> debugging easier in the future
>>> - Correct typos
>>> - Add a clearer view of HW topology in patch description
>>>
>>> Changes since v4:
>>> - Inline function definition when RASPBERRYPI_FIRMWARE is not defined
>>>
>>> Changes since v1:
>>> - Move include into .c file and add forward declaration to .h
>>>
>>>  drivers/firmware/raspberrypi.c             | 61 ++++++++++++++++++++++
>>>  include/soc/bcm2835/raspberrypi-firmware.h |  7 +++
>>>  2 files changed, 68 insertions(+)
>>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
> i modified the code a little bit for testing, but also successfully
> tested it without my modifications:
>
> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> index 0d1422b..f3f4c2d 100644
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -337,8 +337,10 @@ int rpi_firmware_init_vl805(struct pci_dev *pdev)
>          * further down the line.
>          */
>         pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET,
> &version);
> -       if (version)
> -               goto exit;
> +       if (version) {
> +               pci_info(pdev, "VL805 EEPROM firmware version %08x\n",
> version);
> +               return 0;
> +       }
>  
>         dev_addr = pdev->bus->number << 20 | PCI_SLOT(pdev->devfn) << 15 |
>                    PCI_FUNC(pdev->devfn) << 12;
> @@ -353,9 +355,8 @@ int rpi_firmware_init_vl805(struct pci_dev *pdev)
>  
>         pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET,
>                               &version);
> -exit:
> -       pci_info(pdev, "VL805 firmware version %08x\n", version);
>  
> +       pci_info(pdev, "VL805 RAM firmware version %08x\n", version);
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(rpi_firmware_init_vl805);
>
> Here are the my results with 3x Raspberry Pi 4:
>
> VL805 EEPROM firmware version 000137ad
> VL805 EEPROM firmware version 00013701
> VL805 RAM firmware version 000137ad
>
> So the whole patch series is:
>
> Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
>
Sorry, for sending from the wrong address:

Tested-by: Stefan Wahren <stefan.wahren@i2se.com>

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

[Nicolas Saenz Julienne]

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

On Sat, 2020-05-09 at 12:02 +0200, Stefan Wahren wrote:
> Hi Nicolas,
> 
> Am 07.05.20 um 23:48 schrieb Rob Herring:
> > On Tue,  5 May 2020 18:13:15 +0200, Nicolas Saenz Julienne wrote:
> > > The Raspberry Pi 4 gets its USB functionality from VL805, a PCIe chip
> > > that implements xHCI. After a PCI reset, VL805's firmware may either be
> > > loaded directly from an EEPROM or, if not present, by the SoC's
> > > co-processor, VideoCore. RPi4's VideoCore OS contains both the non public
> > > firmware load logic and the VL805 firmware blob. The function this patch
> > > introduces triggers the aforementioned process.
> > > 
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > 
> > > ---
> > > 
> > > Change since v7:
> > > - Use usleep_delay()
> > > - Add comment about PCI errors
> > > - Don't wait on error
> > > - Typos
> > > 
> > > Change since v6:
> > > - Add test to avoid loading the firmware when not needed
> > > - Since we have it around, print VL805's firmware version, it'll make
> > > debugging easier in the future
> > > - Correct typos
> > > - Add a clearer view of HW topology in patch description
> > > 
> > > Changes since v4:
> > > - Inline function definition when RASPBERRYPI_FIRMWARE is not defined
> > > 
> > > Changes since v1:
> > > - Move include into .c file and add forward declaration to .h
> > > 
> > >  drivers/firmware/raspberrypi.c             | 61 ++++++++++++++++++++++
> > >  include/soc/bcm2835/raspberrypi-firmware.h |  7 +++
> > >  2 files changed, 68 insertions(+)
> > > 
> > Reviewed-by: Rob Herring <robh@kernel.org>
> 
> i modified the code a little bit for testing, but also successfully
> tested it without my modifications:
> 
> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> index 0d1422b..f3f4c2d 100644
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -337,8 +337,10 @@ int rpi_firmware_init_vl805(struct pci_dev *pdev)
>          * further down the line.
>          */
>         pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET,
> &version);
> -       if (version)
> -               goto exit;
> +       if (version) {
> +               pci_info(pdev, "VL805 EEPROM firmware version %08x\n",
> version);
> +               return 0;
> +       }
>  
>         dev_addr = pdev->bus->number << 20 | PCI_SLOT(pdev->devfn) << 15 |
>                    PCI_FUNC(pdev->devfn) << 12;
> @@ -353,9 +355,8 @@ int rpi_firmware_init_vl805(struct pci_dev *pdev)
>  
>         pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET,
>                               &version);
> -exit:
> -       pci_info(pdev, "VL805 firmware version %08x\n", version);
>  
> +       pci_info(pdev, "VL805 RAM firmware version %08x\n", version);
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(rpi_firmware_init_vl805);
> 
> Here are the my results with 3x Raspberry Pi 4:
> 
> VL805 EEPROM firmware version 000137ad
> VL805 EEPROM firmware version 00013701
> VL805 RAM firmware version 000137ad
> 
> So the whole patch series is:
> 
> Tested-by: Stefan Wahren <stefan.wahren@i2se.com>

Thanks for taking the time!

Regards,
Nicolas


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

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

[Lorenzo Pieralisi]

On Tue, May 05, 2020 at 06:13:17PM +0200, 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
> VideoCore. Inform VideoCore that VL805 was just reset.
> 
> Also, as this creates a dependency between USB_PCI and VideoCore's
> firmware interface, and since USB_PCI can't be set as a module neither
> this can. Reflect that on the firmware interface Kconfg.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
> 
> Changes since v5:
>  - Fix Kconfig issue with allmodconfig
> 
> Changes since v4:
>  - Do not split up error message
> 
> Changes since v3:
>  - Add more complete error message
> 
> Changes since v1:
>  - Make RASPBERRYPI_FIRMWARE dependent on this quirk to make sure it
>    gets compiled when needed.
> 
>  drivers/firmware/Kconfig      |  3 ++-
>  drivers/usb/host/pci-quirks.c | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+), 1 deletion(-)

Hi Mathias,

I would need your ACK to merge this series, thanks.

Lorenzo

> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 8007d4aa76dc..b42140cff8ac 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -178,8 +178,9 @@ config ISCSI_IBFT
>  	  Otherwise, say N.
>  
>  config RASPBERRYPI_FIRMWARE
> -	tristate "Raspberry Pi Firmware Driver"
> +	bool "Raspberry Pi Firmware Driver"
>  	depends on BCM2835_MBOX
> +	default USB_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 92150ecdb036..0b949acfa258 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. Will continue to attempt to work, but bad things might happen. You should fix this...\n",
> +				 ret);
> +		}
> +	}
> +
>  	if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
>  			pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
>  			pdev->class != PCI_CLASS_SERIAL_USB_EHCI &&
> -- 
> 2.26.2
> 

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

[Mathias Nyman]

On 11.5.2020 17.25, Lorenzo Pieralisi wrote:
> On Tue, May 05, 2020 at 06:13:17PM +0200, 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
>> VideoCore. Inform VideoCore that VL805 was just reset.
>>
>> Also, as this creates a dependency between USB_PCI and VideoCore's
>> firmware interface, and since USB_PCI can't be set as a module neither
>> this can. Reflect that on the firmware interface Kconfg.
>>
>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>> ---
>>
>> Changes since v5:
>>  - Fix Kconfig issue with allmodconfig
>>
>> Changes since v4:
>>  - Do not split up error message
>>
>> Changes since v3:
>>  - Add more complete error message
>>
>> Changes since v1:
>>  - Make RASPBERRYPI_FIRMWARE dependent on this quirk to make sure it
>>    gets compiled when needed.
>>
>>  drivers/firmware/Kconfig      |  3 ++-
>>  drivers/usb/host/pci-quirks.c | 16 ++++++++++++++++
>>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> Hi Mathias,
> 
> I would need your ACK to merge this series, thanks.
> 
> Lorenzo

Ah, yes, of course

Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com>

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

[Lorenzo Pieralisi]

On Tue, May 05, 2020 at 06:13:13PM +0200, 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
> co-processor, VideoCore. This series adds support for the later.
> 
> Note that there are a set of constraints we have to consider:
>  - 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 non-existing state.
> 
> By Florian's suggestion I've been spending some time exploring the device
> link[1] API in order to see if that could save us from explicitly creating
> probe dependencies between pcie-brcmstb and firmware/raspberrypi (patch #3).
> Technically these dependencies could be inferred from DT. It turns out Saravana
> Kannan has been looking at this already. A new boot mechanism, activated with
> fw_devlink=on takes care of the device probe ordering on devices with
> consumer/supplier relationships. For now this relationship is created based on
> the usage of generic DT properties, but has no support for vendor-specifc DT
> properties, which we'd be forced to use in order to create a relationship
> between our two devices since our setup is highly non generic. There will
> probably be at some point support for such properties, and we will then be able
> to revisit some of this code.
> 
> All this is based on the work by Tim Gover in RPi's downstream
> kernel[2].
> 
> [1] https://www.kernel.org/doc/html/v4.13/driver-api/device_link.html
> [2] https://github.com/raspberrypi/linux/commit/9935b4c7e360b4494b4cb6e3ce797238a1ab78bd
> 
> ---
> 
> Changes since v7:
>  - Address Stefan's comments
> 
> Changes since v6:
>  - Make rpi_firmware_init_vl805() more robust
>  - Rewrite comments and patch descriptions to be more accessible to non RPi
>    fluent people
>  - Removed Florian's Reviewed-by in patch #2 as function changed
>    substantially
>  - Tested with/witout u-boot
> 
> Changes since v5:
>  - Fix issues reported by Kbuild test robot
> 
> Changes since v4:
>  - Addressed Sergei's comments
>  - Fix potential warning in patch #2
> 
> 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: Add notify xHCI reset property
>   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                   |  3 +-
>  drivers/firmware/raspberrypi.c             | 61 ++++++++++++++++++++++
>  drivers/pci/controller/pcie-brcmstb.c      | 17 ++++++
>  drivers/usb/host/pci-quirks.c              | 16 ++++++
>  include/soc/bcm2835/raspberrypi-firmware.h |  9 +++-
>  5 files changed, 104 insertions(+), 2 deletions(-)

Hi Nicolas,

should I queue this series via the PCI tree ? Just let me know, most of
the changes are not in the PCI tree, asking in order to
minimize/simplify conflicts handling if possible.

Lorenzo

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

[Nicolas Saenz Julienne]

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

On Wed, 2020-05-13 at 12:17 +0100, Lorenzo Pieralisi wrote:
> On Tue, May 05, 2020 at 06:13:13PM +0200, 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
> > co-processor, VideoCore. This series adds support for the later.
> > 
> > Note that there are a set of constraints we have to consider:
> >  - 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 non-existing state.
> > 
> > By Florian's suggestion I've been spending some time exploring the device
> > link[1] API in order to see if that could save us from explicitly creating
> > probe dependencies between pcie-brcmstb and firmware/raspberrypi (patch #3).
> > Technically these dependencies could be inferred from DT. It turns out
> > Saravana
> > Kannan has been looking at this already. A new boot mechanism, activated
> > with
> > fw_devlink=on takes care of the device probe ordering on devices with
> > consumer/supplier relationships. For now this relationship is created based
> > on
> > the usage of generic DT properties, but has no support for vendor-specifc DT
> > properties, which we'd be forced to use in order to create a relationship
> > between our two devices since our setup is highly non generic. There will
> > probably be at some point support for such properties, and we will then be
> > able
> > to revisit some of this code.
> > 
> > All this is based on the work by Tim Gover in RPi's downstream
> > kernel[2].
> > 
> > [1] https://www.kernel.org/doc/html/v4.13/driver-api/device_link.html
> > [2] 
> > 
https://github.com/raspberrypi/linux/commit/9935b4c7e360b4494b4cb6e3ce797238a1ab78bd
> > 
> > ---
> > 
> > Changes since v7:
> >  - Address Stefan's comments
> > 
> > Changes since v6:
> >  - Make rpi_firmware_init_vl805() more robust
> >  - Rewrite comments and patch descriptions to be more accessible to non RPi
> >    fluent people
> >  - Removed Florian's Reviewed-by in patch #2 as function changed
> >    substantially
> >  - Tested with/witout u-boot
> > 
> > Changes since v5:
> >  - Fix issues reported by Kbuild test robot
> > 
> > Changes since v4:
> >  - Addressed Sergei's comments
> >  - Fix potential warning in patch #2
> > 
> > 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: Add notify xHCI reset property
> >   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                   |  3 +-
> >  drivers/firmware/raspberrypi.c             | 61 ++++++++++++++++++++++
> >  drivers/pci/controller/pcie-brcmstb.c      | 17 ++++++
> >  drivers/usb/host/pci-quirks.c              | 16 ++++++
> >  include/soc/bcm2835/raspberrypi-firmware.h |  9 +++-
> >  5 files changed, 104 insertions(+), 2 deletions(-)
> 
> Hi Nicolas,
> 
> should I queue this series via the PCI tree ? Just let me know, most of
> the changes are not in the PCI tree, asking in order to
> minimize/simplify conflicts handling if possible.

Yes, I agree, it's better if you take the whole thing.

Thanks,
Nicolas


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

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

[Lorenzo Pieralisi]

On Tue, May 05, 2020 at 06:13:13PM +0200, 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
> co-processor, VideoCore. This series adds support for the later.
> 
> Note that there are a set of constraints we have to consider:
>  - 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 non-existing state.
> 
> By Florian's suggestion I've been spending some time exploring the device
> link[1] API in order to see if that could save us from explicitly creating
> probe dependencies between pcie-brcmstb and firmware/raspberrypi (patch #3).
> Technically these dependencies could be inferred from DT. It turns out Saravana
> Kannan has been looking at this already. A new boot mechanism, activated with
> fw_devlink=on takes care of the device probe ordering on devices with
> consumer/supplier relationships. For now this relationship is created based on
> the usage of generic DT properties, but has no support for vendor-specifc DT
> properties, which we'd be forced to use in order to create a relationship
> between our two devices since our setup is highly non generic. There will
> probably be at some point support for such properties, and we will then be able
> to revisit some of this code.
> 
> All this is based on the work by Tim Gover in RPi's downstream
> kernel[2].
> 
> [1] https://www.kernel.org/doc/html/v4.13/driver-api/device_link.html
> [2] https://github.com/raspberrypi/linux/commit/9935b4c7e360b4494b4cb6e3ce797238a1ab78bd
> 
> ---
> 
> Changes since v7:
>  - Address Stefan's comments
> 
> Changes since v6:
>  - Make rpi_firmware_init_vl805() more robust
>  - Rewrite comments and patch descriptions to be more accessible to non RPi
>    fluent people
>  - Removed Florian's Reviewed-by in patch #2 as function changed
>    substantially
>  - Tested with/witout u-boot
> 
> Changes since v5:
>  - Fix issues reported by Kbuild test robot
> 
> Changes since v4:
>  - Addressed Sergei's comments
>  - Fix potential warning in patch #2
> 
> 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: Add notify xHCI reset property
>   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                   |  3 +-
>  drivers/firmware/raspberrypi.c             | 61 ++++++++++++++++++++++
>  drivers/pci/controller/pcie-brcmstb.c      | 17 ++++++
>  drivers/usb/host/pci-quirks.c              | 16 ++++++
>  include/soc/bcm2835/raspberrypi-firmware.h |  9 +++-
>  5 files changed, 104 insertions(+), 2 deletions(-)

Applied to pci/brcmstb, thanks !

Lorenzo

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

[Nicolas Saenz Julienne]

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

On Tue, 2020-05-05 at 18:13 +0200, 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
> VideoCore. Inform VideoCore that VL805 was just reset.
> 
> Also, as this creates a dependency between USB_PCI and VideoCore's
> firmware interface, and since USB_PCI can't be set as a module neither
> this can. Reflect that on the firmware interface Kconfg.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---

It was pointed out to me on the u-boot mailing lists that all this could be
implemented trough a reset controller. In other words have xhci get the reset
controller trough device-tree, assert it, ultamately causing the firmware
routine to be run.

As much as it pains me to go over stuff that's already 'fixed', it seems to me
it's a better solution. On one hand we get over the device-tree dependency mess
(see patch #3), and on the other we transform a pci-quirk into something less
hacky.

That said, before getting my hands dirty, I was wondering if there is any
obvious reasons why I shouldn't do this, note that:

- We're talking here of a PCIe XCHI device, maybe there's an issue integrating
  it with DT, given the fact that, as of today, it's not really represented
  there.

- There is no reset controller support in xhci-pci, maybe there are good
  reasons why. For instance, it's not something that's reflected in any way in
  the spec.

Regards,
Nicolas

> 
> Changes since v5:
>  - Fix Kconfig issue with allmodconfig
> 
> Changes since v4:
>  - Do not split up error message
> 
> Changes since v3:
>  - Add more complete error message
> 
> Changes since v1:
>  - Make RASPBERRYPI_FIRMWARE dependent on this quirk to make sure it
>    gets compiled when needed.
> 
>  drivers/firmware/Kconfig      |  3 ++-
>  drivers/usb/host/pci-quirks.c | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 8007d4aa76dc..b42140cff8ac 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -178,8 +178,9 @@ config ISCSI_IBFT
>  	  Otherwise, say N.
>  
>  config RASPBERRYPI_FIRMWARE
> -	tristate "Raspberry Pi Firmware Driver"
> +	bool "Raspberry Pi Firmware Driver"
>  	depends on BCM2835_MBOX
> +	default USB_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 92150ecdb036..0b949acfa258 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. Will
> continue to attempt to work, but bad things might happen. You should fix
> this...\n",
> +				 ret);
> +		}
> +	}
> +
>  	if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
>  			pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
>  			pdev->class != PCI_CLASS_SERIAL_USB_EHCI &&


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

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

[Florian Fainelli]

On 6/2/2020 3:05 AM, Nicolas Saenz Julienne wrote:
> On Tue, 2020-05-05 at 18:13 +0200, 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
>> VideoCore. Inform VideoCore that VL805 was just reset.
>>
>> Also, as this creates a dependency between USB_PCI and VideoCore's
>> firmware interface, and since USB_PCI can't be set as a module neither
>> this can. Reflect that on the firmware interface Kconfg.
>>
>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>> ---
> 
> It was pointed out to me on the u-boot mailing lists that all this could be
> implemented trough a reset controller. In other words have xhci get the reset
> controller trough device-tree, assert it, ultamately causing the firmware
> routine to be run.

That is actually a clever way to solve that problem.

> 
> As much as it pains me to go over stuff that's already 'fixed', it seems to me
> it's a better solution. On one hand we get over the device-tree dependency mess
> (see patch #3), and on the other we transform a pci-quirk into something less
> hacky.
> 
> That said, before getting my hands dirty, I was wondering if there is any
> obvious reasons why I shouldn't do this, note that:
> 
> - We're talking here of a PCIe XCHI device, maybe there's an issue integrating
>   it with DT, given the fact that, as of today, it's not really represented
>   there.

You can always provide a PCIe device representation within the Device
Tree, this is not very common, but it is sometimes useful for e.g.:
assigning MAC addresses, see this example for instance:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi#n647

(does not assign a MAC address, but it could). This should allow your
XHCI pci_device::of_node pointer to point to node declared in the Device
Tree. There you could add a 'resets' property accordingly.

> 
> - There is no reset controller support in xhci-pci, maybe there are good
>   reasons why. For instance, it's not something that's reflected in any way in
>   the spec.

It seems to me this is not usually necessary for PC systems, so it was
not really needed until now. Maybe you can write a small wrapper around
xhci-pci.c, similar to what xhci-plat.c does which is responsible for
grabbing and releasing the reset.
-- 
Florian