[PATCH 0/3] OMAP: hwmod: reset API proposal

[PATCH 0/3] OMAP: hwmod: reset API proposal

[Omar Ramirez Luna]

Recent changes in omap_hwmod framework have reworked the behaviour
towards hardreset handling, commit 747834a (ARM: OMAP2+: hwmod:
revise hardreset behavior) recommends for drivers to implement
their own reset sequences until code out-of-tree hits mainline
and then their needs and code can be reviewed.

Since it is not clear when this will occur for all drivers and
hwmod code was not deleted (presumably because at some point it
will handle the resets once again), this series exports functions
to handle hardreset lines in an attempt to reduce code duplication
for those who have a common reset sequence.

These APIs are intended to be used by iommu for now, but were
tested with IPU and remoteproc on Pandaboard.
 
Omar Ramirez Luna (3):
  ARM: OMAP: hwmod: partially un-reset hwmods might not be properly
    enabled
  ARM: OMAP: hwmod: revise deassert sequence
  ARM: OMAP: omap_device: expose hwmod assert/deassert to omap devices

 arch/arm/mach-omap2/omap_hwmod.c              |   70 +++++++++++++++++++------
 arch/arm/plat-omap/include/plat/omap_device.h |    4 ++
 arch/arm/plat-omap/omap_device.c              |   45 ++++++++++++++++
 3 files changed, 103 insertions(+), 16 deletions(-)

-- 
1.7.4.1

[PATCH 1/3] ARM: OMAP: hwmod: partially un-reset hwmods might not be properly enabled

[Omar Ramirez Luna]

Some IP blocks might not be using/controlling more than one
reset line, this check loosens the restriction to fully use
hwmod framework for those drivers.

E.g.: ipu has reset lines: mmu_cache, cpu0 and cpu1.
- cpu1 might not be used and hence (with previous check)
  won't be fully enabled by hwmod code.

While at it, prevent _omap4_module_disable if all the hardreset
lines on an IP block are not under reset.

Signed-off-by: Omar Ramirez Luna <omar.luna@linaro.org>
---
 arch/arm/mach-omap2/omap_hwmod.c |   37 ++++++++++++++++++++++---------------
 1 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index bf86f7e..5f0811a 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1537,25 +1537,28 @@ static int _read_hardreset(struct omap_hwmod *oh, const char *name)
 }
 
 /**
- * _are_any_hardreset_lines_asserted - return true if part of @oh is hard-reset
+ * _are_all_hardreset_lines_asserted - return true if the @oh is hard-reset
  * @oh: struct omap_hwmod *
  *
- * If any hardreset line associated with @oh is asserted, then return true.
- * Otherwise, if @oh has no hardreset lines associated with it, or if
- * no hardreset lines associated with @oh are asserted, then return false.
+ * If all hardreset lines associated with @oh are asserted, then return true.
+ * Otherwise, if part of @oh is out hardreset or if no hardreset lines
+ * associated with @oh are asserted, then return false.
  * This function is used to avoid executing some parts of the IP block
- * enable/disable sequence if a hardreset line is set.
+ * enable/disable sequence if its hardreset line is set.
  */
-static bool _are_any_hardreset_lines_asserted(struct omap_hwmod *oh)
+static bool _are_all_hardreset_lines_asserted(struct omap_hwmod *oh)
 {
-	int i;
+	int i, rst_cnt = 0;
 
 	if (oh->rst_lines_cnt == 0)
 		return false;
 
 	for (i = 0; i < oh->rst_lines_cnt; i++)
 		if (_read_hardreset(oh, oh->rst_lines[i].name) > 0)
-			return true;
+			rst_cnt++;
+
+	if (oh->rst_lines_cnt == rst_cnt)
+		return true;
 
 	return false;
 }
@@ -1578,6 +1581,13 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
 	if (!oh->clkdm || !oh->prcm.omap4.modulemode)
 		return -EINVAL;
 
