Hi Rajendra,
On Tue, 13 Mar 2012, Rajendra Nayak wrote:
> omap_hwmod_softreset() does not seem to wait for reset status
> after doing a softreset. Make it use _ocp_softreset() instead
> which does this correctly.
>
> Signed-off-by: Rajendra Nayak <rnayak@xxxxxx>
> Cc: Benoit Cousson <b-cousson@xxxxxx>
> Cc: Paul Walmsley <paul@xxxxxxxxx>
> Cc: Anand Gadiyar <gadiyar@xxxxxx>
> Cc: Shubhrajyoti D <shubhrajyoti@xxxxxx>
Just noticed that this change results in I2C softreset failed messages on
OMAP2/3/4 on v3.4-rc2. For example, on 2420:
[ 0.200378] omap_hwmod: i2c1: softreset failed (waited 10000 usec)
[ 0.222076] omap_hwmod: i2c2: softreset failed (waited 10000 usec)
Looking more closely at the code, I think the intention of the original
code was basically correct. HDQ and I2C both need to execute some custom
reset code after the SOFTRESET bit is set, but before the RESETDONE bit is
tested. After this patch, the internal hwmod code tries to wait for
RESETDONE to change, before the custom code runs -- and that is going to
fail.
Just out of curiosity, was the change in this patch prompted by some code
that needed the change? Or was this a theoretical problem, driven by a
code review? If the latter, we should probably revert this patch, and
maybe improve the documentation and omap_hwmod_softreset() function name
to clarify what it actually does. But if it's the former, it looks like
we'll need a way to support both the full, standard SOFTRESET sequence
(perhaps via omap_hwmod_reset() ?) while also supporting the HDQ/I2C
custom reset code.
regards,
- Paul
> ---
> arch/arm/mach-omap2/omap_hwmod.c | 14 ++------------
> 1 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 6c0e7eb..0c64853 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2423,20 +2423,10 @@ void omap_hwmod_write(u32 v, struct omap_hwmod *oh, u16 reg_offs)
> */
> int omap_hwmod_softreset(struct omap_hwmod *oh)
> {
> - u32 v;
> - int ret;
> -
> - if (!oh || !(oh->_sysc_cache))
> + if (!oh)
> return -EINVAL;
>
> - v = oh->_sysc_cache;
> - ret = _set_softreset(oh, &v);
> - if (ret)
> - goto error;
> - _write_sysconfig(v, oh);
> -
> -error:
> - return ret;
> + return _ocp_softreset(oh);
> }
>
> /**
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
