[PATCH v5 0/4] usb: xhci: Load Raspberry Pi 4 VL805's firmware

Newer revisions of the RPi4 need their xHCI chip, VL805, firmware to be loaded explicitly. Earlier versions didn't need that as they where using an EEPROM for that purpose. This series takes care of setting up the relevant infrastructure and run the firmware loading routine at the right moment.
Note that this builds on top of Sylwester Nawrocki's "USB host support for Raspberry Pi 4 board" series.
This also depends on a DT/bindings patch available on the linux-mailing lists: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2205783.html
Here is the relevant bit for reference/discussion:
&pcie0 { pci@1,0 { #address-cells = <3>; #size-cells = <2>; ranges;
reg = <0 0 0 0 0>;
usb@1,0 { reg = <0x10000 0 0 0 0>; resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>; }; }; };
---
Changes since v4: - Correct DT PCI topology, I separated the root bridge and controller DT nodes - Get rid of PCI core patch as not needed with correct DT PCI topology - Move reset support to xchi core
Changes since v3: - Use reset controller
Changes since v2: - Correct comment on patch #1 - Address Matthias' comments
Changes since v1: - Rename function - Use callback in xhci-pci.c
Nicolas Saenz Julienne (4): arm: rpi: Add function to trigger VL805's firmware load reset: Add Raspberry Pi 4 firmware reset controller configs: Enable support for reset controllers on RPi4 usb: xhci: Add reset controller support
arch/arm/mach-bcm283x/include/mach/mbox.h | 13 ++++ arch/arm/mach-bcm283x/include/mach/msg.h | 7 +++ arch/arm/mach-bcm283x/msg.c | 46 ++++++++++++++ configs/rpi_4_32b_defconfig | 1 + configs/rpi_4_defconfig | 1 + configs/rpi_arm64_defconfig | 1 + drivers/reset/Kconfig | 10 ++++ drivers/reset/Makefile | 1 + drivers/reset/reset-raspberrypi.c | 60 +++++++++++++++++++ drivers/usb/host/xhci-mem.c | 2 + drivers/usb/host/xhci.c | 33 ++++++++++ .../reset/raspberrypi,firmware-reset.h | 13 ++++ include/usb/xhci.h | 2 + 13 files changed, 190 insertions(+) create mode 100644 drivers/reset/reset-raspberrypi.c create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h

