[PATCH v3 0/9] Raspberry Pi 4 USB firmware initialization rework

[PATCH v3 0/9] Raspberry Pi 4 USB firmware initialization rework

[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 reworks how we handle this.

The previous solution makes use of PCI quirks and exporting platform
specific functions. Albeit functional it feels pretty shoehorned. This
proposes an alternative way of handling the triggering of the xHCI chip
initialization trough means of a reset controller.

The benefits are pretty evident: less platform churn in core xHCI code,
and no explicit device dependency management in pcie-brcmstb.

Note that patch #1 depends on another series[1].

The series is based on next-20200611

v2: https://lkml.org/lkml/2020/6/9/875
v1: https://lore.kernel.org/linux-usb/20200608192701.18355-1-nsaenzjulienne@suse.de/T/#t

[1] https://lwn.net/ml/linux-kernel/cover.662a8d401787ef33780d91252a352de91dc4be10.1590594293.git-series.maxime@cerno.tech/

---

Changes since v2:
 - Add reset to resume routine in xhci-pci
 - Correct of refcount in pci-quirks
 - Correct typos
 - Use include file to define firmware reset IDs

Changes since v1:
 - Rework reset controller so it's less USB centric.
 - Use correct reset controller API in xhci-pci
 - Correct typos

Nicolas Saenz Julienne (9):
  dt-bindings: reset: Add a binding for the RPi Firmware reset
    controller
  reset: Add Raspberry Pi 4 firmware reset controller
  ARM: dts: bcm2711: Add firmware usb reset node
  ARM: dts: bcm2711: Add reset controller to xHCI node
  usb: xhci-pci: Add support for reset controllers
  Revert "USB: pci-quirks: Add Raspberry Pi 4 quirk"
  usb: host: pci-quirks: Bypass xHCI quirks for Raspberry Pi 4
  Revert "firmware: raspberrypi: Introduce vl805 init routine"
  Revert "PCI: brcmstb: Wait for Raspberry Pi's firmware when present"

 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml |  21 +++
 arch/arm/boot/dts/bcm2711-rpi-4-b.dts         |  14 ++
 drivers/firmware/Kconfig                      |   3 +-
 drivers/firmware/raspberrypi.c                |  61 ---------
 drivers/pci/controller/pcie-brcmstb.c         |  17 ---
 drivers/reset/Kconfig                         |  11 ++
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-raspberrypi.c             | 122 ++++++++++++++++++
 drivers/usb/host/pci-quirks.c                 |  22 ++--
 drivers/usb/host/xhci-pci.c                   |  10 ++
 drivers/usb/host/xhci.h                       |   2 +
 .../reset/raspberrypi,firmware-reset.h        |  13 ++
 include/soc/bcm2835/raspberrypi-firmware.h    |   7 -
 13 files changed, 207 insertions(+), 97 deletions(-)
 create mode 100644 drivers/reset/reset-raspberrypi.c
 create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h

-- 
2.26.2

[PATCH v3 1/9] dt-bindings: reset: Add a binding for the RPi Firmware reset controller

[Nicolas Saenz Julienne]

The firmware running on the RPi VideoCore can be used to reset and
initialize HW controlled by the firmware.

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

---
Changes since v2:
 - Add include file for reset IDs

Changes since v1:
 - Correct cells binding as per Florian's comment
 - Change compatible string to be more generic

 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++++++++++++++++++
 .../reset/raspberrypi,firmware-reset.h        | 13 ++++++++++++
 2 files changed, 34 insertions(+)
 create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h

diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
index b48ed875eb8e..23a885af3a28 100644
--- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -39,6 +39,22 @@ properties:
       - compatible
       - "#clock-cells"
 
+  reset:
+    type: object
+
+    properties:
+      compatible:
+        const: raspberrypi,firmware-reset
+
+      "#reset-cells":
+        const: 1
+        description: >
+          The argument is the ID of the firmware reset line to affect.
+
+    required:
+      - compatible
+      - "#reset-cells"
+
     additionalProperties: false
 
 required:
@@ -55,5 +71,10 @@ examples:
             compatible = "raspberrypi,firmware-clocks";
             #clock-cells = <1>;
         };
+
+        reset: reset {
+            compatible = "raspberrypi,firmware-reset";
+            #reset-cells = <1>;
+        };
     };
 ...
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 000000000000..1a4f4c792723
--- /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 v3 2/9] 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>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

---

Changes since v2:
 - Get ids from dt-binding

Changes since v1:
  - Make the whole driver less USB centric as per Florian's comments

 drivers/reset/Kconfig             |  11 +++
 drivers/reset/Makefile            |   1 +
 drivers/reset/reset-raspberrypi.c | 122 ++++++++++++++++++++++++++++++
 3 files changed, 134 insertions(+)
 create mode 100644 drivers/reset/reset-raspberrypi.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index d9efbfd29646..97e848740e13 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -140,6 +140,17 @@ config RESET_QCOM_PDC
 	  to control reset signals provided by PDC for Modem, Compute,
 	  Display, GPU, Debug, AOP, Sensors, Audio, SP and APPS.
 
+config RESET_RASPBERRYPI
+	tristate "Raspberry Pi 4 Firmware Reset Driver"
+	depends on RASPBERRYPI_FIRMWARE || (RASPBERRYPI_FIRMWARE=n && COMPILE_TEST)
+	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.
+
 config RESET_SCMI
 	tristate "Reset driver controlled via ARM SCMI interface"
 	depends on ARM_SCMI_PROTOCOL || COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 249ed357c997..16947610cc3b 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
 obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
 obj-$(CONFIG_RESET_QCOM_AOSS) += reset-qcom-aoss.o
 obj-$(CONFIG_RESET_QCOM_PDC) += reset-qcom-pdc.o
