All of lore.kernel.org
 help / color / mirror / Atom feed
* [v3 PATCH 1/6] drivers: reset: TI: SoC reset controller support.
@ 2014-07-17 16:45 ` Dan Murphy
  0 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, devicetree; +Cc: tony, Dan Murphy

The TI SoC reset controller support utilizes the
reset controller framework to give device drivers or
function drivers a common set of APIs to call to reset
a module.

The reset-ti is a common interface to the reset framework.
 The register data is retrieved during initialization
 of the reset driver through the reset-ti-data
file.  The array of data is associated with the compatible from the
respective DT entry.

Once the data is available then this is derefenced within the common
interface.

The device driver has the ability to assert, deassert or perform a
complete reset.

This code was derived from previous work by Rajendra Nayak and Afzal Mohammed.
The code was changed to adopt to the reset core and abstract away the SoC information.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - Resolved comments from v2.  To many to call out here - https://patchwork.kernel.org/patch/4116941/

 drivers/reset/Kconfig    |    9 ++
 drivers/reset/Makefile   |    1 +
 drivers/reset/reset-ti.c |  373 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 383 insertions(+)
 create mode 100644 drivers/reset/reset-ti.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 0615f50..31a5a79 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER
 
 	  If unsure, say no.
 
+config RESET_TI
+	depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST
+	bool "TI reset controller"
+	help
+	  Reset controller support for TI SoC's
+
+	  Reset controller found in TI's AM series of SoC's like
+	  AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7
+
 source "drivers/reset/sti/Kconfig"
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 60fed3d..a5986b9 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_RESET_CONTROLLER) += core.o
 obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
+obj-$(CONFIG_RESET_TI) += reset-ti.o
 obj-$(CONFIG_ARCH_STI) += sti/
diff --git a/drivers/reset/reset-ti.c b/drivers/reset/reset-ti.c
new file mode 100644
index 0000000..e9d4039
--- /dev/null
+++ b/drivers/reset/reset-ti.c
@@ -0,0 +1,373 @@
+/*
+ * reset-ti.c - PRCM reset driver for TI SoC's
+ *
+ * Copyright (C) 2014 Texas Instruments Incorporated -  http://www.ti.com
+ *
+ * Author: Dan Murphy <dmurphy@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#define DRIVER_NAME "prcm_reset_ti"
+#define MAX_RESET_SIGNALS 255
+
+/**
+ * struct ti_reset_reg_data - Structure of the reset register information
+ *		for a particular SoC.
+ * @rstctrl_offs: This is the reset control offset value from
+ *		from the parent reset node.
+ * @rstst_offs: This is the reset status offset value from
+ *		from the parent reset node.
+ * @rstctrl_bit: This is the reset control bit for the module.
+ * @rstst_bit: This is the reset status bit for the module.
+ *
+ * Longer description of this structure.
+ */
+struct ti_reset_reg_data {
+	phandle handle;
+	u32	rstctrl_offs;
+	u32	rstst_offs;
+	u32	rstctrl_bit;
+	u32	rstst_bit;
+};
+
+/**
+ * struct ti_reset_data - Structure that contains the reset register data
+ *	as well as the total number of resets for a particular SoC.
+ * @ti_data:	Pointer to this structure to be dereferenced
+ * @reg_data:	Pointer to the register data structure.
+ * @rcdev:		Reset controller device instance
+ * @dev:		Pointer to the devive structure
+ * @ti_reg_data: Array of register data.  Only reset signal with valid
+ *				phandles will be stored in this array.
+ * @reg_base:	Parent register base address
+ * @lock:		Spinlock for accessing the registers
+ * @nr_resets:	Total number of resets for the SoC in the reset array.
+ *
+ * This structure contains a pointer to the register data and the modules
+ * register base.  The number of resets and reset controller device data is
+ * stored within this structure.
+ *
+ */
+struct ti_reset_data {
+	struct ti_reset_data *ti_data;
+	struct ti_reset_reg_data *reg_data;
+	struct reset_controller_dev rcdev;
+	struct device *dev;
+	struct ti_reset_reg_data ti_reg_data[MAX_RESET_SIGNALS];
+	void __iomem *reg_base;
+	spinlock_t lock;
+	int nr_resets;
+};
+
+static int ti_reset_wait_on_reset(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct ti_reset_data *reset_data = container_of(rcdev,
+						     struct ti_reset_data,
+						     rcdev);
+	void __iomem *status_reg = reset_data->reg_base;
+	u32 bit_mask = 0;
+	u32 val = 0;
+	unsigned long flags;
+	int i = 1000;
+	int ret = 0;
+
+	spin_lock_irqsave(&reset_data->lock, flags);
+	/* Clear the reset status bit to reflect the current status */
+	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
+	bit_mask = reset_data->ti_reg_data[id].rstst_bit;
+
+	do {
+		val = readl(status_reg);
+	} while (--i && (val & (1 << bit_mask)));
+
+	if (i == 0 && val & (1 << bit_mask)) {
+		dev_err(reset_data->dev, "%s: Reset failed\n", __func__);
+		ret = -EIO;
+	}
+
+	spin_unlock_irqrestore(&reset_data->lock, flags);
+	return ret;
+}
+
+static int ti_reset_assert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	struct ti_reset_data *reset_data = container_of(rcdev,
+						     struct ti_reset_data,
+						     rcdev);
+	void __iomem *reg;
+	void __iomem *status_reg;
+	u32 status_bit = 0;
+	u32 bit_mask = 0;
+	u32 val = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&reset_data->lock, flags);
+
+	/* Clear the reset status bit to reflect the current status */
+	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
+	status_bit = reset_data->ti_reg_data[id].rstst_bit;
+	writel(1 << status_bit, status_reg);
+
+	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
+	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
+	val = readl(reg);
+	if (!(val & bit_mask)) {
+		val |= bit_mask;
+		writel(val, reg);
+	}
+
+	spin_unlock_irqrestore(&reset_data->lock, flags);
+	return 0;
+}
+
+static int ti_reset_deassert(struct reset_controller_dev *rcdev,
+			     unsigned long id)
+{
+
+	struct ti_reset_data *reset_data = container_of(rcdev,
+						     struct ti_reset_data,
+						     rcdev);
+	void __iomem *reg;
+	void __iomem *status_reg;
+	u32 status_bit = 0;
+	u32 bit_mask = 0;
+	u32 val = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&reset_data->lock, flags);
+
+	/* Clear the reset status bit to reflect the current status */
+	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
+	status_bit = reset_data->ti_reg_data[id].rstst_bit;
+	writel(1 << status_bit, status_reg);
+
+	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
+	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
+	val = readl(reg);
+	if (val & bit_mask) {
+		val &= ~bit_mask;
+		writel(val, reg);
+	}
+
+	spin_unlock_irqrestore(&reset_data->lock, flags);
+
+	return 0;
+}
+
+static int ti_reset_reset(struct reset_controller_dev *rcdev,
+			  unsigned long id)
+{
+	ti_reset_assert(rcdev, id);
+	ti_reset_deassert(rcdev, id);
+
+	return ti_reset_wait_on_reset(rcdev, id);
+}
+
+static int ti_reset_xlate(struct reset_controller_dev *rcdev,
+			const struct of_phandle_args *reset_spec)
+{
+
+	struct ti_reset_data *reset_data = container_of(rcdev,
+						     struct ti_reset_data,
+						     rcdev);
+	struct device_node *dev_node;
+	int i;
+
+	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
+		return -EINVAL;
+
+	/* Verify that the phandle exists */
+	dev_node = of_find_node_by_phandle((phandle) reset_spec->args[0]);
+	if (!dev_node) {
+		dev_err(reset_data->dev,
+				"%s: Cannot find phandle node\n",
+				__func__);
+		return -EINVAL;
+	}
+
+	for (i = 0; i <= reset_data->nr_resets; i++) {
+		if (reset_data->ti_reg_data[i].handle == dev_node->phandle)
+			return i;
+	}
+
+	return -EINVAL;
+}
+
+static struct reset_control_ops ti_reset_ops = {
+	.reset = ti_reset_reset,
+	.assert = ti_reset_assert,
+	.deassert = ti_reset_deassert,
+};
+
+static int ti_reset_populate_child(struct ti_reset_data *reset_data,
+					struct device_node *reset_child,
+					u32 ctrl_offs, u32 status_offs)
+{
+	struct device_node *next_reset_child = NULL;
+	int i = reset_data->nr_resets;
+	int ret;
+	u32 ctrl_bit;
+
+	while ((next_reset_child = of_get_next_child(reset_child, next_reset_child))) {
+		if (next_reset_child->phandle) {
+			/* Get the bits of each sub-reset */
+			ret = of_property_read_u32(next_reset_child, "control-bit",
+						&ctrl_bit);
+				if (ret < 0) {
+					dev_err(reset_data->dev,
+						"%s: No entry in %s for rstctrl_offs\n",
+						__func__, next_reset_child->name);
+
+				return -ENODEV;
+			}
+
+			ret = of_property_read_u32(next_reset_child, "status-bit",
+						&reset_data->ti_reg_data[i].rstst_bit);
+				if (ret < 0) {
+					dev_err(reset_data->dev,
+							"%s: No entry in %s for rstst_offs\n",
+							__func__, next_reset_child->name);
+
+				return -ENODEV;
+			}
+
+			reset_data->ti_reg_data[i].rstctrl_offs = ctrl_offs;
+			reset_data->ti_reg_data[i].rstst_offs = status_offs;
+			reset_data->ti_reg_data[i].handle = next_reset_child->phandle;
+			i++;
+		}
+	};
+
+	reset_data->nr_resets = i;
+
+	return 0;
+}
+
+static int ti_reset_get_of_data(struct ti_reset_data *reset_data,
+				struct device_node *dev_node)
+{
+	struct device_node *reset_child = NULL;
+	int ret = -EINVAL;
+	u32 ctrl_offs, status_offs;
+
+	/* Loop through all the children and populate a lookup array */
+	while ((reset_child = of_get_next_child(dev_node, reset_child))) {
+		ret = of_property_read_u32_index(reset_child, "reg", 0,
+						 &ctrl_offs);
+		if (ret)
+			return ret;
+
+		ret = of_property_read_u32_index(reset_child, "reg", 1,
+						 &status_offs);
+		if (ret)
+			return ret;
+
+		ret = ti_reset_populate_child(reset_data, reset_child,
+							ctrl_offs, status_offs);
+		if (ret)
+			return ret;
+	};
+
+	return 0;
+}
+
+static int ti_reset_probe(struct platform_device *pdev)
+{
+	struct device_node *resets;
+	struct ti_reset_data *reset_data;
+	struct resource *res;
+	int ret;
+
+	reset_data = devm_kzalloc(&pdev->dev, sizeof(*reset_data), GFP_KERNEL);
+	if (!reset_data)
+		return -ENOMEM;
+
+	resets = of_find_node_by_name(NULL, "resets");
+	if (!resets) {
+		dev_err(&pdev->dev, "%s: missing 'resets' child node.\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reset_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(reset_data->reg_base)) {
+		dev_err(&pdev->dev, "%s: Cannot map reset parent.\n", __func__);
+		return -ENODEV;
+	}
+
+	spin_lock_init(&reset_data->lock);
+	reset_data->dev = &pdev->dev;
+	ret = ti_reset_get_of_data(reset_data, resets);
+	if (ret)
+		return -EINVAL;
+
+	reset_data->rcdev.owner = THIS_MODULE;
+	reset_data->rcdev.of_node = resets;
+	reset_data->rcdev.ops = &ti_reset_ops;
+
+	reset_data->rcdev.of_reset_n_cells = 1;
+	reset_data->rcdev.of_xlate = &ti_reset_xlate;
+
+	reset_data->ti_data = reset_data;
+
+	platform_set_drvdata(pdev, reset_data);
+
+	reset_controller_register(&reset_data->rcdev);
+
+	return 0;
+}
+
+static int ti_reset_remove(struct platform_device *pdev)
+{
+	struct ti_reset_data *reset_data;
+
+	reset_data = platform_get_drvdata(pdev);
+	reset_controller_unregister(&reset_data->rcdev);
+
+	return 0;
+}
+
+static const struct of_device_id ti_reset_of_match[] = {
+	{ .compatible = "ti,omap5-prm" },
+	{ .compatible =	"ti,omap4-prm" },
+	{ .compatible =	"ti,omap5-prm" },
+	{ .compatible =	"ti,dra7-prm" },
+	{ .compatible = "ti,am4-prcm" },
+	{ .compatible =	"ti,am3-prcm" },
+	{},
+};
+
+static struct platform_driver ti_reset_driver = {
+	.probe	= ti_reset_probe,
+	.remove	= ti_reset_remove,
+	.driver	= {
+		.name		= DRIVER_NAME,
+		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(ti_reset_of_match),
+	},
+};
+module_platform_driver(ti_reset_driver);
+
+MODULE_DESCRIPTION("PRCM reset driver for TI SoCs");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 1/6] drivers: reset: TI: SoC reset controller support.
@ 2014-07-17 16:45 ` Dan Murphy
  0 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

The TI SoC reset controller support utilizes the
reset controller framework to give device drivers or
function drivers a common set of APIs to call to reset
a module.

The reset-ti is a common interface to the reset framework.
 The register data is retrieved during initialization
 of the reset driver through the reset-ti-data
file.  The array of data is associated with the compatible from the
respective DT entry.

Once the data is available then this is derefenced within the common
interface.

The device driver has the ability to assert, deassert or perform a
complete reset.

This code was derived from previous work by Rajendra Nayak and Afzal Mohammed.
The code was changed to adopt to the reset core and abstract away the SoC information.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - Resolved comments from v2.  To many to call out here - https://patchwork.kernel.org/patch/4116941/

 drivers/reset/Kconfig    |    9 ++
 drivers/reset/Makefile   |    1 +
 drivers/reset/reset-ti.c |  373 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 383 insertions(+)
 create mode 100644 drivers/reset/reset-ti.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 0615f50..31a5a79 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER
 
 	  If unsure, say no.
 
+config RESET_TI
+	depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST
+	bool "TI reset controller"
+	help
+	  Reset controller support for TI SoC's
+
+	  Reset controller found in TI's AM series of SoC's like
+	  AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7
+
 source "drivers/reset/sti/Kconfig"
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 60fed3d..a5986b9 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_RESET_CONTROLLER) += core.o
 obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
+obj-$(CONFIG_RESET_TI) += reset-ti.o
 obj-$(CONFIG_ARCH_STI) += sti/