On the Raspberry Pi 4, after a PCI reset, VL805's (a xHCI chip) firmware may either be loaded directly from an EEPROM or, if not present, by the SoC's VideCore (the SoC's co-processor). Introduce the function that informs VideCore that VL805 may need its firmware loaded.
Signed-off-by: Nicolas Saenz Julienne nsaenzjulienne@suse.de
--- Changes since v1: - Rename function so it's not mistaken with regular firmware loading
arch/arm/mach-bcm283x/include/mach/mbox.h | 13 +++++++ arch/arm/mach-bcm283x/include/mach/msg.h | 7 ++++ arch/arm/mach-bcm283x/msg.c | 46 +++++++++++++++++++++++ 3 files changed, 66 insertions(+)
diff --git a/arch/arm/mach-bcm283x/include/mach/mbox.h b/arch/arm/mach-bcm283x/include/mach/mbox.h index 60e226ce1d..2ae2d3d97c 100644 --- a/arch/arm/mach-bcm283x/include/mach/mbox.h +++ b/arch/arm/mach-bcm283x/include/mach/mbox.h @@ -491,6 +491,19 @@ struct bcm2835_mbox_tag_set_palette { } body; };
+#define BCM2835_MBOX_TAG_NOTIFY_XHCI_RESET 0x00030058 + +struct bcm2835_mbox_tag_pci_dev_addr { + struct bcm2835_mbox_tag_hdr tag_hdr; + union { + struct { + u32 dev_addr; + } req; + struct { + } resp; + } body; +}; + /* * Pass a raw u32 message to the VC, and receive a raw u32 back. * diff --git a/arch/arm/mach-bcm283x/include/mach/msg.h b/arch/arm/mach-bcm283x/include/mach/msg.h index 4afb08631b..e45c1bf010 100644 --- a/arch/arm/mach-bcm283x/include/mach/msg.h +++ b/arch/arm/mach-bcm283x/include/mach/msg.h @@ -48,4 +48,11 @@ int bcm2835_set_video_params(int *widthp, int *heightp, int depth_bpp, int pixel_order, int alpha_mode, ulong *fb_basep, ulong *fb_sizep, int *pitchp);
+/** + * bcm2711_load_vl805_firmware() - get vl805's firmware loaded + * + * @return 0 if OK, -EIO on error + */ +int bcm2711_notify_vl805_reset(void); + #endif diff --git a/arch/arm/mach-bcm283x/msg.c b/arch/arm/mach-bcm283x/msg.c index 94b75283f8..347aece3cd 100644 --- a/arch/arm/mach-bcm283x/msg.c +++ b/arch/arm/mach-bcm283x/msg.c @@ -7,6 +7,7 @@ #include <memalign.h> #include <phys2bus.h> #include <asm/arch/mbox.h> +#include <linux/delay.h>
struct msg_set_power_state { struct bcm2835_mbox_hdr hdr; @@ -40,6 +41,12 @@ struct msg_setup { u32 end_tag; };
+struct msg_notify_vl805_reset { + struct bcm2835_mbox_hdr hdr; + struct bcm2835_mbox_tag_pci_dev_addr dev_addr; + u32 end_tag; +}; + int bcm2835_power_on_module(u32 module) { ALLOC_CACHE_ALIGN_BUFFER(struct msg_set_power_state, msg_pwr, 1); @@ -151,3 +158,42 @@ int bcm2835_set_video_params(int *widthp, int *heightp, int depth_bpp,
return 0; } + +/* + * On the Raspberry Pi 4, after a PCI reset, VL805's (the xHCI chip) firmware + * may either be loaded directly from an EEPROM or, if not present, by the + * SoC's VideoCore. This informs VideoCore that VL805 needs its firmware + * loaded. + */ +int bcm2711_notify_vl805_reset(void) +{ + ALLOC_CACHE_ALIGN_BUFFER(struct msg_notify_vl805_reset, + msg_notify_vl805_reset, 1); + int ret; + + BCM2835_MBOX_INIT_HDR(msg_notify_vl805_reset); + BCM2835_MBOX_INIT_TAG(&msg_notify_vl805_reset->dev_addr, + NOTIFY_XHCI_RESET); + + /* + * The pci device address is expected like this: + * + * PCI_BUS << 20 | PCI_SLOT << 15 | PCI_FUNC << 12 + * + * But since RPi4's PCIe setup is hardwired, we know the address in + * advance. + */ + msg_notify_vl805_reset->dev_addr.body.req.dev_addr = 0x100000; + + ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, + &msg_notify_vl805_reset->hdr); + if (ret) { + printf("bcm2711: Faild to load vl805's firmware, %d\n", ret); + return -EIO; + } + + udelay(200); + + return 0; +} +

Raspberry Pi 4's co-processor controls some of the board's HW initialization process, but it's up to Linux to trigger it when relevant. Introduce a reset controller capable of interfacing with RPi4's co-processor that models these firmware initialization routines as reset lines.
Signed-off-by: Nicolas Saenz Julienne nsaenzjulienne@suse.de --- drivers/reset/Kconfig | 10 ++++ drivers/reset/Makefile | 1 + drivers/reset/reset-raspberrypi.c | 60 +++++++++++++++++++ .../reset/raspberrypi,firmware-reset.h | 13 ++++ 4 files changed, 84 insertions(+) create mode 100644 drivers/reset/reset-raspberrypi.c create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index 88d3be1593..d02c1522e5 100644 --- a/drivers/reset/Kconfig +++ b/drivers/reset/Kconfig @@ -148,4 +148,14 @@ config RESET_IMX7 help Support for reset controller on i.MX7/8 SoCs.
+config RESET_RASPBERRYPI + bool "Raspberry Pi 4 Firmware Reset Controller Driver" + depends on DM_RESET && ARCH_BCM283X + default USB_XHCI_PCI + help + Raspberry Pi 4's co-processor controls some of the board's HW + initialization process, but it's up to Linux to trigger it when + relevant. This driver provides a reset controller capable of + interfacing with RPi4's co-processor and model these firmware + initialization routines as reset lines. endmenu diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index 0a044d5d8c..be54dae725 100644 --- a/drivers/reset/Makefile +++ b/drivers/reset/Makefile @@ -23,3 +23,4 @@ obj-$(CONFIG_RESET_MTMIPS) += reset-mtmips.o obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o obj-$(CONFIG_RESET_HISILICON) += reset-hisilicon.o obj-$(CONFIG_RESET_IMX7) += reset-imx7.o +obj-$(CONFIG_RESET_RASPBERRYPI) += reset-raspberrypi.o diff --git a/drivers/reset/reset-raspberrypi.c b/drivers/reset/reset-raspberrypi.c new file mode 100644 index 0000000000..e2d284e5ac --- /dev/null +++ b/drivers/reset/reset-raspberrypi.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Raspberry Pi 4 firmware reset driver + * + * Copyright (C) 2020 Nicolas Saenz Julienne nsaenzjulienne@suse.de + */ +#include <common.h> +#include <dm.h> +#include <reset-uclass.h> +#include <asm/arch/msg.h> +#include <dt-bindings/reset/raspberrypi,firmware-reset.h> + +static int raspberrypi_reset_request(struct reset_ctl *reset_ctl) +{ + if (reset_ctl->id >= RASPBERRYPI_FIRMWARE_RESET_NUM_IDS) + return -EINVAL; + + return 0; +} + +static int raspberrypi_reset_free(struct reset_ctl *reset_ctl) +{ + return 0; +} + +static int raspberrypi_reset_assert(struct reset_ctl *reset_ctl) +{ + switch (reset_ctl->id) { + case RASPBERRYPI_FIRMWARE_RESET_ID_USB: + bcm2711_notify_vl805_reset(); + return 0; + default: + return -EINVAL; + } +} + +static int raspberrypi_reset_deassert(struct reset_ctl *reset_ctl) +{ + return 0; +} + +struct reset_ops raspberrypi_reset_ops = { + .request = raspberrypi_reset_request, + .rfree = raspberrypi_reset_free, + .rst_assert = raspberrypi_reset_assert, + .rst_deassert = raspberrypi_reset_deassert, +}; + +static const struct udevice_id raspberrypi_reset_ids[] = { + { .compatible = "raspberrypi,firmware-reset" }, + { } +}; + +U_BOOT_DRIVER(raspberrypi_reset) = { + .name = "raspberrypi-reset", + .id = UCLASS_RESET, + .of_match = raspberrypi_reset_ids, + .ops = &raspberrypi_reset_ops, +}; + diff --git a/include/dt-bindings/reset/raspberrypi,firmware-reset.h b/include/dt-bindings/reset/raspberrypi,firmware-reset.h new file mode 100644 index 0000000000..1a4f4c7927 --- /dev/null +++ b/include/dt-bindings/reset/raspberrypi,firmware-reset.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2020 Nicolas Saenz Julienne + * Author: Nicolas Saenz Julienne nsaenzjulienne@suse.com + */ + +#ifndef _DT_BINDINGS_RASPBERRYPI_FIRMWARE_RESET_H +#define _DT_BINDINGS_RASPBERRYPI_FIRMWARE_RESET_H + +#define RASPBERRYPI_FIRMWARE_RESET_ID_USB 0 +#define RASPBERRYPI_FIRMWARE_RESET_NUM_IDS 1 + +#endif

This is required in order to access the reset controller used to initialize the board's xHCI chip.
Signed-off-by: Nicolas Saenz Julienne nsaenzjulienne@suse.de --- configs/rpi_4_32b_defconfig | 1 + configs/rpi_4_defconfig | 1 + configs/rpi_arm64_defconfig | 1 + 3 files changed, 3 insertions(+)
diff --git a/configs/rpi_4_32b_defconfig b/configs/rpi_4_32b_defconfig index b0797373b5..00c8d963ab 100644 --- a/configs/rpi_4_32b_defconfig +++ b/configs/rpi_4_32b_defconfig @@ -35,6 +35,7 @@ CONFIG_DM_PCI=y CONFIG_PCI_BRCMSTB=y CONFIG_PINCTRL=y # CONFIG_PINCTRL_GENERIC is not set +CONFIG_DM_RESET=y # CONFIG_REQUIRE_SERIAL_CONSOLE is not set CONFIG_USB=y CONFIG_DM_USB=y diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig index 932b768164..c73eccb61c 100644 --- a/configs/rpi_4_defconfig +++ b/configs/rpi_4_defconfig @@ -35,6 +35,7 @@ CONFIG_DM_PCI=y CONFIG_PCI_BRCMSTB=y CONFIG_PINCTRL=y # CONFIG_PINCTRL_GENERIC is not set +CONFIG_DM_RESET=y # CONFIG_REQUIRE_SERIAL_CONSOLE is not set CONFIG_USB=y CONFIG_DM_USB=y diff --git a/configs/rpi_arm64_defconfig b/configs/rpi_arm64_defconfig index 855afcf1cf..800b51e6f5 100644 --- a/configs/rpi_arm64_defconfig +++ b/configs/rpi_arm64_defconfig @@ -32,6 +32,7 @@ CONFIG_DM_PCI=y CONFIG_PCI_BRCMSTB=y CONFIG_PINCTRL=y # CONFIG_PINCTRL_GENERIC is not set +CONFIG_DM_RESET=y # CONFIG_REQUIRE_SERIAL_CONSOLE is not set CONFIG_USB=y CONFIG_DM_USB=y

Some atypical users of xhci might need to manually reset their xHCI controller before starting the HCD setup. Check if a reset controller device is available to the PCI bus and trigger a reset.
Signed-off-by: Nicolas Saenz Julienne nsaenzjulienne@suse.de ---
Changes since v3: - Move reset support to xchi core
drivers/usb/host/xhci-mem.c | 2 ++ drivers/usb/host/xhci.c | 33 +++++++++++++++++++++++++++++++++ include/usb/xhci.h | 2 ++ 3 files changed, 37 insertions(+)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index f446520528..108f4bd8cf 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -180,6 +180,8 @@ void xhci_cleanup(struct xhci_ctrl *ctrl) xhci_free_virt_devices(ctrl); free(ctrl->erst.entries); free(ctrl->dcbaa); + if (reset_valid(&ctrl->reset)) + reset_free(&ctrl->reset); memset(ctrl, '\0', sizeof(struct xhci_ctrl)); }
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index ebd2954571..03b41cc855 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -190,6 +190,35 @@ static int xhci_start(struct xhci_hcor *hcor) return ret; }
+/** + * Resets XHCI Hardware + * + * @param ctrl pointer to host controller + * @return 0 if OK, or a negative error code. + */ +static int xhci_reset_hw(struct xhci_ctrl *ctrl) +{ + int ret; + + ret = reset_get_by_index(ctrl->dev, 0, &ctrl->reset); + if (ret && ret != -ENOENT) { + dev_err(ctrl->dev, "failed to get reset\n"); + return ret; + } + + if (reset_valid(&ctrl->reset)) { + ret = reset_assert(&ctrl->reset); + if (ret) + return ret; + + ret = reset_deassert(&ctrl->reset); + if (ret) + return ret; + } + + return 0; +} + /** * Resets the XHCI Controller * @@ -1508,6 +1537,10 @@ int xhci_register(struct udevice *dev, struct xhci_hccr *hccr,
ctrl->dev = dev;
+ ret = xhci_reset_hw(ctrl); + if (ret) + goto err; + /* * XHCI needs to issue a Address device command to setup * proper device context structures, before it can interact diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 1170c0ac69..7d34103fd5 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -16,6 +16,7 @@ #ifndef HOST_XHCI_H_ #define HOST_XHCI_H_
+#include <reset.h> #include <asm/types.h> #include <asm/cache.h> #include <asm/io.h> @@ -1209,6 +1210,7 @@ struct xhci_ctrl { #if CONFIG_IS_ENABLED(DM_USB) struct udevice *dev; #endif + struct reset_ctl reset; struct xhci_hccr *hccr; /* R/O registers, not need for volatile */ struct xhci_hcor *hcor; struct xhci_doorbell_array *dba;

