[PATCH 2/2] omap2+: pm: cpufreq: Fix loops_per_jiffy calculation

[Premi, Sanjeev]

> -----Original Message-----
> From: Shilimkar, Santosh 
> Sent: Friday, June 24, 2011 6:16 PM
> To: Russell King - ARM Linux
> Cc: Premi, Sanjeev; linux-omap at vger.kernel.org; 
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 2/2] omap2+: pm: cpufreq: Fix 
> loops_per_jiffy calculation
> 
> Sanjeev,
> 
> On 6/24/2011 4:21 PM, Russell King - ARM Linux wrote:
> > On Fri, Jun 24, 2011 at 04:18:31PM +0530, Premi, Sanjeev wrote:
> >>> -----Original Message-----
> >>> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> >>> Sent: Friday, June 24, 2011 4:14 PM
> >>> To: Premi, Sanjeev
> >>> Cc: linux-omap at vger.kernel.org; 
> linux-arm-kernel at lists.infradead.org
> >>> Subject: Re: [PATCH 2/2] omap2+: pm: cpufreq: Fix
> >>> loops_per_jiffy calculation
> >>>
> >>> On Fri, Jun 24, 2011 at 04:05:42PM +0530, Sanjeev Premi wrote:
> >>>> Currently, loops_per_jiffy is being calculated before calling
> >>>> cpufreq_notify_transition().
> >>>>
> >>>> However, generic cpufreq driver adjusts the jiffies as well.
> >>>> Double adjustment leads to incorrect value being assigned to
> >>>> loops_per_jiffy.
> >>>
> >>> Are you sure the generic cpufreq driver adjusts the per-cpu
> >>> loops_per_jiffy
> >>> values?  I don't believe it does.
> >>
> >> Russell,
> >>
> >> I am quoting the function from drivers/cpufreq/cpufreq.h
> >> Follow the arrows:
> >
> > Let's go to war with quotes then, because clearly you 
> haven't read the code
> > properly:
> >
> > | #ifndef CONFIG_SMP
> > ^^^^^^^^^^^^^^^^^^^^^
> > | static unsigned long l_p_j_ref;
> > | static unsigned int  l_p_j_ref_freq;
> > |
> > | static void adjust_jiffies(unsigned long val, struct 
> cpufreq_freqs *ci)
> > | {
> > | 	... code to adjust jiffies ...
> > | }
> > | #else
> > | static inline void adjust_jiffies(unsigned long val, 
> struct cpufreq_freqs *ci)
> > | {
> > |         return;
> > | }
> > | #endif
> >
> > Notice how if CONFIG_SMP is set, adjust_jiffies becomes a 
> no-op.  So if
> > CONFIG_SMP=y, loops_per_jiffy will _not_ be scaled by core code.
> >
> As Russell rightly pointed out, you need to take care of 
> UP/SMP and UP 
> OVER SMP.
> 
> The generic code updates in only in case of UP build. I thought, the
> comment is the code was well explaining that part.

[sp] I did read the code but took long to understand most of it.
     Hence patch 1 of this series.

     I already accepted that I didn't notice definition of adjust_jiffies.
     ...call it perils of leaving job half-done across a good-night-sleep!

     As is, the calculations of UP are incorrect... which I was fixing.
     Anyway, I have an updated patch - currently testing with CONFIG_SMP
     enabled. Will be posting soon.

~sanjeev

> 
> Regards
> Santosh
>