diff mbox

[RFC] OMAP: I2C: Use correct bit for CLOCKACTIVITY

Message ID 12362344552581-git-send-email-ext-eero.nurkkala@nokia.com (mailing list archive)
State Awaiting Upstream, archived
Headers show

Commit Message

ext-eero.nurkkala@nokia.com March 5, 2009, 6:27 a.m. UTC
From: Eero Nurkkala <ext-eero.nurkkala@nokia.com>

It is the bit 8 that is for FCLK. All other blocks in
OMAPs use the bit 8 for denying FCLK idling.

This is an RFC, I'd like some discussion. Somebody
double-check this?

Signed-off-by: Eero Nurkkala <ext-eero.nurkkala@nokia.com>
---
 drivers/i2c/busses/i2c-omap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Paul Walmsley March 5, 2009, 8:42 a.m. UTC | #1
Hello Eero,

On Thu, 5 Mar 2009, ext-eero.nurkkala@nokia.com wrote:

> From: Eero Nurkkala <ext-eero.nurkkala@nokia.com>
> 
> It is the bit 8 that is for FCLK. All other blocks in
> OMAPs use the bit 8 for denying FCLK idling.

Hmm.  Looking at the 34xx Rev O TRM register tables, it looks like most 
modules use bit 9 to indicate that FCLK should be kept on and bit 8 to 
indicate that the ICLK should be kept on?  DSI, DISPC, SR, DMA4 are some 
examples.

This of course contradicts some of the text, such as Table 16-6, 16-60, 
and 18-4.  CLOCKACTIVITY bits seem to attract documentation bugs; Table 
4-554 and 15.3.1.4.1 are other examples.

> This is an RFC, I'd like some discussion. Somebody
> double-check this?

Suggest you doublecheck with TI.  Richard Woodruff cc'ed; he might be able 
to clarify.


- Paul

> 
> Signed-off-by: Eero Nurkkala <ext-eero.nurkkala@nokia.com>
> ---
>  drivers/i2c/busses/i2c-omap.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 0258b55..11057eb 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -157,7 +157,7 @@
>  #define SYSC_AUTOIDLE_MASK		(1 << 0)
>  
>  #define SYSC_IDLEMODE_SMART		0x2
> -#define SYSC_CLOCKACTIVITY_FCLK		0x2
> +#define SYSC_CLOCKACTIVITY_FCLK		0x1
>  
>  
>  struct omap_i2c_dev {
> -- 
> 1.5.2
> 
> --
> 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
ext-eero.nurkkala@nokia.com March 5, 2009, 8:46 a.m. UTC | #2
On Thu, 2009-03-05 at 09:42 +0100, ext Paul Walmsley wrote:
> Hello Eero,
> Hmm.  Looking at the 34xx Rev O TRM register tables, it looks like most 
> modules use bit 9 to indicate that FCLK should be kept on and bit 8 to 
> indicate that the ICLK should be kept on?  DSI, DISPC, SR, DMA4 are some 
> examples.

How about McBSP? Same TRM.. I2C, Table 18-5 tells exactly the opposite
for I2C than what's said in Table 18-44: I2C_SYSC!!

Please, Richard?

- Eero

--
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
ext-eero.nurkkala@nokia.com March 5, 2009, 8:54 a.m. UTC | #3
On Thu, 2009-03-05 at 10:46 +0200, Eero Nurkkala wrote:
> 
> How about McBSP? Same TRM.. I2C, Table 18-5 tells exactly the opposite
> for I2C than what's said in Table 18-44: I2C_SYSC!!
> 
> Please, Richard?
> 
> - Eero

To answer to myself, I am 100% certain the McBSP FCLK is the
bit 8. I should have sent a patch about that already..

Kind of worried as could they really change that much?
>From page to page and block to block and within a block...=)

- Eero

