From: Alexander Beregalov <[email protected]>
Fix this build error when CONFIG_SMP is not set:
drivers/cpufreq/cpufreq.c:941: 'managed_policy' undeclared
Signed-off-by: Alexander Beregalov <[email protected]>
---
Tested on AthlonXP/NForce2
drivers/cpufreq/cpufreq.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index c668ac8..287872f 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -773,11 +773,11 @@ static int cpufreq_add_dev(struct sys_device *sys_dev)
struct cpufreq_policy new_policy;
struct cpufreq_policy *policy;
struct freq_attr **drv_attr;
- struct sys_device *cpu_sys_dev;
unsigned long flags;
unsigned int j;
#ifdef CONFIG_SMP
struct cpufreq_policy *managed_policy;
+ struct sys_device *cpu_sys_dev;
#endif
if (cpu_is_offline(cpu))
@@ -930,6 +930,7 @@ static int cpufreq_add_dev(struct sys_device *sys_dev)
}
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
+#ifdef CONFIG_SMP
/* symlink affected CPUs */
for_each_cpu(j, policy->cpus) {
if (j == cpu)
@@ -947,6 +948,7 @@ static int cpufreq_add_dev(struct sys_device *sys_dev)
goto err_out_unregister;
}
}
+#endif
policy->governor = NULL; /* to assure that the starting sequence is
* run in cpufreq_set_policy */
--
1.6.3.3
On Wed, 8 Jul 2009, Alexander Beregalov wrote:
>
> From: Alexander Beregalov <[email protected]>
>
> Fix this build error when CONFIG_SMP is not set:
> drivers/cpufreq/cpufreq.c:941: 'managed_policy' undeclared
Grr. DaveJ?
That said, I'd much prefer the fix that does _not_ have this crap in it
(not new to your diff - it's pre-existing crap):
> #ifdef CONFIG_SMP
> struct cpufreq_policy *managed_policy;
> + struct sys_device *cpu_sys_dev;
> #endif
and instead those variables should be declared inside the blocks where
they are used, not at the top.
The rule should always be: make the scope of a variable as small as
possible. Don't declare it at the top and try to "save" a declaration when
it can be used inside multiple blocks as multiple different variables.
Also, that whole function could damn well be split into smaller pieces,
which would make it much more readable than that horrible 250+ line piece
of crap monster-function with #ifdef's inside the code.
Please, somebody?
Linus
On Wed, Jul 08, 2009 at 09:57:47PM +0400, Alexander Beregalov wrote:
> From: Alexander Beregalov <[email protected]>
>
> Fix this build error when CONFIG_SMP is not set:
> drivers/cpufreq/cpufreq.c:941: 'managed_policy' undeclared
>
> Signed-off-by: Alexander Beregalov <[email protected]>
Ugh, should have caught this at merge time. Thanks.
I'll add a test to my scripts to do non-SMP build testing before merging.
Dave
On Wed, Jul 08, 2009 at 11:08:12AM -0700, Linus Torvalds wrote:
> (not new to your diff - it's pre-existing crap):
>
> > #ifdef CONFIG_SMP
> > struct cpufreq_policy *managed_policy;
> > + struct sys_device *cpu_sys_dev;
> > #endif
>
> and instead those variables should be declared inside the blocks where
> they are used, not at the top.
>
> The rule should always be: make the scope of a variable as small as
> possible. Don't declare it at the top and try to "save" a declaration when
> it can be used inside multiple blocks as multiple different variables.
>
> Also, that whole function could damn well be split into smaller pieces,
> which would make it much more readable than that horrible 250+ line piece
> of crap monster-function with #ifdef's inside the code.
Yes, cpufreq_add_dev is a monster. Its complexity keeps biting us in new ways.
> Please, somebody?
It's something to look at for .32, agreed.
Dave
On Wed, Jul 08, 2009 at 11:08:12AM -0700, Linus Torvalds wrote:
>
>
> On Wed, 8 Jul 2009, Alexander Beregalov wrote:
> >
> > From: Alexander Beregalov <[email protected]>
> >
> > Fix this build error when CONFIG_SMP is not set:
> > drivers/cpufreq/cpufreq.c:941: 'managed_policy' undeclared
>
> Grr. DaveJ?
>
> That said, I'd much prefer the fix that does _not_ have this crap in it
> (not new to your diff - it's pre-existing crap):
>
> > #ifdef CONFIG_SMP
> > struct cpufreq_policy *managed_policy;
> > + struct sys_device *cpu_sys_dev;
> > #endif
>
> and instead those variables should be declared inside the blocks where
> they are used, not at the top.
>
> The rule should always be: make the scope of a variable as small as
> possible. Don't declare it at the top and try to "save" a declaration when
> it can be used inside multiple blocks as multiple different variables.
>
> Also, that whole function could damn well be split into smaller pieces,
> which would make it much more readable than that horrible 250+ line piece
> of crap monster-function with #ifdef's inside the code.
>
> Please, somebody?
This seems to be a minimal step in the direction of declaring it locally..
Not boot-tested, but should do the right thing.
I'll take a stab at chopping that function up further separately.
Dave
commit 2381f2a2d3b329313b375bc0bf5df34802b9b4ba
Author: Dave Jones <[email protected]>
Date: Wed Jul 8 16:14:23 2009 -0400
[CPUFREQ] Fix compile failure in cpufreq.c
managed_policy is out of scope for the non-smp case.
Declare it locally where used (twice)
Signed-off-by: Dave Jones <[email protected]>
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index c668ac8..b90eda8 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -776,9 +776,6 @@ static int cpufreq_add_dev(struct sys_device *sys_dev)
struct sys_device *cpu_sys_dev;
unsigned long flags;
unsigned int j;
-#ifdef CONFIG_SMP
- struct cpufreq_policy *managed_policy;
-#endif
if (cpu_is_offline(cpu))
return 0;
@@ -854,6 +851,8 @@ static int cpufreq_add_dev(struct sys_device *sys_dev)
#endif
for_each_cpu(j, policy->cpus) {
+ struct cpufreq_policy *managed_policy;
+
if (cpu == j)
continue;
@@ -932,6 +931,8 @@ static int cpufreq_add_dev(struct sys_device *sys_dev)
/* symlink affected CPUs */
for_each_cpu(j, policy->cpus) {
+ struct cpufreq_policy *managed_policy;
+
if (j == cpu)
continue;
if (!cpu_online(j))