On 6/22/20 5:30 PM, Nicolas Saenz Julienne wrote: [...]
diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 1170c0ac69..7d34103fd5 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -16,6 +16,7 @@ #ifndef HOST_XHCI_H_ #define HOST_XHCI_H_
+#include <reset.h> #include <asm/types.h> #include <asm/cache.h> #include <asm/io.h> @@ -1209,6 +1210,7 @@ struct xhci_ctrl { #if CONFIG_IS_ENABLED(DM_USB) struct udevice *dev; #endif
- struct reset_ctl reset;
Should all this reset logic be protected by if CONFIG_IS_ENABLED(DM_RESET) ?
Otherwise the series looks good to me, thanks.

Hi Marek,
On Mon, 2020-06-22 at 17:34 +0200, Marek Vasut wrote:
On 6/22/20 5:30 PM, Nicolas Saenz Julienne wrote: [...]
diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 1170c0ac69..7d34103fd5 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -16,6 +16,7 @@ #ifndef HOST_XHCI_H_ #define HOST_XHCI_H_
+#include <reset.h> #include <asm/types.h> #include <asm/cache.h> #include <asm/io.h> @@ -1209,6 +1210,7 @@ struct xhci_ctrl { #if CONFIG_IS_ENABLED(DM_USB) struct udevice *dev; #endif
- struct reset_ctl reset;
Should all this reset logic be protected by if CONFIG_IS_ENABLED(DM_RESET) ?
As far as building the code there shouldn't be any issues, function/structures are defined in either case.
That said I can see how there could be a runtime issue for the !CONFIG_IS_ENABLED(DM_RESET) case, as xhci_reset_hw() will return an error and make xchi_register() fail.
So I can either protect everything with preprocessor conditionals, both in the struct as in xhci_register(), or catch -ENOTSUPP in xhci_register() and continue the registration as usual. I prefer the later as it adds less preprocessor churn, but I'll go the other way if you prefer it.
Otherwise the series looks good to me, thanks.
Thanks!
Regards, Nicolas

