2008-10-14 06:03:25

by David Miller

[permalink] [raw]
Subject: sparc64 allmodconfig build failure...


sparc64 allmodconfig started failing to build recently:

MODPOST 1946 modules
ERROR: "cpufreq_gov_performance" [arch/sparc64/kernel/us3_cpufreq.ko] undefined!

It seems to be caused by the following commit:

commit c4d14bc0bb5d13e316890651ae4518b764c3344c
Author: Sven Wegener <[email protected]>
Date: Sat Sep 20 16:50:08 2008 +0200

[CPUFREQ] Don't export governors for default governor

We don't need to export the governors for use as the default governor,
because the default governor will be built-in anyway and we can access
the symbol directly.

This also fixes the following sparse warnings:

drivers/cpufreq/cpufreq_conservative.c:578:25: warning: symbol 'cpufreq_gov_conservative' was not declared. Should it be static?
drivers/cpufreq/cpufreq_ondemand.c:582:25: warning: symbol 'cpufreq_gov_ondemand' was not declared. Should it be static?
drivers/cpufreq/cpufreq_performance.c:39:25: warning: symbol 'cpufreq_gov_performance' was not declared. Should it be static?
drivers/cpufreq/cpufreq_powersave.c:38:25: warning: symbol 'cpufreq_gov_powersave' was not declared. Should it be static?
drivers/cpufreq/cpufreq_userspace.c:190:25: warning: symbol 'cpufreq_gov_userspace' was not declared. Should it be static?

Signed-off-by: Sven Wegener <[email protected]>
Signed-off-by: Dave Jones <[email protected]>

diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index 4ee53a4..e265783 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -576,13 +576,15 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
return 0;
}

+#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
+static
+#endif
struct cpufreq_governor cpufreq_gov_conservative = {
.name = "conservative",
.governor = cpufreq_governor_dbs,
.max_transition_latency = TRANSITION_LATENCY_LIMIT,
.owner = THIS_MODULE,
};
-EXPORT_SYMBOL(cpufreq_gov_conservative);

static int __init cpufreq_gov_dbs_init(void)
{
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 39253d6..bd06934 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -626,13 +626,15 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
return 0;
}

+#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND
+static
+#endif
struct cpufreq_governor cpufreq_gov_ondemand = {
.name = "ondemand",
.governor = cpufreq_governor_dbs,
.max_transition_latency = TRANSITION_LATENCY_LIMIT,
.owner = THIS_MODULE,
};
-EXPORT_SYMBOL(cpufreq_gov_ondemand);