+obj-$(CONFIG_RESET_RASPBERRYPI) += reset-raspberrypi.o
 obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
 obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
 obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
diff --git a/drivers/reset/reset-raspberrypi.c b/drivers/reset/reset-raspberrypi.c
new file mode 100644
index 000000000000..91aa29f893b9
--- /dev/null
+++ b/drivers/reset/reset-raspberrypi.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Raspberry Pi 4 firmware reset driver
+ *
+ * Copyright (C) 2020 Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
+ */
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <soc/bcm2835/raspberrypi-firmware.h>
+#include <dt-bindings/reset/raspberrypi,firmware-reset.h>
+
+struct rpi_reset {
+	struct reset_controller_dev rcdev;
+	struct rpi_firmware *fw;
+};
+
+static inline struct rpi_reset *to_rpi(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct rpi_reset, rcdev);
+}
+
+static int rpi_reset_reset(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct rpi_reset *priv = to_rpi(rcdev);
+	u32 dev_addr;
+	int ret;
+
+	switch (id) {
+	case RASPBERRYPI_FIRMWARE_RESET_ID_USB:
+		/*
+		 * 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. rpi's
+		 * VideoCore OS contains both the non public firmware load
+		 * logic and the VL805 firmware blob. This triggers the
+		 * aforementioned process.
+		 *
+		 * The pci device address is expected is expected by the
+		 * firmware encoded like this:
+		 *
+		 *	PCI_BUS << 20 | PCI_SLOT << 15 | PCI_FUNC << 12
+		 *
+		 * But since rpi's PCIe is hardwired, we know the address in
+		 * advance.
+		 */
+		dev_addr = 0x100000;
+		ret = rpi_firmware_property(priv->fw, RPI_FIRMWARE_NOTIFY_XHCI_RESET,
+					    &dev_addr, sizeof(dev_addr));
+		if (ret)
+			return ret;
+
+		/* Wait for vl805 to startup */
+		usleep_range(200, 1000);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct reset_control_ops rpi_reset_ops = {
+	.reset	= rpi_reset_reset,
+};
+
+static int rpi_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *fw_node;
+	struct rpi_firmware *fw;
+	struct rpi_reset *priv;
+
+	fw_node = of_get_parent(dev->of_node);
+	if (!fw_node) {
+		dev_err(dev, "Missing firmware node\n");
+		return -ENOENT;
+	}
+
+	fw = rpi_firmware_get(fw_node);
+	of_node_put(fw_node);
+	if (!fw)
+		return -EPROBE_DEFER;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, priv);
+
+	priv->fw = fw;
+	priv->rcdev.owner = THIS_MODULE;
+	priv->rcdev.nr_resets = RASPBERRYPI_FIRMWARE_RESET_NUM_IDS;
+	priv->rcdev.ops = &rpi_reset_ops;
+	priv->rcdev.of_node = dev->of_node;
+
+	return devm_reset_controller_register(dev, &priv->rcdev);
+}
+
+static const struct of_device_id rpi_reset_of_match[] = {
+	{ .compatible = "raspberrypi,firmware-reset" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rpi_reset_of_match);
+
+static struct platform_driver rpi_reset_driver = {
+	.probe	= rpi_reset_probe,
+	.driver	= {
+		.name = "raspberrypi-reset",
+		.of_match_table = rpi_reset_of_match,
+	},
+};
+module_platform_driver(rpi_reset_driver);
+
+MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de>");
+MODULE_DESCRIPTION("Raspberry Pi 4 firmware reset driver");
+MODULE_LICENSE("GPL");
-- 
2.26.2

[PATCH v3 3/9] ARM: dts: bcm2711: Add firmware usb reset node

[Nicolas Saenz Julienne]

Now that the reset driver exposing Raspberry Pi 4's firmware based USB
reset routine is available, let's add the device tree node exposing it.

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

---

Changes since v1:
 - Update cell nr to match new bindings

 arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index c7f1d97e69bb..0cef95058fb0 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -83,6 +83,11 @@ expgpio: gpio {
 				  "";
 		status = "okay";
 	};
+
+	reset: reset {
+		compatible = "raspberrypi,firmware-reset";
+		#reset-cells = <1>;
+	};
 };
 
 &gpio {
-- 
2.26.2

[PATCH v3 4/9] ARM: dts: bcm2711: Add reset controller to xHCI node

[Nicolas Saenz Julienne]

The chip is hardwired to the board's PCIe bus and needs to be properly
setup trough a firmware routine after a PCI fundamental reset. Pass the
reset controller phandle that takes care of triggering the
initialization to the relevant PCI device.

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

---

Changes since v2:
 - Use dt-bindings to access IDs

Changes since v1:
 - Update to match new binding

 arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index 0cef95058fb0..e20979013414 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -4,6 +4,8 @@
 #include "bcm2835-rpi.dtsi"
 #include "bcm283x-rpi-usb-peripheral.dtsi"
 
+#include <dt-bindings/reset/raspberrypi,firmware-reset.h>
+
 / {
 	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
 	model = "Raspberry Pi 4 Model B";
@@ -207,6 +209,13 @@ phy1: ethernet-phy@1 {
 	};
 };
 
+&pcie0 {
+	usb@1,0 {
+		reg = <0 0 0 0 0>;
+		resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
+	};
+};
+
 /* uart0 communicates with the BT module */
 &uart0 {
 	pinctrl-names = "default";
-- 
2.26.2

[PATCH v3 5/9] usb: xhci-pci: Add support for reset controllers

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

---

Changes since v2:
 - Also reset on resume

Changes since v1:
 - Use proper reset API
 - Make code simpler

 drivers/usb/host/xhci-pci.c | 10 ++++++++++
 drivers/usb/host/xhci.h     |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index ef513c2fb843..e76b9283faa3 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/acpi.h>
+#include <linux/reset.h>
 
 #include "xhci.h"
 #include "xhci-trace.h"
@@ -339,6 +340,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	struct xhci_hcd *xhci;
 	struct usb_hcd *hcd;
 	struct xhci_driver_data *driver_data;
+	struct reset_control *reset;
 
 	driver_data = (struct xhci_driver_data *)id->driver_data;
 	if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
@@ -347,6 +349,11 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 			return retval;
 	}
 
+	reset = devm_reset_control_get_optional_exclusive(&dev->bus->dev, NULL);
+	if (IS_ERR(reset))
+		return PTR_ERR(reset);
+	reset_control_reset(reset);
+
 	/* Prevent runtime suspending between USB-2 and USB-3 initialization */
 	pm_runtime_get_noresume(&dev->dev);
 
@@ -364,6 +371,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	/* USB 2.0 roothub is stored in the PCI device now. */
 	hcd = dev_get_drvdata(&dev->dev);
 	xhci = hcd_to_xhci(hcd);
+	xhci->reset = reset;
 	xhci->shared_hcd = usb_create_shared_hcd(&xhci_pci_hc_driver, &dev->dev,
 						 pci_name(dev), hcd);
 	if (!xhci->shared_hcd) {
@@ -515,6 +523,8 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
 	struct pci_dev		*pdev = to_pci_dev(hcd->self.controller);
 	int			retval = 0;
 
+	reset_control_reset(xhci->reset);
+
 	/* The BIOS on systems with the Intel Panther Point chipset may or may
 	 * not support xHCI natively.  That means that during system resume, it
 	 * may switch the ports back to EHCI so that users can use their
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2c6c4f8d1ee1..379ffa24877d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1770,6 +1770,8 @@ struct xhci_hcd {
 	/* optional clocks */
 	struct clk		*clk;
 	struct clk		*reg_clk;
+	/* optional reset controller */
+	struct reset_control *reset;
 	/* data structures */
 	struct xhci_device_context_array *dcbaa;
 	struct xhci_ring	*cmd_ring;
-- 
2.26.2

[PATCH v3 6/9] Revert "USB: pci-quirks: Add Raspberry Pi 4 quirk"

[Nicolas Saenz Julienne]

This reverts commit c65822fef4adc0ba40c37a47337376ce75f7a7bc.

The initialization of Raspberry Pi 4's USB chip is now handled through a
reset controller. No need to directly call the firmware routine trough a
pci quirk.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/firmware/Kconfig      |  3 +--
 drivers/usb/host/pci-quirks.c | 16 ----------------
 2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index fbd785dd0513..4843e94713a4 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -178,9 +178,8 @@ config ISCSI_IBFT
 	  Otherwise, say N.
 
 config RASPBERRYPI_FIRMWARE
-	bool "Raspberry Pi Firmware Driver"
+	tristate "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 0b949acfa258..92150ecdb036 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,9 +16,6 @@
 #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"
 
@@ -1246,24 +1243,11 @@ 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

[PATCH v3 7/9] usb: host: pci-quirks: Bypass xHCI quirks for Raspberry Pi 4

[Nicolas Saenz Julienne]

The board doesn't need the quirks to be run, and takes care of its own
initialization through a reset controller device. So let's bypass them.

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

---

Changes since v2:
 - Correct reference counting on parent device node
 - Correct typos

Changes since v1:
 - Correct typos

 drivers/usb/host/pci-quirks.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 92150ecdb036..977d0b6d7577 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,6 +16,8 @@
 #include <linux/export.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
+#include <linux/of.h>
+
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"
 
@@ -1243,11 +1245,27 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
 
 static void quirk_usb_early_handoff(struct pci_dev *pdev)
 {
+	struct device_node *parent;
+	bool is_rpi;
+
 	/* 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;
+
+	/*
+	 * Bypass the Raspberry Pi 4 controller xHCI controller, things are
+	 * taken care of by the board's co-processor.
+	 */
+	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483) {
+		parent = of_get_parent(pdev->bus->dev.of_node);
+		is_rpi = of_device_is_compatible(parent, "brcm,bcm2711-pcie");
+		of_node_put(parent);
+		if (is_rpi)
+			return;
+	}
+
 	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

[PATCH v3 8/9] Revert "firmware: raspberrypi: Introduce vl805 init routine"

[Nicolas Saenz Julienne]

This reverts commit fbbc5ff3f7f9f4cad562e530ae2cf5d8964fe6d3.

The vl805 init routine has moved into drivers/reset/reset-raspberrypi.c

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/firmware/raspberrypi.c             | 61 ----------------------
 include/soc/bcm2835/raspberrypi-firmware.h |  7 ---
 2 files changed, 68 deletions(-)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index ef8098856a47..a3e85186f8e6 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -12,8 +12,6 @@
 #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))
@@ -21,8 +19,6 @@
 #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;
 
@@ -284,63 +280,6 @@ 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 3025aca3c358..cc9cdbc66403 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -10,7 +10,6 @@
 #include <linux/of_device.h>
 
 struct rpi_firmware;
-struct pci_dev;
 
 enum rpi_firmware_property_status {
 	RPI_FIRMWARE_STATUS_REQUEST = 0,
@@ -142,7 +141,6 @@ 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)
@@ -160,11 +158,6 @@ 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 v3 9/9] Revert "PCI: brcmstb: Wait for Raspberry Pi's firmware when present"

[Nicolas Saenz Julienne]

This reverts commit 44331189f9082c7e659697bbac1747db3def73e7.

Now that the VL805 init routine is run through a reset controller driver
the device dependencies are being taken care of by the device core. No
need to do it manually here.

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

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 7730ea845ff2..752f5b331579 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -28,8 +28,6 @@
 #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 */
@@ -931,26 +929,11 @@ 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

Re: [PATCH v3 5/9] usb: xhci-pci: Add support for reset controllers

[Florian Fainelli]

On 6/12/2020 10:13 AM, 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>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v3 5/9] usb: xhci-pci: Add support for reset controllers

