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

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

[Nicolas Saenz Julienne]

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 works hand in hand with a series of device tree changes on the
Linux device tree that are still being reviewed:
https://patchwork.kernel.org/patch/11596403/
https://patchwork.kernel.org/patch/11596409/

---

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 (5):
  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
  dm: pci: Assign controller device node to root bridge
  usb: xhci-pci: 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/pci/pci-uclass.c                      | 15 ++++-
 drivers/reset/Kconfig                         | 10 ++++
 drivers/reset/Makefile                        |  1 +
 drivers/reset/reset-raspberrypi.c             | 60 +++++++++++++++++++
 drivers/usb/host/xhci-pci.c                   | 38 +++++++++++-
 .../reset/raspberrypi,firmware-reset.h        | 13 ++++
 12 files changed, 203 insertions(+), 3 deletions(-)
 create mode 100644 drivers/reset/reset-raspberrypi.c
 create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h

-- 
2.26.2

[PATCH v4 1/5] arm: rpi: Add function to trigger VL805's firmware load

[Nicolas Saenz Julienne]

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;
+}
+
-- 
2.26.2

[PATCH v4 2/5] reset: Add Raspberry Pi 4 firmware reset controller

[Nicolas Saenz Julienne]

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
-- 
2.26.2

[PATCH v4 3/5] configs: Enable support for reset controllers on RPi4

[Nicolas Saenz Julienne]

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
-- 
2.26.2

[PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

[Nicolas Saenz Julienne]

There is no distinction in DT between the PCI controller device and the
root bridge, whereas such distinction exists from dm's perspective. Make
sure the root bridge ofnode is assigned to the controller's platform
device node.

This permits setups like this to work correctly:

	pcie {
		compatible = "...";
		...
		dev {
			reg = <0 0 0 0 0>;
			...
		};
	};

Without this the dev node is assigned to the root bridge and the
actual device search starts one level lower than expected.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/pci/pci-uclass.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 9ab3539a49..ea27e78465 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -762,7 +762,20 @@ static int pci_find_and_bind_driver(struct udevice *parent,
 	str = strdup(name);
 	if (!str)
 		return -ENOMEM;
-	drv = bridge ? "pci_bridge_drv" : "pci_generic_drv";
+
+	if (bridge) {
+		drv = "pci_bridge_drv";
+
+		/*
+		 * If we're dealing with the root bridge pass the parent device
+		 * node, as there isn't a distinction in device tree between
+		 * that and the actual controller platform device.
+		 */
+		if (!PCI_MASK_BUS(bdf))
+			node = parent->node;
+	} else {
+		drv = "pci_generic_drv";
+	}
 
 	ret = device_bind_driver_to_node(parent, drv, str, node, devp);
 	if (ret) {
-- 
2.26.2

[PATCH v4 5/5] usb: xhci-pci: Add reset controller support

[Nicolas Saenz Julienne]

Some atypical users of xhci-pci 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>
---
 drivers/usb/host/xhci-pci.c | 38 +++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 9fb6d2f763..710524fbb1 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -10,9 +10,14 @@
 #include <init.h>
 #include <log.h>
 #include <pci.h>
+#include <reset.h>
 #include <usb.h>
 #include <usb/xhci.h>
 
+struct xhci_pci_platdata {
+	struct reset_ctl reset;
+};
+
 static void xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
 			  struct xhci_hcor **ret_hcor)
 {
@@ -39,14 +44,43 @@ static void xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
 
 static int xhci_pci_probe(struct udevice *dev)
 {
+	struct xhci_pci_platdata *plat = dev_get_platdata(dev);
 	struct xhci_hccr *hccr;
 	struct xhci_hcor *hcor;
+	int ret;
+
+	ret = reset_get_by_index(dev, 0, &plat->reset);
+	if (ret && ret != -ENOENT) {
+		dev_err(dev, "failed to get reset\n");
+		return ret;
+	}
+
+	if (reset_valid(&plat->reset)) {
+		ret = reset_assert(&plat->reset);
+		if (ret)
+			return ret;
+
+		ret = reset_deassert(&plat->reset);
+		if (ret)
+			return ret;
+	}
 
 	xhci_pci_init(dev, &hccr, &hcor);
 
 	return xhci_register(dev, hccr, hcor);
 }
 
+static int xhci_pci_remove(struct udevice *dev)
+{
+	struct xhci_pci_platdata *plat = dev_get_platdata(dev);
+
+	xhci_deregister(dev);
+	if (reset_valid(&plat->reset))
+		reset_free(&plat->reset);
+
+	return 0;
+}
+
 static const struct udevice_id xhci_pci_ids[] = {
 	{ .compatible = "xhci-pci" },
 	{ }
@@ -56,10 +90,10 @@ U_BOOT_DRIVER(xhci_pci) = {
 	.name	= "xhci_pci",
 	.id	= UCLASS_USB,
 	.probe = xhci_pci_probe,
-	.remove = xhci_deregister,
+	.remove = xhci_pci_remove,
 	.of_match = xhci_pci_ids,
 	.ops	= &xhci_usb_ops,
-	.platdata_auto_alloc_size = sizeof(struct usb_platdata),
+	.platdata_auto_alloc_size = sizeof(struct xhci_pci_platdata),
 	.priv_auto_alloc_size = sizeof(struct xhci_ctrl),
 	.flags	= DM_FLAG_ALLOC_PRIV_DMA,
 };
-- 
2.26.2

Re: [PATCH v4 5/5] usb: xhci-pci: Add reset controller support

[Marek Vasut]

On 6/12/20 6:46 PM, Nicolas Saenz Julienne wrote:
> Some atypical users of xhci-pci 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>
> ---
>  drivers/usb/host/xhci-pci.c | 38 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)

Can the XHCI core code do the reset management instead ? Then everyone
benefits from it.

The rest of the series is nice, thanks.

Re: [PATCH v4 5/5] usb: xhci-pci: Add reset controller support

[Nicolas Saenz Julienne]

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

On Fri, 2020-06-12 at 19:08 +0200, Marek Vasut wrote:
> On 6/12/20 6:46 PM, Nicolas Saenz Julienne wrote:
> > Some atypical users of xhci-pci 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>
> > ---
> >  drivers/usb/host/xhci-pci.c | 38 +++++++++++++++++++++++++++++++++++--
> >  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> Can the XHCI core code do the reset management instead ? Then everyone
> benefits from it.

I'll look into it.

> The rest of the series is nice, thanks.

Thanks!

Regards,
Nicolas



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

Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

[Simon Glass]

Hi Nicolas,

On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> There is no distinction in DT between the PCI controller device and the
> root bridge, whereas such distinction exists from dm's perspective. Make
> sure the root bridge ofnode is assigned to the controller's platform
> device node.
>
> This permits setups like this to work correctly:
>
>         pcie {
>                 compatible = "...";
>                 ...
>                 dev {
>                         reg = <0 0 0 0 0>;
>                         ...
>                 };
>         };
>
> Without this the dev node is assigned to the root bridge and the
> actual device search starts one level lower than expected.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/pci/pci-uclass.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Can you update the tests to handle this case please?

Regards,
Simon

Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

[Nicolas Saenz Julienne]

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

On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote:
> Hi Nicolas,
> 
> On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > There is no distinction in DT between the PCI controller device and the
> > root bridge, whereas such distinction exists from dm's perspective. Make
> > sure the root bridge ofnode is assigned to the controller's platform
> > device node.
> > 
> > This permits setups like this to work correctly:
> > 
> >         pcie {
> >                 compatible = "...";
> >                 ...
> >                 dev {
> >                         reg = <0 0 0 0 0>;
> >                         ...
> >                 };
> >         };
> > 
> > Without this the dev node is assigned to the root bridge and the
> > actual device search starts one level lower than expected.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> >  drivers/pci/pci-uclass.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> Can you update the tests to handle this case please?

I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give me
some pointers on where/how to test this?

Regards,
Nicolas


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

Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

[Simon Glass]

Hi Nicolas,

On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote:
> > Hi Nicolas,
> >
> > On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
> > <nsaenzjulienne@suse.de> wrote:
> > > There is no distinction in DT between the PCI controller device and the
> > > root bridge, whereas such distinction exists from dm's perspective. Make
> > > sure the root bridge ofnode is assigned to the controller's platform
> > > device node.
> > >
> > > This permits setups like this to work correctly:
> > >
> > >         pcie {
> > >                 compatible = "...";
> > >                 ...
> > >                 dev {
> > >                         reg = <0 0 0 0 0>;
> > >                         ...
> > >                 };
> > >         };
> > >
> > > Without this the dev node is assigned to the root bridge and the
> > > actual device search starts one level lower than expected.
> > >
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > ---
> > >  drivers/pci/pci-uclass.c | 15 ++++++++++++++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > Can you update the tests to handle this case please?
>
> I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give me
> some pointers on where/how to test this?
>

Yes it is at test/dm/pci.c and the device tree is test.dts

'make qcheck' to run all tests. To run one test, build for sandbox and
then something like

u-boot -T -c "ut dm pci_swapcase"

for example.

You can perhaps use an existing PCI controller in test.dts but feel
free to add one more if you need it for your test. Make sure that you
don't break other tests.

Regards,
Simon

Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

[Nicolas Saenz Julienne]

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

On Tue, 2020-06-16 at 17:31 -0600, Simon Glass wrote:
> Hi Nicolas,
>
> On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote:
> > > Hi Nicolas,
> > >
> > > On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
> > > <nsaenzjulienne@suse.de> wrote:
> > > > There is no distinction in DT between the PCI controller device and the
> > > > root bridge, whereas such distinction exists from dm's perspective. Make
> > > > sure the root bridge ofnode is assigned to the controller's platform
> > > > device node.
> > > >
> > > > This permits setups like this to work correctly:
> > > >
> > > >         pcie {
> > > >                 compatible = "...";
> > > >                 ...
> > > >                 dev {
> > > >                         reg = <0 0 0 0 0>;
> > > >                         ...
> > > >                 };
> > > >         };
> > > >
> > > > Without this the dev node is assigned to the root bridge and the
> > > > actual device search starts one level lower than expected.
> > > >
> > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > ---
> > > >  drivers/pci/pci-uclass.c | 15 ++++++++++++++-
> > > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > >
> > > Can you update the tests to handle this case please?
> >
> > I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give
> > me
> > some pointers on where/how to test this?
> >
>
> Yes it is at test/dm/pci.c and the device tree is test.dts
>
> 'make qcheck' to run all tests. To run one test, build for sandbox and
> then something like
>
> u-boot -T -c "ut dm pci_swapcase"
>
> for example.
>
> You can perhaps use an existing PCI controller in test.dts but feel
> free to add one more if you need it for your test. Make sure that you
> don't break other tests.

Thanks for the info.

Actually adding the tests made me doubleguess myself, and now I'm pretty sure
that what I shoudl've done in DT is the following:

	&pcie0 {
	       pci@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>;
		       };
	       };
	};


(with "lspci -tv": [0000:00]---00.0-[01]----00.0  VIA Technologies, Inc. VL805 USB 3.0 Host Controller)

With this the patch above isn't needed, which is great.

I'll send this to upstream Linux just to get a confirmation this is correct,
although if you have any comments it'll be appreciated.

Regards,
Nicolas


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

Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

[Simon Glass]

Hi Nicolas,

On Wed, 17 Jun 2020 at 13:15, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> On Tue, 2020-06-16 at 17:31 -0600, Simon Glass wrote:
> > Hi Nicolas,
> >
> > On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne
> > <nsaenzjulienne@suse.de> wrote:
> > > On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote:
> > > > Hi Nicolas,
> > > >
> > > > On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
> > > > <nsaenzjulienne@suse.de> wrote:
> > > > > There is no distinction in DT between the PCI controller device and the
> > > > > root bridge, whereas such distinction exists from dm's perspective. Make
> > > > > sure the root bridge ofnode is assigned to the controller's platform
> > > > > device node.
> > > > >
> > > > > This permits setups like this to work correctly:
> > > > >
> > > > >         pcie {
> > > > >                 compatible = "...";
> > > > >                 ...
> > > > >                 dev {
> > > > >                         reg = <0 0 0 0 0>;
> > > > >                         ...
> > > > >                 };
> > > > >         };
> > > > >
> > > > > Without this the dev node is assigned to the root bridge and the
> > > > > actual device search starts one level lower than expected.
> > > > >
> > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > > ---
> > > > >  drivers/pci/pci-uclass.c | 15 ++++++++++++++-
> > > > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > >
> > > > Can you update the tests to handle this case please?
> > >
> > > I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give
> > > me
> > > some pointers on where/how to test this?
> > >
> >
> > Yes it is at test/dm/pci.c and the device tree is test.dts
> >
> > 'make qcheck' to run all tests. To run one test, build for sandbox and
> > then something like
> >
> > u-boot -T -c "ut dm pci_swapcase"
> >
> > for example.
> >
> > You can perhaps use an existing PCI controller in test.dts but feel
> > free to add one more if you need it for your test. Make sure that you
> > don't break other tests.
>
> Thanks for the info.
>
> Actually adding the tests made me doubleguess myself, and now I'm pretty sure
> that what I shoudl've done in DT is the following:
>
>         &pcie0 {
>                pci@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>;
>                        };
>                };
>         };
>
>
> (with "lspci -tv": [0000:00]---00.0-[01]----00.0  VIA Technologies, Inc. VL805 USB 3.0 Host Controller)
>
> With this the patch above isn't needed, which is great.
>
> I'll send this to upstream Linux just to get a confirmation this is correct,
> although if you have any comments it'll be appreciated.

Yes it looks OK to me. U-Boot allows PCI and USB devices to be
represented in the device tree.

Regards,
Simon