* [PATCH 1/3] cpufreq: ppc: Add missing #include <asm/smp.h>
@ 2014-04-17 9:53 Geert Uytterhoeven
2014-04-17 9:53 ` [PATCH 2/3] cpufreq: ppc: Fix integer overflow in expression Geert Uytterhoeven
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2014-04-17 9:53 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar
Cc: cpufreq, linux-pm, linuxppc-dev, linux-kernel, Geert Uytterhoeven
If CONFIG_SMP=n, <linux/smp.h> does not include <asm/smp.h>, causing:
drivers/cpufreq/ppc-corenet-cpufreq.c: In function 'corenet_cpufreq_cpu_init':
drivers/cpufreq/ppc-corenet-cpufreq.c:173:3: error: implicit declaration of function 'get_hard_smp_processor_id' [-Werror=implicit-function-declaration]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/cpufreq/ppc-corenet-cpufreq.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
index b7e677be1df0..e78f9c806de4 100644
--- a/drivers/cpufreq/ppc-corenet-cpufreq.c
+++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
@@ -22,6 +22,8 @@
#include <linux/smp.h>
#include <sysdev/fsl_soc.h>
+#include <asm/smp.h>
+
/**
* struct cpu_data - per CPU data struct
* @parent: the parent node of cpu clock
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] cpufreq: ppc: Fix integer overflow in expression
2014-04-17 9:53 [PATCH 1/3] cpufreq: ppc: Add missing #include <asm/smp.h> Geert Uytterhoeven
@ 2014-04-17 9:53 ` Geert Uytterhoeven
2014-04-21 5:52 ` Viresh Kumar
2014-04-17 9:53 ` [PATCH 3/3] cpufreq: ppc: Fix handling of non-existent clocks Geert Uytterhoeven
2014-04-21 5:51 ` [PATCH 1/3] cpufreq: ppc: Add missing #include <asm/smp.h> Viresh Kumar
2 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2014-04-17 9:53 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar
Cc: cpufreq, linux-pm, linuxppc-dev, linux-kernel, Geert Uytterhoeven
On 32-bit, "12 * NSEC_PER_SEC" doesn't fit in "unsigned long"
(NSEC_PER_SEC is a "long" constant), causing an integer overflow:
drivers/cpufreq/ppc-corenet-cpufreq.c: In function 'corenet_cpufreq_cpu_init':
drivers/cpufreq/ppc-corenet-cpufreq.c:211:9: warning: integer overflow in expression [-Woverflow]
Force the intermediate to be 64-bit by adding an "ULL" suffix to the
constant multiplier to fix this.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/cpufreq/ppc-corenet-cpufreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
index e78f9c806de4..53881d78a931 100644
--- a/drivers/cpufreq/ppc-corenet-cpufreq.c
+++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
@@ -208,7 +208,7 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy *policy)
per_cpu(cpu_data, i) = data;
policy->cpuinfo.transition_latency =
- (12 * NSEC_PER_SEC) / fsl_get_sys_freq();
+ (12ULL * NSEC_PER_SEC) / fsl_get_sys_freq();
of_node_put(np);
return 0;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] cpufreq: ppc: Fix handling of non-existent clocks
2014-04-17 9:53 [PATCH 1/3] cpufreq: ppc: Add missing #include <asm/smp.h> Geert Uytterhoeven
2014-04-17 9:53 ` [PATCH 2/3] cpufreq: ppc: Fix integer overflow in expression Geert Uytterhoeven
@ 2014-04-17 9:53 ` Geert Uytterhoeven
2014-04-21 6:01 ` Viresh Kumar
2014-04-21 5:51 ` [PATCH 1/3] cpufreq: ppc: Add missing #include <asm/smp.h> Viresh Kumar
2 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2014-04-17 9:53 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar
Cc: cpufreq, linux-pm, linuxppc-dev, linux-kernel, Geert Uytterhoeven
If the clock doesn't exist, clk_get_rate() returns -EINVAL, which becomes
a large number (freq is u32), failing the "freq < min_cpufreq" test.
Explicitly test for "(u32)-EINVAL" to fix this.
Update the comment, and fix a grammer issue while we're at it.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/cpufreq/ppc-corenet-cpufreq.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
index 53881d78a931..7027eab814ce 100644
--- a/drivers/cpufreq/ppc-corenet-cpufreq.c
+++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
@@ -179,10 +179,11 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy *policy)
clk = of_clk_get(data->parent, i);
freq = clk_get_rate(clk);
/*
- * the clock is valid if its frequency is not masked
- * and large than minimum allowed frequency.
+ * the clock is valid if it exists, its frequency is not
+ * masked, and larger than minimum allowed frequency.
*/
- if (freq < min_cpufreq || (mask & (1 << i)))
+ if (freq == (u32)-EINVAL || freq < min_cpufreq ||
+ (mask & (1 << i)))
table[i].frequency = CPUFREQ_ENTRY_INVALID;
else
table[i].frequency = freq / 1000;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cpufreq: ppc: Add missing #include <asm/smp.h>
2014-04-17 9:53 [PATCH 1/3] cpufreq: ppc: Add missing #include <asm/smp.h> Geert Uytterhoeven
2014-04-17 9:53 ` [PATCH 2/3] cpufreq: ppc: Fix integer overflow in expression Geert Uytterhoeven
2014-04-17 9:53 ` [PATCH 3/3] cpufreq: ppc: Fix handling of non-existent clocks Geert Uytterhoeven
@ 2014-04-21 5:51 ` Viresh Kumar
2 siblings, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2014-04-21 5:51 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rafael J. Wysocki, cpufreq, linux-pm, linuxppc-dev,
Linux Kernel Mailing List
On 17 April 2014 15:23, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> If CONFIG_SMP=n, <linux/smp.h> does not include <asm/smp.h>, causing:
>
> drivers/cpufreq/ppc-corenet-cpufreq.c: In function 'corenet_cpufreq_cpu_init':
> drivers/cpufreq/ppc-corenet-cpufreq.c:173:3: error: implicit declaration of function 'get_hard_smp_processor_id' [-Werror=implicit-function-declaration]
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/cpufreq/ppc-corenet-cpufreq.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
> index b7e677be1df0..e78f9c806de4 100644
> --- a/drivers/cpufreq/ppc-corenet-cpufreq.c
> +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
> @@ -22,6 +22,8 @@
> #include <linux/smp.h>
> #include <sysdev/fsl_soc.h>
>
> +#include <asm/smp.h>
> +
Probably similar to what Srivatsa did:
+#include <asm/smp.h> /* Required for cpu_sibling_mask() in UP configs */
??
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] cpufreq: ppc: Fix integer overflow in expression
2014-04-17 9:53 ` [PATCH 2/3] cpufreq: ppc: Fix integer overflow in expression Geert Uytterhoeven
@ 2014-04-21 5:52 ` Viresh Kumar
0 siblings, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2014-04-21 5:52 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rafael J. Wysocki, cpufreq, linux-pm, linuxppc-dev,
Linux Kernel Mailing List
On 17 April 2014 15:23, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> On 32-bit, "12 * NSEC_PER_SEC" doesn't fit in "unsigned long"
> (NSEC_PER_SEC is a "long" constant), causing an integer overflow:
>
> drivers/cpufreq/ppc-corenet-cpufreq.c: In function 'corenet_cpufreq_cpu_init':
> drivers/cpufreq/ppc-corenet-cpufreq.c:211:9: warning: integer overflow in expression [-Woverflow]
>
> Force the intermediate to be 64-bit by adding an "ULL" suffix to the
> constant multiplier to fix this.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/cpufreq/ppc-corenet-cpufreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
> index e78f9c806de4..53881d78a931 100644
> --- a/drivers/cpufreq/ppc-corenet-cpufreq.c
> +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
> @@ -208,7 +208,7 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy *policy)
> per_cpu(cpu_data, i) = data;
>
> policy->cpuinfo.transition_latency =
> - (12 * NSEC_PER_SEC) / fsl_get_sys_freq();
> + (12ULL * NSEC_PER_SEC) / fsl_get_sys_freq();
> of_node_put(np);
>
> return 0;
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] cpufreq: ppc: Fix handling of non-existent clocks
2014-04-17 9:53 ` [PATCH 3/3] cpufreq: ppc: Fix handling of non-existent clocks Geert Uytterhoeven
@ 2014-04-21 6:01 ` Viresh Kumar
2014-04-21 8:01 ` Geert Uytterhoeven
0 siblings, 1 reply; 7+ messages in thread
From: Viresh Kumar @ 2014-04-21 6:01 UTC (permalink / raw)
To: Geert Uytterhoeven, Mike Turquette
Cc: Rafael J. Wysocki, cpufreq, linux-pm, linuxppc-dev,
Linux Kernel Mailing List
On 17 April 2014 15:23, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> If the clock doesn't exist, clk_get_rate() returns -EINVAL
You clk_get_rate() isn't written well then, it should return zero.
@Mike: I didn't see this clearly mentioned in clk.h, should we fix
that?
>, which becomes
> a large number (freq is u32), failing the "freq < min_cpufreq" test.
> Explicitly test for "(u32)-EINVAL" to fix this.
That's a bad check. We should have done this instead:
(s32)freq < 0; but that would be true for high values of clock. And that's
why clk_get_rate() must return zero for errors.
> Update the comment, and fix a grammer issue while we're at it.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/cpufreq/ppc-corenet-cpufreq.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
> index 53881d78a931..7027eab814ce 100644
> --- a/drivers/cpufreq/ppc-corenet-cpufreq.c
> +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
> @@ -179,10 +179,11 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy *policy)
> clk = of_clk_get(data->parent, i);
> freq = clk_get_rate(clk);
> /*
> - * the clock is valid if its frequency is not masked
> - * and large than minimum allowed frequency.
> + * the clock is valid if it exists, its frequency is not
> + * masked, and larger than minimum allowed frequency.
> */
> - if (freq < min_cpufreq || (mask & (1 << i)))
> + if (freq == (u32)-EINVAL || freq < min_cpufreq ||
> + (mask & (1 << i)))
> table[i].frequency = CPUFREQ_ENTRY_INVALID;
> else
> table[i].frequency = freq / 1000;
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] cpufreq: ppc: Fix handling of non-existent clocks
2014-04-21 6:01 ` Viresh Kumar
@ 2014-04-21 8:01 ` Geert Uytterhoeven
0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2014-04-21 8:01 UTC (permalink / raw)
To: Viresh Kumar
Cc: Geert Uytterhoeven, Mike Turquette, Rafael J. Wysocki, cpufreq,
linux-pm, linuxppc-dev, Linux Kernel Mailing List
Hi Viresh,
On Mon, Apr 21, 2014 at 8:01 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 17 April 2014 15:23, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>> If the clock doesn't exist, clk_get_rate() returns -EINVAL
>
> You clk_get_rate() isn't written well then, it should return zero.
You're right, thanks!
Once again I looked at the wrong clk_get_rate() implementation :-(
These non-CCF variants should die soon...
So the original code was correct: if the clock is not valid, freq == 0,
and thus "freq < min_cpufreq" is true.
> @Mike: I didn't see this clearly mentioned in clk.h, should we fix
> that?
>
>>, which becomes
>> a large number (freq is u32), failing the "freq < min_cpufreq" test.
>> Explicitly test for "(u32)-EINVAL" to fix this.
>
> That's a bad check. We should have done this instead:
>
> (s32)freq < 0; but that would be true for high values of clock. And that's
> why clk_get_rate() must return zero for errors.
>
>> Update the comment, and fix a grammer issue while we're at it.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> drivers/cpufreq/ppc-corenet-cpufreq.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
>> index 53881d78a931..7027eab814ce 100644
>> --- a/drivers/cpufreq/ppc-corenet-cpufreq.c
>> +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
>> @@ -179,10 +179,11 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy *policy)
>> clk = of_clk_get(data->parent, i);
>> freq = clk_get_rate(clk);
>> /*
>> - * the clock is valid if its frequency is not masked
>> - * and large than minimum allowed frequency.
>> + * the clock is valid if it exists, its frequency is not
>> + * masked, and larger than minimum allowed frequency.
>> */
>> - if (freq < min_cpufreq || (mask & (1 << i)))
>> + if (freq == (u32)-EINVAL || freq < min_cpufreq ||
>> + (mask & (1 << i)))
>> table[i].frequency = CPUFREQ_ENTRY_INVALID;
>> else
>> table[i].frequency = freq / 1000;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-04-21 8:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-17 9:53 [PATCH 1/3] cpufreq: ppc: Add missing #include <asm/smp.h> Geert Uytterhoeven
2014-04-17 9:53 ` [PATCH 2/3] cpufreq: ppc: Fix integer overflow in expression Geert Uytterhoeven
2014-04-21 5:52 ` Viresh Kumar
2014-04-17 9:53 ` [PATCH 3/3] cpufreq: ppc: Fix handling of non-existent clocks Geert Uytterhoeven
2014-04-21 6:01 ` Viresh Kumar
2014-04-21 8:01 ` Geert Uytterhoeven
2014-04-21 5:51 ` [PATCH 1/3] cpufreq: ppc: Add missing #include <asm/smp.h> Viresh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).