[PATCH] ARM: omap4: i2c reset regs postidle

[PATCH] ARM: omap4: i2c reset regs postidle

[Jan Weitzel]

Without OMAP_I2C_FLAG_RESET_REGS_POSTIDLE I got i2c controller
timeouts on each accsess after an NACK message.
Taking this flag fix it.

This patch is on top of the i2c feature implementation flags
patches

Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>
---
 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 */
-- 
1.7.0.4

Re: [PATCH] ARM: omap4: i2c reset regs postidle

[Andy Green]

On 04/21/2011 12:47 PM, Somebody in the thread at some point said:
> Without OMAP_I2C_FLAG_RESET_REGS_POSTIDLE I got i2c controller
> timeouts on each accsess after an NACK message.
> Taking this flag fix it.

Ahhh that will explain why if you accidentally configure LM75 system 
monitor support, which isn't there on Panda, the I2C controllers die 
slowly on boot.  Good catch!

-Andy

Re: [PATCH] ARM: omap4: i2c reset regs postidle

[Jan Weitzel]

Am Donnerstag, den 21.04.2011, 13:13 +0100 schrieb Andy Green:
> On 04/21/2011 12:47 PM, Somebody in the thread at some point said:
> > Without OMAP_I2C_FLAG_RESET_REGS_POSTIDLE I got i2c controller
> > timeouts on each accsess after an NACK message.
> > Taking this flag fix it.
> 
> Ahhh that will explain why if you accidentally configure LM75 system 
> monitor support, which isn't there on Panda, the I2C controllers die 
> slowly on boot.  Good catch!

Can I get an Acked-by ?

Jan

Re: [PATCH] ARM: omap4: i2c reset regs postidle

[Andy Green]

On 05/16/2011 09:56 AM, Somebody in the thread at some point said:
> Am Donnerstag, den 21.04.2011, 13:13 +0100 schrieb Andy Green:
>> On 04/21/2011 12:47 PM, Somebody in the thread at some point said:
>>> Without OMAP_I2C_FLAG_RESET_REGS_POSTIDLE I got i2c controller
>>> timeouts on each accsess after an NACK message.
>>> Taking this flag fix it.
>>
>> Ahhh that will explain why if you accidentally configure LM75 system
>> monitor support, which isn't there on Panda, the I2C controllers die
>> slowly on boot.  Good catch!
>
> Can I get an Acked-by ?

Sure, I had it on my tree for a while too.

Tested-by: Andy Green <andy.green@linaro.org>
Acked-by: Andy Green <andy.green@linaro.org>

-Andy

[PATCH v2] ARM: omap4: i2c reset regs postidle

[Jan Weitzel]

Without OMAP_I2C_FLAG_RESET_REGS_POSTIDLE I got i2c controller
timeouts on each accsess after an NACK message.
Taking this flag fix it.

This patch is on top of the i2c feature implementation flags
patches

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>
---
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 */
-- 
1.7.0.4

Re: [PATCH v2] ARM: omap4: i2c reset regs postidle

[Cousson, Benoit]

Hi Jan,

On 5/16/2011 11:18 AM, Jan Weitzel wrote:
> Without OMAP_I2C_FLAG_RESET_REGS_POSTIDLE I got i2c controller
> timeouts on each accsess after an NACK message.
> Taking this flag fix it.
>
> This patch is on top of the i2c feature implementation flags
> patches

Could you please give us more information and potentially provide a link 
to the patches you are referring to?
It will help us assessing the need for such flag in the hwmod data.

Please note that the changelog will stay forever in the GIT tree, so you 
should avoid using "the patch" or making reference to something that is 
not an existing commit id or email archive.
You should fix as well the typo.

Thanks,
Benoit

[PATCH v3] ARM: omap4: i2c reset regs postidle

[Jan Weitzel]

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(-)

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 */
-- 
1.7.0.4

Re: [PATCH v3] ARM: omap4: i2c reset regs postidle

[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.

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 */

Re: [PATCH v3] ARM: omap4: i2c reset regs postidle

[Jan Weitzel]

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

Re: [PATCH v3] ARM: omap4: i2c reset regs postidle

[Kevin Hilman]

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

[RFC] ARM: OMAP2+ i2c NACK without STP

[Jan Weitzel]

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

Re: [RFC] ARM: OMAP2+ i2c NACK without STP

[Tony Lindgren]

* 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
> 

[PATCH] ARM: OMAP2+ i2c NACK without STP

[Jan Weitzel]

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)
Fixes also booting on 2430sdp.

Signed-off-by: Jan Weitzel <j.weitzel-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 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

Re: [PATCH] ARM: OMAP2+ i2c NACK without STP

[T Krishnamoorthy, Balaji]

On Tue, Jun 14, 2011 at 4:24 PM, Jan Weitzel <j.weitzel@phytec.de> wrote:
> 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)
> Fixes also booting on 2430sdp.
>
> Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>
> Acked-by: Tony Lindgren <tony@atomide.com>

Acked-by: Balaji T K <balajitk@ti.com>
Can you please queue this patch

This patch has been tested in linux-omap tree
http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap.git;a=commit;h=464fca36d98b97ccd05e4f200ab7483d3acc8345

> ---
>  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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP2+ i2c NACK without STP

[Kevin Hilman]

"T Krishnamoorthy, Balaji" <balajitk-l0cyMroinI0@public.gmane.org> writes:

> On Tue, Jun 14, 2011 at 4:24 PM, Jan Weitzel <j.weitzel-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org> wrote:
>> 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)
>> Fixes also booting on 2430sdp.
>>
>> Signed-off-by: Jan Weitzel <j.weitzel-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
>> Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
>
> Acked-by: Balaji T K <balajitk-l0cyMroinI0@public.gmane.org>
> Can you please queue this patch
>
> This patch has been tested in linux-omap tree
> http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap.git;a=commit;h=464fca36d98b97ccd05e4f200ab7483d3acc8345

I have this patch in my queue (branch: for_3.3/i2c/misc), but
unfortunately,  the maintainer has not responded to any of my queries or
pull requests for a couple months now.

I have sent some more off-list queries recently in hopes to get this
moving again.

Kevin