static int __init cpufreq_gov_dbs_init(void)
{
diff --git a/drivers/cpufreq/cpufreq_performance.c b/drivers/cpufreq/cpufreq_performance.c
index e8e1451..7e2e515 100644
--- a/drivers/cpufreq/cpufreq_performance.c
+++ b/drivers/cpufreq/cpufreq_performance.c
@@ -36,12 +36,14 @@ static int cpufreq_governor_performance(struct cpufreq_policy *policy,
return 0;
}

+#ifdef CONFIG_CPU_FREQ_GOV_PERFORMANCE_MODULE
+static
+#endif
struct cpufreq_governor cpufreq_gov_performance = {
.name = "performance",
.governor = cpufreq_governor_performance,
.owner = THIS_MODULE,
};
-EXPORT_SYMBOL(cpufreq_gov_performance);


static int __init cpufreq_gov_performance_init(void)
diff --git a/drivers/cpufreq/cpufreq_powersave.c b/drivers/cpufreq/cpufreq_powersave.c
index 88d2f44..e6db5fa 100644
--- a/drivers/cpufreq/cpufreq_powersave.c
+++ b/drivers/cpufreq/cpufreq_powersave.c
@@ -35,12 +35,14 @@ static int cpufreq_governor_powersave(struct cpufreq_policy *policy,
return 0;
}

+#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_POWERSAVE
+static
+#endif
struct cpufreq_governor cpufreq_gov_powersave = {
.name = "powersave",
.governor = cpufreq_governor_powersave,
.owner = THIS_MODULE,
};
-EXPORT_SYMBOL(cpufreq_gov_powersave);

static int __init cpufreq_gov_powersave_init(void)
{
diff --git a/drivers/cpufreq/cpufreq_userspace.c b/drivers/cpufreq/cpufreq_userspace.c
index 32244aa..1442bba 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -187,6 +187,9 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
}


+#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE
+static
+#endif
struct cpufreq_governor cpufreq_gov_userspace = {
.name = "userspace",
.governor = cpufreq_governor_userspace,
@@ -194,7 +197,6 @@ struct cpufreq_governor cpufreq_gov_userspace = {
.show_setspeed = show_speed,
.owner = THIS_MODULE,
};
-EXPORT_SYMBOL(cpufreq_gov_userspace);

static int __init cpufreq_gov_userspace_init(void)
{


2008-10-14 08:30:46

by Sven Wegener

[permalink] [raw]
Subject: Re: sparc64 allmodconfig build failure...

On Mon, 13 Oct 2008, David Miller wrote:

> sparc64 allmodconfig started failing to build recently:
>
> MODPOST 1946 modules
> ERROR: "cpufreq_gov_performance" [arch/sparc64/kernel/us3_cpufreq.ko] undefined!
>
> It seems to be caused by the following commit:
>
> commit c4d14bc0bb5d13e316890651ae4518b764c3344c
> Author: Sven Wegener <[email protected]>
> Date: Sat Sep 20 16:50:08 2008 +0200
>
> [CPUFREQ] Don't export governors for default governor
>
> We don't need to export the governors for use as the default governor,
> because the default governor will be built-in anyway and we can access
> the symbol directly.

Uhm, we could resolve this dependency with the patch below. Although we
might as well revert the above commit or introduce an exported function
that sets the default governor on a policy.

Subject: [CPUFREQ] Fix build failure on sparc64

Commit c4d14bc0bb5d13e316890651ae4518b764c3344c ("[CPUFREQ] Don't export
governors for default governor") caused a build failure, because there are
several architectures that have cpufreq code that can be built as a
module, but the code also requires access to the default governor. Export
the default governor so that it can be accessed by these module.

Reported-by: David Miller <[email protected]>
Signed-off-by: Sven Wegener <[email protected]>
---
drivers/cpufreq/cpufreq_conservative.c | 4 +++-
drivers/cpufreq/cpufreq_ondemand.c | 4 +++-
drivers/cpufreq/cpufreq_performance.c | 4 +++-
drivers/cpufreq/cpufreq_powersave.c | 4 +++-
drivers/cpufreq/cpufreq_userspace.c | 4 +++-
5 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index e265783..65297e3 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -576,7 +576,9 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
return 0;
}

-#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
+#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
+EXPORT_SYMBOL(cpufreq_gov_conservative);
+#else
static
#endif
struct cpufreq_governor cpufreq_gov_conservative = {
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 2ab3c12..78348b3 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -626,7 +626,9 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
return 0;
}

-#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND
+#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND
+EXPORT_SYMBOL(cpufreq_gov_ondemand);
+#else
static
#endif
struct cpufreq_governor cpufreq_gov_ondemand = {
diff --git a/drivers/cpufreq/cpufreq_performance.c b/drivers/cpufreq/cpufreq_performance.c
index 7e2e515..799bfd1 100644
--- a/drivers/cpufreq/cpufreq_performance.c
+++ b/drivers/cpufreq/cpufreq_performance.c
@@ -36,7 +36,9 @@ static int cpufreq_governor_performance(struct cpufreq_policy *policy,
return 0;
}

-#ifdef CONFIG_CPU_FREQ_GOV_PERFORMANCE_MODULE
+#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE
+EXPORT_SYMBOL(cpufreq_gov_performance);
+#elif defined(CONFIG_CPU_FREQ_GOV_PERFORMANCE_MODULE)
static
#endif
struct cpufreq_governor cpufreq_gov_performance = {
diff --git a/drivers/cpufreq/cpufreq_powersave.c b/drivers/cpufreq/cpufreq_powersave.c
index e6db5fa..9db749d 100644
--- a/drivers/cpufreq/cpufreq_powersave.c
+++ b/drivers/cpufreq/cpufreq_powersave.c
@@ -35,7 +35,9 @@ static int cpufreq_governor_powersave(struct cpufreq_policy *policy,
return 0;
}

-#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_POWERSAVE
+#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_POWERSAVE
+EXPORT_SYMBOL(cpufreq_gov_powersave);
+#else
static
#endif
struct cpufreq_governor cpufreq_gov_powersave = {
diff --git a/drivers/cpufreq/cpufreq_userspace.c b/drivers/cpufreq/cpufreq_userspace.c
index 1442bba..fb47b29 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -187,7 +187,9 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
}


-#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE
+#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE
+EXPORT_SYMBOL(cpufreq_gov_userspace);
+#else
static
#endif
struct cpufreq_governor cpufreq_gov_userspace = {

2008-10-14 12:03:22

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: sparc64 allmodconfig build failure...

On Tue, Oct 14, 2008 at 10:30:20AM +0200, Sven Wegener wrote:
> On Mon, 13 Oct 2008, David Miller wrote:
>
> > sparc64 allmodconfig started failing to build recently:
> >
> > MODPOST 1946 modules
> > ERROR: "cpufreq_gov_performance" [arch/sparc64/kernel/us3_cpufreq.ko] undefined!
> >
> > It seems to be caused by the following commit:
> >
> > commit c4d14bc0bb5d13e316890651ae4518b764c3344c
> > Author: Sven Wegener <[email protected]>
> > Date: Sat Sep 20 16:50:08 2008 +0200
> >
> > [CPUFREQ] Don't export governors for default governor
> >
> > We don't need to export the governors for use as the default governor,
> > because the default governor will be built-in anyway and we can access
> > the symbol directly.
>
> Uhm, we could resolve this dependency with the patch below. Although we
> might as well revert the above commit or introduce an exported function
> that sets the default governor on a policy.
>
> Subject: [CPUFREQ] Fix build failure on sparc64
>
> Commit c4d14bc0bb5d13e316890651ae4518b764c3344c ("[CPUFREQ] Don't export
> governors for default governor") caused a build failure, because there are
> several architectures that have cpufreq code that can be built as a
> module, but the code also requires access to the default governor. Export
> the default governor so that it can be accessed by these module.

> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -576,7 +576,9 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
> return 0;
> }
>
> -#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
> +#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
> +EXPORT_SYMBOL(cpufreq_gov_conservative);
> +#else
> static
> #endif

This is ugly, ugly.

2008-10-14 21:09:01

by Sven Wegener

[permalink] [raw]
Subject: Re: sparc64 allmodconfig build failure...

On Tue, 14 Oct 2008, Alexey Dobriyan wrote:

> On Tue, Oct 14, 2008 at 10:30:20AM +0200, Sven Wegener wrote:
> > On Mon, 13 Oct 2008, David Miller wrote:
> >
> > > sparc64 allmodconfig started failing to build recently:
> > >
> > > MODPOST 1946 modules
> > > ERROR: "cpufreq_gov_performance" [arch/sparc64/kernel/us3_cpufreq.ko] undefined!
> > >
> > > It seems to be caused by the following commit:
> > >
> > > commit c4d14bc0bb5d13e316890651ae4518b764c3344c
> > > Author: Sven Wegener <[email protected]>
> > > Date: Sat Sep 20 16:50:08 2008 +0200
> > >
> > > [CPUFREQ] Don't export governors for default governor
> > >
> > > We don't need to export the governors for use as the default governor,
> > > because the default governor will be built-in anyway and we can access
> > > the symbol directly.
> >
> > Uhm, we could resolve this dependency with the patch below. Although we
> > might as well revert the above commit or introduce an exported function
> > that sets the default governor on a policy.
> >
> > Subject: [CPUFREQ] Fix build failure on sparc64
> >
> > Commit c4d14bc0bb5d13e316890651ae4518b764c3344c ("[CPUFREQ] Don't export
> > governors for default governor") caused a build failure, because there are
> > several architectures that have cpufreq code that can be built as a
> > module, but the code also requires access to the default governor. Export
> > the default governor so that it can be accessed by these module.
>
> > --- a/drivers/cpufreq/cpufreq_conservative.c
> > +++ b/drivers/cpufreq/cpufreq_conservative.c
> > @@ -576,7 +576,9 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
> > return 0;
> > }
> >
> > -#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
> > +#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
> > +EXPORT_SYMBOL(cpufreq_gov_conservative);
> > +#else
> > static
> > #endif
>
> This is ugly, ugly.

Yeah, with the default governor being any of supported governors, we
either need some ugly #ifdef mess to only export the governor that is
selected or just revert to the old code, which unconditionally exports all
governors. Or something like the patch below. With the additional modules
that I oversaw and that require the export, I'm fine with just reverting
the commit, although I think that encapsulating the default governor
decision in a built-in function is cleaner than having modules reference
different symbols depending on configuration.



Subject: [CPUFREQ] Fix build failure on sparc64

Commit c4d14bc0bb5d13e316890651ae4518b764c3344c ("[CPUFREQ] Don't export
governors for default governor") caused a build failure, because there
are several architectures that have cpufreq code that can be built as a
module, but the code also requires access to the default governor.
Instead of messing around with even more #ifdef code, add an exported
function that sets the default governor on a policy.

diff --git a/arch/arm/mach-integrator/cpu.c b/arch/arm/mach-integrator/cpu.c
index e4f72d2..58a9b77 100644
--- a/arch/arm/mach-integrator/cpu.c
+++ b/arch/arm/mach-integrator/cpu.c
@@ -184,7 +184,7 @@ static int integrator_cpufreq_init(struct cpufreq_policy *policy)
{

/* set default policy and cpuinfo */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);
policy->cpuinfo.max_freq = 160000;
policy->cpuinfo.min_freq = 12000;
policy->cpuinfo.transition_latency = 1000000; /* 1 ms, assumed */
diff --git a/arch/arm/mach-pxa/cpufreq-pxa2xx.c b/arch/arm/mach-pxa/cpufreq-pxa2xx.c
index d82528e..e013621 100644
--- a/arch/arm/mach-pxa/cpufreq-pxa2xx.c
+++ b/arch/arm/mach-pxa/cpufreq-pxa2xx.c
@@ -335,7 +335,7 @@ static __init int pxa_cpufreq_init(struct cpufreq_policy *policy)
pxa27x_guess_max_freq();

/* set default policy and cpuinfo */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);
if (cpu_is_pxa25x())
policy->policy = CPUFREQ_POLICY_PERFORMANCE;
policy->cpuinfo.transition_latency = 1000; /* FIXME: 1 ms, assumed */
diff --git a/arch/arm/mach-pxa/cpufreq-pxa3xx.c b/arch/arm/mach-pxa/cpufreq-pxa3xx.c
index 1ea0c9c..27a295b 100644
--- a/arch/arm/mach-pxa/cpufreq-pxa3xx.c
+++ b/arch/arm/mach-pxa/cpufreq-pxa3xx.c
@@ -210,7 +210,7 @@ static __init int pxa3xx_cpufreq_init(struct cpufreq_policy *policy)
int ret = -EINVAL;

/* set default policy and cpuinfo */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);
policy->cpuinfo.min_freq = 104000;
policy->cpuinfo.max_freq = (cpu_is_pxa320()) ? 806000 : 624000;
policy->cpuinfo.transition_latency = 1000; /* FIXME: 1 ms, assumed */
diff --git a/arch/arm/mach-sa1100/cpu-sa1100.c b/arch/arm/mach-sa1100/cpu-sa1100.c
index f7fa034..94a3cde 100644
--- a/arch/arm/mach-sa1100/cpu-sa1100.c
+++ b/arch/arm/mach-sa1100/cpu-sa1100.c
@@ -224,7 +224,7 @@ static int __init sa1100_cpu_init(struct cpufreq_policy *policy)
if (policy->cpu != 0)
return -EINVAL;
policy->cur = policy->min = policy->max = sa11x0_getspeed(0);
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);
policy->cpuinfo.min_freq = 59000;
policy->cpuinfo.max_freq = 287000;
policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
diff --git a/arch/avr32/mach-at32ap/cpufreq.c b/arch/avr32/mach-at32ap/cpufreq.c
index 5dd8d25..20e9e79 100644
--- a/arch/avr32/mach-at32ap/cpufreq.c
+++ b/arch/avr32/mach-at32ap/cpufreq.c
@@ -87,7 +87,7 @@ static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy)
policy->cur = at32_get_speed(0);
policy->min = policy->cpuinfo.min_freq;
policy->max = policy->cpuinfo.max_freq;
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);

printk("cpufreq: AT32AP CPU frequency driver\n");

diff --git a/arch/blackfin/mach-common/cpufreq.c b/arch/blackfin/mach-common/cpufreq.c
index 75cdad2..fe76fec 100644
--- a/arch/blackfin/mach-common/cpufreq.c
+++ b/arch/blackfin/mach-common/cpufreq.c
@@ -158,7 +158,7 @@ static int __init __bfin_cpu_init(struct cpufreq_policy *policy)
dpm_state_table[index].tscale);
}

- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);

policy->cpuinfo.transition_latency = (bfin_read_PLL_LOCKCNT() / (sclk / 1000000)) * 1000;
/*Now ,only support one cpu */
diff --git a/arch/cris/arch-v32/mach-a3/cpufreq.c b/arch/cris/arch-v32/mach-a3/cpufreq.c
index 8e5a3ca..4d64157 100644
--- a/arch/cris/arch-v32/mach-a3/cpufreq.c
+++ b/arch/cris/arch-v32/mach-a3/cpufreq.c
@@ -85,7 +85,7 @@ static int cris_freq_cpu_init(struct cpufreq_policy *policy)
int result;

/* cpuinfo and default policy values */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);
policy->cpuinfo.transition_latency = 1000000; /* 1ms */
policy->cur = cris_freq_get_cpu_frequency(0);

diff --git a/arch/cris/arch-v32/mach-fs/cpufreq.c b/arch/cris/arch-v32/mach-fs/cpufreq.c
index d57631c..245c1d0 100644
--- a/arch/cris/arch-v32/mach-fs/cpufreq.c
+++ b/arch/cris/arch-v32/mach-fs/cpufreq.c
@@ -81,7 +81,7 @@ static int cris_freq_cpu_init(struct cpufreq_policy *policy)
int result;

