[U-Boot] [PATCH] OMAP3: Avoid re-write to PRM_CLKSRC_CTRL

[Tom <Tom.Rix@windriver.com>]

Premi, Sanjeev wrote:
>> -----Original Message-----
>> From: Tom [mailto:Tom.Rix at windriver.com] 
>> Sent: Wednesday, January 27, 2010 7:15 PM
>> To: Premi, Sanjeev
>> Cc: u-boot at 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