clock regression and note.

clock regression and note.

[Woodruff, Richard]

Hi Paul,

When booting 2430 it looks like its calculating the crystal rate wrong now. Likely one of your fixes needs a tweak for 2430.  I noticed the bogomips was too low.  Then I see the proc/omap_clocks was gone.  Right now a few user space entities read this to get cpurate.  Seems Igor told me the N800 user space monitor might do this also.  It would be nicer to leave this part out of the debug define.

2430 has a 26MHz input which gets /2 to 13Mhz by the first level divider.  One of your patches did touch this.

<6>Clocking rate (Crystal/DPLL/MPU): 6.5/330/165 MHz
<3>Clock apll54_ck didn't lock in 100000 tries
<6>GPMC revision 3.0
<6>IRQ: Found an INTC at 0xd80fe000 (revision 3.0) with 96 i

I'll take a quick look.

Regards,
Richard W.

RE: clock regression and note.

[Woodruff, Richard]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

> 2430 has a 26MHz input which gets /2 to 13Mhz by the first level divider.
> One of your patches did touch this.
> 
> <6>Clocking rate (Crystal/DPLL/MPU): 6.5/330/165 MHz
> <3>Clock apll54_ck didn't lock in 100000 tries
> <6>GPMC revision 3.0
> <6>IRQ: Found an INTC at 0xd80fe000 (revision 3.0) with 96 i
> 
> I'll take a quick look.

This fixes it.  Patch #2 of #22 was not right.

What is happening is the u-boot (or other) flash is expected to set the APLL field properly via gauging at boot.  It must do this before enabling the APLLs.

Therefore when you start up you can look at the APLL value to see what your sys_clk rate is actually set to.  From there you multiply the upper divider a 1 or 2 to the sys_clk rate to get the osc_clk rate.

--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -1149,7 +1149,7 @@ static void __init omap2_get_crystal_rate(struct clk *osc, struct clk *sys)
 	div &= OMAP_SYSCLKDIV_MASK;
 	div >>= sys->rate_offset;
 
-	osc->rate = sclk;
+	osc->rate = sclk * div;
 	sys->rate = osc->rate / div;
 }

[-- Attachment #2: fix_osc_calc.diff --]
[-- Type: application/octet-stream, Size: 308 bytes --]

--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -1149,7 +1149,7 @@ static void __init omap2_get_crystal_rate(struct clk *osc, struct clk *sys)
 	div &= OMAP_SYSCLKDIV_MASK;
 	div >>= sys->rate_offset;
 
-	osc->rate = sclk;
+	osc->rate = sclk * div;
 	sys->rate = osc->rate / div;
 }
 

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: clock regression and note.

[Igor Stoppa]

On Fri, 2007-08-10 at 15:29 -0500, ext Woodruff, Richard wrote:
> Hi Paul,
> 
> When booting 2430 it looks like its calculating the crystal rate wrong now. Likely one of your fixes needs a tweak for 2430.  I noticed the bogomips was too low.  Then I see the proc/omap_clocks was gone.  Right now a few user space entities read this to get cpurate.  

> Seems Igor told me the N800 user space monitor might do this also.  It would be nicer to leave this part out of the debug define.

Currently we are monitoring the current cpu frequency from cpufreq sysfs
entries, although i don't know yet if it will be the final
implementation.

-- 
Cheers, Igor

Igor Stoppa <igor.stoppa@nokia.com>
(Nokia Multimedia - CP - OSSO / Helsinki, Finland)

RE: clock regression and note.

[Paul Walmsley]

Hi Richard,

On Fri, 10 Aug 2007, Woodruff, Richard wrote:

> This fixes it.  Patch #2 of #22 was not right.
>
> What is happening is the u-boot (or other) flash is expected to set the 
> APLL field properly via gauging at boot.  It must do this before 
> enabling the APLLs.
>
> Therefore when you start up you can look at the APLL value to see what 
> your sys_clk rate is actually set to.  From there you multiply the upper 
> divider a 1 or 2 to the sys_clk rate to get the osc_clk rate.

Thanks - makes sense.  That patch should be replaced with the original 
code.  omap2_get_crystal_rate() has to work backwards from the APLL rate 
backwards (towards the root of the clock tree, rather than from the root 
outwards), since there doesn't seem to be any way to query the sys_clk 
rate directly.

Might be nice if we put a comment to that effect on the function.  It's a 
little counterintuitive to be multiplying by 'div'...

- Paul