From: Sekhar Nori <nsekhar@ti.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: rjw@sisk.pl, arvind.chauhan@arm.com, robin.randhawa@arm.com,
Steve.Bannister@arm.com, Liviu.Dudau@arm.com,
charles.garcia-tobin@arm.com, cpufreq@vger.kernel.org,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
linaro-kernel@lists.linaro.org,
Sascha Hauer <kernel@pengutronix.de>,
Paul Mundt <lethal@linux-sh.org>,
linux-sh@vger.kernel.org, linux-omap@vger.kernel.org
Subject: Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members
Date: Mon, 25 Mar 2013 14:06:17 +0530 [thread overview]
Message-ID: <51500C81.7040307@ti.com> (raw)
In-Reply-To: <07b96c5b68470001197445fb3d28786ad9acdbe0.1364138740.git.viresh.kumar@linaro.org>
On 3/24/2013 8:59 PM, Viresh Kumar wrote:
> Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
> aren't required as part of it is done by cpufreq driver or cpufreq core.
>
> Remove them.
>
> At some places we merge multiple lines together too.
>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: linux-sh@vger.kernel.org
> Cc: linux-omap@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Documentation/cpu-freq/cpu-drivers.txt | 5 +++--
> arch/arm/mach-davinci/cpufreq.c | 4 ----
> arch/arm/mach-imx/cpufreq.c | 3 ---
> arch/sh/kernel/cpufreq.c | 9 +++------
> drivers/cpufreq/cpufreq-nforce2.c | 6 ++----
> drivers/cpufreq/omap-cpufreq.c | 4 +---
> 6 files changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
> index 72f70b1..c94383f 100644
> --- a/Documentation/cpu-freq/cpu-drivers.txt
> +++ b/Documentation/cpu-freq/cpu-drivers.txt
> @@ -108,8 +108,9 @@ policy->governor must contain the "default policy" for
> cpufreq_driver.target is called with
> these values.
>
> -For setting some of these values, the frequency table helpers might be
> -helpful. See the section 2 for more information on them.
> +For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
> +frequency table helpers might be helpful. See the section 2 for more information
> +on them.
>
> SMP systems normally have same clock source for a group of cpus. For these the
> .init() would be called only once for the first online cpu. Here the .init()
> diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
> index 8fb0c2a..ff46862 100644
> --- a/arch/arm/mach-davinci/cpufreq.c
> +++ b/arch/arm/mach-davinci/cpufreq.c
> @@ -149,10 +149,6 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
> policy->cpuinfo.max_freq = policy->max;
> }
>
> - policy->min = policy->cpuinfo.min_freq;
> - policy->max = policy->cpuinfo.max_freq;
> - policy->cur = davinci_getspeed(0);
There is a line in the code a little above the ones you deleted that
also sets these same variables. I guess you were relying on that line to
set policy->cur, but that also sets policy->{min, max} which can be
cleaned up.
Thanks,
Sekhar
WARNING: multiple messages have this Message-ID (diff)
From: Sekhar Nori <nsekhar@ti.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: rjw@sisk.pl, arvind.chauhan@arm.com, robin.randhawa@arm.com,
Steve.Bannister@arm.com, Liviu.Dudau@arm.com,
charles.garcia-tobin@arm.com, cpufreq@vger.kernel.org,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
linaro-kernel@lists.linaro.org,
Sascha Hauer <kernel@pengutronix.de>,
Paul Mundt <lethal@linux-sh.org>,
linux-sh@vger.kernel.org, linux-omap@vger.kernel.org
Subject: Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members
Date: Mon, 25 Mar 2013 08:48:17 +0000 [thread overview]
Message-ID: <51500C81.7040307@ti.com> (raw)
In-Reply-To: <07b96c5b68470001197445fb3d28786ad9acdbe0.1364138740.git.viresh.kumar@linaro.org>
On 3/24/2013 8:59 PM, Viresh Kumar wrote:
> Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
> aren't required as part of it is done by cpufreq driver or cpufreq core.
>
> Remove them.
>
> At some places we merge multiple lines together too.
>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: linux-sh@vger.kernel.org
> Cc: linux-omap@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Documentation/cpu-freq/cpu-drivers.txt | 5 +++--
> arch/arm/mach-davinci/cpufreq.c | 4 ----
> arch/arm/mach-imx/cpufreq.c | 3 ---
> arch/sh/kernel/cpufreq.c | 9 +++------
> drivers/cpufreq/cpufreq-nforce2.c | 6 ++----
> drivers/cpufreq/omap-cpufreq.c | 4 +---
> 6 files changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
> index 72f70b1..c94383f 100644
> --- a/Documentation/cpu-freq/cpu-drivers.txt
> +++ b/Documentation/cpu-freq/cpu-drivers.txt
> @@ -108,8 +108,9 @@ policy->governor must contain the "default policy" for
> cpufreq_driver.target is called with
> these values.
>
> -For setting some of these values, the frequency table helpers might be
> -helpful. See the section 2 for more information on them.
> +For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
> +frequency table helpers might be helpful. See the section 2 for more information
> +on them.
>
> SMP systems normally have same clock source for a group of cpus. For these the
> .init() would be called only once for the first online cpu. Here the .init()
> diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
> index 8fb0c2a..ff46862 100644
> --- a/arch/arm/mach-davinci/cpufreq.c
> +++ b/arch/arm/mach-davinci/cpufreq.c
> @@ -149,10 +149,6 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
> policy->cpuinfo.max_freq = policy->max;
> }
>
> - policy->min = policy->cpuinfo.min_freq;
> - policy->max = policy->cpuinfo.max_freq;
> - policy->cur = davinci_getspeed(0);
There is a line in the code a little above the ones you deleted that
also sets these same variables. I guess you were relying on that line to
set policy->cur, but that also sets policy->{min, max} which can be
cleaned up.
Thanks,
Sekhar
WARNING: multiple messages have this Message-ID (diff)
From: Sekhar Nori <nsekhar@ti.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: <rjw@sisk.pl>, <arvind.chauhan@arm.com>, <robin.randhawa@arm.com>,
<Steve.Bannister@arm.com>, <Liviu.Dudau@arm.com>,
<charles.garcia-tobin@arm.com>, <cpufreq@vger.kernel.org>,
<linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linaro-kernel@lists.linaro.org>,
Sascha Hauer <kernel@pengutronix.de>,
Paul Mundt <lethal@linux-sh.org>, <linux-sh@vger.kernel.org>,
<linux-omap@vger.kernel.org>
Subject: Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members
Date: Mon, 25 Mar 2013 14:06:17 +0530 [thread overview]
Message-ID: <51500C81.7040307@ti.com> (raw)
In-Reply-To: <07b96c5b68470001197445fb3d28786ad9acdbe0.1364138740.git.viresh.kumar@linaro.org>
On 3/24/2013 8:59 PM, Viresh Kumar wrote:
> Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
> aren't required as part of it is done by cpufreq driver or cpufreq core.
>
> Remove them.
>
> At some places we merge multiple lines together too.
>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: linux-sh@vger.kernel.org
> Cc: linux-omap@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Documentation/cpu-freq/cpu-drivers.txt | 5 +++--
> arch/arm/mach-davinci/cpufreq.c | 4 ----
> arch/arm/mach-imx/cpufreq.c | 3 ---
> arch/sh/kernel/cpufreq.c | 9 +++------
> drivers/cpufreq/cpufreq-nforce2.c | 6 ++----
> drivers/cpufreq/omap-cpufreq.c | 4 +---
> 6 files changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
> index 72f70b1..c94383f 100644
> --- a/Documentation/cpu-freq/cpu-drivers.txt
> +++ b/Documentation/cpu-freq/cpu-drivers.txt
> @@ -108,8 +108,9 @@ policy->governor must contain the "default policy" for
> cpufreq_driver.target is called with
> these values.
>
> -For setting some of these values, the frequency table helpers might be
> -helpful. See the section 2 for more information on them.
> +For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
> +frequency table helpers might be helpful. See the section 2 for more information
> +on them.
>
> SMP systems normally have same clock source for a group of cpus. For these the
> .init() would be called only once for the first online cpu. Here the .init()
> diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
> index 8fb0c2a..ff46862 100644
> --- a/arch/arm/mach-davinci/cpufreq.c
> +++ b/arch/arm/mach-davinci/cpufreq.c
> @@ -149,10 +149,6 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
> policy->cpuinfo.max_freq = policy->max;
> }
>
> - policy->min = policy->cpuinfo.min_freq;
> - policy->max = policy->cpuinfo.max_freq;
> - policy->cur = davinci_getspeed(0);
There is a line in the code a little above the ones you deleted that
also sets these same variables. I guess you were relying on that line to
set policy->cur, but that also sets policy->{min, max} which can be
cleaned up.
Thanks,
Sekhar
next prev parent reply other threads:[~2013-03-25 8:36 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-24 15:29 [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() Viresh Kumar
2013-03-24 15:29 ` [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members Viresh Kumar
2013-03-24 15:41 ` Viresh Kumar
2013-03-25 8:36 ` Sekhar Nori [this message]
2013-03-25 8:48 ` Sekhar Nori
2013-03-25 8:36 ` Sekhar Nori
2013-03-25 8:45 ` Viresh Kumar
2013-03-25 8:57 ` Viresh Kumar
2013-03-25 9:41 ` Sekhar Nori
2013-03-25 9:53 ` Sekhar Nori
2013-03-25 9:41 ` Sekhar Nori
2013-03-25 10:24 ` Viresh Kumar
2013-03-25 10:36 ` Viresh Kumar
2013-03-26 6:06 ` Sekhar Nori
2013-03-26 6:18 ` Sekhar Nori
2013-03-26 6:06 ` Sekhar Nori
2013-03-27 13:55 ` [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() Linus Walleij
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=51500C81.7040307@ti.com \
--to=nsekhar@ti.com \
--cc=Liviu.Dudau@arm.com \
--cc=Steve.Bannister@arm.com \
--cc=arvind.chauhan@arm.com \
--cc=charles.garcia-tobin@arm.com \
--cc=cpufreq@vger.kernel.org \
--cc=kernel@pengutronix.de \
--cc=lethal@linux-sh.org \
--cc=linaro-kernel@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=rjw@sisk.pl \
--cc=robin.randhawa@arm.com \
--cc=viresh.kumar@linaro.org \
/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.