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.
---
Changes since v1:
- Rename function
- Use callback in xhci-pci.c
Nicolas Saenz Julienne (2):
arm: rpi: Add function to trigger VL805's firmware load
usb: xhci: Load Raspberry Pi 4 VL805's firmware
arch/arm/mach-bcm283x/include/mach/mbox.h | 13 +++++++
arch/arm/mach-bcm283x/include/mach/msg.h | 7 ++++
arch/arm/mach-bcm283x/msg.c | 43 +++++++++++++++++++++++
board/raspberrypi/rpi/rpi.c | 12 +++++++
drivers/usb/host/xhci-pci.c | 6 ++++
include/usb/xhci.h | 3 ++
6 files changed, 84 insertions(+)
--
2.26.2
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 VideoCore (the SoC's co-processor). Introduce the function that
informs VideoCore that VL805 may need its firmware loaded.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
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 | 43 +++++++++++++++++++++++
3 files changed, 63 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..a338190d33 100644
--- a/arch/arm/mach-bcm283x/msg.c
+++ b/arch/arm/mach-bcm283x/msg.c
@@ -40,6 +40,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 +157,40 @@ 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;
+ }
+
+ return 0;
+}
+
--
2.26.2
When needed, RPi4's co-processor (called VideoCore) has to be instructed
to load VL805's firmware (the chip providing xHCI support). VideoCore's
firmware expects the board's PCIe bus to be already configured in order
for it to load the xHCI chip firmware. So we have to make sure this
happens in between the PCIe configuration and xHCI startup.
Introduce a callback in xhci_pci_probe() to run this platform specific
routine.
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
Changes since v1:
- Create callback
board/raspberrypi/rpi/rpi.c | 12 ++++++++++++
drivers/usb/host/xhci-pci.c | 6 ++++++
include/usb/xhci.h | 3 +++
3 files changed, 21 insertions(+)
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index e367ba3092..8aa78d1f48 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -14,6 +14,7 @@
#include <lcd.h>
#include <memalign.h>
#include <mmc.h>
+#include <usb/xhci.h>
#include <asm/gpio.h>
#include <asm/arch/mbox.h>
#include <asm/arch/msg.h>
@@ -494,3 +495,14 @@ int ft_board_setup(void *blob, bd_t *bd)
return 0;
}
+
+#ifdef CONFIG_BCM2711
+void xhci_pci_fixup(struct udevice *dev)
+{
+ int ret;
+
+ ret = bcm2711_notify_vl805_reset();
+ if (ret)
+ printf("RPI: Failed to notify VideoCore about vl805's firmware\n");
+}
+#endif
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index c1f60da541..1285dde1ef 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -11,6 +11,10 @@
#include <usb.h>
#include <usb/xhci.h>
+__weak void xhci_pci_fixup(struct udevice *dev)
+{
+}
+
static void xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
struct xhci_hcor **ret_hcor)
{
@@ -40,6 +44,8 @@ static int xhci_pci_probe(struct udevice *dev)
struct xhci_hccr *hccr;
struct xhci_hcor *hcor;
+ xhci_pci_fixup(dev);
+
xhci_pci_init(dev, &hccr, &hcor);
return xhci_register(dev, hccr, hcor);
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index c16106a2fc..57feed7603 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -16,6 +16,7 @@
#ifndef HOST_XHCI_H_
#define HOST_XHCI_H_
+#include <usb.h>
#include <asm/types.h>
#include <asm/cache.h>
#include <asm/io.h>
@@ -1281,4 +1282,6 @@ extern struct dm_usb_ops xhci_usb_ops;
struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
+extern void xhci_pci_fixup(struct udevice *dev);
+
#endif /* HOST_XHCI_H_ */
--
2.26.2
On Thu, 2020-04-30 at 15:31 +0200, Mark Kettenis wrote:
> > From: Nicolas Saenz Julienne <[email protected]>
> > Date: Thu, 30 Apr 2020 15:04:32 +0200
> >
> > 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 VideoCore (the SoC's co-processor). Introduce the function that
> > informs VideoCore that VL805 may need its firmware loaded.
> >
> > Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> >
> > ---
> > 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 | 43 +++++++++++++++++++++++
> > 3 files changed, 63 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);
>
> Now the comment and function prototype don't agree :(.
Aargh, went too fast... I'll fix it on the next revision.
Regards,
Nicolas
> From: Nicolas Saenz Julienne <[email protected]>
> Date: Thu, 30 Apr 2020 15:04:32 +0200
>
> 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 VideoCore (the SoC's co-processor). Introduce the function that
> informs VideoCore that VL805 may need its firmware loaded.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>
> ---
> 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 | 43 +++++++++++++++++++++++
> 3 files changed, 63 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);
Now the comment and function prototype don't agree :(.
> +
> #endif
> diff --git a/arch/arm/mach-bcm283x/msg.c b/arch/arm/mach-bcm283x/msg.c
> index 94b75283f8..a338190d33 100644
> --- a/arch/arm/mach-bcm283x/msg.c
> +++ b/arch/arm/mach-bcm283x/msg.c
> @@ -40,6 +40,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 +157,40 @@ 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;
> + }
> +
> + return 0;
> +}
> +
> --
> 2.26.2
>
>
On 30/04/2020 15:04, Nicolas Saenz Julienne wrote:
> When needed, RPi4's co-processor (called VideoCore) has to be instructed
> to load VL805's firmware (the chip providing xHCI support). VideoCore's
> firmware expects the board's PCIe bus to be already configured in order
> for it to load the xHCI chip firmware. So we have to make sure this
> happens in between the PCIe configuration and xHCI startup.
>
> Introduce a callback in xhci_pci_probe() to run this platform specific
> routine.
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>
> ---
>
> Changes since v1:
> - Create callback
>
> board/raspberrypi/rpi/rpi.c | 12 ++++++++++++
> drivers/usb/host/xhci-pci.c | 6 ++++++
> include/usb/xhci.h | 3 +++
> 3 files changed, 21 insertions(+)
>
> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> index e367ba3092..8aa78d1f48 100644
> --- a/board/raspberrypi/rpi/rpi.c
> +++ b/board/raspberrypi/rpi/rpi.c
> @@ -14,6 +14,7 @@
> #include <lcd.h>
> #include <memalign.h>
> #include <mmc.h>
> +#include <usb/xhci.h>
> #include <asm/gpio.h>
> #include <asm/arch/mbox.h>
> #include <asm/arch/msg.h>
> @@ -494,3 +495,14 @@ int ft_board_setup(void *blob, bd_t *bd)
>
> return 0;
> }
> +
> +#ifdef CONFIG_BCM2711
This won't work with rpi_arm64_defconfig.
Can't we just evaluate at runtime if we need to do anything in xhci_pci_fixup.
I wonder if the newer RPi4 have also a newer revision ID (see get_board_rev). If
so we could add another bool to struct rpi_model which will indicate us if we
need to notify VideoCore about vl805's firmware.
Regards,
Matthias
> +void xhci_pci_fixup(struct udevice *dev)
> +{
> + int ret;
> +
> + ret = bcm2711_notify_vl805_reset();
> + if (ret)
> + printf("RPI: Failed to notify VideoCore about vl805's firmware\n");
> +}
> +#endif
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index c1f60da541..1285dde1ef 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -11,6 +11,10 @@
> #include <usb.h>
> #include <usb/xhci.h>
>
> +__weak void xhci_pci_fixup(struct udevice *dev)
> +{
> +}
> +
> static void xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
> struct xhci_hcor **ret_hcor)
> {
> @@ -40,6 +44,8 @@ static int xhci_pci_probe(struct udevice *dev)
> struct xhci_hccr *hccr;
> struct xhci_hcor *hcor;
>
> + xhci_pci_fixup(dev);
> +
> xhci_pci_init(dev, &hccr, &hcor);
>
> return xhci_register(dev, hccr, hcor);
> diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> index c16106a2fc..57feed7603 100644
> --- a/include/usb/xhci.h
> +++ b/include/usb/xhci.h
> @@ -16,6 +16,7 @@
> #ifndef HOST_XHCI_H_
> #define HOST_XHCI_H_
>
> +#include <usb.h>
> #include <asm/types.h>
> #include <asm/cache.h>
> #include <asm/io.h>
> @@ -1281,4 +1282,6 @@ extern struct dm_usb_ops xhci_usb_ops;
>
> struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
>
> +extern void xhci_pci_fixup(struct udevice *dev);
> +
> #endif /* HOST_XHCI_H_ */
>
>
> On 30/04/2020 15:04, Nicolas Saenz Julienne wrote:
> > When needed, RPi4's co-processor (called VideoCore) has to be instructed
> > to load VL805's firmware (the chip providing xHCI support). VideoCore's
> > firmware expects the board's PCIe bus to be already configured in order
> > for it to load the xHCI chip firmware. So we have to make sure this
> > happens in between the PCIe configuration and xHCI startup.
> >
> > Introduce a callback in xhci_pci_probe() to run this platform specific
> > routine.
> >
> > Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> >
> > ---
> >
> > Changes since v1:
> > - Create callback
> >
> > board/raspberrypi/rpi/rpi.c | 12 ++++++++++++
> > drivers/usb/host/xhci-pci.c | 6 ++++++
> > include/usb/xhci.h | 3 +++
> > 3 files changed, 21 insertions(+)
> >
> > diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> > index e367ba3092..8aa78d1f48 100644
> > --- a/board/raspberrypi/rpi/rpi.c
> > +++ b/board/raspberrypi/rpi/rpi.c
> > @@ -14,6 +14,7 @@
> > #include <lcd.h>
> > #include <memalign.h>
> > #include <mmc.h>
> > +#include <usb/xhci.h>
> > #include <asm/gpio.h>
> > #include <asm/arch/mbox.h>
> > #include <asm/arch/msg.h>
> > @@ -494,3 +495,14 @@ int ft_board_setup(void *blob, bd_t *bd)
> >
> > return 0;
> > }
> > +
> > +#ifdef CONFIG_BCM2711
>
> This won't work with rpi_arm64_defconfig.
> Can't we just evaluate at runtime if we need to do anything in xhci_pci_fixup.
>
> I wonder if the newer RPi4 have also a newer revision ID (see get_board_rev). If
> so we could add another bool to struct rpi_model which will indicate us if we
> need to notify VideoCore about vl805's firmware.
I believe they're ones ending in 03112:
https://github.com/raspberrypi/documentation/tree/master/hardware/raspberrypi/revision-codes
Hi Matthias,
On Tue, 2020-05-05 at 14:15 +0200, Matthias Brugger wrote:
>
> On 30/04/2020 15:04, Nicolas Saenz Julienne wrote:
> > When needed, RPi4's co-processor (called VideoCore) has to be instructed
> > to load VL805's firmware (the chip providing xHCI support). VideoCore's
> > firmware expects the board's PCIe bus to be already configured in order
> > for it to load the xHCI chip firmware. So we have to make sure this
> > happens in between the PCIe configuration and xHCI startup.
> >
> > Introduce a callback in xhci_pci_probe() to run this platform specific
> > routine.
> >
> > Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> >
> > ---
> >
> > Changes since v1:
> > - Create callback
> >
> > board/raspberrypi/rpi/rpi.c | 12 ++++++++++++
> > drivers/usb/host/xhci-pci.c | 6 ++++++
> > include/usb/xhci.h | 3 +++
> > 3 files changed, 21 insertions(+)
> >
> > diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> > index e367ba3092..8aa78d1f48 100644
> > --- a/board/raspberrypi/rpi/rpi.c
> > +++ b/board/raspberrypi/rpi/rpi.c
> > @@ -14,6 +14,7 @@
> > #include <lcd.h>
> > #include <memalign.h>
> > #include <mmc.h>
> > +#include <usb/xhci.h>
> > #include <asm/gpio.h>
> > #include <asm/arch/mbox.h>
> > #include <asm/arch/msg.h>
> > @@ -494,3 +495,14 @@ int ft_board_setup(void *blob, bd_t *bd)
> >
> > return 0;
> > }
> > +
> > +#ifdef CONFIG_BCM2711
>
> This won't work with rpi_arm64_defconfig.
> Can't we just evaluate at runtime if we need to do anything in xhci_pci_fixup.
I can't see why, who is going to call xhci_pci_probe() in RPi3?
Regards,
Nicolas
> I wonder if the newer RPi4 have also a newer revision ID (see get_board_rev).
> If
> so we could add another bool to struct rpi_model which will indicate us if we
> need to notify VideoCore about vl805's firmware.
>
> > +void xhci_pci_fixup(struct udevice *dev)
> > +{
> > + int ret;
> > +
> > + ret = bcm2711_notify_vl805_reset();
> > + if (ret)
> > + printf("RPI: Failed to notify VideoCore about vl805's
> > firmware\n");
> > +}
> > +#endif
> > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > index c1f60da541..1285dde1ef 100644
> > --- a/drivers/usb/host/xhci-pci.c
> > +++ b/drivers/usb/host/xhci-pci.c
> > @@ -11,6 +11,10 @@
> > #include <usb.h>
> > #include <usb/xhci.h>
> >
> > +__weak void xhci_pci_fixup(struct udevice *dev)
> > +{
> > +}
> > +
> > static void xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
> > struct xhci_hcor **ret_hcor)
> > {
> > @@ -40,6 +44,8 @@ static int xhci_pci_probe(struct udevice *dev)
> > struct xhci_hccr *hccr;
> > struct xhci_hcor *hcor;
> >
> > + xhci_pci_fixup(dev);
> > +
> > xhci_pci_init(dev, &hccr, &hcor);
> >
> > return xhci_register(dev, hccr, hcor);
> > diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> > index c16106a2fc..57feed7603 100644
> > --- a/include/usb/xhci.h
> > +++ b/include/usb/xhci.h
> > @@ -16,6 +16,7 @@
> > #ifndef HOST_XHCI_H_
> > #define HOST_XHCI_H_
> >
> > +#include <usb.h>
> > #include <asm/types.h>
> > #include <asm/cache.h>
> > #include <asm/io.h>
> > @@ -1281,4 +1282,6 @@ extern struct dm_usb_ops xhci_usb_ops;
> >
> > struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
> >
> > +extern void xhci_pci_fixup(struct udevice *dev);
> > +
> > #endif /* HOST_XHCI_H_ */
> >
On 05/05/2020 14:53, Nicolas Saenz Julienne wrote:
> Hi Matthias,
>
> On Tue, 2020-05-05 at 14:15 +0200, Matthias Brugger wrote:
>>
>> On 30/04/2020 15:04, Nicolas Saenz Julienne wrote:
>>> When needed, RPi4's co-processor (called VideoCore) has to be instructed
>>> to load VL805's firmware (the chip providing xHCI support). VideoCore's
>>> firmware expects the board's PCIe bus to be already configured in order
>>> for it to load the xHCI chip firmware. So we have to make sure this
>>> happens in between the PCIe configuration and xHCI startup.
>>>
>>> Introduce a callback in xhci_pci_probe() to run this platform specific
>>> routine.
>>>
>>> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>>>
>>> ---
>>>
>>> Changes since v1:
>>> - Create callback
>>>
>>> board/raspberrypi/rpi/rpi.c | 12 ++++++++++++
>>> drivers/usb/host/xhci-pci.c | 6 ++++++
>>> include/usb/xhci.h | 3 +++
>>> 3 files changed, 21 insertions(+)
>>>
>>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>>> index e367ba3092..8aa78d1f48 100644
>>> --- a/board/raspberrypi/rpi/rpi.c
>>> +++ b/board/raspberrypi/rpi/rpi.c
>>> @@ -14,6 +14,7 @@
>>> #include <lcd.h>
>>> #include <memalign.h>
>>> #include <mmc.h>
>>> +#include <usb/xhci.h>
>>> #include <asm/gpio.h>
>>> #include <asm/arch/mbox.h>
>>> #include <asm/arch/msg.h>
>>> @@ -494,3 +495,14 @@ int ft_board_setup(void *blob, bd_t *bd)
>>>
>>> return 0;
>>> }
>>> +
>>> +#ifdef CONFIG_BCM2711
>>
>> This won't work with rpi_arm64_defconfig.
>> Can't we just evaluate at runtime if we need to do anything in xhci_pci_fixup.
>
> I can't see why, who is going to call xhci_pci_probe() in RPi3?
>
If you do make rpi_arm64_defconfig and inspect the .config, you will see that
CONFIG_BCM2711 is not defined, so this code won't be called on RPi4.
Regards,
Matthias
> Regards,
> Nicolas
>
>> I wonder if the newer RPi4 have also a newer revision ID (see get_board_rev).
>> If
>> so we could add another bool to struct rpi_model which will indicate us if we
>> need to notify VideoCore about vl805's firmware.
>>
>>> +void xhci_pci_fixup(struct udevice *dev)
>>> +{
>>> + int ret;
>>> +
>>> + ret = bcm2711_notify_vl805_reset();
>>> + if (ret)
>>> + printf("RPI: Failed to notify VideoCore about vl805's
>>> firmware\n");
>>> +}
>>> +#endif
>>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>>> index c1f60da541..1285dde1ef 100644
>>> --- a/drivers/usb/host/xhci-pci.c
>>> +++ b/drivers/usb/host/xhci-pci.c
>>> @@ -11,6 +11,10 @@
>>> #include <usb.h>
>>> #include <usb/xhci.h>
>>>
>>> +__weak void xhci_pci_fixup(struct udevice *dev)
>>> +{
>>> +}
>>> +
>>> static void xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
>>> struct xhci_hcor **ret_hcor)
>>> {
>>> @@ -40,6 +44,8 @@ static int xhci_pci_probe(struct udevice *dev)
>>> struct xhci_hccr *hccr;
>>> struct xhci_hcor *hcor;
>>>
>>> + xhci_pci_fixup(dev);
>>> +
>>> xhci_pci_init(dev, &hccr, &hcor);
>>>
>>> return xhci_register(dev, hccr, hcor);
>>> diff --git a/include/usb/xhci.h b/include/usb/xhci.h
>>> index c16106a2fc..57feed7603 100644
>>> --- a/include/usb/xhci.h
>>> +++ b/include/usb/xhci.h
>>> @@ -16,6 +16,7 @@
>>> #ifndef HOST_XHCI_H_
>>> #define HOST_XHCI_H_
>>>
>>> +#include <usb.h>
>>> #include <asm/types.h>
>>> #include <asm/cache.h>
>>> #include <asm/io.h>
>>> @@ -1281,4 +1282,6 @@ extern struct dm_usb_ops xhci_usb_ops;
>>>
>>> struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
>>>
>>> +extern void xhci_pci_fixup(struct udevice *dev);
>>> +
>>> #endif /* HOST_XHCI_H_ */
>>>
>
On Tue, 2020-05-05 at 15:39 +0200, Matthias Brugger wrote:
>
> On 05/05/2020 14:53, Nicolas Saenz Julienne wrote:
> > Hi Matthias,
> >
> > On Tue, 2020-05-05 at 14:15 +0200, Matthias Brugger wrote:
> > > On 30/04/2020 15:04, Nicolas Saenz Julienne wrote:
> > > > When needed, RPi4's co-processor (called VideoCore) has to be instructed
> > > > to load VL805's firmware (the chip providing xHCI support). VideoCore's
> > > > firmware expects the board's PCIe bus to be already configured in order
> > > > for it to load the xHCI chip firmware. So we have to make sure this
> > > > happens in between the PCIe configuration and xHCI startup.
> > > >
> > > > Introduce a callback in xhci_pci_probe() to run this platform specific
> > > > routine.
> > > >
> > > > Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> > > >
> > > > ---
> > > >
> > > > Changes since v1:
> > > > - Create callback
> > > >
> > > > board/raspberrypi/rpi/rpi.c | 12 ++++++++++++
> > > > drivers/usb/host/xhci-pci.c | 6 ++++++
> > > > include/usb/xhci.h | 3 +++
> > > > 3 files changed, 21 insertions(+)
> > > >
> > > > diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> > > > index e367ba3092..8aa78d1f48 100644
> > > > --- a/board/raspberrypi/rpi/rpi.c
> > > > +++ b/board/raspberrypi/rpi/rpi.c
> > > > @@ -14,6 +14,7 @@
> > > > #include <lcd.h>
> > > > #include <memalign.h>
> > > > #include <mmc.h>
> > > > +#include <usb/xhci.h>
> > > > #include <asm/gpio.h>
> > > > #include <asm/arch/mbox.h>
> > > > #include <asm/arch/msg.h>
> > > > @@ -494,3 +495,14 @@ int ft_board_setup(void *blob, bd_t *bd)
> > > >
> > > > return 0;
> > > > }
> > > > +
> > > > +#ifdef CONFIG_BCM2711
> > >
> > > This won't work with rpi_arm64_defconfig.
> > > Can't we just evaluate at runtime if we need to do anything in
> > > xhci_pci_fixup.
> >
> > I can't see why, who is going to call xhci_pci_probe() in RPi3?
> >
>
> If you do make rpi_arm64_defconfig and inspect the .config, you will see that
> CONFIG_BCM2711 is not defined, so this code won't be called on RPi4.
Oh! Understood.
Well, given that only xhci_pci_probe() is called if we're running on RPi4, I
think I can disregard those defines altogether. I'll double-check that.
Regards,
Nicolas
> Regards,
> Matthias
>
> > Regards,
> > Nicolas
> >
> > > I wonder if the newer RPi4 have also a newer revision ID (see
> > > get_board_rev).
> > > If
> > > so we could add another bool to struct rpi_model which will indicate us if
> > > we
> > > need to notify VideoCore about vl805's firmware.
> > >
> > > > +void xhci_pci_fixup(struct udevice *dev)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + ret = bcm2711_notify_vl805_reset();
> > > > + if (ret)
> > > > + printf("RPI: Failed to notify VideoCore about vl805's
> > > > firmware\n");
> > > > +}
> > > > +#endif
> > > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > > > index c1f60da541..1285dde1ef 100644
> > > > --- a/drivers/usb/host/xhci-pci.c
> > > > +++ b/drivers/usb/host/xhci-pci.c
> > > > @@ -11,6 +11,10 @@
> > > > #include <usb.h>
> > > > #include <usb/xhci.h>
> > > >
> > > > +__weak void xhci_pci_fixup(struct udevice *dev)
> > > > +{
> > > > +}
> > > > +
> > > > static void xhci_pci_init(struct udevice *dev, struct xhci_hccr
> > > > **ret_hccr,
> > > > struct xhci_hcor **ret_hcor)
> > > > {
> > > > @@ -40,6 +44,8 @@ static int xhci_pci_probe(struct udevice *dev)
> > > > struct xhci_hccr *hccr;
> > > > struct xhci_hcor *hcor;
> > > >
> > > > + xhci_pci_fixup(dev);
> > > > +
> > > > xhci_pci_init(dev, &hccr, &hcor);
> > > >
> > > > return xhci_register(dev, hccr, hcor);
> > > > diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> > > > index c16106a2fc..57feed7603 100644
> > > > --- a/include/usb/xhci.h
> > > > +++ b/include/usb/xhci.h
> > > > @@ -16,6 +16,7 @@
> > > > #ifndef HOST_XHCI_H_
> > > > #define HOST_XHCI_H_
> > > >
> > > > +#include <usb.h>
> > > > #include <asm/types.h>
> > > > #include <asm/cache.h>
> > > > #include <asm/io.h>
> > > > @@ -1281,4 +1282,6 @@ extern struct dm_usb_ops xhci_usb_ops;
> > > >
> > > > struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
> > > >
> > > > +extern void xhci_pci_fixup(struct udevice *dev);
> > > > +
> > > > #endif /* HOST_XHCI_H_ */
> > > >
On 05/05/2020 15:47, Nicolas Saenz Julienne wrote:
> On Tue, 2020-05-05 at 15:39 +0200, Matthias Brugger wrote:
>>
>> On 05/05/2020 14:53, Nicolas Saenz Julienne wrote:
>>> Hi Matthias,
>>>
>>> On Tue, 2020-05-05 at 14:15 +0200, Matthias Brugger wrote:
>>>> On 30/04/2020 15:04, Nicolas Saenz Julienne wrote:
>>>>> When needed, RPi4's co-processor (called VideoCore) has to be instructed
>>>>> to load VL805's firmware (the chip providing xHCI support). VideoCore's
>>>>> firmware expects the board's PCIe bus to be already configured in order
>>>>> for it to load the xHCI chip firmware. So we have to make sure this
>>>>> happens in between the PCIe configuration and xHCI startup.
>>>>>
>>>>> Introduce a callback in xhci_pci_probe() to run this platform specific
>>>>> routine.
>>>>>
>>>>> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>>>>>
>>>>> ---
>>>>>
>>>>> Changes since v1:
>>>>> - Create callback
>>>>>
>>>>> board/raspberrypi/rpi/rpi.c | 12 ++++++++++++
>>>>> drivers/usb/host/xhci-pci.c | 6 ++++++
>>>>> include/usb/xhci.h | 3 +++
>>>>> 3 files changed, 21 insertions(+)
>>>>>
>>>>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>>>>> index e367ba3092..8aa78d1f48 100644
>>>>> --- a/board/raspberrypi/rpi/rpi.c
>>>>> +++ b/board/raspberrypi/rpi/rpi.c
>>>>> @@ -14,6 +14,7 @@
>>>>> #include <lcd.h>
>>>>> #include <memalign.h>
>>>>> #include <mmc.h>
>>>>> +#include <usb/xhci.h>
>>>>> #include <asm/gpio.h>
>>>>> #include <asm/arch/mbox.h>
>>>>> #include <asm/arch/msg.h>
>>>>> @@ -494,3 +495,14 @@ int ft_board_setup(void *blob, bd_t *bd)
>>>>>
>>>>> return 0;
>>>>> }
>>>>> +
>>>>> +#ifdef CONFIG_BCM2711
>>>>
>>>> This won't work with rpi_arm64_defconfig.
>>>> Can't we just evaluate at runtime if we need to do anything in
>>>> xhci_pci_fixup.
>>>
>>> I can't see why, who is going to call xhci_pci_probe() in RPi3?
>>>
>>
>> If you do make rpi_arm64_defconfig and inspect the .config, you will see that
>> CONFIG_BCM2711 is not defined, so this code won't be called on RPi4.
>
> Oh! Understood.
>
> Well, given that only xhci_pci_probe() is called if we're running on RPi4, I
> think I can disregard those defines altogether. I'll double-check that.
>
Yes but from my understanding we only need to call the function on newer
revisions of RPi4. Does it have any side effect on older revisions, e.g. we get
error messages (see below)?
[...]>>>> I wonder if the newer RPi4 have also a newer revision ID (see
>>>> get_board_rev).
>>>> If
>>>> so we could add another bool to struct rpi_model which will indicate us if
>>>> we
>>>> need to notify VideoCore about vl805's firmware.
>>>>
>>>>> +void xhci_pci_fixup(struct udevice *dev)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + ret = bcm2711_notify_vl805_reset();
>>>>> + if (ret)
>>>>> + printf("RPI: Failed to notify VideoCore about vl805's
>>>>> firmware\n");
We already have
printf("bcm2711: Faild to load vl805's firmware, %d\n", ret); in
bcm2711_notify_vl805_reset(). Do we really need two error messages?
Regards,
Matthias
>>>>> +}
>>>>> +#endif
>>>>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>>>>> index c1f60da541..1285dde1ef 100644
>>>>> --- a/drivers/usb/host/xhci-pci.c
>>>>> +++ b/drivers/usb/host/xhci-pci.c
>>>>> @@ -11,6 +11,10 @@
>>>>> #include <usb.h>
>>>>> #include <usb/xhci.h>
>>>>>
>>>>> +__weak void xhci_pci_fixup(struct udevice *dev)
>>>>> +{
>>>>> +}
>>>>> +
>>>>> static void xhci_pci_init(struct udevice *dev, struct xhci_hccr
>>>>> **ret_hccr,
>>>>> struct xhci_hcor **ret_hcor)
>>>>> {
>>>>> @@ -40,6 +44,8 @@ static int xhci_pci_probe(struct udevice *dev)
>>>>> struct xhci_hccr *hccr;
>>>>> struct xhci_hcor *hcor;
>>>>>
>>>>> + xhci_pci_fixup(dev);
>>>>> +
>>>>> xhci_pci_init(dev, &hccr, &hcor);
>>>>>
>>>>> return xhci_register(dev, hccr, hcor);
>>>>> diff --git a/include/usb/xhci.h b/include/usb/xhci.h
>>>>> index c16106a2fc..57feed7603 100644
>>>>> --- a/include/usb/xhci.h
>>>>> +++ b/include/usb/xhci.h
>>>>> @@ -16,6 +16,7 @@
>>>>> #ifndef HOST_XHCI_H_
>>>>> #define HOST_XHCI_H_
>>>>>
>>>>> +#include <usb.h>
>>>>> #include <asm/types.h>
>>>>> #include <asm/cache.h>
>>>>> #include <asm/io.h>
>>>>> @@ -1281,4 +1282,6 @@ extern struct dm_usb_ops xhci_usb_ops;
>>>>>
>>>>> struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
>>>>>
>>>>> +extern void xhci_pci_fixup(struct udevice *dev);
>>>>> +
>>>>> #endif /* HOST_XHCI_H_ */
>>>>>
>
On Tue, 2020-05-05 at 16:59 +0200, Matthias Brugger wrote:
[...]
> > > > > > +#ifdef CONFIG_BCM2711
> > > > >
> > > > > This won't work with rpi_arm64_defconfig.
> > > > > Can't we just evaluate at runtime if we need to do anything in
> > > > > xhci_pci_fixup.
> > > >
> > > > I can't see why, who is going to call xhci_pci_probe() in RPi3?
> > > >
> > >
> > > If you do make rpi_arm64_defconfig and inspect the .config, you will see
> > > that
> > > CONFIG_BCM2711 is not defined, so this code won't be called on RPi4.
> >
> > Oh! Understood.
> >
> > Well, given that only xhci_pci_probe() is called if we're running on RPi4, I
> > think I can disregard those defines altogether. I'll double-check that.
> >
>
> Yes but from my understanding we only need to call the function on newer
> revisions of RPi4. Does it have any side effect on older revisions, e.g. we
> get
> error messages (see below)?
The firmware quirk supports older rpi4s and simply does nothing. Note that the
downstream Linux implementation runs this on all rpi4s.
> [...]>>>> I wonder if the newer RPi4 have also a newer revision ID (see
> > > > > get_board_rev).
> > > > > If
> > > > > so we could add another bool to struct rpi_model which will indicate
> > > > > us if
> > > > > we
> > > > > need to notify VideoCore about vl805's firmware.
> > > > >
> > > > > > +void xhci_pci_fixup(struct udevice *dev)
> > > > > > +{
> > > > > > + int ret;
> > > > > > +
> > > > > > + ret = bcm2711_notify_vl805_reset();
> > > > > > + if (ret)
> > > > > > + printf("RPI: Failed to notify VideoCore about vl805's
> > > > > > firmware\n");
>
> We already have
> printf("bcm2711: Faild to load vl805's firmware, %d\n", ret); in
> bcm2711_notify_vl805_reset(). Do we really need two error messages?
Agree, it's a little redundant. I'll get rid of it.
Regards,
Nicolas