Premi, Sanjeev wrote:
>> -----Original Message-----
>> From: Tom [mailto:tom....@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 <pr...@ti.com>
>>> Signed-off-by: Hiremath Vaibhav <hvaib...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
