[PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver

[PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver

[Afzal Mohammed]

Hi,

This is an attempt to achieve reset on AM43x/AM335x based SoC's with
reset driver making use of the reset framework.

prcm node is added in device tree, which would hold reset bindings.
Initially node was made as a one that represents reset functionality
of SoC. but ended up with node for prcm (which is felt to be natural
choice) instead. I am a bit doubtful whether placement of prcm node in
root node as in this series is the right thing.

Reset driver gets probed for specific prcm node, the same defines
register details to be used for a particular SoC (using ".data" field
associated with ".compatible" in driver match table).

Another option to handle different SoC's would be to add register
details to DT and let the driver extract it from DT. I vaguely
remember seeing a thread mentioning that putting register details in
DT is not preferred. But open to putting register level details
instead in DT if that is being generally preferred. This would have
advantage that adding reset support for a new SoC would be easier, but
would have to put more thought before doing so as DT bindings should
not change.

With the approach taken here, for supporting a new SoC with new prcm
register details, driver would have to be updated much like the way a
pci based ethernet driver would have to be updated to handle a new IP
version that is not register level compatible with the existing ones.

In this series out of the three IP's (gfx, m3, pruss) that would need
reset support, here as a proof of concept only gfx is taken care.
Other's can be easily supported by adding new register data array
entries.

Two new reset API's are provided to check whether reset is ready and
to clear reset. This would be required in case IP needs to mix reset
handling procedure with power/clock managment procedure to achieve
proper reset and these procedures are sometimes IP specific that would
make it difficult to handle reset fully in pm_runtime platform support.

*------*
client IP handling s/w (DT based) should do as follows:

1. Specify reset handle in the relevant DT node, for eg.

	myip@deadbeef {
		:
		:
		/* here prcm is the handle to reset binding node */
		resets = <&prcm 0>;
	};

2. In driver, that require reset to be deasserted,
 (this is the sequence required for gfx on AM43x/AM335x, this would
  depend on requirements of the IP)

	mydriver_probe(struct platform device *pdev)
	{
		:
		:
		reset_control_get(&pdev->dev, NULL);
		reset_control_clear_reset();
		reset_control_deassert();
		pm_runtime_get_sync();
		if (reset_control_is_reset() != true)
			goto err;
		reset_control_put();
		:
		:
	}

*------*

May be removing reset handling in hwmod can be considered by making
use of reset driver.

Or as another extreme, perhaps, other logic's in the prcm can be
handled by a new prcm driver and then this reset driver can be a child
of it.

Regards
Afzal


Afzal Mohammed (6):
  reset: is_reset and clear_reset api's
  doc: dt: binding: omap: am43x/am335x prcm reset
  reset: am43x/am335x support
  ARM: OMAP2+: AM43x/AM335x: have reset controller
  ARM: dts: AM335x: prcm node (for reset)
  ARM: dts: AM4372: prcm node (for reset)

 .../devicetree/bindings/arm/omap/prcm.txt          |  13 ++
 arch/arm/boot/dts/am33xx.dtsi                      |   6 +
 arch/arm/boot/dts/am4372.dtsi                      |   6 +
 arch/arm/mach-omap2/Kconfig                        |   2 +
 drivers/reset/Kconfig                              |  14 ++
 drivers/reset/Makefile                             |   1 +
 drivers/reset/amx3_reset.c                         | 157 +++++++++++++++++++++
 drivers/reset/core.c                               |  32 +++++
 include/linux/reset-controller.h                   |   2 +
 include/linux/reset.h                              |   2 +
 10 files changed, 235 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/prcm.txt
 create mode 100644 drivers/reset/amx3_reset.c

-- 
1.8.3.4

[PATCH RFC 1/6] reset: is_reset and clear_reset api's

[Afzal Mohammed]

Enhance reset framework with "is_reset" and "clear_reset" api's.
is_reset - used by client driver to know reset status
clear_reset - used by client driver to clear reset status

These functionalities may sometimes be achieved by using existing api
like deassert. But in some scenarios, steps to achieve reset requires
clearing reset, deassert reset, enabling clock to module and then
checking reset status. Here enabling clock module is coming in between
reset procedure, hence enhance framework with additional api's.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/reset/core.c             | 32 ++++++++++++++++++++++++++++++++
 include/linux/reset-controller.h |  2 ++
 include/linux/reset.h            |  2 ++
 3 files changed, 36 insertions(+)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index d1b6089..ba12171 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -127,6 +127,38 @@ int reset_control_deassert(struct reset_control *rstc)
 EXPORT_SYMBOL_GPL(reset_control_deassert);
 
 /**
+ * reset_control_is_reset - check reset status
+ * @rstc: reset controller
+ *
+ * Returns a boolean or negative error code
+ *
+ */
+int reset_control_is_reset(struct reset_control *rstc)
+{
+	if (rstc->rcdev->ops->is_reset)
+		return rstc->rcdev->ops->is_reset(rstc->rcdev, rstc->id);
+
+	return -ENOSYS;
+}
+EXPORT_SYMBOL_GPL(reset_control_is_reset);
+
+/**
+ * reset_control_clear_reset - clear the reset
+ * @rstc: reset controller
+ *
+ * Returns zero on success or negative error code
+ *
+ */
+int reset_control_clear_reset(struct reset_control *rstc)
+{
+	if (rstc->rcdev->ops->clear_reset)
+		return rstc->rcdev->ops->clear_reset(rstc->rcdev, rstc->id);
+
+	return -ENOSYS;
+}
+EXPORT_SYMBOL_GPL(reset_control_clear_reset);
+
+/**
  * reset_control_get - Lookup and obtain a reference to a reset controller.
  * @dev: device to be reset by the controller
  * @id: reset line name
diff --git a/include/linux/reset-controller.h b/include/linux/reset-controller.h
index 2f61311..c9bbadb 100644
--- a/include/linux/reset-controller.h
+++ b/include/linux/reset-controller.h
@@ -17,6 +17,8 @@ struct reset_control_ops {
 	int (*reset)(struct reset_controller_dev *rcdev, unsigned long id);
 	int (*assert)(struct reset_controller_dev *rcdev, unsigned long id);
 	int (*deassert)(struct reset_controller_dev *rcdev, unsigned long id);
+	int (*is_reset)(struct reset_controller_dev *rcdev, unsigned long id);
+	int (*clear_reset)(struct reset_controller_dev *rcdev, unsigned long i);
 };
 
 struct module;
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 6082247..da59f9f 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -7,6 +7,8 @@ struct reset_control;
 int reset_control_reset(struct reset_control *rstc);
 int reset_control_assert(struct reset_control *rstc);
 int reset_control_deassert(struct reset_control *rstc);
+int reset_control_is_reset(struct reset_control *rstc);
+int reset_control_clear_reset(struct reset_control *rstc);
 
 struct reset_control *reset_control_get(struct device *dev, const char *id);
 void reset_control_put(struct reset_control *rstc);
-- 
1.8.3.4

[PATCH RFC 2/6] doc: dt: binding: omap: am43x/am335x prcm reset

[Afzal Mohammed]

prcm reset binding for AM43x/AM335x SoC's.

This was started with an attempt to add reset binding without a clear
idea on the device node where binding should appear. So a new node
with compatible "am4372-reset" to represent reset managment in prcm
was added. But finally ended up with a node to represent prcm (with
compatible "am4372-prcm") which was felt to be the natural one.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 Documentation/devicetree/bindings/arm/omap/prcm.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/prcm.txt

diff --git a/Documentation/devicetree/bindings/arm/omap/prcm.txt b/Documentation/devicetree/bindings/arm/omap/prcm.txt
new file mode 100644
index 0000000..ad25abc
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/prcm.txt
@@ -0,0 +1,13 @@
+TI Power Reset Clock Manager (PRCM)
+
+Properties:
+- compatible:	"ti,am4372-prcm" for prcm in am43x SoC's
+		"ti,am3352-prcm" for prcm in am335x SoC's
+- #reset-cells: 1 (refer generic reset bindings for details)
+
+example:
+	prcm: prcm@44df0000 {
+		compatible = "ti,am4372-prcm";
+		reg = <0x44df0000 0xa000>;
+		#reset-cells = <1>;
+	};
-- 
1.8.3.4

[PATCH RFC 3/6] reset: am43x/am335x support

[Afzal Mohammed]

Driver to handle reset block in prcm of AM43x, AM335x SoC's. There are
three reset's that can be handled by this reset driver - gfx, m3 and
pruss. Of this only gfx has been handled here, adding support for the
remaining only require adding array entries with details of pruss and
m3.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/reset/Kconfig      |  14 ++++
 drivers/reset/Makefile     |   1 +
 drivers/reset/amx3_reset.c | 157 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 172 insertions(+)
 create mode 100644 drivers/reset/amx3_reset.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index c9d04f7..2af81b9 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -11,3 +11,17 @@ menuconfig RESET_CONTROLLER
 	  via GPIOs or SoC-internal reset controller modules.
 
 	  If unsure, say no.
+
+if	RESET_CONTROLLER
+
+config	RESET_AMX3
+	bool "AMx3 reset controller"
+	help
+	  Reset controller support for AM43x/AM335x SoC's
+
+	  Reset controller found in TI's AM series of SoC's like
+	  AM335x and AM43x
+
+	  If unsure, say no.
+
+endif
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 1e2d83f..b8c1afe 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_RESET_CONTROLLER) += core.o
+obj-$(CONFIG_RESET_AMX3) += amx3_reset.o
diff --git a/drivers/reset/amx3_reset.c b/drivers/reset/amx3_reset.c
new file mode 100644
index 0000000..4e476a5
--- /dev/null
+++ b/drivers/reset/amx3_reset.c
@@ -0,0 +1,157 @@
+/*
+ * PRCM reset driver for AM335x & AM43x SoC's
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/reset.h>
+#include <linux/reset-controller.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME "amx3_reset"
+
+struct amx3_reset_reg_data {
+	u32	rstctrl_offs;
+	u32	rstst_offs;
+	u8	rstctrl_bit;
+	u8	rstst_bit;
+};
+
+struct amx3_reset_data {
+	struct	amx3_reset_reg_data *reg_data;
+	u8	nr_resets;
+};
+
+static void __iomem *reg_base;
+static const struct amx3_reset_data *amx3_reset_data;
+
+static struct amx3_reset_reg_data am335x_reset_reg_data[] = {
+	{
+		.rstctrl_offs	= 0x1104,
+		.rstst_offs	= 0x1114,
+		.rstctrl_bit	= 0,
+		.rstst_bit	= 0,
+	},
+};
+
+static struct amx3_reset_data am335x_reset_data = {
+	.reg_data	= am335x_reset_reg_data,
+	.nr_resets	= ARRAY_SIZE(am335x_reset_reg_data),
+};
+
+static struct amx3_reset_reg_data am43x_reset_reg_data[] = {
+	{
+		.rstctrl_offs	= 0x410,
+		.rstst_offs	= 0x414,
+		.rstctrl_bit	= 0,
+		.rstst_bit	= 0,
+	},
+};
+
+static struct amx3_reset_data am43x_reset_data = {
+	.reg_data	= am43x_reset_reg_data,
+	.nr_resets	= ARRAY_SIZE(am43x_reset_reg_data),
+};
+
+static int amx3_reset_clear_reset(struct reset_controller_dev *rcdev,
+				  unsigned long id)
+{
+	void __iomem *reg = amx3_reset_data->reg_data[id].rstst_offs + reg_base;
+	u8 bit = amx3_reset_data->reg_data[id].rstst_bit;
+	u32 val = readl(reg);
+
+	val &= ~(1 << bit);
+	val |= 1 << bit;
+	writel(val, reg);
+	return 0;
+}
+
+static int amx3_reset_is_reset(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	void __iomem *reg = amx3_reset_data->reg_data[id].rstst_offs + reg_base;
+	u8 bit = amx3_reset_data->reg_data[id].rstst_bit;
+	u32 val = readl(reg);
+
+	val &= (1 << bit);
+	return !!val;
+}
+
+static int amx3_reset_deassert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	void __iomem *reg = amx3_reset_data->reg_data[id].rstctrl_offs +
+				reg_base;
+	u8 bit = amx3_reset_data->reg_data[id].rstctrl_bit;
+	u32 val = readl(reg);
+
+	val &= ~(1 << bit);
+	writel(val, reg);
+	return 0;
+}
+
+static struct reset_control_ops amx3_reset_ops = {
+	.deassert = amx3_reset_deassert,
+	.is_reset = amx3_reset_is_reset,
+	.clear_reset = amx3_reset_clear_reset,
+};
+
+static struct reset_controller_dev amx3_reset_controller = {
+	.ops = &amx3_reset_ops,
+};
+
+static const struct of_device_id amx3_reset_of_match[] = {
+	{ .compatible = "ti,am3352-prcm", .data = &am335x_reset_data,},
+	{ .compatible = "ti,am4372-prcm", .data = &am43x_reset_data,},
+	{},
+};
+
+static int amx3_reset_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	const struct of_device_id *id;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(reg_base))
+		return PTR_ERR(reg_base);
+
+	amx3_reset_controller.of_node = pdev->dev.of_node;
+	id = of_match_device(amx3_reset_of_match, &pdev->dev);
+	amx3_reset_data = id->data;
+	amx3_reset_controller.nr_resets = amx3_reset_data->nr_resets;
+
+	reset_controller_register(&amx3_reset_controller);
+
+	return 0;
+}
+
+static int amx3_reset_remove(struct platform_device *pdev)
+{
+	reset_controller_unregister(&amx3_reset_controller);
+
+	return 0;
+}
+
+static struct platform_driver amx3_reset_driver = {
+	.probe	= amx3_reset_probe,
+	.remove	= amx3_reset_remove,
+	.driver	= {
+		.name		= DRIVER_NAME,
+		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(amx3_reset_of_match),
+	},
+};
+module_platform_driver(amx3_reset_driver);
+
+MODULE_DESCRIPTION("PRCM reset driver for TI AM43x/AM335x SoC's");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
1.8.3.4

[PATCH RFC 4/6] ARM: OMAP2+: AM43x/AM335x: have reset controller

[Afzal Mohammed]

AM43x, AM335x have reset block as part of prcm, let reset driver be
usable with these SoC's.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 arch/arm/mach-omap2/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 3eed000..fa28d1d 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -72,6 +72,7 @@ config SOC_AM33XX
 	select CPU_V7
 	select MULTI_IRQ_HANDLER
 	select COMMON_CLK
+	select ARCH_HAS_RESET_CONTROLLER
 
 config SOC_AM43XX
 	bool "TI AM43x"
@@ -82,6 +83,7 @@ config SOC_AM43XX
 	select ARM_GIC
 	select COMMON_CLK
 	select MACH_OMAP_GENERIC
+	select ARCH_HAS_RESET_CONTROLLER
 
 config ARCH_OMAP2PLUS
 	bool
-- 
1.8.3.4

[PATCH RFC 5/6] ARM: dts: AM335x: prcm node (for reset)

[Afzal Mohammed]

Add AM335x prcm node with reset binding.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 arch/arm/boot/dts/am33xx.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 4701e3c..c2ccf94 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -530,6 +530,12 @@
 			#size-cells = <1>;
 			status = "disabled";
 		};
+
+		prcm: prcm@44e00000 {
+			compatible = "ti,am3352-prcm";
+			reg = <0x44e00000 0x1300>;
+			#reset-cells = <1>;
+		};
 	};
 
 	clocks {
-- 
1.8.3.4

[PATCH RFC 6/6] ARM: dts: AM4372: prcm node (for reset)

[Afzal Mohammed]

Add AM4372 prcm node with reset binding.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 5a68fde..d0d11b3 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -411,6 +411,12 @@
 			ti,hwmods = "epwmss5";
 			status = "disabled";
 		};
+
+		prcm: prcm@44df0000 {
+			compatible = "ti,am4372-prcm";
+			reg = <0x44df0000 0xa000>;
+			#reset-cells = <1>;
+		};
 	};
 
 	clocks {
-- 
1.8.3.4

Re: [PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver

[Philipp Zabel]

Hi Afzal,

Am Montag, den 02.09.2013, 19:41 +0530 schrieb Afzal Mohammed:
> Hi,
> 
> This is an attempt to achieve reset on AM43x/AM335x based SoC's with
> reset driver making use of the reset framework.
> 
> prcm node is added in device tree, which would hold reset bindings.
> Initially node was made as a one that represents reset functionality
> of SoC. but ended up with node for prcm (which is felt to be natural
> choice) instead. I am a bit doubtful whether placement of prcm node in
> root node as in this series is the right thing.
> 
> Reset driver gets probed for specific prcm node, the same defines
> register details to be used for a particular SoC (using ".data" field
> associated with ".compatible" in driver match table).
> 
> Another option to handle different SoC's would be to add register
> details to DT and let the driver extract it from DT. I vaguely
> remember seeing a thread mentioning that putting register details in
> DT is not preferred. But open to putting register level details
> instead in DT if that is being generally preferred. This would have
> advantage that adding reset support for a new SoC would be easier, but
> would have to put more thought before doing so as DT bindings should
> not change.
> 
> With the approach taken here, for supporting a new SoC with new prcm
> register details, driver would have to be updated much like the way a
> pci based ethernet driver would have to be updated to handle a new IP
> version that is not register level compatible with the existing ones.
> 
> In this series out of the three IP's (gfx, m3, pruss) that would need
> reset support, here as a proof of concept only gfx is taken care.
> Other's can be easily supported by adding new register data array
> entries.
> 
> Two new reset API's are provided to check whether reset is ready and
> to clear reset. This would be required in case IP needs to mix reset
> handling procedure with power/clock managment procedure to achieve
> proper reset and these procedures are sometimes IP specific that would
> make it difficult to handle reset fully in pm_runtime platform support.
> 
> *------*
> client IP handling s/w (DT based) should do as follows:
> 
> 1. Specify reset handle in the relevant DT node, for eg.
> 
> 	myip@deadbeef {
> 		:
> 		:
> 		/* here prcm is the handle to reset binding node */
> 		resets = <&prcm 0>;
> 	};
> 
> 2. In driver, that require reset to be deasserted,
>  (this is the sequence required for gfx on AM43x/AM335x, this would
>   depend on requirements of the IP)
> 
> 	mydriver_probe(struct platform device *pdev)
> 	{
> 		:
> 		:
> 		reset_control_get(&pdev->dev, NULL);
> 		reset_control_clear_reset();
> 		reset_control_deassert();
> 		pm_runtime_get_sync();
> 		if (reset_control_is_reset() != true)
> 			goto err;
> 		reset_control_put();
> 		:
> 		:
> 	}
> 
> *------*

if possible, I'd like to move this logic into the reset controller
driver. Can this be reordered to enable power before deasserting the
reset line (assuming it is initially asserted)? In this case, I'd
suggest to just call device_reset:

	pm_runtime_get_sync(&pdev->dev);
	ret = device_reset(&pdev->dev);
	if (ret)
		goto err;

The ops.reset callback in the prcm driver then can handle clearing
the reset status bit, deasserting the reset control bit, and waiting for
the reset status bit to be set (or timing out with an error).

> May be removing reset handling in hwmod can be considered by making
> use of reset driver.
> 
> Or as another extreme, perhaps, other logic's in the prcm can be
> handled by a new prcm driver and then this reset driver can be a child
> of it.
> 
> Regards
> Afzal
> 
> 
> Afzal Mohammed (6):
>   reset: is_reset and clear_reset api's
>   doc: dt: binding: omap: am43x/am335x prcm reset
>   reset: am43x/am335x support
>   ARM: OMAP2+: AM43x/AM335x: have reset controller
>   ARM: dts: AM335x: prcm node (for reset)
>   ARM: dts: AM4372: prcm node (for reset)
> 
>  .../devicetree/bindings/arm/omap/prcm.txt          |  13 ++
>  arch/arm/boot/dts/am33xx.dtsi                      |   6 +
>  arch/arm/boot/dts/am4372.dtsi                      |   6 +
>  arch/arm/mach-omap2/Kconfig                        |   2 +
>  drivers/reset/Kconfig                              |  14 ++
>  drivers/reset/Makefile                             |   1 +
>  drivers/reset/amx3_reset.c                         | 157 +++++++++++++++++++++
>  drivers/reset/core.c                               |  32 +++++
>  include/linux/reset-controller.h                   |   2 +
>  include/linux/reset.h                              |   2 +
>  10 files changed, 235 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/prcm.txt
>  create mode 100644 drivers/reset/amx3_reset.c

regards
Philipp

Re: [PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver

[Afzal Mohammed]

Hi Philipp,

On Thursday 05 September 2013 03:37 PM, Philipp Zabel wrote:
> Am Montag, den 02.09.2013, 19:41 +0530 schrieb Afzal Mohammed:

>> Two new reset API's are provided to check whether reset is ready and
>> to clear reset. This would be required in case IP needs to mix reset
>> handling procedure with power/clock managment procedure to achieve
>> proper reset and these procedures are sometimes IP specific that would
>> make it difficult to handle reset fully in pm_runtime platform support.

>> client IP handling s/w (DT based) should do as follows:

>> 2. In driver, that require reset to be deasserted,
>>  (this is the sequence required for gfx on AM43x/AM335x, this would
>>   depend on requirements of the IP)
>>
>> 	mydriver_probe(struct platform device *pdev)
>> 	{
>> 		:
>> 		:
>> 		reset_control_get(&pdev->dev, NULL);
>> 		reset_control_clear_reset();
>> 		reset_control_deassert();
>> 		pm_runtime_get_sync();
>> 		if (reset_control_is_reset() != true)
>> 			goto err;
>> 		reset_control_put();
>> 		:
>> 		:
>> 	}

> if possible, I'd like to move this logic into the reset controller
> driver. Can this be reordered to enable power before deasserting the
> reset line (assuming it is initially asserted)? In this case, I'd
> suggest to just call device_reset:
> 
> 	pm_runtime_get_sync(&pdev->dev);
> 	ret = device_reset(&pdev->dev);
> 	if (ret)
> 		goto err;
> 
> The ops.reset callback in the prcm driver then can handle clearing
> the reset status bit, deasserting the reset control bit, and waiting for
> the reset status bit to be set (or timing out with an error).

I too would have loved to have it in such a clean way and was initially
proceeding in that direction, but there is an issue specific to OMAP
family SoC's, which was required to be taken care of (even though
present use case for AM335x/AM43x would work [as it does not have
hardware supervised clockdomain mode] with a small change in platform
level power management support code - a generic one shared with other
OMAP family SoC's)

For a module to be reset, clock domain to which the module clock belongs
should be set to software supervised mode. During "pm_runtime_get_sync",
in OMAP platform level handling code, it first put clockdomain into
software supervised mode, enables clock to module, and once module is
ready, module will be put to hardware supervised mode. In hardware
supervised mode, reset may not happen.

So if device_reset() is done after pm_runtime_get_sync(), reset may not
happen as by the time device_reset() is called, clockdomain would be in
hardware supervised mode. But in other case, as reset is already
deasserted when pm_runtime_get_sync() is called, module would be reset
as it first puts to software supervised mode.

And device reset would happen only upon enabling clock to module (if
reset was deasserted) by pm_runtime_get_sync(), so reset status has to
be checked after pm call, preventing us having device_reset() before
pm_runtime_get_sync(), or else in that case we have to sacrifice on
reset status checking (which may not be reliable)

Another alternative would have been to integrate this reset handling in
low level power management code thus hiding all reset handling from
driver, but as far as I know the reset sequence to be done is sometimes
IP specific, preventing it.

Regards
Afzal

Re: [PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver

[Philipp Zabel]

Am Donnerstag, den 05.09.2013, 21:26 +0530 schrieb Afzal Mohammed:
> Hi Philipp,
> 
> On Thursday 05 September 2013 03:37 PM, Philipp Zabel wrote:
> > Am Montag, den 02.09.2013, 19:41 +0530 schrieb Afzal Mohammed:
> 
> >> Two new reset API's are provided to check whether reset is ready and
> >> to clear reset. This would be required in case IP needs to mix reset
> >> handling procedure with power/clock managment procedure to achieve
> >> proper reset and these procedures are sometimes IP specific that would
> >> make it difficult to handle reset fully in pm_runtime platform support.
> 
> >> client IP handling s/w (DT based) should do as follows:
> 
> >> 2. In driver, that require reset to be deasserted,
> >>  (this is the sequence required for gfx on AM43x/AM335x, this would
> >>   depend on requirements of the IP)
> >>
> >> 	mydriver_probe(struct platform device *pdev)
> >> 	{
> >> 		:
> >> 		:
> >> 		reset_control_get(&pdev->dev, NULL);
> >> 		reset_control_clear_reset();
> >> 		reset_control_deassert();
> >> 		pm_runtime_get_sync();
> >> 		if (reset_control_is_reset() != true)
> >> 			goto err;
> >> 		reset_control_put();
> >> 		:
> >> 		:
> >> 	}
> 
> > if possible, I'd like to move this logic into the reset controller
> > driver. Can this be reordered to enable power before deasserting the
> > reset line (assuming it is initially asserted)? In this case, I'd
> > suggest to just call device_reset:
> > 
> > 	pm_runtime_get_sync(&pdev->dev);
> > 	ret = device_reset(&pdev->dev);
> > 	if (ret)
> > 		goto err;
> > 
> > The ops.reset callback in the prcm driver then can handle clearing
> > the reset status bit, deasserting the reset control bit, and waiting for
> > the reset status bit to be set (or timing out with an error).
> 
> I too would have loved to have it in such a clean way and was initially
> proceeding in that direction, but there is an issue specific to OMAP
> family SoC's, which was required to be taken care of (even though
> present use case for AM335x/AM43x would work [as it does not have
> hardware supervised clockdomain mode] with a small change in platform
> level power management support code - a generic one shared with other
> OMAP family SoC's)
> 
> For a module to be reset, clock domain to which the module clock belongs
> should be set to software supervised mode. During "pm_runtime_get_sync",
> in OMAP platform level handling code, it first put clockdomain into
> software supervised mode, enables clock to module, and once module is
> ready, module will be put to hardware supervised mode. In hardware
> supervised mode, reset may not happen.
> 
> So if device_reset() is done after pm_runtime_get_sync(), reset may not
> happen as by the time device_reset() is called, clockdomain would be in
> hardware supervised mode. But in other case, as reset is already
> deasserted when pm_runtime_get_sync() is called, module would be reset
> as it first puts to software supervised mode.
> 
> And device reset would happen only upon enabling clock to module (if
> reset was deasserted) by pm_runtime_get_sync(), so reset status has to
> be checked after pm call, preventing us having device_reset() before
> pm_runtime_get_sync(), or else in that case we have to sacrifice on
> reset status checking (which may not be reliable)
> 
> Another alternative would have been to integrate this reset handling in
> low level power management code thus hiding all reset handling from
> driver, but as far as I know the reset sequence to be done is sometimes
> IP specific, preventing it.

So if I understand correctly, the only problem is that on OMAP the clock
needs to be enabled to deassert the reset, but as long as the clock
domain is in hardware supervised mode, it won't be enabled?
Would it be possible to create an internal API to switch the clock
domain to software supervised mode, which can be used both by the code
behind pm_runtime_get_sync and reset_control_deassert?

regards
Philipp

Re: [PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver

[Afzal Mohammed]

Hi Philipp,

On Monday 09 September 2013 02:36 PM, Philipp Zabel wrote:

> So if I understand correctly, the only problem is that on OMAP the clock
> needs to be enabled to deassert the reset, but as long as the clock
> domain is in hardware supervised mode, it won't be enabled?

Yes, enabling clock with reset deassertion might not reset the module if
the clock domain is in hardware supervised mode.

> Would it be possible to create an internal API to switch the clock
> domain to software supervised mode, which can be used both by the code
> behind pm_runtime_get_sync and reset_control_deassert?

I will see if that is acceptable.

Another option that would have to be explored is invoking device_reset()
(taking care of clear, deassert & status checking as you suggested)
midway through pm_run_time_get_sync(), when the clockdomain is in
software supervised mode with reset driver taking care of any particular
sequence in the case of multiple reset signals, instead of the IP driver
requiring to take care of it.

Regards
Afzal

Re: [PATCH RFC 4/6] ARM: OMAP2+: AM43x/AM335x: have reset controller

[Tony Lindgren]

* Afzal Mohammed <afzal@ti.com> [130902 07:27]:
> AM43x, AM335x have reset block as part of prcm, let reset driver be
> usable with these SoC's.

Once the driver is ready, this can be merged along with the 
reset driver:

Acked-by: Tony Lindgren <tony@atomide.com>
 
> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> ---
>  arch/arm/mach-omap2/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index 3eed000..fa28d1d 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -72,6 +72,7 @@ config SOC_AM33XX
>  	select CPU_V7
>  	select MULTI_IRQ_HANDLER
>  	select COMMON_CLK
> +	select ARCH_HAS_RESET_CONTROLLER
>  
>  config SOC_AM43XX
>  	bool "TI AM43x"
> @@ -82,6 +83,7 @@ config SOC_AM43XX
>  	select ARM_GIC
>  	select COMMON_CLK
>  	select MACH_OMAP_GENERIC
> +	select ARCH_HAS_RESET_CONTROLLER
>  
>  config ARCH_OMAP2PLUS
>  	bool
> -- 
> 1.8.3.4
> 

Re: [PATCH RFC 5/6] ARM: dts: AM335x: prcm node (for reset)

[Tony Lindgren]

* Afzal Mohammed <afzal@ti.com> [130902 07:30]:
> Add AM335x prcm node with reset binding.

Please put the .dts changes into a separate series from
driver changes so the driver maintainers don't accidentally
queue also the .dts changes. This is to avoid pointless
self-inflicted merge conflicts.

Regards,

Tony
 
> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> ---
>  arch/arm/boot/dts/am33xx.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 4701e3c..c2ccf94 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -530,6 +530,12 @@
>  			#size-cells = <1>;
>  			status = "disabled";
>  		};
> +
> +		prcm: prcm@44e00000 {
> +			compatible = "ti,am3352-prcm";
> +			reg = <0x44e00000 0x1300>;
> +			#reset-cells = <1>;
> +		};
>  	};
>  
>  	clocks {
> -- 
> 1.8.3.4
>