+	/*
+	 * Since integration code might still be doing something, only
+	 * disable if all lines are under hardreset.
+	 */
+	if (!_are_all_hardreset_lines_asserted(oh))
+		return 0;
+
 	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
 
 	omap4_cminst_module_disable(oh->clkdm->prcm_partition,
@@ -1585,9 +1595,6 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
 				    oh->clkdm->clkdm_offs,
 				    oh->prcm.omap4.clkctrl_offs);
 
-	if (_are_any_hardreset_lines_asserted(oh))
-		return 0;
-
 	v = _omap4_wait_target_disable(oh);
 	if (v)
 		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
@@ -1779,7 +1786,7 @@ static int _enable(struct omap_hwmod *oh)
 	}
 
 	/*
-	 * If an IP block contains HW reset lines and any of them are
+	 * If an IP block contains HW reset lines and all of them are
 	 * asserted, we let integration code associated with that
 	 * block handle the enable.  We've received very little
 	 * information on what those driver authors need, and until
@@ -1787,7 +1794,7 @@ static int _enable(struct omap_hwmod *oh)
 	 * posted to the public lists, this is probably the best we
 	 * can do.
 	 */
-	if (_are_any_hardreset_lines_asserted(oh))
+	if (_are_all_hardreset_lines_asserted(oh))
 		return 0;
 
 	/* Mux pins for device runtime if populated */