/* cpuinfo and default policy values */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);
policy->cpuinfo.transition_latency = 1000000; /* 1ms */
policy->cur = cris_freq_get_cpu_frequency(0);

diff --git a/arch/sparc64/kernel/us3_cpufreq.c b/arch/sparc64/kernel/us3_cpufreq.c
index 47e3aca..7bc5c78 100644
--- a/arch/sparc64/kernel/us3_cpufreq.c
+++ b/arch/sparc64/kernel/us3_cpufreq.c
@@ -183,7 +183,7 @@ static int __init us3_freq_cpu_init(struct cpufreq_policy *policy)
table[3].index = 0;
table[3].frequency = CPUFREQ_TABLE_END;

- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);
policy->cpuinfo.transition_latency = 0;
policy->cur = clock_tick;

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 31d6f53..c6bb3b7 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -813,7 +813,7 @@ static int cpufreq_add_dev(struct sys_device *sys_dev)
INIT_WORK(&policy->update, handle_update);

/* Set governor before ->init, so that driver could check it */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+ cpufreq_policy_set_default_governor(policy);
/* call driver. From then on the cpufreq must be able
* to accept all calls to ->verify and ->setpolicy for this CPU
*/
@@ -1554,6 +1554,11 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
return ret;
}

+void cpufreq_policy_set_default_governor(struct cpufreq_policy *policy)
+{
+ policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+}
+EXPORT_SYMBOL_GPL(cpufreq_policy_set_default_governor);

