On 6/24/2011 6:22 PM, Premi, Sanjeev wrote:
-----Original Message-----
From: Shilimkar, Santosh
Sent: Friday, June 24, 2011 6:16 PM
To: Russell King - ARM Linux
Cc: Premi, Sanjeev; linux-omap@vger.kernel.org;
linux-arm-ker...@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:li...@arm.linux.org.uk]
Sent: Friday, June 24, 2011 4:14 PM
To: Premi, Sanjeev
Cc: linux-omap@vger.kernel.org;
linux-arm-ker...@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.

I can imagine what you are gonna post based on Russell comment, but will
wait for your patch before commenting.

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to