@@ -1863,7 +1870,7 @@ static int _idle(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
-	if (_are_any_hardreset_lines_asserted(oh))
+	if (_are_all_hardreset_lines_asserted(oh))
 		return 0;
 
 	if (oh->class->sysc)
@@ -1948,7 +1955,7 @@ static int _shutdown(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
-	if (_are_any_hardreset_lines_asserted(oh))
+	if (_are_all_hardreset_lines_asserted(oh))
 		return 0;
 
 	pr_debug("omap_hwmod: %s: disabling\n", oh->name);
-- 
1.7.4.1

[PATCH 2/3] ARM: OMAP: hwmod: revise deassert sequence

[Omar Ramirez Luna]

For a reset sequence to complete cleanly, a module needs its
associated clocks to be enabled, otherwise the timeout check
in prcm code can print a false failure (failed to hardreset)
that occurs because the clocks aren't powered ON and the status
bit checked can't transition without them.

Signed-off-by: Omar Ramirez Luna <omar.luna@linaro.org>
---
 arch/arm/mach-omap2/omap_hwmod.c |   33 ++++++++++++++++++++++++++++++++-
 1 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5f0811a..8a7e5bc 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1472,7 +1472,7 @@ static int _assert_hardreset(struct omap_hwmod *oh, const char *name)
 static int _deassert_hardreset(struct omap_hwmod *oh, const char *name)
 {
 	struct omap_hwmod_rst_info ohri;
-	int ret;
+	int ret, hwsup = 0;
 
 	if (!oh)
 		return -EINVAL;
@@ -1481,6 +1481,23 @@ static int _deassert_hardreset(struct omap_hwmod *oh, const char *name)
 	if (IS_ERR_VALUE(ret))
 		return ret;
 
+	if (oh->clkdm) {
+		/*
+		 * A clockdomain must be in SW_SUP otherwise reset
+		 * might not be completed. The clockdomain can be set
+		 * in HW_AUTO only when the module become ready.
+		 */
+		hwsup = clkdm_in_hwsup(oh->clkdm);
+		ret = clkdm_hwmod_enable(oh->clkdm, oh);
+		if (ret) {
+			WARN(1, "omap_hwmod: %s: could not enable clockdomain %s: %d\n",
+			     oh->name, oh->clkdm->name, ret);
+			return ret;
+		}
+	}
+
+	_enable_clocks(oh);
+
 	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
 		ret = omap2_prm_deassert_hardreset(oh->prcm.omap2.module_offs,
 						   ohri.rst_shift,
@@ -1497,9 +1514,23 @@ static int _deassert_hardreset(struct omap_hwmod *oh, const char *name)
 		return -EINVAL;
 	}
 
+	_disable_clocks(oh);
+
 	if (ret == -EBUSY)
 		pr_warning("omap_hwmod: %s: failed to hardreset\n", oh->name);
 
+	if (!ret) {
+		/*
+		 * Set the clockdomain to HW_AUTO, assuming that the
+		 * previous state was HW_AUTO.
+		 */
+		if (oh->clkdm && hwsup)
+			clkdm_allow_idle(oh->clkdm);
+	} else {
+		if (oh->clkdm)
+			clkdm_hwmod_disable(oh->clkdm, oh);
+	}
+
 	return ret;
 }
 
-- 
1.7.4.1

[PATCH 3/3] ARM: OMAP: omap_device: expose hwmod assert/deassert to omap devices

[Omar Ramirez Luna]

This APIs are meant to be an interface to hwmod assert/deassert
function, omap devices can call them through their platform data
to control their reset lines, they are expected to know the name
of the reset line they are trying to control.

Signed-off-by: Omar Ramirez Luna <omar.luna@linaro.org>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    4 ++
 arch/arm/plat-omap/omap_device.c              |   45 +++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index 4327b2c..27bcc24 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -118,6 +118,10 @@ int omap_device_get_context_loss_count(struct platform_device *pdev);
 
 /* Other */
 
+int omap_device_assert_hardreset(struct platform_device *pdev,
+				 const char *name);
+int omap_device_deassert_hardreset(struct platform_device *pdev,
+				 const char *name);
 int omap_device_idle_hwmods(struct omap_device *od);
 int omap_device_enable_hwmods(struct omap_device *od);
 
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index c490240..8883074 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -925,6 +925,51 @@ int omap_device_shutdown(struct platform_device *pdev)
 }
 
 /**
+ * omap_device_assert_hardreset - set a device's reset line
+ * @pdev: struct platform_device * to reset
+ * @name: const char * with the name of the reset line
+ *
+ * According to @name, set the reset line of the hwmods associated
+ * with this @pdev deivce.
+ */
+int omap_device_assert_hardreset(struct platform_device *pdev, const char *name)
+{
+	int i, ret = 0;
+	struct omap_device *od = to_omap_device(pdev);
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		ret = omap_hwmod_assert_hardreset(od->hwmods[i], name);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+/**
+ * omap_device_deassert_hardreset - lift a device's reset line
+ * @pdev: struct platform_device * to reset
+ * @name: const char * with the name of the reset line
+ *
+ * According to @name, lift the reset line of the hwmods associated
+ * with this @pdev deivce.
+ */
+int omap_device_deassert_hardreset(struct platform_device *pdev,
+				const char *name)
+{
+	int i, ret = 0;
+	struct omap_device *od = to_omap_device(pdev);
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		ret = omap_hwmod_deassert_hardreset(od->hwmods[i], name);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+/**
  * omap_device_align_pm_lat - activate/deactivate device to match wakeup lat lim
  * @od: struct omap_device *
  *
-- 
1.7.4.1

[PATCH 0/3] OMAP: hwmod: reset API proposal

[Omar Ramirez Luna]

Hi Paul,

On 15 June 2012 20:54, Omar Ramirez Luna <omar.luna@linaro.org> wrote:
> Recent changes in omap_hwmod framework have reworked the behaviour
> towards hardreset handling, commit 747834a (ARM: OMAP2+: hwmod:
> revise hardreset behavior) recommends for drivers to implement
> their own reset sequences until code out-of-tree hits mainline
> and then their needs and code can be reviewed.
>
> Since it is not clear when this will occur for all drivers and
> hwmod code was not deleted (presumably because at some point it
> will handle the resets once again), this series exports functions
> to handle hardreset lines in an attempt to reduce code duplication
> for those who have a common reset sequence.
>
> These APIs are intended to be used by iommu for now, but were
> tested with IPU and remoteproc on Pandaboard.

Do you have any comments for these patches?

Regards,

Omar

[PATCH 0/3] OMAP: hwmod: reset API proposal

[Omar Ramirez Luna]

From: Omar Ramirez Luna <omar.ramirez@copitl.com>

***
RESEND: http://bit.ly/PZDbcc
Needed for ipu, dsp and their mmu reset lines.
***

Recent changes in omap_hwmod framework have reworked the behaviour
towards hardreset handling, commit 747834a (ARM: OMAP2+: hwmod:
revise hardreset behavior) recommends for drivers to implement
their own reset sequences until code out-of-tree hits mainline
and then their needs and code can be reviewed.

Since it is not clear when this will occur for all drivers and
hwmod code was not deleted (presumably because at some point it
will handle the resets once again), this series exports functions
to handle hardreset lines in an attempt to reduce code duplication
for those who have a common reset sequence.

These APIs are intended to be used by iommu for now, but were
tested with IPU and remoteproc on Pandaboard.

Omar Ramirez Luna (3):
  ARM: OMAP: hwmod: partially un-reset hwmods might not be properly
    enabled
  ARM: OMAP: hwmod: revise deassert sequence
  ARM: OMAP: omap_device: expose hwmod assert/deassert to omap devices

 arch/arm/mach-omap2/omap_hwmod.c              |   70 +++++++++++++++++++-----
 arch/arm/plat-omap/include/plat/omap_device.h |    4 ++
 arch/arm/plat-omap/omap_device.c              |   45 ++++++++++++++++
 3 files changed, 104 insertions(+), 15 deletions(-)

-- 
1.7.4.1

[PATCH 3/3] ARM: OMAP: omap_device: expose hwmod assert/deassert to omap devices

[Omar Ramirez Luna]

This APIs are meant to be an interface to hwmod assert/deassert
function, omap devices can call them through their platform data
to control their reset lines, they are expected to know the name
of the reset line they are trying to control.

Signed-off-by: Omar Ramirez Luna <omar.luna@linaro.org>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    4 ++
 arch/arm/plat-omap/omap_device.c              |   45 +++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index 4327b2c..27bcc24 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -118,6 +118,10 @@ int omap_device_get_context_loss_count(struct platform_device *pdev);
 
 /* Other */
 
+int omap_device_assert_hardreset(struct platform_device *pdev,
+				 const char *name);
+int omap_device_deassert_hardreset(struct platform_device *pdev,
+				 const char *name);
 int omap_device_idle_hwmods(struct omap_device *od);
 int omap_device_enable_hwmods(struct omap_device *od);
 
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index c490240..8883074 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -925,6 +925,51 @@ int omap_device_shutdown(struct platform_device *pdev)
 }
 
 /**
+ * omap_device_assert_hardreset - set a device's reset line
+ * @pdev: struct platform_device * to reset
+ * @name: const char * with the name of the reset line
+ *
+ * According to @name, set the reset line of the hwmods associated
+ * with this @pdev deivce.
+ */
+int omap_device_assert_hardreset(struct platform_device *pdev, const char *name)
+{
+	int i, ret = 0;
+	struct omap_device *od = to_omap_device(pdev);
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		ret = omap_hwmod_assert_hardreset(od->hwmods[i], name);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+/**
+ * omap_device_deassert_hardreset - lift a device's reset line
+ * @pdev: struct platform_device * to reset
+ * @name: const char * with the name of the reset line
+ *
+ * According to @name, lift the reset line of the hwmods associated
+ * with this @pdev deivce.
+ */
+int omap_device_deassert_hardreset(struct platform_device *pdev,
+				const char *name)
+{
+	int i, ret = 0;
+	struct omap_device *od = to_omap_device(pdev);
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		ret = omap_hwmod_deassert_hardreset(od->hwmods[i], name);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+/**
  * omap_device_align_pm_lat - activate/deactivate device to match wakeup lat lim
  * @od: struct omap_device *
  *
-- 
1.7.4.1

[PATCH 3/3] ARM: OMAP: omap_device: expose hwmod assert/deassert to omap devices

[Paul Walmsley]

Hi Omar

On Mon, 16 Jul 2012, Omar Ramirez Luna wrote:

> This APIs are meant to be an interface to hwmod assert/deassert
> function, omap devices can call them through their platform data
> to control their reset lines, they are expected to know the name
> of the reset line they are trying to control.
> 
> Signed-off-by: Omar Ramirez Luna <omar.luna@linaro.org>

This one has been queued for 3.7 with a few changes. Some more detail was 
added to the function documentationrovement.  Also the multiple 
assignments were removed per Documentation/CodingStyle chapter 1:

"Don't put multiple assignments on a single line either."

Please let me know if you have any comments.


- Paul

From: Omar Ramirez Luna <omar.luna@linaro.org>
Date: Mon, 16 Jul 2012 14:21:25 -0500
Subject: [PATCH] ARM: OMAP2+: omap_device: expose hwmod assert/deassert to
 omap devices

This API is meant to be an interface to hwmod assert/deassert
function, omap devices can call them through their platform data to
control their reset lines, they are expected to know the name of the
reset line they are trying to control.

Signed-off-by: Omar Ramirez Luna <omar.luna@linaro.org>
[paul at pwsan.com: tweaked some documentation; fixed CodingStyle issue]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    4 ++
 arch/arm/plat-omap/omap_device.c              |   55 +++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index 4327b2c..27bcc24 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -118,6 +118,10 @@ int omap_device_get_context_loss_count(struct platform_device *pdev);
 
 /* Other */
 
+int omap_device_assert_hardreset(struct platform_device *pdev,
+				 const char *name);
+int omap_device_deassert_hardreset(struct platform_device *pdev,
+				 const char *name);
 int omap_device_idle_hwmods(struct omap_device *od);
 int omap_device_enable_hwmods(struct omap_device *od);
 
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index c490240..3b02312 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -925,6 +925,61 @@ int omap_device_shutdown(struct platform_device *pdev)
 }
 
 /**
+ * omap_device_assert_hardreset - set a device's hardreset line
+ * @pdev: struct platform_device * to reset
+ * @name: const char * name of the reset line
+ *
+ * Set the hardreset line identified by @name on the IP blocks
+ * associated with the hwmods backing the platform_device @pdev.  All
+ * of the hwmods associated with @pdev must have the same hardreset
+ * line linked to them for this to work.  Passes along the return value
+ * of omap_hwmod_assert_hardreset() in the event of any failure, or
+ * returns 0 upon success.
+ */
+int omap_device_assert_hardreset(struct platform_device *pdev, const char *name)
+{
+	struct omap_device *od = to_omap_device(pdev);
+	int ret = 0;
+	int i;
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		ret = omap_hwmod_assert_hardreset(od->hwmods[i], name);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+/**
+ * omap_device_deassert_hardreset - release a device's hardreset line
+ * @pdev: struct platform_device * to reset
+ * @name: const char * name of the reset line
+ *
+ * Release the hardreset line identified by @name on the IP blocks
+ * associated with the hwmods backing the platform_device @pdev.  All
+ * of the hwmods associated with @pdev must have the same hardreset
+ * line linked to them for this to work.  Passes along the return
+ * value of omap_hwmod_deassert_hardreset() in the event of any
+ * failure, or returns 0 upon success.
+ */
+int omap_device_deassert_hardreset(struct platform_device *pdev,
+				   const char *name)
+{
+	struct omap_device *od = to_omap_device(pdev);
+	int ret = 0;
+	int i;
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		ret = omap_hwmod_deassert_hardreset(od->hwmods[i], name);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+/**
  * omap_device_align_pm_lat - activate/deactivate device to match wakeup lat lim
  * @od: struct omap_device *
  *
-- 
1.7.10.4

[PATCH 3/3] ARM: OMAP: omap_device: expose hwmod assert/deassert to omap devices

[Omar Ramirez Luna]

On 2 August 2012 02:43, Paul Walmsley <paul@pwsan.com> wrote:
>> This APIs are meant to be an interface to hwmod assert/deassert
>> function, omap devices can call them through their platform data
>> to control their reset lines, they are expected to know the name
>> of the reset line they are trying to control.
>>
>> Signed-off-by: Omar Ramirez Luna <omar.luna@linaro.org>
>
> This one has been queued for 3.7 with a few changes. Some more detail was
> added to the function documentationrovement.  Also the multiple
> assignments were removed per Documentation/CodingStyle chapter 1:
>
> "Don't put multiple assignments on a single line either."
>
> Please let me know if you have any comments.

Agree.

Thanks,

Omar