[Mathias Nyman]

On 12.6.2020 20.13, 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>

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

Re: [PATCH v3 1/9] dt-bindings: reset: Add a binding for the RPi Firmware reset controller

[Philipp Zabel]

Hi Nicolas,

On Fri, 2020-06-12 at 19:13 +0200, Nicolas Saenz Julienne wrote:
> The firmware running on the RPi VideoCore can be used to reset and
> initialize HW controlled by the firmware.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> ---
> Changes since v2:
>  - Add include file for reset IDs
> 
> Changes since v1:
>  - Correct cells binding as per Florian's comment
>  - Change compatible string to be more generic
> 
>  .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++++++++++++++++++
>  .../reset/raspberrypi,firmware-reset.h        | 13 ++++++++++++
>  2 files changed, 34 insertions(+)
>  create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> index b48ed875eb8e..23a885af3a28 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> @@ -39,6 +39,22 @@ properties:
>        - compatible
>        - "#clock-cells"
>  
> +  reset:
> +    type: object
> +
> +    properties:
> +      compatible:
> +        const: raspberrypi,firmware-reset
> +
> +      "#reset-cells":
> +        const: 1
> +        description: >
> +          The argument is the ID of the firmware reset line to affect.
> +
> +    required:
> +      - compatible
> +      - "#reset-cells"
> +
>      additionalProperties: false
>  
>  required:
> @@ -55,5 +71,10 @@ examples:
>              compatible = "raspberrypi,firmware-clocks";
>              #clock-cells = <1>;
>          };
> +
> +        reset: reset {
> +            compatible = "raspberrypi,firmware-reset";
> +            #reset-cells = <1>;
> +        };
>      };
>  ...
> 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 000000000000..1a4f4c792723
> --- /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

