[PATCH v2 0/8] ARM: OMAP2+: hwmod/timer: first set of cleanups for 3.4

[PATCH v2 0/8] ARM: OMAP2+: hwmod/timer: first set of cleanups for 3.4

[Paul Walmsley]

Hi

This series does some cleanup and documentation on the OMAP hwmod code
(and a bit of the OMAP4 data) and timer code. It is the first
prerequisite series to removing a big chunk of hwmod data -- that
will be done in a later series.  This second version reimplements
the hardreset code in a way that will hopefully fix some of the
issues that Kevin was seeing.

Boot-tested on an OMAP35xx BeagleBoard and OMAP44xx ES2 Pandaboard.

This series is also available via git from git://git.pwsan.com/linux-2.6 in
the branch "hwmod_code_cleanup_3.4".


- Paul

---

hwmod_code_cleanup_3.4
   text	   data	    bss	    dec	    hex	filename
6600418	 680476	5593820	12874714	 c473da	vmlinux.omap2plus_defconfig.orig
6600374	 679788	5593820	12873982	 c470fe	vmlinux.omap2plus_defconfig

Paul Walmsley (8):
      ARM: OMAP2+: hwmod: control all hardreset lines attached to a hwmod
      ARM: OMAP4: hwmod data: remove pseudo-hwmods associated with hardreset lines
      ARM: OMAP2+: hwmod: reorganize and document the setup process
      ARM: OMAP2+: hwmod: revise hardreset behavior
      ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset
      ARM: OMAP2+: hwmod: provide a function to return the address space of the MPU RT
      ARM: OMAP2+: hwmod: add omap_hwmod_get_resource_byname()
      ARM: OMAP2+: timer: use a proper interface to get hwmod data


 arch/arm/mach-omap2/omap_hwmod.c             |  671 +++++++++++++++++++-------
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c   |  101 ----
 arch/arm/mach-omap2/timer.c                  |   15 -
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    2 
 4 files changed, 520 insertions(+), 269 deletions(-)

[PATCH v2 0/8] ARM: OMAP2+: hwmod/timer: first set of cleanups for 3.4

[Paul Walmsley]

Hi

This series does some cleanup and documentation on the OMAP hwmod code
(and a bit of the OMAP4 data) and timer code. It is the first
prerequisite series to removing a big chunk of hwmod data -- that
will be done in a later series.  This second version reimplements
the hardreset code in a way that will hopefully fix some of the
issues that Kevin was seeing.

Boot-tested on an OMAP35xx BeagleBoard and OMAP44xx ES2 Pandaboard.

This series is also available via git from git://git.pwsan.com/linux-2.6 in
the branch "hwmod_code_cleanup_3.4".


- Paul

---

hwmod_code_cleanup_3.4
   text	   data	    bss	    dec	    hex	filename
6600418	 680476	5593820	12874714	 c473da	vmlinux.omap2plus_defconfig.orig
6600374	 679788	5593820	12873982	 c470fe	vmlinux.omap2plus_defconfig

Paul Walmsley (8):
      ARM: OMAP2+: hwmod: control all hardreset lines attached to a hwmod
      ARM: OMAP4: hwmod data: remove pseudo-hwmods associated with hardreset lines
      ARM: OMAP2+: hwmod: reorganize and document the setup process
      ARM: OMAP2+: hwmod: revise hardreset behavior
      ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset
      ARM: OMAP2+: hwmod: provide a function to return the address space of the MPU RT
      ARM: OMAP2+: hwmod: add omap_hwmod_get_resource_byname()
      ARM: OMAP2+: timer: use a proper interface to get hwmod data


 arch/arm/mach-omap2/omap_hwmod.c             |  671 +++++++++++++++++++-------
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c   |  101 ----
 arch/arm/mach-omap2/timer.c                  |   15 -
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    2 
 4 files changed, 520 insertions(+), 269 deletions(-)

[PATCH v2 1/8] ARM: OMAP2+: hwmod: control all hardreset lines attached to a hwmod

[Paul Walmsley]

Parts of the hwmod code test to see if a module has one and only one
hardreset line before taking an action.  It seems more appropriate
to control all hardreset lines associated with a hwmod, not just one.

It so happens that with the current hwmod data, this patch will not
change any behavior, since hwmods with hardreset lines have only one
hardreset line associated with them, and 'pseudo-hwmods' are used to
handle the other hardreset lines.  But future hwmod data patches to
remove the pseudo-hwmods will change this.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index eba6cd3..543b0dd 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1475,6 +1475,11 @@ static int _reset(struct omap_hwmod *oh)
 
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
+	/*
+	 * XXX We're not resetting modules with hardreset lines
+	 * automatically here.  Should we do this also, or just expect
+	 * those modules to define custom reset functions?
+	 */
 	ret = (oh->class->reset) ? oh->class->reset(oh) : _ocp_softreset(oh);
 
 	return ret;
