All of lore.kernel.org
 help / color / mirror / Atom feed
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: "Premi, Sanjeev" <premi@ti.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/2] omap2+: pm: cpufreq: Fix loops_per_jiffy calculation
Date: Fri, 24 Jun 2011 18:15:49 +0530	[thread overview]
Message-ID: <4E0486FD.2080604@ti.com> (raw)
In-Reply-To: <20110624105144.GL9449@n2100.arm.linux.org.uk>

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@arm.linux.org.uk]
>>> Sent: Friday, June 24, 2011 4:14 PM
>>> To: Premi, Sanjeev
>>> Cc: linux-omap@vger.kernel.org; linux-arm-kernel@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.

Regards
Santosh

WARNING: multiple messages have this Message-ID (diff)
From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] omap2+: pm: cpufreq: Fix loops_per_jiffy calculation
Date: Fri, 24 Jun 2011 18:15:49 +0530	[thread overview]
Message-ID: <4E0486FD.2080604@ti.com> (raw)
In-Reply-To: <20110624105144.GL9449@n2100.arm.linux.org.uk>

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.

Regards
Santosh

  parent reply	other threads:[~2011-06-24 12:45 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-24 10:35 [PATCH 0/2] omap2+: pm: Fix code syntax and BogoMIPS Sanjeev Premi
2011-06-24 10:35 ` Sanjeev Premi
2011-06-24 10:35 ` [PATCH 1/2] omap2+: pm: Use if...else instead of goto Sanjeev Premi
2011-06-24 10:35   ` Sanjeev Premi
2011-06-24 12:48   ` Santosh Shilimkar
2011-06-24 12:48     ` Santosh Shilimkar
2011-06-24 12:54     ` Premi, Sanjeev
2011-06-24 12:54       ` Premi, Sanjeev
2011-06-24 12:59       ` Santosh Shilimkar
2011-06-24 12:59         ` Santosh Shilimkar
2011-06-24 13:01         ` Premi, Sanjeev
2011-06-24 13:01           ` Premi, Sanjeev
2011-06-24 10:35 ` [PATCH 2/2] omap2+: pm: cpufreq: Fix loops_per_jiffy calculation Sanjeev Premi
2011-06-24 10:35   ` Sanjeev Premi
2011-06-24 10:43   ` Russell King - ARM Linux
2011-06-24 10:43     ` Russell King - ARM Linux
2011-06-24 10:48     ` Premi, Sanjeev
2011-06-24 10:48       ` Premi, Sanjeev
2011-06-24 10:51       ` Russell King - ARM Linux
2011-06-24 10:51         ` Russell King - ARM Linux
2011-06-24 11:11         ` Premi, Sanjeev
2011-06-24 11:11           ` Premi, Sanjeev
2011-06-24 12:45         ` Santosh Shilimkar [this message]
2011-06-24 12:45           ` Santosh Shilimkar
2011-06-24 12:52           ` Premi, Sanjeev
2011-06-24 12:52             ` Premi, Sanjeev
2011-06-24 13:01             ` Santosh Shilimkar
2011-06-24 13:01               ` Santosh Shilimkar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E0486FD.2080604@ti.com \
    --to=santosh.shilimkar@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=premi@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.