Are there going to be any more firmware controlled resets in the future?

regards
Philipp

Re: [PATCH v3 2/9] reset: Add Raspberry Pi 4 firmware reset controller

[Philipp Zabel]

Hi Nicolas,

On Fri, 2020-06-12 at 19:13 +0200, Nicolas Saenz Julienne wrote:
> 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>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

If there is a good reason for the single DT specified reset id, I can
pick up patches 1 and 2.

If you change the dts patch 4 to use a number instead of the reset id
define for now, there wouldn't even be a dependency between these reset
and dts patches.

regards
Philipp

Re: [PATCH v3 5/9] usb: xhci-pci: Add support for reset controllers

[Philipp Zabel]

On Fri, 2020-06-12 at 19:13 +0200, 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>
> 
> ---
> 
> Changes since v2:
>  - Also reset on resume
> 
> Changes since v1:
>  - Use proper reset API
>  - Make code simpler
> 
>  drivers/usb/host/xhci-pci.c | 10 ++++++++++
>  drivers/usb/host/xhci.h     |  2 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index ef513c2fb843..e76b9283faa3 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -12,6 +12,7 @@
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <linux/acpi.h>
> +#include <linux/reset.h>
>  
>  #include "xhci.h"
>  #include "xhci-trace.h"
> @@ -339,6 +340,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	struct xhci_hcd *xhci;
>  	struct usb_hcd *hcd;
>  	struct xhci_driver_data *driver_data;
> +	struct reset_control *reset;
>  
>  	driver_data = (struct xhci_driver_data *)id->driver_data;
>  	if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
> @@ -347,6 +349,11 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  			return retval;
>  	}
>  
> +	reset = devm_reset_control_get_optional_exclusive(&dev->bus->dev, NULL);
> +	if (IS_ERR(reset))
> +		return PTR_ERR(reset);
> +	reset_control_reset(reset);

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [PATCH v3 1/9] dt-bindings: reset: Add a binding for the RPi Firmware reset controller

[Nicolas Saenz Julienne]

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

Hi,

