[v3] ARM: omap4: i2c reset regs postidle

omap4430 get i2c timeouts at each access after an NACK message.
OMAP_I2C_FLAG_RESET_REGS_POSTIDLE fix it.

Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>
Tested-by: Andy Green <andy.green@linaro.org>
Acked-by: Andy Green <andy.green@linaro.org>
---
Works on top of tmlind linux-omap-2.6.git
v3: commit message
v2: add Tested-by / Acked-by

 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Jan Weitzel <j.weitzel@phytec.de> writes:

> omap4430 get i2c timeouts at each access after an NACK message.
> OMAP_I2C_FLAG_RESET_REGS_POSTIDLE fix it.

We need a little better changelog here.  Specifically, *why* does this
flag fix the problem?   What exactly is going wrong such that this fix
is needed.

Does this happen all the time?  only when off-mode is used?  etc.

Looking closer at how this flag is used in the driver, I think the
driver's usage of runtime PM is a bit broken.  I'm not sure if it's
related to this problem, but I'll send a short series in a little bit to
clean up the runtime PM usage, and get rid of the dev->idle flag which
duplicates usage counting already provided by runtime PM.

Kevin

> Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>
> Tested-by: Andy Green <andy.green@linaro.org>
> Acked-by: Andy Green <andy.green@linaro.org>
> ---
> Works on top of tmlind linux-omap-2.6.git
> v3: commit message
> v2: add Tested-by / Acked-by
>
>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index 5f4a1b2..3d3b4f4 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -2130,7 +2130,8 @@ static struct omap_hwmod_class omap44xx_i2c_hwmod_class = {
>  };
>  
>  static struct omap_i2c_dev_attr i2c_dev_attr = {
> -	.flags  = OMAP_I2C_FLAG_BUS_SHIFT_NONE,
> +	.flags  = OMAP_I2C_FLAG_BUS_SHIFT_NONE |
> +		  OMAP_I2C_FLAG_RESET_REGS_POSTIDLE,
>  };
>  
>  /* i2c1 */

Am Dienstag, den 17.05.2011, 16:12 +0200 schrieb Kevin Hilman:
> Jan Weitzel <j.weitzel@phytec.de> writes:
> 
> > omap4430 get i2c timeouts at each access after an NACK message.
> > OMAP_I2C_FLAG_RESET_REGS_POSTIDLE fix it.
> 
> We need a little better changelog here.  Specifically, *why* does this
> flag fix the problem?   What exactly is going wrong such that this fix
> is needed.
> 
> Does this happen all the time?  only when off-mode is used?  etc.
Yes all time. i2cdetect say "controller timed out" every two adresses.

> Looking closer at how this flag is used in the driver, I think the
> driver's usage of runtime PM is a bit broken.  I'm not sure if it's
> related to this problem, but I'll send a short series in a little bit to
> clean up the runtime PM usage, and get rid of the dev->idle flag which
> duplicates usage counting already provided by runtime PM.
In the isr OMAP_I2C_STAT_NACK set OMAP_I2C_CON_REG to OMAP_I2C_CON_STP,
clearing all other flags. Maybe this is wrong?

On OMAP3 the OMAP_I2C_CON_REG flags are set again in the
OMAP_I2C_FLAG_RESET_REGS_POSTIDLE path of omap_i2c_unidle.
I am not sure if this is also the right place for OMAP4 

Jan

Jan Weitzel <J.Weitzel@phytec.de> writes:

> Am Dienstag, den 17.05.2011, 16:12 +0200 schrieb Kevin Hilman:
>> Jan Weitzel <j.weitzel@phytec.de> writes:
>> 
>> > omap4430 get i2c timeouts at each access after an NACK message.
>> > OMAP_I2C_FLAG_RESET_REGS_POSTIDLE fix it.
>> 
>> We need a little better changelog here.  Specifically, *why* does this
>> flag fix the problem?   What exactly is going wrong such that this fix
>> is needed.
>> 
>> Does this happen all the time?  only when off-mode is used?  etc.
> Yes all time. i2cdetect say "controller timed out" every two adresses.
>
>> Looking closer at how this flag is used in the driver, I think the
>> driver's usage of runtime PM is a bit broken.  I'm not sure if it's
>> related to this problem, but I'll send a short series in a little bit to
>> clean up the runtime PM usage, and get rid of the dev->idle flag which
>> duplicates usage counting already provided by runtime PM.
> In the isr OMAP_I2C_STAT_NACK set OMAP_I2C_CON_REG to OMAP_I2C_CON_STP,
> clearing all other flags. Maybe this is wrong?
>
> On OMAP3 the OMAP_I2C_CON_REG flags are set again in the
> OMAP_I2C_FLAG_RESET_REGS_POSTIDLE path of omap_i2c_unidle.
> I am not sure if this is also the right place for OMAP4 

Right, and I'm not sure either.  That's why we need a better
explanation/changelog about exactly what is happening and why this is
the right fix.

Kevin

* Jan Weitzel <j.weitzel@phytec.de> [110614 03:01]:
> On OMAP4 OMAP_I2C_STAT_NACK is causing a timeout on the next access.
> The isr cleans all flags in OMAP_I2C_CON_REG by setting OMAP_I2C_CON_STP
> OMAP_I2C_CON_STP is also set in omap_i2c_xfer_msg on the last message.
> 
> According to the TI TSR the sequence for OMAP_I2C_STAT_NACK and
> OMAP_I2C_STAT_AL are nearly the same.
> Removing the OMAP_I2C_CON_STP part in the isr fix the problem.
> Tested on OMAP4430 and OMAP3530 (here NACK was not a problem)
> 
> Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>

Please resend this to Ben Dooks and linux-i2c list Cc'd and also mention
that this fixes booting on 2430sdp that's been failing for a while now.

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  drivers/i2c/busses/i2c-omap.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 58a58c7..670f2a2 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -836,11 +836,9 @@ complete:
>  				~(OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR |
>  				OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
>  
> -		if (stat & OMAP_I2C_STAT_NACK) {
> +		if (stat & OMAP_I2C_STAT_NACK)
>  			err |= OMAP_I2C_STAT_NACK;
> -			omap_i2c_write_reg(dev, OMAP_I2C_CON_REG,
> -					   OMAP_I2C_CON_STP);
> -		}
> +
>  		if (stat & OMAP_I2C_STAT_AL) {
>  			dev_err(dev->dev, "Arbitration lost\n");
>  			err |= OMAP_I2C_STAT_AL;
> -- 
> 1.7.0.4
>