int cpufreq_register_governor(struct cpufreq_governor *governor)
{
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 1ee608f..3651092 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -190,6 +190,7 @@ extern int __cpufreq_driver_target(struct cpufreq_policy *policy,
extern int __cpufreq_driver_getavg(struct cpufreq_policy *policy,
unsigned int cpu);

+void cpufreq_policy_set_default_governor(struct cpufreq_policy *policy);
int cpufreq_register_governor(struct cpufreq_governor *governor);
void cpufreq_unregister_governor(struct cpufreq_governor *governor);

2008-10-14 22:49:52

by David Miller

[permalink] [raw]
Subject: Re: sparc64 allmodconfig build failure...

From: Sven Wegener <[email protected]>
Date: Tue, 14 Oct 2008 23:08:17 +0200 (CEST)

> Yeah, with the default governor being any of supported governors, we
> either need some ugly #ifdef mess to only export the governor that
> is selected or just revert to the old code, which unconditionally
> exports all governors. Or something like the patch below. With the
> additional modules that I oversaw and that require the export, I'm
> fine with just reverting the commit, although I think that
> encapsulating the default governor decision in a built-in function
> is cleaner than having modules reference different symbols depending
> on configuration.

Personally I prefer the revert.

All of the alternative solutions shown far have been extremely
ugly.

2008-10-15 22:06:18

by Sven Wegener

[permalink] [raw]
Subject: Re: sparc64 allmodconfig build failure...

On Tue, 14 Oct 2008, David Miller wrote:

> From: Sven Wegener <[email protected]>
> Date: Tue, 14 Oct 2008 23:08:17 +0200 (CEST)
>
> > Yeah, with the default governor being any of supported governors, we
> > either need some ugly #ifdef mess to only export the governor that
> > is selected or just revert to the old code, which unconditionally
> > exports all governors. Or something like the patch below. With the
> > additional modules that I oversaw and that require the export, I'm
> > fine with just reverting the commit, although I think that
> > encapsulating the default governor decision in a built-in function
> > is cleaner than having modules reference different symbols depending
> > on configuration.
>
> Personally I prefer the revert.

Fine with me. Dave J. or Linus, please revert
c4d14bc0bb5d13e316890651ae4518b764c3344c, because it breaks allmodconfig
as reported by David Miller on architectures that have cpufreq code that
can be build as a module. All ways to only export the default governor are
a ifdef mess and ugly, let's just revert the commit.

Thanks,
Sven

2008-10-15 22:11:19

by Dominik Brodowski

[permalink] [raw]
Subject: Re: sparc64 allmodconfig build failure...

On Thu, Oct 16, 2008 at 12:05:46AM +0200, Sven Wegener wrote:
> On Tue, 14 Oct 2008, David Miller wrote:
>
> > From: Sven Wegener <[email protected]>
> > Date: Tue, 14 Oct 2008 23:08:17 +0200 (CEST)
> >
> > > Yeah, with the default governor being any of supported governors, we
> > > either need some ugly #ifdef mess to only export the governor that
> > > is selected or just revert to the old code, which unconditionally
> > > exports all governors. Or something like the patch below. With the
> > > additional modules that I oversaw and that require the export, I'm
> > > fine with just reverting the commit, although I think that
> > > encapsulating the default governor decision in a built-in function
> > > is cleaner than having modules reference different symbols depending
> > > on configuration.
> >
> > Personally I prefer the revert.
>
> Fine with me. Dave J. or Linus, please revert
> c4d14bc0bb5d13e316890651ae4518b764c3344c, because it breaks allmodconfig
> as reported by David Miller on architectures that have cpufreq code that
> can be build as a module. All ways to only export the default governor are
> a ifdef mess and ugly, let's just revert the commit.

Ever wondered why this build failure only happened on sparc, and not on x86?
Because the line
policy->governor = CPUFREQ_DEFAULT_GOVERNOR
actually isn't needed in any cpufreq driver (no x86 driver sets it) as it is
already set this way in the cpufreq core. Therefore, we can remove this line
from all drivers, and keep the cleanup commit
c4d14bc0bb5d13e316890651ae4518b764c3344c .

Best,
Dominik


[PATCH] cpufreq: remove policy->governor setting in drivers initialization

As policy->governor is already set to CPUFREQ_DEFAULT_GOVERNOR in the
(always built-in) cpufreq core, we do not need to set it in the drivers.
This fixes the sparc64 allmodconfig build failure.

Also, remove a totally useles setting of ->policy in cpufreq-pxa3xx.c.

Signed-off-by: Dominik Brodowski <[email protected]>

diff --git a/arch/arm/mach-integrator/cpu.c b/arch/arm/mach-integrator/cpu.c
index e4f72d2..44d4c2e 100644
--- a/arch/arm/mach-integrator/cpu.c
+++ b/arch/arm/mach-integrator/cpu.c
@@ -184,7 +184,6 @@ static int integrator_cpufreq_init(struct cpufreq_policy *policy)
{

/* set default policy and cpuinfo */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
policy->cpuinfo.max_freq = 160000;
policy->cpuinfo.min_freq = 12000;
policy->cpuinfo.transition_latency = 1000000; /* 1 ms, assumed */
diff --git a/arch/arm/mach-pxa/cpufreq-pxa2xx.c b/arch/arm/mach-pxa/cpufreq-pxa2xx.c
index d82528e..1f272ea 100644
--- a/arch/arm/mach-pxa/cpufreq-pxa2xx.c
+++ b/arch/arm/mach-pxa/cpufreq-pxa2xx.c
@@ -335,9 +335,6 @@ static __init int pxa_cpufreq_init(struct cpufreq_policy *policy)
pxa27x_guess_max_freq();

/* set default policy and cpuinfo */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
- if (cpu_is_pxa25x())
- policy->policy = CPUFREQ_POLICY_PERFORMANCE;
policy->cpuinfo.transition_latency = 1000; /* FIXME: 1 ms, assumed */
policy->cur = get_clk_frequency_khz(0); /* current freq */
policy->min = policy->max = policy->cur;
diff --git a/arch/arm/mach-pxa/cpufreq-pxa3xx.c b/arch/arm/mach-pxa/cpufreq-pxa3xx.c
index 1ea0c9c..968c830 100644
--- a/arch/arm/mach-pxa/cpufreq-pxa3xx.c
+++ b/arch/arm/mach-pxa/cpufreq-pxa3xx.c
@@ -210,7 +210,6 @@ static __init int pxa3xx_cpufreq_init(struct cpufreq_policy *policy)
int ret = -EINVAL;

/* set default policy and cpuinfo */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
policy->cpuinfo.min_freq = 104000;
policy->cpuinfo.max_freq = (cpu_is_pxa320()) ? 806000 : 624000;
policy->cpuinfo.transition_latency = 1000; /* FIXME: 1 ms, assumed */
diff --git a/arch/arm/mach-sa1100/cpu-sa1100.c b/arch/arm/mach-sa1100/cpu-sa1100.c
index f7fa034..244d595 100644
--- a/arch/arm/mach-sa1100/cpu-sa1100.c
+++ b/arch/arm/mach-sa1100/cpu-sa1100.c
@@ -224,7 +224,6 @@ static int __init sa1100_cpu_init(struct cpufreq_policy *policy)
if (policy->cpu != 0)
return -EINVAL;
policy->cur = policy->min = policy->max = sa11x0_getspeed(0);
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
policy->cpuinfo.min_freq = 59000;
policy->cpuinfo.max_freq = 287000;
policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
diff --git a/arch/avr32/mach-at32ap/cpufreq.c b/arch/avr32/mach-at32ap/cpufreq.c
index 5dd8d25..d84efe4 100644
--- a/arch/avr32/mach-at32ap/cpufreq.c
+++ b/arch/avr32/mach-at32ap/cpufreq.c
@@ -87,7 +87,6 @@ static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy)
policy->cur = at32_get_speed(0);
policy->min = policy->cpuinfo.min_freq;
policy->max = policy->cpuinfo.max_freq;
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;

printk("cpufreq: AT32AP CPU frequency driver\n");

diff --git a/arch/blackfin/mach-common/cpufreq.c b/arch/blackfin/mach-common/cpufreq.c
index 75cdad2..c22c47b 100644
--- a/arch/blackfin/mach-common/cpufreq.c
+++ b/arch/blackfin/mach-common/cpufreq.c
@@ -158,8 +158,6 @@ static int __init __bfin_cpu_init(struct cpufreq_policy *policy)
dpm_state_table[index].tscale);
}

- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
-
policy->cpuinfo.transition_latency = (bfin_read_PLL_LOCKCNT() / (sclk / 1000000)) * 1000;
/*Now ,only support one cpu */
policy->cur = cclk;
diff --git a/arch/cris/arch-v32/mach-a3/cpufreq.c b/arch/cris/arch-v32/mach-a3/cpufreq.c
index 8e5a3ca..ee391ec 100644
--- a/arch/cris/arch-v32/mach-a3/cpufreq.c
+++ b/arch/cris/arch-v32/mach-a3/cpufreq.c
@@ -85,7 +85,6 @@ static int cris_freq_cpu_init(struct cpufreq_policy *policy)
int result;

/* cpuinfo and default policy values */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
policy->cpuinfo.transition_latency = 1000000; /* 1ms */
policy->cur = cris_freq_get_cpu_frequency(0);

diff --git a/arch/cris/arch-v32/mach-fs/cpufreq.c b/arch/cris/arch-v32/mach-fs/cpufreq.c
index d57631c..58bd71e 100644
--- a/arch/cris/arch-v32/mach-fs/cpufreq.c
+++ b/arch/cris/arch-v32/mach-fs/cpufreq.c
@@ -81,7 +81,6 @@ static int cris_freq_cpu_init(struct cpufreq_policy *policy)
int result;