On Wed, 2020-06-17 at 11:55 +0200, Philipp Zabel wrote:
> Hi Nicolas,
> 
> On Fri, 2020-06-12 at 19:13 +0200, Nicolas Saenz Julienne wrote:
> > The firmware running on the RPi VideoCore can be used to reset and
> > initialize HW controlled by the firmware.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > 
> > ---
> > Changes since v2:
> >  - Add include file for reset IDs
> > 
> > Changes since v1:
> >  - Correct cells binding as per Florian's comment
> >  - Change compatible string to be more generic
> > 
> >  .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++++++++++++++++++
> >  .../reset/raspberrypi,firmware-reset.h        | 13 ++++++++++++
> >  2 files changed, 34 insertions(+)
> >  create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > firmware.yaml
> > b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > firmware.yaml
> > index b48ed875eb8e..23a885af3a28 100644
> > --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > firmware.yaml
> > +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > firmware.yaml
> > @@ -39,6 +39,22 @@ properties:
> >        - compatible
> >        - "#clock-cells"
> >  
> > +  reset:
> > +    type: object
> > +
> > +    properties:
> > +      compatible:
> > +        const: raspberrypi,firmware-reset
> > +
> > +      "#reset-cells":
> > +        const: 1
> > +        description: >
> > +          The argument is the ID of the firmware reset line to affect.
> > +
> > +    required:
> > +      - compatible
> > +      - "#reset-cells"
> > +
> >      additionalProperties: false
> >  
> >  required:
> > @@ -55,5 +71,10 @@ examples:
> >              compatible = "raspberrypi,firmware-clocks";
> >              #clock-cells = <1>;
> >          };
> > +
> > +        reset: reset {
> > +            compatible = "raspberrypi,firmware-reset";
> > +            #reset-cells = <1>;
> > +        };
> >      };
> >  ...
> > 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 000000000000..1a4f4c792723
> > --- /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
> 
> Are there going to be any more firmware controlled resets in the future?

There are not right now, but it's likely some will show up in the future. I
have some contenders in mind, which I'll request once we settle on a design
here, but it ultimately depends on what the RPi people decide to implement.

Regards,
Nicolas


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

Re: [PATCH v3 2/9] reset: Add Raspberry Pi 4 firmware reset controller

[Nicolas Saenz Julienne]

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

On Wed, 2020-06-17 at 12:02 +0200, Philipp Zabel wrote:
> Hi Nicolas,
> 
> On Fri, 2020-06-12 at 19:13 +0200, Nicolas Saenz Julienne wrote:
> > 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>
> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

Thanks!

> If there is a good reason for the single DT specified reset id, I can
> pick up patches 1 and 2.

The idea here is to make sure we're reasonably covered against further changes
in firmware. If we define constraints too narrow it can be a pain to support
new features without breaking backwards compatibility in dt.

> If you change the dts patch 4 to use a number instead of the reset id
> define for now, there wouldn't even be a dependency between these reset
> and dts patches.

I was under the impression that having an explicit definition was nice to have.
What's troubling about creating the dependency?

Regards,
Nicolas


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

Re: [PATCH v3 4/9] ARM: dts: bcm2711: Add reset controller to xHCI node

[Nicolas Saenz Julienne]

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

Hi All,

On Fri, 2020-06-12 at 19:13 +0200, Nicolas Saenz Julienne wrote:
> The chip is hardwired to the board's PCIe bus and needs to be properly
> setup trough a firmware routine after a PCI fundamental reset. Pass the
> reset controller phandle that takes care of triggering the
> initialization to the relevant PCI device.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>
> ---
>
> Changes since v2:
>  - Use dt-bindings to access IDs
>
> Changes since v1:
>  - Update to match new binding
>
>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> index 0cef95058fb0..e20979013414 100644
> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> @@ -4,6 +4,8 @@
>  #include "bcm2835-rpi.dtsi"
>  #include "bcm283x-rpi-usb-peripheral.dtsi"
>
> +#include <dt-bindings/reset/raspberrypi,firmware-reset.h>
> +
>  / {
>  	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
>  	model = "Raspberry Pi 4 Model B";
> @@ -207,6 +209,13 @@ phy1: ethernet-phy@1 {
>  	};
>  };
>
> +&pcie0 {
> +	usb@1,0 {
> +		reg = <0 0 0 0 0>;
> +		resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
> +	};
> +};
> +

I'm now double-guessing this is correct. With this lspci -tv output:

[0000:00]---00.0-[01]----00.0  VIA Technologies, Inc. VL805 USB 3.0 Host Controller

The DT patch should be more like this:

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

Small details aside I'm pretty confident this is the way to go, but would
appreciate some comments/validation.

Regards,
Nicolas


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

Re: [PATCH v3 2/9] reset: Add Raspberry Pi 4 firmware reset controller

[Philipp Zabel]

On Wed, 2020-06-17 at 12:44 +0200, Nicolas Saenz Julienne wrote:
> On Wed, 2020-06-17 at 12:02 +0200, Philipp Zabel wrote:
> > Hi Nicolas,
> > 
> > On Fri, 2020-06-12 at 19:13 +0200, Nicolas Saenz Julienne wrote:
> > > 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>
> > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > 
> > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> Thanks!
> 
> > If there is a good reason for the single DT specified reset id, I can
> > pick up patches 1 and 2.
> 
> The idea here is to make sure we're reasonably covered against further changes
> in firmware. If we define constraints too narrow it can be a pain to support
> new features without breaking backwards compatibility in dt.

Ok.

> > If you change the dts patch 4 to use a number instead of the reset id
> > define for now, there wouldn't even be a dependency between these reset
> > and dts patches.
> 
> I was under the impression that having an explicit definition was nice to have.
> What's troubling about creating the dependency?

Just that the last patch has to wait for the reset patches to be merged
before it can be applied.

regards
Philipp

