[v3,2/9] reset: Add Raspberry Pi 4 firmware reset controller
diff mbox series

Message ID 20200612171334.26385-3-nsaenzjulienne@suse.de
State Superseded
Headers show
Series
  • Raspberry Pi 4 USB firmware initialization rework
Related show

Commit Message

Nicolas Saenz Julienne June 12, 2020, 5:13 p.m. UTC
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

Comments

Philipp Zabel June 17, 2020, 10:02 a.m. UTC | #1
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
Nicolas Saenz Julienne June 17, 2020, 10:44 a.m. UTC | #2
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
Philipp Zabel June 26, 2020, 10:43 a.m. UTC | #3
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
Nicolas Saenz Julienne June 29, 2020, 3:19 p.m. UTC | #4
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

Patch
diff mbox series

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");