/* cpuinfo and default policy values */
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
policy->cpuinfo.transition_latency = 1000000; /* 1ms */
policy->cur = cris_freq_get_cpu_frequency(0);

diff --git a/arch/sparc64/kernel/us3_cpufreq.c b/arch/sparc64/kernel/us3_cpufreq.c
index 47e3aca..365b646 100644
--- a/arch/sparc64/kernel/us3_cpufreq.c
+++ b/arch/sparc64/kernel/us3_cpufreq.c
@@ -183,7 +183,6 @@ static int __init us3_freq_cpu_init(struct cpufreq_policy *policy)
table[3].index = 0;
table[3].frequency = CPUFREQ_TABLE_END;

- policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
policy->cpuinfo.transition_latency = 0;
policy->cur = clock_tick;





2008-10-15 22:41:37

by David Miller

[permalink] [raw]
Subject: Re: sparc64 allmodconfig build failure...

From: Dominik Brodowski <[email protected]>
Date: Thu, 16 Oct 2008 00:11:04 +0200

> [PATCH] cpufreq: remove policy->governor setting in drivers initialization
>
> As policy->governor is already set to CPUFREQ_DEFAULT_GOVERNOR in the
> (always built-in) cpufreq core, we do not need to set it in the drivers.
> This fixes the sparc64 allmodconfig build failure.
>
> Also, remove a totally useles setting of ->policy in cpufreq-pxa3xx.c.
>
> Signed-off-by: Dominik Brodowski <[email protected]>

This works for me:

Acked-by: David S. Miller <[email protected]>