--
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
Nishanth Menon March 5, 2009, 12:29 p.m. UTC | #4
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Eero Nurkkala
> Sent: Thursday, March 05, 2009 10:46 AM
> To: ext Paul Walmsley
> Cc: linux-omap@vger.kernel.org; Woodruff, Richard
> Subject: Re: [RFC] OMAP: I2C: Use correct bit for CLOCKACTIVITY
> 
> On Thu, 2009-03-05 at 09:42 +0100, ext Paul Walmsley wrote:
> > Hello Eero,
> > Hmm.  Looking at the 34xx Rev O TRM register tables, it looks like most
> > modules use bit 9 to indicate that FCLK should be kept on and bit 8 to
> > indicate that the ICLK should be kept on?  DSI, DISPC, SR, DMA4 are some
> > examples.
> 
> How about McBSP? Same TRM.. I2C, Table 18-5 tells exactly the opposite
> for I2C than what's said in Table 18-44: I2C_SYSC!!
> 
Yep, that does look nuts :(.. Have pinged internally.. 

Regards,
Nishanth Menon
--
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
ext-eero.nurkkala@nokia.com March 5, 2009, 12:32 p.m. UTC | #5
On Thu, 2009-03-05 at 13:29 +0100, ext Menon, Nishanth wrote:
> > 
> Yep, that does look nuts :(.. Have pinged internally.. 
> 
> Regards,
> Nishanth Menon

Thank you!

Could you please consider taking a loot at every single block
(SPI, I2C, DMA... etc).

If I were you, I'd copy-paste the CLOCKACTIVITY features as
is it to every single HW block. So it'd be coherent. I have a
distant feeling that's what you've done, but TRM says something
else..

- Eero


--
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
Nishanth Menon March 5, 2009, 12:54 p.m. UTC | #6
> -----Original Message-----
> From: Eero Nurkkala [mailto:ext-eero.nurkkala@nokia.com]
> Sent: Thursday, March 05, 2009 2:33 PM
> To: Menon, Nishanth
> Cc: ext Paul Walmsley; linux-omap@vger.kernel.org; Woodruff, Richard
> Subject: RE: [RFC] OMAP: I2C: Use correct bit for CLOCKACTIVITY
> 
> Thank you!
> 
> Could you please consider taking a loot at every single block
> (SPI, I2C, DMA... etc).
> 
> If I were you, I'd copy-paste the CLOCKACTIVITY features as
> is it to every single HW block. So it'd be coherent. I have a
> distant feeling that's what you've done, but TRM says something
> else..
me? I am just a code junkie ;).. "They" wont let me write the TRM :(.. :D... 

Anyways, I am told internally that the right setting is the one given in table 18-44, i.e.:

9:8 CLOCKACTIVITY Clock Activity selection bits RW 0x0
    0x0: Both clocks can be cut off
    0x1: Only interface clock must be kept active; functional clock can be cut off
    0x2: Only functional clock must be kept active; interface clock can be cut off
    0x3: Both clocks must be kept active

Have asked for the rest too.. But I am told that in case of these kind of issues, we should refer to the one in the register settings.

Regards,
Nishanth Menon
--
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
ext-eero.nurkkala@nokia.com March 6, 2009, 1:41 p.m. UTC | #7
On Thu, 2009-03-05 at 13:54 +0100, ext Menon, Nishanth wrote:
> me? I am just a code junkie ;).. "They" wont let me write the TRM :(.. :D... 

I was referring "you" in plural, meaning TI, not you personally =)

- Eero

--
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
Nishanth Menon March 6, 2009, 1:51 p.m. UTC | #8
Eero Nurkkala said the following on 03/06/2009 03:41 PM:
> On Thu, 2009-03-05 at 13:54 +0100, ext Menon, Nishanth wrote:
>   
>> me? I am just a code junkie ;).. "They" wont let me write the TRM :(.. :D... 
>>     
>
> I was referring "you" in plural, meaning TI, not you personally =)
>
>   
Aaaah.. for a moment you had me worried.. even had a nightmare of me
sitting in front of a candle with a quilt pen and a parchment writing
the 3k+page TRM as a penance for the coding mistakes I had done :P..

Regards,
NM
--
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
Woodruff, Richard March 11, 2009, 3:55 a.m. UTC | #9
> > How about McBSP? Same TRM.. I2C, Table 18-5 tells exactly the opposite
> > for I2C than what's said in Table 18-44: I2C_SYSC!!
> >
> > Please, Richard?
> >
> > - Eero
>
> To answer to myself, I am 100% certain the McBSP FCLK is the
> bit 8. I should have sent a patch about that already..
>
> Kind of worried as could they really change that much?
> >From page to page and block to block and within a block...=)

I think Nishant already said this but TRM is going to be updated to correct McBSP.

CBSPLP_SYSCONFIG_REG[9:8] CLOCKACTIVITY field description will be updated in next TRM version: OMAP34XX v-R

0x0: McBSPi_ICLK clock can be switched-off
PRCM Functional clock can be switched-off

0x1: McBSPi_ICLK clock must be maintained during wake up period
PRCM Functional clock can be switched-off

0x2: McBSPi_ICLK clock can be switched-off
PRCM Functional clock must be maintained during wake up period

0x3: McBSPi_ICLK clock must be maintained during wake up period
PRCM Functional clock must be maintained during wakeup period

Regards,
Richard W.
--
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
ext-eero.nurkkala@nokia.com March 11, 2009, 5:51 a.m. UTC | #10
On Wed, 2009-03-11 at 04:55 +0100, ext Woodruff, Richard wrote:
> I think Nishant already said this but TRM is going to be updated to correct McBSP.
> 
> CBSPLP_SYSCONFIG_REG[9:8] CLOCKACTIVITY field description will be updated in next TRM version: OMAP34XX v-R
> 
> 0x0: McBSPi_ICLK clock can be switched-off
> PRCM Functional clock can be switched-off
> 
> 0x1: McBSPi_ICLK clock must be maintained during wake up period
> PRCM Functional clock can be switched-off
> 
> 0x2: McBSPi_ICLK clock can be switched-off
> PRCM Functional clock must be maintained during wake up period
> 
> 0x3: McBSPi_ICLK clock must be maintained during wake up period
> PRCM Functional clock must be maintained during wakeup period
> 
> Regards,
> Richard W.

I don't remember him saying exactly that. Wow, this looks like one
really really good explanation for one of our PM effors with McBSP
audio.. thank you!

- Eero

--
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/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 0258b55..11057eb 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -157,7 +157,7 @@ 
 #define SYSC_AUTOIDLE_MASK		(1 << 0)
 
 #define SYSC_IDLEMODE_SMART		0x2
-#define SYSC_CLOCKACTIVITY_FCLK		0x2
+#define SYSC_CLOCKACTIVITY_FCLK		0x1
 
 
 struct omap_i2c_dev {