@@ -1490,7 +1495,7 @@ static int _reset(struct omap_hwmod *oh)
  */
 static int _enable(struct omap_hwmod *oh)
 {
-	int r;
+	int r, i;
 	int hwsup = 0;
 
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
@@ -1522,15 +1527,15 @@ static int _enable(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
-
 	/*
-	 * If an IP contains only one HW reset line, then de-assert it in order
+	 * If an IP contains HW reset lines, then de-assert them in order
 	 * to allow the module state transition. Otherwise the PRCM will return
 	 * Intransition status, and the init will failed.
 	 */
-	if ((oh->_state == _HWMOD_STATE_INITIALIZED ||
-	     oh->_state == _HWMOD_STATE_DISABLED) && oh->rst_lines_cnt == 1)
-		_deassert_hardreset(oh, oh->rst_lines[0].name);
+	if (oh->_state == _HWMOD_STATE_INITIALIZED ||
+	    oh->_state == _HWMOD_STATE_DISABLED)
+		for (i = 0; i < oh->rst_lines_cnt; i++)
+			_deassert_hardreset(oh, oh->rst_lines[i].name);
 
 	/* Mux pins for device runtime if populated */
 	if (oh->mux && (!oh->mux->enabled ||
@@ -1677,7 +1682,7 @@ int omap_hwmod_set_ocp_autoidle(struct omap_hwmod *oh, u8 autoidle)
  */
 static int _shutdown(struct omap_hwmod *oh)
 {
-	int ret;
+	int ret, i;
 	u8 prev_state;
 
 	if (oh->_state != _HWMOD_STATE_IDLE &&
@@ -1718,12 +1723,8 @@ static int _shutdown(struct omap_hwmod *oh)
 	}
 	/* XXX Should this code also force-disable the optional clocks? */
 
-	/*
-	 * If an IP contains only one HW reset line, then assert it
-	 * after disabling the clocks and before shutting down the IP.
-	 */
-	if (oh->rst_lines_cnt == 1)
-		_assert_hardreset(oh, oh->rst_lines[0].name);
+	for (i = 0; i < oh->rst_lines_cnt; i++)
+		_assert_hardreset(oh, oh->rst_lines[i].name);
 
 	/* Mux pins to safe mode or use populated off mode values */
 	if (oh->mux)
@@ -1776,7 +1777,7 @@ static int _setup(struct omap_hwmod *oh, void *data)
 	 * reset asserted. Exit without warning because that behavior is
 	 * expected.
 	 */
-	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt == 1)
+	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
 		return 0;
 
 	r = _enable(oh);


--
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

[PATCH v2 1/8] ARM: OMAP2+: hwmod: control all hardreset lines attached to a hwmod

[Paul Walmsley]

Parts of the hwmod code test to see if a module has one and only one
hardreset line before taking an action.  It seems more appropriate
to control all hardreset lines associated with a hwmod, not just one.

It so happens that with the current hwmod data, this patch will not
change any behavior, since hwmods with hardreset lines have only one
hardreset line associated with them, and 'pseudo-hwmods' are used to
handle the other hardreset lines.  But future hwmod data patches to
remove the pseudo-hwmods will change this.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index eba6cd3..543b0dd 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1475,6 +1475,11 @@ static int _reset(struct omap_hwmod *oh)
 
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
+	/*
+	 * XXX We're not resetting modules with hardreset lines
+	 * automatically here.  Should we do this also, or just expect
+	 * those modules to define custom reset functions?
+	 */
 	ret = (oh->class->reset) ? oh->class->reset(oh) : _ocp_softreset(oh);
 
 	return ret;
@@ -1490,7 +1495,7 @@ static int _reset(struct omap_hwmod *oh)
  */
 static int _enable(struct omap_hwmod *oh)
 {
-	int r;
+	int r, i;
 	int hwsup = 0;
 
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
@@ -1522,15 +1527,15 @@ static int _enable(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
-
 	/*
-	 * If an IP contains only one HW reset line, then de-assert it in order
+	 * If an IP contains HW reset lines, then de-assert them in order
 	 * to allow the module state transition. Otherwise the PRCM will return
 	 * Intransition status, and the init will failed.
 	 */
-	if ((oh->_state == _HWMOD_STATE_INITIALIZED ||
-	     oh->_state == _HWMOD_STATE_DISABLED) && oh->rst_lines_cnt == 1)
-		_deassert_hardreset(oh, oh->rst_lines[0].name);
+	if (oh->_state == _HWMOD_STATE_INITIALIZED ||
+	    oh->_state == _HWMOD_STATE_DISABLED)
+		for (i = 0; i < oh->rst_lines_cnt; i++)
+			_deassert_hardreset(oh, oh->rst_lines[i].name);
 
 	/* Mux pins for device runtime if populated */
 	if (oh->mux && (!oh->mux->enabled ||
@@ -1677,7 +1682,7 @@ int omap_hwmod_set_ocp_autoidle(struct omap_hwmod *oh, u8 autoidle)
  */
 static int _shutdown(struct omap_hwmod *oh)
 {
-	int ret;
+	int ret, i;
 	u8 prev_state;
 
 	if (oh->_state != _HWMOD_STATE_IDLE &&
@@ -1718,12 +1723,8 @@ static int _shutdown(struct omap_hwmod *oh)
 	}
 	/* XXX Should this code also force-disable the optional clocks? */
 
-	/*
-	 * If an IP contains only one HW reset line, then assert it
-	 * after disabling the clocks and before shutting down the IP.
-	 */
-	if (oh->rst_lines_cnt == 1)
-		_assert_hardreset(oh, oh->rst_lines[0].name);
+	for (i = 0; i < oh->rst_lines_cnt; i++)
+		_assert_hardreset(oh, oh->rst_lines[i].name);
 
 	/* Mux pins to safe mode or use populated off mode values */
 	if (oh->mux)
@@ -1776,7 +1777,7 @@ static int _setup(struct omap_hwmod *oh, void *data)
 	 * reset asserted. Exit without warning because that behavior is
 	 * expected.
 	 */
-	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt == 1)
+	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
 		return 0;
 
 	r = _enable(oh);

[PATCH v2 2/8] ARM: OMAP4: hwmod data: remove pseudo-hwmods associated with hardreset lines

[Paul Walmsley]

Remove the pseudo-hwmods associated with hardreset lines from the
OMAP4 data file.  Future patches will convert this data to register
hwmods by interfaces, rather than registering hwmods directly.  The
pseudo-hwmods aren't associated with any interfaces, so this will
create a problem.

After this change, the hwmod code will reset processor IPs at the
hwmod level, rather than by individual hardreset lines.  So, for
example, if the IVA device driver code wishes to place one of the
sequencer cores into reset, while leaving the other active, it must do
so itself by calling the appropriate PRM functions.

This patch will cause a change in the initialization behavior of the
DSP, IVA, and IPU.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  101 +---------------------------
 1 files changed, 3 insertions(+), 98 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index ef0524c..d2cb189 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -1107,11 +1107,8 @@ static struct omap_hwmod_irq_info omap44xx_dsp_irqs[] = {
 };
 
 static struct omap_hwmod_rst_info omap44xx_dsp_resets[] = {
-	{ .name = "mmu_cache", .rst_shift = 1 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_dsp_c0_resets[] = {
 	{ .name = "dsp", .rst_shift = 0 },
+	{ .name = "mmu_cache", .rst_shift = 1 },
 };
 
 /* dsp -> iva */
@@ -1141,21 +1138,6 @@ static struct omap_hwmod_ocp_if *omap44xx_dsp_slaves[] = {
 	&omap44xx_l4_cfg__dsp,
 };
 
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_dsp_c0_hwmod = {
-	.name		= "dsp_c0",
-	.class		= &omap44xx_dsp_hwmod_class,
-	.clkdm_name	= "tesla_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_dsp_c0_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_dsp_c0_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_TESLA_RSTCTRL_OFFSET,
-		},
-	},
-};
-
 static struct omap_hwmod omap44xx_dsp_hwmod = {
 	.name		= "dsp",
 	.class		= &omap44xx_dsp_hwmod_class,
@@ -2504,15 +2486,9 @@ static struct omap_hwmod_irq_info omap44xx_ipu_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_rst_info omap44xx_ipu_c0_resets[] = {
+static struct omap_hwmod_rst_info omap44xx_ipu_resets[] = {
 	{ .name = "cpu0", .rst_shift = 0 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_ipu_c1_resets[] = {
 	{ .name = "cpu1", .rst_shift = 1 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_ipu_resets[] = {
 	{ .name = "mmu_cache", .rst_shift = 2 },
 };
 
@@ -2534,36 +2510,6 @@ static struct omap_hwmod_ocp_if *omap44xx_ipu_slaves[] = {
 	&omap44xx_l3_main_2__ipu,
 };
 
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_ipu_c0_hwmod = {
-	.name		= "ipu_c0",
-	.class		= &omap44xx_ipu_hwmod_class,
-	.clkdm_name	= "ducati_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_ipu_c0_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_ipu_c0_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
-		},
-	},
-};
-
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_ipu_c1_hwmod = {
-	.name		= "ipu_c1",
-	.class		= &omap44xx_ipu_hwmod_class,
-	.clkdm_name	= "ducati_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_ipu_c1_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_ipu_c1_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
-		},
-	},
-};
-
 static struct omap_hwmod omap44xx_ipu_hwmod = {
 	.name		= "ipu",
 	.class		= &omap44xx_ipu_hwmod_class,
@@ -2693,15 +2639,9 @@ static struct omap_hwmod_irq_info omap44xx_iva_irqs[] = {
 };
 
 static struct omap_hwmod_rst_info omap44xx_iva_resets[] = {
-	{ .name = "logic", .rst_shift = 2 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_iva_seq0_resets[] = {
 	{ .name = "seq0", .rst_shift = 0 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_iva_seq1_resets[] = {
 	{ .name = "seq1", .rst_shift = 1 },
+	{ .name = "logic", .rst_shift = 2 },
 };
 
 /* iva master ports */
@@ -2734,36 +2674,6 @@ static struct omap_hwmod_ocp_if *omap44xx_iva_slaves[] = {
 	&omap44xx_l3_main_2__iva,
 };
 
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_iva_seq0_hwmod = {
-	.name		= "iva_seq0",
-	.class		= &omap44xx_iva_hwmod_class,
-	.clkdm_name	= "ivahd_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_iva_seq0_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_iva_seq0_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_IVAHD_RSTCTRL_OFFSET,
-		},
-	},
-};
-
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_iva_seq1_hwmod = {
-	.name		= "iva_seq1",
-	.class		= &omap44xx_iva_hwmod_class,
-	.clkdm_name	= "ivahd_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_iva_seq1_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_iva_seq1_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_IVAHD_RSTCTRL_OFFSET,
-		},
-	},
-};
-
 static struct omap_hwmod omap44xx_iva_hwmod = {
 	.name		= "iva",
 	.class		= &omap44xx_iva_hwmod_class,
@@ -5522,7 +5432,6 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 
 	/* dsp class */
 	&omap44xx_dsp_hwmod,
-	&omap44xx_dsp_c0_hwmod,
 
 	/* dss class */
 	&omap44xx_dss_hwmod,
@@ -5552,16 +5461,12 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 
 	/* ipu class */
 	&omap44xx_ipu_hwmod,
-	&omap44xx_ipu_c0_hwmod,
-	&omap44xx_ipu_c1_hwmod,
 
 	/* iss class */
 /*	&omap44xx_iss_hwmod, */
 
 	/* iva class */
 	&omap44xx_iva_hwmod,
-	&omap44xx_iva_seq0_hwmod,
-	&omap44xx_iva_seq1_hwmod,
 
 	/* kbd class */
 	&omap44xx_kbd_hwmod,


--
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

[PATCH v2 2/8] ARM: OMAP4: hwmod data: remove pseudo-hwmods associated with hardreset lines

[Paul Walmsley]

Remove the pseudo-hwmods associated with hardreset lines from the
OMAP4 data file.  Future patches will convert this data to register
hwmods by interfaces, rather than registering hwmods directly.  The
pseudo-hwmods aren't associated with any interfaces, so this will
create a problem.

After this change, the hwmod code will reset processor IPs at the
hwmod level, rather than by individual hardreset lines.  So, for
example, if the IVA device driver code wishes to place one of the
sequencer cores into reset, while leaving the other active, it must do
so itself by calling the appropriate PRM functions.

This patch will cause a change in the initialization behavior of the
DSP, IVA, and IPU.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  101 +---------------------------
 1 files changed, 3 insertions(+), 98 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index ef0524c..d2cb189 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -1107,11 +1107,8 @@ static struct omap_hwmod_irq_info omap44xx_dsp_irqs[] = {
 };
 
 static struct omap_hwmod_rst_info omap44xx_dsp_resets[] = {
-	{ .name = "mmu_cache", .rst_shift = 1 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_dsp_c0_resets[] = {
 	{ .name = "dsp", .rst_shift = 0 },
+	{ .name = "mmu_cache", .rst_shift = 1 },
 };
 
 /* dsp -> iva */
@@ -1141,21 +1138,6 @@ static struct omap_hwmod_ocp_if *omap44xx_dsp_slaves[] = {
 	&omap44xx_l4_cfg__dsp,
 };
 
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_dsp_c0_hwmod = {
-	.name		= "dsp_c0",
-	.class		= &omap44xx_dsp_hwmod_class,
-	.clkdm_name	= "tesla_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_dsp_c0_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_dsp_c0_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_TESLA_RSTCTRL_OFFSET,
-		},
-	},
-};
-
 static struct omap_hwmod omap44xx_dsp_hwmod = {
 	.name		= "dsp",
 	.class		= &omap44xx_dsp_hwmod_class,
@@ -2504,15 +2486,9 @@ static struct omap_hwmod_irq_info omap44xx_ipu_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_rst_info omap44xx_ipu_c0_resets[] = {
+static struct omap_hwmod_rst_info omap44xx_ipu_resets[] = {
 	{ .name = "cpu0", .rst_shift = 0 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_ipu_c1_resets[] = {
 	{ .name = "cpu1", .rst_shift = 1 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_ipu_resets[] = {
 	{ .name = "mmu_cache", .rst_shift = 2 },
 };
 
@@ -2534,36 +2510,6 @@ static struct omap_hwmod_ocp_if *omap44xx_ipu_slaves[] = {
 	&omap44xx_l3_main_2__ipu,
 };
 
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_ipu_c0_hwmod = {
-	.name		= "ipu_c0",
-	.class		= &omap44xx_ipu_hwmod_class,
-	.clkdm_name	= "ducati_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_ipu_c0_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_ipu_c0_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
-		},
-	},
-};
-
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_ipu_c1_hwmod = {
-	.name		= "ipu_c1",
-	.class		= &omap44xx_ipu_hwmod_class,
-	.clkdm_name	= "ducati_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_ipu_c1_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_ipu_c1_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
-		},
-	},
-};
-
 static struct omap_hwmod omap44xx_ipu_hwmod = {
 	.name		= "ipu",
 	.class		= &omap44xx_ipu_hwmod_class,
@@ -2693,15 +2639,9 @@ static struct omap_hwmod_irq_info omap44xx_iva_irqs[] = {
 };
 
 static struct omap_hwmod_rst_info omap44xx_iva_resets[] = {
-	{ .name = "logic", .rst_shift = 2 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_iva_seq0_resets[] = {
 	{ .name = "seq0", .rst_shift = 0 },
-};
-
-static struct omap_hwmod_rst_info omap44xx_iva_seq1_resets[] = {
 	{ .name = "seq1", .rst_shift = 1 },
+	{ .name = "logic", .rst_shift = 2 },
 };
 
 /* iva master ports */
@@ -2734,36 +2674,6 @@ static struct omap_hwmod_ocp_if *omap44xx_iva_slaves[] = {
 	&omap44xx_l3_main_2__iva,
 };
 
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_iva_seq0_hwmod = {
-	.name		= "iva_seq0",
-	.class		= &omap44xx_iva_hwmod_class,
-	.clkdm_name	= "ivahd_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_iva_seq0_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_iva_seq0_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_IVAHD_RSTCTRL_OFFSET,
-		},
-	},
-};
-
-/* Pseudo hwmod for reset control purpose only */
-static struct omap_hwmod omap44xx_iva_seq1_hwmod = {
-	.name		= "iva_seq1",
-	.class		= &omap44xx_iva_hwmod_class,
-	.clkdm_name	= "ivahd_clkdm",
-	.flags		= HWMOD_INIT_NO_RESET,
-	.rst_lines	= omap44xx_iva_seq1_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_iva_seq1_resets),
-	.prcm = {
-		.omap4 = {
-			.rstctrl_offs = OMAP4_RM_IVAHD_RSTCTRL_OFFSET,
-		},
-	},
-};
-
 static struct omap_hwmod omap44xx_iva_hwmod = {
 	.name		= "iva",
 	.class		= &omap44xx_iva_hwmod_class,
@@ -5522,7 +5432,6 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 
 	/* dsp class */
 	&omap44xx_dsp_hwmod,
-	&omap44xx_dsp_c0_hwmod,
 
 	/* dss class */
 	&omap44xx_dss_hwmod,
@@ -5552,16 +5461,12 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 
 	/* ipu class */
 	&omap44xx_ipu_hwmod,
-	&omap44xx_ipu_c0_hwmod,
-	&omap44xx_ipu_c1_hwmod,
 
 	/* iss class */
 /*	&omap44xx_iss_hwmod, */
 
 	/* iva class */
 	&omap44xx_iva_hwmod,
-	&omap44xx_iva_seq0_hwmod,
-	&omap44xx_iva_seq1_hwmod,
 
 	/* kbd class */
 	&omap44xx_kbd_hwmod,

[PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Paul Walmsley]

Reorganize the code involved in initializing and configuring an IP
block to make it easier to read and maintain.  This involves improving
documentation, splitting some large functions up into smaller ones to
better conform with Documentation/CodingStyle, and removing some
unnecessary code.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |  361 +++++++++++++++++++++++++-------------
 1 files changed, 242 insertions(+), 119 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 543b0dd..5b368ee 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2,7 +2,7 @@
  * omap_hwmod implementation for OMAP2/3/4
  *
  * Copyright (C) 2009-2011 Nokia Corporation
- * Copyright (C) 2011 Texas Instruments, Inc.
+ * Copyright (C) 2011-2012 Texas Instruments, Inc.
  *
  * Paul Walmsley, Benoît Cousson, Kevin Hilman
  *
@@ -1457,32 +1457,45 @@ dis_opt_clks:
  * _reset - reset an omap_hwmod
  * @oh: struct omap_hwmod *
  *
- * Resets an omap_hwmod @oh.  The default software reset mechanism for
- * most OMAP IP blocks is triggered via the OCP_SYSCONFIG.SOFTRESET
- * bit.  However, some hwmods cannot be reset via this method: some
- * are not targets and therefore have no OCP header registers to
- * access; others (like the IVA) have idiosyncratic reset sequences.
- * So for these relatively rare cases, custom reset code can be
- * supplied in the struct omap_hwmod_class .reset function pointer.
- * Passes along the return value from either _reset() or the custom
- * reset function - these must return -EINVAL if the hwmod cannot be
- * reset this way or if the hwmod is in the wrong state, -ETIMEDOUT if
- * the module did not reset in time, or 0 upon success.
+ * Resets an omap_hwmod @oh.  If the module has a custom reset
+ * function pointer defined, then call it to reset the IP block, and
+ * pass along its return value to the caller.  Otherwise, if the IP
+ * block has an OCP_SYSCONFIG register with a SOFTRESET bitfield
+ * associated with it, call a function to reset the IP block via that
+ * method, and pass along the return value to the caller.  Finally, if
+ * the IP block has some hardreset lines associated with it, assert
+ * all of those, but do _not_ deassert them. (This is because driver
+ * authors have expressed an apparent requirement to control the
+ * deassertion of the hardreset lines themselves.)
+ *
+ * The default software reset mechanism for most OMAP IP blocks is
+ * triggered via the OCP_SYSCONFIG.SOFTRESET bit.  However, some
+ * hwmods cannot be reset via this method.  Some are not targets and
+ * therefore have no OCP header registers to access.  Others (like the
+ * IVA) have idiosyncratic reset sequences.  So for these relatively
+ * rare cases, custom reset code can be supplied in the struct
+ * omap_hwmod_class .reset function pointer.  Passes along the return
+ * value from either _reset() or the custom reset function - these
+ * must return -EINVAL if the hwmod cannot be reset this way or if the
+ * hwmod is in the wrong state, -ETIMEDOUT if the module did not reset
+ * in time, or 0 upon success.
  */
 static int _reset(struct omap_hwmod *oh)
 {
-	int ret;
+	int i;
 
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
-	/*
-	 * XXX We're not resetting modules with hardreset lines
-	 * automatically here.  Should we do this also, or just expect
-	 * those modules to define custom reset functions?
-	 */
-	ret = (oh->class->reset) ? oh->class->reset(oh) : _ocp_softreset(oh);
+	if (oh->class->reset)
+		return oh->class->reset(oh);
 
-	return ret;
+	if (!oh->rst_lines_cnt)
+		return _ocp_softreset(oh);
+
+	for (i = 0; i < oh->rst_lines_cnt; i++)
+		_assert_hardreset(oh, oh->rst_lines[i].name);
+
+	return 0;
 }
 
 /**
@@ -1501,10 +1514,9 @@ static int _enable(struct omap_hwmod *oh)
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
 
 	/*
-	 * hwmods with HWMOD_INIT_NO_IDLE flag set are left
-	 * in enabled state at init.
-	 * Now that someone is really trying to enable them,
-	 * just ensure that the hwmod mux is set.
+	 * hwmods with HWMOD_INIT_NO_IDLE flag set are left in enabled
+	 * state at init.  Now that someone is really trying to enable
+	 * them, just ensure that the hwmod mux is set.
 	 */
 	if (oh->_int_flags & _HWMOD_SKIP_ENABLE) {
 		/*
@@ -1736,46 +1748,77 @@ static int _shutdown(struct omap_hwmod *oh)
 }
 
 /**
- * _setup - do initial configuration of omap_hwmod
+ * _init_mpu_rt_base - populate the virtual address for a hwmod
+ * @oh: struct omap_hwmod * to locate the virtual address
+ *
+ * Cache the virtual address used by the MPU to access this IP block's
+ * registers.  This address is needed early so the OCP registers that
+ * are part of the device's address space can be ioremapped properly.
+ * No return value.
+ */
+static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
+{
+	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
+		return 0;
+
+	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
+}
+
+/**
+ * _setup_iclk_autoidle - configure an IP block's interface clocks
  * @oh: struct omap_hwmod *
  *
- * Writes the CLOCKACTIVITY bits @clockact to the hwmod @oh
- * OCP_SYSCONFIG register.  Returns 0.
+ * Set up the module's interface clocks.  XXX This function is still mostly
+ * a stub; implementing this properly requires iclk autoidle usecounting in
+ * the clock code.   No return value.
  */
-static int _setup(struct omap_hwmod *oh, void *data)
+static void __init _setup_iclk_autoidle(struct omap_hwmod *oh)
 {
-	int i, r;
-	u8 postsetup_state;
+	int i;
 
-	if (oh->_state != _HWMOD_STATE_CLKS_INITED)
-		return 0;
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return;
 
-	/* Set iclk autoidle mode */
-	if (oh->slaves_cnt > 0) {
-		for (i = 0; i < oh->slaves_cnt; i++) {
-			struct omap_hwmod_ocp_if *os = oh->slaves[i];
-			struct clk *c = os->_clk;
+	for (i = 0; i < oh->slaves_cnt; i++) {
+		struct omap_hwmod_ocp_if *os = oh->slaves[i];
+		struct clk *c = os->_clk;
 
-			if (!c)
-				continue;
+		if (!c)
+			continue;
 
-			if (os->flags & OCPIF_SWSUP_IDLE) {
-				/* XXX omap_iclk_deny_idle(c); */
-			} else {
-				/* XXX omap_iclk_allow_idle(c); */
-				clk_enable(c);
-			}
+		if (os->flags & OCPIF_SWSUP_IDLE) {
+			/* XXX omap_iclk_deny_idle(c); */
+		} else {
+			/* XXX omap_iclk_allow_idle(c); */
+			clk_enable(c);
 		}
 	}
 
-	oh->_state = _HWMOD_STATE_INITIALIZED;
+	return;
+}
+
+/**
+ * _setup_reset - reset an IP block during the setup process
+ * @oh: struct omap_hwmod *
+ *
+ * Reset the IP block corresponding to the hwmod @oh during the setup
+ * process.  The IP block is first enabled so it can be successfully
+ * reset.  Returns 0 upon success or a negative error code upon
+ * failure.
+ */
+static int __init _setup_reset(struct omap_hwmod *oh)
+{
+	int r = 0;
+
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return -EINVAL;
 
 	/*
 	 * In the case of hwmod with hardreset that should not be
 	 * de-assert at boot time, we have to keep the module
 	 * initialized, because we cannot enable it properly with the
-	 * reset asserted. Exit without warning because that behavior is
-	 * expected.
+	 * reset asserted. Exit without warning because that behavior
+	 * is expected.
 	 */
 	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
 		return 0;
@@ -1787,20 +1830,65 @@ static int _setup(struct omap_hwmod *oh, void *data)
 		return 0;
 	}
 
-	if (!(oh->flags & HWMOD_INIT_NO_RESET)) {
-		_reset(oh);
+	if (!(oh->flags & HWMOD_INIT_NO_RESET))
+		r = _reset(oh);
 
-		/*
-		 * OCP_SYSCONFIG bits need to be reprogrammed after a softreset.
-		 * The _enable() function should be split to
-		 * avoid the rewrite of the OCP_SYSCONFIG register.
-		 */
-		if (oh->class->sysc) {
-			_update_sysc_cache(oh);
-			_enable_sysc(oh);
-		}
+	/*
+	 * OCP_SYSCONFIG bits need to be reprogrammed after a
+	 * softreset.  The _enable() function should be split to avoid
+	 * the rewrite of the OCP_SYSCONFIG register.
+	 */
+	if (oh->class->sysc) {
+		_update_sysc_cache(oh);
+		_enable_sysc(oh);
 	}
 
+	return r;
+}
+
+/**
+ * _setup_postsetup - transition to the appropriate state after _setup
+ * @oh: struct omap_hwmod *
+ *
+ * Place an IP block represented by @oh into a "post-setup" state --
+ * either IDLE, ENABLED, or DISABLED.  ("post-setup" simply means that
+ * this function is called at the end of _setup().)  The postsetup
+ * state for an IP block can be changed by calling
+ * omap_hwmod_enter_postsetup_state() early in the boot process,
+ * before one of the omap_hwmod_setup*() functions are called for the
+ * IP block.
+ *
+ * The IP block stays in this state until a PM runtime-based driver is
+ * loaded for that IP block.  A post-setup state of IDLE is
+ * appropriate for almost all IP blocks with runtime PM-enabled
+ * drivers, since those drivers are able to enable the IP block.  A
+ * post-setup state of ENABLED is appropriate for kernels with PM
+ * runtime disabled.  The DISABLED state is appropriate for unusual IP
+ * blocks such as the MPU WDTIMER on kernels without WDTIMER drivers
+ * included, since the WDTIMER starts running on reset and will reset
+ * the MPU if left active.
+ *
+ * This post-setup mechanism is deprecated.  Once all of the OMAP
+ * drivers have been converted to use PM runtime, and all of the IP
+ * block data and interconnect data is available to the hwmod code, it
+ * should be possible to replace this mechanism with a "lazy reset"
+ * arrangement.  In a "lazy reset" setup, each IP block is enabled
+ * when the driver first probes, then all remaining IP blocks without
+ * drivers are either shut down or enabled after the drivers have
+ * loaded.  However, this cannot take place until the above
+ * preconditions have been met, since otherwise the late reset code
+ * has no way of knowing which IP blocks are in use by drivers, and
+ * which ones are unused.
+ *
+ * No return value.
+ */
+static void __init _setup_postsetup(struct omap_hwmod *oh)
+{
+	u8 postsetup_state;
+
+	if (oh->rst_lines_cnt > 0)
+		return;
+
 	postsetup_state = oh->_postsetup_state;
 	if (postsetup_state == _HWMOD_STATE_UNKNOWN)
 		postsetup_state = _HWMOD_STATE_ENABLED;
@@ -1823,6 +1911,68 @@ static int _setup(struct omap_hwmod *oh, void *data)
 		WARN(1, "hwmod: %s: unknown postsetup state %d! defaulting to enabled\n",
 		     oh->name, postsetup_state);
 
+	return;
+}
+
+/**
+ * _init - initialize internal data for the hwmod @oh
+ * @oh: struct omap_hwmod *
+ * @n: (unused)
+ *
+ * Look up the clocks and the address space used by the MPU to access
+ * registers belonging to the hwmod @oh.  @oh must already be
+ * registered at this point.  This is the first of two phases for
+ * hwmod initialization.  Code called here does not touch any hardware
+ * registers, it simply prepares internal data structures.  Returns 0
+ * upon success or if the hwmod isn't registered, or -EINVAL upon
+ * failure.
+ */
+static int __init _init(struct omap_hwmod *oh, void *n)
+{
+	int r;
+
+	if (oh->_state != _HWMOD_STATE_REGISTERED)
+		return 0;
+
+	_init_mpu_rt_base(oh, NULL);
+
+	r = _init_clocks(oh, NULL);
+	if (IS_ERR_VALUE(r)) {
+		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh->name);
+		return -EINVAL;
+	}
+
+	oh->_state = _HWMOD_STATE_INITIALIZED;
+
+	return 0;
+}
+
+/**
+ * _setup - prepare IP block hardware for use
+ * @oh: struct omap_hwmod *
+ * @n: (unused, pass NULL)
+ *
+ * Configure the IP block represented by @oh.  This may include
+ * enabling the IP block, resetting it, and placing it into a
+ * post-setup state, depending on the type of IP block and applicable
+ * flags.  IP blocks are reset to prevent any previous configuration
+ * by the bootloader or previous operating system from interfering
+ * with power management or other parts of the system.  The reset can
+ * be avoided; see omap_hwmod_no_setup_reset().  This is the second of
+ * two phases for hwmod initialization.  Code called here generally
+ * affects the IP block hardware, or system integration hardware
+ * associated with the IP block.  Returns 0.
+ */
+static int __init _setup(struct omap_hwmod *oh, void *n)
+{
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return 0;
+
+	_setup_iclk_autoidle(oh);
+
+	if (!_setup_reset(oh))
+		_setup_postsetup(oh);
+
 	return 0;
 }
 
@@ -2030,96 +2180,69 @@ int __init omap_hwmod_register(struct omap_hwmod **ohs)
 	return 0;
 }
 
-/*
- * _populate_mpu_rt_base - populate the virtual address for a hwmod
+/**
+ * _ensure_mpu_hwmod_is_setup - ensure the MPU SS hwmod is init'ed and set up
+ * @oh: pointer to the hwmod currently being set up (usually not the MPU)
  *
- * Must be called only from omap_hwmod_setup_*() so ioremap works properly.
- * Assumes the caller takes care of locking if needed.
+ * If the hwmod data corresponding to the MPU subsystem IP block
+ * hasn't been initialized and set up yet, do so now.  This must be
+ * done first since sleep dependencies may be added from other hwmods
+ * to the MPU.  Intended to be called only by omap_hwmod_setup*().  No
+ * return value.
  */
-static int __init _populate_mpu_rt_base(struct omap_hwmod *oh, void *data)
+static void __init _ensure_mpu_hwmod_is_setup(struct omap_hwmod *oh)
 {
-	if (oh->_state != _HWMOD_STATE_REGISTERED)
-		return 0;
-
-	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
-		return 0;
-
-	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
-
-	return 0;
+	if (!mpu_oh || mpu_oh->_state == _HWMOD_STATE_UNKNOWN) {
+		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
+		       __func__, MPU_INITIATOR_NAME);
+	} else if (mpu_oh->_state == _HWMOD_STATE_REGISTERED && oh != mpu_oh)
+		omap_hwmod_setup_one(MPU_INITIATOR_NAME);
 }
 
 /**
  * omap_hwmod_setup_one - set up a single hwmod
  * @oh_name: const char * name of the already-registered hwmod to set up
  *
- * Must be called after omap2_clk_init().  Resolves the struct clk
- * names to struct clk pointers for each registered omap_hwmod.  Also
- * calls _setup() on each hwmod.  Returns -EINVAL upon error or 0 upon
- * success.
+ * Initialize and set up a single hwmod.  Intended to be used for a
+ * small number of early devices, such as the timer IP blocks used for
+ * the scheduler clock.  Must be called after omap2_clk_init().
+ * Resolves the struct clk names to struct clk pointers for each
+ * registered omap_hwmod.  Also calls _setup() on each hwmod.  Returns
+ * -EINVAL upon error or 0 upon success.
  */
 int __init omap_hwmod_setup_one(const char *oh_name)
 {
 	struct omap_hwmod *oh;
-	int r;
 
 	pr_debug("omap_hwmod: %s: %s\n", oh_name, __func__);
 
-	if (!mpu_oh) {
-		pr_err("omap_hwmod: %s: cannot setup_one: MPU initiator hwmod %s not yet registered\n",
-		       oh_name, MPU_INITIATOR_NAME);
-		return -EINVAL;
-	}
-
 	oh = _lookup(oh_name);
 	if (!oh) {
 		WARN(1, "omap_hwmod: %s: hwmod not yet registered\n", oh_name);
 		return -EINVAL;
 	}
 
-	if (mpu_oh->_state == _HWMOD_STATE_REGISTERED && oh != mpu_oh)
-		omap_hwmod_setup_one(MPU_INITIATOR_NAME);
-
-	r = _populate_mpu_rt_base(oh, NULL);
-	if (IS_ERR_VALUE(r)) {
-		WARN(1, "omap_hwmod: %s: couldn't set mpu_rt_base\n", oh_name);
-		return -EINVAL;
-	}
-
-	r = _init_clocks(oh, NULL);
-	if (IS_ERR_VALUE(r)) {
-		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh_name);
-		return -EINVAL;
-	}
+	_ensure_mpu_hwmod_is_setup(oh);
 
+	_init(oh, NULL);
 	_setup(oh, NULL);
 
 	return 0;
 }
 
 /**
- * omap_hwmod_setup - do some post-clock framework initialization
+ * omap_hwmod_setup_all - set up all registered IP blocks
  *
- * Must be called after omap2_clk_init().  Resolves the struct clk names
- * to struct clk pointers for each registered omap_hwmod.  Also calls
- * _setup() on each hwmod.  Returns 0 upon success.
+ * Initialize and set up all IP blocks registered with the hwmod code.
+ * Must be called after omap2_clk_init().  Resolves the struct clk
+ * names to struct clk pointers for each registered omap_hwmod.  Also
+ * calls _setup() on each hwmod.  Returns 0 upon success.
  */
 static int __init omap_hwmod_setup_all(void)
 {
-	int r;
-
-	if (!mpu_oh) {
-		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
-		       __func__, MPU_INITIATOR_NAME);
-		return -EINVAL;
-	}
-
-	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
-
-	r = omap_hwmod_for_each(_init_clocks, NULL);
-	WARN(IS_ERR_VALUE(r),
-	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
+	_ensure_mpu_hwmod_is_setup(NULL);
 
+	omap_hwmod_for_each(_init, NULL);
 	omap_hwmod_for_each(_setup, NULL);
 
 	return 0;
@@ -2651,10 +2774,10 @@ int omap_hwmod_for_each_by_class(const char *classname,
  * @state: state that _setup() should leave the hwmod in
  *
  * Sets the hwmod state that @oh will enter at the end of _setup()
- * (called by omap_hwmod_setup_*()).  Only valid to call between
- * calling omap_hwmod_register() and omap_hwmod_setup_*().  Returns
- * 0 upon success or -EINVAL if there is a problem with the arguments
- * or if the hwmod is in the wrong state.
+ * (called by omap_hwmod_setup_*()).  See also the documentation
+ * for _setup_postsetup(), above.  Returns 0 upon success or
+ * -EINVAL if there is a problem with the arguments or if the hwmod is
+ * in the wrong state.
  */
 int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state)
 {


--
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

[PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Paul Walmsley]

Reorganize the code involved in initializing and configuring an IP
block to make it easier to read and maintain.  This involves improving
documentation, splitting some large functions up into smaller ones to
better conform with Documentation/CodingStyle, and removing some
unnecessary code.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |  361 +++++++++++++++++++++++++-------------
 1 files changed, 242 insertions(+), 119 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 543b0dd..5b368ee 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2,7 +2,7 @@
  * omap_hwmod implementation for OMAP2/3/4
  *
  * Copyright (C) 2009-2011 Nokia Corporation
- * Copyright (C) 2011 Texas Instruments, Inc.
+ * Copyright (C) 2011-2012 Texas Instruments, Inc.
  *
  * Paul Walmsley, Beno?t Cousson, Kevin Hilman
  *
@@ -1457,32 +1457,45 @@ dis_opt_clks:
  * _reset - reset an omap_hwmod
  * @oh: struct omap_hwmod *
  *
- * Resets an omap_hwmod @oh.  The default software reset mechanism for
- * most OMAP IP blocks is triggered via the OCP_SYSCONFIG.SOFTRESET
- * bit.  However, some hwmods cannot be reset via this method: some
- * are not targets and therefore have no OCP header registers to
- * access; others (like the IVA) have idiosyncratic reset sequences.
- * So for these relatively rare cases, custom reset code can be
- * supplied in the struct omap_hwmod_class .reset function pointer.
- * Passes along the return value from either _reset() or the custom
- * reset function - these must return -EINVAL if the hwmod cannot be
- * reset this way or if the hwmod is in the wrong state, -ETIMEDOUT if
- * the module did not reset in time, or 0 upon success.
+ * Resets an omap_hwmod @oh.  If the module has a custom reset
+ * function pointer defined, then call it to reset the IP block, and
+ * pass along its return value to the caller.  Otherwise, if the IP
+ * block has an OCP_SYSCONFIG register with a SOFTRESET bitfield
+ * associated with it, call a function to reset the IP block via that
+ * method, and pass along the return value to the caller.  Finally, if
+ * the IP block has some hardreset lines associated with it, assert
+ * all of those, but do _not_ deassert them. (This is because driver
+ * authors have expressed an apparent requirement to control the
+ * deassertion of the hardreset lines themselves.)
+ *
+ * The default software reset mechanism for most OMAP IP blocks is
+ * triggered via the OCP_SYSCONFIG.SOFTRESET bit.  However, some
+ * hwmods cannot be reset via this method.  Some are not targets and
+ * therefore have no OCP header registers to access.  Others (like the
+ * IVA) have idiosyncratic reset sequences.  So for these relatively
+ * rare cases, custom reset code can be supplied in the struct
+ * omap_hwmod_class .reset function pointer.  Passes along the return
+ * value from either _reset() or the custom reset function - these
+ * must return -EINVAL if the hwmod cannot be reset this way or if the
+ * hwmod is in the wrong state, -ETIMEDOUT if the module did not reset
+ * in time, or 0 upon success.
  */
 static int _reset(struct omap_hwmod *oh)
 {
-	int ret;
+	int i;
 
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
-	/*
-	 * XXX We're not resetting modules with hardreset lines
-	 * automatically here.  Should we do this also, or just expect
-	 * those modules to define custom reset functions?
-	 */
-	ret = (oh->class->reset) ? oh->class->reset(oh) : _ocp_softreset(oh);
+	if (oh->class->reset)
+		return oh->class->reset(oh);
 
-	return ret;
+	if (!oh->rst_lines_cnt)
+		return _ocp_softreset(oh);
+
+	for (i = 0; i < oh->rst_lines_cnt; i++)
+		_assert_hardreset(oh, oh->rst_lines[i].name);
+
+	return 0;
 }
 
 /**
@@ -1501,10 +1514,9 @@ static int _enable(struct omap_hwmod *oh)
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
 
 	/*
-	 * hwmods with HWMOD_INIT_NO_IDLE flag set are left
-	 * in enabled state at init.
-	 * Now that someone is really trying to enable them,
-	 * just ensure that the hwmod mux is set.
+	 * hwmods with HWMOD_INIT_NO_IDLE flag set are left in enabled
+	 * state at init.  Now that someone is really trying to enable
+	 * them, just ensure that the hwmod mux is set.
 	 */
 	if (oh->_int_flags & _HWMOD_SKIP_ENABLE) {
 		/*
@@ -1736,46 +1748,77 @@ static int _shutdown(struct omap_hwmod *oh)
 }
 
 /**
- * _setup - do initial configuration of omap_hwmod
+ * _init_mpu_rt_base - populate the virtual address for a hwmod
+ * @oh: struct omap_hwmod * to locate the virtual address
+ *
+ * Cache the virtual address used by the MPU to access this IP block's
+ * registers.  This address is needed early so the OCP registers that
+ * are part of the device's address space can be ioremapped properly.
+ * No return value.
+ */
+static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
+{
+	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
+		return 0;
+
+	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
+}
+
+/**
+ * _setup_iclk_autoidle - configure an IP block's interface clocks
  * @oh: struct omap_hwmod *
  *
- * Writes the CLOCKACTIVITY bits @clockact to the hwmod @oh
- * OCP_SYSCONFIG register.  Returns 0.
+ * Set up the module's interface clocks.  XXX This function is still mostly
+ * a stub; implementing this properly requires iclk autoidle usecounting in
+ * the clock code.   No return value.
  */
-static int _setup(struct omap_hwmod *oh, void *data)
+static void __init _setup_iclk_autoidle(struct omap_hwmod *oh)
 {
-	int i, r;
-	u8 postsetup_state;
+	int i;
 
-	if (oh->_state != _HWMOD_STATE_CLKS_INITED)
-		return 0;
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return;
 
-	/* Set iclk autoidle mode */
-	if (oh->slaves_cnt > 0) {
-		for (i = 0; i < oh->slaves_cnt; i++) {
-			struct omap_hwmod_ocp_if *os = oh->slaves[i];
-			struct clk *c = os->_clk;
+	for (i = 0; i < oh->slaves_cnt; i++) {
+		struct omap_hwmod_ocp_if *os = oh->slaves[i];
+		struct clk *c = os->_clk;
 
-			if (!c)
-				continue;
+		if (!c)
+			continue;
 
-			if (os->flags & OCPIF_SWSUP_IDLE) {
-				/* XXX omap_iclk_deny_idle(c); */
-			} else {
-				/* XXX omap_iclk_allow_idle(c); */
-				clk_enable(c);
-			}
+		if (os->flags & OCPIF_SWSUP_IDLE) {
+			/* XXX omap_iclk_deny_idle(c); */
+		} else {
+			/* XXX omap_iclk_allow_idle(c); */
+			clk_enable(c);
 		}
 	}
 
-	oh->_state = _HWMOD_STATE_INITIALIZED;
+	return;
+}
+
+/**
+ * _setup_reset - reset an IP block during the setup process
+ * @oh: struct omap_hwmod *
+ *
+ * Reset the IP block corresponding to the hwmod @oh during the setup
+ * process.  The IP block is first enabled so it can be successfully
+ * reset.  Returns 0 upon success or a negative error code upon
+ * failure.
+ */
+static int __init _setup_reset(struct omap_hwmod *oh)
+{
+	int r = 0;
+
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return -EINVAL;
 
 	/*
 	 * In the case of hwmod with hardreset that should not be
 	 * de-assert at boot time, we have to keep the module
 	 * initialized, because we cannot enable it properly with the
-	 * reset asserted. Exit without warning because that behavior is
-	 * expected.
+	 * reset asserted. Exit without warning because that behavior
+	 * is expected.
 	 */
 	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
 		return 0;
@@ -1787,20 +1830,65 @@ static int _setup(struct omap_hwmod *oh, void *data)
 		return 0;
 	}
 
-	if (!(oh->flags & HWMOD_INIT_NO_RESET)) {
-		_reset(oh);
+	if (!(oh->flags & HWMOD_INIT_NO_RESET))
+		r = _reset(oh);
 
-		/*
-		 * OCP_SYSCONFIG bits need to be reprogrammed after a softreset.
-		 * The _enable() function should be split to
-		 * avoid the rewrite of the OCP_SYSCONFIG register.
-		 */
-		if (oh->class->sysc) {
-			_update_sysc_cache(oh);
-			_enable_sysc(oh);
-		}
+	/*
+	 * OCP_SYSCONFIG bits need to be reprogrammed after a
+	 * softreset.  The _enable() function should be split to avoid
+	 * the rewrite of the OCP_SYSCONFIG register.
+	 */
+	if (oh->class->sysc) {
+		_update_sysc_cache(oh);
+		_enable_sysc(oh);
 	}
 
+	return r;
+}
+
+/**
+ * _setup_postsetup - transition to the appropriate state after _setup
+ * @oh: struct omap_hwmod *
+ *
+ * Place an IP block represented by @oh into a "post-setup" state --
+ * either IDLE, ENABLED, or DISABLED.  ("post-setup" simply means that
+ * this function is called at the end of _setup().)  The postsetup
+ * state for an IP block can be changed by calling
+ * omap_hwmod_enter_postsetup_state() early in the boot process,
+ * before one of the omap_hwmod_setup*() functions are called for the
+ * IP block.
+ *
+ * The IP block stays in this state until a PM runtime-based driver is
+ * loaded for that IP block.  A post-setup state of IDLE is
+ * appropriate for almost all IP blocks with runtime PM-enabled
+ * drivers, since those drivers are able to enable the IP block.  A
+ * post-setup state of ENABLED is appropriate for kernels with PM
+ * runtime disabled.  The DISABLED state is appropriate for unusual IP
+ * blocks such as the MPU WDTIMER on kernels without WDTIMER drivers
+ * included, since the WDTIMER starts running on reset and will reset
+ * the MPU if left active.
+ *
+ * This post-setup mechanism is deprecated.  Once all of the OMAP
+ * drivers have been converted to use PM runtime, and all of the IP
+ * block data and interconnect data is available to the hwmod code, it
+ * should be possible to replace this mechanism with a "lazy reset"
+ * arrangement.  In a "lazy reset" setup, each IP block is enabled
+ * when the driver first probes, then all remaining IP blocks without
+ * drivers are either shut down or enabled after the drivers have
+ * loaded.  However, this cannot take place until the above
+ * preconditions have been met, since otherwise the late reset code
+ * has no way of knowing which IP blocks are in use by drivers, and
+ * which ones are unused.
+ *
+ * No return value.
+ */
+static void __init _setup_postsetup(struct omap_hwmod *oh)
+{
+	u8 postsetup_state;
+
+	if (oh->rst_lines_cnt > 0)
+		return;
+
 	postsetup_state = oh->_postsetup_state;
 	if (postsetup_state == _HWMOD_STATE_UNKNOWN)
 		postsetup_state = _HWMOD_STATE_ENABLED;
@@ -1823,6 +1911,68 @@ static int _setup(struct omap_hwmod *oh, void *data)
 		WARN(1, "hwmod: %s: unknown postsetup state %d! defaulting to enabled\n",
 		     oh->name, postsetup_state);
 
+	return;
+}
+
+/**
+ * _init - initialize internal data for the hwmod @oh
+ * @oh: struct omap_hwmod *
+ * @n: (unused)
+ *
+ * Look up the clocks and the address space used by the MPU to access
+ * registers belonging to the hwmod @oh.  @oh must already be
+ * registered at this point.  This is the first of two phases for
+ * hwmod initialization.  Code called here does not touch any hardware
+ * registers, it simply prepares internal data structures.  Returns 0
+ * upon success or if the hwmod isn't registered, or -EINVAL upon
+ * failure.
+ */
+static int __init _init(struct omap_hwmod *oh, void *n)
+{
+	int r;
+
+	if (oh->_state != _HWMOD_STATE_REGISTERED)
+		return 0;
+
+	_init_mpu_rt_base(oh, NULL);
+
+	r = _init_clocks(oh, NULL);
+	if (IS_ERR_VALUE(r)) {
+		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh->name);
+		return -EINVAL;
+	}
+
+	oh->_state = _HWMOD_STATE_INITIALIZED;
+
+	return 0;
+}
+
+/**
+ * _setup - prepare IP block hardware for use
+ * @oh: struct omap_hwmod *
+ * @n: (unused, pass NULL)
+ *
+ * Configure the IP block represented by @oh.  This may include
+ * enabling the IP block, resetting it, and placing it into a
+ * post-setup state, depending on the type of IP block and applicable
+ * flags.  IP blocks are reset to prevent any previous configuration
+ * by the bootloader or previous operating system from interfering
+ * with power management or other parts of the system.  The reset can
+ * be avoided; see omap_hwmod_no_setup_reset().  This is the second of
+ * two phases for hwmod initialization.  Code called here generally
+ * affects the IP block hardware, or system integration hardware
+ * associated with the IP block.  Returns 0.
+ */
+static int __init _setup(struct omap_hwmod *oh, void *n)
+{
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return 0;
+
+	_setup_iclk_autoidle(oh);
+
+	if (!_setup_reset(oh))
+		_setup_postsetup(oh);
+
 	return 0;
 }
 
@@ -2030,96 +2180,69 @@ int __init omap_hwmod_register(struct omap_hwmod **ohs)
 	return 0;
 }
 
-/*
- * _populate_mpu_rt_base - populate the virtual address for a hwmod
+/**
+ * _ensure_mpu_hwmod_is_setup - ensure the MPU SS hwmod is init'ed and set up
+ * @oh: pointer to the hwmod currently being set up (usually not the MPU)
  *
- * Must be called only from omap_hwmod_setup_*() so ioremap works properly.
- * Assumes the caller takes care of locking if needed.
+ * If the hwmod data corresponding to the MPU subsystem IP block
+ * hasn't been initialized and set up yet, do so now.  This must be
+ * done first since sleep dependencies may be added from other hwmods
+ * to the MPU.  Intended to be called only by omap_hwmod_setup*().  No
+ * return value.
  */
-static int __init _populate_mpu_rt_base(struct omap_hwmod *oh, void *data)
+static void __init _ensure_mpu_hwmod_is_setup(struct omap_hwmod *oh)
 {
-	if (oh->_state != _HWMOD_STATE_REGISTERED)
-		return 0;
-
-	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
-		return 0;
-
-	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
-
-	return 0;
+	if (!mpu_oh || mpu_oh->_state == _HWMOD_STATE_UNKNOWN) {
+		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
+		       __func__, MPU_INITIATOR_NAME);
+	} else if (mpu_oh->_state == _HWMOD_STATE_REGISTERED && oh != mpu_oh)
+		omap_hwmod_setup_one(MPU_INITIATOR_NAME);
 }
 
 /**
  * omap_hwmod_setup_one - set up a single hwmod
  * @oh_name: const char * name of the already-registered hwmod to set up
  *
- * Must be called after omap2_clk_init().  Resolves the struct clk
- * names to struct clk pointers for each registered omap_hwmod.  Also
- * calls _setup() on each hwmod.  Returns -EINVAL upon error or 0 upon
- * success.
+ * Initialize and set up a single hwmod.  Intended to be used for a
+ * small number of early devices, such as the timer IP blocks used for
+ * the scheduler clock.  Must be called after omap2_clk_init().
+ * Resolves the struct clk names to struct clk pointers for each
+ * registered omap_hwmod.  Also calls _setup() on each hwmod.  Returns
+ * -EINVAL upon error or 0 upon success.
  */
 int __init omap_hwmod_setup_one(const char *oh_name)
 {
 	struct omap_hwmod *oh;
-	int r;
 
 	pr_debug("omap_hwmod: %s: %s\n", oh_name, __func__);
 
-	if (!mpu_oh) {
-		pr_err("omap_hwmod: %s: cannot setup_one: MPU initiator hwmod %s not yet registered\n",
-		       oh_name, MPU_INITIATOR_NAME);
-		return -EINVAL;
-	}
-
 	oh = _lookup(oh_name);
 	if (!oh) {
 		WARN(1, "omap_hwmod: %s: hwmod not yet registered\n", oh_name);
 		return -EINVAL;
 	}
 
-	if (mpu_oh->_state == _HWMOD_STATE_REGISTERED && oh != mpu_oh)
-		omap_hwmod_setup_one(MPU_INITIATOR_NAME);
-
-	r = _populate_mpu_rt_base(oh, NULL);
-	if (IS_ERR_VALUE(r)) {
-		WARN(1, "omap_hwmod: %s: couldn't set mpu_rt_base\n", oh_name);
-		return -EINVAL;
-	}
-
-	r = _init_clocks(oh, NULL);
-	if (IS_ERR_VALUE(r)) {
-		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh_name);
-		return -EINVAL;
-	}
+	_ensure_mpu_hwmod_is_setup(oh);
 
+	_init(oh, NULL);
 	_setup(oh, NULL);
 
 	return 0;
 }
 
 /**
- * omap_hwmod_setup - do some post-clock framework initialization
+ * omap_hwmod_setup_all - set up all registered IP blocks
  *
- * Must be called after omap2_clk_init().  Resolves the struct clk names
- * to struct clk pointers for each registered omap_hwmod.  Also calls
- * _setup() on each hwmod.  Returns 0 upon success.
+ * Initialize and set up all IP blocks registered with the hwmod code.
+ * Must be called after omap2_clk_init().  Resolves the struct clk
+ * names to struct clk pointers for each registered omap_hwmod.  Also
+ * calls _setup() on each hwmod.  Returns 0 upon success.
  */
 static int __init omap_hwmod_setup_all(void)
 {
-	int r;
-
-	if (!mpu_oh) {
-		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
-		       __func__, MPU_INITIATOR_NAME);
-		return -EINVAL;
-	}
-
-	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
-
-	r = omap_hwmod_for_each(_init_clocks, NULL);
-	WARN(IS_ERR_VALUE(r),
-	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
+	_ensure_mpu_hwmod_is_setup(NULL);
 
+	omap_hwmod_for_each(_init, NULL);
 	omap_hwmod_for_each(_setup, NULL);
 
 	return 0;
@@ -2651,10 +2774,10 @@ int omap_hwmod_for_each_by_class(const char *classname,
  * @state: state that _setup() should leave the hwmod in
  *
  * Sets the hwmod state that @oh will enter at the end of _setup()
- * (called by omap_hwmod_setup_*()).  Only valid to call between
- * calling omap_hwmod_register() and omap_hwmod_setup_*().  Returns
- * 0 upon success or -EINVAL if there is a problem with the arguments
- * or if the hwmod is in the wrong state.
+ * (called by omap_hwmod_setup_*()).  See also the documentation
+ * for _setup_postsetup(), above.  Returns 0 upon success or
+ * -EINVAL if there is a problem with the arguments or if the hwmod is
+ * in the wrong state.
  */
 int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state)
 {

[PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Paul Walmsley]

Change the way that hardreset lines are handled by the hwmod code.
Hardreset lines are generally associated with initiator IP blocks.
Prior to this change, the hwmod code expected to control hardreset
lines itself, asserting them on shutdown and deasserting them upon
enable.  But driver authors inside TI have commented to us that their
drivers require direct control over these lines.  Unfortunately, these
drivers haven't been posted publicly yet, so it's hard to determine
exactly what is needed, a priori.  This change attempts to set forth
some reasonable semantics that should be an improvement over the
current code.

The semantics implemented by this patch are as follows:

- If the hwmod is not marked with HWMOD_INIT_NO_RESET, then assert all
  associated hardreset lines during IP block setup.  This is intended
  to place the IP blocks into a known state that will not interfere
  with other devices during kernel boot.

- IP blocks with hardreset lines will not be automatically enabled or
  idled during setup.  Instead, they will be left in the INITIALIZED
  state.

- When the hwmod code is asked to enable the IP block -- presumably by
  the driver -- all clocks will be enabled, but the hardreset lines
  will remain untouched.  For this to not return timeout warnings if
  the hardreset lines are asserted, we bypass the PRCM transition
  test.

- When the hwmod code is asked to idle the IP block -- presumably by
  the driver -- all clocks will be disabled, but the hardreset lines
  will remain untouched.  For this to not return timeout warnings if
  the hardreset lines are asserted, we bypass the PRCM transition
  test.

Custom reset functions for IP blocks with hardreset lines still should
be supported and are strongly endorsed.  It is intended that every
subsystem with hardreset lines should have a custom reset function
that can place their subsystem into quiescent idle with the hardreset
lines deasserted.

This reverts most of commit 5365efbe29250a227502256cc912351fe2157b42
("OMAP: hwmod: Add hardreset management support").  Later code
reorganizations caused the sequencing of the code from this patch to
be changed, anyway.

It is expected that drivers may require additional functions to be
exposed from the hwmod code, such as a function to allow drivers to
wait for IDLEST bits to change after hardreset lines are toggled.
Driver authors are expected to propose these if needed.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   51 ++++++++++++++++++--------------------
 1 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5b368ee..db4ad41 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -805,6 +805,9 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
 				    oh->clkdm->clkdm_offs,
 				    oh->prcm.omap4.clkctrl_offs);
 
+	if (oh->rst_lines_cnt >= 0)
+		return 0;
+
 	v = _omap4_wait_target_disable(oh);
 	if (v)
 		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
@@ -1508,7 +1511,7 @@ static int _reset(struct omap_hwmod *oh)
  */
 static int _enable(struct omap_hwmod *oh)
 {
-	int r, i;
+	int r;
 	int hwsup = 0;
 
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
@@ -1539,16 +1542,6 @@ static int _enable(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
-	/*
-	 * If an IP contains HW reset lines, then de-assert them in order
-	 * to allow the module state transition. Otherwise the PRCM will return
-	 * Intransition status, and the init will failed.
-	 */
-	if (oh->_state == _HWMOD_STATE_INITIALIZED ||
-	    oh->_state == _HWMOD_STATE_DISABLED)
-		for (i = 0; i < oh->rst_lines_cnt; i++)
-			_deassert_hardreset(oh, oh->rst_lines[i].name);
-
 	/* Mux pins for device runtime if populated */
 	if (oh->mux && (!oh->mux->enabled ||
 			((oh->_state == _HWMOD_STATE_IDLE) &&
@@ -1575,7 +1568,19 @@ static int _enable(struct omap_hwmod *oh)
 	_enable_clocks(oh);
 	_enable_module(oh);
 
-	r = _wait_target_ready(oh);
+	/*
+	 * If an IP contains HW reset lines, we leave them
+	 * asserted.  But this will block the module's idle state
+	 * transition - the PRCM will return Intransition status.  So
+	 * we need to avoid the target ready-wait in this case.  XXX
+	 * We also need to give the drivers a way to wait for the
+	 * target to become ready once they decide to deassert some
+	 * hardreset lines.  XXX Is this strategy going to break PM
+	 * because the clockdomain may not be able to enter idle while
+	 * the module's idle status is in-transition?  We may just need
+	 * custom reset blocks for all IPs with hardreset lines.
+	 */
+	r = (oh->rst_lines_cnt == 0) ? _wait_target_ready(oh) : 1;
 	if (!r) {
 		/*
 		 * Set the clockdomain to HW_AUTO only if the target is ready,
@@ -1813,21 +1818,13 @@ static int __init _setup_reset(struct omap_hwmod *oh)
 	if (oh->_state != _HWMOD_STATE_INITIALIZED)
 		return -EINVAL;
 
-	/*
-	 * In the case of hwmod with hardreset that should not be
-	 * de-assert at boot time, we have to keep the module
-	 * initialized, because we cannot enable it properly with the
-	 * reset asserted. Exit without warning because that behavior
-	 * is expected.
-	 */
-	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
-		return 0;
-
-	r = _enable(oh);
-	if (r) {
-		pr_warning("omap_hwmod: %s: cannot be enabled (%d)\n",
-			   oh->name, oh->_state);
-		return 0;
+	if (oh->rst_lines_cnt == 0) {
+		r = _enable(oh);
+		if (r) {
+			pr_warning("omap_hwmod: %s: cannot be enabled for reset (%d)\n",
+				   oh->name, oh->_state);
+			return -EINVAL;
+		}
 	}
 
 	if (!(oh->flags & HWMOD_INIT_NO_RESET))


--
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

[PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Paul Walmsley]

Change the way that hardreset lines are handled by the hwmod code.
Hardreset lines are generally associated with initiator IP blocks.
Prior to this change, the hwmod code expected to control hardreset
lines itself, asserting them on shutdown and deasserting them upon
enable.  But driver authors inside TI have commented to us that their
drivers require direct control over these lines.  Unfortunately, these
drivers haven't been posted publicly yet, so it's hard to determine
exactly what is needed, a priori.  This change attempts to set forth
some reasonable semantics that should be an improvement over the
current code.

The semantics implemented by this patch are as follows:

- If the hwmod is not marked with HWMOD_INIT_NO_RESET, then assert all
  associated hardreset lines during IP block setup.  This is intended
  to place the IP blocks into a known state that will not interfere
  with other devices during kernel boot.

- IP blocks with hardreset lines will not be automatically enabled or
  idled during setup.  Instead, they will be left in the INITIALIZED
  state.

- When the hwmod code is asked to enable the IP block -- presumably by
  the driver -- all clocks will be enabled, but the hardreset lines
  will remain untouched.  For this to not return timeout warnings if
  the hardreset lines are asserted, we bypass the PRCM transition
  test.

- When the hwmod code is asked to idle the IP block -- presumably by
  the driver -- all clocks will be disabled, but the hardreset lines
  will remain untouched.  For this to not return timeout warnings if
  the hardreset lines are asserted, we bypass the PRCM transition
  test.

Custom reset functions for IP blocks with hardreset lines still should
be supported and are strongly endorsed.  It is intended that every
subsystem with hardreset lines should have a custom reset function
that can place their subsystem into quiescent idle with the hardreset
lines deasserted.

This reverts most of commit 5365efbe29250a227502256cc912351fe2157b42
("OMAP: hwmod: Add hardreset management support").  Later code
reorganizations caused the sequencing of the code from this patch to
be changed, anyway.

It is expected that drivers may require additional functions to be
exposed from the hwmod code, such as a function to allow drivers to
wait for IDLEST bits to change after hardreset lines are toggled.
Driver authors are expected to propose these if needed.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   51 ++++++++++++++++++--------------------
 1 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5b368ee..db4ad41 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -805,6 +805,9 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
 				    oh->clkdm->clkdm_offs,
 				    oh->prcm.omap4.clkctrl_offs);
 
+	if (oh->rst_lines_cnt >= 0)
+		return 0;
+
 	v = _omap4_wait_target_disable(oh);
 	if (v)
 		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
@@ -1508,7 +1511,7 @@ static int _reset(struct omap_hwmod *oh)
  */
 static int _enable(struct omap_hwmod *oh)
 {
-	int r, i;
+	int r;
 	int hwsup = 0;
 
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
@@ -1539,16 +1542,6 @@ static int _enable(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
-	/*
-	 * If an IP contains HW reset lines, then de-assert them in order
-	 * to allow the module state transition. Otherwise the PRCM will return
-	 * Intransition status, and the init will failed.
-	 */
-	if (oh->_state == _HWMOD_STATE_INITIALIZED ||
-	    oh->_state == _HWMOD_STATE_DISABLED)
-		for (i = 0; i < oh->rst_lines_cnt; i++)
-			_deassert_hardreset(oh, oh->rst_lines[i].name);
-
 	/* Mux pins for device runtime if populated */
 	if (oh->mux && (!oh->mux->enabled ||
 			((oh->_state == _HWMOD_STATE_IDLE) &&
@@ -1575,7 +1568,19 @@ static int _enable(struct omap_hwmod *oh)
 	_enable_clocks(oh);
 	_enable_module(oh);
 
-	r = _wait_target_ready(oh);
+	/*
+	 * If an IP contains HW reset lines, we leave them
+	 * asserted.  But this will block the module's idle state
+	 * transition - the PRCM will return Intransition status.  So
+	 * we need to avoid the target ready-wait in this case.  XXX
+	 * We also need to give the drivers a way to wait for the
+	 * target to become ready once they decide to deassert some
+	 * hardreset lines.  XXX Is this strategy going to break PM
+	 * because the clockdomain may not be able to enter idle while
+	 * the module's idle status is in-transition?  We may just need
+	 * custom reset blocks for all IPs with hardreset lines.
+	 */
+	r = (oh->rst_lines_cnt == 0) ? _wait_target_ready(oh) : 1;
 	if (!r) {
 		/*
 		 * Set the clockdomain to HW_AUTO only if the target is ready,
@@ -1813,21 +1818,13 @@ static int __init _setup_reset(struct omap_hwmod *oh)
 	if (oh->_state != _HWMOD_STATE_INITIALIZED)
 		return -EINVAL;
 
-	/*
-	 * In the case of hwmod with hardreset that should not be
-	 * de-assert at boot time, we have to keep the module
-	 * initialized, because we cannot enable it properly with the
-	 * reset asserted. Exit without warning because that behavior
-	 * is expected.
-	 */
-	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
-		return 0;
-
-	r = _enable(oh);
-	if (r) {
-		pr_warning("omap_hwmod: %s: cannot be enabled (%d)\n",
-			   oh->name, oh->_state);
-		return 0;
+	if (oh->rst_lines_cnt == 0) {
+		r = _enable(oh);
+		if (r) {
+			pr_warning("omap_hwmod: %s: cannot be enabled for reset (%d)\n",
+				   oh->name, oh->_state);
+			return -EINVAL;
+		}
 	}
 
 	if (!(oh->flags & HWMOD_INIT_NO_RESET))

[PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Paul Walmsley]

Move the code that reprograms the OCP_SYSCONFIG bits into the _reset()
function to ensure that it is called after every reset.  The code was
previously in the _setup() function.  So, before this patch, if
_reset() was called from another function, the SYSCONFIG register
won't be reprogrammed.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   31 +++++++++++++++----------------
 1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index db4ad41..aeb6f4c 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1490,13 +1490,22 @@ static int _reset(struct omap_hwmod *oh)
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
 	if (oh->class->reset)
-		return oh->class->reset(oh);
-
-	if (!oh->rst_lines_cnt)
-		return _ocp_softreset(oh);
+		oh->class->reset(oh);
+	else if (!oh->rst_lines_cnt)
+		_ocp_softreset(oh);
+	else
+		for (i = 0; i < oh->rst_lines_cnt; i++)
+			_assert_hardreset(oh, oh->rst_lines[i].name);
 
-	for (i = 0; i < oh->rst_lines_cnt; i++)
-		_assert_hardreset(oh, oh->rst_lines[i].name);
+	/*
+	 * OCP_SYSCONFIG bits need to be reprogrammed after a
+	 * softreset.  The _enable() function should be split to avoid
+	 * the rewrite of the OCP_SYSCONFIG register.
+	 */
+	if (oh->class->sysc) {
+		_update_sysc_cache(oh);
+		_enable_sysc(oh);
+	}
 
 	return 0;
 }
@@ -1830,16 +1839,6 @@ static int __init _setup_reset(struct omap_hwmod *oh)
 	if (!(oh->flags & HWMOD_INIT_NO_RESET))
 		r = _reset(oh);
 
-	/*
-	 * OCP_SYSCONFIG bits need to be reprogrammed after a
-	 * softreset.  The _enable() function should be split to avoid
-	 * the rewrite of the OCP_SYSCONFIG register.
-	 */
-	if (oh->class->sysc) {
-		_update_sysc_cache(oh);
-		_enable_sysc(oh);
-	}
-
 	return r;
 }
 


--
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

[PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Paul Walmsley]

Move the code that reprograms the OCP_SYSCONFIG bits into the _reset()
function to ensure that it is called after every reset.  The code was
previously in the _setup() function.  So, before this patch, if
_reset() was called from another function, the SYSCONFIG register
won't be reprogrammed.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   31 +++++++++++++++----------------
 1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index db4ad41..aeb6f4c 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1490,13 +1490,22 @@ static int _reset(struct omap_hwmod *oh)
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
 	if (oh->class->reset)
-		return oh->class->reset(oh);
-
-	if (!oh->rst_lines_cnt)
-		return _ocp_softreset(oh);
+		oh->class->reset(oh);
+	else if (!oh->rst_lines_cnt)
+		_ocp_softreset(oh);
+	else
+		for (i = 0; i < oh->rst_lines_cnt; i++)
+			_assert_hardreset(oh, oh->rst_lines[i].name);
 
-	for (i = 0; i < oh->rst_lines_cnt; i++)
-		_assert_hardreset(oh, oh->rst_lines[i].name);
+	/*
+	 * OCP_SYSCONFIG bits need to be reprogrammed after a
+	 * softreset.  The _enable() function should be split to avoid
+	 * the rewrite of the OCP_SYSCONFIG register.
+	 */
+	if (oh->class->sysc) {
+		_update_sysc_cache(oh);
+		_enable_sysc(oh);
+	}
 
 	return 0;
 }
@@ -1830,16 +1839,6 @@ static int __init _setup_reset(struct omap_hwmod *oh)
 	if (!(oh->flags & HWMOD_INIT_NO_RESET))
 		r = _reset(oh);
 
-	/*
-	 * OCP_SYSCONFIG bits need to be reprogrammed after a
-	 * softreset.  The _enable() function should be split to avoid
-	 * the rewrite of the OCP_SYSCONFIG register.
-	 */
-	if (oh->class->sysc) {
-		_update_sysc_cache(oh);
-		_enable_sysc(oh);
-	}
-
 	return r;
 }
 

[PATCH v2 6/8] ARM: OMAP2+: hwmod: provide a function to return the address space of the MPU RT

[Paul Walmsley]

A subsequent patch will need to know the struct omap_hwmod_addr_space
record corresponding to the module's register target, used by the MPU.
So, convert _find_mpu_rt_base() into _find_mpu_rt_addr_space().  Then
modify its sole current user, _populate_mpu_rt_base(), to extract the
MPU RT base address itself from the struct omap_hwmod_addr_space record.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   57 +++++++++++++++++++++-----------------
 1 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index aeb6f4c..88b6d96 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -920,24 +920,22 @@ static int __init _find_mpu_port_index(struct omap_hwmod *oh)
 }
 
 /**
- * _find_mpu_rt_base - find hwmod register target base addr accessible by MPU
+ * _find_mpu_rt_addr_space - return MPU register target address space for @oh
  * @oh: struct omap_hwmod *
  *
- * Return the virtual address of the base of the register target of
- * device @oh, or NULL on error.
+ * Returns a pointer to the struct omap_hwmod_addr_space record representing
+ * the register target MPU address space; or returns NULL upon error.
  */
-static void __iomem * __init _find_mpu_rt_base(struct omap_hwmod *oh, u8 index)
+static struct omap_hwmod_addr_space * __init _find_mpu_rt_addr_space(struct omap_hwmod *oh)
 {
 	struct omap_hwmod_ocp_if *os;
 	struct omap_hwmod_addr_space *mem;
-	int i = 0, found = 0;
-	void __iomem *va_start;
+	int found = 0, i = 0;
 
-	if (!oh || oh->slaves_cnt == 0)
+	if (!oh || oh->_int_flags & _HWMOD_NO_MPU_PORT || oh->slaves_cnt == 0)
 		return NULL;
 
-	os = oh->slaves[index];
-
+	os = oh->slaves[oh->_mpu_port_index];
 	if (!os->addr)
 		return NULL;
 
@@ -947,20 +945,7 @@ static void __iomem * __init _find_mpu_rt_base(struct omap_hwmod *oh, u8 index)
 			found = 1;
 	} while (!found && mem->pa_start != mem->pa_end);
 
-	if (found) {
-		va_start = ioremap(mem->pa_start, mem->pa_end - mem->pa_start);
-		if (!va_start) {
-			pr_err("omap_hwmod: %s: Could not ioremap\n", oh->name);
-			return NULL;
-		}
-		pr_debug("omap_hwmod: %s: MPU register target at va %p\n",
-			 oh->name, va_start);
-	} else {
-		pr_debug("omap_hwmod: %s: no MPU register target found\n",
-			 oh->name);
-	}
-
-	return (found) ? va_start : NULL;
+	return (found) ? mem : NULL;
 }
 
 /**
@@ -1772,10 +1757,32 @@ static int _shutdown(struct omap_hwmod *oh)
  */
 static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
 {
+	struct omap_hwmod_addr_space *mem;
+	void __iomem *va_start;
+
+	if (!oh)
+		return;
+
 	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
-		return 0;
+		return;
+
+	mem = _find_mpu_rt_addr_space(oh);
+	if (!mem) {
+		pr_debug("omap_hwmod: %s: no MPU register target found\n",
+			 oh->name);
+		return;
+	}
+
+	va_start = ioremap(mem->pa_start, mem->pa_end - mem->pa_start);
+	if (!va_start) {
+		pr_err("omap_hwmod: %s: Could not ioremap\n", oh->name);
+		return;
+	}
+
+	pr_debug("omap_hwmod: %s: MPU register target at va %p\n",
+		 oh->name, va_start);
 
-	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
+	oh->_mpu_rt_va = va_start;
 }
 
 /**


--
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

[PATCH v2 6/8] ARM: OMAP2+: hwmod: provide a function to return the address space of the MPU RT

[Paul Walmsley]

A subsequent patch will need to know the struct omap_hwmod_addr_space
record corresponding to the module's register target, used by the MPU.
So, convert _find_mpu_rt_base() into _find_mpu_rt_addr_space().  Then
modify its sole current user, _populate_mpu_rt_base(), to extract the
MPU RT base address itself from the struct omap_hwmod_addr_space record.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   57 +++++++++++++++++++++-----------------
 1 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index aeb6f4c..88b6d96 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -920,24 +920,22 @@ static int __init _find_mpu_port_index(struct omap_hwmod *oh)
 }
 
 /**
- * _find_mpu_rt_base - find hwmod register target base addr accessible by MPU
+ * _find_mpu_rt_addr_space - return MPU register target address space for @oh
  * @oh: struct omap_hwmod *
  *
- * Return the virtual address of the base of the register target of
- * device @oh, or NULL on error.
+ * Returns a pointer to the struct omap_hwmod_addr_space record representing
+ * the register target MPU address space; or returns NULL upon error.
  */
-static void __iomem * __init _find_mpu_rt_base(struct omap_hwmod *oh, u8 index)
+static struct omap_hwmod_addr_space * __init _find_mpu_rt_addr_space(struct omap_hwmod *oh)
 {
 	struct omap_hwmod_ocp_if *os;
 	struct omap_hwmod_addr_space *mem;
-	int i = 0, found = 0;
-	void __iomem *va_start;
+	int found = 0, i = 0;
 
-	if (!oh || oh->slaves_cnt == 0)
+	if (!oh || oh->_int_flags & _HWMOD_NO_MPU_PORT || oh->slaves_cnt == 0)
 		return NULL;
 
-	os = oh->slaves[index];
-
+	os = oh->slaves[oh->_mpu_port_index];
 	if (!os->addr)
 		return NULL;
 
@@ -947,20 +945,7 @@ static void __iomem * __init _find_mpu_rt_base(struct omap_hwmod *oh, u8 index)
 			found = 1;
 	} while (!found && mem->pa_start != mem->pa_end);
 
-	if (found) {
-		va_start = ioremap(mem->pa_start, mem->pa_end - mem->pa_start);
-		if (!va_start) {
-			pr_err("omap_hwmod: %s: Could not ioremap\n", oh->name);
-			return NULL;
-		}
-		pr_debug("omap_hwmod: %s: MPU register target at va %p\n",
-			 oh->name, va_start);
-	} else {
-		pr_debug("omap_hwmod: %s: no MPU register target found\n",
-			 oh->name);
-	}
-
-	return (found) ? va_start : NULL;
+	return (found) ? mem : NULL;
 }
 
 /**
@@ -1772,10 +1757,32 @@ static int _shutdown(struct omap_hwmod *oh)
  */
 static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
 {
+	struct omap_hwmod_addr_space *mem;
+	void __iomem *va_start;
+
+	if (!oh)
+		return;
+
 	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
-		return 0;
+		return;
+
+	mem = _find_mpu_rt_addr_space(oh);
+	if (!mem) {
+		pr_debug("omap_hwmod: %s: no MPU register target found\n",
+			 oh->name);
+		return;
+	}
+
+	va_start = ioremap(mem->pa_start, mem->pa_end - mem->pa_start);
+	if (!va_start) {
+		pr_err("omap_hwmod: %s: Could not ioremap\n", oh->name);
+		return;
+	}
+
+	pr_debug("omap_hwmod: %s: MPU register target at va %p\n",
+		 oh->name, va_start);
 
-	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
+	oh->_mpu_rt_va = va_start;
 }
 
 /**

[PATCH v2 7/8] ARM: OMAP2+: hwmod: add omap_hwmod_get_resource_byname()

[Paul Walmsley]

The timer integration code pokes around in hwmod data structures.
Those data structures are about to change.  Define a function,
omap_hwmod_get_resource_byname(), for the timer integration code to
use instead.

The original patch has been changed to use struct resource by Tony's
request, although the caller of this function should not be a driver._
Platform drivers should get their data through the regular platform_*
functions; DT drivers through the appropriate of_* functions.  This a
function is only for use by OMAP core code in arch/arm/*omap*.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |  208 ++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    2 
 2 files changed, 210 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 88b6d96..f9fca98 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -886,6 +886,147 @@ static int _count_ocp_if_addr_spaces(struct omap_hwmod_ocp_if *os)
 }
 
 /**
+ * _get_mpu_irq_by_name - fetch MPU interrupt line number by name
+ * @oh: struct omap_hwmod * to operate on
+ * @name: pointer to the name of the MPU interrupt number to fetch (optional)
+ * @irq: pointer to an unsigned int to store the MPU IRQ number to
+ *
+ * Retrieve a MPU hardware IRQ line number named by @name associated
+ * with the IP block pointed to by @oh.  The IRQ number will be filled
+ * into the address pointed to by @dma.  When @name is non-null, the
+ * IRQ line number associated with the named entry will be returned.
+ * If @name is null, the first matching entry will be returned.  Data
+ * order is not meaningful in hwmod data, so callers are strongly
+ * encouraged to use a non-null @name whenever possible to avoid
+ * unpredictable effects if hwmod data is later added that causes data
+ * ordering to change.  Returns 0 upon success or a negative error
+ * code upon error.
+ */
+static int _get_mpu_irq_by_name(struct omap_hwmod *oh, const char *name,
+				unsigned int *irq)
+{
+	int i;
+	bool found = false;
+
+	if (!oh->mpu_irqs)
+		return -ENOENT;
+
+	i = 0;
+	while (oh->mpu_irqs[i].irq != -1) {
+		if (name == oh->mpu_irqs[i].name ||
+		    !strcmp(name, oh->mpu_irqs[i].name)) {
+			found = true;
+			break;
+		}
+		i++;
+	}
+
+	if (!found)
+		return -ENOENT;
+
+	*irq = oh->mpu_irqs[i].irq;
+
+	return 0;
+}
+
+/**
+ * _get_sdma_req_by_name - fetch SDMA request line ID by name
+ * @oh: struct omap_hwmod * to operate on
+ * @name: pointer to the name of the SDMA request line to fetch (optional)
+ * @dma: pointer to an unsigned int to store the request line ID to
+ *
+ * Retrieve an SDMA request line ID named by @name on the IP block
+ * pointed to by @oh.  The ID will be filled into the address pointed
+ * to by @dma.  When @name is non-null, the request line ID associated
+ * with the named entry will be returned.  If @name is null, the first
+ * matching entry will be returned.  Data order is not meaningful in
+ * hwmod data, so callers are strongly encouraged to use a non-null
+ * @name whenever possible to avoid unpredictable effects if hwmod
+ * data is later added that causes data ordering to change.  Returns 0
+ * upon success or a negative error code upon error.
+ */
+static int _get_sdma_req_by_name(struct omap_hwmod *oh, const char *name,
+				 unsigned int *dma)
+{
+	int i;
+	bool found = false;
+
+	if (!oh->sdma_reqs)
+		return -ENOENT;
+
+	i = 0;
+	while (oh->sdma_reqs[i].dma_req != -1) {
+		if (name == oh->sdma_reqs[i].name ||
+		    !strcmp(name, oh->sdma_reqs[i].name)) {
+			found = true;
+			break;
+		}
+		i++;
+	}
+
+	if (!found)
+		return -ENOENT;
+
+	*dma = oh->sdma_reqs[i].dma_req;
+
+	return 0;
+}
+
+/**
+ * _get_addr_space_by_name - fetch address space start & end by name
+ * @oh: struct omap_hwmod * to operate on
+ * @name: pointer to the name of the address space to fetch (optional)
+ * @pa_start: pointer to a u32 to store the starting address to
+ * @pa_end: pointer to a u32 to store the ending address to
+ *
+ * Retrieve address space start and end addresses for the IP block
+ * pointed to by @oh.  The data will be filled into the addresses
+ * pointed to by @pa_start and @pa_end.  When @name is non-null, the
+ * address space data associated with the named entry will be
+ * returned.  If @name is null, the first matching entry will be
+ * returned.  Data order is not meaningful in hwmod data, so callers
+ * are strongly encouraged to use a non-null @name whenever possible
+ * to avoid unpredictable effects if hwmod data is later added that
+ * causes data ordering to change.  Returns 0 upon success or a
+ * negative error code upon error.
+ */
+static int _get_addr_space_by_name(struct omap_hwmod *oh, const char *name,
+				   u32 *pa_start, u32 *pa_end)
+{
+	int i, j;
+	struct omap_hwmod_ocp_if *os;
+	bool found = false;
+
+	for (i = 0; i < oh->slaves_cnt; i++) {
+		os = oh->slaves[i];
+
+		if (!os->addr)
+			return -ENOENT;
+
+		j = 0;
+		while (os->addr[j].pa_start != os->addr[j].pa_end) {
+			if (name == os->addr[j].name ||
+			    !strcmp(name, os->addr[j].name)) {
+				found = true;
+				break;
+			}
+			j++;
+		}
+
+		if (found)
+			break;
+	}
+
+	if (!found)
+		return -ENOENT;
+
+	*pa_start = os->addr[j].pa_start;
+	*pa_end = os->addr[j].pa_end;
+
+	return 0;
+}
+
+/**
  * _find_mpu_port_index - find hwmod OCP slave port ID intended for MPU use
  * @oh: struct omap_hwmod *
  *
@@ -2402,6 +2543,10 @@ int omap_hwmod_reset(struct omap_hwmod *oh)
 	return r;
 }
 
+/*
+ * IP block data retrieval functions
+ */
+
 /**
  * omap_hwmod_count_resources - count number of struct resources needed by hwmod
  * @oh: struct omap_hwmod *
@@ -2485,6 +2630,69 @@ int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res)
 }
 
 /**
+ * omap_hwmod_get_resource_byname - fetch IP block integration data by name
+ * @oh: struct omap_hwmod * to operate on
+ * @type: one of the IORESOURCE_* constants from include/linux/ioport.h
+ * @name: pointer to the name of the data to fetch (optional)
+ * @rsrc: pointer to a struct resource, allocated by the caller
+ *
+ * Retrieve MPU IRQ, SDMA request line, or address space start/end
+ * data for the IP block pointed to by @oh.  The data will be filled
+ * into a struct resource record pointed to by @rsrc.  The struct
+ * resource must be allocated by the caller.  When @name is non-null,
+ * the data associated with the matching entry in the IRQ/SDMA/address
+ * space hwmod data arrays will be returned.  If @name is null, the
+ * first array entry will be returned.  Data order is not meaningful
+ * in hwmod data, so callers are strongly encouraged to use a non-null
+ * @name whenever possible to avoid unpredictable effects if hwmod
+ * data is later added that causes data ordering to change.  This
+ * function is only intended for use by OMAP core code.  Device
+ * drivers should not call this function - the appropriate bus-related
+ * data accessor functions should be used instead.  Returns 0 upon
+ * success or a negative error code upon error.
+ */
+int omap_hwmod_get_resource_byname(struct omap_hwmod *oh, unsigned int type,
+				   const char *name, struct resource *rsrc)
+{
+	int r;
+	unsigned int irq, dma;
+	u32 pa_start, pa_end;
+
+	if (!oh || !rsrc)
+		return -EINVAL;
+
+	if (type == IORESOURCE_IRQ) {
+		r = _get_mpu_irq_by_name(oh, name, &irq);
+		if (r)
+			return r;
+
+		rsrc->start = irq;
+		rsrc->end = irq;
+	} else if (type == IORESOURCE_DMA) {
+		r = _get_sdma_req_by_name(oh, name, &dma);
+		if (r)
+			return r;
+
+		rsrc->start = dma;
+		rsrc->end = dma;
+	} else if (type == IORESOURCE_MEM) {
+		r = _get_addr_space_by_name(oh, name, &pa_start, &pa_end);
+		if (r)
+			return r;
+
+		rsrc->start = pa_start;
+		rsrc->end = pa_end;
+	} else {
+		return -EINVAL;
+	}
+
+	rsrc->flags = type;
+	rsrc->name = name;
+
+	return 0;
+}
+
+/**
  * omap_hwmod_get_pwrdm - return pointer to this module's main powerdomain
  * @oh: struct omap_hwmod *
  *
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 6470101..ff4d7f9 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -581,6 +581,8 @@ int omap_hwmod_softreset(struct omap_hwmod *oh);
 
 int omap_hwmod_count_resources(struct omap_hwmod *oh);
 int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res);
+int omap_hwmod_get_resource_byname(struct omap_hwmod *oh, unsigned int type,
+				   const char *name, struct resource *res);
 
 struct powerdomain *omap_hwmod_get_pwrdm(struct omap_hwmod *oh);
 void __iomem *omap_hwmod_get_mpu_rt_va(struct omap_hwmod *oh);


--
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

[PATCH v2 7/8] ARM: OMAP2+: hwmod: add omap_hwmod_get_resource_byname()

[Paul Walmsley]

The timer integration code pokes around in hwmod data structures.
Those data structures are about to change.  Define a function,
omap_hwmod_get_resource_byname(), for the timer integration code to
use instead.

The original patch has been changed to use struct resource by Tony's
request, although the caller of this function should not be a driver._
Platform drivers should get their data through the regular platform_*
functions; DT drivers through the appropriate of_* functions.  This a
function is only for use by OMAP core code in arch/arm/*omap*.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |  208 ++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    2 
 2 files changed, 210 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 88b6d96..f9fca98 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -886,6 +886,147 @@ static int _count_ocp_if_addr_spaces(struct omap_hwmod_ocp_if *os)
 }
 
 /**
+ * _get_mpu_irq_by_name - fetch MPU interrupt line number by name
+ * @oh: struct omap_hwmod * to operate on
+ * @name: pointer to the name of the MPU interrupt number to fetch (optional)
+ * @irq: pointer to an unsigned int to store the MPU IRQ number to
+ *
+ * Retrieve a MPU hardware IRQ line number named by @name associated
+ * with the IP block pointed to by @oh.  The IRQ number will be filled
+ * into the address pointed to by @dma.  When @name is non-null, the
+ * IRQ line number associated with the named entry will be returned.
+ * If @name is null, the first matching entry will be returned.  Data
+ * order is not meaningful in hwmod data, so callers are strongly
+ * encouraged to use a non-null @name whenever possible to avoid
+ * unpredictable effects if hwmod data is later added that causes data
+ * ordering to change.  Returns 0 upon success or a negative error
+ * code upon error.
+ */
+static int _get_mpu_irq_by_name(struct omap_hwmod *oh, const char *name,
+				unsigned int *irq)
+{
+	int i;
+	bool found = false;
+
+	if (!oh->mpu_irqs)
+		return -ENOENT;
+
+	i = 0;
+	while (oh->mpu_irqs[i].irq != -1) {
+		if (name == oh->mpu_irqs[i].name ||
+		    !strcmp(name, oh->mpu_irqs[i].name)) {
+			found = true;
+			break;
+		}
+		i++;
+	}
+
+	if (!found)
+		return -ENOENT;
+
+	*irq = oh->mpu_irqs[i].irq;
+
+	return 0;
+}
+
+/**
+ * _get_sdma_req_by_name - fetch SDMA request line ID by name
+ * @oh: struct omap_hwmod * to operate on
+ * @name: pointer to the name of the SDMA request line to fetch (optional)
+ * @dma: pointer to an unsigned int to store the request line ID to
+ *
+ * Retrieve an SDMA request line ID named by @name on the IP block
+ * pointed to by @oh.  The ID will be filled into the address pointed
+ * to by @dma.  When @name is non-null, the request line ID associated
+ * with the named entry will be returned.  If @name is null, the first
+ * matching entry will be returned.  Data order is not meaningful in
+ * hwmod data, so callers are strongly encouraged to use a non-null
+ * @name whenever possible to avoid unpredictable effects if hwmod
+ * data is later added that causes data ordering to change.  Returns 0
+ * upon success or a negative error code upon error.
+ */
+static int _get_sdma_req_by_name(struct omap_hwmod *oh, const char *name,
+				 unsigned int *dma)
+{
+	int i;
+	bool found = false;
+
+	if (!oh->sdma_reqs)
+		return -ENOENT;
+
+	i = 0;
+	while (oh->sdma_reqs[i].dma_req != -1) {
+		if (name == oh->sdma_reqs[i].name ||
+		    !strcmp(name, oh->sdma_reqs[i].name)) {
+			found = true;
+			break;
+		}
+		i++;
+	}
+
+	if (!found)
+		return -ENOENT;
+
+	*dma = oh->sdma_reqs[i].dma_req;
+
+	return 0;
+}
+
+/**
+ * _get_addr_space_by_name - fetch address space start & end by name
+ * @oh: struct omap_hwmod * to operate on
+ * @name: pointer to the name of the address space to fetch (optional)
+ * @pa_start: pointer to a u32 to store the starting address to
+ * @pa_end: pointer to a u32 to store the ending address to
+ *
+ * Retrieve address space start and end addresses for the IP block
+ * pointed to by @oh.  The data will be filled into the addresses
+ * pointed to by @pa_start and @pa_end.  When @name is non-null, the
+ * address space data associated with the named entry will be
+ * returned.  If @name is null, the first matching entry will be
+ * returned.  Data order is not meaningful in hwmod data, so callers
+ * are strongly encouraged to use a non-null @name whenever possible
+ * to avoid unpredictable effects if hwmod data is later added that
+ * causes data ordering to change.  Returns 0 upon success or a
+ * negative error code upon error.
+ */
+static int _get_addr_space_by_name(struct omap_hwmod *oh, const char *name,
+				   u32 *pa_start, u32 *pa_end)
+{
+	int i, j;
+	struct omap_hwmod_ocp_if *os;
+	bool found = false;
+
+	for (i = 0; i < oh->slaves_cnt; i++) {
+		os = oh->slaves[i];
+
+		if (!os->addr)
+			return -ENOENT;
+
+		j = 0;
+		while (os->addr[j].pa_start != os->addr[j].pa_end) {
+			if (name == os->addr[j].name ||
+			    !strcmp(name, os->addr[j].name)) {
+				found = true;
+				break;
+			}
+			j++;
+		}
+
+		if (found)
+			break;
+	}
+
+	if (!found)
+		return -ENOENT;
+
+	*pa_start = os->addr[j].pa_start;
+	*pa_end = os->addr[j].pa_end;
+
+	return 0;
+}
+
+/**
  * _find_mpu_port_index - find hwmod OCP slave port ID intended for MPU use
  * @oh: struct omap_hwmod *
  *
@@ -2402,6 +2543,10 @@ int omap_hwmod_reset(struct omap_hwmod *oh)
 	return r;
 }
 
+/*
+ * IP block data retrieval functions
+ */
+
 /**
  * omap_hwmod_count_resources - count number of struct resources needed by hwmod
  * @oh: struct omap_hwmod *
@@ -2485,6 +2630,69 @@ int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res)
 }
 
 /**
+ * omap_hwmod_get_resource_byname - fetch IP block integration data by name
+ * @oh: struct omap_hwmod * to operate on
+ * @type: one of the IORESOURCE_* constants from include/linux/ioport.h
+ * @name: pointer to the name of the data to fetch (optional)
+ * @rsrc: pointer to a struct resource, allocated by the caller
+ *
+ * Retrieve MPU IRQ, SDMA request line, or address space start/end
+ * data for the IP block pointed to by @oh.  The data will be filled
+ * into a struct resource record pointed to by @rsrc.  The struct
+ * resource must be allocated by the caller.  When @name is non-null,
+ * the data associated with the matching entry in the IRQ/SDMA/address
+ * space hwmod data arrays will be returned.  If @name is null, the
+ * first array entry will be returned.  Data order is not meaningful
+ * in hwmod data, so callers are strongly encouraged to use a non-null
+ * @name whenever possible to avoid unpredictable effects if hwmod
+ * data is later added that causes data ordering to change.  This
+ * function is only intended for use by OMAP core code.  Device
+ * drivers should not call this function - the appropriate bus-related
+ * data accessor functions should be used instead.  Returns 0 upon
+ * success or a negative error code upon error.
+ */
+int omap_hwmod_get_resource_byname(struct omap_hwmod *oh, unsigned int type,
+				   const char *name, struct resource *rsrc)
+{
+	int r;
+	unsigned int irq, dma;
+	u32 pa_start, pa_end;
+
+	if (!oh || !rsrc)
+		return -EINVAL;
+
+	if (type == IORESOURCE_IRQ) {
+		r = _get_mpu_irq_by_name(oh, name, &irq);
+		if (r)
+			return r;
+
+		rsrc->start = irq;
+		rsrc->end = irq;
+	} else if (type == IORESOURCE_DMA) {
+		r = _get_sdma_req_by_name(oh, name, &dma);
+		if (r)
+			return r;
+
+		rsrc->start = dma;
+		rsrc->end = dma;
+	} else if (type == IORESOURCE_MEM) {
+		r = _get_addr_space_by_name(oh, name, &pa_start, &pa_end);
+		if (r)
+			return r;
+
+		rsrc->start = pa_start;
+		rsrc->end = pa_end;
+	} else {
+		return -EINVAL;
+	}
+
+	rsrc->flags = type;
+	rsrc->name = name;
+
+	return 0;
+}
+
+/**
  * omap_hwmod_get_pwrdm - return pointer to this module's main powerdomain
  * @oh: struct omap_hwmod *
  *
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 6470101..ff4d7f9 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -581,6 +581,8 @@ int omap_hwmod_softreset(struct omap_hwmod *oh);
 
 int omap_hwmod_count_resources(struct omap_hwmod *oh);
 int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res);
+int omap_hwmod_get_resource_byname(struct omap_hwmod *oh, unsigned int type,
+				   const char *name, struct resource *res);
 
 struct powerdomain *omap_hwmod_get_pwrdm(struct omap_hwmod *oh);
 void __iomem *omap_hwmod_get_mpu_rt_va(struct omap_hwmod *oh);

[PATCH v2 8/8] ARM: OMAP2+: timer: use a proper interface to get hwmod data

[Paul Walmsley]

arch/arm/mach-omap2/timer.c pokes around inside the hwmod data
structures.  Since the hwmod data structures are about to change, this
code will break.  This patch modifies the timer code to use
recently-added hwmod functions instead.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/timer.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 5c9acea..f073016 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -145,8 +145,10 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 {
 	char name[10]; /* 10 = sizeof("gptXX_Xck0") */
 	struct omap_hwmod *oh;
+	struct resource irq_rsrc, mem_rsrc;
 	size_t size;
 	int res = 0;
+	int r;
 
 	sprintf(name, "timer%d", gptimer_id);
 	omap_hwmod_setup_one(name);
@@ -154,9 +156,16 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	if (!oh)
 		return -ENODEV;
 
-	timer->irq = oh->mpu_irqs[0].irq;
-	timer->phys_base = oh->slaves[0]->addr->pa_start;
-	size = oh->slaves[0]->addr->pa_end - timer->phys_base;
+	r = omap_hwmod_get_resource_byname(oh, IORESOURCE_IRQ, NULL, &irq_rsrc);
+	if (r)
+		return -ENXIO;
+	timer->irq = irq_rsrc.start;
+
+	r = omap_hwmod_get_resource_byname(oh, IORESOURCE_MEM, NULL, &mem_rsrc);
+	if (r)
+		return -ENXIO;
+	timer->phys_base = mem_rsrc.start;
+	size = mem_rsrc.end - mem_rsrc.start;
 
 	/* Static mapping, never released */
 	timer->io_base = ioremap(timer->phys_base, size);

[PATCH v2 8/8] ARM: OMAP2+: timer: use a proper interface to get hwmod data

[Paul Walmsley]

arch/arm/mach-omap2/timer.c pokes around inside the hwmod data
structures.  Since the hwmod data structures are about to change, this
code will break.  This patch modifies the timer code to use
recently-added hwmod functions instead.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/timer.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 5c9acea..f073016 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -145,8 +145,10 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 {
 	char name[10]; /* 10 = sizeof("gptXX_Xck0") */
 	struct omap_hwmod *oh;
+	struct resource irq_rsrc, mem_rsrc;
 	size_t size;
 	int res = 0;
+	int r;
 
 	sprintf(name, "timer%d", gptimer_id);
 	omap_hwmod_setup_one(name);
@@ -154,9 +156,16 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	if (!oh)
 		return -ENODEV;
 
-	timer->irq = oh->mpu_irqs[0].irq;
-	timer->phys_base = oh->slaves[0]->addr->pa_start;
-	size = oh->slaves[0]->addr->pa_end - timer->phys_base;
+	r = omap_hwmod_get_resource_byname(oh, IORESOURCE_IRQ, NULL, &irq_rsrc);
+	if (r)
+		return -ENXIO;
+	timer->irq = irq_rsrc.start;
+
+	r = omap_hwmod_get_resource_byname(oh, IORESOURCE_MEM, NULL, &mem_rsrc);
+	if (r)
+		return -ENXIO;
+	timer->phys_base = mem_rsrc.start;
+	size = mem_rsrc.end - mem_rsrc.start;
 
 	/* Static mapping, never released */
 	timer->io_base = ioremap(timer->phys_base, size);

Re: [PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Ramirez Luna, Omar]

Hi Paul,

2012/2/27 Paul Walmsley <paul@pwsan.com>:
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index db4ad41..aeb6f4c 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -1490,13 +1490,22 @@ static int _reset(struct omap_hwmod *oh)
>        pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>
>        if (oh->class->reset)
> -               return oh->class->reset(oh);
> -
> -       if (!oh->rst_lines_cnt)
> -               return _ocp_softreset(oh);
> +               oh->class->reset(oh);
> +       else if (!oh->rst_lines_cnt)
> +               _ocp_softreset(oh);
> +       else
> +               for (i = 0; i < oh->rst_lines_cnt; i++)
> +                       _assert_hardreset(oh, oh->rst_lines[i].name);

If we reached here the reset lines will be asserted, and then the code
below touches sysc registers on a module under reset.

This would crash on _setup->_setup_reset->_reset.

Adding a 'return 0' I believe fixes the behavior, boots the board and
leaves the hwmod under reset.

-	else
+	} else {
 		for (i = 0; i < oh->rst_lines_cnt; i++)
 			_assert_hardreset(oh, oh->rst_lines[i].name);
+		return 0;
+	}

>
> -       for (i = 0; i < oh->rst_lines_cnt; i++)
> -               _assert_hardreset(oh, oh->rst_lines[i].name);
> +       /*
> +        * OCP_SYSCONFIG bits need to be reprogrammed after a
> +        * softreset.  The _enable() function should be split to avoid
> +        * the rewrite of the OCP_SYSCONFIG register.
> +        */
> +       if (oh->class->sysc) {
> +               _update_sysc_cache(oh);
> +               _enable_sysc(oh);
> +       }

Best Regards,

Omar

[PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Ramirez Luna, Omar]

Hi Paul,

2012/2/27 Paul Walmsley <paul@pwsan.com>:
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index db4ad41..aeb6f4c 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -1490,13 +1490,22 @@ static int _reset(struct omap_hwmod *oh)
> ? ? ? ?pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>
> ? ? ? ?if (oh->class->reset)
> - ? ? ? ? ? ? ? return oh->class->reset(oh);
> -
> - ? ? ? if (!oh->rst_lines_cnt)
> - ? ? ? ? ? ? ? return _ocp_softreset(oh);
> + ? ? ? ? ? ? ? oh->class->reset(oh);
> + ? ? ? else if (!oh->rst_lines_cnt)
> + ? ? ? ? ? ? ? _ocp_softreset(oh);
> + ? ? ? else
> + ? ? ? ? ? ? ? for (i = 0; i < oh->rst_lines_cnt; i++)
> + ? ? ? ? ? ? ? ? ? ? ? _assert_hardreset(oh, oh->rst_lines[i].name);

If we reached here the reset lines will be asserted, and then the code
below touches sysc registers on a module under reset.

This would crash on _setup->_setup_reset->_reset.

Adding a 'return 0' I believe fixes the behavior, boots the board and
leaves the hwmod under reset.

-	else
+	} else {
 		for (i = 0; i < oh->rst_lines_cnt; i++)
 			_assert_hardreset(oh, oh->rst_lines[i].name);
+		return 0;
+	}

>
> - ? ? ? for (i = 0; i < oh->rst_lines_cnt; i++)
> - ? ? ? ? ? ? ? _assert_hardreset(oh, oh->rst_lines[i].name);
> + ? ? ? /*
> + ? ? ? ?* OCP_SYSCONFIG bits need to be reprogrammed after a
> + ? ? ? ?* softreset. ?The _enable() function should be split to avoid
> + ? ? ? ?* the rewrite of the OCP_SYSCONFIG register.
> + ? ? ? ?*/
> + ? ? ? if (oh->class->sysc) {
> + ? ? ? ? ? ? ? _update_sysc_cache(oh);
> + ? ? ? ? ? ? ? _enable_sysc(oh);
> + ? ? ? }

Best Regards,

Omar

Re: [PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Paul Walmsley]

Hola Omar,

On Wed, 14 Mar 2012, Ramirez Luna, Omar wrote:

> If we reached here the reset lines will be asserted, and then the code
> below touches sysc registers on a module under reset.

Do you know of any case where this would be a problem?  Seems like it 
would only affect IP blocks that have both hard reset lines and SYSCONFIG 
registers, and as far as I know, we don't have any of those defined?


- Paul

[PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Paul Walmsley]

Hola Omar,

On Wed, 14 Mar 2012, Ramirez Luna, Omar wrote:

> If we reached here the reset lines will be asserted, and then the code
> below touches sysc registers on a module under reset.

Do you know of any case where this would be a problem?  Seems like it 
would only affect IP blocks that have both hard reset lines and SYSCONFIG 
registers, and as far as I know, we don't have any of those defined?


- Paul

Re: [PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Ramirez Luna, Omar]

On Thu, Mar 15, 2012 at 1:25 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Hola Omar,

Hola :)

> On Wed, 14 Mar 2012, Ramirez Luna, Omar wrote:
>
>> If we reached here the reset lines will be asserted, and then the code
>> below touches sysc registers on a module under reset.
>
> Do you know of any case where this would be a problem?  Seems like it
> would only affect IP blocks that have both hard reset lines and SYSCONFIG
> registers, and as far as I know, we don't have any of those defined?

MMU (not yet mainlined) has both, a reset line and a sysconfig.

I have been holding the hwmod for some time, but now that
rpmsg/remoteproc is going to mainline it could make use of it along
with omap3isp, however now I need to define functions to handle the
reset lines (although I was fine with hwmod handling it).

AFAIKnew, hwmod handling the reset line was fine (IMHO), the only 2 things were:
- For the drivers to somehow make use of shutdown/enable if they
needed they hwmod under reset and out.
- The annoying: hwmod XX failed to hardreset because of the wrong
reset sequence but causing no functional issues.

Regards,

Omar
--
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

[PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Ramirez Luna, Omar]

On Thu, Mar 15, 2012 at 1:25 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Hola Omar,

Hola :)

> On Wed, 14 Mar 2012, Ramirez Luna, Omar wrote:
>
>> If we reached here the reset lines will be asserted, and then the code
>> below touches sysc registers on a module under reset.
>
> Do you know of any case where this would be a problem? ?Seems like it
> would only affect IP blocks that have both hard reset lines and SYSCONFIG
> registers, and as far as I know, we don't have any of those defined?

MMU (not yet mainlined) has both, a reset line and a sysconfig.

I have been holding the hwmod for some time, but now that
rpmsg/remoteproc is going to mainline it could make use of it along
with omap3isp, however now I need to define functions to handle the
reset lines (although I was fine with hwmod handling it).

AFAIKnew, hwmod handling the reset line was fine (IMHO), the only 2 things were:
- For the drivers to somehow make use of shutdown/enable if they
needed they hwmod under reset and out.
- The annoying: hwmod XX failed to hardreset because of the wrong
reset sequence but causing no functional issues.

Regards,

Omar

Re: [PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4824 bytes --]

On Thu, 15 Mar 2012, Ramirez Luna, Omar wrote:

> On Thu, Mar 15, 2012 at 1:25 AM, Paul Walmsley <paul@pwsan.com> wrote:
> > On Wed, 14 Mar 2012, Ramirez Luna, Omar wrote:
> >
> >> If we reached here the reset lines will be asserted, and then the code
> >> below touches sysc registers on a module under reset.
> >
> > Do you know of any case where this would be a problem?  Seems like it
> > would only affect IP blocks that have both hard reset lines and SYSCONFIG
> > registers, and as far as I know, we don't have any of those defined?
> 
> MMU (not yet mainlined) has both, a reset line and a sysconfig.

Thanks, I've updated this patch with this fix, included below; rebased 
the series on v3.4-rc2 (the branch is now "hwmod_code_cleanup_3.5"); and 
will ask Tony to re-pull it.

> I have been holding the hwmod for some time, but now that
> rpmsg/remoteproc is going to mainline it could make use of it along
> with omap3isp, however now I need to define functions to handle the
> reset lines (although I was fine with hwmod handling it).

OK.

> AFAIKnew, hwmod handling the reset line was fine (IMHO), the only 2 things were:
> - For the drivers to somehow make use of shutdown/enable if they
> needed they hwmod under reset and out.
> - The annoying: hwmod XX failed to hardreset because of the wrong
> reset sequence but causing no functional issues.

Thanks for the feedback.  Apparently the OMAP4 DSP/ISS people had some 
issues with the old sequence, although I haven't heard anything from them 
directly, so it's unclear what exactly is going on with their code.
 
Anyway, please don't hesitate to let us know if you find any other issues 
with the updated behavior.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Mon, 27 Feb 2012 15:23:56 -0700
Subject: [PATCH] ARM: OMAP2+: hwmod: update OCP_SYSCONFIG registers after a
 custom reset

Since OCP_SYSCONFIG bits are cleared during reset, they should be
reprogrammed unless the IP block is being left in reset.  (The only IP
blocks that are left in reset are IP blocks with hardreset lines and
no custom reset function.)  If the IP block is left in reset, then it
is inaccessible to the MPU, and an access to the OCP_SYSCONFIG
register will cause an abort.

This version incorporates comments from Omar Ramirez Luna
<omar.ramirez@ti.com> to skip the OCP_SYSCONFIG access after asserting
hardreset lines.  This allows the MMU (IOMMU) IP block, which has
both hardreset lines and an OCP_SYSCONFIG register.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
Cc: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   39 ++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index fed805c..451e865 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1480,10 +1480,10 @@ dis_opt_clks:
  * IVA) have idiosyncratic reset sequences.  So for these relatively
  * rare cases, custom reset code can be supplied in the struct
  * omap_hwmod_class .reset function pointer.  Passes along the return
- * value from either _reset() or the custom reset function - these
- * must return -EINVAL if the hwmod cannot be reset this way or if the
- * hwmod is in the wrong state, -ETIMEDOUT if the module did not reset
- * in time, or 0 upon success.
+ * value from either _ocp_softreset() or the custom reset function -
+ * these must return -EINVAL if the hwmod cannot be reset this way or
+ * if the hwmod is in the wrong state, -ETIMEDOUT if the module did
+ * not reset in time, or 0 upon success.
  */
 static int _reset(struct omap_hwmod *oh)
 {
@@ -1491,22 +1491,29 @@ static int _reset(struct omap_hwmod *oh)
 
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
-	if (oh->class->reset)
-		return oh->class->reset(oh);
-
-	if (!oh->rst_lines_cnt) {
-		r = _ocp_softreset(oh);
-		if (oh->class->sysc) {
-			_update_sysc_cache(oh);
-			_enable_sysc(oh);
+	if (oh->class->reset) {
+		r = oh->class->reset(oh);
+	} else {
+		if (oh->rst_lines_cnt > 0) {
+			for (i = 0; i < oh->rst_lines_cnt; i++)
+				_assert_hardreset(oh, oh->rst_lines[i].name);
+			return 0;
+		} else {
+			r = _ocp_softreset(oh);
 		}
-		return r;
 	}
 
-	for (i = 0; i < oh->rst_lines_cnt; i++)
-		_assert_hardreset(oh, oh->rst_lines[i].name);
+	/*
+	 * OCP_SYSCONFIG bits need to be reprogrammed after a
+	 * softreset.  The _enable() function should be split to avoid
+	 * the rewrite of the OCP_SYSCONFIG register.
+	 */
+	if (oh->class->sysc) {
+		_update_sysc_cache(oh);
+		_enable_sysc(oh);
+	}
 
-	return 0;
+	return r;
 }
 
 /**
-- 
1.7.9.5

[PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Paul Walmsley]

On Thu, 15 Mar 2012, Ramirez Luna, Omar wrote:

> On Thu, Mar 15, 2012 at 1:25 AM, Paul Walmsley <paul@pwsan.com> wrote:
> > On Wed, 14 Mar 2012, Ramirez Luna, Omar wrote:
> >
> >> If we reached here the reset lines will be asserted, and then the code
> >> below touches sysc registers on a module under reset.
> >
> > Do you know of any case where this would be a problem? ?Seems like it
> > would only affect IP blocks that have both hard reset lines and SYSCONFIG
> > registers, and as far as I know, we don't have any of those defined?
> 
> MMU (not yet mainlined) has both, a reset line and a sysconfig.

Thanks, I've updated this patch with this fix, included below; rebased 
the series on v3.4-rc2 (the branch is now "hwmod_code_cleanup_3.5"); and 
will ask Tony to re-pull it.

> I have been holding the hwmod for some time, but now that
> rpmsg/remoteproc is going to mainline it could make use of it along
> with omap3isp, however now I need to define functions to handle the
> reset lines (although I was fine with hwmod handling it).

OK.

> AFAIKnew, hwmod handling the reset line was fine (IMHO), the only 2 things were:
> - For the drivers to somehow make use of shutdown/enable if they
> needed they hwmod under reset and out.
> - The annoying: hwmod XX failed to hardreset because of the wrong
> reset sequence but causing no functional issues.

Thanks for the feedback.  Apparently the OMAP4 DSP/ISS people had some 
issues with the old sequence, although I haven't heard anything from them 
directly, so it's unclear what exactly is going on with their code.
 
Anyway, please don't hesitate to let us know if you find any other issues 
with the updated behavior.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Mon, 27 Feb 2012 15:23:56 -0700
Subject: [PATCH] ARM: OMAP2+: hwmod: update OCP_SYSCONFIG registers after a
 custom reset

Since OCP_SYSCONFIG bits are cleared during reset, they should be
reprogrammed unless the IP block is being left in reset.  (The only IP
blocks that are left in reset are IP blocks with hardreset lines and
no custom reset function.)  If the IP block is left in reset, then it
is inaccessible to the MPU, and an access to the OCP_SYSCONFIG
register will cause an abort.

This version incorporates comments from Omar Ramirez Luna
<omar.ramirez@ti.com> to skip the OCP_SYSCONFIG access after asserting
hardreset lines.  This allows the MMU (IOMMU) IP block, which has
both hardreset lines and an OCP_SYSCONFIG register.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
Cc: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   39 ++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index fed805c..451e865 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1480,10 +1480,10 @@ dis_opt_clks:
  * IVA) have idiosyncratic reset sequences.  So for these relatively
  * rare cases, custom reset code can be supplied in the struct
  * omap_hwmod_class .reset function pointer.  Passes along the return
- * value from either _reset() or the custom reset function - these
- * must return -EINVAL if the hwmod cannot be reset this way or if the
- * hwmod is in the wrong state, -ETIMEDOUT if the module did not reset
- * in time, or 0 upon success.
+ * value from either _ocp_softreset() or the custom reset function -
+ * these must return -EINVAL if the hwmod cannot be reset this way or
+ * if the hwmod is in the wrong state, -ETIMEDOUT if the module did
+ * not reset in time, or 0 upon success.
  */
 static int _reset(struct omap_hwmod *oh)
 {
@@ -1491,22 +1491,29 @@ static int _reset(struct omap_hwmod *oh)
 
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
-	if (oh->class->reset)
-		return oh->class->reset(oh);
-
-	if (!oh->rst_lines_cnt) {
-		r = _ocp_softreset(oh);
-		if (oh->class->sysc) {
-			_update_sysc_cache(oh);
-			_enable_sysc(oh);
+	if (oh->class->reset) {
+		r = oh->class->reset(oh);
+	} else {
+		if (oh->rst_lines_cnt > 0) {
+			for (i = 0; i < oh->rst_lines_cnt; i++)
+				_assert_hardreset(oh, oh->rst_lines[i].name);
+			return 0;
+		} else {
+			r = _ocp_softreset(oh);
 		}
-		return r;
 	}
 
-	for (i = 0; i < oh->rst_lines_cnt; i++)
-		_assert_hardreset(oh, oh->rst_lines[i].name);
+	/*
+	 * OCP_SYSCONFIG bits need to be reprogrammed after a
+	 * softreset.  The _enable() function should be split to avoid
+	 * the rewrite of the OCP_SYSCONFIG register.
+	 */
+	if (oh->class->sysc) {
+		_update_sysc_cache(oh);
+		_enable_sysc(oh);
+	}
 
-	return 0;
+	return r;
 }
 
 /**
-- 
1.7.9.5

Re: [PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 12108 bytes --]

Hi Benoît,

On Mon, 27 Feb 2012, Paul Walmsley wrote:

> Change the way that hardreset lines are handled by the hwmod code.
> Hardreset lines are generally associated with initiator IP blocks.
> Prior to this change, the hwmod code expected to control hardreset
> lines itself, asserting them on shutdown and deasserting them upon
> enable.  But driver authors inside TI have commented to us that their
> drivers require direct control over these lines.  Unfortunately, these
> drivers haven't been posted publicly yet, so it's hard to determine
> exactly what is needed, a priori.  This change attempts to set forth
> some reasonable semantics that should be an improvement over the
> current code.

I took another look at this patch, and upon further thought, there are 
some aspects of this design that really don't make sense.

> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 5b368ee..db4ad41 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -805,6 +805,9 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
>  				    oh->clkdm->clkdm_offs,
>  				    oh->prcm.omap4.clkctrl_offs);
>  
> +	if (oh->rst_lines_cnt >= 0)
> +		return 0;

This change prevents any IP block from waiting for the target to disable 
-- which is not what we want.  The naïve fix would be to only skip the 
disable-wait if oh->rst_lines_cnt is greater than zero.  But if there are 
no hardreset lines asserted, then we should probably wait for the disable 
in that case.

>  	v = _omap4_wait_target_disable(oh);
>  	if (v)
>  		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",

[...]

> @@ -1575,7 +1568,19 @@ static int _enable(struct omap_hwmod *oh)
>  	_enable_clocks(oh);
>  	_enable_module(oh);
>  
> -	r = _wait_target_ready(oh);
> +	/*
> +	 * If an IP contains HW reset lines, we leave them
> +	 * asserted.  But this will block the module's idle state
> +	 * transition - the PRCM will return Intransition status.  So
> +	 * we need to avoid the target ready-wait in this case.  XXX
> +	 * We also need to give the drivers a way to wait for the
> +	 * target to become ready once they decide to deassert some
> +	 * hardreset lines.  XXX Is this strategy going to break PM
> +	 * because the clockdomain may not be able to enter idle while
> +	 * the module's idle status is in-transition?  We may just need
> +	 * custom reset blocks for all IPs with hardreset lines.
> +	 */
> +	r = (oh->rst_lines_cnt == 0) ? _wait_target_ready(oh) : 1;

And this part is at odds with the patch description.  If there are 
hardreset lines associated with an IP block, then this code will cause the 
following code to unconditionally disable the module clocks.  Considering 
that this is the _enable() function, this seems counterproductive.

I looked into changing this code to align with the original semantics we 
discussed.  It seems quite challenging.  With the current codebase, we'd 
have to bail out in the middle of the enable sequence.  Then we'd lose the 
clockdomain state (the 'hwsup' variable).

So I've updated the patch to essentially bail out early from all hwmod 
enable, idle, and shutdown code, if any hardreset lines associated with 
the IP block are asserted.  It will then be the driver integration code's 
responsibility for enabling the IP block when the hardreset lines are 
asserted.  When the hardreset lines are deasserted, the usual hwmod code 
will be executed -- I'd assume this would be the case during normal 
operation of the device.  I think this is probably the best we can do 
until we actually hear back from the people responsible for drivers for 
these special IP blocks.

A revised patch is below.  Care to take a look and see if it makes sense 
to you?


regards,

- Paul


From: Paul Walmsley <paul@pwsan.com>
Date: Wed, 18 Apr 2012 19:10:04 -0600
Subject: [PATCH] ARM: OMAP2+: hwmod: revise hardreset behavior

Change the way that hardreset lines are handled by the hwmod code.
Hardreset lines are generally associated with initiator IP blocks.
Prior to this change, the hwmod code expected to control hardreset
lines itself, asserting them on shutdown and deasserting them upon
enable.  But driver authors inside TI have commented to us that their
drivers require direct control over these lines.  Unfortunately, these
drivers haven't been posted publicly yet, so it's hard to determine
exactly what is needed, a priori.  This change attempts to set forth
some reasonable semantics that should be an improvement over the
current code.

The semantics implemented by this patch are as follows:

- If the hwmod is not marked with HWMOD_INIT_NO_RESET, then assert all
  associated hardreset lines during IP block setup.  This is intended
  to place the IP blocks into a known state that will not interfere
  with other devices during kernel boot.

- IP blocks with hardreset lines will not be automatically enabled or
  idled during setup.  Instead, they will be left in the INITIALIZED
  state.

- When the hwmod code is asked to enable, idle, or shutdown an IP
  block with asserted hardreset lines, the hwmod code will do nothing.
  The driver integration code must do the remaining work needed to
  control these IP blocks.  Once this driver integration code is posted
  to the lists, hopefully we can consolidate it and move it inside the
  hwmod code.

Custom reset functions for IP blocks with hardreset lines still should
be supported and are strongly endorsed.  It is intended that every
subsystem with hardreset lines should have a custom reset function
that can place their subsystem into quiescent idle with the hardreset
lines deasserted.

This reverts most of commit 5365efbe29250a227502256cc912351fe2157b42
("OMAP: hwmod: Add hardreset management support").  Later code
reorganizations caused the sequencing of the code from this patch to
be changed, anyway.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |  140 ++++++++++++++++++++++----------------
 1 file changed, 83 insertions(+), 57 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e6aa14f..a5c3a9a 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -781,39 +781,6 @@ static int _omap4_wait_target_disable(struct omap_hwmod *oh)
 }
 
 /**
- * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4
- * @oh: struct omap_hwmod *
- *
- * Disable the PRCM module mode related to the hwmod @oh.
- * Return EINVAL if the modulemode is not supported and 0 in case of success.
- */
-static int _omap4_disable_module(struct omap_hwmod *oh)
-{
-	int v;
-
-	/* The module mode does not exist prior OMAP4 */
-	if (!cpu_is_omap44xx())
-		return -EINVAL;
-
-	if (!oh->clkdm || !oh->prcm.omap4.modulemode)
-		return -EINVAL;
-
-	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
-
-	omap4_cminst_module_disable(oh->clkdm->prcm_partition,
-				    oh->clkdm->cm_inst,
-				    oh->clkdm->clkdm_offs,
-				    oh->prcm.omap4.clkctrl_offs);
-
-	v = _omap4_wait_target_disable(oh);
-	if (v)
-		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
-			oh->name);
-
-	return 0;
-}
-
-/**
  * _count_mpu_irqs - count the number of MPU IRQ lines associated with @oh
  * @oh: struct omap_hwmod *oh
  *
@@ -1378,6 +1345,66 @@ 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
+ * @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.
+ * This function is used to avoid executing some parts of the IP block
+ * enable/disable sequence if a hardreset line is set.
+ */
+static bool _are_any_hardreset_lines_asserted(struct omap_hwmod *oh)
+{
+	int i;
+
+	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;
+
+	return false;
+}
+
+/**
+ * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4
+ * @oh: struct omap_hwmod *
+ *
+ * Disable the PRCM module mode related to the hwmod @oh.
+ * Return EINVAL if the modulemode is not supported and 0 in case of success.
+ */
+static int _omap4_disable_module(struct omap_hwmod *oh)
+{
+	int v;
+
+	/* The module mode does not exist prior OMAP4 */
+	if (!cpu_is_omap44xx())
+		return -EINVAL;
+
+	if (!oh->clkdm || !oh->prcm.omap4.modulemode)
+		return -EINVAL;
+
+	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
+
+	omap4_cminst_module_disable(oh->clkdm->prcm_partition,
+				    oh->clkdm->cm_inst,
+				    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",
+			oh->name);
+
+	return 0;
+}
+
+/**
  * _ocp_softreset - reset an omap_hwmod via the OCP_SYSCONFIG bit
  * @oh: struct omap_hwmod *
  *
@@ -1519,7 +1546,7 @@ static int _reset(struct omap_hwmod *oh)
  */
 static int _enable(struct omap_hwmod *oh)
 {
-	int r, i;
+	int r;
 	int hwsup = 0;
 
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
@@ -1551,14 +1578,16 @@ static int _enable(struct omap_hwmod *oh)
 	}
 
 	/*
-	 * If an IP contains HW reset lines, then de-assert them in order
-	 * to allow the module state transition. Otherwise the PRCM will return
-	 * Intransition status, and the init will failed.
+	 * If an IP block contains HW reset lines and any 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
+	 * detailed information is provided and the driver code is
+	 * posted to the public lists, this is probably the best we
+	 * can do.
 	 */
-	if (oh->_state == _HWMOD_STATE_INITIALIZED ||
-	    oh->_state == _HWMOD_STATE_DISABLED)
-		for (i = 0; i < oh->rst_lines_cnt; i++)
-			_deassert_hardreset(oh, oh->rst_lines[i].name);
+	if (_are_any_hardreset_lines_asserted(oh))
+		return 0;
 
 	/* Mux pins for device runtime if populated */
 	if (oh->mux && (!oh->mux->enabled ||
@@ -1633,6 +1662,9 @@ static int _idle(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
+	if (_are_any_hardreset_lines_asserted(oh))
+		return 0;
+
 	if (oh->class->sysc)
 		_idle_sysc(oh);
 	_del_initiator_dep(oh, mpu_oh);
@@ -1715,6 +1747,9 @@ static int _shutdown(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
+	if (_are_any_hardreset_lines_asserted(oh))
+		return 0;
+
 	pr_debug("omap_hwmod: %s: disabling\n", oh->name);
 
 	if (oh->class->pre_shutdown) {
@@ -1824,27 +1859,18 @@ static int __init _setup_reset(struct omap_hwmod *oh)
 	if (oh->_state != _HWMOD_STATE_INITIALIZED)
 		return -EINVAL;
 
-	/*
-	 * In the case of hwmod with hardreset that should not be
-	 * de-assert at boot time, we have to keep the module
-	 * initialized, because we cannot enable it properly with the
-	 * reset asserted. Exit without warning because that behavior
-	 * is expected.
-	 */
-	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
-		return 0;
-
-	r = _enable(oh);
-	if (r) {
-		pr_warning("omap_hwmod: %s: cannot be enabled (%d)\n",
-			   oh->name, oh->_state);
-		return 0;
+	if (oh->rst_lines_cnt == 0) {
+		r = _enable(oh);
+		if (r) {
+			pr_warning("omap_hwmod: %s: cannot be enabled for reset (%d)\n",
+				   oh->name, oh->_state);
+			return -EINVAL;
+		}
 	}
 
 	if (!(oh->flags & HWMOD_INIT_NO_RESET))
 		r = _reset(oh);
 
-
 	return r;
 }
 /**
-- 
1.7.10

[PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Paul Walmsley]

Hi Beno?t,

On Mon, 27 Feb 2012, Paul Walmsley wrote:

> Change the way that hardreset lines are handled by the hwmod code.
> Hardreset lines are generally associated with initiator IP blocks.
> Prior to this change, the hwmod code expected to control hardreset
> lines itself, asserting them on shutdown and deasserting them upon
> enable.  But driver authors inside TI have commented to us that their
> drivers require direct control over these lines.  Unfortunately, these
> drivers haven't been posted publicly yet, so it's hard to determine
> exactly what is needed, a priori.  This change attempts to set forth
> some reasonable semantics that should be an improvement over the
> current code.

I took another look at this patch, and upon further thought, there are 
some aspects of this design that really don't make sense.

> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 5b368ee..db4ad41 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -805,6 +805,9 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
>  				    oh->clkdm->clkdm_offs,
>  				    oh->prcm.omap4.clkctrl_offs);
>  
> +	if (oh->rst_lines_cnt >= 0)
> +		return 0;

This change prevents any IP block from waiting for the target to disable 
-- which is not what we want.  The na?ve fix would be to only skip the 
disable-wait if oh->rst_lines_cnt is greater than zero.  But if there are 
no hardreset lines asserted, then we should probably wait for the disable 
in that case.

>  	v = _omap4_wait_target_disable(oh);
>  	if (v)
>  		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",

[...]

> @@ -1575,7 +1568,19 @@ static int _enable(struct omap_hwmod *oh)
>  	_enable_clocks(oh);
>  	_enable_module(oh);
>  
> -	r = _wait_target_ready(oh);
> +	/*
> +	 * If an IP contains HW reset lines, we leave them
> +	 * asserted.  But this will block the module's idle state
> +	 * transition - the PRCM will return Intransition status.  So
> +	 * we need to avoid the target ready-wait in this case.  XXX
> +	 * We also need to give the drivers a way to wait for the
> +	 * target to become ready once they decide to deassert some
> +	 * hardreset lines.  XXX Is this strategy going to break PM
> +	 * because the clockdomain may not be able to enter idle while
> +	 * the module's idle status is in-transition?  We may just need
> +	 * custom reset blocks for all IPs with hardreset lines.
> +	 */
> +	r = (oh->rst_lines_cnt == 0) ? _wait_target_ready(oh) : 1;

And this part is at odds with the patch description.  If there are 
hardreset lines associated with an IP block, then this code will cause the 
following code to unconditionally disable the module clocks.  Considering 
that this is the _enable() function, this seems counterproductive.

I looked into changing this code to align with the original semantics we 
discussed.  It seems quite challenging.  With the current codebase, we'd 
have to bail out in the middle of the enable sequence.  Then we'd lose the 
clockdomain state (the 'hwsup' variable).

So I've updated the patch to essentially bail out early from all hwmod 
enable, idle, and shutdown code, if any hardreset lines associated with 
the IP block are asserted.  It will then be the driver integration code's 
responsibility for enabling the IP block when the hardreset lines are 
asserted.  When the hardreset lines are deasserted, the usual hwmod code 
will be executed -- I'd assume this would be the case during normal 
operation of the device.  I think this is probably the best we can do 
until we actually hear back from the people responsible for drivers for 
these special IP blocks.

A revised patch is below.  Care to take a look and see if it makes sense 
to you?


regards,

- Paul


From: Paul Walmsley <paul@pwsan.com>
Date: Wed, 18 Apr 2012 19:10:04 -0600
Subject: [PATCH] ARM: OMAP2+: hwmod: revise hardreset behavior

Change the way that hardreset lines are handled by the hwmod code.
Hardreset lines are generally associated with initiator IP blocks.
Prior to this change, the hwmod code expected to control hardreset
lines itself, asserting them on shutdown and deasserting them upon
enable.  But driver authors inside TI have commented to us that their
drivers require direct control over these lines.  Unfortunately, these
drivers haven't been posted publicly yet, so it's hard to determine
exactly what is needed, a priori.  This change attempts to set forth
some reasonable semantics that should be an improvement over the
current code.

The semantics implemented by this patch are as follows:

- If the hwmod is not marked with HWMOD_INIT_NO_RESET, then assert all
  associated hardreset lines during IP block setup.  This is intended
  to place the IP blocks into a known state that will not interfere
  with other devices during kernel boot.

- IP blocks with hardreset lines will not be automatically enabled or
  idled during setup.  Instead, they will be left in the INITIALIZED
  state.

- When the hwmod code is asked to enable, idle, or shutdown an IP
  block with asserted hardreset lines, the hwmod code will do nothing.
  The driver integration code must do the remaining work needed to
  control these IP blocks.  Once this driver integration code is posted
  to the lists, hopefully we can consolidate it and move it inside the
  hwmod code.

Custom reset functions for IP blocks with hardreset lines still should
be supported and are strongly endorsed.  It is intended that every
subsystem with hardreset lines should have a custom reset function
that can place their subsystem into quiescent idle with the hardreset
lines deasserted.

This reverts most of commit 5365efbe29250a227502256cc912351fe2157b42
("OMAP: hwmod: Add hardreset management support").  Later code
reorganizations caused the sequencing of the code from this patch to
be changed, anyway.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |  140 ++++++++++++++++++++++----------------
 1 file changed, 83 insertions(+), 57 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e6aa14f..a5c3a9a 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -781,39 +781,6 @@ static int _omap4_wait_target_disable(struct omap_hwmod *oh)
 }
 
 /**
- * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4
- * @oh: struct omap_hwmod *
- *
- * Disable the PRCM module mode related to the hwmod @oh.
- * Return EINVAL if the modulemode is not supported and 0 in case of success.
- */
-static int _omap4_disable_module(struct omap_hwmod *oh)
-{
-	int v;
-
-	/* The module mode does not exist prior OMAP4 */
-	if (!cpu_is_omap44xx())
-		return -EINVAL;
-
-	if (!oh->clkdm || !oh->prcm.omap4.modulemode)
-		return -EINVAL;
-
-	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
-
-	omap4_cminst_module_disable(oh->clkdm->prcm_partition,
-				    oh->clkdm->cm_inst,
-				    oh->clkdm->clkdm_offs,
-				    oh->prcm.omap4.clkctrl_offs);
-
-	v = _omap4_wait_target_disable(oh);
-	if (v)
-		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
-			oh->name);
-
-	return 0;
-}
-
-/**
  * _count_mpu_irqs - count the number of MPU IRQ lines associated with @oh
  * @oh: struct omap_hwmod *oh
  *
@@ -1378,6 +1345,66 @@ 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
+ * @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.
+ * This function is used to avoid executing some parts of the IP block
+ * enable/disable sequence if a hardreset line is set.
+ */
+static bool _are_any_hardreset_lines_asserted(struct omap_hwmod *oh)
+{
+	int i;
+
+	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;
+
+	return false;
+}
+
+/**
+ * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4
+ * @oh: struct omap_hwmod *
+ *
+ * Disable the PRCM module mode related to the hwmod @oh.
+ * Return EINVAL if the modulemode is not supported and 0 in case of success.
+ */
+static int _omap4_disable_module(struct omap_hwmod *oh)
+{
+	int v;
+
+	/* The module mode does not exist prior OMAP4 */
+	if (!cpu_is_omap44xx())
+		return -EINVAL;
+
+	if (!oh->clkdm || !oh->prcm.omap4.modulemode)
+		return -EINVAL;
+
+	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
+
+	omap4_cminst_module_disable(oh->clkdm->prcm_partition,
+				    oh->clkdm->cm_inst,
+				    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",
+			oh->name);
+
+	return 0;
+}
+
+/**
  * _ocp_softreset - reset an omap_hwmod via the OCP_SYSCONFIG bit
  * @oh: struct omap_hwmod *
  *
@@ -1519,7 +1546,7 @@ static int _reset(struct omap_hwmod *oh)
  */
 static int _enable(struct omap_hwmod *oh)
 {
-	int r, i;
+	int r;
 	int hwsup = 0;
 
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
@@ -1551,14 +1578,16 @@ static int _enable(struct omap_hwmod *oh)
 	}
 
 	/*
-	 * If an IP contains HW reset lines, then de-assert them in order
-	 * to allow the module state transition. Otherwise the PRCM will return
-	 * Intransition status, and the init will failed.
+	 * If an IP block contains HW reset lines and any 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
+	 * detailed information is provided and the driver code is
+	 * posted to the public lists, this is probably the best we
+	 * can do.
 	 */
-	if (oh->_state == _HWMOD_STATE_INITIALIZED ||
-	    oh->_state == _HWMOD_STATE_DISABLED)
-		for (i = 0; i < oh->rst_lines_cnt; i++)
-			_deassert_hardreset(oh, oh->rst_lines[i].name);
+	if (_are_any_hardreset_lines_asserted(oh))
+		return 0;
 
 	/* Mux pins for device runtime if populated */
 	if (oh->mux && (!oh->mux->enabled ||
@@ -1633,6 +1662,9 @@ static int _idle(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
+	if (_are_any_hardreset_lines_asserted(oh))
+		return 0;
+
 	if (oh->class->sysc)
 		_idle_sysc(oh);
 	_del_initiator_dep(oh, mpu_oh);
@@ -1715,6 +1747,9 @@ static int _shutdown(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
+	if (_are_any_hardreset_lines_asserted(oh))
+		return 0;
+
 	pr_debug("omap_hwmod: %s: disabling\n", oh->name);
 
 	if (oh->class->pre_shutdown) {
@@ -1824,27 +1859,18 @@ static int __init _setup_reset(struct omap_hwmod *oh)
 	if (oh->_state != _HWMOD_STATE_INITIALIZED)
 		return -EINVAL;
 
-	/*
-	 * In the case of hwmod with hardreset that should not be
-	 * de-assert at boot time, we have to keep the module
-	 * initialized, because we cannot enable it properly with the
-	 * reset asserted. Exit without warning because that behavior
-	 * is expected.
-	 */
-	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
-		return 0;
-
-	r = _enable(oh);
-	if (r) {
-		pr_warning("omap_hwmod: %s: cannot be enabled (%d)\n",
-			   oh->name, oh->_state);
-		return 0;
+	if (oh->rst_lines_cnt == 0) {
+		r = _enable(oh);
+		if (r) {
+			pr_warning("omap_hwmod: %s: cannot be enabled for reset (%d)\n",
+				   oh->name, oh->_state);
+			return -EINVAL;
+		}
 	}
 
 	if (!(oh->flags & HWMOD_INIT_NO_RESET))
 		r = _reset(oh);
 
-
 	return r;
 }
 /**
-- 
1.7.10

Re: [PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 8993 bytes --]

Hi

On Mon, 27 Feb 2012, Paul Walmsley wrote:

> Reorganize the code involved in initializing and configuring an IP
> block to make it easier to read and maintain.  This involves improving
> documentation, splitting some large functions up into smaller ones to
> better conform with Documentation/CodingStyle, and removing some
> unnecessary code.
> 
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Benoît Cousson <b-cousson@ti.com>

A few changes have been made to this patch:

- The reset changes have been consolidated with the changes in patch 5.

- The changes to the _init function have been split into a separate patch
  to make it easier to debug.

The updated patch follows.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Wed, 18 Apr 2012 19:10:03 -0600
Subject: [PATCH 3/3] ARM: OMAP2+: hwmod: reorganize and document the reset
 and configuration process

Reorganize the code involved in resetting and configuring an IP block
to make it easier to read and maintain.  This involves improving
documentation, splitting some large functions up into smaller ones to
better conform with Documentation/CodingStyle, and removing some
unnecessary code.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |  152 ++++++++++++++++++++++++++++++--------
 1 file changed, 120 insertions(+), 32 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 253e942..0afbebc 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1534,10 +1534,9 @@ static int _enable(struct omap_hwmod *oh)
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
 
 	/*
-	 * hwmods with HWMOD_INIT_NO_IDLE flag set are left
-	 * in enabled state at init.
-	 * Now that someone is really trying to enable them,
-	 * just ensure that the hwmod mux is set.
+	 * hwmods with HWMOD_INIT_NO_IDLE flag set are left in enabled
+	 * state at init.  Now that someone is really trying to enable
+	 * them, just ensure that the hwmod mux is set.
 	 */
 	if (oh->_int_flags & _HWMOD_SKIP_ENABLE) {
 		/*
@@ -1819,46 +1818,60 @@ static int __init _init(struct omap_hwmod *oh, void *n)
 }
 
 /**
- * _setup - do initial configuration of omap_hwmod
+ * _setup_iclk_autoidle - configure an IP block's interface clocks
  * @oh: struct omap_hwmod *
  *
- * Writes the CLOCKACTIVITY bits @clockact to the hwmod @oh
- * OCP_SYSCONFIG register.  Returns 0.
+ * Set up the module's interface clocks.  XXX This function is still mostly
+ * a stub; implementing this properly requires iclk autoidle usecounting in
+ * the clock code.   No return value.
  */
-static int _setup(struct omap_hwmod *oh, void *data)
+static void __init _setup_iclk_autoidle(struct omap_hwmod *oh)
 {
-	int i, r;
-	u8 postsetup_state;
+	int i;
 
 	if (oh->_state != _HWMOD_STATE_INITIALIZED)
-		return 0;
+		return;
 
-	/* Set iclk autoidle mode */
-	if (oh->slaves_cnt > 0) {
-		for (i = 0; i < oh->slaves_cnt; i++) {
-			struct omap_hwmod_ocp_if *os = oh->slaves[i];
-			struct clk *c = os->_clk;
+	for (i = 0; i < oh->slaves_cnt; i++) {
+		struct omap_hwmod_ocp_if *os = oh->slaves[i];
+		struct clk *c = os->_clk;
 
-			if (!c)
-				continue;
+		if (!c)
+			continue;
 
-			if (os->flags & OCPIF_SWSUP_IDLE) {
-				/* XXX omap_iclk_deny_idle(c); */
-			} else {
-				/* XXX omap_iclk_allow_idle(c); */
-				clk_enable(c);
-			}
+		if (os->flags & OCPIF_SWSUP_IDLE) {
+			/* XXX omap_iclk_deny_idle(c); */
+		} else {
+			/* XXX omap_iclk_allow_idle(c); */
+			clk_enable(c);
 		}
 	}
 
-	oh->_state = _HWMOD_STATE_INITIALIZED;
+	return;
+}
+
+/**
+ * _setup_reset - reset an IP block during the setup process
+ * @oh: struct omap_hwmod *
+ *
+ * Reset the IP block corresponding to the hwmod @oh during the setup
+ * process.  The IP block is first enabled so it can be successfully
+ * reset.  Returns 0 upon success or a negative error code upon
+ * failure.
+ */
+static int __init _setup_reset(struct omap_hwmod *oh)
+{
+	int r;
+
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return -EINVAL;
 
 	/*
 	 * In the case of hwmod with hardreset that should not be
 	 * de-assert at boot time, we have to keep the module
 	 * initialized, because we cannot enable it properly with the
-	 * reset asserted. Exit without warning because that behavior is
-	 * expected.
+	 * reset asserted. Exit without warning because that behavior
+	 * is expected.
 	 */
 	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
 		return 0;
@@ -1871,7 +1884,53 @@ static int _setup(struct omap_hwmod *oh, void *data)
 	}
 
 	if (!(oh->flags & HWMOD_INIT_NO_RESET))
-		_reset(oh);
+		r = _reset(oh);
+
+	return r;
+}
+
+/**
+ * _setup_postsetup - transition to the appropriate state after _setup
+ * @oh: struct omap_hwmod *
+ *
+ * Place an IP block represented by @oh into a "post-setup" state --
+ * either IDLE, ENABLED, or DISABLED.  ("post-setup" simply means that
+ * this function is called at the end of _setup().)  The postsetup
+ * state for an IP block can be changed by calling
+ * omap_hwmod_enter_postsetup_state() early in the boot process,
+ * before one of the omap_hwmod_setup*() functions are called for the
+ * IP block.
+ *
+ * The IP block stays in this state until a PM runtime-based driver is
+ * loaded for that IP block.  A post-setup state of IDLE is
+ * appropriate for almost all IP blocks with runtime PM-enabled
+ * drivers, since those drivers are able to enable the IP block.  A
+ * post-setup state of ENABLED is appropriate for kernels with PM
+ * runtime disabled.  The DISABLED state is appropriate for unusual IP
+ * blocks such as the MPU WDTIMER on kernels without WDTIMER drivers
+ * included, since the WDTIMER starts running on reset and will reset
+ * the MPU if left active.
+ *
+ * This post-setup mechanism is deprecated.  Once all of the OMAP
+ * drivers have been converted to use PM runtime, and all of the IP
+ * block data and interconnect data is available to the hwmod code, it
+ * should be possible to replace this mechanism with a "lazy reset"
+ * arrangement.  In a "lazy reset" setup, each IP block is enabled
+ * when the driver first probes, then all remaining IP blocks without
+ * drivers are either shut down or enabled after the drivers have
+ * loaded.  However, this cannot take place until the above
+ * preconditions have been met, since otherwise the late reset code
+ * has no way of knowing which IP blocks are in use by drivers, and
+ * which ones are unused.
+ *
+ * No return value.
+ */
+static void __init _setup_postsetup(struct omap_hwmod *oh)
+{
+	u8 postsetup_state;
+
+	if (oh->rst_lines_cnt > 0)
+		return;
 
 	postsetup_state = oh->_postsetup_state;
 	if (postsetup_state == _HWMOD_STATE_UNKNOWN)
@@ -1895,6 +1954,35 @@ static int _setup(struct omap_hwmod *oh, void *data)
 		WARN(1, "hwmod: %s: unknown postsetup state %d! defaulting to enabled\n",
 		     oh->name, postsetup_state);
 
+	return;
+}
+
+/**
+ * _setup - prepare IP block hardware for use
+ * @oh: struct omap_hwmod *
+ * @n: (unused, pass NULL)
+ *
+ * Configure the IP block represented by @oh.  This may include
+ * enabling the IP block, resetting it, and placing it into a
+ * post-setup state, depending on the type of IP block and applicable
+ * flags.  IP blocks are reset to prevent any previous configuration
+ * by the bootloader or previous operating system from interfering
+ * with power management or other parts of the system.  The reset can
+ * be avoided; see omap_hwmod_no_setup_reset().  This is the second of
+ * two phases for hwmod initialization.  Code called here generally
+ * affects the IP block hardware, or system integration hardware
+ * associated with the IP block.  Returns 0.
+ */
+static int __init _setup(struct omap_hwmod *oh, void *n)
+{
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return 0;
+
+	_setup_iclk_autoidle(oh);
+
+	if (!_setup_reset(oh))
+		_setup_postsetup(oh);
+
 	return 0;
 }
 
@@ -2700,10 +2788,10 @@ int omap_hwmod_for_each_by_class(const char *classname,
  * @state: state that _setup() should leave the hwmod in
  *
  * Sets the hwmod state that @oh will enter at the end of _setup()
- * (called by omap_hwmod_setup_*()).  Only valid to call between
- * calling omap_hwmod_register() and omap_hwmod_setup_*().  Returns
- * 0 upon success or -EINVAL if there is a problem with the arguments
- * or if the hwmod is in the wrong state.
+ * (called by omap_hwmod_setup_*()).  See also the documentation
+ * for _setup_postsetup(), above.  Returns 0 upon success or
+ * -EINVAL if there is a problem with the arguments or if the hwmod is
+ * in the wrong state.
  */
 int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state)
 {
-- 
1.7.10

[PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Paul Walmsley]

Hi

On Mon, 27 Feb 2012, Paul Walmsley wrote:

> Reorganize the code involved in initializing and configuring an IP
> block to make it easier to read and maintain.  This involves improving
> documentation, splitting some large functions up into smaller ones to
> better conform with Documentation/CodingStyle, and removing some
> unnecessary code.
> 
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Beno?t Cousson <b-cousson@ti.com>

A few changes have been made to this patch:

- The reset changes have been consolidated with the changes in patch 5.

- The changes to the _init function have been split into a separate patch
  to make it easier to debug.

The updated patch follows.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Wed, 18 Apr 2012 19:10:03 -0600
Subject: [PATCH 3/3] ARM: OMAP2+: hwmod: reorganize and document the reset
 and configuration process

Reorganize the code involved in resetting and configuring an IP block
to make it easier to read and maintain.  This involves improving
documentation, splitting some large functions up into smaller ones to
better conform with Documentation/CodingStyle, and removing some
unnecessary code.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |  152 ++++++++++++++++++++++++++++++--------
 1 file changed, 120 insertions(+), 32 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 253e942..0afbebc 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1534,10 +1534,9 @@ static int _enable(struct omap_hwmod *oh)
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
 
 	/*
-	 * hwmods with HWMOD_INIT_NO_IDLE flag set are left
-	 * in enabled state at init.
-	 * Now that someone is really trying to enable them,
-	 * just ensure that the hwmod mux is set.
+	 * hwmods with HWMOD_INIT_NO_IDLE flag set are left in enabled
+	 * state at init.  Now that someone is really trying to enable
+	 * them, just ensure that the hwmod mux is set.
 	 */
 	if (oh->_int_flags & _HWMOD_SKIP_ENABLE) {
 		/*
@@ -1819,46 +1818,60 @@ static int __init _init(struct omap_hwmod *oh, void *n)
 }
 
 /**
- * _setup - do initial configuration of omap_hwmod
+ * _setup_iclk_autoidle - configure an IP block's interface clocks
  * @oh: struct omap_hwmod *
  *
- * Writes the CLOCKACTIVITY bits @clockact to the hwmod @oh
- * OCP_SYSCONFIG register.  Returns 0.
+ * Set up the module's interface clocks.  XXX This function is still mostly
+ * a stub; implementing this properly requires iclk autoidle usecounting in
+ * the clock code.   No return value.
  */
-static int _setup(struct omap_hwmod *oh, void *data)
+static void __init _setup_iclk_autoidle(struct omap_hwmod *oh)
 {
-	int i, r;
-	u8 postsetup_state;
+	int i;
 
 	if (oh->_state != _HWMOD_STATE_INITIALIZED)
-		return 0;
+		return;
 
-	/* Set iclk autoidle mode */
-	if (oh->slaves_cnt > 0) {
-		for (i = 0; i < oh->slaves_cnt; i++) {
-			struct omap_hwmod_ocp_if *os = oh->slaves[i];
-			struct clk *c = os->_clk;
+	for (i = 0; i < oh->slaves_cnt; i++) {
+		struct omap_hwmod_ocp_if *os = oh->slaves[i];
+		struct clk *c = os->_clk;
 
-			if (!c)
-				continue;
+		if (!c)
+			continue;
 
-			if (os->flags & OCPIF_SWSUP_IDLE) {
-				/* XXX omap_iclk_deny_idle(c); */
-			} else {
-				/* XXX omap_iclk_allow_idle(c); */
-				clk_enable(c);
-			}
+		if (os->flags & OCPIF_SWSUP_IDLE) {
+			/* XXX omap_iclk_deny_idle(c); */
+		} else {
+			/* XXX omap_iclk_allow_idle(c); */
+			clk_enable(c);
 		}
 	}
 
-	oh->_state = _HWMOD_STATE_INITIALIZED;
+	return;
+}
+
+/**
+ * _setup_reset - reset an IP block during the setup process
+ * @oh: struct omap_hwmod *
+ *
+ * Reset the IP block corresponding to the hwmod @oh during the setup
+ * process.  The IP block is first enabled so it can be successfully
+ * reset.  Returns 0 upon success or a negative error code upon
+ * failure.
+ */
+static int __init _setup_reset(struct omap_hwmod *oh)
+{
+	int r;
+
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return -EINVAL;
 
 	/*
 	 * In the case of hwmod with hardreset that should not be
 	 * de-assert at boot time, we have to keep the module
 	 * initialized, because we cannot enable it properly with the
-	 * reset asserted. Exit without warning because that behavior is
-	 * expected.
+	 * reset asserted. Exit without warning because that behavior
+	 * is expected.
 	 */
 	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt > 0)
 		return 0;
@@ -1871,7 +1884,53 @@ static int _setup(struct omap_hwmod *oh, void *data)
 	}
 
 	if (!(oh->flags & HWMOD_INIT_NO_RESET))
-		_reset(oh);
+		r = _reset(oh);
+
+	return r;
+}
+
+/**
+ * _setup_postsetup - transition to the appropriate state after _setup
+ * @oh: struct omap_hwmod *
+ *
+ * Place an IP block represented by @oh into a "post-setup" state --
+ * either IDLE, ENABLED, or DISABLED.  ("post-setup" simply means that
+ * this function is called at the end of _setup().)  The postsetup
+ * state for an IP block can be changed by calling
+ * omap_hwmod_enter_postsetup_state() early in the boot process,
+ * before one of the omap_hwmod_setup*() functions are called for the
+ * IP block.
+ *
+ * The IP block stays in this state until a PM runtime-based driver is
+ * loaded for that IP block.  A post-setup state of IDLE is
+ * appropriate for almost all IP blocks with runtime PM-enabled
+ * drivers, since those drivers are able to enable the IP block.  A
+ * post-setup state of ENABLED is appropriate for kernels with PM
+ * runtime disabled.  The DISABLED state is appropriate for unusual IP
+ * blocks such as the MPU WDTIMER on kernels without WDTIMER drivers
+ * included, since the WDTIMER starts running on reset and will reset
+ * the MPU if left active.
+ *
+ * This post-setup mechanism is deprecated.  Once all of the OMAP
+ * drivers have been converted to use PM runtime, and all of the IP
+ * block data and interconnect data is available to the hwmod code, it
+ * should be possible to replace this mechanism with a "lazy reset"
+ * arrangement.  In a "lazy reset" setup, each IP block is enabled
+ * when the driver first probes, then all remaining IP blocks without
+ * drivers are either shut down or enabled after the drivers have
+ * loaded.  However, this cannot take place until the above
+ * preconditions have been met, since otherwise the late reset code
+ * has no way of knowing which IP blocks are in use by drivers, and
+ * which ones are unused.
+ *
+ * No return value.
+ */
+static void __init _setup_postsetup(struct omap_hwmod *oh)
+{
+	u8 postsetup_state;
+
+	if (oh->rst_lines_cnt > 0)
+		return;
 
 	postsetup_state = oh->_postsetup_state;
 	if (postsetup_state == _HWMOD_STATE_UNKNOWN)
@@ -1895,6 +1954,35 @@ static int _setup(struct omap_hwmod *oh, void *data)
 		WARN(1, "hwmod: %s: unknown postsetup state %d! defaulting to enabled\n",
 		     oh->name, postsetup_state);
 
+	return;
+}
+
+/**
+ * _setup - prepare IP block hardware for use
+ * @oh: struct omap_hwmod *
+ * @n: (unused, pass NULL)
+ *
+ * Configure the IP block represented by @oh.  This may include
+ * enabling the IP block, resetting it, and placing it into a
+ * post-setup state, depending on the type of IP block and applicable
+ * flags.  IP blocks are reset to prevent any previous configuration
+ * by the bootloader or previous operating system from interfering
+ * with power management or other parts of the system.  The reset can
+ * be avoided; see omap_hwmod_no_setup_reset().  This is the second of
+ * two phases for hwmod initialization.  Code called here generally
+ * affects the IP block hardware, or system integration hardware
+ * associated with the IP block.  Returns 0.
+ */
+static int __init _setup(struct omap_hwmod *oh, void *n)
+{
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
+		return 0;
+
+	_setup_iclk_autoidle(oh);
+
+	if (!_setup_reset(oh))
+		_setup_postsetup(oh);
+
 	return 0;
 }
 
@@ -2700,10 +2788,10 @@ int omap_hwmod_for_each_by_class(const char *classname,
  * @state: state that _setup() should leave the hwmod in
  *
  * Sets the hwmod state that @oh will enter at the end of _setup()
- * (called by omap_hwmod_setup_*()).  Only valid to call between
- * calling omap_hwmod_register() and omap_hwmod_setup_*().  Returns
- * 0 upon success or -EINVAL if there is a problem with the arguments
- * or if the hwmod is in the wrong state.
+ * (called by omap_hwmod_setup_*()).  See also the documentation
+ * for _setup_postsetup(), above.  Returns 0 upon success or
+ * -EINVAL if there is a problem with the arguments or if the hwmod is
+ * in the wrong state.
  */
 int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state)
 {
-- 
1.7.10

Re: [PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 7508 bytes --]

Hi

On Thu, 19 Apr 2012, Paul Walmsley wrote:

> - The changes to the _init function have been split into a separate patch
>   to make it easier to debug.

Here is the patch that deals specifically with _init().  It's the same 
code that was originally part of the patch mentioned in the subject line, 
split into a new patch for clarity.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Thu, 19 Apr 2012 00:58:22 -0600
Subject: [PATCH 2/3] ARM: OMAP2+: hwmod: reorganize and document the
 initialization process

Reorganize the code involved in initializing the internal data for
each hwmod to make it easier to read and maintain.  This involves
improving documentation and removing some duplicated and unnecessary
code.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |  137 ++++++++++++++++++++++----------------
 1 file changed, 80 insertions(+), 57 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5b07ad0..253e942 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1769,6 +1769,56 @@ static int _shutdown(struct omap_hwmod *oh)
 }
 
 /**
+ * _init_mpu_rt_base - populate the virtual address for a hwmod
+ * @oh: struct omap_hwmod * to locate the virtual address
+ *
+ * Cache the virtual address used by the MPU to access this IP block's
+ * registers.  This address is needed early so the OCP registers that
+ * are part of the device's address space can be ioremapped properly.
+ * No return value.
+ */
+static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
+{
+	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
+		return;
+
+	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
+}
+
+/**
+ * _init - initialize internal data for the hwmod @oh
+ * @oh: struct omap_hwmod *
+ * @n: (unused)
+ *
+ * Look up the clocks and the address space used by the MPU to access
+ * registers belonging to the hwmod @oh.  @oh must already be
+ * registered at this point.  This is the first of two phases for
+ * hwmod initialization.  Code called here does not touch any hardware
+ * registers, it simply prepares internal data structures.  Returns 0
+ * upon success or if the hwmod isn't registered, or -EINVAL upon
+ * failure.
+ */
+static int __init _init(struct omap_hwmod *oh, void *n)
+{
+	int r;
+
+	if (oh->_state != _HWMOD_STATE_REGISTERED)
+		return 0;
+
+	_init_mpu_rt_base(oh, NULL);
+
+	r = _init_clocks(oh, NULL);
+	if (IS_ERR_VALUE(r)) {
+		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh->name);
+		return -EINVAL;
+	}
+
+	oh->_state = _HWMOD_STATE_INITIALIZED;
+
+	return 0;
+}
+
+/**
  * _setup - do initial configuration of omap_hwmod
  * @oh: struct omap_hwmod *
  *
@@ -1780,7 +1830,7 @@ static int _setup(struct omap_hwmod *oh, void *data)
 	int i, r;
 	u8 postsetup_state;
 
-	if (oh->_state != _HWMOD_STATE_CLKS_INITED)
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
 		return 0;
 
 	/* Set iclk autoidle mode */
@@ -2052,96 +2102,69 @@ int __init omap_hwmod_register(struct omap_hwmod **ohs)
 	return 0;
 }
 
-/*
- * _populate_mpu_rt_base - populate the virtual address for a hwmod
+/**
+ * _ensure_mpu_hwmod_is_setup - ensure the MPU SS hwmod is init'ed and set up
+ * @oh: pointer to the hwmod currently being set up (usually not the MPU)
  *
- * Must be called only from omap_hwmod_setup_*() so ioremap works properly.
- * Assumes the caller takes care of locking if needed.
+ * If the hwmod data corresponding to the MPU subsystem IP block
+ * hasn't been initialized and set up yet, do so now.  This must be
+ * done first since sleep dependencies may be added from other hwmods
+ * to the MPU.  Intended to be called only by omap_hwmod_setup*().  No
+ * return value.
  */
-static int __init _populate_mpu_rt_base(struct omap_hwmod *oh, void *data)
+static void __init _ensure_mpu_hwmod_is_setup(struct omap_hwmod *oh)
 {
-	if (oh->_state != _HWMOD_STATE_REGISTERED)
-		return 0;
-
-	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
-		return 0;
-
-	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
-
-	return 0;
+	if (!mpu_oh || mpu_oh->_state == _HWMOD_STATE_UNKNOWN)
+		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
+		       __func__, MPU_INITIATOR_NAME);
+	else if (mpu_oh->_state == _HWMOD_STATE_REGISTERED && oh != mpu_oh)
+		omap_hwmod_setup_one(MPU_INITIATOR_NAME);
 }
 
 /**
  * omap_hwmod_setup_one - set up a single hwmod
  * @oh_name: const char * name of the already-registered hwmod to set up
  *
- * Must be called after omap2_clk_init().  Resolves the struct clk
- * names to struct clk pointers for each registered omap_hwmod.  Also
- * calls _setup() on each hwmod.  Returns -EINVAL upon error or 0 upon
- * success.
+ * Initialize and set up a single hwmod.  Intended to be used for a
+ * small number of early devices, such as the timer IP blocks used for
+ * the scheduler clock.  Must be called after omap2_clk_init().
+ * Resolves the struct clk names to struct clk pointers for each
+ * registered omap_hwmod.  Also calls _setup() on each hwmod.  Returns
+ * -EINVAL upon error or 0 upon success.
  */
 int __init omap_hwmod_setup_one(const char *oh_name)
 {
 	struct omap_hwmod *oh;
-	int r;
 
 	pr_debug("omap_hwmod: %s: %s\n", oh_name, __func__);
 
-	if (!mpu_oh) {
-		pr_err("omap_hwmod: %s: cannot setup_one: MPU initiator hwmod %s not yet registered\n",
-		       oh_name, MPU_INITIATOR_NAME);
-		return -EINVAL;
-	}
-
 	oh = _lookup(oh_name);
 	if (!oh) {
 		WARN(1, "omap_hwmod: %s: hwmod not yet registered\n", oh_name);
 		return -EINVAL;
 	}
 
-	if (mpu_oh->_state == _HWMOD_STATE_REGISTERED && oh != mpu_oh)
-		omap_hwmod_setup_one(MPU_INITIATOR_NAME);
-
-	r = _populate_mpu_rt_base(oh, NULL);
-	if (IS_ERR_VALUE(r)) {
-		WARN(1, "omap_hwmod: %s: couldn't set mpu_rt_base\n", oh_name);
-		return -EINVAL;
-	}
-
-	r = _init_clocks(oh, NULL);
-	if (IS_ERR_VALUE(r)) {
-		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh_name);
-		return -EINVAL;
-	}
+	_ensure_mpu_hwmod_is_setup(oh);
 
+	_init(oh, NULL);
 	_setup(oh, NULL);
 
 	return 0;
 }
 
 /**
- * omap_hwmod_setup - do some post-clock framework initialization
+ * omap_hwmod_setup_all - set up all registered IP blocks
  *
- * Must be called after omap2_clk_init().  Resolves the struct clk names
- * to struct clk pointers for each registered omap_hwmod.  Also calls
- * _setup() on each hwmod.  Returns 0 upon success.
+ * Initialize and set up all IP blocks registered with the hwmod code.
+ * Must be called after omap2_clk_init().  Resolves the struct clk
+ * names to struct clk pointers for each registered omap_hwmod.  Also
+ * calls _setup() on each hwmod.  Returns 0 upon success.
  */
 static int __init omap_hwmod_setup_all(void)
 {
-	int r;
-
-	if (!mpu_oh) {
-		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
-		       __func__, MPU_INITIATOR_NAME);
-		return -EINVAL;
-	}
-
-	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
-
-	r = omap_hwmod_for_each(_init_clocks, NULL);
-	WARN(IS_ERR_VALUE(r),
-	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
+	_ensure_mpu_hwmod_is_setup(NULL);
 
+	omap_hwmod_for_each(_init, NULL);
 	omap_hwmod_for_each(_setup, NULL);
 
 	return 0;
-- 
1.7.10

[PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Paul Walmsley]

Hi

On Thu, 19 Apr 2012, Paul Walmsley wrote:

> - The changes to the _init function have been split into a separate patch
>   to make it easier to debug.

Here is the patch that deals specifically with _init().  It's the same 
code that was originally part of the patch mentioned in the subject line, 
split into a new patch for clarity.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Thu, 19 Apr 2012 00:58:22 -0600
Subject: [PATCH 2/3] ARM: OMAP2+: hwmod: reorganize and document the
 initialization process

Reorganize the code involved in initializing the internal data for
each hwmod to make it easier to read and maintain.  This involves
improving documentation and removing some duplicated and unnecessary
code.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |  137 ++++++++++++++++++++++----------------
 1 file changed, 80 insertions(+), 57 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5b07ad0..253e942 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1769,6 +1769,56 @@ static int _shutdown(struct omap_hwmod *oh)
 }
 
 /**
+ * _init_mpu_rt_base - populate the virtual address for a hwmod
+ * @oh: struct omap_hwmod * to locate the virtual address
+ *
+ * Cache the virtual address used by the MPU to access this IP block's
+ * registers.  This address is needed early so the OCP registers that
+ * are part of the device's address space can be ioremapped properly.
+ * No return value.
+ */
+static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
+{
+	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
+		return;
+
+	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
+}
+
+/**
+ * _init - initialize internal data for the hwmod @oh
+ * @oh: struct omap_hwmod *
+ * @n: (unused)
+ *
+ * Look up the clocks and the address space used by the MPU to access
+ * registers belonging to the hwmod @oh.  @oh must already be
+ * registered at this point.  This is the first of two phases for
+ * hwmod initialization.  Code called here does not touch any hardware
+ * registers, it simply prepares internal data structures.  Returns 0
+ * upon success or if the hwmod isn't registered, or -EINVAL upon
+ * failure.
+ */
+static int __init _init(struct omap_hwmod *oh, void *n)
+{
+	int r;
+
+	if (oh->_state != _HWMOD_STATE_REGISTERED)
+		return 0;
+
+	_init_mpu_rt_base(oh, NULL);
+
+	r = _init_clocks(oh, NULL);
+	if (IS_ERR_VALUE(r)) {
+		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh->name);
+		return -EINVAL;
+	}
+
+	oh->_state = _HWMOD_STATE_INITIALIZED;
+
+	return 0;
+}
+
+/**
  * _setup - do initial configuration of omap_hwmod
  * @oh: struct omap_hwmod *
  *
@@ -1780,7 +1830,7 @@ static int _setup(struct omap_hwmod *oh, void *data)
 	int i, r;
 	u8 postsetup_state;
 
-	if (oh->_state != _HWMOD_STATE_CLKS_INITED)
+	if (oh->_state != _HWMOD_STATE_INITIALIZED)
 		return 0;
 
 	/* Set iclk autoidle mode */
@@ -2052,96 +2102,69 @@ int __init omap_hwmod_register(struct omap_hwmod **ohs)
 	return 0;
 }
 
-/*
- * _populate_mpu_rt_base - populate the virtual address for a hwmod
+/**
+ * _ensure_mpu_hwmod_is_setup - ensure the MPU SS hwmod is init'ed and set up
+ * @oh: pointer to the hwmod currently being set up (usually not the MPU)
  *
- * Must be called only from omap_hwmod_setup_*() so ioremap works properly.
- * Assumes the caller takes care of locking if needed.
+ * If the hwmod data corresponding to the MPU subsystem IP block
+ * hasn't been initialized and set up yet, do so now.  This must be
+ * done first since sleep dependencies may be added from other hwmods
+ * to the MPU.  Intended to be called only by omap_hwmod_setup*().  No
+ * return value.
  */
-static int __init _populate_mpu_rt_base(struct omap_hwmod *oh, void *data)
+static void __init _ensure_mpu_hwmod_is_setup(struct omap_hwmod *oh)
 {
-	if (oh->_state != _HWMOD_STATE_REGISTERED)
-		return 0;
-
-	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
-		return 0;
-
-	oh->_mpu_rt_va = _find_mpu_rt_base(oh, oh->_mpu_port_index);
-
-	return 0;
+	if (!mpu_oh || mpu_oh->_state == _HWMOD_STATE_UNKNOWN)
+		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
+		       __func__, MPU_INITIATOR_NAME);
+	else if (mpu_oh->_state == _HWMOD_STATE_REGISTERED && oh != mpu_oh)
+		omap_hwmod_setup_one(MPU_INITIATOR_NAME);
 }
 
 /**
  * omap_hwmod_setup_one - set up a single hwmod
  * @oh_name: const char * name of the already-registered hwmod to set up
  *
- * Must be called after omap2_clk_init().  Resolves the struct clk
- * names to struct clk pointers for each registered omap_hwmod.  Also
- * calls _setup() on each hwmod.  Returns -EINVAL upon error or 0 upon
- * success.
+ * Initialize and set up a single hwmod.  Intended to be used for a
+ * small number of early devices, such as the timer IP blocks used for
+ * the scheduler clock.  Must be called after omap2_clk_init().
+ * Resolves the struct clk names to struct clk pointers for each
+ * registered omap_hwmod.  Also calls _setup() on each hwmod.  Returns
+ * -EINVAL upon error or 0 upon success.
  */
 int __init omap_hwmod_setup_one(const char *oh_name)
 {
 	struct omap_hwmod *oh;
-	int r;
 
 	pr_debug("omap_hwmod: %s: %s\n", oh_name, __func__);
 
-	if (!mpu_oh) {
-		pr_err("omap_hwmod: %s: cannot setup_one: MPU initiator hwmod %s not yet registered\n",
-		       oh_name, MPU_INITIATOR_NAME);
-		return -EINVAL;
-	}
-
 	oh = _lookup(oh_name);
 	if (!oh) {
 		WARN(1, "omap_hwmod: %s: hwmod not yet registered\n", oh_name);
 		return -EINVAL;
 	}
 
-	if (mpu_oh->_state == _HWMOD_STATE_REGISTERED && oh != mpu_oh)
-		omap_hwmod_setup_one(MPU_INITIATOR_NAME);
-
-	r = _populate_mpu_rt_base(oh, NULL);
-	if (IS_ERR_VALUE(r)) {
-		WARN(1, "omap_hwmod: %s: couldn't set mpu_rt_base\n", oh_name);
-		return -EINVAL;
-	}
-
-	r = _init_clocks(oh, NULL);
-	if (IS_ERR_VALUE(r)) {
-		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh_name);
-		return -EINVAL;
-	}
+	_ensure_mpu_hwmod_is_setup(oh);
 
+	_init(oh, NULL);
 	_setup(oh, NULL);
 
 	return 0;
 }
 
 /**
- * omap_hwmod_setup - do some post-clock framework initialization
+ * omap_hwmod_setup_all - set up all registered IP blocks
  *
- * Must be called after omap2_clk_init().  Resolves the struct clk names
- * to struct clk pointers for each registered omap_hwmod.  Also calls
- * _setup() on each hwmod.  Returns 0 upon success.
+ * Initialize and set up all IP blocks registered with the hwmod code.
+ * Must be called after omap2_clk_init().  Resolves the struct clk
+ * names to struct clk pointers for each registered omap_hwmod.  Also
+ * calls _setup() on each hwmod.  Returns 0 upon success.
  */
 static int __init omap_hwmod_setup_all(void)
 {
-	int r;
-
-	if (!mpu_oh) {
-		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
-		       __func__, MPU_INITIATOR_NAME);
-		return -EINVAL;
-	}
-
-	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
-
-	r = omap_hwmod_for_each(_init_clocks, NULL);
-	WARN(IS_ERR_VALUE(r),
-	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
+	_ensure_mpu_hwmod_is_setup(NULL);
 
+	omap_hwmod_for_each(_init, NULL);
 	omap_hwmod_for_each(_setup, NULL);
 
 	return 0;
-- 
1.7.10

Re: [PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6718 bytes --]

Hi

On Mon, 27 Feb 2012, Paul Walmsley wrote:

> Move the code that reprograms the OCP_SYSCONFIG bits into the _reset()
> function to ensure that it is called after every reset.  The code was
> previously in the _setup() function.  So, before this patch, if
> _reset() was called from another function, the SYSCONFIG register
> won't be reprogrammed.

As mentioned earlier, the _reset() changes previously in patch 3 have been 
rolled into this patch, as follows.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Thu, 19 Apr 2012 00:49:09 -0600
Subject: [PATCH] ARM: OMAP2+: hwmod: revise the IP block reset process

Revise the IP block reset process.  This patch ensures that the
OCP_SYSCONFIG registers are reloaded after a custom reset.  Since
OCP_SYSCONFIG bits are cleared during reset, they should be
reprogrammed unless the IP block is being left in reset.  (The only IP
blocks that are left in reset are IP blocks with hardreset lines and
no custom reset function.)  If the IP block is left in reset, then it
is inaccessible to the MPU, and an access to the OCP_SYSCONFIG
register will cause an abort.

This version incorporates comments from Omar Ramirez Luna
<omar.ramirez@ti.com> to skip the OCP_SYSCONFIG access after asserting
hardreset lines.  This allows the MMU (IOMMU) IP block, which has
both hardreset lines and an OCP_SYSCONFIG register.

Also, ignore _ocp_softreset() errors if the IP block doesn't include a
softreset bit.  This is needed since a subsequent patch will start
taking the return value of the _reset() function seriously.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
Cc: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   69 +++++++++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 4997c1a..5b07ad0 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2,7 +2,7 @@
  * omap_hwmod implementation for OMAP2/3/4
  *
  * Copyright (C) 2009-2011 Nokia Corporation
- * Copyright (C) 2011 Texas Instruments, Inc.
+ * Copyright (C) 2011-2012 Texas Instruments, Inc.
  *
  * Paul Walmsley, Benoît Cousson, Kevin Hilman
  *
@@ -1382,9 +1382,9 @@ static int _read_hardreset(struct omap_hwmod *oh, const char *name)
  * @oh: struct omap_hwmod *
  *
  * Resets an omap_hwmod @oh via the OCP_SYSCONFIG bit.  hwmod must be
- * enabled for this to work.  Returns -EINVAL if the hwmod cannot be
- * reset this way or if the hwmod is in the wrong state, -ETIMEDOUT if
- * the module did not reset in time, or 0 upon success.
+ * enabled for this to work.  Returns -ENOENT if the hwmod cannot be
+ * reset this way, -EINVAL if the hwmod is in the wrong state,
+ * -ETIMEDOUT if the module did not reset in time, or 0 upon success.
  *
  * In OMAP3 a specific SYSSTATUS register is used to get the reset status.
  * Starting in OMAP4, some IPs do not have SYSSTATUS registers and instead
@@ -1401,7 +1401,7 @@ static int _ocp_softreset(struct omap_hwmod *oh)
 
 	if (!oh->class->sysc ||
 	    !(oh->class->sysc->sysc_flags & SYSC_HAS_SOFTRESET))
-		return -EINVAL;
+		return -ENOENT;
 
 	/* clocks must be on for this operation */
 	if (oh->_state != _HWMOD_STATE_ENABLED) {
@@ -1462,37 +1462,60 @@ dis_opt_clks:
  * _reset - reset an omap_hwmod
  * @oh: struct omap_hwmod *
  *
- * Resets an omap_hwmod @oh.  The default software reset mechanism for
- * most OMAP IP blocks is triggered via the OCP_SYSCONFIG.SOFTRESET
- * bit.  However, some hwmods cannot be reset via this method: some
- * are not targets and therefore have no OCP header registers to
- * access; others (like the IVA) have idiosyncratic reset sequences.
- * So for these relatively rare cases, custom reset code can be
- * supplied in the struct omap_hwmod_class .reset function pointer.
- * Passes along the return value from either _reset() or the custom
- * reset function - these must return -EINVAL if the hwmod cannot be
- * reset this way or if the hwmod is in the wrong state, -ETIMEDOUT if
- * the module did not reset in time, or 0 upon success.
+ * Resets an omap_hwmod @oh.  If the module has a custom reset
+ * function pointer defined, then call it to reset the IP block, and
+ * pass along its return value to the caller.  Otherwise, if the IP
+ * block has an OCP_SYSCONFIG register with a SOFTRESET bitfield
+ * associated with it, call a function to reset the IP block via that
+ * method, and pass along the return value to the caller.  Finally, if
+ * the IP block has some hardreset lines associated with it, assert
+ * all of those, but do _not_ deassert them. (This is because driver
+ * authors have expressed an apparent requirement to control the
+ * deassertion of the hardreset lines themselves.)
+ *
+ * The default software reset mechanism for most OMAP IP blocks is
+ * triggered via the OCP_SYSCONFIG.SOFTRESET bit.  However, some
+ * hwmods cannot be reset via this method.  Some are not targets and
+ * therefore have no OCP header registers to access.  Others (like the
+ * IVA) have idiosyncratic reset sequences.  So for these relatively
+ * rare cases, custom reset code can be supplied in the struct
+ * omap_hwmod_class .reset function pointer.  Passes along the return
+ * value from either _ocp_softreset() or the custom reset function -
+ * these must return -EINVAL if the hwmod cannot be reset this way or
+ * if the hwmod is in the wrong state, -ETIMEDOUT if the module did
+ * not reset in time, or 0 upon success.
  */
 static int _reset(struct omap_hwmod *oh)
 {
-	int ret;
+	int i, r;
 
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
+	if (oh->class->reset) {
+		r = oh->class->reset(oh);
+	} else {
+		if (oh->rst_lines_cnt > 0) {
+			for (i = 0; i < oh->rst_lines_cnt; i++)
+				_assert_hardreset(oh, oh->rst_lines[i].name);
+			return 0;
+		} else {
+			r = _ocp_softreset(oh);
+			if (r == -ENOENT)
+				r = 0;
+		}
+	}
+
 	/*
-	 * XXX We're not resetting modules with hardreset lines
-	 * automatically here.  Should we do this also, or just expect
-	 * those modules to define custom reset functions?
+	 * OCP_SYSCONFIG bits need to be reprogrammed after a
+	 * softreset.  The _enable() function should be split to avoid
+	 * the rewrite of the OCP_SYSCONFIG register.
 	 */
-	ret = (oh->class->reset) ? oh->class->reset(oh) : _ocp_softreset(oh);
-
 	if (oh->class->sysc) {
 		_update_sysc_cache(oh);
 		_enable_sysc(oh);
 	}
 
-	return ret;
+	return r;
 }
 
 /**
-- 
1.7.10

[PATCH v2 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset

[Paul Walmsley]

Hi

On Mon, 27 Feb 2012, Paul Walmsley wrote:

> Move the code that reprograms the OCP_SYSCONFIG bits into the _reset()
> function to ensure that it is called after every reset.  The code was
> previously in the _setup() function.  So, before this patch, if
> _reset() was called from another function, the SYSCONFIG register
> won't be reprogrammed.

As mentioned earlier, the _reset() changes previously in patch 3 have been 
rolled into this patch, as follows.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Thu, 19 Apr 2012 00:49:09 -0600
Subject: [PATCH] ARM: OMAP2+: hwmod: revise the IP block reset process

Revise the IP block reset process.  This patch ensures that the
OCP_SYSCONFIG registers are reloaded after a custom reset.  Since
OCP_SYSCONFIG bits are cleared during reset, they should be
reprogrammed unless the IP block is being left in reset.  (The only IP
blocks that are left in reset are IP blocks with hardreset lines and
no custom reset function.)  If the IP block is left in reset, then it
is inaccessible to the MPU, and an access to the OCP_SYSCONFIG
register will cause an abort.

This version incorporates comments from Omar Ramirez Luna
<omar.ramirez@ti.com> to skip the OCP_SYSCONFIG access after asserting
hardreset lines.  This allows the MMU (IOMMU) IP block, which has
both hardreset lines and an OCP_SYSCONFIG register.

Also, ignore _ocp_softreset() errors if the IP block doesn't include a
softreset bit.  This is needed since a subsequent patch will start
taking the return value of the _reset() function seriously.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
Cc: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   69 +++++++++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 4997c1a..5b07ad0 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2,7 +2,7 @@
  * omap_hwmod implementation for OMAP2/3/4
  *
  * Copyright (C) 2009-2011 Nokia Corporation
- * Copyright (C) 2011 Texas Instruments, Inc.
+ * Copyright (C) 2011-2012 Texas Instruments, Inc.
  *
  * Paul Walmsley, Beno?t Cousson, Kevin Hilman
  *
@@ -1382,9 +1382,9 @@ static int _read_hardreset(struct omap_hwmod *oh, const char *name)
  * @oh: struct omap_hwmod *
  *
  * Resets an omap_hwmod @oh via the OCP_SYSCONFIG bit.  hwmod must be
- * enabled for this to work.  Returns -EINVAL if the hwmod cannot be
- * reset this way or if the hwmod is in the wrong state, -ETIMEDOUT if
- * the module did not reset in time, or 0 upon success.
+ * enabled for this to work.  Returns -ENOENT if the hwmod cannot be
+ * reset this way, -EINVAL if the hwmod is in the wrong state,
+ * -ETIMEDOUT if the module did not reset in time, or 0 upon success.
  *
  * In OMAP3 a specific SYSSTATUS register is used to get the reset status.
  * Starting in OMAP4, some IPs do not have SYSSTATUS registers and instead
@@ -1401,7 +1401,7 @@ static int _ocp_softreset(struct omap_hwmod *oh)
 
 	if (!oh->class->sysc ||
 	    !(oh->class->sysc->sysc_flags & SYSC_HAS_SOFTRESET))
-		return -EINVAL;
+		return -ENOENT;
 
 	/* clocks must be on for this operation */
 	if (oh->_state != _HWMOD_STATE_ENABLED) {
@@ -1462,37 +1462,60 @@ dis_opt_clks:
  * _reset - reset an omap_hwmod
  * @oh: struct omap_hwmod *
  *
- * Resets an omap_hwmod @oh.  The default software reset mechanism for
- * most OMAP IP blocks is triggered via the OCP_SYSCONFIG.SOFTRESET
- * bit.  However, some hwmods cannot be reset via this method: some
- * are not targets and therefore have no OCP header registers to
- * access; others (like the IVA) have idiosyncratic reset sequences.
- * So for these relatively rare cases, custom reset code can be
- * supplied in the struct omap_hwmod_class .reset function pointer.
- * Passes along the return value from either _reset() or the custom
- * reset function - these must return -EINVAL if the hwmod cannot be
- * reset this way or if the hwmod is in the wrong state, -ETIMEDOUT if
- * the module did not reset in time, or 0 upon success.
+ * Resets an omap_hwmod @oh.  If the module has a custom reset
+ * function pointer defined, then call it to reset the IP block, and
+ * pass along its return value to the caller.  Otherwise, if the IP
+ * block has an OCP_SYSCONFIG register with a SOFTRESET bitfield
+ * associated with it, call a function to reset the IP block via that
+ * method, and pass along the return value to the caller.  Finally, if
+ * the IP block has some hardreset lines associated with it, assert
+ * all of those, but do _not_ deassert them. (This is because driver
+ * authors have expressed an apparent requirement to control the
+ * deassertion of the hardreset lines themselves.)
+ *
+ * The default software reset mechanism for most OMAP IP blocks is
+ * triggered via the OCP_SYSCONFIG.SOFTRESET bit.  However, some
+ * hwmods cannot be reset via this method.  Some are not targets and
+ * therefore have no OCP header registers to access.  Others (like the
+ * IVA) have idiosyncratic reset sequences.  So for these relatively
+ * rare cases, custom reset code can be supplied in the struct
+ * omap_hwmod_class .reset function pointer.  Passes along the return
+ * value from either _ocp_softreset() or the custom reset function -
+ * these must return -EINVAL if the hwmod cannot be reset this way or
+ * if the hwmod is in the wrong state, -ETIMEDOUT if the module did
+ * not reset in time, or 0 upon success.
  */
 static int _reset(struct omap_hwmod *oh)
 {
-	int ret;
+	int i, r;
 
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
+	if (oh->class->reset) {
+		r = oh->class->reset(oh);
+	} else {
+		if (oh->rst_lines_cnt > 0) {
+			for (i = 0; i < oh->rst_lines_cnt; i++)
+				_assert_hardreset(oh, oh->rst_lines[i].name);
+			return 0;
+		} else {
+			r = _ocp_softreset(oh);
+			if (r == -ENOENT)
+				r = 0;
+		}
+	}
+
 	/*
-	 * XXX We're not resetting modules with hardreset lines
-	 * automatically here.  Should we do this also, or just expect
-	 * those modules to define custom reset functions?
+	 * OCP_SYSCONFIG bits need to be reprogrammed after a
+	 * softreset.  The _enable() function should be split to avoid
+	 * the rewrite of the OCP_SYSCONFIG register.
 	 */
-	ret = (oh->class->reset) ? oh->class->reset(oh) : _ocp_softreset(oh);
-
 	if (oh->class->sysc) {
 		_update_sysc_cache(oh);
 		_enable_sysc(oh);
 	}
 
-	return ret;
+	return r;
 }
 
 /**
-- 
1.7.10

Re: [PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Cousson, Benoit]

Hi Paul,

On 4/19/2012 10:17 AM, Paul Walmsley wrote:

...

>   static int __init omap_hwmod_setup_all(void)
>   {
> -	int r;
> -
> -	if (!mpu_oh) {
> -		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
> -		       __func__, MPU_INITIATOR_NAME);
> -		return -EINVAL;
> -	}
> -
> -	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
> -
> -	r = omap_hwmod_for_each(_init_clocks, NULL);
> -	WARN(IS_ERR_VALUE(r),
> -	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
> +	_ensure_mpu_hwmod_is_setup(NULL);
>
> +	omap_hwmod_for_each(_init, NULL);
>   	omap_hwmod_for_each(_setup, NULL);

Does it make sense to iterate twice? Cannot we just iterate over a _init 
+ _setup single call?

Regards,
Benoit

[PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Cousson, Benoit]

Hi Paul,

On 4/19/2012 10:17 AM, Paul Walmsley wrote:

...

>   static int __init omap_hwmod_setup_all(void)
>   {
> -	int r;
> -
> -	if (!mpu_oh) {
> -		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet registered\n",
> -		       __func__, MPU_INITIATOR_NAME);
> -		return -EINVAL;
> -	}
> -
> -	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
> -
> -	r = omap_hwmod_for_each(_init_clocks, NULL);
> -	WARN(IS_ERR_VALUE(r),
> -	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
> +	_ensure_mpu_hwmod_is_setup(NULL);
>
> +	omap_hwmod_for_each(_init, NULL);
>   	omap_hwmod_for_each(_setup, NULL);

Does it make sense to iterate twice? Cannot we just iterate over a _init 
+ _setup single call?

Regards,
Benoit

Re: [PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1118 bytes --]

Hi Benoît,

On Thu, 19 Apr 2012, Cousson, Benoit wrote:

> On 4/19/2012 10:17 AM, Paul Walmsley wrote:
> 
> >   static int __init omap_hwmod_setup_all(void)
> >   {
> > -	int r;
> > -
> > -	if (!mpu_oh) {
> > -		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet
> > registered\n",
> > -		       __func__, MPU_INITIATOR_NAME);
> > -		return -EINVAL;
> > -	}
> > -
> > -	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
> > -
> > -	r = omap_hwmod_for_each(_init_clocks, NULL);
> > -	WARN(IS_ERR_VALUE(r),
> > -	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
> > +	_ensure_mpu_hwmod_is_setup(NULL);
> > 
> > +	omap_hwmod_for_each(_init, NULL);
> >   	omap_hwmod_for_each(_setup, NULL);
> 
> Does it make sense to iterate twice? Cannot we just iterate over a _init +
> _setup single call?

It's a good idea but I don't think we can do that until we add the code to 
walk the hwmod data in dependency order.  The DSS custom reset function 
accesses registers in another hwmod, which won't have its clocks 
initialized yet if _init() and setup() are combined.


- Paul

[PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Paul Walmsley]

Hi Beno?t,

On Thu, 19 Apr 2012, Cousson, Benoit wrote:

> On 4/19/2012 10:17 AM, Paul Walmsley wrote:
> 
> >   static int __init omap_hwmod_setup_all(void)
> >   {
> > -	int r;
> > -
> > -	if (!mpu_oh) {
> > -		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet
> > registered\n",
> > -		       __func__, MPU_INITIATOR_NAME);
> > -		return -EINVAL;
> > -	}
> > -
> > -	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
> > -
> > -	r = omap_hwmod_for_each(_init_clocks, NULL);
> > -	WARN(IS_ERR_VALUE(r),
> > -	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
> > +	_ensure_mpu_hwmod_is_setup(NULL);
> > 
> > +	omap_hwmod_for_each(_init, NULL);
> >   	omap_hwmod_for_each(_setup, NULL);
> 
> Does it make sense to iterate twice? Cannot we just iterate over a _init +
> _setup single call?

It's a good idea but I don't think we can do that until we add the code to 
walk the hwmod data in dependency order.  The DSS custom reset function 
accesses registers in another hwmod, which won't have its clocks 
initialized yet if _init() and setup() are combined.


- Paul

Re: [PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Cousson, Benoit]

On 4/19/2012 11:49 AM, Paul Walmsley wrote:
> Hi Benoît,
>
> On Thu, 19 Apr 2012, Cousson, Benoit wrote:
>
>> On 4/19/2012 10:17 AM, Paul Walmsley wrote:
>>
>>>    static int __init omap_hwmod_setup_all(void)
>>>    {
>>> -	int r;
>>> -
>>> -	if (!mpu_oh) {
>>> -		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet
>>> registered\n",
>>> -		       __func__, MPU_INITIATOR_NAME);
>>> -		return -EINVAL;
>>> -	}
>>> -
>>> -	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
>>> -
>>> -	r = omap_hwmod_for_each(_init_clocks, NULL);
>>> -	WARN(IS_ERR_VALUE(r),
>>> -	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
>>> +	_ensure_mpu_hwmod_is_setup(NULL);
>>>
>>> +	omap_hwmod_for_each(_init, NULL);
>>>    	omap_hwmod_for_each(_setup, NULL);
>>
>> Does it make sense to iterate twice? Cannot we just iterate over a _init +
>> _setup single call?
>
> It's a good idea but I don't think we can do that until we add the code to
> walk the hwmod data in dependency order.  The DSS custom reset function
> accesses registers in another hwmod, which won't have its clocks
> initialized yet if _init() and setup() are combined.

Outch, that's unfortunate!

I'm wondering if the whole automatic reset management at hwmod level 
does make sense for such IP.

In the case of the DSS, having some reset support in the driver model 
should allow potentially a reset done at parent level to be broadcasted 
to every children.

Regards,
Benoit
--
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

[PATCH v2 3/8] ARM: OMAP2+: hwmod: reorganize and document the setup process

[Cousson, Benoit]

On 4/19/2012 11:49 AM, Paul Walmsley wrote:
> Hi Beno?t,
>
> On Thu, 19 Apr 2012, Cousson, Benoit wrote:
>
>> On 4/19/2012 10:17 AM, Paul Walmsley wrote:
>>
>>>    static int __init omap_hwmod_setup_all(void)
>>>    {
>>> -	int r;
>>> -
>>> -	if (!mpu_oh) {
>>> -		pr_err("omap_hwmod: %s: MPU initiator hwmod %s not yet
>>> registered\n",
>>> -		       __func__, MPU_INITIATOR_NAME);
>>> -		return -EINVAL;
>>> -	}
>>> -
>>> -	r = omap_hwmod_for_each(_populate_mpu_rt_base, NULL);
>>> -
>>> -	r = omap_hwmod_for_each(_init_clocks, NULL);
>>> -	WARN(IS_ERR_VALUE(r),
>>> -	     "omap_hwmod: %s: _init_clocks failed\n", __func__);
>>> +	_ensure_mpu_hwmod_is_setup(NULL);
>>>
>>> +	omap_hwmod_for_each(_init, NULL);
>>>    	omap_hwmod_for_each(_setup, NULL);
>>
>> Does it make sense to iterate twice? Cannot we just iterate over a _init +
>> _setup single call?
>
> It's a good idea but I don't think we can do that until we add the code to
> walk the hwmod data in dependency order.  The DSS custom reset function
> accesses registers in another hwmod, which won't have its clocks
> initialized yet if _init() and setup() are combined.

Outch, that's unfortunate!

I'm wondering if the whole automatic reset management at hwmod level 
does make sense for such IP.

In the case of the DSS, having some reset support in the driver model 
should allow potentially a reset done at parent level to be broadcasted 
to every children.

Regards,
Benoit

Re: [PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Cousson, Benoit]

+ Omar and Ohad,

Salut Paul,

On 4/19/2012 8:53 AM, Paul Walmsley wrote:
> Hi Benoît,
>
> On Mon, 27 Feb 2012, Paul Walmsley wrote:
>
>> Change the way that hardreset lines are handled by the hwmod code.
>> Hardreset lines are generally associated with initiator IP blocks.
>> Prior to this change, the hwmod code expected to control hardreset
>> lines itself, asserting them on shutdown and deasserting them upon
>> enable.  But driver authors inside TI have commented to us that their
>> drivers require direct control over these lines.  Unfortunately, these
>> drivers haven't been posted publicly yet, so it's hard to determine
>> exactly what is needed, a priori.  This change attempts to set forth
>> some reasonable semantics that should be an improvement over the
>> current code.
>
> I took another look at this patch, and upon further thought, there are
> some aspects of this design that really don't make sense.

That's perfect timing, because I was struggling with some hardreset 
issue on OMAP5 yesterday, so I thought as well about that patch.

This is just some more thoughts on that topic more than a pure comment 
on this patch :-)

What I realized is that the main issue was due to the fake hwmods to 
control processor reset lines. So these one has to be removed, it was 
clearly a mistake.

The other issue was due to the wrong association between the reset lines 
and the hwmod. In fact I was trying to create ipu & dsp hwmods, whereas 
these subsystem do not have any direct connection with the main 
interconnect but only through their MMU.
This is exactly what Omar raised when he tried to introduce the MMU hwmods.
And in fact the new name used in OMAP5 are highlight that a little bit 
netter than on OMAP4"

     'ipu':
         'rst_cpu0'
         'rst_cpu1'
         'rst_ipu_mmu_cache'

     'dsp':
         'rst_dsp'
         'rst_dsp_mmu_cache'

     'iva':
         'rst_seq1'
         'rst_seq2'
         'rst_logic'

The concern of the people doing SW in these embedded processors was 
mainly because we were releasing the reset of processor without loading 
any SW first and thus the processor was executing some random instructions.

So if we consider a better hwmods definition, we can potentially fix the 
MMU case:

     'mmu_ipu':
         'rst_ipu_mmu_cache'

     'mmu_dsp':
         'rst_dsp_mmu_cache'

     'iva_config':
         'rst_logic'


But then we do have the processor resets that have to be exposed somewhere.

     'ipu':
         'rst_cpu0'
         'rst_cpu1'

     'dsp':
         'rst_dsp'

     'iva':
         'rst_seq1'
         'rst_seq2'

None of these one should be controlled automatically.

>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index 5b368ee..db4ad41 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -805,6 +805,9 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
>>   				    oh->clkdm->clkdm_offs,
>>   				    oh->prcm.omap4.clkctrl_offs);
>>
>> +	if (oh->rst_lines_cnt>= 0)
>> +		return 0;
>
> This change prevents any IP block from waiting for the target to disable
> -- which is not what we want.  The naïve fix would be to only skip the
> disable-wait if oh->rst_lines_cnt is greater than zero.  But if there are
> no hardreset lines asserted, then we should probably wait for the disable
> in that case.
>
>>   	v = _omap4_wait_target_disable(oh);
>>   	if (v)
>>   		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
>
> [...]
>
>> @@ -1575,7 +1568,19 @@ static int _enable(struct omap_hwmod *oh)
>>   	_enable_clocks(oh);
>>   	_enable_module(oh);
>>
>> -	r = _wait_target_ready(oh);
>> +	/*
>> +	 * If an IP contains HW reset lines, we leave them
>> +	 * asserted.  But this will block the module's idle state
>> +	 * transition - the PRCM will return Intransition status.  So
>> +	 * we need to avoid the target ready-wait in this case.  XXX
>> +	 * We also need to give the drivers a way to wait for the
>> +	 * target to become ready once they decide to deassert some
>> +	 * hardreset lines.  XXX Is this strategy going to break PM
>> +	 * because the clockdomain may not be able to enter idle while
>> +	 * the module's idle status is in-transition?  We may just need
>> +	 * custom reset blocks for all IPs with hardreset lines.
>> +	 */
>> +	r = (oh->rst_lines_cnt == 0) ? _wait_target_ready(oh) : 1;
>
> And this part is at odds with the patch description.  If there are
> hardreset lines associated with an IP block, then this code will cause the
> following code to unconditionally disable the module clocks.  Considering
> that this is the _enable() function, this seems counterproductive.
>
> I looked into changing this code to align with the original semantics we
> discussed.  It seems quite challenging.  With the current codebase, we'd
> have to bail out in the middle of the enable sequence.  Then we'd lose the
> clockdomain state (the 'hwsup' variable).
>
> So I've updated the patch to essentially bail out early from all hwmod
> enable, idle, and shutdown code, if any hardreset lines associated with
> the IP block are asserted.  It will then be the driver integration code's
> responsibility for enabling the IP block when the hardreset lines are
> asserted.  When the hardreset lines are deasserted, the usual hwmod code
> will be executed -- I'd assume this would be the case during normal
> operation of the device.  I think this is probably the best we can do
> until we actually hear back from the people responsible for drivers for
> these special IP blocks.
>
> A revised patch is below.  Care to take a look and see if it makes sense
> to you?
>
>
> regards,
>
> - Paul
>
>
> From: Paul Walmsley<paul@pwsan.com>
> Date: Wed, 18 Apr 2012 19:10:04 -0600
> Subject: [PATCH] ARM: OMAP2+: hwmod: revise hardreset behavior
>
> Change the way that hardreset lines are handled by the hwmod code.
> Hardreset lines are generally associated with initiator IP blocks.
> Prior to this change, the hwmod code expected to control hardreset
> lines itself, asserting them on shutdown and deasserting them upon
> enable.  But driver authors inside TI have commented to us that their
> drivers require direct control over these lines.  Unfortunately, these
> drivers haven't been posted publicly yet, so it's hard to determine
> exactly what is needed, a priori.  This change attempts to set forth
> some reasonable semantics that should be an improvement over the
> current code.
>
> The semantics implemented by this patch are as follows:
>
> - If the hwmod is not marked with HWMOD_INIT_NO_RESET, then assert all
>    associated hardreset lines during IP block setup.  This is intended
>    to place the IP blocks into a known state that will not interfere
>    with other devices during kernel boot.
>
> - IP blocks with hardreset lines will not be automatically enabled or
>    idled during setup.  Instead, they will be left in the INITIALIZED
>    state.
>
> - When the hwmod code is asked to enable, idle, or shutdown an IP
>    block with asserted hardreset lines, the hwmod code will do nothing.
>    The driver integration code must do the remaining work needed to
>    control these IP blocks.  Once this driver integration code is posted
>    to the lists, hopefully we can consolidate it and move it inside the
>    hwmod code.
>
> Custom reset functions for IP blocks with hardreset lines still should
> be supported and are strongly endorsed.  It is intended that every
> subsystem with hardreset lines should have a custom reset function
> that can place their subsystem into quiescent idle with the hardreset
> lines deasserted.
>
> This reverts most of commit 5365efbe29250a227502256cc912351fe2157b42
> ("OMAP: hwmod: Add hardreset management support").  Later code
> reorganizations caused the sequencing of the code from this patch to
> be changed, anyway.
>
> Signed-off-by: Paul Walmsley<paul@pwsan.com>
> Cc: Benoît Cousson<b-cousson@ti.com>
> ---
>   arch/arm/mach-omap2/omap_hwmod.c |  140 ++++++++++++++++++++++----------------
>   1 file changed, 83 insertions(+), 57 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index e6aa14f..a5c3a9a 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -781,39 +781,6 @@ static int _omap4_wait_target_disable(struct omap_hwmod *oh)
>   }
>
>   /**
> - * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4
> - * @oh: struct omap_hwmod *
> - *
> - * Disable the PRCM module mode related to the hwmod @oh.
> - * Return EINVAL if the modulemode is not supported and 0 in case of success.
> - */
> -static int _omap4_disable_module(struct omap_hwmod *oh)
> -{
> -	int v;
> -
> -	/* The module mode does not exist prior OMAP4 */
> -	if (!cpu_is_omap44xx())
> -		return -EINVAL;
> -
> -	if (!oh->clkdm || !oh->prcm.omap4.modulemode)
> -		return -EINVAL;
> -
> -	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
> -
> -	omap4_cminst_module_disable(oh->clkdm->prcm_partition,
> -				    oh->clkdm->cm_inst,
> -				    oh->clkdm->clkdm_offs,
> -				    oh->prcm.omap4.clkctrl_offs);
> -
> -	v = _omap4_wait_target_disable(oh);
> -	if (v)
> -		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
> -			oh->name);
> -
> -	return 0;
> -}
> -
> -/**
>    * _count_mpu_irqs - count the number of MPU IRQ lines associated with @oh
>    * @oh: struct omap_hwmod *oh
>    *
> @@ -1378,6 +1345,66 @@ 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
> + * @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.
> + * This function is used to avoid executing some parts of the IP block
> + * enable/disable sequence if a hardreset line is set.
> + */
> +static bool _are_any_hardreset_lines_asserted(struct omap_hwmod *oh)
> +{
> +	int i;
> +
> +	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;
> +
> +	return false;
> +}
> +
> +/**
> + * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4
> + * @oh: struct omap_hwmod *
> + *
> + * Disable the PRCM module mode related to the hwmod @oh.
> + * Return EINVAL if the modulemode is not supported and 0 in case of success.
> + */
> +static int _omap4_disable_module(struct omap_hwmod *oh)
> +{
> +	int v;
> +
> +	/* The module mode does not exist prior OMAP4 */
> +	if (!cpu_is_omap44xx())
> +		return -EINVAL;
> +
> +	if (!oh->clkdm || !oh->prcm.omap4.modulemode)
> +		return -EINVAL;
> +
> +	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
> +
> +	omap4_cminst_module_disable(oh->clkdm->prcm_partition,
> +				    oh->clkdm->cm_inst,
> +				    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",
> +			oh->name);
> +
> +	return 0;
> +}
> +
> +/**
>    * _ocp_softreset - reset an omap_hwmod via the OCP_SYSCONFIG bit
>    * @oh: struct omap_hwmod *
>    *
> @@ -1519,7 +1546,7 @@ static int _reset(struct omap_hwmod *oh)
>    */
>   static int _enable(struct omap_hwmod *oh)
>   {
> -	int r, i;
> +	int r;
>   	int hwsup = 0;
>
>   	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
> @@ -1551,14 +1578,16 @@ static int _enable(struct omap_hwmod *oh)
>   	}
>
>   	/*
> -	 * If an IP contains HW reset lines, then de-assert them in order
> -	 * to allow the module state transition. Otherwise the PRCM will return
> -	 * Intransition status, and the init will failed.
> +	 * If an IP block contains HW reset lines and any 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
> +	 * detailed information is provided and the driver code is
> +	 * posted to the public lists, this is probably the best we
> +	 * can do.

Maybe we should keep that with some mechanism to prevent it for some IP 
like processors .

I guess that with that patch, we cannot enable anymore IPU/DSP and IVA 
at boot time.

Otherwise, it looks fine to me.

Thanks,
Benoit

>   	 */
> -	if (oh->_state == _HWMOD_STATE_INITIALIZED ||
> -	    oh->_state == _HWMOD_STATE_DISABLED)
> -		for (i = 0; i<  oh->rst_lines_cnt; i++)
> -			_deassert_hardreset(oh, oh->rst_lines[i].name);
> +	if (_are_any_hardreset_lines_asserted(oh))
> +		return 0;
>
>   	/* Mux pins for device runtime if populated */
>   	if (oh->mux&&  (!oh->mux->enabled ||
> @@ -1633,6 +1662,9 @@ static int _idle(struct omap_hwmod *oh)
>   		return -EINVAL;
>   	}
>
> +	if (_are_any_hardreset_lines_asserted(oh))
> +		return 0;
> +
>   	if (oh->class->sysc)
>   		_idle_sysc(oh);
>   	_del_initiator_dep(oh, mpu_oh);
> @@ -1715,6 +1747,9 @@ static int _shutdown(struct omap_hwmod *oh)
>   		return -EINVAL;
>   	}
>
> +	if (_are_any_hardreset_lines_asserted(oh))
> +		return 0;
> +
>   	pr_debug("omap_hwmod: %s: disabling\n", oh->name);
>
>   	if (oh->class->pre_shutdown) {
> @@ -1824,27 +1859,18 @@ static int __init _setup_reset(struct omap_hwmod *oh)
>   	if (oh->_state != _HWMOD_STATE_INITIALIZED)
>   		return -EINVAL;
>
> -	/*
> -	 * In the case of hwmod with hardreset that should not be
> -	 * de-assert at boot time, we have to keep the module
> -	 * initialized, because we cannot enable it properly with the
> -	 * reset asserted. Exit without warning because that behavior
> -	 * is expected.
> -	 */
> -	if ((oh->flags&  HWMOD_INIT_NO_RESET)&&  oh->rst_lines_cnt>  0)
> -		return 0;
> -
> -	r = _enable(oh);
> -	if (r) {
> -		pr_warning("omap_hwmod: %s: cannot be enabled (%d)\n",
> -			   oh->name, oh->_state);
> -		return 0;
> +	if (oh->rst_lines_cnt == 0) {
> +		r = _enable(oh);
> +		if (r) {
> +			pr_warning("omap_hwmod: %s: cannot be enabled for reset (%d)\n",
> +				   oh->name, oh->_state);
> +			return -EINVAL;
> +		}
>   	}
>
>   	if (!(oh->flags&  HWMOD_INIT_NO_RESET))
>   		r = _reset(oh);
>
> -
>   	return r;
>   }
>   /**

--
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

[PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Cousson, Benoit]

+ Omar and Ohad,

Salut Paul,

On 4/19/2012 8:53 AM, Paul Walmsley wrote:
> Hi Beno?t,
>
> On Mon, 27 Feb 2012, Paul Walmsley wrote:
>
>> Change the way that hardreset lines are handled by the hwmod code.
>> Hardreset lines are generally associated with initiator IP blocks.
>> Prior to this change, the hwmod code expected to control hardreset
>> lines itself, asserting them on shutdown and deasserting them upon
>> enable.  But driver authors inside TI have commented to us that their
>> drivers require direct control over these lines.  Unfortunately, these
>> drivers haven't been posted publicly yet, so it's hard to determine
>> exactly what is needed, a priori.  This change attempts to set forth
>> some reasonable semantics that should be an improvement over the
>> current code.
>
> I took another look at this patch, and upon further thought, there are
> some aspects of this design that really don't make sense.

That's perfect timing, because I was struggling with some hardreset 
issue on OMAP5 yesterday, so I thought as well about that patch.

This is just some more thoughts on that topic more than a pure comment 
on this patch :-)

What I realized is that the main issue was due to the fake hwmods to 
control processor reset lines. So these one has to be removed, it was 
clearly a mistake.

The other issue was due to the wrong association between the reset lines 
and the hwmod. In fact I was trying to create ipu & dsp hwmods, whereas 
these subsystem do not have any direct connection with the main 
interconnect but only through their MMU.
This is exactly what Omar raised when he tried to introduce the MMU hwmods.
And in fact the new name used in OMAP5 are highlight that a little bit 
netter than on OMAP4"

     'ipu':
         'rst_cpu0'
         'rst_cpu1'
         'rst_ipu_mmu_cache'

     'dsp':
         'rst_dsp'
         'rst_dsp_mmu_cache'

     'iva':
         'rst_seq1'
         'rst_seq2'
         'rst_logic'

The concern of the people doing SW in these embedded processors was 
mainly because we were releasing the reset of processor without loading 
any SW first and thus the processor was executing some random instructions.

So if we consider a better hwmods definition, we can potentially fix the 
MMU case:

     'mmu_ipu':
         'rst_ipu_mmu_cache'

     'mmu_dsp':
         'rst_dsp_mmu_cache'

     'iva_config':
         'rst_logic'


But then we do have the processor resets that have to be exposed somewhere.

     'ipu':
         'rst_cpu0'
         'rst_cpu1'

     'dsp':
         'rst_dsp'

     'iva':
         'rst_seq1'
         'rst_seq2'

None of these one should be controlled automatically.

>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index 5b368ee..db4ad41 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -805,6 +805,9 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
>>   				    oh->clkdm->clkdm_offs,
>>   				    oh->prcm.omap4.clkctrl_offs);
>>
>> +	if (oh->rst_lines_cnt>= 0)
>> +		return 0;
>
> This change prevents any IP block from waiting for the target to disable
> -- which is not what we want.  The na?ve fix would be to only skip the
> disable-wait if oh->rst_lines_cnt is greater than zero.  But if there are
> no hardreset lines asserted, then we should probably wait for the disable
> in that case.
>
>>   	v = _omap4_wait_target_disable(oh);
>>   	if (v)
>>   		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
>
> [...]
>
>> @@ -1575,7 +1568,19 @@ static int _enable(struct omap_hwmod *oh)
>>   	_enable_clocks(oh);
>>   	_enable_module(oh);
>>
>> -	r = _wait_target_ready(oh);
>> +	/*
>> +	 * If an IP contains HW reset lines, we leave them
>> +	 * asserted.  But this will block the module's idle state
>> +	 * transition - the PRCM will return Intransition status.  So
>> +	 * we need to avoid the target ready-wait in this case.  XXX
>> +	 * We also need to give the drivers a way to wait for the
>> +	 * target to become ready once they decide to deassert some
>> +	 * hardreset lines.  XXX Is this strategy going to break PM
>> +	 * because the clockdomain may not be able to enter idle while
>> +	 * the module's idle status is in-transition?  We may just need
>> +	 * custom reset blocks for all IPs with hardreset lines.
>> +	 */
>> +	r = (oh->rst_lines_cnt == 0) ? _wait_target_ready(oh) : 1;
>
> And this part is at odds with the patch description.  If there are
> hardreset lines associated with an IP block, then this code will cause the
> following code to unconditionally disable the module clocks.  Considering
> that this is the _enable() function, this seems counterproductive.
>
> I looked into changing this code to align with the original semantics we
> discussed.  It seems quite challenging.  With the current codebase, we'd
> have to bail out in the middle of the enable sequence.  Then we'd lose the
> clockdomain state (the 'hwsup' variable).
>
> So I've updated the patch to essentially bail out early from all hwmod
> enable, idle, and shutdown code, if any hardreset lines associated with
> the IP block are asserted.  It will then be the driver integration code's
> responsibility for enabling the IP block when the hardreset lines are
> asserted.  When the hardreset lines are deasserted, the usual hwmod code
> will be executed -- I'd assume this would be the case during normal
> operation of the device.  I think this is probably the best we can do
> until we actually hear back from the people responsible for drivers for
> these special IP blocks.
>
> A revised patch is below.  Care to take a look and see if it makes sense
> to you?
>
>
> regards,
>
> - Paul
>
>
> From: Paul Walmsley<paul@pwsan.com>
> Date: Wed, 18 Apr 2012 19:10:04 -0600
> Subject: [PATCH] ARM: OMAP2+: hwmod: revise hardreset behavior
>
> Change the way that hardreset lines are handled by the hwmod code.
> Hardreset lines are generally associated with initiator IP blocks.
> Prior to this change, the hwmod code expected to control hardreset
> lines itself, asserting them on shutdown and deasserting them upon
> enable.  But driver authors inside TI have commented to us that their
> drivers require direct control over these lines.  Unfortunately, these
> drivers haven't been posted publicly yet, so it's hard to determine
> exactly what is needed, a priori.  This change attempts to set forth
> some reasonable semantics that should be an improvement over the
> current code.
>
> The semantics implemented by this patch are as follows:
>
> - If the hwmod is not marked with HWMOD_INIT_NO_RESET, then assert all
>    associated hardreset lines during IP block setup.  This is intended
>    to place the IP blocks into a known state that will not interfere
>    with other devices during kernel boot.
>
> - IP blocks with hardreset lines will not be automatically enabled or
>    idled during setup.  Instead, they will be left in the INITIALIZED
>    state.
>
> - When the hwmod code is asked to enable, idle, or shutdown an IP
>    block with asserted hardreset lines, the hwmod code will do nothing.
>    The driver integration code must do the remaining work needed to
>    control these IP blocks.  Once this driver integration code is posted
>    to the lists, hopefully we can consolidate it and move it inside the
>    hwmod code.
>
> Custom reset functions for IP blocks with hardreset lines still should
> be supported and are strongly endorsed.  It is intended that every
> subsystem with hardreset lines should have a custom reset function
> that can place their subsystem into quiescent idle with the hardreset
> lines deasserted.
>
> This reverts most of commit 5365efbe29250a227502256cc912351fe2157b42
> ("OMAP: hwmod: Add hardreset management support").  Later code
> reorganizations caused the sequencing of the code from this patch to
> be changed, anyway.
>
> Signed-off-by: Paul Walmsley<paul@pwsan.com>
> Cc: Beno?t Cousson<b-cousson@ti.com>
> ---
>   arch/arm/mach-omap2/omap_hwmod.c |  140 ++++++++++++++++++++++----------------
>   1 file changed, 83 insertions(+), 57 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index e6aa14f..a5c3a9a 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -781,39 +781,6 @@ static int _omap4_wait_target_disable(struct omap_hwmod *oh)
>   }
>
>   /**
> - * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4
> - * @oh: struct omap_hwmod *
> - *
> - * Disable the PRCM module mode related to the hwmod @oh.
> - * Return EINVAL if the modulemode is not supported and 0 in case of success.
> - */
> -static int _omap4_disable_module(struct omap_hwmod *oh)
> -{
> -	int v;
> -
> -	/* The module mode does not exist prior OMAP4 */
> -	if (!cpu_is_omap44xx())
> -		return -EINVAL;
> -
> -	if (!oh->clkdm || !oh->prcm.omap4.modulemode)
> -		return -EINVAL;
> -
> -	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
> -
> -	omap4_cminst_module_disable(oh->clkdm->prcm_partition,
> -				    oh->clkdm->cm_inst,
> -				    oh->clkdm->clkdm_offs,
> -				    oh->prcm.omap4.clkctrl_offs);
> -
> -	v = _omap4_wait_target_disable(oh);
> -	if (v)
> -		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
> -			oh->name);
> -
> -	return 0;
> -}
> -
> -/**
>    * _count_mpu_irqs - count the number of MPU IRQ lines associated with @oh
>    * @oh: struct omap_hwmod *oh
>    *
> @@ -1378,6 +1345,66 @@ 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
> + * @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.
> + * This function is used to avoid executing some parts of the IP block
> + * enable/disable sequence if a hardreset line is set.
> + */
> +static bool _are_any_hardreset_lines_asserted(struct omap_hwmod *oh)
> +{
> +	int i;
> +
> +	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;
> +
> +	return false;
> +}
> +
> +/**
> + * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4
> + * @oh: struct omap_hwmod *
> + *
> + * Disable the PRCM module mode related to the hwmod @oh.
> + * Return EINVAL if the modulemode is not supported and 0 in case of success.
> + */
> +static int _omap4_disable_module(struct omap_hwmod *oh)
> +{
> +	int v;
> +
> +	/* The module mode does not exist prior OMAP4 */
> +	if (!cpu_is_omap44xx())
> +		return -EINVAL;
> +
> +	if (!oh->clkdm || !oh->prcm.omap4.modulemode)
> +		return -EINVAL;
> +
> +	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
> +
> +	omap4_cminst_module_disable(oh->clkdm->prcm_partition,
> +				    oh->clkdm->cm_inst,
> +				    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",
> +			oh->name);
> +
> +	return 0;
> +}
> +
> +/**
>    * _ocp_softreset - reset an omap_hwmod via the OCP_SYSCONFIG bit
>    * @oh: struct omap_hwmod *
>    *
> @@ -1519,7 +1546,7 @@ static int _reset(struct omap_hwmod *oh)
>    */
>   static int _enable(struct omap_hwmod *oh)
>   {
> -	int r, i;
> +	int r;
>   	int hwsup = 0;
>
>   	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
> @@ -1551,14 +1578,16 @@ static int _enable(struct omap_hwmod *oh)
>   	}
>
>   	/*
> -	 * If an IP contains HW reset lines, then de-assert them in order
> -	 * to allow the module state transition. Otherwise the PRCM will return
> -	 * Intransition status, and the init will failed.
> +	 * If an IP block contains HW reset lines and any 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
> +	 * detailed information is provided and the driver code is
> +	 * posted to the public lists, this is probably the best we
> +	 * can do.

Maybe we should keep that with some mechanism to prevent it for some IP 
like processors .

I guess that with that patch, we cannot enable anymore IPU/DSP and IVA 
at boot time.

Otherwise, it looks fine to me.

Thanks,
Benoit

>   	 */
> -	if (oh->_state == _HWMOD_STATE_INITIALIZED ||
> -	    oh->_state == _HWMOD_STATE_DISABLED)
> -		for (i = 0; i<  oh->rst_lines_cnt; i++)
> -			_deassert_hardreset(oh, oh->rst_lines[i].name);
> +	if (_are_any_hardreset_lines_asserted(oh))
> +		return 0;
>
>   	/* Mux pins for device runtime if populated */
>   	if (oh->mux&&  (!oh->mux->enabled ||
> @@ -1633,6 +1662,9 @@ static int _idle(struct omap_hwmod *oh)
>   		return -EINVAL;
>   	}
>
> +	if (_are_any_hardreset_lines_asserted(oh))
> +		return 0;
> +
>   	if (oh->class->sysc)
>   		_idle_sysc(oh);
>   	_del_initiator_dep(oh, mpu_oh);
> @@ -1715,6 +1747,9 @@ static int _shutdown(struct omap_hwmod *oh)
>   		return -EINVAL;
>   	}
>
> +	if (_are_any_hardreset_lines_asserted(oh))
> +		return 0;
> +
>   	pr_debug("omap_hwmod: %s: disabling\n", oh->name);
>
>   	if (oh->class->pre_shutdown) {
> @@ -1824,27 +1859,18 @@ static int __init _setup_reset(struct omap_hwmod *oh)
>   	if (oh->_state != _HWMOD_STATE_INITIALIZED)
>   		return -EINVAL;
>
> -	/*
> -	 * In the case of hwmod with hardreset that should not be
> -	 * de-assert at boot time, we have to keep the module
> -	 * initialized, because we cannot enable it properly with the
> -	 * reset asserted. Exit without warning because that behavior
> -	 * is expected.
> -	 */
> -	if ((oh->flags&  HWMOD_INIT_NO_RESET)&&  oh->rst_lines_cnt>  0)
> -		return 0;
> -
> -	r = _enable(oh);
> -	if (r) {
> -		pr_warning("omap_hwmod: %s: cannot be enabled (%d)\n",
> -			   oh->name, oh->_state);
> -		return 0;
> +	if (oh->rst_lines_cnt == 0) {
> +		r = _enable(oh);
> +		if (r) {
> +			pr_warning("omap_hwmod: %s: cannot be enabled for reset (%d)\n",
> +				   oh->name, oh->_state);
> +			return -EINVAL;
> +		}
>   	}
>
>   	if (!(oh->flags&  HWMOD_INIT_NO_RESET))
>   		r = _reset(oh);
>
> -
>   	return r;
>   }
>   /**

Re: [PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3474 bytes --]

Hi Benoît,

On Thu, 19 Apr 2012, Cousson, Benoit wrote:

> The concern of the people doing SW in these embedded processors was mainly
> because we were releasing the reset of processor without loading any SW first
> and thus the processor was executing some random instructions.
> 
> So if we consider a better hwmods definition, we can potentially fix the MMU
> case:
> 
>     'mmu_ipu':
>         'rst_ipu_mmu_cache'
> 
>     'mmu_dsp':
>         'rst_dsp_mmu_cache'
> 
>     'iva_config':
>         'rst_logic'

Wouldn't these still be "pseudo-hwmods?"  Or do each of these actually 
have address space, interconnect ports, etc.?

Another approach that might not require defining pseudo-hwmods is to 
define a per-hard-reset-line flag that could be used to alter the way the 
hwmod code handles the hardreset line.

> But then we do have the processor resets that have to be exposed somewhere.
> 
>     'ipu':
>         'rst_cpu0'
>         'rst_cpu1'
> 
>     'dsp':
>         'rst_dsp'
> 
>     'iva':
>         'rst_seq1'
>         'rst_seq2'
> 
> None of these one should be controlled automatically.

Don't we still want to put these IP blocks into reset until a driver for 
these IP blocks is loaded?

> >   	/*
> > -	 * If an IP contains HW reset lines, then de-assert them in order
> > -	 * to allow the module state transition. Otherwise the PRCM will
> > return
> > -	 * Intransition status, and the init will failed.
> > +	 * If an IP block contains HW reset lines and any 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
> > +	 * detailed information is provided and the driver code is
> > +	 * posted to the public lists, this is probably the best we
> > +	 * can do.
> 
> Maybe we should keep that with some mechanism to prevent it for some IP like
> processors .
> 
> I guess that with that patch, we cannot enable anymore IPU/DSP and IVA at boot
> time.

I am probably misunderstanding your comment, but I don't think there's any 
way at the moment to generically enable those IP blocks without driver 
integration code assistance.

If we leave the hardreset lines asserted in _enable(), then the PRCM 
status for those modules will be stuck in In-transition state, according 
to the previous comments in the code.  I think this will block PM idle 
transitions.  Also we have no way to restore the clockdomain idle settings 
during the second part of the hwmod enable sequence, I think, since it 
will need to be executed by driver integration code :-(

And I don't think we can deassert the hardreset lines in _enable() right 
now, for the reason that you mentioned in your message: unless the driver 
integration code implements a custom reset function, we don't have any way 
to load code into the IP block before deasserting the hardreset.

So at this point I'd propose that we encourage these driver authors to 
implement custom reset functions for their IP blocks that load firmware if 
needed, and put them into idle, similar to what omap3_iva_idle() does in 
mach-omap2/pm34xx.c.  If the custom reset function takes the IP block out 
of hardreset, then the subsequent hwmod enable/idle/shutdown calls should 
work, after this patch.

> Otherwise, it looks fine to me.

Thanks for the review.


- Paul

[PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Paul Walmsley]

Hi Beno?t,

On Thu, 19 Apr 2012, Cousson, Benoit wrote:

> The concern of the people doing SW in these embedded processors was mainly
> because we were releasing the reset of processor without loading any SW first
> and thus the processor was executing some random instructions.
> 
> So if we consider a better hwmods definition, we can potentially fix the MMU
> case:
> 
>     'mmu_ipu':
>         'rst_ipu_mmu_cache'
> 
>     'mmu_dsp':
>         'rst_dsp_mmu_cache'
> 
>     'iva_config':
>         'rst_logic'

Wouldn't these still be "pseudo-hwmods?"  Or do each of these actually 
have address space, interconnect ports, etc.?

Another approach that might not require defining pseudo-hwmods is to 
define a per-hard-reset-line flag that could be used to alter the way the 
hwmod code handles the hardreset line.

> But then we do have the processor resets that have to be exposed somewhere.
> 
>     'ipu':
>         'rst_cpu0'
>         'rst_cpu1'
> 
>     'dsp':
>         'rst_dsp'
> 
>     'iva':
>         'rst_seq1'
>         'rst_seq2'
> 
> None of these one should be controlled automatically.

Don't we still want to put these IP blocks into reset until a driver for 
these IP blocks is loaded?

> >   	/*
> > -	 * If an IP contains HW reset lines, then de-assert them in order
> > -	 * to allow the module state transition. Otherwise the PRCM will
> > return
> > -	 * Intransition status, and the init will failed.
> > +	 * If an IP block contains HW reset lines and any 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
> > +	 * detailed information is provided and the driver code is
> > +	 * posted to the public lists, this is probably the best we
> > +	 * can do.
> 
> Maybe we should keep that with some mechanism to prevent it for some IP like
> processors .
> 
> I guess that with that patch, we cannot enable anymore IPU/DSP and IVA at boot
> time.

I am probably misunderstanding your comment, but I don't think there's any 
way at the moment to generically enable those IP blocks without driver 
integration code assistance.

If we leave the hardreset lines asserted in _enable(), then the PRCM 
status for those modules will be stuck in In-transition state, according 
to the previous comments in the code.  I think this will block PM idle 
transitions.  Also we have no way to restore the clockdomain idle settings 
during the second part of the hwmod enable sequence, I think, since it 
will need to be executed by driver integration code :-(

And I don't think we can deassert the hardreset lines in _enable() right 
now, for the reason that you mentioned in your message: unless the driver 
integration code implements a custom reset function, we don't have any way 
to load code into the IP block before deasserting the hardreset.

So at this point I'd propose that we encourage these driver authors to 
implement custom reset functions for their IP blocks that load firmware if 
needed, and put them into idle, similar to what omap3_iva_idle() does in 
mach-omap2/pm34xx.c.  If the custom reset function takes the IP block out 
of hardreset, then the subsequent hwmod enable/idle/shutdown calls should 
work, after this patch.

> Otherwise, it looks fine to me.

Thanks for the review.


- Paul

Re: [PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Cousson, Benoit]

Hi Paul,

On 4/19/2012 7:17 PM, Paul Walmsley wrote:
> Hi Benoît,
>
> On Thu, 19 Apr 2012, Cousson, Benoit wrote:
>
>> The concern of the people doing SW in these embedded processors was mainly
>> because we were releasing the reset of processor without loading any SW first
>> and thus the processor was executing some random instructions.
>>
>> So if we consider a better hwmods definition, we can potentially fix the MMU
>> case:
>>
>>      'mmu_ipu':
>>          'rst_ipu_mmu_cache'
>>
>>      'mmu_dsp':
>>          'rst_dsp_mmu_cache'
>>
>>      'iva_config':
>>          'rst_logic'
>
> Wouldn't these still be "pseudo-hwmods?"  Or do each of these actually
> have address space, interconnect ports, etc.?

Yes, these ones are the only real IPs for MPU stand point, with 
interconnect port and configuration registers.

> Another approach that might not require defining pseudo-hwmods is to
> define a per-hard-reset-line flag that could be used to alter the way the
> hwmod code handles the hardreset line.

Yes, I think this is what Rajendra proposed long time ago...

>> But then we do have the processor resets that have to be exposed somewhere.
>>
>>      'ipu':
>>          'rst_cpu0'
>>          'rst_cpu1'
>>
>>      'dsp':
>>          'rst_dsp'
>>
>>      'iva':
>>          'rst_seq1'
>>          'rst_seq2'
>>
>> None of these one should be controlled automatically.
>
> Don't we still want to put these IP blocks into reset until a driver for
> these IP blocks is loaded?

Yes, indeed, my point is where are we going to declare these reset lines 
if we do not have any fake hwmods to store them.

>>>    	/*
>>> -	 * If an IP contains HW reset lines, then de-assert them in order
>>> -	 * to allow the module state transition. Otherwise the PRCM will
>>> return
>>> -	 * Intransition status, and the init will failed.
>>> +	 * If an IP block contains HW reset lines and any 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
>>> +	 * detailed information is provided and the driver code is
>>> +	 * posted to the public lists, this is probably the best we
>>> +	 * can do.
>>
>> Maybe we should keep that with some mechanism to prevent it for some IP like
>> processors .
>>
>> I guess that with that patch, we cannot enable anymore IPU/DSP and IVA at boot
>> time.
>
> I am probably misunderstanding your comment, but I don't think there's any
> way at the moment to generically enable those IP blocks without driver
> integration code assistance.

Well, for the MMU we can, these are regular IPs (almost). The real 
issues was for the processors.

> If we leave the hardreset lines asserted in _enable(), then the PRCM
> status for those modules will be stuck in In-transition state, according
> to the previous comments in the code.  I think this will block PM idle
> transitions.  Also we have no way to restore the clockdomain idle settings
> during the second part of the hwmod enable sequence, I think, since it
> will need to be executed by driver integration code :-(
>
> And I don't think we can deassert the hardreset lines in _enable() right
> now, for the reason that you mentioned in your message: unless the driver
> integration code implements a custom reset function, we don't have any way
> to load code into the IP block before deasserting the hardreset.

Fully agree, what I was trying to highlight is that we do have two types 
of hardreset here: the processors ones and the MMU / Logic ones.
Only the processors do require some special management because a 
firmware has to be loaded first.

> So at this point I'd propose that we encourage these driver authors to
> implement custom reset functions for their IP blocks that load firmware if
> needed, and put them into idle, similar to what omap3_iva_idle() does in
> mach-omap2/pm34xx.c.  If the custom reset function takes the IP block out
> of hardreset, then the subsequent hwmod enable/idle/shutdown calls should
> work, after this patch.

First they will have to release their driver :-)

Regards,
Benoit
--
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

[PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Cousson, Benoit]

Hi Paul,

On 4/19/2012 7:17 PM, Paul Walmsley wrote:
> Hi Beno?t,
>
> On Thu, 19 Apr 2012, Cousson, Benoit wrote:
>
>> The concern of the people doing SW in these embedded processors was mainly
>> because we were releasing the reset of processor without loading any SW first
>> and thus the processor was executing some random instructions.
>>
>> So if we consider a better hwmods definition, we can potentially fix the MMU
>> case:
>>
>>      'mmu_ipu':
>>          'rst_ipu_mmu_cache'
>>
>>      'mmu_dsp':
>>          'rst_dsp_mmu_cache'
>>
>>      'iva_config':
>>          'rst_logic'
>
> Wouldn't these still be "pseudo-hwmods?"  Or do each of these actually
> have address space, interconnect ports, etc.?

Yes, these ones are the only real IPs for MPU stand point, with 
interconnect port and configuration registers.

> Another approach that might not require defining pseudo-hwmods is to
> define a per-hard-reset-line flag that could be used to alter the way the
> hwmod code handles the hardreset line.

Yes, I think this is what Rajendra proposed long time ago...

>> But then we do have the processor resets that have to be exposed somewhere.
>>
>>      'ipu':
>>          'rst_cpu0'
>>          'rst_cpu1'
>>
>>      'dsp':
>>          'rst_dsp'
>>
>>      'iva':
>>          'rst_seq1'
>>          'rst_seq2'
>>
>> None of these one should be controlled automatically.
>
> Don't we still want to put these IP blocks into reset until a driver for
> these IP blocks is loaded?

Yes, indeed, my point is where are we going to declare these reset lines 
if we do not have any fake hwmods to store them.

>>>    	/*
>>> -	 * If an IP contains HW reset lines, then de-assert them in order
>>> -	 * to allow the module state transition. Otherwise the PRCM will
>>> return
>>> -	 * Intransition status, and the init will failed.
>>> +	 * If an IP block contains HW reset lines and any 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
>>> +	 * detailed information is provided and the driver code is
>>> +	 * posted to the public lists, this is probably the best we
>>> +	 * can do.
>>
>> Maybe we should keep that with some mechanism to prevent it for some IP like
>> processors .
>>
>> I guess that with that patch, we cannot enable anymore IPU/DSP and IVA at boot
>> time.
>
> I am probably misunderstanding your comment, but I don't think there's any
> way at the moment to generically enable those IP blocks without driver
> integration code assistance.

Well, for the MMU we can, these are regular IPs (almost). The real 
issues was for the processors.

> If we leave the hardreset lines asserted in _enable(), then the PRCM
> status for those modules will be stuck in In-transition state, according
> to the previous comments in the code.  I think this will block PM idle
> transitions.  Also we have no way to restore the clockdomain idle settings
> during the second part of the hwmod enable sequence, I think, since it
> will need to be executed by driver integration code :-(
>
> And I don't think we can deassert the hardreset lines in _enable() right
> now, for the reason that you mentioned in your message: unless the driver
> integration code implements a custom reset function, we don't have any way
> to load code into the IP block before deasserting the hardreset.

Fully agree, what I was trying to highlight is that we do have two types 
of hardreset here: the processors ones and the MMU / Logic ones.
Only the processors do require some special management because a 
firmware has to be loaded first.

> So at this point I'd propose that we encourage these driver authors to
> implement custom reset functions for their IP blocks that load firmware if
> needed, and put them into idle, similar to what omap3_iva_idle() does in
> mach-omap2/pm34xx.c.  If the custom reset function takes the IP block out
> of hardreset, then the subsequent hwmod enable/idle/shutdown calls should
> work, after this patch.

First they will have to release their driver :-)

Regards,
Benoit

Re: [PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2663 bytes --]

Hi Benoît,

On Thu, 19 Apr 2012, Cousson, Benoit wrote:

> On 4/19/2012 7:17 PM, Paul Walmsley wrote:
> > On Thu, 19 Apr 2012, Cousson, Benoit wrote:
> > 
> > > The concern of the people doing SW in these embedded processors was mainly
> > > because we were releasing the reset of processor without loading any SW
> > > first
> > > and thus the processor was executing some random instructions.
> > > 
> > > So if we consider a better hwmods definition, we can potentially fix the
> > > MMU
> > > case:
> > > 
> > >      'mmu_ipu':
> > >          'rst_ipu_mmu_cache'
> > > 
> > >      'mmu_dsp':
> > >          'rst_dsp_mmu_cache'
> > > 
> > >      'iva_config':
> > >          'rst_logic'
> > 
> > Wouldn't these still be "pseudo-hwmods?"  Or do each of these actually
> > have address space, interconnect ports, etc.?
> 
> Yes, these ones are the only real IPs for MPU stand point, with interconnect
> port and configuration registers.

These are the MMUs documented in Chapter 20 of the OMAP4430 TRM 
vX, right?  

I don't really understand the interaction of the hardreset lines with 
these IPs.  Chapter 20 doesn't seem to mention the PRCM-controllable 
hardreset lines at all.  It only mentions the OCP_SYSCONFIG registers 
associated with them.

Meanwhile Table 3-375 "RM_DSP_RSTCTRL" mentions a DSP MMU, cache, and 
slave interface reset line.  Is that referring to the same MMU that is 
mentioned in Chapter 20?  The end of Section 20.2 "MMU Integration" 
mentions two different DSP MMUs, a "shared cache MMU" and an "L2 MMU" - 
maybe the hardreset line only pertains to the first MMU?

> > > But then we do have the processor resets that have to be exposed
> > > somewhere.
> > > 
> > >      'ipu':
> > >          'rst_cpu0'
> > >          'rst_cpu1'
> > > 
> > >      'dsp':
> > >          'rst_dsp'
> > > 
> > >      'iva':
> > >          'rst_seq1'
> > >          'rst_seq2'
> > > 
> > > None of these one should be controlled automatically.
> > 
> > Don't we still want to put these IP blocks into reset until a driver for
> > these IP blocks is loaded?
> 
> Yes, indeed, my point is where are we going to declare these reset lines if we
> do not have any fake hwmods to store them.

Wouldn't we associate them with the processor hwmods?  e.g., 
omap44xx_iva_hwmod, omap44xx_dsp_hwmod ?

> Well, for the MMU we can, these are regular IPs (almost). The real issues was
> for the processors.

Omar, do you know  how these hardreset lines interact with the MMUs 
described in the TRM Chapter 20?

> First they will have to release their driver :-)

Hehe, indeed.


- Paul

[PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Paul Walmsley]

Hi Beno?t,

On Thu, 19 Apr 2012, Cousson, Benoit wrote:

> On 4/19/2012 7:17 PM, Paul Walmsley wrote:
> > On Thu, 19 Apr 2012, Cousson, Benoit wrote:
> > 
> > > The concern of the people doing SW in these embedded processors was mainly
> > > because we were releasing the reset of processor without loading any SW
> > > first
> > > and thus the processor was executing some random instructions.
> > > 
> > > So if we consider a better hwmods definition, we can potentially fix the
> > > MMU
> > > case:
> > > 
> > >      'mmu_ipu':
> > >          'rst_ipu_mmu_cache'
> > > 
> > >      'mmu_dsp':
> > >          'rst_dsp_mmu_cache'
> > > 
> > >      'iva_config':
> > >          'rst_logic'
> > 
> > Wouldn't these still be "pseudo-hwmods?"  Or do each of these actually
> > have address space, interconnect ports, etc.?
> 
> Yes, these ones are the only real IPs for MPU stand point, with interconnect
> port and configuration registers.

These are the MMUs documented in Chapter 20 of the OMAP4430 TRM 
vX, right?  

I don't really understand the interaction of the hardreset lines with 
these IPs.  Chapter 20 doesn't seem to mention the PRCM-controllable 
hardreset lines at all.  It only mentions the OCP_SYSCONFIG registers 
associated with them.

Meanwhile Table 3-375 "RM_DSP_RSTCTRL" mentions a DSP MMU, cache, and 
slave interface reset line.  Is that referring to the same MMU that is 
mentioned in Chapter 20?  The end of Section 20.2 "MMU Integration" 
mentions two different DSP MMUs, a "shared cache MMU" and an "L2 MMU" - 
maybe the hardreset line only pertains to the first MMU?

> > > But then we do have the processor resets that have to be exposed
> > > somewhere.
> > > 
> > >      'ipu':
> > >          'rst_cpu0'
> > >          'rst_cpu1'
> > > 
> > >      'dsp':
> > >          'rst_dsp'
> > > 
> > >      'iva':
> > >          'rst_seq1'
> > >          'rst_seq2'
> > > 
> > > None of these one should be controlled automatically.
> > 
> > Don't we still want to put these IP blocks into reset until a driver for
> > these IP blocks is loaded?
> 
> Yes, indeed, my point is where are we going to declare these reset lines if we
> do not have any fake hwmods to store them.

Wouldn't we associate them with the processor hwmods?  e.g., 
omap44xx_iva_hwmod, omap44xx_dsp_hwmod ?

> Well, for the MMU we can, these are regular IPs (almost). The real issues was
> for the processors.

Omar, do you know  how these hardreset lines interact with the MMUs 
described in the TRM Chapter 20?

> First they will have to release their driver :-)

Hehe, indeed.


- Paul

Re: [PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Ramirez Luna, Omar]

Hi Paul,

On Thu, Apr 19, 2012 at 2:46 PM, Paul Walmsley <paul@pwsan.com> wrote:
> Hi Benoît,
>
> On Thu, 19 Apr 2012, Cousson, Benoit wrote:
>
>> On 4/19/2012 7:17 PM, Paul Walmsley wrote:
>> > On Thu, 19 Apr 2012, Cousson, Benoit wrote:
>> >
>> > > The concern of the people doing SW in these embedded processors was mainly
>> > > because we were releasing the reset of processor without loading any SW
>> > > first
>> > > and thus the processor was executing some random instructions.
>> > >
>> > > So if we consider a better hwmods definition, we can potentially fix the
>> > > MMU
>> > > case:
>> > >
>> > >      'mmu_ipu':
>> > >          'rst_ipu_mmu_cache'
>> > >
>> > >      'mmu_dsp':
>> > >          'rst_dsp_mmu_cache'
>> > >
>> > >      'iva_config':
>> > >          'rst_logic'
>> >
>> > Wouldn't these still be "pseudo-hwmods?"  Or do each of these actually
>> > have address space, interconnect ports, etc.?
>>
>> Yes, these ones are the only real IPs for MPU stand point, with interconnect
>> port and configuration registers.
>
> These are the MMUs documented in Chapter 20 of the OMAP4430 TRM
> vX, right?
>
> I don't really understand the interaction of the hardreset lines with
> these IPs.  Chapter 20 doesn't seem to mention the PRCM-controllable
> hardreset lines at all.  It only mentions the OCP_SYSCONFIG registers
> associated with them.
>
> Meanwhile Table 3-375 "RM_DSP_RSTCTRL" mentions a DSP MMU, cache, and
> slave interface reset line.  Is that referring to the same MMU that is
> mentioned in Chapter 20?  The end of Section 20.2 "MMU Integration"
> mentions two different DSP MMUs, a "shared cache MMU" and an "L2 MMU" -
> maybe the hardreset line only pertains to the first MMU?

I don't thinks so, because trying to read the L2 MMU registers with
DSP RST2 asserted causes an L3 error.

DSP's "shared cache MMU" refers to section 5.3.2.5 Attribute MMU which
is inside the dsp megamodule, OTOH "L2 MMU" refers to section 5.3.4
MMU, the latter is the one being referenced in section 20.

>> > > But then we do have the processor resets that have to be exposed
>> > > somewhere.
>> > >
>> > >      'ipu':
>> > >          'rst_cpu0'
>> > >          'rst_cpu1'
>> > >
>> > >      'dsp':
>> > >          'rst_dsp'
>> > >
>> > >      'iva':
>> > >          'rst_seq1'
>> > >          'rst_seq2'
>> > >
>> > > None of these one should be controlled automatically.
>> >
>> > Don't we still want to put these IP blocks into reset until a driver for
>> > these IP blocks is loaded?
>>
>> Yes, indeed, my point is where are we going to declare these reset lines if we
>> do not have any fake hwmods to store them.
>
> Wouldn't we associate them with the processor hwmods?  e.g.,
> omap44xx_iva_hwmod, omap44xx_dsp_hwmod ?
>
>> Well, for the MMU we can, these are regular IPs (almost). The real issues was
>> for the processors.
>
> Omar, do you know  how these hardreset lines interact with the MMUs
> described in the TRM Chapter 20?

For M3, RM_MPU_M3_RSTCTRL[2].RST3 releases both the cache and mmu
interfaces, this one is needed if you want to program the mmu by
touching any of the MMU registers, typical initialization is to
deassert this line and then enable the M3 clock (since it is shared
with its MMU) which should allow the reset to complete *only* after
the clock has been enabled.

For DSP, RM_DSP_RSTCTRL[1].RST2 it also releases dsp, cache and mmu
(however the dsp doesn't boot until RST1 is deasserted), it is the
same rationale as above; you deassert RST2, enable dsp clock (so reset
can complete) and then program mmu.

Touching any mmu registers without deasserting its reset line (even if
the associated clock is ON) generates a L3 error.

This can be found in 3.5.6 Reset sequences for DSP and M3.

Regards,

Omar

[PATCH v2 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior

[Ramirez Luna, Omar]

Hi Paul,

On Thu, Apr 19, 2012 at 2:46 PM, Paul Walmsley <paul@pwsan.com> wrote:
> Hi Beno?t,
>
> On Thu, 19 Apr 2012, Cousson, Benoit wrote:
>
>> On 4/19/2012 7:17 PM, Paul Walmsley wrote:
>> > On Thu, 19 Apr 2012, Cousson, Benoit wrote:
>> >
>> > > The concern of the people doing SW in these embedded processors was mainly
>> > > because we were releasing the reset of processor without loading any SW
>> > > first
>> > > and thus the processor was executing some random instructions.
>> > >
>> > > So if we consider a better hwmods definition, we can potentially fix the
>> > > MMU
>> > > case:
>> > >
>> > > ? ? ?'mmu_ipu':
>> > > ? ? ? ? ?'rst_ipu_mmu_cache'
>> > >
>> > > ? ? ?'mmu_dsp':
>> > > ? ? ? ? ?'rst_dsp_mmu_cache'
>> > >
>> > > ? ? ?'iva_config':
>> > > ? ? ? ? ?'rst_logic'
>> >
>> > Wouldn't these still be "pseudo-hwmods?" ?Or do each of these actually
>> > have address space, interconnect ports, etc.?
>>
>> Yes, these ones are the only real IPs for MPU stand point, with interconnect
>> port and configuration registers.
>
> These are the MMUs documented in Chapter 20 of the OMAP4430 TRM
> vX, right?
>
> I don't really understand the interaction of the hardreset lines with
> these IPs. ?Chapter 20 doesn't seem to mention the PRCM-controllable
> hardreset lines at all. ?It only mentions the OCP_SYSCONFIG registers
> associated with them.
>
> Meanwhile Table 3-375 "RM_DSP_RSTCTRL" mentions a DSP MMU, cache, and
> slave interface reset line. ?Is that referring to the same MMU that is
> mentioned in Chapter 20? ?The end of Section 20.2 "MMU Integration"
> mentions two different DSP MMUs, a "shared cache MMU" and an "L2 MMU" -
> maybe the hardreset line only pertains to the first MMU?

I don't thinks so, because trying to read the L2 MMU registers with
DSP RST2 asserted causes an L3 error.

DSP's "shared cache MMU" refers to section 5.3.2.5 Attribute MMU which
is inside the dsp megamodule, OTOH "L2 MMU" refers to section 5.3.4
MMU, the latter is the one being referenced in section 20.

>> > > But then we do have the processor resets that have to be exposed
>> > > somewhere.
>> > >
>> > > ? ? ?'ipu':
>> > > ? ? ? ? ?'rst_cpu0'
>> > > ? ? ? ? ?'rst_cpu1'
>> > >
>> > > ? ? ?'dsp':
>> > > ? ? ? ? ?'rst_dsp'
>> > >
>> > > ? ? ?'iva':
>> > > ? ? ? ? ?'rst_seq1'
>> > > ? ? ? ? ?'rst_seq2'
>> > >
>> > > None of these one should be controlled automatically.
>> >
>> > Don't we still want to put these IP blocks into reset until a driver for
>> > these IP blocks is loaded?
>>
>> Yes, indeed, my point is where are we going to declare these reset lines if we
>> do not have any fake hwmods to store them.
>
> Wouldn't we associate them with the processor hwmods? ?e.g.,
> omap44xx_iva_hwmod, omap44xx_dsp_hwmod ?
>
>> Well, for the MMU we can, these are regular IPs (almost). The real issues was
>> for the processors.
>
> Omar, do you know ?how these hardreset lines interact with the MMUs
> described in the TRM Chapter 20?

For M3, RM_MPU_M3_RSTCTRL[2].RST3 releases both the cache and mmu
interfaces, this one is needed if you want to program the mmu by
touching any of the MMU registers, typical initialization is to
deassert this line and then enable the M3 clock (since it is shared
with its MMU) which should allow the reset to complete *only* after
the clock has been enabled.

For DSP, RM_DSP_RSTCTRL[1].RST2 it also releases dsp, cache and mmu
(however the dsp doesn't boot until RST1 is deasserted), it is the
same rationale as above; you deassert RST2, enable dsp clock (so reset
can complete) and then program mmu.

Touching any mmu registers without deasserting its reset line (even if
the associated clock is ON) generates a L3 error.

This can be found in 3.5.6 Reset sequences for DSP and M3.

Regards,

Omar