Re: [PATCH v3 2/9] reset: Add Raspberry Pi 4 firmware reset controller

[Nicolas Saenz Julienne]

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

On Fri, 2020-06-26 at 12:43 +0200, Philipp Zabel wrote:
> On Wed, 2020-06-17 at 12:44 +0200, Nicolas Saenz Julienne wrote:
> > On Wed, 2020-06-17 at 12:02 +0200, Philipp Zabel wrote:
> > > Hi Nicolas,
> > > 
> > > On Fri, 2020-06-12 at 19:13 +0200, Nicolas Saenz Julienne wrote:
> > > > 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>
> > > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > > 
> > > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> > 
> > Thanks!
> > 
> > > If there is a good reason for the single DT specified reset id, I can
> > > pick up patches 1 and 2.
> > 
> > The idea here is to make sure we're reasonably covered against further
> > changes
> > in firmware. If we define constraints too narrow it can be a pain to support
> > new features without breaking backwards compatibility in dt.
> 
> Ok.
> 
> > > If you change the dts patch 4 to use a number instead of the reset id
> > > define for now, there wouldn't even be a dependency between these reset
> > > and dts patches.
> > 
> > I was under the impression that having an explicit definition was nice to
> > have.
> > What's troubling about creating the dependency?
> 
> Just that the last patch has to wait for the reset patches to be merged
> before it can be applied.

TBH there is no hurry, this only provides a better design on something that's
already available upstream. USB works on RPi4, so I don't mind if this gets
delayed a release.

Regards,
Nicolas


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

Re: [PATCH v3 1/9] dt-bindings: reset: Add a binding for the RPi Firmware reset controller

[Rob Herring]

On Fri, Jun 12, 2020 at 07:13:25PM +0200, Nicolas Saenz Julienne wrote:
> The firmware running on the RPi VideoCore can be used to reset and
> initialize HW controlled by the firmware.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> ---
> Changes since v2:
>  - Add include file for reset IDs
> 
> Changes since v1:
>  - Correct cells binding as per Florian's comment
>  - Change compatible string to be more generic
> 
>  .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++++++++++++++++++
>  .../reset/raspberrypi,firmware-reset.h        | 13 ++++++++++++
>  2 files changed, 34 insertions(+)
>  create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> index b48ed875eb8e..23a885af3a28 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> @@ -39,6 +39,22 @@ properties:
>        - compatible
>        - "#clock-cells"
>  
> +  reset:

I'm not really thrilled how this is evolving with a node per provider. 
There's no reason you can't just add #clock-cells and #reset-cells to 
the parent firmware node.

I probably should have complained with the clocks node, but that's only 
pending for 5.9.

The bigger issue is this stuff is just trickling in one bit at a time 
which gives no context for review. What's next? Is it really a mystery 
as to what functions the firmware provides? You don't have to have a 
driver in place for every function.

Rob

Re: [PATCH v3 1/9] dt-bindings: reset: Add a binding for the RPi Firmware reset controller

[Nicolas Saenz Julienne]

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

On Mon, 2020-07-13 at 12:23 -0600, Rob Herring wrote:
> On Fri, Jun 12, 2020 at 07:13:25PM +0200, Nicolas Saenz Julienne wrote:
> > The firmware running on the RPi VideoCore can be used to reset and
> > initialize HW controlled by the firmware.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > 
> > ---
> > Changes since v2:
> >  - Add include file for reset IDs
> > 
> > Changes since v1:
> >  - Correct cells binding as per Florian's comment
> >  - Change compatible string to be more generic
> > 
> >  .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++++++++++++++++++
> >  .../reset/raspberrypi,firmware-reset.h        | 13 ++++++++++++
> >  2 files changed, 34 insertions(+)
> >  create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > firmware.yaml
> > b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > firmware.yaml
> > index b48ed875eb8e..23a885af3a28 100644
> > --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > firmware.yaml
> > +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > firmware.yaml
> > @@ -39,6 +39,22 @@ properties:
> >        - compatible
> >        - "#clock-cells"
> >  
> > +  reset:
> 
> I'm not really thrilled how this is evolving with a node per provider. 
> There's no reason you can't just add #clock-cells and #reset-cells to 
> the parent firmware node.

What are the downsides? The way I see it there is not much difference. And this
way of handling things is feels more intuitive and flexible (overlays can
control what to enable easily, we can take advantage of the platform device
core).

> I probably should have complained with the clocks node, but that's only 
> pending for 5.9.

Note that there are more users for this pattern: "raspberrypi,firmware-ts" and
"raspberrypi,firmware-gpio". Actually you were the one to originally propose
this it[1]. :P

There already is a fair amount of churn in these drivers because of all the DT
changes we did in the past, and if we need to change how we integrate these
again, I'd really like it to be for good.

> The bigger issue is this stuff is just trickling in one bit at a time 
> which gives no context for review. What's next? Is it really a mystery 
> as to what functions the firmware provides?

We have no control over it, RPi engineers integrate new designs and new
firmware interfaces show up. This is a good example of it.

I proposed them to use SCMI as it covers most of what they are already
providing here. But no luck so far.

> You don't have to have a driver in place for every function.

I see your point, it could be more monolithic, that said, having a driver is
essential. See the reverts I managed to pull off at the end of the series.

[1] https://patchwork.kernel.org/patch/10166783/#21421571


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

Re: [PATCH v3 1/9] dt-bindings: reset: Add a binding for the RPi Firmware reset controller

