[v3,12/12] reset: Add Tegra BPMP reset driver

Message ID 20160819173233.13260-13-thierry.reding@gmail.com
State New
Headers show

Commit Message

Thierry Reding Aug. 19, 2016, 5:32 p.m.
From: Thierry Reding <treding@nvidia.com>

This driver uses the services provided by the BPMP firmware driver to
implement a reset driver based on the MRQ_RESET request.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/reset/Makefile           |  1 +
 drivers/reset/tegra/Makefile     |  1 +
 drivers/reset/tegra/reset-bpmp.c | 63 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)
 create mode 100644 drivers/reset/tegra/Makefile
 create mode 100644 drivers/reset/tegra/reset-bpmp.c

Comments

Stephen Warren Aug. 22, 2016, 7:56 p.m. | #1
On 08/19/2016 11:32 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> This driver uses the services provided by the BPMP firmware driver to
> implement a reset driver based on the MRQ_RESET request.

> diff --git a/drivers/reset/tegra/reset-bpmp.c b/drivers/reset/tegra/reset-bpmp.c

> +static struct tegra_bpmp *to_tegra_bpmp(struct reset_controller_dev *rstc)
> +{
> +	return container_of(rstc, struct tegra_bpmp, rstc);
> +}

It seems rather odd for the BPMP to include a struct 
reset_controller_dev inside it. Rather, I'd expect a separate reset 
controller struct to be defined here, which contains a pointer to the 
BPMP. Otherwise, the BPMP structure is going to get all kinds of stuff 
added to it.

Put another way, I'd expect the BPMP driver to be nothing more than an 
IPC provider, with all the clock/reset/power-domain/I2C/... logic 
contained in drivers that simply use that IPC service and nothing more, 
rather than clock/reset/power-domain/I2C/... being entirely in-bed with 
the core BPMP driver.

(I haven't reviewed patch 5/12, the BPMP driver yet BTW).

> +int tegra_bpmp_init_resets(struct tegra_bpmp *bpmp)
...
> +	bpmp->rstc.nr_resets = bpmp->soc->num_resets;

Presumably the BPMP already validates reset IDs. Can we leave all the 
validation to the BPMP? That way, we wouldn't need the SoC data to 
include a num_resets field, and hence wouldn't need to update the Linux 
driver in order to support a FW that supported more resets.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 5d65a93d3c43..523f2d705d24 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -6,6 +6,7 @@  obj-$(CONFIG_MACH_PISTACHIO) += reset-pistachio.o
 obj-$(CONFIG_ARCH_MESON) += reset-meson.o
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_ARCH_STI) += sti/
+obj-$(CONFIG_ARCH_TEGRA) += tegra/
 obj-$(CONFIG_ARCH_HISI) += hisilicon/
 obj-$(CONFIG_ARCH_ZYNQ) += reset-zynq.o
 obj-$(CONFIG_ATH79) += reset-ath79.o
diff --git a/drivers/reset/tegra/Makefile b/drivers/reset/tegra/Makefile
new file mode 100644
index 000000000000..fd943b1ae029
--- /dev/null
+++ b/drivers/reset/tegra/Makefile
@@ -0,0 +1 @@ 
+obj-$(CONFIG_ARCH_TEGRA_186_SOC)	+= reset-bpmp.o
diff --git a/drivers/reset/tegra/reset-bpmp.c b/drivers/reset/tegra/reset-bpmp.c
new file mode 100644
index 000000000000..a06e45dc3f30
--- /dev/null
+++ b/drivers/reset/tegra/reset-bpmp.c
@@ -0,0 +1,63 @@ 
+#include <linux/reset-controller.h>
+
+#include <soc/tegra/bpmp.h>
+#include <soc/tegra/bpmp-abi.h>
+
+static struct tegra_bpmp *to_tegra_bpmp(struct reset_controller_dev *rstc)
+{
+	return container_of(rstc, struct tegra_bpmp, rstc);
+}
+
+static int tegra_bpmp_reset_common(struct reset_controller_dev *rstc,
+				   enum mrq_reset_commands command,
+				   unsigned int id)
+{
+	struct tegra_bpmp *bpmp = to_tegra_bpmp(rstc);
+	struct mrq_reset_request request;
+	struct tegra_bpmp_message msg;
+
+	memset(&request, 0, sizeof(request));
+	request.cmd = command;
+	request.reset_id = id;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.mrq = MRQ_RESET;
+	msg.tx.data = &request;
+	msg.tx.size = sizeof(request);
+
+	return tegra_bpmp_transfer(bpmp, &msg);
+}
+
+static int tegra_bpmp_reset_module(struct reset_controller_dev *rstc,
+				   unsigned long id)
+{
+	return tegra_bpmp_reset_common(rstc, CMD_RESET_MODULE, id);
+}
+
+static int tegra_bpmp_reset_assert(struct reset_controller_dev *rstc,
+				   unsigned long id)
+{
+	return tegra_bpmp_reset_common(rstc, CMD_RESET_ASSERT, id);
+}
+
+static int tegra_bpmp_reset_deassert(struct reset_controller_dev *rstc,
+				     unsigned long id)
+{
+	return tegra_bpmp_reset_common(rstc, CMD_RESET_DEASSERT, id);
+}
+
+static const struct reset_control_ops tegra_bpmp_reset_ops = {
+	.reset = tegra_bpmp_reset_module,
+	.assert = tegra_bpmp_reset_assert,
+	.deassert = tegra_bpmp_reset_deassert,
+};
+
+int tegra_bpmp_init_resets(struct tegra_bpmp *bpmp)
+{
+	bpmp->rstc.ops = &tegra_bpmp_reset_ops;
+	bpmp->rstc.owner = THIS_MODULE;
+	bpmp->rstc.of_node = bpmp->dev->of_node;
+	bpmp->rstc.nr_resets = bpmp->soc->num_resets;
+
+	return devm_reset_controller_register(bpmp->dev, &bpmp->rstc);
+}