Message ID | 20120228053657.16278.76132.stgit@dusk |
---|---|
State | New |
Headers | show |
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
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
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
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; }
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(-)