[PATCH 1/2 v2] OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

[PATCH 1/2 v2] OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

[Avinash.H.M.]

Date: Thu, 7 Apr 2011 12:12:37 +0530
From: "Mahadeva, Avinash" <avinashhm@ti.com>
To: Kevin Hilman <khilman@ti.com>
Cc: linux-omap at vger.kernel.org, linux-arm-kernel at lists.infradead.org,
	Rajendra Nayak <rnayak@ti.com>, Paul Walmsley <paul@pwsan.com>,
	Benoit Cousson <b-cousson@ti.com>
Subject: Re: [PATCH 1/2 v2] OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

On Tue, Apr 5, 2011 at 11:57 PM, Kevin Hilman <khilman@ti.com> wrote:

> "Avinash.H.M" <avinashhm@ti.com> writes:
>
> > The i2c module has a special reset sequence. The sequence is
> > - Disable the I2C.
> > - Write to SOFTRESET bit.
> > - Enable the I2C.
> > - Poll on the RESETDONE bit.
> > This sequence must be followed for i2c reset in omap2, omap3. The
> sequence is
> > implemented as a function and the i2c_class is updated with the correct
> > 'reset' pointer.
> >
> > Cc: Rajendra Nayak <rnayak@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Benoit Cousson <b-cousson@ti.com>
> > Cc: Kevin Hilman <khilman@ti.com>
> > Signed-off-by: Avinash.H.M <avinashhm@ti.com>
>
> [...]
>
> > +
> > +/**
> > + * omap_i2c_reset- reset the omap i2c module.
> > + * @oh: struct omap_hwmod *
> > + *
> > + * The i2c moudle in omap2, omap3 had a special sequence to reset. The
> > + * sequence is:
> > + * - Disable the I2C.
> > + * - Write to SOFTRESET bit.
> > + * - Enable the I2C.
> > + * - Poll on the RESETDONE bit.
> > + * The sequence is implemented in below function. This is called for
> 2420,
> > + * 2430 and omap3.
> > + */
> > +int omap_i2c_reset(struct omap_hwmod *oh)
> > +{
> > +     u32 v;
> > +     int c = 0;
> > +
> > +     /* Disable I2C */
> > +     v = omap_hwmod_read(oh, I2C_CON_OFFSET);
> > +     v = v & ~I2C_EN;
> > +     omap_hwmod_write(v, oh, I2C_CON_OFFSET);
> > +
> > +     /* Write to the SOFTRESET bit */
> > +     v = oh->_sysc_cache;
> > +     v |= (0x1 << oh->class->sysc->sysc_fields->srst_shift);
> > +
> > +     oh->_sysc_cache = v;
> > +     omap_hwmod_write(v, oh, oh->class->sysc->sysc_offs);
>
> Direct SYSCONFIG access isn't right here.   This should go through
> omap_hwmod.
>
> What is probably needed is exposing _ocp_softreset to device code
> via something like omap_hwmod_ocp_softreset() and calling that here.
>
>

* Sorry, somehow the mail bounced from l-o. Resending it agian. *

Hi Kevin ,

thanks for the review.

OK. I ll create a wrapper function ' omap_hwmod_ocp_softreset'
around '_ocp_softreset' and call it here .

b r ,
- avinash.



> Kevin
>

[PATCH 1/2 v2] OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

[Avinash.H.M.]

> > > +     oh->_sysc_cache = v;
> > > +     omap_hwmod_write(v, oh, oh->class->sysc->sysc_offs);
> >
> > Direct SYSCONFIG access isn't right here.   This should go through
> > omap_hwmod.
> >
> > What is probably needed is exposing _ocp_softreset to device code
> > via something like omap_hwmod_ocp_softreset() and calling that here.
> >

Hi Kevin , 

Looking more closely, i realised that, the sequence of _ocp_softreset
doesn't work for I2C. It has a special programming sequence which needs
to be followed to reset the IP. That was the reason, we created
omap_i2c_reset.  So i feel we need not expose _ocp_softreset.

Since, the problem here is accessing the SYSCONFIG here, instead, what i
can do is, something like below.

	omap_hwmod_softreset(struct omap_hwmod *oh)
	{

		v = oh->_sysc_cache;
		v |= (0x1 << oh->class->sysc->sysc_fields->srst_shift);

		oh->_sysc_cache = v;
		omap_hwmod_write(v, oh, oh->class->sysc->sysc_offs);

	}

And then call this in omap_i2c_reset. Is this OK ?


- Avinash

[PATCH 1/2 v2] OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

[Kevin Hilman]

On Mon, 2011-04-11 at 18:41 +0530, Avinash.H.M. wrote:
> > > > +     oh->_sysc_cache = v;
> > > > +     omap_hwmod_write(v, oh, oh->class->sysc->sysc_offs);
> > >
> > > Direct SYSCONFIG access isn't right here.   This should go through
> > > omap_hwmod.
> > >
> > > What is probably needed is exposing _ocp_softreset to device code
> > > via something like omap_hwmod_ocp_softreset() and calling that here.
> > >
> 
> Hi Kevin , 
> 
> Looking more closely, i realised that, the sequence of _ocp_softreset
> doesn't work for I2C. It has a special programming sequence which needs
> to be followed to reset the IP. That was the reason, we created
> omap_i2c_reset.  So i feel we need not expose _ocp_softreset.
> 
> Since, the problem here is accessing the SYSCONFIG here, instead, what i
> can do is, something like below.
> 
> 	omap_hwmod_softreset(struct omap_hwmod *oh)
> 	{
> 
> 		v = oh->_sysc_cache;
> 		v |= (0x1 << oh->class->sysc->sysc_fields->srst_shift);
> 
> 		oh->_sysc_cache = v;
> 		omap_hwmod_write(v, oh, oh->class->sysc->sysc_offs);
> 
> 	}
> 
> And then call this in omap_i2c_reset. Is this OK ?

Yes, sounds reasonable.

Please be sure to describe the reasoning above in the changelog as well.

Thanks,

Kevin

[PATCH 0/2 v2] OMAP2/3: fix the i2c,gpio reset timeouts during boot.

[Avinash.H.M]

Hi ,

The patches solve the reset timeouts seen in i2c and gpio modules while
booting omap2 and omap3.

version: v2
* For discussion on v1, please refer 
 	http://www.spinics.net/lists/linux-omap/msg49483.html
* changes from v1:
	- moved i2c specific things from hwmod files to i2c files.
	- fixed comments from Paul.

Baseline:
The patches are based on 
* git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap-2.6.git
* master branch.
* v2.6.39-rc1 commit (0ce790e)

Testing:
* build tested omap2plus_defconfig for warnings and errors. none introduced.
* boot tested on 2430. 
* tested for 'core off' in suspend resume on 3430 sdp. core off counters
  increment after suspend resume. Core off works with below commands,

echo 1 > /debug/pm_debug/enable_off_mode
echo mem > /sys/power/state
cat /debug/pm_debug/count | grep ^core

Dependencies:
* need patch "OMAP2+: hwmod data: Set hwmod flags to only allow 16-bit
  accesses to i2c" from Andry Green for accessing i2c_sysc. Without this even
  with this patch, we will see i2c reset timeouts.

br ,
- Avinash

---

Avinash.H.M (2):
  OMAP2/3: hwmod: fix the i2c-reset timeout during bootup
  OMAP2/3: hwmod: fix gpio-reset timeouts seen during bootup.

 arch/arm/mach-omap2/i2c.c                  |   58 ++++++++++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_2420_data.c |    5 ++
 arch/arm/mach-omap2/omap_hwmod_2430_data.c |    6 +++
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    7 +++
 arch/arm/plat-omap/include/plat/i2c.h      |    3 +
 5 files changed, 79 insertions(+), 0 deletions(-)

[PATCH 1/2 v2] OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

[Avinash.H.M]

The i2c module has a special reset sequence. The sequence is
- Disable the I2C.
- Write to SOFTRESET bit.
- Enable the I2C.
- Poll on the RESETDONE bit.
This sequence must be followed for i2c reset in omap2, omap3. The sequence is
implemented as a function and the i2c_class is updated with the correct
'reset' pointer.

Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
Signed-off-by: Avinash.H.M <avinashhm@ti.com>
---
 arch/arm/mach-omap2/i2c.c                  |   58 ++++++++++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_2420_data.c |    1 +
 arch/arm/mach-omap2/omap_hwmod_2430_data.c |    1 +
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    1 +
 arch/arm/plat-omap/include/plat/i2c.h      |    3 +
 5 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/i2c.c b/arch/arm/mach-omap2/i2c.c
index 79c478c..67c7957 100644
--- a/arch/arm/mach-omap2/i2c.c
+++ b/arch/arm/mach-omap2/i2c.c
@@ -21,9 +21,16 @@
 
 #include <plat/cpu.h>
 #include <plat/i2c.h>
+#include <plat/common.h>
 
 #include "mux.h"
 
+/* In register I2C_CON, Bit 15 is the I2C enable bit */
+#define I2C_EN				BIT(15)
+#define I2C_CON_OFFSET			0x24
+/* Maximum microseconds to wait for OMAP module to softreset */
+#define MAX_MODULE_SOFTRESET_WAIT	10000
+
 void __init omap2_i2c_mux_pins(int bus_id)
 {
 	char mux_name[sizeof("i2c2_scl.i2c2_scl")];
@@ -37,3 +44,54 @@ void __init omap2_i2c_mux_pins(int bus_id)
 	sprintf(mux_name, "i2c%i_sda.i2c%i_sda", bus_id, bus_id);
 	omap_mux_init_signal(mux_name, OMAP_PIN_INPUT);
 }
+
+/**
+ * omap_i2c_reset- reset the omap i2c module.
+ * @oh: struct omap_hwmod *
+ *
+ * The i2c moudle in omap2, omap3 had a special sequence to reset. The
+ * sequence is:
+ * - Disable the I2C.
+ * - Write to SOFTRESET bit.
+ * - Enable the I2C.
+ * - Poll on the RESETDONE bit.
+ * The sequence is implemented in below function. This is called for 2420,
+ * 2430 and omap3.
+ */
+int omap_i2c_reset(struct omap_hwmod *oh)
+{
+	u32 v;
+	int c = 0;
+
+	/* Disable I2C */
+	v = omap_hwmod_read(oh, I2C_CON_OFFSET);
+	v = v & ~I2C_EN;
+	omap_hwmod_write(v, oh, I2C_CON_OFFSET);
+
+	/* Write to the SOFTRESET bit */
+	v = oh->_sysc_cache;
+	v |= (0x1 << oh->class->sysc->sysc_fields->srst_shift);
+
+	oh->_sysc_cache = v;
+	omap_hwmod_write(v, oh, oh->class->sysc->sysc_offs);
+
+	/* Enable I2C */
+	v = omap_hwmod_read(oh, I2C_CON_OFFSET);
+	v |= I2C_EN;
+	omap_hwmod_write(v, oh, I2C_CON_OFFSET);
+
+	/* Poll on RESETDONE bit */
+	omap_test_timeout((omap_hwmod_read(oh,
+				oh->class->sysc->syss_offs)
+				& SYSS_RESETDONE_MASK),
+				MAX_MODULE_SOFTRESET_WAIT, c);
+
+	if (c == MAX_MODULE_SOFTRESET_WAIT)
+		pr_warning("%s: %s: softreset failed (waited %d usec)\n",
+			__func__, oh->name, MAX_MODULE_SOFTRESET_WAIT);
+	else
+		pr_debug("%s: %s: softreset in %d usec\n", __func__,
+			oh->name, c);
+
+	return 0;
+}
diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
index 8eb3ce1..82ff5f7 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
@@ -1447,6 +1447,7 @@ static struct omap_hwmod_class_sysconfig i2c_sysc = {
 static struct omap_hwmod_class i2c_class = {
 	.name		= "i2c",
 	.sysc		= &i2c_sysc,
+	.reset		= &omap_i2c_reset,
 };
 
 static struct omap_i2c_dev_attr i2c_dev_attr;
diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
index a860fb5..ce292f0 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
@@ -1524,6 +1524,7 @@ static struct omap_hwmod_class_sysconfig i2c_sysc = {
 static struct omap_hwmod_class i2c_class = {
 	.name		= "i2c",
 	.sysc		= &i2c_sysc,
+	.reset		= &omap_i2c_reset,
 };
 
 static struct omap_i2c_dev_attr i2c_dev_attr = {
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index b98e2df..c74f972 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -1460,6 +1460,7 @@ static struct omap_hwmod omap3xxx_uart4_hwmod = {
 static struct omap_hwmod_class i2c_class = {
 	.name = "i2c",
 	.sysc = &i2c_sysc,
+	.reset = &omap_i2c_reset,
 };
 
 /*
diff --git a/arch/arm/plat-omap/include/plat/i2c.h b/arch/arm/plat-omap/include/plat/i2c.h
index 878d632..749306f 100644
--- a/arch/arm/plat-omap/include/plat/i2c.h
+++ b/arch/arm/plat-omap/include/plat/i2c.h
@@ -22,6 +22,7 @@
 #define __ASM__ARCH_OMAP_I2C_H
 
 #include <linux/i2c.h>
+#include <plat/omap_hwmod.h>
 
 #if defined(CONFIG_I2C_OMAP) || defined(CONFIG_I2C_OMAP_MODULE)
 extern int omap_register_i2c_bus(int bus_id, u32 clkrate,
@@ -52,4 +53,6 @@ struct omap_i2c_dev_attr {
 void __init omap1_i2c_mux_pins(int bus_id);
 void __init omap2_i2c_mux_pins(int bus_id);
 
+int omap_i2c_reset(struct omap_hwmod *oh);
+
 #endif /* __ASM__ARCH_OMAP_I2C_H */
-- 
1.7.1

[PATCH 1/2 v2] OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

[Kevin Hilman]

"Avinash.H.M" <avinashhm@ti.com> writes:

> The i2c module has a special reset sequence. The sequence is
> - Disable the I2C.
> - Write to SOFTRESET bit.
> - Enable the I2C.
> - Poll on the RESETDONE bit.
> This sequence must be followed for i2c reset in omap2, omap3. The sequence is
> implemented as a function and the i2c_class is updated with the correct
> 'reset' pointer.
>
> Cc: Rajendra Nayak <rnayak@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Signed-off-by: Avinash.H.M <avinashhm@ti.com>

[...]

> +
> +/**
> + * omap_i2c_reset- reset the omap i2c module.
> + * @oh: struct omap_hwmod *
> + *
> + * The i2c moudle in omap2, omap3 had a special sequence to reset. The
> + * sequence is:
> + * - Disable the I2C.
> + * - Write to SOFTRESET bit.
> + * - Enable the I2C.
> + * - Poll on the RESETDONE bit.
> + * The sequence is implemented in below function. This is called for 2420,
> + * 2430 and omap3.
> + */
> +int omap_i2c_reset(struct omap_hwmod *oh)
> +{
> +	u32 v;
> +	int c = 0;
> +
> +	/* Disable I2C */
> +	v = omap_hwmod_read(oh, I2C_CON_OFFSET);
> +	v = v & ~I2C_EN;
> +	omap_hwmod_write(v, oh, I2C_CON_OFFSET);
> +
> +	/* Write to the SOFTRESET bit */
> +	v = oh->_sysc_cache;
> +	v |= (0x1 << oh->class->sysc->sysc_fields->srst_shift);
> +
> +	oh->_sysc_cache = v;
> +	omap_hwmod_write(v, oh, oh->class->sysc->sysc_offs);

Direct SYSCONFIG access isn't right here.   This should go through
omap_hwmod.

What is probably needed is exposing _ocp_softreset to device code
via something like omap_hwmod_ocp_softreset() and calling that here.

Kevin

[PATCH 1/2 v2] OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

[Mahadeva, Avinash]

On Tue, Apr 5, 2011 at 11:57 PM, Kevin Hilman <khilman@ti.com> wrote:

> "Avinash.H.M" <avinashhm@ti.com> writes:
>
> > The i2c module has a special reset sequence. The sequence is
> > - Disable the I2C.
> > - Write to SOFTRESET bit.
> > - Enable the I2C.
> > - Poll on the RESETDONE bit.
> > This sequence must be followed for i2c reset in omap2, omap3. The
> sequence is
> > implemented as a function and the i2c_class is updated with the correct
> > 'reset' pointer.
> >
> > Cc: Rajendra Nayak <rnayak@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Benoit Cousson <b-cousson@ti.com>
> > Cc: Kevin Hilman <khilman@ti.com>
> > Signed-off-by: Avinash.H.M <avinashhm@ti.com>
>
> [...]
>
> > +
> > +/**
> > + * omap_i2c_reset- reset the omap i2c module.
> > + * @oh: struct omap_hwmod *
> > + *
> > + * The i2c moudle in omap2, omap3 had a special sequence to reset. The
> > + * sequence is:
> > + * - Disable the I2C.
> > + * - Write to SOFTRESET bit.
> > + * - Enable the I2C.
> > + * - Poll on the RESETDONE bit.
> > + * The sequence is implemented in below function. This is called for
> 2420,
> > + * 2430 and omap3.
> > + */
> > +int omap_i2c_reset(struct omap_hwmod *oh)
> > +{
> > +     u32 v;
> > +     int c = 0;
> > +
> > +     /* Disable I2C */
> > +     v = omap_hwmod_read(oh, I2C_CON_OFFSET);
> > +     v = v & ~I2C_EN;
> > +     omap_hwmod_write(v, oh, I2C_CON_OFFSET);
> > +
> > +     /* Write to the SOFTRESET bit */
> > +     v = oh->_sysc_cache;
> > +     v |= (0x1 << oh->class->sysc->sysc_fields->srst_shift);
> > +
> > +     oh->_sysc_cache = v;
> > +     omap_hwmod_write(v, oh, oh->class->sysc->sysc_offs);
>
> Direct SYSCONFIG access isn't right here.   This should go through
> omap_hwmod.
>
> What is probably needed is exposing _ocp_softreset to device code
> via something like omap_hwmod_ocp_softreset() and calling that here.
>
>
Hi Kevin ,

thanks for the review.

OK. I ll create a wrapper function ' omap_hwmod_ocp_softreset'
around '_ocp_softreset' and call it here .

b r ,
- avinash.



> Kevin
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110407/198e07f9/attachment-0001.html>