diff --git a/drivers/reset/reset-ti.c b/drivers/reset/reset-ti.c
new file mode 100644
index 0000000..e9d4039
--- /dev/null
+++ b/drivers/reset/reset-ti.c
@@ -0,0 +1,373 @@
+/*
+ * reset-ti.c - PRCM reset driver for TI SoC's
+ *
+ * Copyright (C) 2014 Texas Instruments Incorporated -  http://www.ti.com
+ *
+ * Author: Dan Murphy <dmurphy@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#define DRIVER_NAME "prcm_reset_ti"
+#define MAX_RESET_SIGNALS 255
+
+/**
+ * struct ti_reset_reg_data - Structure of the reset register information
+ *		for a particular SoC.
+ * @rstctrl_offs: This is the reset control offset value from
+ *		from the parent reset node.
+ * @rstst_offs: This is the reset status offset value from
+ *		from the parent reset node.
+ * @rstctrl_bit: This is the reset control bit for the module.
+ * @rstst_bit: This is the reset status bit for the module.
+ *
+ * Longer description of this structure.
+ */
+struct ti_reset_reg_data {
+	phandle handle;
+	u32	rstctrl_offs;
+	u32	rstst_offs;
+	u32	rstctrl_bit;
+	u32	rstst_bit;
+};
+
+/**
+ * struct ti_reset_data - Structure that contains the reset register data
+ *	as well as the total number of resets for a particular SoC.
+ * @ti_data:	Pointer to this structure to be dereferenced
+ * @reg_data:	Pointer to the register data structure.
+ * @rcdev:		Reset controller device instance
+ * @dev:		Pointer to the devive structure
+ * @ti_reg_data: Array of register data.  Only reset signal with valid
+ *				phandles will be stored in this array.
+ * @reg_base:	Parent register base address
+ * @lock:		Spinlock for accessing the registers
+ * @nr_resets:	Total number of resets for the SoC in the reset array.
+ *
+ * This structure contains a pointer to the register data and the modules
+ * register base.  The number of resets and reset controller device data is
+ * stored within this structure.
+ *
+ */
+struct ti_reset_data {
+	struct ti_reset_data *ti_data;
+	struct ti_reset_reg_data *reg_data;
+	struct reset_controller_dev rcdev;
+	struct device *dev;
+	struct ti_reset_reg_data ti_reg_data[MAX_RESET_SIGNALS];
+	void __iomem *reg_base;
+	spinlock_t lock;
+	int nr_resets;
+};
+
+static int ti_reset_wait_on_reset(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct ti_reset_data *reset_data = container_of(rcdev,
+						     struct ti_reset_data,
+						     rcdev);
+	void __iomem *status_reg = reset_data->reg_base;
+	u32 bit_mask = 0;
+	u32 val = 0;
+	unsigned long flags;
+	int i = 1000;
+	int ret = 0;
+
+	spin_lock_irqsave(&reset_data->lock, flags);
+	/* Clear the reset status bit to reflect the current status */
+	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
+	bit_mask = reset_data->ti_reg_data[id].rstst_bit;
+
+	do {
+		val = readl(status_reg);
+	} while (--i && (val & (1 << bit_mask)));
+
+	if (i == 0 && val & (1 << bit_mask)) {
+		dev_err(reset_data->dev, "%s: Reset failed\n", __func__);
+		ret = -EIO;
+	}
+
+	spin_unlock_irqrestore(&reset_data->lock, flags);
+	return ret;
+}
+
+static int ti_reset_assert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	struct ti_reset_data *reset_data = container_of(rcdev,
+						     struct ti_reset_data,
+						     rcdev);
+	void __iomem *reg;
+	void __iomem *status_reg;
+	u32 status_bit = 0;
+	u32 bit_mask = 0;
+	u32 val = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&reset_data->lock, flags);
+
+	/* Clear the reset status bit to reflect the current status */
+	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
+	status_bit = reset_data->ti_reg_data[id].rstst_bit;
+	writel(1 << status_bit, status_reg);
+
+	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
+	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
+	val = readl(reg);
+	if (!(val & bit_mask)) {
+		val |= bit_mask;
+		writel(val, reg);
+	}
+
+	spin_unlock_irqrestore(&reset_data->lock, flags);
+	return 0;
+}
+
+static int ti_reset_deassert(struct reset_controller_dev *rcdev,
+			     unsigned long id)
+{
+
+	struct ti_reset_data *reset_data = container_of(rcdev,
+						     struct ti_reset_data,
+						     rcdev);
+	void __iomem *reg;
+	void __iomem *status_reg;
+	u32 status_bit = 0;
+	u32 bit_mask = 0;
+	u32 val = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&reset_data->lock, flags);
+
+	/* Clear the reset status bit to reflect the current status */
+	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
+	status_bit = reset_data->ti_reg_data[id].rstst_bit;
+	writel(1 << status_bit, status_reg);
+
+	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
+	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
+	val = readl(reg);
+	if (val & bit_mask) {
+		val &= ~bit_mask;
+		writel(val, reg);
+	}
+
+	spin_unlock_irqrestore(&reset_data->lock, flags);
+
+	return 0;
+}
+
+static int ti_reset_reset(struct reset_controller_dev *rcdev,
+			  unsigned long id)
+{
+	ti_reset_assert(rcdev, id);
+	ti_reset_deassert(rcdev, id);
+
+	return ti_reset_wait_on_reset(rcdev, id);
+}
+
+static int ti_reset_xlate(struct reset_controller_dev *rcdev,
+			const struct of_phandle_args *reset_spec)
+{
+
+	struct ti_reset_data *reset_data = container_of(rcdev,
+						     struct ti_reset_data,
+						     rcdev);
+	struct device_node *dev_node;
+	int i;
+
+	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
+		return -EINVAL;
+
+	/* Verify that the phandle exists */
+	dev_node = of_find_node_by_phandle((phandle) reset_spec->args[0]);
+	if (!dev_node) {
+		dev_err(reset_data->dev,
+				"%s: Cannot find phandle node\n",
+				__func__);
+		return -EINVAL;
+	}
+
+	for (i = 0; i <= reset_data->nr_resets; i++) {
+		if (reset_data->ti_reg_data[i].handle == dev_node->phandle)
+			return i;
+	}
+
+	return -EINVAL;
+}
+
+static struct reset_control_ops ti_reset_ops = {
+	.reset = ti_reset_reset,
+	.assert = ti_reset_assert,
+	.deassert = ti_reset_deassert,
+};
+
+static int ti_reset_populate_child(struct ti_reset_data *reset_data,
+					struct device_node *reset_child,
+					u32 ctrl_offs, u32 status_offs)
+{
+	struct device_node *next_reset_child = NULL;
+	int i = reset_data->nr_resets;
+	int ret;
+	u32 ctrl_bit;
+
+	while ((next_reset_child = of_get_next_child(reset_child, next_reset_child))) {
+		if (next_reset_child->phandle) {
+			/* Get the bits of each sub-reset */
+			ret = of_property_read_u32(next_reset_child, "control-bit",
+						&ctrl_bit);
+				if (ret < 0) {
+					dev_err(reset_data->dev,
+						"%s: No entry in %s for rstctrl_offs\n",
+						__func__, next_reset_child->name);
+
+				return -ENODEV;
+			}
+
+			ret = of_property_read_u32(next_reset_child, "status-bit",
+						&reset_data->ti_reg_data[i].rstst_bit);
+				if (ret < 0) {
+					dev_err(reset_data->dev,
+							"%s: No entry in %s for rstst_offs\n",
+							__func__, next_reset_child->name);
+
+				return -ENODEV;
+			}
+
+			reset_data->ti_reg_data[i].rstctrl_offs = ctrl_offs;
+			reset_data->ti_reg_data[i].rstst_offs = status_offs;
+			reset_data->ti_reg_data[i].handle = next_reset_child->phandle;
+			i++;
+		}
+	};
+
+	reset_data->nr_resets = i;
+
+	return 0;
+}
+
+static int ti_reset_get_of_data(struct ti_reset_data *reset_data,
+				struct device_node *dev_node)
+{
+	struct device_node *reset_child = NULL;
+	int ret = -EINVAL;
+	u32 ctrl_offs, status_offs;
+
+	/* Loop through all the children and populate a lookup array */
+	while ((reset_child = of_get_next_child(dev_node, reset_child))) {
+		ret = of_property_read_u32_index(reset_child, "reg", 0,
+						 &ctrl_offs);
+		if (ret)
+			return ret;
+
+		ret = of_property_read_u32_index(reset_child, "reg", 1,
+						 &status_offs);
+		if (ret)
+			return ret;
+
+		ret = ti_reset_populate_child(reset_data, reset_child,
+							ctrl_offs, status_offs);
+		if (ret)
+			return ret;
+	};
+
+	return 0;
+}
+
+static int ti_reset_probe(struct platform_device *pdev)
+{
+	struct device_node *resets;
+	struct ti_reset_data *reset_data;
+	struct resource *res;
+	int ret;
+
+	reset_data = devm_kzalloc(&pdev->dev, sizeof(*reset_data), GFP_KERNEL);
+	if (!reset_data)
+		return -ENOMEM;
+
+	resets = of_find_node_by_name(NULL, "resets");
+	if (!resets) {
+		dev_err(&pdev->dev, "%s: missing 'resets' child node.\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reset_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(reset_data->reg_base)) {
+		dev_err(&pdev->dev, "%s: Cannot map reset parent.\n", __func__);
+		return -ENODEV;
+	}
+
+	spin_lock_init(&reset_data->lock);
+	reset_data->dev = &pdev->dev;
+	ret = ti_reset_get_of_data(reset_data, resets);
+	if (ret)
+		return -EINVAL;
+
+	reset_data->rcdev.owner = THIS_MODULE;
+	reset_data->rcdev.of_node = resets;
+	reset_data->rcdev.ops = &ti_reset_ops;
+
+	reset_data->rcdev.of_reset_n_cells = 1;
+	reset_data->rcdev.of_xlate = &ti_reset_xlate;
+
+	reset_data->ti_data = reset_data;
+
+	platform_set_drvdata(pdev, reset_data);
+
+	reset_controller_register(&reset_data->rcdev);
+
+	return 0;
+}
+
+static int ti_reset_remove(struct platform_device *pdev)
+{
+	struct ti_reset_data *reset_data;
+
+	reset_data = platform_get_drvdata(pdev);
+	reset_controller_unregister(&reset_data->rcdev);
+
+	return 0;
+}
+
+static const struct of_device_id ti_reset_of_match[] = {
+	{ .compatible = "ti,omap5-prm" },
+	{ .compatible =	"ti,omap4-prm" },
+	{ .compatible =	"ti,omap5-prm" },
+	{ .compatible =	"ti,dra7-prm" },
+	{ .compatible = "ti,am4-prcm" },
+	{ .compatible =	"ti,am3-prcm" },
+	{},
+};
+
+static struct platform_driver ti_reset_driver = {
+	.probe	= ti_reset_probe,
+	.remove	= ti_reset_remove,
+	.driver	= {
+		.name		= DRIVER_NAME,
+		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(ti_reset_of_match),
+	},
+};
+module_platform_driver(ti_reset_driver);
+
+MODULE_DESCRIPTION("PRCM reset driver for TI SoCs");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 2/6] dt: TI: Describe the ti reset DT entries
  2014-07-17 16:45 ` Dan Murphy
@ 2014-07-17 16:45   ` Dan Murphy
  -1 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, devicetree; +Cc: tony, Dan Murphy

Describe the TI reset DT entries for TI SoC's.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - Changed Headline no other changes

 .../devicetree/bindings/reset/ti,reset.txt         |  103 ++++++++++++++++++++
 1 file changed, 103 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/ti,reset.txt

diff --git a/Documentation/devicetree/bindings/reset/ti,reset.txt b/Documentation/devicetree/bindings/reset/ti,reset.txt
new file mode 100644
index 0000000..9d5c29c
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/ti,reset.txt
@@ -0,0 +1,103 @@
+Texas Instruments Reset Controller
+======================================
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Specifying the reset entries for the IP module
+==============================================
+Parent module:
+This is the module node that contains the reset registers and bits.
+
+example:
+			prcm_resets: resets {
+				compatible = "ti,dra7-resets";
+				#reset-cells = <1>;
+			};
+
+Required parent properties:
+- compatible : Should be one of,
+		"ti,omap4-prm" for OMAP4 PRM instances
+		"ti,omap5-prm" for OMAP5 PRM instances
+		"ti,dra7-prm" for DRA7xx PRM instances
+		"ti,am4-prcm" for AM43xx PRCM instances
+		"ti,am3-prcm" for AM33xx PRCM instances
+
+Required child reset property:
+- compatible : Should be
+		"resets" for All TI SoCs
+
+example:
+		prm: prm@4ae06000 {
+			compatible = "ti,omap5-prm";
+			reg = <0x4ae06000 0x3000>;
+
+			prm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
+		};
+
+
+Reset node declaration
+==============================================
+The reset node is declared in a parent child relationship.  The main parent
+is the PRCM module which contains the base address.  The first child within
+the reset parent declares the target modules reset name.  This is followed by
+the control and status offsets.
+
+Within the first reset child node is a secondary child node which declares the
+reset signal of interest.  Under this node the control and status bits
+are declared.  These bits declare the bit mask for the target reset.
+
+
+Required properties:
+reg - This is the register offset from the PRCM parent.
+	This must be declared as:
+
+	reg = <control register offset>,
+		  <status register offset>;
+
+control-bit - This is the bit within the register which controls the reset
+	of the target module.  This is declared as a bit mask for the register.
+status-bit - This is the bit within the register which contains the status of
+	the reset of the target module.
+	This is declared as a bit mask for the register.
+
+example:
+&prm_resets {
+	dsp_rstctrl {
+		reg = <0x1c00>,
+			  <0x1c04>;
+
+		dsp_reset: dsp_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+};
+
+
+
+Client Node Declaration
+==============================================
+This is the consumer of the parent node to declare what resets this
+particular module is interested in.
+
+example:
+	src: src@55082000 {
+		    resets = <&reset_src phandle>;
+			reset-names = "<reset_name>";
+	};
+
+Required Properties:
+reset_src - This is the parent DT entry for the reset controller
+phandle - This is the phandle of the specific reset to be used by the clien
+	driver.
+reset-names - This is the reset name of module that the device driver
+	needs to be able to reset.  This value must correspond to a value within
+	the reset controller array.
+
+example:
+resets = <&prm_resets &dsp_mmu_reset>;
+reset-names = "dsp_mmu_reset";
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 2/6] dt: TI: Describe the ti reset DT entries
@ 2014-07-17 16:45   ` Dan Murphy
  0 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

Describe the TI reset DT entries for TI SoC's.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - Changed Headline no other changes

 .../devicetree/bindings/reset/ti,reset.txt         |  103 ++++++++++++++++++++
 1 file changed, 103 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/ti,reset.txt

diff --git a/Documentation/devicetree/bindings/reset/ti,reset.txt b/Documentation/devicetree/bindings/reset/ti,reset.txt
new file mode 100644
index 0000000..9d5c29c
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/ti,reset.txt
@@ -0,0 +1,103 @@
+Texas Instruments Reset Controller
+======================================
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Specifying the reset entries for the IP module
+==============================================
+Parent module:
+This is the module node that contains the reset registers and bits.
+
+example:
+			prcm_resets: resets {
+				compatible = "ti,dra7-resets";
+				#reset-cells = <1>;
+			};
+
+Required parent properties:
+- compatible : Should be one of,
+		"ti,omap4-prm" for OMAP4 PRM instances
+		"ti,omap5-prm" for OMAP5 PRM instances
+		"ti,dra7-prm" for DRA7xx PRM instances
+		"ti,am4-prcm" for AM43xx PRCM instances
+		"ti,am3-prcm" for AM33xx PRCM instances
+
+Required child reset property:
+- compatible : Should be
+		"resets" for All TI SoCs
+
+example:
+		prm: prm at 4ae06000 {
+			compatible = "ti,omap5-prm";
+			reg = <0x4ae06000 0x3000>;
+
+			prm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
+		};
+
+
+Reset node declaration
+==============================================
+The reset node is declared in a parent child relationship.  The main parent
+is the PRCM module which contains the base address.  The first child within
+the reset parent declares the target modules reset name.  This is followed by
+the control and status offsets.
+
+Within the first reset child node is a secondary child node which declares the
+reset signal of interest.  Under this node the control and status bits
+are declared.  These bits declare the bit mask for the target reset.
+
+
+Required properties:
+reg - This is the register offset from the PRCM parent.
+	This must be declared as:
+
+	reg = <control register offset>,
+		  <status register offset>;
+
+control-bit - This is the bit within the register which controls the reset
+	of the target module.  This is declared as a bit mask for the register.
+status-bit - This is the bit within the register which contains the status of
+	the reset of the target module.
+	This is declared as a bit mask for the register.
+
+example:
+&prm_resets {
+	dsp_rstctrl {
+		reg = <0x1c00>,
+			  <0x1c04>;
+
+		dsp_reset: dsp_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+};
+
+
+
+Client Node Declaration
+==============================================
+This is the consumer of the parent node to declare what resets this
+particular module is interested in.
+
+example:
+	src: src at 55082000 {
+		    resets = <&reset_src phandle>;
+			reset-names = "<reset_name>";
+	};
+
+Required Properties:
+reset_src - This is the parent DT entry for the reset controller
+phandle - This is the phandle of the specific reset to be used by the clien
+	driver.
+reset-names - This is the reset name of module that the device driver
+	needs to be able to reset.  This value must correspond to a value within
+	the reset controller array.
+
+example:
+resets = <&prm_resets &dsp_mmu_reset>;
+reset-names = "dsp_mmu_reset";
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 3/6] ARM: dts: am33xx: Add prcm_resets node
  2014-07-17 16:45 ` Dan Murphy
@ 2014-07-17 16:45   ` Dan Murphy
  -1 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, devicetree; +Cc: tony, Dan Murphy

Add the prcm_resets node to the prcm parent node.

Add the am33xx_resets file to define the
am33xx reset lines that are handled by this reset
framework.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - No changes

 arch/arm/boot/dts/am33xx-resets.dtsi |   42 ++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/am33xx.dtsi        |    7 ++++++
 2 files changed, 49 insertions(+)
 create mode 100644 arch/arm/boot/dts/am33xx-resets.dtsi

diff --git a/arch/arm/boot/dts/am33xx-resets.dtsi b/arch/arm/boot/dts/am33xx-resets.dtsi
new file mode 100644
index 0000000..9260626
--- /dev/null
+++ b/arch/arm/boot/dts/am33xx-resets.dtsi
@@ -0,0 +1,42 @@
+/*
+ * Device Tree Source for AM33XX reset data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+&prcm_resets {
+	gfx_rstctrl {
+		reg = <0x1104>,
+			  <0x1114>;
+
+		gfx_reset: gfx_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	per_rstctrl {
+		reg = <0xD00>,
+			  <0xD0C>;
+
+		iva_reset: iva_reset {
+			control-bit = <0x04>;
+			status-bit = <0x10>;
+		};
+	};
+
+	device_rstctrl {
+		reg = <0xf00>,
+			  <0xf08>;
+
+		device_reset: device_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+};
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 4a4e02d..5cdc8f0 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -117,6 +117,12 @@
 
 			prcm_clockdomains: clockdomains {
 			};
+
+			prcm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
 		};
 
 		scrm: scrm@44e10000 {
@@ -834,3 +840,4 @@
 };
 
 /include/ "am33xx-clocks.dtsi"
+/include/ "am33xx-resets.dtsi"
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 3/6] ARM: dts: am33xx: Add prcm_resets node
@ 2014-07-17 16:45   ` Dan Murphy
  0 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

