Re: [PATCH v2 2/2] ARM: omap: hwmod: Make omap_hwmod_softreset wait for reset status

[Rajendra Nayak <rnayak@ti.com>]

Hi Paul,

On Wednesday 11 April 2012 05:41 AM, Paul Walmsley wrote:
> 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?

The changes were done to fix up random L3 interconnect errors that
Anand G was seeing(during i2c reset operation) on some customer 
platforms. I seem to have completely overlooked the I2C_EN programming 
part done in omap_i2c_reset() function when I did the patch.

While the patch did fix the issue for Anand, I guess it
was because of the additional delay post reset, waiting on the
RESETDONE bit and timing out, before accessing the i2c_con register.
So looks like this patch should certainly be reverted but atleast some
platforms/modules (the issue was seen on OMAP4/i2c) will need some
delay between the omap_hwmod_softreset() call and any subsequent module
register accesses.
The patch from Fernando [1] can be quite useful if we can use the
'srst_udelay' field to populate the appropriate delay needed which can
then be used up in omap_hwmod_softreset() function.
I am out sick today, but I can try some on these lines tomorrow once
I am in office, if the approach seems fine to you.
Btw, is [1] queued already by you/Benoit to go upstream or are there
issues with it?

regards,
Rajendra

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-February/086713.html