Premi, Sanjeev wrote:

...

-----Original Message----- From: Tom [mailto:Tom.Rix@windriver.com] Sent: Wednesday, January 27, 2010 7:15 PM To: Premi, Sanjeev Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] OMAP3: Avoid re-write to PRM_CLKSRC_CTRL

Sanjeev Premi wrote:

...

In function get_osc_clk_speed(), do not change/ update the divider for SYS_CLK as it can has cascading effect on the other derived clocks.

Sudden change in divider value can lead to inconsistent behavior in the system - often leading to crashes.

The problem was found when working with OMAP3EVM using DM3730 processor card.

The patch has been tested with OMAP3530 on OMAP3EVM as well

I believe the intent of this function is only to be called once early in the clock setup when the other clocks have not been setup to determine emperically the master system clock. Can you provide were else it is being called ?

Tom,

Yes, the intent of function is to determine the master clock. But, by the time this function is executed, the divider has a value. (Usually set by the x-loader for OMAP boards).

Before we hit this code, the some clocks are already derived (either by default OR by x-loader). The function as it exists Today, does not follow any sequence to contain the effect of changing the divider.

In case of 3730 (similar to 3630) we are possibly overshooting the tolerance zone. Using Lauterbach we can see that contents of each window starts to 'auto-refresh' as soon as the divider Changes - sometimes after restoring the divider. A clear indication of system becoming unstable.

Of course, once in 4-5 times this issue was noticed in x-loader as well which has almost same code for the purpose.

The intent of function is to determine the OSC frequency; which can be determined without having to change the divider... by multiplying by the same factor.

Overall, the empirical formula is same; but this patch ensures that HW need not change for the calculations.

This is a good explanation. Please include this in the commit log of the next (cleaned up for ws) version of this patch.

Since this is a clock change, it would be good to have it tested on as many boards as possible.

Thanks Tom

Best regards, Sanjeev

...

...

Signed-off-by: Sanjeev Premi premi@ti.com Signed-off-by: Hiremath Vaibhav hvaibhav@ti.com

---

cpu/arm_cortexa8/omap3/clock.c | 20 ++++++++++++++++---- 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/cpu/arm_cortexa8/omap3/clock.c

b/cpu/arm_cortexa8/omap3/clock.c

...

index 174c453..6330c9e 100644 --- a/cpu/arm_cortexa8/omap3/clock.c +++ b/cpu/arm_cortexa8/omap3/clock.c @@ -40,7 +40,7 @@

---

***************/

...

u32 get_osc_clk_speed(void) {

• u32 start, cstart, cend, cdiff, val;

• u32 start, cstart, cend, cdiff, cdiv, val; struct prcm *prcm_base = (struct prcm *)PRCM_BASE; struct prm *prm_base = (struct prm *)PRM_BASE; struct gptimer *gpt1_base = (struct gptimer *)OMAP34XX_GPT1;

@@ -48,9 +48,15 @@ u32 get_osc_clk_speed(void)

val = readl(&prm_base->clksrc_ctrl);

• /* If SYS_CLK is being divided by 2, remove for now */
• val = (val & (~SYSCLKDIV_2)) | SYSCLKDIV_1;
• writel(val, &prm_base->clksrc_ctrl);

• if (val & SYSCLKDIV_2)

• cdiv = 2;
  

• else if (val & SYSCLKDIV_1)

• cdiv = 1;
  

• else

• /*
  

•  * Should never reach here! (Assume divider as 1)
  

•  */
  

Could reduce this to a single line, Good comment.

...

• cdiv = 1;
  
  

  /* enable timer2 */ val = readl(&prcm_base->clksel_wkup) | CLKSEL_GPT1;

@@ -61,6 +67,7 @@ u32 get_osc_clk_speed(void) /* Enable I and F Clocks for GPT1 */ val = readl(&prcm_base->iclken_wkup) | EN_GPT1 | EN_32KSYNC; writel(val, &prcm_base->iclken_wkup);

Extra space. Remove.

...

val = readl(&prcm_base->fclken_wkup) | EN_GPT1; writel(val, &prcm_base->fclken_wkup);

@@ -83,6 +90,11 @@ u32 get_osc_clk_speed(void) cend = readl(&gpt1_base->tcrr); /* get end

sys_clk count */

...

cdiff = cend - cstart; /* get elapsed ticks */

• if (cdiv == 2)
• {

• cdiff *= 2;
  

• }

Braces not needed

...

/* based on number of ticks assign speed */ if (cdiff > 19000) return S38_4M;

Tom