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
next prev 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.