[Rob Herring]

On Tue, Jul 14, 2020 at 01:59:21PM +0200, Nicolas Saenz Julienne wrote:
> On Mon, 2020-07-13 at 12:23 -0600, Rob Herring wrote:
> > On Fri, Jun 12, 2020 at 07:13:25PM +0200, Nicolas Saenz Julienne wrote:
> > > The firmware running on the RPi VideoCore can be used to reset and
> > > initialize HW controlled by the firmware.
> > > 
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > > 
> > > ---
> > > Changes since v2:
> > >  - Add include file for reset IDs
> > > 
> > > Changes since v1:
> > >  - Correct cells binding as per Florian's comment
> > >  - Change compatible string to be more generic
> > > 
> > >  .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++++++++++++++++++
> > >  .../reset/raspberrypi,firmware-reset.h        | 13 ++++++++++++
> > >  2 files changed, 34 insertions(+)
> > >  create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > > firmware.yaml
> > > b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > > firmware.yaml
> > > index b48ed875eb8e..23a885af3a28 100644
> > > --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > > firmware.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> > > firmware.yaml
> > > @@ -39,6 +39,22 @@ properties:
> > >        - compatible
> > >        - "#clock-cells"
> > >  
> > > +  reset:
> > 
> > I'm not really thrilled how this is evolving with a node per provider. 
> > There's no reason you can't just add #clock-cells and #reset-cells to 
> > the parent firmware node.
> 
> What are the downsides? The way I see it there is not much difference. And this
> way of handling things is feels more intuitive and flexible (overlays can
> control what to enable easily, we can take advantage of the platform device
> core).

What the OS wants can evolve, so designing around the current needs of 
the OS is not how bindings should be done.

Using overlays to add clocks or resets wouldn't really work given they 
are spread out over the tree. And with clocks in particular, you'd have 
to replace dummy fixed clocks with actual firmware clocks. Sounds 
fragile and messy...

> > I probably should have complained with the clocks node, but that's only 
> > pending for 5.9.
> 
> Note that there are more users for this pattern: "raspberrypi,firmware-ts" and
> "raspberrypi,firmware-gpio". Actually you were the one to originally propose
> this it[1]. :P

Sigh, this is why I dislike incomplete examples...

Based on that,

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

And please get gpio and ts converted to schema and referenced here 
before the next time I look at this.

> There already is a fair amount of churn in these drivers because of all the DT
> changes we did in the past, and if we need to change how we integrate these
> again, I'd really like it to be for good.
> 
> > The bigger issue is this stuff is just trickling in one bit at a time 
> > which gives no context for review. What's next? Is it really a mystery 
> > as to what functions the firmware provides?
> 
> We have no control over it, RPi engineers integrate new designs and new
> firmware interfaces show up. This is a good example of it.
> 
> I proposed them to use SCMI as it covers most of what they are already
> providing here. But no luck so far.

Once we get tired of supporting all the different firmware interfaces 
and the mess they become, we'll just have to start refusing custom ones. 
Worked for PSCI.

> > You don't have to have a driver in place for every function.
> 
> I see your point, it could be more monolithic, that said, having a driver is
> essential. See the reverts I managed to pull off at the end of the series.
> 
> [1] https://patchwork.kernel.org/patch/10166783/#21421571
> 

Re: [PATCH v3 1/9] dt-bindings: reset: Add a binding for the RPi Firmware reset controller

[Florian Fainelli]

On 7/14/2020 2:07 PM, Rob Herring wrote:
> On Tue, Jul 14, 2020 at 01:59:21PM +0200, Nicolas Saenz Julienne wrote:
>> On Mon, 2020-07-13 at 12:23 -0600, Rob Herring wrote:
>>> On Fri, Jun 12, 2020 at 07:13:25PM +0200, Nicolas Saenz Julienne wrote:
>>>> The firmware running on the RPi VideoCore can be used to reset and
>>>> initialize HW controlled by the firmware.
>>>>
>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>>>>
>>>> ---
>>>> Changes since v2:
>>>>  - Add include file for reset IDs
>>>>
>>>> Changes since v1:
>>>>  - Correct cells binding as per Florian's comment
>>>>  - Change compatible string to be more generic
>>>>
>>>>  .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++++++++++++++++++
>>>>  .../reset/raspberrypi,firmware-reset.h        | 13 ++++++++++++
>>>>  2 files changed, 34 insertions(+)
>>>>  create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
>>>> firmware.yaml
>>>> b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
>>>> firmware.yaml
>>>> index b48ed875eb8e..23a885af3a28 100644
>>>> --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
>>>> firmware.yaml
>>>> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
>>>> firmware.yaml
>>>> @@ -39,6 +39,22 @@ properties:
>>>>        - compatible
>>>>        - "#clock-cells"
>>>>  
>>>> +  reset:
>>>
>>> I'm not really thrilled how this is evolving with a node per provider. 
>>> There's no reason you can't just add #clock-cells and #reset-cells to 
>>> the parent firmware node.
>>
>> What are the downsides? The way I see it there is not much difference. And this
>> way of handling things is feels more intuitive and flexible (overlays can
>> control what to enable easily, we can take advantage of the platform device
>> core).
> 
> What the OS wants can evolve, so designing around the current needs of 
> the OS is not how bindings should be done.
> 
> Using overlays to add clocks or resets wouldn't really work given they 
> are spread out over the tree. And with clocks in particular, you'd have 
> to replace dummy fixed clocks with actual firmware clocks. Sounds 
> fragile and messy...
> 
>>> I probably should have complained with the clocks node, but that's only 
>>> pending for 5.9.
>>
>> Note that there are more users for this pattern: "raspberrypi,firmware-ts" and
>> "raspberrypi,firmware-gpio". Actually you were the one to originally propose
>> this it[1]. :P
> 
> Sigh, this is why I dislike incomplete examples...
> 
> Based on that,
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 
> And please get gpio and ts converted to schema and referenced here 
> before the next time I look at this.
> 
>> There already is a fair amount of churn in these drivers because of all the DT
>> changes we did in the past, and if we need to change how we integrate these
>> again, I'd really like it to be for good.
>>
>>> The bigger issue is this stuff is just trickling in one bit at a time 
>>> which gives no context for review. What's next? Is it really a mystery 
>>> as to what functions the firmware provides?
>>
>> We have no control over it, RPi engineers integrate new designs and new
>> firmware interfaces show up. This is a good example of it.
>>
>> I proposed them to use SCMI as it covers most of what they are already
>> providing here. But no luck so far.
> 
> Once we get tired of supporting all the different firmware interfaces 
> and the mess they become, we'll just have to start refusing custom ones. 
> Worked for PSCI.

