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

login
register
mail settings
Submitter Paul Walmsley
Date Feb. 28, 2012, 5:37 a.m.
Message ID <20120228053657.16278.76132.stgit@dusk>
Download mbox | patch
Permalink /patch/143331/
State New
Headers show

Comments

Paul Walmsley - Feb. 28, 2012, 5:37 a.m.
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(-)
Ramirez Luna, Omar - March 15, 2012, 12:31 a.m.
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
Paul Walmsley - March 15, 2012, 6:25 a.m.
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
Ramirez Luna, Omar - March 15, 2012, 3:21 p.m.
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

Patch

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