Add the prcm_resets node to the prcm parent node.

Add the am33xx_resets file to define the
am33xx reset lines that are handled by this reset
framework.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - No changes

 arch/arm/boot/dts/am33xx-resets.dtsi |   42 ++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/am33xx.dtsi        |    7 ++++++
 2 files changed, 49 insertions(+)
 create mode 100644 arch/arm/boot/dts/am33xx-resets.dtsi

diff --git a/arch/arm/boot/dts/am33xx-resets.dtsi b/arch/arm/boot/dts/am33xx-resets.dtsi
new file mode 100644
index 0000000..9260626
--- /dev/null
+++ b/arch/arm/boot/dts/am33xx-resets.dtsi
@@ -0,0 +1,42 @@
+/*
+ * Device Tree Source for AM33XX reset data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+&prcm_resets {
+	gfx_rstctrl {
+		reg = <0x1104>,
+			  <0x1114>;
+
+		gfx_reset: gfx_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	per_rstctrl {
+		reg = <0xD00>,
+			  <0xD0C>;
+
+		iva_reset: iva_reset {
+			control-bit = <0x04>;
+			status-bit = <0x10>;
+		};
+	};
+
+	device_rstctrl {
+		reg = <0xf00>,
+			  <0xf08>;
+
+		device_reset: device_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+};
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 4a4e02d..5cdc8f0 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -117,6 +117,12 @@
 
 			prcm_clockdomains: clockdomains {
 			};
+
+			prcm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
 		};
 
 		scrm: scrm at 44e10000 {
@@ -834,3 +840,4 @@
 };
 
 /include/ "am33xx-clocks.dtsi"
+/include/ "am33xx-resets.dtsi"
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 4/6] ARM: dts: am4372: Add prcm_resets node
  2014-07-17 16:45 ` Dan Murphy
@ 2014-07-17 16:45   ` Dan Murphy
  -1 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, devicetree; +Cc: tony, Dan Murphy

Add the prcm_resets node to the prcm parent node.

Add the am34xx_resets file to define the
am34xx reset lines that are handled by this reset
framework.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - No changes

 arch/arm/boot/dts/am4372.dtsi        |    7 +++++
 arch/arm/boot/dts/am43xx-resets.dtsi |   52 ++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 arch/arm/boot/dts/am43xx-resets.dtsi

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 49fa596..d0aa9c9 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -88,6 +88,12 @@
 
 			prcm_clockdomains: clockdomains {
 			};
+
+			prcm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
 		};
 
 		scrm: scrm@44e10000 {
@@ -892,3 +898,4 @@
 };
 
 /include/ "am43xx-clocks.dtsi"
+/include/ "am43xx-resets.dtsi"
diff --git a/arch/arm/boot/dts/am43xx-resets.dtsi b/arch/arm/boot/dts/am43xx-resets.dtsi
new file mode 100644
index 0000000..ef338ba
--- /dev/null
+++ b/arch/arm/boot/dts/am43xx-resets.dtsi
@@ -0,0 +1,52 @@
+/*
+ * Device Tree Source for AM43XX reset data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+&prcm_resets {
+	icss_rstctrl {
+		reg = <0x810>,
+			  <0x814>;
+
+		icss_reset: icss_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	gfx_rstctrl {
+		reg = <0x410>,
+			  <0x414>;
+
+		gfx_reset: gfx_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	per_rstctrl {
+		reg = <0x2010>,
+			  <0x2014>;
+
+		iva_reset: iva_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	device_rstctrl {
+		reg = <0x4000>,
+			  <0x4004>;
+
+		device_reset: device_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+};
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 4/6] ARM: dts: am4372: Add prcm_resets node
@ 2014-07-17 16:45   ` Dan Murphy
  0 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

Add the prcm_resets node to the prcm parent node.

Add the am34xx_resets file to define the
am34xx reset lines that are handled by this reset
framework.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - No changes

 arch/arm/boot/dts/am4372.dtsi        |    7 +++++
 arch/arm/boot/dts/am43xx-resets.dtsi |   52 ++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 arch/arm/boot/dts/am43xx-resets.dtsi

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 49fa596..d0aa9c9 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -88,6 +88,12 @@
 
 			prcm_clockdomains: clockdomains {
 			};
+
+			prcm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
 		};
 
 		scrm: scrm at 44e10000 {
@@ -892,3 +898,4 @@
 };
 
 /include/ "am43xx-clocks.dtsi"
+/include/ "am43xx-resets.dtsi"
diff --git a/arch/arm/boot/dts/am43xx-resets.dtsi b/arch/arm/boot/dts/am43xx-resets.dtsi
new file mode 100644
index 0000000..ef338ba
--- /dev/null
+++ b/arch/arm/boot/dts/am43xx-resets.dtsi
@@ -0,0 +1,52 @@
+/*
+ * Device Tree Source for AM43XX reset data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+&prcm_resets {
+	icss_rstctrl {
+		reg = <0x810>,
+			  <0x814>;
+
+		icss_reset: icss_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	gfx_rstctrl {
+		reg = <0x410>,
+			  <0x414>;
+
+		gfx_reset: gfx_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	per_rstctrl {
+		reg = <0x2010>,
+			  <0x2014>;
+
+		iva_reset: iva_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	device_rstctrl {
+		reg = <0x4000>,
+			  <0x4004>;
+
+		device_reset: device_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 5/6] ARM: dts: dra7: Add prm_resets node
  2014-07-17 16:45 ` Dan Murphy
@ 2014-07-17 16:45     ` Dan Murphy
  -1 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: tony-4v6yS6AI5VpBDgjK7y7TUQ, Dan Murphy

Add the prcm_resets node to the prm parent node.

Add the draxx_resets file to define the
dra7xx reset lines that are handled by this reset
framework.

Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
---

v3 - No changes

 arch/arm/boot/dts/dra7.dtsi          |    7 +++
 arch/arm/boot/dts/dra7xx-resets.dtsi |   82 ++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)
 create mode 100644 arch/arm/boot/dts/dra7xx-resets.dtsi

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 8012763..f3a8819 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -93,6 +93,12 @@
 
 			prm_clockdomains: clockdomains {
 			};
+
+			prm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
 		};
 
 		cm_core_aon: cm_core_aon@4a005000 {
@@ -998,3 +1004,4 @@
 };
 
 /include/ "dra7xx-clocks.dtsi"
+/include/ "dra7xx-resets.dtsi"
diff --git a/arch/arm/boot/dts/dra7xx-resets.dtsi b/arch/arm/boot/dts/dra7xx-resets.dtsi
new file mode 100644
index 0000000..4c4966d
--- /dev/null
+++ b/arch/arm/boot/dts/dra7xx-resets.dtsi
@@ -0,0 +1,82 @@
+/*
+ * Device Tree Source for DRA7XX reset data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+&prm_resets {
+	dsp_rstctrl {
+		reg = <0x410>,
+			  <0x414>;
+
+		dsp_reset: dsp_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		dsp_mmu_reset: dsp_mmu_reset {
+			control-bit = <0x02>;
+			status-bit = <0x02>;
+		};
+	};
+
+	ipu_rstctrl {
+		reg = <0x510>,
+			  <0x514>;
+
+		ipu_cpu0_reset: ipu_cpu0_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		ipu_cpu1_reset: ipu_cpu1_reset {
+			control-bit = <0x02>;
+			status-bit = <0x02>;
+		};
+
+		ipu_mmu_reset: ipu_mmu_reset {
+			control-bit = <0x04>;
+			status-bit = <0x04>;
+		};
+	};
+
+	iva_rstctrl {
+		reg = <0xf10>,
+			  <0xf14>;
+
+		iva_reset: iva_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	pcie_rstctrl {
+		reg = <0x1310>,
+			  <0x1314>;
+
+		pcie1_reset: pcie1_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		pcie2_reset: pcie2_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	device_rstctrl {
+		reg = <0x1D00>,
+			  <0x1D04>;
+
+		device_reset: device_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+};
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 5/6] ARM: dts: dra7: Add prm_resets node
@ 2014-07-17 16:45     ` Dan Murphy
  0 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

Add the prcm_resets node to the prm parent node.

Add the draxx_resets file to define the
dra7xx reset lines that are handled by this reset
framework.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - No changes

 arch/arm/boot/dts/dra7.dtsi          |    7 +++
 arch/arm/boot/dts/dra7xx-resets.dtsi |   82 ++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)
 create mode 100644 arch/arm/boot/dts/dra7xx-resets.dtsi

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 8012763..f3a8819 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -93,6 +93,12 @@
 
 			prm_clockdomains: clockdomains {
 			};
+
+			prm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
 		};
 
 		cm_core_aon: cm_core_aon at 4a005000 {
@@ -998,3 +1004,4 @@
 };
 
 /include/ "dra7xx-clocks.dtsi"
+/include/ "dra7xx-resets.dtsi"
diff --git a/arch/arm/boot/dts/dra7xx-resets.dtsi b/arch/arm/boot/dts/dra7xx-resets.dtsi
new file mode 100644
index 0000000..4c4966d
--- /dev/null
+++ b/arch/arm/boot/dts/dra7xx-resets.dtsi
@@ -0,0 +1,82 @@
+/*
+ * Device Tree Source for DRA7XX reset data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+&prm_resets {
+	dsp_rstctrl {
+		reg = <0x410>,
+			  <0x414>;
+
+		dsp_reset: dsp_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		dsp_mmu_reset: dsp_mmu_reset {
+			control-bit = <0x02>;
+			status-bit = <0x02>;
+		};
+	};
+
+	ipu_rstctrl {
+		reg = <0x510>,
+			  <0x514>;
+
+		ipu_cpu0_reset: ipu_cpu0_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		ipu_cpu1_reset: ipu_cpu1_reset {
+			control-bit = <0x02>;
+			status-bit = <0x02>;
+		};
+
+		ipu_mmu_reset: ipu_mmu_reset {
+			control-bit = <0x04>;
+			status-bit = <0x04>;
+		};
+	};
+
+	iva_rstctrl {
+		reg = <0xf10>,
+			  <0xf14>;
+
+		iva_reset: iva_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	pcie_rstctrl {
+		reg = <0x1310>,
+			  <0x1314>;
+
+		pcie1_reset: pcie1_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		pcie2_reset: pcie2_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	device_rstctrl {
+		reg = <0x1D00>,
+			  <0x1D04>;
+
+		device_reset: device_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 6/6] ARM: dts: omap5: Add prm_resets node
  2014-07-17 16:45 ` Dan Murphy
@ 2014-07-17 16:45   ` Dan Murphy
  -1 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, devicetree; +Cc: tony, Dan Murphy

Add the prm_resets node to the prm parent node.

Add the omap54xx_resets file to define the
omap5 reset lines that are handled by this reset
framework.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - No changes

 arch/arm/boot/dts/omap5.dtsi           |    7 ++++
 arch/arm/boot/dts/omap54xx-resets.dtsi |   66 ++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 arch/arm/boot/dts/omap54xx-resets.dtsi

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index a4ed549..97bfef5 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -139,6 +139,12 @@
 
 			prm_clockdomains: clockdomains {
 			};
+
+			prm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
 		};
 
 		cm_core_aon: cm_core_aon@4a004000 {
@@ -989,3 +995,4 @@
 };
 
 /include/ "omap54xx-clocks.dtsi"
+/include/ "omap54xx-resets.dtsi"
diff --git a/arch/arm/boot/dts/omap54xx-resets.dtsi b/arch/arm/boot/dts/omap54xx-resets.dtsi
new file mode 100644
index 0000000..cba6f52
--- /dev/null
+++ b/arch/arm/boot/dts/omap54xx-resets.dtsi
@@ -0,0 +1,66 @@
+/*
+ * Device Tree Source for OMAP5 reset data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+&prm_resets {
+	dsp_rstctrl {
+		reg = <0x1c00>,
+			  <0x1c04>;
+
+		dsp_reset: dsp_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		dsp_mmu_reset: dsp_mmu_reset {
+			control-bit = <0x02>;
+			status-bit = <0x02>;
+		};
+	};
+
+	ipu_rstctrl {
+		reg = <0x910>,
+			  <0x914>;
+
+		ipu_cpu0_reset: ipu_cpu0_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		ipu_cpu1_reset: ipu_cpu1_reset {
+			control-bit = <0x02>;
+			status-bit = <0x02>;
+		};
+
+		ipu_mmu_reset: ipu_mmu_reset {
+			control-bit = <0x04>;
+			status-bit = <0x04>;
+		};
+	};
+
+	iva_rstctrl {
+		reg = <0x1210>,
+			  <0x1214>;
+
+		iva_reset: iva_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	device_rstctrl {
+		reg = <0x1c00>,
+			  <0x1c04>;
+
+		device_reset: device_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+};
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [v3 PATCH 6/6] ARM: dts: omap5: Add prm_resets node
@ 2014-07-17 16:45   ` Dan Murphy
  0 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2014-07-17 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

Add the prm_resets node to the prm parent node.

Add the omap54xx_resets file to define the
omap5 reset lines that are handled by this reset
framework.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3 - No changes

 arch/arm/boot/dts/omap5.dtsi           |    7 ++++
 arch/arm/boot/dts/omap54xx-resets.dtsi |   66 ++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 arch/arm/boot/dts/omap54xx-resets.dtsi

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index a4ed549..97bfef5 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -139,6 +139,12 @@
 
 			prm_clockdomains: clockdomains {
 			};
+
+			prm_resets: resets {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				#reset-cells = <1>;
+			};
 		};
 
 		cm_core_aon: cm_core_aon at 4a004000 {
@@ -989,3 +995,4 @@
 };
 
 /include/ "omap54xx-clocks.dtsi"
+/include/ "omap54xx-resets.dtsi"
diff --git a/arch/arm/boot/dts/omap54xx-resets.dtsi b/arch/arm/boot/dts/omap54xx-resets.dtsi
new file mode 100644
index 0000000..cba6f52
--- /dev/null
+++ b/arch/arm/boot/dts/omap54xx-resets.dtsi
@@ -0,0 +1,66 @@
+/*
+ * Device Tree Source for OMAP5 reset data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+&prm_resets {
+	dsp_rstctrl {
+		reg = <0x1c00>,
+			  <0x1c04>;
+
+		dsp_reset: dsp_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		dsp_mmu_reset: dsp_mmu_reset {
+			control-bit = <0x02>;
+			status-bit = <0x02>;
+		};
+	};
+
+	ipu_rstctrl {
+		reg = <0x910>,
+			  <0x914>;
+
+		ipu_cpu0_reset: ipu_cpu0_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+
+		ipu_cpu1_reset: ipu_cpu1_reset {
+			control-bit = <0x02>;
+			status-bit = <0x02>;
+		};
+
+		ipu_mmu_reset: ipu_mmu_reset {
+			control-bit = <0x04>;
+			status-bit = <0x04>;
+		};
+	};
+
+	iva_rstctrl {
+		reg = <0x1210>,
+			  <0x1214>;
+
+		iva_reset: iva_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+
+	device_rstctrl {
+		reg = <0x1c00>,
+			  <0x1c04>;
+
+		device_reset: device_reset {
+			control-bit = <0x01>;
+			status-bit = <0x01>;
+		};
+	};
+};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [v3 PATCH 1/6] drivers: reset: TI: SoC reset controller support.
  2014-07-17 16:45 ` Dan Murphy
@ 2014-07-18  6:41   ` Lothar Waßmann
  -1 siblings, 0 replies; 28+ messages in thread
From: Lothar Waßmann @ 2014-07-18  6:41 UTC (permalink / raw)
  To: Dan Murphy; +Cc: linux-omap, linux-arm-kernel, devicetree, tony

Hi,

Dan Murphy wrote:
> The TI SoC reset controller support utilizes the
> reset controller framework to give device drivers or
> function drivers a common set of APIs to call to reset
> a module.
> 
> The reset-ti is a common interface to the reset framework.
>  The register data is retrieved during initialization
>  of the reset driver through the reset-ti-data
> file.  The array of data is associated with the compatible from the
> respective DT entry.
> 
> Once the data is available then this is derefenced within the common
> interface.
> 
> The device driver has the ability to assert, deassert or perform a
> complete reset.
> 
> This code was derived from previous work by Rajendra Nayak and Afzal Mohammed.
> The code was changed to adopt to the reset core and abstract away the SoC information.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - Resolved comments from v2.  To many to call out here - https://patchwork.kernel.org/patch/4116941/
> 
>  drivers/reset/Kconfig    |    9 ++
>  drivers/reset/Makefile   |    1 +
>  drivers/reset/reset-ti.c |  373 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 383 insertions(+)
>  create mode 100644 drivers/reset/reset-ti.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 0615f50..31a5a79 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER
>  
>  	  If unsure, say no.
>  
> +config RESET_TI
> +	depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST
> +	bool "TI reset controller"
> +	help
> +	  Reset controller support for TI SoC's
> +
> +	  Reset controller found in TI's AM series of SoC's like
> +	  AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7
> +
>  source "drivers/reset/sti/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 60fed3d..a5986b9 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_RESET_CONTROLLER) += core.o
>  obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TI) += reset-ti.o
>  obj-$(CONFIG_ARCH_STI) += sti/
> diff --git a/drivers/reset/reset-ti.c b/drivers/reset/reset-ti.c
> new file mode 100644
> index 0000000..e9d4039
> --- /dev/null
> +++ b/drivers/reset/reset-ti.c
> @@ -0,0 +1,373 @@
> +/*
> + * reset-ti.c - PRCM reset driver for TI SoC's
> + *
> + * Copyright (C) 2014 Texas Instruments Incorporated -  http://www.ti.com
> + *
> + * Author: Dan Murphy <dmurphy@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define DRIVER_NAME "prcm_reset_ti"
> +#define MAX_RESET_SIGNALS 255
> +
> +/**
> + * struct ti_reset_reg_data - Structure of the reset register information
> + *		for a particular SoC.
> + * @rstctrl_offs: This is the reset control offset value from
> + *		from the parent reset node.
> + * @rstst_offs: This is the reset status offset value from
> + *		from the parent reset node.
> + * @rstctrl_bit: This is the reset control bit for the module.
> + * @rstst_bit: This is the reset status bit for the module.
> + *
> + * Longer description of this structure.
> + */
> +struct ti_reset_reg_data {
> +	phandle handle;
> +	u32	rstctrl_offs;
> +	u32	rstst_offs;
> +	u32	rstctrl_bit;
> +	u32	rstst_bit;
> +};
> +
> +/**
> + * struct ti_reset_data - Structure that contains the reset register data
> + *	as well as the total number of resets for a particular SoC.
> + * @ti_data:	Pointer to this structure to be dereferenced
> + * @reg_data:	Pointer to the register data structure.
> + * @rcdev:		Reset controller device instance
> + * @dev:		Pointer to the devive structure
> + * @ti_reg_data: Array of register data.  Only reset signal with valid
> + *				phandles will be stored in this array.
> + * @reg_base:	Parent register base address
> + * @lock:		Spinlock for accessing the registers
> + * @nr_resets:	Total number of resets for the SoC in the reset array.
> + *
> + * This structure contains a pointer to the register data and the modules
> + * register base.  The number of resets and reset controller device data is
> + * stored within this structure.
> + *
> + */
> +struct ti_reset_data {
> +	struct ti_reset_data *ti_data;
> +	struct ti_reset_reg_data *reg_data;
> +	struct reset_controller_dev rcdev;
> +	struct device *dev;
> +	struct ti_reset_reg_data ti_reg_data[MAX_RESET_SIGNALS];
> +	void __iomem *reg_base;
> +	spinlock_t lock;
> +	int nr_resets;
> +};
> +
> +static int ti_reset_wait_on_reset(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *status_reg = reset_data->reg_base;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
> +	unsigned long flags;
> +	int i = 1000;
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstst_bit;
> +
> +	do {
> +		val = readl(status_reg);
> +	} while (--i && (val & (1 << bit_mask)));
> +
Is there no better way to detect timeout than counting down from some
magic number? Does the reference manual give any hints about how fast
the reset should be completed, so the timeout can be hinged on some
real microseconds rather than a number of CPU loops?

> +	if (i == 0 && val & (1 << bit_mask)) {
> +		dev_err(reset_data->dev, "%s: Reset failed\n", __func__);
> +		ret = -EIO;
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +	return ret;
> +}
> +
> +static int ti_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *reg;
> +	void __iomem *status_reg;
> +	u32 status_bit = 0;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
>
Useless initialization!

> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	status_bit = reset_data->ti_reg_data[id].rstst_bit;
> +	writel(1 << status_bit, status_reg);
> +
> +	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> +	val = readl(reg);
> +	if (!(val & bit_mask)) {
> +		val |= bit_mask;
> +		writel(val, reg);
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +	return 0;
> +}
> +
> +static int ti_reset_deassert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *reg;
> +	void __iomem *status_reg;
> +	u32 status_bit = 0;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
>
dto.

> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	status_bit = reset_data->ti_reg_data[id].rstst_bit;
> +	writel(1 << status_bit, status_reg);
> +
> +	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> +	val = readl(reg);
> +	if (val & bit_mask) {
> +		val &= ~bit_mask;
> +		writel(val, reg);
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int ti_reset_reset(struct reset_controller_dev *rcdev,
> +			  unsigned long id)
> +{
> +	ti_reset_assert(rcdev, id);
> +	ti_reset_deassert(rcdev, id);
> +
> +	return ti_reset_wait_on_reset(rcdev, id);
> +}
> +
> +static int ti_reset_xlate(struct reset_controller_dev *rcdev,
> +			const struct of_phandle_args *reset_spec)
> +{
> +
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	struct device_node *dev_node;
> +	int i;
> +
> +	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> +		return -EINVAL;
> +
> +	/* Verify that the phandle exists */
> +	dev_node = of_find_node_by_phandle((phandle) reset_spec->args[0]);
> +	if (!dev_node) {
> +		dev_err(reset_data->dev,
> +				"%s: Cannot find phandle node\n",
> +				__func__);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i <= reset_data->nr_resets; i++) {
> +		if (reset_data->ti_reg_data[i].handle == dev_node->phandle)
> +			return i;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static struct reset_control_ops ti_reset_ops = {
> +	.reset = ti_reset_reset,
> +	.assert = ti_reset_assert,
> +	.deassert = ti_reset_deassert,
> +};
> +
> +static int ti_reset_populate_child(struct ti_reset_data *reset_data,
> +					struct device_node *reset_child,
> +					u32 ctrl_offs, u32 status_offs)
> +{
> +	struct device_node *next_reset_child = NULL;
> +	int i = reset_data->nr_resets;
> +	int ret;
> +	u32 ctrl_bit;
> +
> +	while ((next_reset_child = of_get_next_child(reset_child, next_reset_child))) {
> +		if (next_reset_child->phandle) {
> +			/* Get the bits of each sub-reset */
> +			ret = of_property_read_u32(next_reset_child, "control-bit",
> +						&ctrl_bit);
> +				if (ret < 0) {
> +					dev_err(reset_data->dev,
> +						"%s: No entry in %s for rstctrl_offs\n",
> +						__func__, next_reset_child->name);
> +
> +				return -ENODEV;
>
return the error code from of_property_read()?

> +			}
> +
> +			ret = of_property_read_u32(next_reset_child, "status-bit",
> +						&reset_data->ti_reg_data[i].rstst_bit);
> +				if (ret < 0) {
> +					dev_err(reset_data->dev,
> +							"%s: No entry in %s for rstst_offs\n",
> +							__func__, next_reset_child->name);
> +
> +				return -ENODEV;
>
dto.

> +			}
> +
> +			reset_data->ti_reg_data[i].rstctrl_offs = ctrl_offs;
> +			reset_data->ti_reg_data[i].rstst_offs = status_offs;
> +			reset_data->ti_reg_data[i].handle = next_reset_child->phandle;
> +			i++;
> +		}
> +	};
> +
> +	reset_data->nr_resets = i;
> +
> +	return 0;
> +}
> +
> +static int ti_reset_get_of_data(struct ti_reset_data *reset_data,
> +				struct device_node *dev_node)
> +{
> +	struct device_node *reset_child = NULL;
> +	int ret = -EINVAL;
>
useless initialization.

> +	u32 ctrl_offs, status_offs;
> +
> +	/* Loop through all the children and populate a lookup array */
> +	while ((reset_child = of_get_next_child(dev_node, reset_child))) {
>
I'd prefer:
	while ((reset_child = of_get_next_child(dev_node,
reset_child)) != NULL) { to make it abundantly clear that the = is not meant as a comparison
operator but an assignment.

> +		ret = of_property_read_u32_index(reset_child, "reg", 0,
> +						 &ctrl_offs);
> +		if (ret)
> +			return ret;
> +
> +		ret = of_property_read_u32_index(reset_child, "reg", 1,
> +						 &status_offs);
> +		if (ret)
> +			return ret;
> +
> +		ret = ti_reset_populate_child(reset_data, reset_child,
> +							ctrl_offs, status_offs);
> +		if (ret)
> +			return ret;
> +	};
> +
> +	return 0;
> +}
> +
> +static int ti_reset_probe(struct platform_device *pdev)
> +{
> +	struct device_node *resets;
> +	struct ti_reset_data *reset_data;
> +	struct resource *res;
> +	int ret;
> +
> +	reset_data = devm_kzalloc(&pdev->dev, sizeof(*reset_data), GFP_KERNEL);
> +	if (!reset_data)
> +		return -ENOMEM;
> +
> +	resets = of_find_node_by_name(NULL, "resets");
> +	if (!resets) {
> +		dev_err(&pdev->dev, "%s: missing 'resets' child node.\n",
> +			__func__);
> +		return -EINVAL;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reset_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(reset_data->reg_base)) {
> +		dev_err(&pdev->dev, "%s: Cannot map reset parent.\n", __func__);
> +		return -ENODEV;
>
return the error code from devm_ioremap_resource.

> +	}
> +
> +	spin_lock_init(&reset_data->lock);
> +	reset_data->dev = &pdev->dev;
> +	ret = ti_reset_get_of_data(reset_data, resets);
> +	if (ret)
> +		return -EINVAL;
>
return ret!

> +	reset_data->rcdev.owner = THIS_MODULE;
> +	reset_data->rcdev.of_node = resets;
> +	reset_data->rcdev.ops = &ti_reset_ops;
> +
> +	reset_data->rcdev.of_reset_n_cells = 1;
> +	reset_data->rcdev.of_xlate = &ti_reset_xlate;
> +
> +	reset_data->ti_data = reset_data;
> +
> +	platform_set_drvdata(pdev, reset_data);
> +
> +	reset_controller_register(&reset_data->rcdev);
> +
> +	return 0;
> +}
> +
> +static int ti_reset_remove(struct platform_device *pdev)
> +{
> +	struct ti_reset_data *reset_data;
> +
> +	reset_data = platform_get_drvdata(pdev);
>
struct ti_reset_data *reset_data = platform_get_drvdata(pdev);

> +	reset_controller_unregister(&reset_data->rcdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ti_reset_of_match[] = {
> +	{ .compatible = "ti,omap5-prm" },
> +	{ .compatible =	"ti,omap4-prm" },
> +	{ .compatible =	"ti,omap5-prm" },
> +	{ .compatible =	"ti,dra7-prm" },
> +	{ .compatible = "ti,am4-prcm" },
> +	{ .compatible =	"ti,am3-prcm" },
> +	{},
>
useless comma after list terminator.
Without the comma an entry inadvertedly added after the terminator
(e.g. due to a badly resolved merge conflict) will lead to a compile
error while otherwise the new entry will silently be ignored.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [v3 PATCH 1/6] drivers: reset: TI: SoC reset controller support.
@ 2014-07-18  6:41   ` Lothar Waßmann
  0 siblings, 0 replies; 28+ messages in thread
From: Lothar Waßmann @ 2014-07-18  6:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

Dan Murphy wrote:
> The TI SoC reset controller support utilizes the
> reset controller framework to give device drivers or
> function drivers a common set of APIs to call to reset
> a module.
> 
> The reset-ti is a common interface to the reset framework.
>  The register data is retrieved during initialization
>  of the reset driver through the reset-ti-data
> file.  The array of data is associated with the compatible from the
> respective DT entry.
> 
> Once the data is available then this is derefenced within the common
> interface.
> 
> The device driver has the ability to assert, deassert or perform a
> complete reset.
> 
> This code was derived from previous work by Rajendra Nayak and Afzal Mohammed.
> The code was changed to adopt to the reset core and abstract away the SoC information.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - Resolved comments from v2.  To many to call out here - https://patchwork.kernel.org/patch/4116941/
> 
>  drivers/reset/Kconfig    |    9 ++
>  drivers/reset/Makefile   |    1 +
>  drivers/reset/reset-ti.c |  373 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 383 insertions(+)
>  create mode 100644 drivers/reset/reset-ti.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 0615f50..31a5a79 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER
>  
>  	  If unsure, say no.
>  
> +config RESET_TI
> +	depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST
> +	bool "TI reset controller"
> +	help
> +	  Reset controller support for TI SoC's
> +
> +	  Reset controller found in TI's AM series of SoC's like
> +	  AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7
> +
>  source "drivers/reset/sti/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 60fed3d..a5986b9 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_RESET_CONTROLLER) += core.o
>  obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TI) += reset-ti.o
>  obj-$(CONFIG_ARCH_STI) += sti/
> diff --git a/drivers/reset/reset-ti.c b/drivers/reset/reset-ti.c
> new file mode 100644
> index 0000000..e9d4039
> --- /dev/null
> +++ b/drivers/reset/reset-ti.c
> @@ -0,0 +1,373 @@
> +/*
> + * reset-ti.c - PRCM reset driver for TI SoC's
> + *
> + * Copyright (C) 2014 Texas Instruments Incorporated -  http://www.ti.com
> + *
> + * Author: Dan Murphy <dmurphy@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define DRIVER_NAME "prcm_reset_ti"
> +#define MAX_RESET_SIGNALS 255
> +
> +/**
> + * struct ti_reset_reg_data - Structure of the reset register information
> + *		for a particular SoC.
> + * @rstctrl_offs: This is the reset control offset value from
> + *		from the parent reset node.
> + * @rstst_offs: This is the reset status offset value from
> + *		from the parent reset node.
> + * @rstctrl_bit: This is the reset control bit for the module.
> + * @rstst_bit: This is the reset status bit for the module.
> + *
> + * Longer description of this structure.
> + */
> +struct ti_reset_reg_data {
> +	phandle handle;
> +	u32	rstctrl_offs;
> +	u32	rstst_offs;
> +	u32	rstctrl_bit;
> +	u32	rstst_bit;
> +};
> +
> +/**
> + * struct ti_reset_data - Structure that contains the reset register data
> + *	as well as the total number of resets for a particular SoC.
> + * @ti_data:	Pointer to this structure to be dereferenced
> + * @reg_data:	Pointer to the register data structure.
> + * @rcdev:		Reset controller device instance
> + * @dev:		Pointer to the devive structure
> + * @ti_reg_data: Array of register data.  Only reset signal with valid
> + *				phandles will be stored in this array.
> + * @reg_base:	Parent register base address
> + * @lock:		Spinlock for accessing the registers
> + * @nr_resets:	Total number of resets for the SoC in the reset array.
> + *
> + * This structure contains a pointer to the register data and the modules
> + * register base.  The number of resets and reset controller device data is
> + * stored within this structure.
> + *
> + */
> +struct ti_reset_data {
> +	struct ti_reset_data *ti_data;
> +	struct ti_reset_reg_data *reg_data;
> +	struct reset_controller_dev rcdev;
> +	struct device *dev;
> +	struct ti_reset_reg_data ti_reg_data[MAX_RESET_SIGNALS];
> +	void __iomem *reg_base;
> +	spinlock_t lock;
> +	int nr_resets;
> +};
> +
> +static int ti_reset_wait_on_reset(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *status_reg = reset_data->reg_base;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
> +	unsigned long flags;
> +	int i = 1000;
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstst_bit;
> +
> +	do {
> +		val = readl(status_reg);
> +	} while (--i && (val & (1 << bit_mask)));
> +
Is there no better way to detect timeout than counting down from some
magic number? Does the reference manual give any hints about how fast
the reset should be completed, so the timeout can be hinged on some
real microseconds rather than a number of CPU loops?

> +	if (i == 0 && val & (1 << bit_mask)) {
> +		dev_err(reset_data->dev, "%s: Reset failed\n", __func__);
> +		ret = -EIO;
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +	return ret;
> +}
> +
> +static int ti_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *reg;
> +	void __iomem *status_reg;
> +	u32 status_bit = 0;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
>
Useless initialization!

> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	status_bit = reset_data->ti_reg_data[id].rstst_bit;
> +	writel(1 << status_bit, status_reg);
> +
> +	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> +	val = readl(reg);
> +	if (!(val & bit_mask)) {
> +		val |= bit_mask;
> +		writel(val, reg);
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +	return 0;
> +}
> +
> +static int ti_reset_deassert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *reg;
> +	void __iomem *status_reg;
> +	u32 status_bit = 0;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
>
dto.

> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	status_bit = reset_data->ti_reg_data[id].rstst_bit;
> +	writel(1 << status_bit, status_reg);
> +
> +	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> +	val = readl(reg);
> +	if (val & bit_mask) {
> +		val &= ~bit_mask;
> +		writel(val, reg);
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int ti_reset_reset(struct reset_controller_dev *rcdev,
> +			  unsigned long id)
> +{
> +	ti_reset_assert(rcdev, id);
> +	ti_reset_deassert(rcdev, id);
> +
> +	return ti_reset_wait_on_reset(rcdev, id);
> +}
> +
> +static int ti_reset_xlate(struct reset_controller_dev *rcdev,
> +			const struct of_phandle_args *reset_spec)
> +{
> +
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	struct device_node *dev_node;
> +	int i;
> +
> +	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> +		return -EINVAL;
> +
> +	/* Verify that the phandle exists */
> +	dev_node = of_find_node_by_phandle((phandle) reset_spec->args[0]);
> +	if (!dev_node) {
> +		dev_err(reset_data->dev,
> +				"%s: Cannot find phandle node\n",
> +				__func__);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i <= reset_data->nr_resets; i++) {
> +		if (reset_data->ti_reg_data[i].handle == dev_node->phandle)
> +			return i;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static struct reset_control_ops ti_reset_ops = {
> +	.reset = ti_reset_reset,
> +	.assert = ti_reset_assert,
> +	.deassert = ti_reset_deassert,
> +};
> +
> +static int ti_reset_populate_child(struct ti_reset_data *reset_data,
> +					struct device_node *reset_child,
> +					u32 ctrl_offs, u32 status_offs)
> +{
> +	struct device_node *next_reset_child = NULL;
> +	int i = reset_data->nr_resets;
> +	int ret;
> +	u32 ctrl_bit;
> +
> +	while ((next_reset_child = of_get_next_child(reset_child, next_reset_child))) {
> +		if (next_reset_child->phandle) {
> +			/* Get the bits of each sub-reset */
> +			ret = of_property_read_u32(next_reset_child, "control-bit",
> +						&ctrl_bit);
> +				if (ret < 0) {
> +					dev_err(reset_data->dev,
> +						"%s: No entry in %s for rstctrl_offs\n",
> +						__func__, next_reset_child->name);
> +
> +				return -ENODEV;
>
return the error code from of_property_read()?

> +			}
> +
> +			ret = of_property_read_u32(next_reset_child, "status-bit",
> +						&reset_data->ti_reg_data[i].rstst_bit);
> +				if (ret < 0) {
> +					dev_err(reset_data->dev,
> +							"%s: No entry in %s for rstst_offs\n",
> +							__func__, next_reset_child->name);
> +
> +				return -ENODEV;
>
dto.

> +			}
> +
> +			reset_data->ti_reg_data[i].rstctrl_offs = ctrl_offs;
> +			reset_data->ti_reg_data[i].rstst_offs = status_offs;
> +			reset_data->ti_reg_data[i].handle = next_reset_child->phandle;
> +			i++;
> +		}
> +	};
> +
> +	reset_data->nr_resets = i;
> +
> +	return 0;
> +}
> +
> +static int ti_reset_get_of_data(struct ti_reset_data *reset_data,
> +				struct device_node *dev_node)
> +{
> +	struct device_node *reset_child = NULL;
> +	int ret = -EINVAL;
>
useless initialization.

> +	u32 ctrl_offs, status_offs;
> +
> +	/* Loop through all the children and populate a lookup array */
> +	while ((reset_child = of_get_next_child(dev_node, reset_child))) {
>
I'd prefer:
	while ((reset_child = of_get_next_child(dev_node,
reset_child)) != NULL) { to make it abundantly clear that the = is not meant as a comparison
operator but an assignment.

> +		ret = of_property_read_u32_index(reset_child, "reg", 0,
> +						 &ctrl_offs);
> +		if (ret)
> +			return ret;
> +
> +		ret = of_property_read_u32_index(reset_child, "reg", 1,
> +						 &status_offs);
> +		if (ret)
> +			return ret;
> +
> +		ret = ti_reset_populate_child(reset_data, reset_child,
> +							ctrl_offs, status_offs);
> +		if (ret)
> +			return ret;
> +	};
> +
> +	return 0;
> +}
> +
> +static int ti_reset_probe(struct platform_device *pdev)
> +{
> +	struct device_node *resets;
> +	struct ti_reset_data *reset_data;
> +	struct resource *res;
> +	int ret;
> +
> +	reset_data = devm_kzalloc(&pdev->dev, sizeof(*reset_data), GFP_KERNEL);
> +	if (!reset_data)
> +		return -ENOMEM;
> +
> +	resets = of_find_node_by_name(NULL, "resets");
> +	if (!resets) {
> +		dev_err(&pdev->dev, "%s: missing 'resets' child node.\n",
> +			__func__);
> +		return -EINVAL;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reset_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(reset_data->reg_base)) {
> +		dev_err(&pdev->dev, "%s: Cannot map reset parent.\n", __func__);
> +		return -ENODEV;
>
return the error code from devm_ioremap_resource.

> +	}
> +
> +	spin_lock_init(&reset_data->lock);
> +	reset_data->dev = &pdev->dev;
> +	ret = ti_reset_get_of_data(reset_data, resets);
> +	if (ret)
> +		return -EINVAL;
>
return ret!

> +	reset_data->rcdev.owner = THIS_MODULE;
> +	reset_data->rcdev.of_node = resets;
> +	reset_data->rcdev.ops = &ti_reset_ops;
> +
> +	reset_data->rcdev.of_reset_n_cells = 1;
> +	reset_data->rcdev.of_xlate = &ti_reset_xlate;
> +
> +	reset_data->ti_data = reset_data;
> +
> +	platform_set_drvdata(pdev, reset_data);
> +
> +	reset_controller_register(&reset_data->rcdev);
> +
> +	return 0;
> +}
> +
> +static int ti_reset_remove(struct platform_device *pdev)
> +{
> +	struct ti_reset_data *reset_data;
> +
> +	reset_data = platform_get_drvdata(pdev);
>
struct ti_reset_data *reset_data = platform_get_drvdata(pdev);

> +	reset_controller_unregister(&reset_data->rcdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ti_reset_of_match[] = {
> +	{ .compatible = "ti,omap5-prm" },
> +	{ .compatible =	"ti,omap4-prm" },
> +	{ .compatible =	"ti,omap5-prm" },
> +	{ .compatible =	"ti,dra7-prm" },
> +	{ .compatible = "ti,am4-prcm" },
> +	{ .compatible =	"ti,am3-prcm" },
> +	{},
>
useless comma after list terminator.
Without the comma an entry inadvertedly added after the terminator
(e.g. due to a badly resolved merge conflict) will lead to a compile
error while otherwise the new entry will silently be ignored.


Lothar Wa?mann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [v3 PATCH 2/6] dt: TI: Describe the ti reset DT entries
  2014-07-17 16:45   ` Dan Murphy
@ 2014-07-21  7:01     ` Tony Lindgren
  -1 siblings, 0 replies; 28+ messages in thread
From: Tony Lindgren @ 2014-07-21  7:01 UTC (permalink / raw)
  To: Dan Murphy; +Cc: linux-omap, linux-arm-kernel, devicetree

* Dan Murphy <dmurphy@ti.com> [140717 09:47]:
> +
> +example:
> +		prm: prm@4ae06000 {
> +			compatible = "ti,omap5-prm";
> +			reg = <0x4ae06000 0x3000>;
> +
> +			prm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				#reset-cells = <1>;
> +			};
> +		};

Probably the way to go with mapping various PRCM registers to separate
drivers is to provide one or more syscon entries like we already have
for the SCM. You may want to coordinate this with Tero.

Regards,

Tony

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [v3 PATCH 2/6] dt: TI: Describe the ti reset DT entries
@ 2014-07-21  7:01     ` Tony Lindgren
  0 siblings, 0 replies; 28+ messages in thread
From: Tony Lindgren @ 2014-07-21  7:01 UTC (permalink / raw)
  To: linux-arm-kernel

* Dan Murphy <dmurphy@ti.com> [140717 09:47]:
> +
> +example:
> +		prm: prm at 4ae06000 {
> +			compatible = "ti,omap5-prm";
> +			reg = <0x4ae06000 0x3000>;
> +
> +			prm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				#reset-cells = <1>;
> +			};
> +		};

Probably the way to go with mapping various PRCM registers to separate
drivers is to provide one or more syscon entries like we already have
for the SCM. You may want to coordinate this with Tero.

Regards,

Tony

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [v3 PATCH 2/6] dt: TI: Describe the ti reset DT entries
  2014-07-17 16:45   ` Dan Murphy
@ 2014-07-22 18:51     ` Suman Anna
  -1 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 18:51 UTC (permalink / raw)
  To: Murphy, Dan, linux-omap, linux-arm-kernel, devicetree; +Cc: tony

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Describe the TI reset DT entries for TI SoC's.

The document definitely needs some cleanup. Here are comments from a
quick glance. In any case, I think this binding will change as you
address Tony's comments.

> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - Changed Headline no other changes
> 
>  .../devicetree/bindings/reset/ti,reset.txt         |  103 ++++++++++++++++++++
>  1 file changed, 103 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/ti,reset.txt
> 
> diff --git a/Documentation/devicetree/bindings/reset/ti,reset.txt b/Documentation/devicetree/bindings/reset/ti,reset.txt
> new file mode 100644
> index 0000000..9d5c29c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/ti,reset.txt
> @@ -0,0 +1,103 @@
> +Texas Instruments Reset Controller
> +======================================
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Specifying the reset entries for the IP module
> +==============================================
> +Parent module:
> +This is the module node that contains the reset registers and bits.
> +
> +example:
> +			prcm_resets: resets {
> +				compatible = "ti,dra7-resets";
> +				#reset-cells = <1>;
> +			};
> +
> +Required parent properties:
> +- compatible : Should be one of,
> +		"ti,omap4-prm" for OMAP4 PRM instances
> +		"ti,omap5-prm" for OMAP5 PRM instances
> +		"ti,dra7-prm" for DRA7xx PRM instances
> +		"ti,am4-prcm" for AM43xx PRCM instances
> +		"ti,am3-prcm" for AM33xx PRCM instances

I am not sure if this belongs to the reset driver bindings, as the PRM
and CM nodes are defined at the SoC level. This document should only be
describing the reset nodes bindings.

Also, please list all the required node properties first, before you can
give an example.

> +
> +Required child reset property:
> +- compatible : Should be
> +		"resets" for All TI SoCs

There is no "compatible" child property. This should have been the name
of the node, right? Also, please list all the properties required under
this node like #address-cells, #reset-cells etc.

> +
> +example:
> +		prm: prm@4ae06000 {
> +			compatible = "ti,omap5-prm";
> +			reg = <0x4ae06000 0x3000>;
> +
> +			prm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

This is wrong. You haven't corrected this from v2. The reg field is used
to give the offsets for control register and status register, and does
not use any size fields.

> +				#reset-cells = <1>;
> +			};
> +		};
> +
> +
> +Reset node declaration
> +==============================================
> +The reset node is declared in a parent child relationship.  The main parent
> +is the PRCM module which contains the base address.  The first child within
> +the reset parent declares the target modules reset name.  This is followed by
> +the control and status offsets.

This is not clear. This is the case with each child node, which is
describing a particular reset domain, and then the individual resets
themselves are defined as child nodes of this reset domain child node.

> +
> +Within the first reset child node is a secondary child node which declares the
> +reset signal of interest.  Under this node the control and status bits
> +are declared.  These bits declare the bit mask for the target reset.
> +
> +
> +Required properties:
> +reg - This is the register offset from the PRCM parent.
> +	This must be declared as:
> +
> +	reg = <control register offset>,
> +		  <status register offset>;
> +
> +control-bit - This is the bit within the register which controls the reset
> +	of the target module.  This is declared as a bit mask for the register.
> +status-bit - This is the bit within the register which contains the status of
> +	the reset of the target module.
> +	This is declared as a bit mask for the register.
> +
> +example:
> +&prm_resets {
> +	dsp_rstctrl {
> +		reg = <0x1c00>,
> +			  <0x1c04>;

I guess both these entries can be defined in one line.

> +
> +		dsp_reset: dsp_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +};
> +
> +
> +
> +Client Node Declaration
> +==============================================
> +This is the consumer of the parent node to declare what resets this
> +particular module is interested in.
> +
> +example:
> +	src: src@55082000 {
> +		    resets = <&reset_src phandle>;
> +			reset-names = "<reset_name>";
> +	};
> +
> +Required Properties:
> +reset_src - This is the parent DT entry for the reset controller
> +phandle - This is the phandle of the specific reset to be used by the clien
> +	driver.
> +reset-names - This is the reset name of module that the device driver
> +	needs to be able to reset.  This value must correspond to a value within
> +	the reset controller array.

The reset-names is optional as per the reset.txt, so it is not clear if
this is mandatory for this binding.

regards
Suman

> +
> +example:
> +resets = <&prm_resets &dsp_mmu_reset>;
> +reset-names = "dsp_mmu_reset";
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [v3 PATCH 2/6] dt: TI: Describe the ti reset DT entries
@ 2014-07-22 18:51     ` Suman Anna
  0 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 18:51 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Describe the TI reset DT entries for TI SoC's.

The document definitely needs some cleanup. Here are comments from a
quick glance. In any case, I think this binding will change as you
address Tony's comments.

> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - Changed Headline no other changes
> 
>  .../devicetree/bindings/reset/ti,reset.txt         |  103 ++++++++++++++++++++
>  1 file changed, 103 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/ti,reset.txt
> 
> diff --git a/Documentation/devicetree/bindings/reset/ti,reset.txt b/Documentation/devicetree/bindings/reset/ti,reset.txt
> new file mode 100644
> index 0000000..9d5c29c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/ti,reset.txt
> @@ -0,0 +1,103 @@
> +Texas Instruments Reset Controller
> +======================================
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Specifying the reset entries for the IP module
> +==============================================
> +Parent module:
> +This is the module node that contains the reset registers and bits.
> +
> +example:
> +			prcm_resets: resets {
> +				compatible = "ti,dra7-resets";
> +				#reset-cells = <1>;
> +			};
> +
> +Required parent properties:
> +- compatible : Should be one of,
> +		"ti,omap4-prm" for OMAP4 PRM instances
> +		"ti,omap5-prm" for OMAP5 PRM instances
> +		"ti,dra7-prm" for DRA7xx PRM instances
> +		"ti,am4-prcm" for AM43xx PRCM instances
> +		"ti,am3-prcm" for AM33xx PRCM instances

I am not sure if this belongs to the reset driver bindings, as the PRM
and CM nodes are defined at the SoC level. This document should only be
describing the reset nodes bindings.

Also, please list all the required node properties first, before you can
give an example.

> +
> +Required child reset property:
> +- compatible : Should be
> +		"resets" for All TI SoCs

There is no "compatible" child property. This should have been the name
of the node, right? Also, please list all the properties required under
this node like #address-cells, #reset-cells etc.

> +
> +example:
> +		prm: prm at 4ae06000 {
> +			compatible = "ti,omap5-prm";
> +			reg = <0x4ae06000 0x3000>;
> +
> +			prm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

This is wrong. You haven't corrected this from v2. The reg field is used
to give the offsets for control register and status register, and does
not use any size fields.

> +				#reset-cells = <1>;
> +			};
> +		};
> +
> +
> +Reset node declaration
> +==============================================
> +The reset node is declared in a parent child relationship.  The main parent
> +is the PRCM module which contains the base address.  The first child within
> +the reset parent declares the target modules reset name.  This is followed by
> +the control and status offsets.

This is not clear. This is the case with each child node, which is
describing a particular reset domain, and then the individual resets
themselves are defined as child nodes of this reset domain child node.

> +
> +Within the first reset child node is a secondary child node which declares the
> +reset signal of interest.  Under this node the control and status bits
> +are declared.  These bits declare the bit mask for the target reset.
> +
> +
> +Required properties:
> +reg - This is the register offset from the PRCM parent.
> +	This must be declared as:
> +
> +	reg = <control register offset>,
> +		  <status register offset>;
> +
> +control-bit - This is the bit within the register which controls the reset
> +	of the target module.  This is declared as a bit mask for the register.
> +status-bit - This is the bit within the register which contains the status of
> +	the reset of the target module.
> +	This is declared as a bit mask for the register.
> +
> +example:
> +&prm_resets {
> +	dsp_rstctrl {
> +		reg = <0x1c00>,
> +			  <0x1c04>;

I guess both these entries can be defined in one line.

> +
> +		dsp_reset: dsp_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +};
> +
> +
> +
> +Client Node Declaration
> +==============================================
> +This is the consumer of the parent node to declare what resets this
> +particular module is interested in.
> +
> +example:
> +	src: src at 55082000 {
> +		    resets = <&reset_src phandle>;
> +			reset-names = "<reset_name>";
> +	};
> +
> +Required Properties:
> +reset_src - This is the parent DT entry for the reset controller
> +phandle - This is the phandle of the specific reset to be used by the clien
> +	driver.
> +reset-names - This is the reset name of module that the device driver
> +	needs to be able to reset.  This value must correspond to a value within
> +	the reset controller array.

The reset-names is optional as per the reset.txt, so it is not clear if
this is mandatory for this binding.

regards
Suman

> +
> +example:
> +resets = <&prm_resets &dsp_mmu_reset>;
> +reset-names = "dsp_mmu_reset";
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [v3 PATCH 3/6] ARM: dts: am33xx: Add prcm_resets node
  2014-07-17 16:45   ` Dan Murphy
@ 2014-07-22 18:54     ` Suman Anna
  -1 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 18:54 UTC (permalink / raw)
  To: Murphy, Dan, linux-omap, linux-arm-kernel, devicetree; +Cc: tony

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Add the prcm_resets node to the prcm parent node.
> 
> Add the am33xx_resets file to define the
> am33xx reset lines that are handled by this reset
> framework.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - No changes
> 
>  arch/arm/boot/dts/am33xx-resets.dtsi |   42 ++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/am33xx.dtsi        |    7 ++++++
>  2 files changed, 49 insertions(+)
>  create mode 100644 arch/arm/boot/dts/am33xx-resets.dtsi
> 
> diff --git a/arch/arm/boot/dts/am33xx-resets.dtsi b/arch/arm/boot/dts/am33xx-resets.dtsi
> new file mode 100644
> index 0000000..9260626
> --- /dev/null
> +++ b/arch/arm/boot/dts/am33xx-resets.dtsi
> @@ -0,0 +1,42 @@
> +/*
> + * Device Tree Source for AM33XX reset data
> + *
> + * Copyright (C) 2014 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +&prcm_resets {
> +	gfx_rstctrl {
> +		reg = <0x1104>,
> +			  <0x1114>;
> +
> +		gfx_reset: gfx_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +	per_rstctrl {
> +		reg = <0xD00>,
> +			  <0xD0C>;
> +
> +		iva_reset: iva_reset {

There is no IVA on AM33xx, this should be corrected to reflect the PRU_ICSS.

> +			control-bit = <0x04>;
> +			status-bit = <0x10>;
> +		};
> +	};

Not defining the WkupM3 reset control?

> +
> +	device_rstctrl {
> +		reg = <0xf00>,
> +			  <0xf08>;
> +
> +		device_reset: device_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +};
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 4a4e02d..5cdc8f0 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -117,6 +117,12 @@
>  
>  			prcm_clockdomains: clockdomains {
>  			};
> +
> +			prcm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

Should be corrected as per comment on DT bindings.

regards
Suman

> +				#reset-cells = <1>;
> +			};
>  		};
>  
>  		scrm: scrm@44e10000 {
> @@ -834,3 +840,4 @@
>  };
>  
>  /include/ "am33xx-clocks.dtsi"
> +/include/ "am33xx-resets.dtsi"
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [v3 PATCH 3/6] ARM: dts: am33xx: Add prcm_resets node
@ 2014-07-22 18:54     ` Suman Anna
  0 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 18:54 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Add the prcm_resets node to the prcm parent node.
> 
> Add the am33xx_resets file to define the
> am33xx reset lines that are handled by this reset
> framework.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - No changes
> 
>  arch/arm/boot/dts/am33xx-resets.dtsi |   42 ++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/am33xx.dtsi        |    7 ++++++
>  2 files changed, 49 insertions(+)
>  create mode 100644 arch/arm/boot/dts/am33xx-resets.dtsi
> 
> diff --git a/arch/arm/boot/dts/am33xx-resets.dtsi b/arch/arm/boot/dts/am33xx-resets.dtsi
> new file mode 100644
> index 0000000..9260626
> --- /dev/null
> +++ b/arch/arm/boot/dts/am33xx-resets.dtsi
> @@ -0,0 +1,42 @@
> +/*
> + * Device Tree Source for AM33XX reset data
> + *
> + * Copyright (C) 2014 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +&prcm_resets {
> +	gfx_rstctrl {
> +		reg = <0x1104>,
> +			  <0x1114>;
> +
> +		gfx_reset: gfx_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +	per_rstctrl {
> +		reg = <0xD00>,
> +			  <0xD0C>;
> +
> +		iva_reset: iva_reset {

There is no IVA on AM33xx, this should be corrected to reflect the PRU_ICSS.

> +			control-bit = <0x04>;
> +			status-bit = <0x10>;
> +		};
> +	};

Not defining the WkupM3 reset control?

> +
> +	device_rstctrl {
> +		reg = <0xf00>,
> +			  <0xf08>;
> +
> +		device_reset: device_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +};
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 4a4e02d..5cdc8f0 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -117,6 +117,12 @@
>  
>  			prcm_clockdomains: clockdomains {
>  			};
> +
> +			prcm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

Should be corrected as per comment on DT bindings.

regards
Suman

> +				#reset-cells = <1>;
> +			};
>  		};
>  
>  		scrm: scrm at 44e10000 {
> @@ -834,3 +840,4 @@
>  };
>  
>  /include/ "am33xx-clocks.dtsi"
> +/include/ "am33xx-resets.dtsi"
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [v3 PATCH 4/6] ARM: dts: am4372: Add prcm_resets node
  2014-07-17 16:45   ` Dan Murphy
@ 2014-07-22 19:01     ` Suman Anna
  -1 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 19:01 UTC (permalink / raw)
  To: Murphy, Dan, linux-omap, linux-arm-kernel, devicetree; +Cc: tony

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Add the prcm_resets node to the prcm parent node.
> 
> Add the am34xx_resets file to define the
> am34xx reset lines that are handled by this reset
> framework.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - No changes
> 
>  arch/arm/boot/dts/am4372.dtsi        |    7 +++++
>  arch/arm/boot/dts/am43xx-resets.dtsi |   52 ++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
>  create mode 100644 arch/arm/boot/dts/am43xx-resets.dtsi
> 
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index 49fa596..d0aa9c9 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -88,6 +88,12 @@
>  
>  			prcm_clockdomains: clockdomains {
>  			};
> +
> +			prcm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

Should be corrected as per comment on DT bindings.

> +				#reset-cells = <1>;
> +			};
>  		};
>  
>  		scrm: scrm@44e10000 {
> @@ -892,3 +898,4 @@
>  };
>  
>  /include/ "am43xx-clocks.dtsi"
> +/include/ "am43xx-resets.dtsi"
> diff --git a/arch/arm/boot/dts/am43xx-resets.dtsi b/arch/arm/boot/dts/am43xx-resets.dtsi
> new file mode 100644
> index 0000000..ef338ba
> --- /dev/null
> +++ b/arch/arm/boot/dts/am43xx-resets.dtsi
> @@ -0,0 +1,52 @@
> +/*
> + * Device Tree Source for AM43XX reset data
> + *
> + * Copyright (C) 2014 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +&prcm_resets {
> +	icss_rstctrl {
> +		reg = <0x810>,
> +			  <0x814>;
> +
> +		icss_reset: icss_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +	gfx_rstctrl {
> +		reg = <0x410>,
> +			  <0x414>;
> +
> +		gfx_reset: gfx_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +	per_rstctrl {
> +		reg = <0x2010>,
> +			  <0x2014>;
> +
> +		iva_reset: iva_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};

There's no IVA on AM4372. Looking at the offset, it looks like you were
defining this for the WkupM3, in which case you got the initial node
name wrong. The PER rstctrl has the reset management for PRU-ICSS, so
you also need to correct the icss_rstctrl accordingly.

regards
Suman

> +	};
> +
> +	device_rstctrl {
> +		reg = <0x4000>,
> +			  <0x4004>;
> +
> +		device_reset: device_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +};
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [v3 PATCH 4/6] ARM: dts: am4372: Add prcm_resets node
@ 2014-07-22 19:01     ` Suman Anna
  0 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 19:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Add the prcm_resets node to the prcm parent node.
> 
> Add the am34xx_resets file to define the
> am34xx reset lines that are handled by this reset
> framework.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - No changes
> 
>  arch/arm/boot/dts/am4372.dtsi        |    7 +++++
>  arch/arm/boot/dts/am43xx-resets.dtsi |   52 ++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
>  create mode 100644 arch/arm/boot/dts/am43xx-resets.dtsi
> 
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index 49fa596..d0aa9c9 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -88,6 +88,12 @@
>  
>  			prcm_clockdomains: clockdomains {
>  			};
> +
> +			prcm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

Should be corrected as per comment on DT bindings.

> +				#reset-cells = <1>;
> +			};
>  		};
>  
>  		scrm: scrm at 44e10000 {
> @@ -892,3 +898,4 @@
>  };
>  
>  /include/ "am43xx-clocks.dtsi"
> +/include/ "am43xx-resets.dtsi"
> diff --git a/arch/arm/boot/dts/am43xx-resets.dtsi b/arch/arm/boot/dts/am43xx-resets.dtsi
> new file mode 100644
> index 0000000..ef338ba
> --- /dev/null
> +++ b/arch/arm/boot/dts/am43xx-resets.dtsi
> @@ -0,0 +1,52 @@
> +/*
> + * Device Tree Source for AM43XX reset data
> + *
> + * Copyright (C) 2014 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +&prcm_resets {
> +	icss_rstctrl {
> +		reg = <0x810>,
> +			  <0x814>;
> +
> +		icss_reset: icss_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +	gfx_rstctrl {
> +		reg = <0x410>,
> +			  <0x414>;
> +
> +		gfx_reset: gfx_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +	per_rstctrl {
> +		reg = <0x2010>,
> +			  <0x2014>;
> +
> +		iva_reset: iva_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};

There's no IVA on AM4372. Looking at the offset, it looks like you were
defining this for the WkupM3, in which case you got the initial node
name wrong. The PER rstctrl has the reset management for PRU-ICSS, so
you also need to correct the icss_rstctrl accordingly.

regards
Suman

> +	};
> +
> +	device_rstctrl {
> +		reg = <0x4000>,
> +			  <0x4004>;
> +
> +		device_reset: device_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +};
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [v3 PATCH 5/6] ARM: dts: dra7: Add prm_resets node
  2014-07-17 16:45     ` Dan Murphy
@ 2014-07-22 19:07       ` Suman Anna
  -1 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 19:07 UTC (permalink / raw)
  To: Murphy, Dan, linux-omap, linux-arm-kernel, devicetree; +Cc: tony

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Add the prcm_resets node to the prm parent node.
> 
> Add the draxx_resets file to define the
> dra7xx reset lines that are handled by this reset
> framework.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - No changes
> 
>  arch/arm/boot/dts/dra7.dtsi          |    7 +++
>  arch/arm/boot/dts/dra7xx-resets.dtsi |   82 ++++++++++++++++++++++++++++++++++
>  2 files changed, 89 insertions(+)
>  create mode 100644 arch/arm/boot/dts/dra7xx-resets.dtsi
> 
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index 8012763..f3a8819 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -93,6 +93,12 @@
>  
>  			prm_clockdomains: clockdomains {
>  			};
> +
> +			prm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

Should be corrected as per comment on DT bindings.

> +				#reset-cells = <1>;
> +			};
>  		};
>  
>  		cm_core_aon: cm_core_aon@4a005000 {
> @@ -998,3 +1004,4 @@
>  };
>  
>  /include/ "dra7xx-clocks.dtsi"
> +/include/ "dra7xx-resets.dtsi"
> diff --git a/arch/arm/boot/dts/dra7xx-resets.dtsi b/arch/arm/boot/dts/dra7xx-resets.dtsi
> new file mode 100644
> index 0000000..4c4966d
> --- /dev/null
> +++ b/arch/arm/boot/dts/dra7xx-resets.dtsi
> @@ -0,0 +1,82 @@
> +/*
> + * Device Tree Source for DRA7XX reset data
> + *
> + * Copyright (C) 2014 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +&prm_resets {
> +	dsp_rstctrl {
> +		reg = <0x410>,
> +			  <0x414>;
> +
> +		dsp_reset: dsp_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		dsp_mmu_reset: dsp_mmu_reset {
> +			control-bit = <0x02>;
> +			status-bit = <0x02>;
> +		};
> +	};
> +
> +	ipu_rstctrl {
> +		reg = <0x510>,
> +			  <0x514>;
> +
> +		ipu_cpu0_reset: ipu_cpu0_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		ipu_cpu1_reset: ipu_cpu1_reset {
> +			control-bit = <0x02>;
> +			status-bit = <0x02>;
> +		};
> +
> +		ipu_mmu_reset: ipu_mmu_reset {
> +			control-bit = <0x04>;
> +			status-bit = <0x04>;
> +		};
> +	};

There are two IPUs on DRA7. Which one is this node for? And please add
the other reset node for the other IPU as well.

> +
> +	iva_rstctrl {
> +		reg = <0xf10>,
> +			  <0xf14>;
> +
> +		iva_reset: iva_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};

IVA also has three resets, one for logic and two for the sequencers. You
are describing only one of them.

> +	};
> +
> +	pcie_rstctrl {
> +		reg = <0x1310>,
> +			  <0x1314>;
> +
> +		pcie1_reset: pcie1_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		pcie2_reset: pcie2_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};

Copy-paste error? Both pcie resets cannot have the same control and
status bits.

regards
Suman

> +	};
> +
> +	device_rstctrl {
> +		reg = <0x1D00>,
> +			  <0x1D04>;
> +
> +		device_reset: device_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +};
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [v3 PATCH 5/6] ARM: dts: dra7: Add prm_resets node
@ 2014-07-22 19:07       ` Suman Anna
  0 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 19:07 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Add the prcm_resets node to the prm parent node.
> 
> Add the draxx_resets file to define the
> dra7xx reset lines that are handled by this reset
> framework.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - No changes
> 
>  arch/arm/boot/dts/dra7.dtsi          |    7 +++
>  arch/arm/boot/dts/dra7xx-resets.dtsi |   82 ++++++++++++++++++++++++++++++++++
>  2 files changed, 89 insertions(+)
>  create mode 100644 arch/arm/boot/dts/dra7xx-resets.dtsi
> 
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index 8012763..f3a8819 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -93,6 +93,12 @@
>  
>  			prm_clockdomains: clockdomains {
>  			};
> +
> +			prm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

Should be corrected as per comment on DT bindings.

> +				#reset-cells = <1>;
> +			};
>  		};
>  
>  		cm_core_aon: cm_core_aon at 4a005000 {
> @@ -998,3 +1004,4 @@
>  };
>  
>  /include/ "dra7xx-clocks.dtsi"
> +/include/ "dra7xx-resets.dtsi"
> diff --git a/arch/arm/boot/dts/dra7xx-resets.dtsi b/arch/arm/boot/dts/dra7xx-resets.dtsi
> new file mode 100644
> index 0000000..4c4966d
> --- /dev/null
> +++ b/arch/arm/boot/dts/dra7xx-resets.dtsi
> @@ -0,0 +1,82 @@
> +/*
> + * Device Tree Source for DRA7XX reset data
> + *
> + * Copyright (C) 2014 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +&prm_resets {
> +	dsp_rstctrl {
> +		reg = <0x410>,
> +			  <0x414>;
> +
> +		dsp_reset: dsp_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		dsp_mmu_reset: dsp_mmu_reset {
> +			control-bit = <0x02>;
> +			status-bit = <0x02>;
> +		};
> +	};
> +
> +	ipu_rstctrl {
> +		reg = <0x510>,
> +			  <0x514>;
> +
> +		ipu_cpu0_reset: ipu_cpu0_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		ipu_cpu1_reset: ipu_cpu1_reset {
> +			control-bit = <0x02>;
> +			status-bit = <0x02>;
> +		};
> +
> +		ipu_mmu_reset: ipu_mmu_reset {
> +			control-bit = <0x04>;
> +			status-bit = <0x04>;
> +		};
> +	};

There are two IPUs on DRA7. Which one is this node for? And please add
the other reset node for the other IPU as well.

> +
> +	iva_rstctrl {
> +		reg = <0xf10>,
> +			  <0xf14>;
> +
> +		iva_reset: iva_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};

IVA also has three resets, one for logic and two for the sequencers. You
are describing only one of them.

> +	};
> +
> +	pcie_rstctrl {
> +		reg = <0x1310>,
> +			  <0x1314>;
> +
> +		pcie1_reset: pcie1_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		pcie2_reset: pcie2_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};

Copy-paste error? Both pcie resets cannot have the same control and
status bits.

regards
Suman

> +	};
> +
> +	device_rstctrl {
> +		reg = <0x1D00>,
> +			  <0x1D04>;
> +
> +		device_reset: device_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +
> +};
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [v3 PATCH 6/6] ARM: dts: omap5: Add prm_resets node
  2014-07-17 16:45   ` Dan Murphy
@ 2014-07-22 19:09     ` Suman Anna
  -1 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 19:09 UTC (permalink / raw)
  To: Murphy, Dan, linux-omap, linux-arm-kernel, devicetree; +Cc: tony

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Add the prm_resets node to the prm parent node.
> 
> Add the omap54xx_resets file to define the
> omap5 reset lines that are handled by this reset
> framework.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - No changes
> 
>  arch/arm/boot/dts/omap5.dtsi           |    7 ++++
>  arch/arm/boot/dts/omap54xx-resets.dtsi |   66 ++++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
>  create mode 100644 arch/arm/boot/dts/omap54xx-resets.dtsi
> 
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index a4ed549..97bfef5 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -139,6 +139,12 @@
>  
>  			prm_clockdomains: clockdomains {
>  			};
> +
> +			prm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

Should be corrected as per comment on DT bindings.

> +				#reset-cells = <1>;
> +			};
>  		};
>  
>  		cm_core_aon: cm_core_aon@4a004000 {
> @@ -989,3 +995,4 @@
>  };
>  
>  /include/ "omap54xx-clocks.dtsi"
> +/include/ "omap54xx-resets.dtsi"
> diff --git a/arch/arm/boot/dts/omap54xx-resets.dtsi b/arch/arm/boot/dts/omap54xx-resets.dtsi
> new file mode 100644
> index 0000000..cba6f52
> --- /dev/null
> +++ b/arch/arm/boot/dts/omap54xx-resets.dtsi
> @@ -0,0 +1,66 @@
> +/*
> + * Device Tree Source for OMAP5 reset data
> + *
> + * Copyright (C) 2014 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +&prm_resets {
> +	dsp_rstctrl {
> +		reg = <0x1c00>,
> +			  <0x1c04>;
> +
> +		dsp_reset: dsp_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		dsp_mmu_reset: dsp_mmu_reset {
> +			control-bit = <0x02>;
> +			status-bit = <0x02>;
> +		};
> +	};
> +
> +	ipu_rstctrl {
> +		reg = <0x910>,
> +			  <0x914>;
> +
> +		ipu_cpu0_reset: ipu_cpu0_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		ipu_cpu1_reset: ipu_cpu1_reset {
> +			control-bit = <0x02>;
> +			status-bit = <0x02>;
> +		};
> +
> +		ipu_mmu_reset: ipu_mmu_reset {
> +			control-bit = <0x04>;
> +			status-bit = <0x04>;
> +		};
> +	};

Missing reset node for SGX/GFX?

> +
> +	iva_rstctrl {
> +		reg = <0x1210>,
> +			  <0x1214>;
> +
> +		iva_reset: iva_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};

IVA also has three resets, one for logic and two for the sequencers. You
are describing only one of them.

regards
Suman

> +	};
> +
> +	device_rstctrl {
> +		reg = <0x1c00>,
> +			  <0x1c04>;
> +
> +		device_reset: device_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +};
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [v3 PATCH 6/6] ARM: dts: omap5: Add prm_resets node
@ 2014-07-22 19:09     ` Suman Anna
  0 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 19:09 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> Add the prm_resets node to the prm parent node.
> 
> Add the omap54xx_resets file to define the
> omap5 reset lines that are handled by this reset
> framework.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - No changes
> 
>  arch/arm/boot/dts/omap5.dtsi           |    7 ++++
>  arch/arm/boot/dts/omap54xx-resets.dtsi |   66 ++++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
>  create mode 100644 arch/arm/boot/dts/omap54xx-resets.dtsi
> 
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index a4ed549..97bfef5 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -139,6 +139,12 @@
>  
>  			prm_clockdomains: clockdomains {
>  			};
> +
> +			prm_resets: resets {
> +				#address-cells = <1>;
> +				#size-cells = <1>;

Should be corrected as per comment on DT bindings.

> +				#reset-cells = <1>;
> +			};
>  		};
>  
>  		cm_core_aon: cm_core_aon at 4a004000 {
> @@ -989,3 +995,4 @@
>  };
>  
>  /include/ "omap54xx-clocks.dtsi"
> +/include/ "omap54xx-resets.dtsi"
> diff --git a/arch/arm/boot/dts/omap54xx-resets.dtsi b/arch/arm/boot/dts/omap54xx-resets.dtsi
> new file mode 100644
> index 0000000..cba6f52
> --- /dev/null
> +++ b/arch/arm/boot/dts/omap54xx-resets.dtsi
> @@ -0,0 +1,66 @@
> +/*
> + * Device Tree Source for OMAP5 reset data
> + *
> + * Copyright (C) 2014 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +&prm_resets {
> +	dsp_rstctrl {
> +		reg = <0x1c00>,
> +			  <0x1c04>;
> +
> +		dsp_reset: dsp_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		dsp_mmu_reset: dsp_mmu_reset {
> +			control-bit = <0x02>;
> +			status-bit = <0x02>;
> +		};
> +	};
> +
> +	ipu_rstctrl {
> +		reg = <0x910>,
> +			  <0x914>;
> +
> +		ipu_cpu0_reset: ipu_cpu0_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +
> +		ipu_cpu1_reset: ipu_cpu1_reset {
> +			control-bit = <0x02>;
> +			status-bit = <0x02>;
> +		};
> +
> +		ipu_mmu_reset: ipu_mmu_reset {
> +			control-bit = <0x04>;
> +			status-bit = <0x04>;
> +		};
> +	};

Missing reset node for SGX/GFX?

> +
> +	iva_rstctrl {
> +		reg = <0x1210>,
> +			  <0x1214>;
> +
> +		iva_reset: iva_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};

IVA also has three resets, one for logic and two for the sequencers. You
are describing only one of them.

regards
Suman

> +	};
> +
> +	device_rstctrl {
> +		reg = <0x1c00>,
> +			  <0x1c04>;
> +
> +		device_reset: device_reset {
> +			control-bit = <0x01>;
> +			status-bit = <0x01>;
> +		};
> +	};
> +};
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [v3 PATCH 1/6] drivers: reset: TI: SoC reset controller support.
  2014-07-17 16:45 ` Dan Murphy
@ 2014-07-22 20:16   ` Suman Anna
  -1 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 20:16 UTC (permalink / raw)
  To: Murphy, Dan, linux-omap, linux-arm-kernel, devicetree; +Cc: tony

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> The TI SoC reset controller support utilizes the
> reset controller framework to give device drivers or
> function drivers a common set of APIs to call to reset
> a module.
> 
> The reset-ti is a common interface to the reset framework.
>  The register data is retrieved during initialization
>  of the reset driver through the reset-ti-data
> file. The array of data is associated with the compatible from the
> respective DT entry.

Outdated commit description, this is no longer correct.

> 
> Once the data is available then this is derefenced within the common
> interface.
> 
> The device driver has the ability to assert, deassert or perform a
> complete reset.
> 
> This code was derived from previous work by Rajendra Nayak and Afzal Mohammed.
> The code was changed to adopt to the reset core and abstract away the SoC information.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - Resolved comments from v2.  To many to call out here - https://patchwork.kernel.org/patch/4116941/

Please add a cover letter for the series next time and make sure you cc
the reset driver maintainer.

> 
>  drivers/reset/Kconfig    |    9 ++
>  drivers/reset/Makefile   |    1 +
>  drivers/reset/reset-ti.c |  373 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 383 insertions(+)
>  create mode 100644 drivers/reset/reset-ti.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 0615f50..31a5a79 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER
>  
>  	  If unsure, say no.
>  
> +config RESET_TI
> +	depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST
> +	bool "TI reset controller"
> +	help
> +	  Reset controller support for TI SoC's
> +
> +	  Reset controller found in TI's AM series of SoC's like
> +	  AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7
> +
>  source "drivers/reset/sti/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 60fed3d..a5986b9 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_RESET_CONTROLLER) += core.o
>  obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TI) += reset-ti.o
>  obj-$(CONFIG_ARCH_STI) += sti/
> diff --git a/drivers/reset/reset-ti.c b/drivers/reset/reset-ti.c
> new file mode 100644
> index 0000000..e9d4039
> --- /dev/null
> +++ b/drivers/reset/reset-ti.c
> @@ -0,0 +1,373 @@
> +/*
> + * reset-ti.c - PRCM reset driver for TI SoC's
> + *
> + * Copyright (C) 2014 Texas Instruments Incorporated -  http://www.ti.com
> + *
> + * Author: Dan Murphy <dmurphy@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>

This header is no longer needed, now that you are not using of_iomap.

> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define DRIVER_NAME "prcm_reset_ti"
> +#define MAX_RESET_SIGNALS 255

This sounds like a lot, I think you should reduce this to not waste
memory. Start with a small number, and add a trace for increasing this
if you run out of the current slots.

> +
> +/**
> + * struct ti_reset_reg_data - Structure of the reset register information
> + *		for a particular SoC.
> + * @rstctrl_offs: This is the reset control offset value from
> + *		from the parent reset node.
> + * @rstst_offs: This is the reset status offset value from
> + *		from the parent reset node.
> + * @rstctrl_bit: This is the reset control bit for the module.
> + * @rstst_bit: This is the reset status bit for the module.
> + *
> + * Longer description of this structure.
> + */
> +struct ti_reset_reg_data {
> +	phandle handle;
> +	u32	rstctrl_offs;
> +	u32	rstst_offs;
> +	u32	rstctrl_bit;
> +	u32	rstst_bit;
> +};
> +
> +/**
> + * struct ti_reset_data - Structure that contains the reset register data
> + *	as well as the total number of resets for a particular SoC.
> + * @ti_data:	Pointer to this structure to be dereferenced
> + * @reg_data:	Pointer to the register data structure.
> + * @rcdev:		Reset controller device instance
> + * @dev:		Pointer to the devive structure
> + * @ti_reg_data: Array of register data.  Only reset signal with valid
> + *				phandles will be stored in this array.
> + * @reg_base:	Parent register base address
> + * @lock:		Spinlock for accessing the registers
> + * @nr_resets:	Total number of resets for the SoC in the reset array.
> + *
> + * This structure contains a pointer to the register data and the modules
> + * register base.  The number of resets and reset controller device data is
> + * stored within this structure.
> + *
> + */
> +struct ti_reset_data {
> +	struct ti_reset_data *ti_data;

I initially thought this was for list management to deal with multiple
reset nodes, but why do we need this self-referencing pointer?

> +	struct ti_reset_reg_data *reg_data;

What do you need this for? I haven't seen it getting used anywhere, and
ti_reg_data looks very similar.

> +	struct reset_controller_dev rcdev;
> +	struct device *dev;
> +	struct ti_reset_reg_data ti_reg_data[MAX_RESET_SIGNALS];
> +	void __iomem *reg_base;
> +	spinlock_t lock;
> +	int nr_resets;
> +};
> +
> +static int ti_reset_wait_on_reset(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *status_reg = reset_data->reg_base;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
> +	unsigned long flags;
> +	int i = 1000;
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstst_bit;
> +
> +	do {
> +		val = readl(status_reg);
> +	} while (--i && (val & (1 << bit_mask)));

Others have already pointed this out, I believe you want something
equivalent to omap_test_timeout here. Please check with Tero if that
would be available, otherwise, you can define the same in this file.

> +
> +	if (i == 0 && val & (1 << bit_mask)) {
> +		dev_err(reset_data->dev, "%s: Reset failed\n", __func__);
> +		ret = -EIO;
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +	return ret;
> +}
> +
> +static int ti_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *reg;
> +	void __iomem *status_reg;
> +	u32 status_bit = 0;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	status_bit = reset_data->ti_reg_data[id].rstst_bit;
> +	writel(1 << status_bit, status_reg);
> +
> +	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> +	val = readl(reg);
> +	if (!(val & bit_mask)) {
> +		val |= bit_mask;
> +		writel(val, reg);
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +	return 0;
> +}
> +
> +static int ti_reset_deassert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *reg;
> +	void __iomem *status_reg;
> +	u32 status_bit = 0;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	status_bit = reset_data->ti_reg_data[id].rstst_bit;
> +	writel(1 << status_bit, status_reg);

Is there a need for clearing the reset status both in assert and deassert?

> +
> +	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> +	val = readl(reg);
> +	if (val & bit_mask) {
> +		val &= ~bit_mask;
> +		writel(val, reg);
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int ti_reset_reset(struct reset_controller_dev *rcdev,
> +			  unsigned long id)
> +{
> +	ti_reset_assert(rcdev, id);
> +	ti_reset_deassert(rcdev, id);
> +
> +	return ti_reset_wait_on_reset(rcdev, id);
> +}
> +
> +static int ti_reset_xlate(struct reset_controller_dev *rcdev,
> +			const struct of_phandle_args *reset_spec)
> +{
> +
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	struct device_node *dev_node;
> +	int i;
> +
> +	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> +		return -EINVAL;
> +
> +	/* Verify that the phandle exists */
> +	dev_node = of_find_node_by_phandle((phandle) reset_spec->args[0]);
> +	if (!dev_node) {
> +		dev_err(reset_data->dev,
> +				"%s: Cannot find phandle node\n",
> +				__func__);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i <= reset_data->nr_resets; i++) {
> +		if (reset_data->ti_reg_data[i].handle == dev_node->phandle)
> +			return i;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static struct reset_control_ops ti_reset_ops = {
> +	.reset = ti_reset_reset,
> +	.assert = ti_reset_assert,
> +	.deassert = ti_reset_deassert,
> +};
> +
> +static int ti_reset_populate_child(struct ti_reset_data *reset_data,
> +					struct device_node *reset_child,
> +					u32 ctrl_offs, u32 status_offs)
> +{
> +	struct device_node *next_reset_child = NULL;
> +	int i = reset_data->nr_resets;
> +	int ret;
> +	u32 ctrl_bit;
> +
> +	while ((next_reset_child = of_get_next_child(reset_child, next_reset_child))) {

Add a check for array bounds on the ti_reg_data array here, rather than
choosing a large number to begin with.

> +		if (next_reset_child->phandle) {
> +			/* Get the bits of each sub-reset */
> +			ret = of_property_read_u32(next_reset_child, "control-bit",
> +						&ctrl_bit);
> +				if (ret < 0) {
> +					dev_err(reset_data->dev,
> +						"%s: No entry in %s for rstctrl_offs\n",
> +						__func__, next_reset_child->name);
> +
> +				return -ENODEV;
> +			}
> +
> +			ret = of_property_read_u32(next_reset_child, "status-bit",
> +						&reset_data->ti_reg_data[i].rstst_bit);
> +				if (ret < 0) {
> +					dev_err(reset_data->dev,
> +							"%s: No entry in %s for rstst_offs\n",
> +							__func__, next_reset_child->name);
> +
> +				return -ENODEV;
> +			}
> +
> +			reset_data->ti_reg_data[i].rstctrl_offs = ctrl_offs;
> +			reset_data->ti_reg_data[i].rstst_offs = status_offs;
> +			reset_data->ti_reg_data[i].handle = next_reset_child->phandle;
> +			i++;
> +		}
> +	};
> +
> +	reset_data->nr_resets = i;
> +
> +	return 0;
> +}
> +
> +static int ti_reset_get_of_data(struct ti_reset_data *reset_data,
> +				struct device_node *dev_node)
> +{
> +	struct device_node *reset_child = NULL;
> +	int ret = -EINVAL;
> +	u32 ctrl_offs, status_offs;
> +
> +	/* Loop through all the children and populate a lookup array */
> +	while ((reset_child = of_get_next_child(dev_node, reset_child))) {
> +		ret = of_property_read_u32_index(reset_child, "reg", 0,
> +						 &ctrl_offs);
> +		if (ret)
> +			return ret;
> +
> +		ret = of_property_read_u32_index(reset_child, "reg", 1,
> +						 &status_offs);
> +		if (ret)
> +			return ret;
> +
> +		ret = ti_reset_populate_child(reset_data, reset_child,
> +							ctrl_offs, status_offs);
> +		if (ret)
> +			return ret;
> +	};
> +
> +	return 0;
> +}
> +
> +static int ti_reset_probe(struct platform_device *pdev)
> +{
> +	struct device_node *resets;
> +	struct ti_reset_data *reset_data;
> +	struct resource *res;
> +	int ret;
> +
> +	reset_data = devm_kzalloc(&pdev->dev, sizeof(*reset_data), GFP_KERNEL);
> +	if (!reset_data)
> +		return -ENOMEM;
> +
> +	resets = of_find_node_by_name(NULL, "resets");
> +	if (!resets) {
> +		dev_err(&pdev->dev, "%s: missing 'resets' child node.\n",
> +			__func__);
> +		return -EINVAL;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reset_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(reset_data->reg_base)) {
> +		dev_err(&pdev->dev, "%s: Cannot map reset parent.\n", __func__);
> +		return -ENODEV;
> +	}

If the PRM node address is accessed by someone else as well, then this
will always fail.

> +
> +	spin_lock_init(&reset_data->lock);
> +	reset_data->dev = &pdev->dev;
> +	ret = ti_reset_get_of_data(reset_data, resets);
> +	if (ret)
> +		return -EINVAL;
> +
> +	reset_data->rcdev.owner = THIS_MODULE;
> +	reset_data->rcdev.of_node = resets;
> +	reset_data->rcdev.ops = &ti_reset_ops;
> +
> +	reset_data->rcdev.of_reset_n_cells = 1;
> +	reset_data->rcdev.of_xlate = &ti_reset_xlate;
> +
> +	reset_data->ti_data = reset_data;
> +
> +	platform_set_drvdata(pdev, reset_data);
> +
> +	reset_controller_register(&reset_data->rcdev);
> +
> +	return 0;
> +}
> +
> +static int ti_reset_remove(struct platform_device *pdev)
> +{
> +	struct ti_reset_data *reset_data;
> +
> +	reset_data = platform_get_drvdata(pdev);
> +	reset_controller_unregister(&reset_data->rcdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ti_reset_of_match[] = {
> +	{ .compatible = "ti,omap5-prm" },
> +	{ .compatible =	"ti,omap4-prm" },
> +	{ .compatible =	"ti,omap5-prm" },

Duplicate entry

> +	{ .compatible =	"ti,dra7-prm" },
> +	{ .compatible = "ti,am4-prcm" },
> +	{ .compatible =	"ti,am3-prcm" },
> +	{},
> +};

You are using the parent SoC parent nodes as the compatibles, but
this has to restricted to just the resets. I believe this driver is
designed only to deal with the reset nodes under those PRM/PRCM nodes -
we won't be handling all the other nodes under these PRM/PRCM nodes.

Please also see Tony's comment about working with Tero's PRM cleanup.

regards
Suman

> +
> +static struct platform_driver ti_reset_driver = {
> +	.probe	= ti_reset_probe,
> +	.remove	= ti_reset_remove,
> +	.driver	= {
> +		.name		= DRIVER_NAME,
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= of_match_ptr(ti_reset_of_match),
> +	},
> +};
> +module_platform_driver(ti_reset_driver);
> +
> +MODULE_DESCRIPTION("PRCM reset driver for TI SoCs");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [v3 PATCH 1/6] drivers: reset: TI: SoC reset controller support.
@ 2014-07-22 20:16   ` Suman Anna
  0 siblings, 0 replies; 28+ messages in thread
From: Suman Anna @ 2014-07-22 20:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dan,

On 07/17/2014 11:45 AM, Murphy, Dan wrote:
> The TI SoC reset controller support utilizes the
> reset controller framework to give device drivers or
> function drivers a common set of APIs to call to reset
> a module.
> 
> The reset-ti is a common interface to the reset framework.
>  The register data is retrieved during initialization
>  of the reset driver through the reset-ti-data
> file. The array of data is associated with the compatible from the
> respective DT entry.

Outdated commit description, this is no longer correct.

> 
> Once the data is available then this is derefenced within the common
> interface.
> 
> The device driver has the ability to assert, deassert or perform a
> complete reset.
> 
> This code was derived from previous work by Rajendra Nayak and Afzal Mohammed.
> The code was changed to adopt to the reset core and abstract away the SoC information.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3 - Resolved comments from v2.  To many to call out here - https://patchwork.kernel.org/patch/4116941/

Please add a cover letter for the series next time and make sure you cc
the reset driver maintainer.

> 
>  drivers/reset/Kconfig    |    9 ++
>  drivers/reset/Makefile   |    1 +
>  drivers/reset/reset-ti.c |  373 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 383 insertions(+)
>  create mode 100644 drivers/reset/reset-ti.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 0615f50..31a5a79 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER
>  
>  	  If unsure, say no.
>  
> +config RESET_TI
> +	depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST
> +	bool "TI reset controller"
> +	help
> +	  Reset controller support for TI SoC's
> +
> +	  Reset controller found in TI's AM series of SoC's like
> +	  AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7
> +
>  source "drivers/reset/sti/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 60fed3d..a5986b9 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_RESET_CONTROLLER) += core.o
>  obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TI) += reset-ti.o
>  obj-$(CONFIG_ARCH_STI) += sti/
> diff --git a/drivers/reset/reset-ti.c b/drivers/reset/reset-ti.c
> new file mode 100644
> index 0000000..e9d4039
> --- /dev/null
> +++ b/drivers/reset/reset-ti.c
> @@ -0,0 +1,373 @@
> +/*
> + * reset-ti.c - PRCM reset driver for TI SoC's
> + *
> + * Copyright (C) 2014 Texas Instruments Incorporated -  http://www.ti.com
> + *
> + * Author: Dan Murphy <dmurphy@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>

This header is no longer needed, now that you are not using of_iomap.

> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define DRIVER_NAME "prcm_reset_ti"
> +#define MAX_RESET_SIGNALS 255

This sounds like a lot, I think you should reduce this to not waste
memory. Start with a small number, and add a trace for increasing this
if you run out of the current slots.

> +
> +/**
> + * struct ti_reset_reg_data - Structure of the reset register information
> + *		for a particular SoC.
> + * @rstctrl_offs: This is the reset control offset value from
> + *		from the parent reset node.
> + * @rstst_offs: This is the reset status offset value from
> + *		from the parent reset node.
> + * @rstctrl_bit: This is the reset control bit for the module.
> + * @rstst_bit: This is the reset status bit for the module.
> + *
> + * Longer description of this structure.
> + */
> +struct ti_reset_reg_data {
> +	phandle handle;
> +	u32	rstctrl_offs;
> +	u32	rstst_offs;
> +	u32	rstctrl_bit;
> +	u32	rstst_bit;
> +};
> +
> +/**
> + * struct ti_reset_data - Structure that contains the reset register data
> + *	as well as the total number of resets for a particular SoC.
> + * @ti_data:	Pointer to this structure to be dereferenced
> + * @reg_data:	Pointer to the register data structure.
> + * @rcdev:		Reset controller device instance
> + * @dev:		Pointer to the devive structure
> + * @ti_reg_data: Array of register data.  Only reset signal with valid
> + *				phandles will be stored in this array.
> + * @reg_base:	Parent register base address
> + * @lock:		Spinlock for accessing the registers
> + * @nr_resets:	Total number of resets for the SoC in the reset array.
> + *
> + * This structure contains a pointer to the register data and the modules
> + * register base.  The number of resets and reset controller device data is
> + * stored within this structure.
> + *
> + */
> +struct ti_reset_data {
> +	struct ti_reset_data *ti_data;

I initially thought this was for list management to deal with multiple
reset nodes, but why do we need this self-referencing pointer?

> +	struct ti_reset_reg_data *reg_data;

What do you need this for? I haven't seen it getting used anywhere, and
ti_reg_data looks very similar.

> +	struct reset_controller_dev rcdev;
> +	struct device *dev;
> +	struct ti_reset_reg_data ti_reg_data[MAX_RESET_SIGNALS];
> +	void __iomem *reg_base;
> +	spinlock_t lock;
> +	int nr_resets;
> +};
> +
> +static int ti_reset_wait_on_reset(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *status_reg = reset_data->reg_base;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
> +	unsigned long flags;
> +	int i = 1000;
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstst_bit;
> +
> +	do {
> +		val = readl(status_reg);
> +	} while (--i && (val & (1 << bit_mask)));

Others have already pointed this out, I believe you want something
equivalent to omap_test_timeout here. Please check with Tero if that
would be available, otherwise, you can define the same in this file.

> +
> +	if (i == 0 && val & (1 << bit_mask)) {
> +		dev_err(reset_data->dev, "%s: Reset failed\n", __func__);
> +		ret = -EIO;
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +	return ret;
> +}
> +
> +static int ti_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *reg;
> +	void __iomem *status_reg;
> +	u32 status_bit = 0;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	status_bit = reset_data->ti_reg_data[id].rstst_bit;
> +	writel(1 << status_bit, status_reg);
> +
> +	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> +	val = readl(reg);
> +	if (!(val & bit_mask)) {
> +		val |= bit_mask;
> +		writel(val, reg);
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +	return 0;
> +}
> +
> +static int ti_reset_deassert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	void __iomem *reg;
> +	void __iomem *status_reg;
> +	u32 status_bit = 0;
> +	u32 bit_mask = 0;
> +	u32 val = 0;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&reset_data->lock, flags);
> +
> +	/* Clear the reset status bit to reflect the current status */
> +	status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> +	status_bit = reset_data->ti_reg_data[id].rstst_bit;
> +	writel(1 << status_bit, status_reg);

Is there a need for clearing the reset status both in assert and deassert?

> +
> +	reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> +	bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> +	val = readl(reg);
> +	if (val & bit_mask) {
> +		val &= ~bit_mask;
> +		writel(val, reg);
> +	}
> +
> +	spin_unlock_irqrestore(&reset_data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int ti_reset_reset(struct reset_controller_dev *rcdev,
> +			  unsigned long id)
> +{
> +	ti_reset_assert(rcdev, id);
> +	ti_reset_deassert(rcdev, id);
> +
> +	return ti_reset_wait_on_reset(rcdev, id);
> +}
> +
> +static int ti_reset_xlate(struct reset_controller_dev *rcdev,
> +			const struct of_phandle_args *reset_spec)
> +{
> +
> +	struct ti_reset_data *reset_data = container_of(rcdev,
> +						     struct ti_reset_data,
> +						     rcdev);
> +	struct device_node *dev_node;
> +	int i;
> +
> +	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> +		return -EINVAL;
> +
> +	/* Verify that the phandle exists */
> +	dev_node = of_find_node_by_phandle((phandle) reset_spec->args[0]);
> +	if (!dev_node) {
> +		dev_err(reset_data->dev,
> +				"%s: Cannot find phandle node\n",
> +				__func__);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i <= reset_data->nr_resets; i++) {
> +		if (reset_data->ti_reg_data[i].handle == dev_node->phandle)
> +			return i;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static struct reset_control_ops ti_reset_ops = {
> +	.reset = ti_reset_reset,
> +	.assert = ti_reset_assert,
> +	.deassert = ti_reset_deassert,
> +};
> +
> +static int ti_reset_populate_child(struct ti_reset_data *reset_data,
> +					struct device_node *reset_child,
> +					u32 ctrl_offs, u32 status_offs)
> +{
> +	struct device_node *next_reset_child = NULL;
> +	int i = reset_data->nr_resets;
> +	int ret;
> +	u32 ctrl_bit;
> +
> +	while ((next_reset_child = of_get_next_child(reset_child, next_reset_child))) {

Add a check for array bounds on the ti_reg_data array here, rather than
choosing a large number to begin with.

> +		if (next_reset_child->phandle) {
> +			/* Get the bits of each sub-reset */
> +			ret = of_property_read_u32(next_reset_child, "control-bit",
> +						&ctrl_bit);
> +				if (ret < 0) {
> +					dev_err(reset_data->dev,
> +						"%s: No entry in %s for rstctrl_offs\n",
> +						__func__, next_reset_child->name);
> +
> +				return -ENODEV;
> +			}
> +
> +			ret = of_property_read_u32(next_reset_child, "status-bit",
> +						&reset_data->ti_reg_data[i].rstst_bit);
> +				if (ret < 0) {
> +					dev_err(reset_data->dev,
> +							"%s: No entry in %s for rstst_offs\n",
> +							__func__, next_reset_child->name);
> +
> +				return -ENODEV;
> +			}
> +
> +			reset_data->ti_reg_data[i].rstctrl_offs = ctrl_offs;
> +			reset_data->ti_reg_data[i].rstst_offs = status_offs;
> +			reset_data->ti_reg_data[i].handle = next_reset_child->phandle;
> +			i++;
> +		}
> +	};
> +
> +	reset_data->nr_resets = i;
> +
> +	return 0;
> +}
> +
> +static int ti_reset_get_of_data(struct ti_reset_data *reset_data,
> +				struct device_node *dev_node)
> +{
> +	struct device_node *reset_child = NULL;
> +	int ret = -EINVAL;
> +	u32 ctrl_offs, status_offs;
> +
> +	/* Loop through all the children and populate a lookup array */
> +	while ((reset_child = of_get_next_child(dev_node, reset_child))) {
> +		ret = of_property_read_u32_index(reset_child, "reg", 0,
> +						 &ctrl_offs);
> +		if (ret)
> +			return ret;
> +
> +		ret = of_property_read_u32_index(reset_child, "reg", 1,
> +						 &status_offs);
> +		if (ret)
> +			return ret;
> +
> +		ret = ti_reset_populate_child(reset_data, reset_child,
> +							ctrl_offs, status_offs);
> +		if (ret)
> +			return ret;
> +	};
> +
> +	return 0;
> +}
> +
> +static int ti_reset_probe(struct platform_device *pdev)
> +{
> +	struct device_node *resets;
> +	struct ti_reset_data *reset_data;
> +	struct resource *res;
> +	int ret;
> +
> +	reset_data = devm_kzalloc(&pdev->dev, sizeof(*reset_data), GFP_KERNEL);
> +	if (!reset_data)
> +		return -ENOMEM;
> +
> +	resets = of_find_node_by_name(NULL, "resets");
> +	if (!resets) {
> +		dev_err(&pdev->dev, "%s: missing 'resets' child node.\n",
> +			__func__);
> +		return -EINVAL;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reset_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(reset_data->reg_base)) {
> +		dev_err(&pdev->dev, "%s: Cannot map reset parent.\n", __func__);
> +		return -ENODEV;
> +	}

If the PRM node address is accessed by someone else as well, then this
will always fail.

> +
> +	spin_lock_init(&reset_data->lock);
> +	reset_data->dev = &pdev->dev;
> +	ret = ti_reset_get_of_data(reset_data, resets);
> +	if (ret)
> +		return -EINVAL;
> +
> +	reset_data->rcdev.owner = THIS_MODULE;
> +	reset_data->rcdev.of_node = resets;
> +	reset_data->rcdev.ops = &ti_reset_ops;
> +
> +	reset_data->rcdev.of_reset_n_cells = 1;
> +	reset_data->rcdev.of_xlate = &ti_reset_xlate;
> +
> +	reset_data->ti_data = reset_data;
> +
> +	platform_set_drvdata(pdev, reset_data);
> +
> +	reset_controller_register(&reset_data->rcdev);
> +
> +	return 0;
> +}
> +
> +static int ti_reset_remove(struct platform_device *pdev)
> +{
> +	struct ti_reset_data *reset_data;
> +
> +	reset_data = platform_get_drvdata(pdev);
> +	reset_controller_unregister(&reset_data->rcdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ti_reset_of_match[] = {
> +	{ .compatible = "ti,omap5-prm" },
> +	{ .compatible =	"ti,omap4-prm" },
> +	{ .compatible =	"ti,omap5-prm" },

Duplicate entry

> +	{ .compatible =	"ti,dra7-prm" },
> +	{ .compatible = "ti,am4-prcm" },
> +	{ .compatible =	"ti,am3-prcm" },
> +	{},
> +};

You are using the parent SoC parent nodes as the compatibles, but
this has to restricted to just the resets. I believe this driver is
designed only to deal with the reset nodes under those PRM/PRCM nodes -
we won't be handling all the other nodes under these PRM/PRCM nodes.

Please also see Tony's comment about working with Tero's PRM cleanup.

regards
Suman

> +
> +static struct platform_driver ti_reset_driver = {
> +	.probe	= ti_reset_probe,
> +	.remove	= ti_reset_remove,
> +	.driver	= {
> +		.name		= DRIVER_NAME,
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= of_match_ptr(ti_reset_of_match),
> +	},
> +};
> +module_platform_driver(ti_reset_driver);
> +
> +MODULE_DESCRIPTION("PRCM reset driver for TI SoCs");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2014-07-22 20:16 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-17 16:45 [v3 PATCH 1/6] drivers: reset: TI: SoC reset controller support Dan Murphy
2014-07-17 16:45 ` Dan Murphy
2014-07-17 16:45 ` [v3 PATCH 2/6] dt: TI: Describe the ti reset DT entries Dan Murphy
2014-07-17 16:45   ` Dan Murphy
2014-07-21  7:01   ` Tony Lindgren
2014-07-21  7:01     ` Tony Lindgren
2014-07-22 18:51   ` Suman Anna
2014-07-22 18:51     ` Suman Anna
2014-07-17 16:45 ` [v3 PATCH 3/6] ARM: dts: am33xx: Add prcm_resets node Dan Murphy
2014-07-17 16:45   ` Dan Murphy
2014-07-22 18:54   ` Suman Anna
2014-07-22 18:54     ` Suman Anna
2014-07-17 16:45 ` [v3 PATCH 4/6] ARM: dts: am4372: " Dan Murphy
2014-07-17 16:45   ` Dan Murphy
2014-07-22 19:01   ` Suman Anna
2014-07-22 19:01     ` Suman Anna
     [not found] ` <1405615531-15649-1-git-send-email-dmurphy-l0cyMroinI0@public.gmane.org>
2014-07-17 16:45   ` [v3 PATCH 5/6] ARM: dts: dra7: Add prm_resets node Dan Murphy
2014-07-17 16:45     ` Dan Murphy
2014-07-22 19:07     ` Suman Anna
2014-07-22 19:07       ` Suman Anna
2014-07-17 16:45 ` [v3 PATCH 6/6] ARM: dts: omap5: " Dan Murphy
2014-07-17 16:45   ` Dan Murphy
2014-07-22 19:09   ` Suman Anna
2014-07-22 19:09     ` Suman Anna
2014-07-18  6:41 ` [v3 PATCH 1/6] drivers: reset: TI: SoC reset controller support Lothar Waßmann
2014-07-18  6:41   ` Lothar Waßmann
2014-07-22 20:16 ` Suman Anna
2014-07-22 20:16   ` Suman Anna

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.