PRCM_CLKSRC_CTRL bits set incorrectly in omap2_en_osc_ck

PATCH Seth Forshee 3 participants 3 messages Apr 25, 2008

Subsystems (4)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP clock framework support

lore

Share & tools

lore

Seth Forshee

author

2008-04-25 01:57 UTC #0 lore raw

On my OMAP2430 board I was seeing the contents of DRAM become corrupted shortly after the kernel started. I tracked this down to omap2_enable_osc_ck(). I think this function is intended to only clear the bits in OMAP_AUTOEXTCLKMODE_MASK, but it has the side-effect of setting all of the other bits in the register. This includes setting some fields to reserved values.

I'm not 100% sure whether the problem is with omap2_enable_osc_ck() or prm_rmw_reg_bits() (maybe this should be masking off any bits not set in mask), but based on what I see elsewhere I think that omap2_enable_osc_ck() is to blame. If this is the case, the patch below fixes the problem.

----------
From: Seth Forshee <[email protected]>
Subject: [PATCH] ARM: OMAP2: Set PRCM_CLKSRC_CTRL correctly in omap2_enable_osc_ck

This patch fixes an incorrect use of prm_rmw_reg_bits() in omap2_enable_osc_ck() which is changing bits in PRCM_CLKSRC_CTRL that are unrelated to the function it is performing.

Signed-off-by Seth Forshee <[email protected]>

---
 arch/arm/mach-omap2/clock24xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/clock24xx.c +1 -1

index 9b7fd15..e7968e7 100644
@@ -78,7 +78,7 @@ static u32 omap2_get_dpll_rate_24xx(struct clk *tclk)
 static int omap2_enable_osc_ck(struct clk *clk)
 {
 
-	prm_rmw_reg_bits(OMAP_AUTOEXTCLKMODE_MASK, ~OMAP_AUTOEXTCLKMODE_MASK,
+	prm_rmw_reg_bits(OMAP_AUTOEXTCLKMODE_MASK, 0,
 			 OMAP24XX_PRCM_CLKSRC_CTRL);
 
 	return 0;

Paul Walmsley

maintainer Acked-by

2008-04-25 08:01 UTC #1 lore raw

Hi Seth,

Quoted 17 lines · Seth Forshee

On my OMAP2430 board I was seeing the contents of DRAM become corrupted shortly after the kernel started. I tracked this down to omap2_enable_osc_ck(). I think this function is intended to only clear the bits in OMAP_AUTOEXTCLKMODE_MASK, but it has the side-effect of setting all of the other bits in the register. This includes setting some fields to reserved values.
I'm not 100% sure whether the problem is with omap2_enable_osc_ck() or prm_rmw_reg_bits() (maybe this should be masking off any bits not set in mask), but based on what I see elsewhere I think that omap2_enable_osc_ck() is to blame. If this is the case, the patch below fixes the problem.
----------
From: Seth Forshee <[email protected]>
Subject: [PATCH] ARM: OMAP2: Set PRCM_CLKSRC_CTRL correctly in omap2_enable_osc_ck

Quoted 5 lines

This patch fixes an incorrect use of prm_rmw_reg_bits() in omap2_enable_osc_ck() which is changing bits in PRCM_CLKSRC_CTRL that are unrelated to the function it is performing.
Signed-off-by Seth Forshee <[email protected]>

Indeed, you have found a bug - thanks for the patch!

Acked-by Paul Walmsley <[email protected]>

Quoted 18 lines

---
 arch/arm/mach-omap2/clock24xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/clock24xx.c +1 -2

index 9b7fd15..e7968e7 100644
@@ -78,7 +78,7 @@ static u32 omap2_get_dpll_rate_24xx(struct clk *tclk)
 static int omap2_enable_osc_ck(struct clk *clk)
 {
 
-	prm_rmw_reg_bits(OMAP_AUTOEXTCLKMODE_MASK, ~OMAP_AUTOEXTCLKMODE_MASK,
+	prm_rmw_reg_bits(OMAP_AUTOEXTCLKMODE_MASK, 0,
 			 OMAP24XX_PRCM_CLKSRC_CTRL);
 
 	return 0;
--

Quoted 4 lines

To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

- Paul

Tony Lindgren

maintainer

2008-04-25 17:44 UTC #2 lore raw

Quoted 21 lines · Paul Walmsley

Hi Seth,
Quoted 17 lines · Seth Forshee
On my OMAP2430 board I was seeing the contents of DRAM become corrupted shortly after the kernel started. I tracked this down to omap2_enable_osc_ck(). I think this function is intended to only clear the bits in OMAP_AUTOEXTCLKMODE_MASK, but it has the side-effect of setting all of the other bits in the register. This includes setting some fields to reserved values.
I'm not 100% sure whether the problem is with omap2_enable_osc_ck() or prm_rmw_reg_bits() (maybe this should be masking off any bits not set in mask), but based on what I see elsewhere I think that omap2_enable_osc_ck() is to blame. If this is the case, the patch below fixes the problem.
----------
From: Seth Forshee <[email protected]>
Subject: [PATCH] ARM: OMAP2: Set PRCM_CLKSRC_CTRL correctly in omap2_enable_osc_ck

Quoted 7 lines

Quoted 5 lines
This patch fixes an incorrect use of prm_rmw_reg_bits() in omap2_enable_osc_ck() which is changing bits in PRCM_CLKSRC_CTRL that are unrelated to the function it is performing.
Signed-off-by Seth Forshee <[email protected]>
Indeed, you have found a bug - thanks for the patch!

Pushing today.

Tony

Quoted 31 lines

Acked-by Paul Walmsley <[email protected]>
Quoted 18 lines
---
 arch/arm/mach-omap2/clock24xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/clock24xx.c +1 -2
index 9b7fd15..e7968e7 100644
@@ -78,7 +78,7 @@ static u32 omap2_get_dpll_rate_24xx(struct clk *tclk)
 static int omap2_enable_osc_ck(struct clk *clk)
 {
 
-	prm_rmw_reg_bits(OMAP_AUTOEXTCLKMODE_MASK, ~OMAP_AUTOEXTCLKMODE_MASK,
+	prm_rmw_reg_bits(OMAP_AUTOEXTCLKMODE_MASK, 0,
 			 OMAP24XX_PRCM_CLKSRC_CTRL);
 
 	return 0;
--
Quoted 4 lines
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html