OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

PATCH Superseded Avinash.H.M. 2 participants 3 messages Apr 7, 2011 – Apr 16, 2011

Share & tools

Avinash.H.M.

author

2011-04-07 07:03 UTC #0 lore raw

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

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

Quoted 55 lines · Kevin Hilman

Quoted 6 lines · 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
Quoted 8 lines
implemented as a function and the i2c_class is updated with the correct 'reset' pointer.
Cc Rajendra Nayak <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Benoit Cousson <[email protected]>
Cc Kevin Hilman <[email protected]>
Signed-off-by Avinash.H.M <[email protected]>
[...]
Review
Quoted 12 lines
(quoted diff) +12 -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,
Quoted 8 lines
+ * 2430 and omap3.
+ */
(quoted diff) +6 -0
+int omap_i2c_reset(struct omap_hwmod *oh)
+{
+     u32 v;
+     int c = 0;
+
+     /* Disable I2C */
Review
Quoted 10 lines
(quoted diff) +10 -0
+     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.

Quoted 4 lines

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.

Quoted 2 lines

Kevin

Avinash.H.M.

author

2011-04-11 13:11 UTC #1 lore raw

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

Quoted 9 lines

Quoted 9 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+     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

Kevin Hilman

2011-04-16 16:42 UTC #2 lore raw

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

Quoted 31 lines · Avinash.H.M.

Quoted 9 lines
Quoted 9 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+     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);

	}

Quoted 1 line

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