On 6/24/20 12:55 PM, Nicolas Saenz Julienne wrote:
Hi Marek,
Hi,
On Mon, 2020-06-22 at 17:34 +0200, Marek Vasut wrote:
On 6/22/20 5:30 PM, Nicolas Saenz Julienne wrote: [...]
diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 1170c0ac69..7d34103fd5 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -16,6 +16,7 @@ #ifndef HOST_XHCI_H_ #define HOST_XHCI_H_
+#include <reset.h> #include <asm/types.h> #include <asm/cache.h> #include <asm/io.h> @@ -1209,6 +1210,7 @@ struct xhci_ctrl { #if CONFIG_IS_ENABLED(DM_USB) struct udevice *dev; #endif
- struct reset_ctl reset;
Should all this reset logic be protected by if CONFIG_IS_ENABLED(DM_RESET) ?
As far as building the code there shouldn't be any issues, function/structures are defined in either case.
That said I can see how there could be a runtime issue for the !CONFIG_IS_ENABLED(DM_RESET) case, as xhci_reset_hw() will return an error and make xchi_register() fail.
So I can either protect everything with preprocessor conditionals, both in the struct as in xhci_register(), or catch -ENOTSUPP in xhci_register() and continue the registration as usual. I prefer the later as it adds less preprocessor churn, but I'll go the other way if you prefer it.
I agree, lets go with the later, thanks.
participants (2)
-
Marek Vasut
-
Nicolas Saenz Julienne