In this particular case, the Raspberry Pi Foundation VPU firmware should
just implement SCMI and that would avoid having to write new client
drivers for Linux, it is not clear to me why this has not been done yet.
-- 
Florian

Re: [PATCH v3 1/9] dt-bindings: reset: Add a binding for the RPi Firmware reset controller

[Rob Herring]

On Tue, Jul 14, 2020 at 3:18 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 7/14/2020 2:07 PM, Rob Herring wrote:
> > On Tue, Jul 14, 2020 at 01:59:21PM +0200, Nicolas Saenz Julienne wrote:
> >> On Mon, 2020-07-13 at 12:23 -0600, Rob Herring wrote:
> >>> On Fri, Jun 12, 2020 at 07:13:25PM +0200, Nicolas Saenz Julienne wrote:
> >>>> The firmware running on the RPi VideoCore can be used to reset and
> >>>> initialize HW controlled by the firmware.
> >>>>
> >>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> >>>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> >>>>
> >>>> ---
> >>>> Changes since v2:
> >>>>  - Add include file for reset IDs
> >>>>
> >>>> Changes since v1:
> >>>>  - Correct cells binding as per Florian's comment
> >>>>  - Change compatible string to be more generic
> >>>>
> >>>>  .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++++++++++++++++++
> >>>>  .../reset/raspberrypi,firmware-reset.h        | 13 ++++++++++++
> >>>>  2 files changed, 34 insertions(+)
> >>>>  create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> >>>> firmware.yaml
> >>>> b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> >>>> firmware.yaml
> >>>> index b48ed875eb8e..23a885af3a28 100644
> >>>> --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> >>>> firmware.yaml
> >>>> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-
> >>>> firmware.yaml
> >>>> @@ -39,6 +39,22 @@ properties:
> >>>>        - compatible
> >>>>        - "#clock-cells"
> >>>>
> >>>> +  reset:
> >>>
> >>> I'm not really thrilled how this is evolving with a node per provider.
> >>> There's no reason you can't just add #clock-cells and #reset-cells to
> >>> the parent firmware node.
> >>
> >> What are the downsides? The way I see it there is not much difference. And this
> >> way of handling things is feels more intuitive and flexible (overlays can
> >> control what to enable easily, we can take advantage of the platform device
> >> core).
> >
> > What the OS wants can evolve, so designing around the current needs of
> > the OS is not how bindings should be done.
> >
> > Using overlays to add clocks or resets wouldn't really work given they
> > are spread out over the tree. And with clocks in particular, you'd have
> > to replace dummy fixed clocks with actual firmware clocks. Sounds
> > fragile and messy...
> >
> >>> I probably should have complained with the clocks node, but that's only
> >>> pending for 5.9.
> >>
> >> Note that there are more users for this pattern: "raspberrypi,firmware-ts" and
> >> "raspberrypi,firmware-gpio". Actually you were the one to originally propose
> >> this it[1]. :P
> >
> > Sigh, this is why I dislike incomplete examples...
> >
> > Based on that,
> >
> > Acked-by: Rob Herring <robh@kernel.org>
> >
> > And please get gpio and ts converted to schema and referenced here
> > before the next time I look at this.
> >
> >> There already is a fair amount of churn in these drivers because of all the DT
> >> changes we did in the past, and if we need to change how we integrate these
> >> again, I'd really like it to be for good.
> >>
> >>> The bigger issue is this stuff is just trickling in one bit at a time
> >>> which gives no context for review. What's next? Is it really a mystery
> >>> as to what functions the firmware provides?
> >>
> >> We have no control over it, RPi engineers integrate new designs and new
> >> firmware interfaces show up. This is a good example of it.
> >>
> >> I proposed them to use SCMI as it covers most of what they are already
> >> providing here. But no luck so far.
> >
> > Once we get tired of supporting all the different firmware interfaces
> > and the mess they become, we'll just have to start refusing custom ones.
> > Worked for PSCI.
>
> In this particular case, the Raspberry Pi Foundation VPU firmware should
> just implement SCMI and that would avoid having to write new client
> drivers for Linux, it is not clear to me why this has not been done yet.

Writing drivers is fun?

Perhaps we should start refusing new firmware interfaces now.

Rob