diff mbox

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

Message ID 1301672047-31903-2-git-send-email-avinashhm@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Avinash H.M. April 1, 2011, 3:34 p.m. UTC
None

Comments

Avinash H.M. April 2, 2011, 7:30 a.m. UTC | #1
Hi Kevin, 

> 
> Shouldn't the final state be disabled after reset?  IOW, Shouldn't the
> I2C be disabled again after the polling?

Yes, the I2C should be disabled after polling. We need not explicitly
program it to disabled state because, the reset value of the I2C_EN bit
is '0'. So, it is kept disabled after reset.

> 
> Also, when reposting, please be sure to Cc the linux-arm-kernel mailing
> list for patches that are targetted for upstream.

Thanks for pointing it kevin.  I wasn't aware of this.  I will CC lak
for my further patches from now onwards.

thanks,

- avinash

--
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
Avinash H.M. April 5, 2011, 6:40 a.m. UTC | #2
Hi Paul ,

Thanks for the review. 

> > +/* In register I2C_CON, Bit 15 is the I2C enable bit */
> > +#define I2C_EN				BIT(15)
> > +#define I2C_CON_OFFSET			0x24
> 
> This stuff, along with omap_i2c_reset(), doesn't belong in omap_hwmod.c, 
> which is common code that is not I2C-specific.  Please put it in 
> mach-omap2/i2c.c instead.

I agree. I ll move these I2C specific things from omap_hwmod.c to
mach-omap2/i2c.c.


[...]


> > + */
> > +int omap_i2c_reset(struct omap_hwmod *oh)
> > +{
> > +	u32 v = 0;
> 
> no need to initialize this to 0
> 
> > +	int ret = 0, c = 0;
> 
> no need to initialize ret to 0

OK . I ll remove initialization of 'v'.

Since now i am moving the function to i2c.c, i can't call _set_softreset
and _write_sysconfig(static functions in omap_hwmod.c). Instead, i ll
set the SOFTRESET bit and write into SYSC register directly. So i do not
need 'ret'. I ll remove it.

Just curios. I understand there is no use of initializing 'v' to 0
here. But by programming practice, i usually initialize local variables
to '0'. Is there anything wrong in doing this ? Any negative impact ?


[...]


> >  int omap_hwmod_register(struct omap_hwmod **ohs);
> > +int omap_i2c_reset(struct omap_hwmod *oh);
> 
> This should go into plat-omap/include/plat/i2c.h

Sure. I will move this change to plat/i2c.h. 

I ll send out the v2 of this shortly after testing.

thanks ,

- avinash

> 
> > +
> >  struct omap_hwmod *omap_hwmod_lookup(const char *name);
> >  int omap_hwmod_for_each(int (*fn)(struct omap_hwmod *oh, void *data),
> >  			void *data);
> > -- 
> > 1.7.1
> > 
> 
> 
> - Paul
--
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
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e034294..f61c9c8 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -156,6 +156,10 @@ 
 /* Name of the OMAP hwmod for the MPU */
 #define MPU_INITIATOR_NAME		"mpu"
 
+/* In register I2C_CON, Bit 15 is the I2C enable bit */
+#define I2C_EN				BIT(15)
+#define I2C_CON_OFFSET			0x24
+
 /* omap_hwmod_list contains all registered struct omap_hwmods */
 static LIST_HEAD(omap_hwmod_list);
 
@@ -2369,3 +2373,58 @@  int omap_hwmod_no_setup_reset(struct omap_hwmod *oh)
 
 	return 0;
 }
+
+/**
+ * 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 = 0;
+	int ret = 0, 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;
+	ret = _set_softreset(oh, &v);
+	if (ret)
+		goto err1;
+	_write_sysconfig(v, oh);
+
+	/* 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;
+
+err1:
+	pr_warning("%s: returned error from _set_softreset\n", __func__);
+	return ret;
+}
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/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 1adea9c..26b7ad3 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -545,6 +545,8 @@  struct omap_hwmod {
 };
 
 int omap_hwmod_register(struct omap_hwmod **ohs);
+int omap_i2c_reset(struct omap_hwmod *oh);
+
 struct omap_hwmod *omap_hwmod_lookup(const char *name);
 int omap_hwmod_for_each(int (*fn)(struct omap_hwmod *oh, void *data),
 			void *data);