[PATCH 0/6] acpi-cpufreq and powernow-k8 fixes

[PATCH 0/6] acpi-cpufreq and powernow-k8 fixes

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Hi all,

here are a couple of patches fixing acpi-cpufreq and powernow-k8 loading
sequence and handoff.

Patch 0001 should be in Rafael's tree already but I'm adding it here for
the sake of completeness. Basically, we want to address all possible
cases of powernow-k8 and acpi-cpufreq:

(M: module, Y: built-in)

powernow-k8		acpi-cpufreq
M			M		- powernow-k8 requests acpi-cpufreq
M			Y		- powernow-k8 sees already loaded acpi-cpufreq
Y			M		- impossible after patch 0001
Y			Y		- patches 2,3 fix that

Anyway, please take a look and let me know.

Thanks.




Borislav Petkov (5):
  powernow-k8: Add a kconfig dependency on acpi-cpufreq
  cpufreq: Make acpi-cpufreq link first
  cpufreq: Add a get_current_driver helper
  powernow-k8: Cleanup module request
  powernow-k8: Cleanup init function

Matthew Garrett (1):
  acpi-cpufreq: Do not load on K8

 drivers/cpufreq/Kconfig.x86    |  2 +-
 drivers/cpufreq/Makefile       |  5 +++--
 drivers/cpufreq/acpi-cpufreq.c |  6 ++++++
 drivers/cpufreq/cpufreq.c      | 14 +++++++++++++
 drivers/cpufreq/powernow-k8.c  | 46 ++++++++++++++++++++++++++++++------------
 include/linux/cpufreq.h        |  1 +
 6 files changed, 58 insertions(+), 16 deletions(-)

-- 
1.8.1.rc3

[PATCH 1/6] powernow-k8: Add a kconfig dependency on acpi-cpufreq

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Andreas reports in https://bugzilla.kernel.org/show_bug.cgi?id=51741
that with his Gentoo config, acpi-cpufreq wasn't enabled and powernow-k8
couldn't handoff properly to acpi-cpufreq leading to running without
P-state support (i.e., cores are constantly in P0).

To alleaviate that, we need to make powernow-k8 depend on acpi-cpufreq
so that acpi-cpufreq is always present.

Reported-by: Andreas <linuxuser330250@gmx.net>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/cpufreq/Kconfig.x86 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index 934854ae5eb4..7227cd734042 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -106,7 +106,7 @@ config X86_POWERNOW_K7_ACPI
 config X86_POWERNOW_K8
 	tristate "AMD Opteron/Athlon64 PowerNow!"
 	select CPU_FREQ_TABLE
-	depends on ACPI && ACPI_PROCESSOR
+	depends on ACPI && ACPI_PROCESSOR && X86_ACPI_CPUFREQ
 	help
 	  This adds the CPUFreq driver for K8/early Opteron/Athlon64 processors.
 	  Support for K10 and newer processors is now in acpi-cpufreq.
-- 
1.8.1.rc3

[PATCH 2/6] acpi-cpufreq: Do not load on K8

[Borislav Petkov]

From: Matthew Garrett <mjg59@srcf.ucam.org>

de3ed81d746d ("[CPUFREQ] Change link order of x86 cpufreq modules")
changed cpufreq drivers link order so that powernow-k8 gets loaded first
due to earlier K8s having BIOS bugs.

However, now that acpi-cpufreq supports both AMD and Intel CPUs with HW
P-states, we want to load it first, so that cases where acpi-cpufreq and
powernow-k8 are both built-in and powernow-k8 initializing first, can be
addressed.

So, make sure that even if acpi-cpufreq gets loaded first, it errors out
on K8s and powernow-k8 can be loaded then successfully.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
Link: http://lkml.kernel.org/r/20130118162347.GA31499@srcf.ucam.org
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/cpufreq/acpi-cpufreq.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 0d048f6a2b23..31b06bbab54c 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -762,6 +762,12 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 
 	switch (perf->control_register.space_id) {
 	case ACPI_ADR_SPACE_SYSTEM_IO:
+		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
+		    boot_cpu_data.x86 == 0xf) {
+			pr_debug("AMD K8 systems must use native drivers.\n");
+			result = -ENODEV;
+			goto err_unreg;
+		}
 		pr_debug("SYSTEM IO addr space\n");
 		data->cpu_feature = SYSTEM_IO_CAPABLE;
 		break;
-- 
1.8.1.rc3

[PATCH 3/6] cpufreq: Make acpi-cpufreq link first

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Now that the majority of x86 CPUs out there are supported by
acpi-cpufreq, we want it to load first and, in the AMD case, drop to
powernow-k8 only on K8s. If, however, both powernow-k8 and acpi-cpufreq
are built-in, the link order matters. Correct that.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/cpufreq/Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index fadc4d496e2f..24d4a63fd6cf 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -19,11 +19,12 @@ obj-$(CONFIG_GENERIC_CPUFREQ_CPU0)	+= cpufreq-cpu0.o
 ##################################################################################
 # x86 drivers.
 # Link order matters. K8 is preferred to ACPI because of firmware bugs in early
-# K8 systems. ACPI is preferred to all other hardware-specific drivers.
+# K8 systems. This is still the case but acpi-cpufreq errors out so that
+# powernow-k8 can load then. ACPI is preferred to all other hardware-specific drivers.
 # speedstep-* is preferred over p4-clockmod.
 
-obj-$(CONFIG_X86_POWERNOW_K8)		+= powernow-k8.o
 obj-$(CONFIG_X86_ACPI_CPUFREQ)		+= acpi-cpufreq.o mperf.o
+obj-$(CONFIG_X86_POWERNOW_K8)		+= powernow-k8.o
 obj-$(CONFIG_X86_PCC_CPUFREQ)		+= pcc-cpufreq.o
 obj-$(CONFIG_X86_POWERNOW_K6)		+= powernow-k6.o
 obj-$(CONFIG_X86_POWERNOW_K7)		+= powernow-k7.o
-- 
1.8.1.rc3

[PATCH 4/6] cpufreq: Add a get_current_driver helper

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Add a helper function to return cpufreq_driver->name.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/cpufreq/cpufreq.c | 14 ++++++++++++++
 include/linux/cpufreq.h   |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1f93dbd72355..6ed3c1377caf 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1386,6 +1386,20 @@ static struct syscore_ops cpufreq_syscore_ops = {
 	.resume		= cpufreq_bp_resume,
 };
 
+/**
+ *	cpufreq_get_current_driver - return current driver's name
+ *
+ *	Return the name string of the currently loaded cpufreq driver
+ *	or NULL, if none.
+ */
+const char *cpufreq_get_current_driver(void)
+{
+	if (cpufreq_driver)
+		return cpufreq_driver->name;
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(cpufreq_get_current_driver);
 
 /*********************************************************************
  *                     NOTIFIER LISTS INTERFACE                      *
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index a55b88eaf96a..a018da2d2a7c 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -407,4 +407,5 @@ void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table,
 				      unsigned int cpu);
 
 void cpufreq_frequency_table_put_attr(unsigned int cpu);
+extern const char *cpufreq_get_current_driver(void);
 #endif /* _LINUX_CPUFREQ_H */
-- 
1.8.1.rc3

[PATCH 5/6] powernow-k8: Cleanup module request

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Check whether we've actually already loaded acpi-cpufreq before
requesting it.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/cpufreq/powernow-k8.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index 056faf6af1a9..0234d0c6d68c 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -1249,6 +1249,24 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
 	.attr		= powernow_k8_attr,
 };
 
+static void __request_acpi_cpufreq(void)
+{
+	const char *cur_drv, *drv = "acpi-cpufreq";
+
+	cur_drv = cpufreq_get_current_driver();
+	if (!cur_drv)
+		goto request;
+
+	if (strncmp(cur_drv, drv, min_t(size_t, strlen(cur_drv), strlen(drv))))
+		pr_warn(PFX "WTF driver: %s\n", cur_drv);
+
+	return;
+
+ request:
+	pr_warn(PFX "This CPU is not supported anymore, using acpi-cpufreq instead.\n");
+	request_module(drv);
+}
+
 /* driver entry point for init */
 static int __cpuinit powernowk8_init(void)
 {
@@ -1256,8 +1274,7 @@ static int __cpuinit powernowk8_init(void)
 	int rv;
 
 	if (static_cpu_has(X86_FEATURE_HW_PSTATE)) {
-		pr_warn(PFX "this CPU is not supported anymore, using acpi-cpufreq instead.\n");
-		request_module("acpi-cpufreq");
+		__request_acpi_cpufreq();
 		return -ENODEV;
 	}
 
-- 
1.8.1.rc3

[PATCH 6/6] powernow-k8: Cleanup init function

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Make it hotplug-safe and cleanup formatting.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/cpufreq/powernow-k8.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index 0234d0c6d68c..d13a13678b5f 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -1271,7 +1271,7 @@ static void __request_acpi_cpufreq(void)
 static int __cpuinit powernowk8_init(void)
 {
 	unsigned int i, supported_cpus = 0;
-	int rv;
+	int ret;
 
 	if (static_cpu_has(X86_FEATURE_HW_PSTATE)) {
 		__request_acpi_cpufreq();
@@ -1281,24 +1281,27 @@ static int __cpuinit powernowk8_init(void)
 	if (!x86_match_cpu(powernow_k8_ids))
 		return -ENODEV;
 
+	get_online_cpus();
 	for_each_online_cpu(i) {
-		int rc;
-		smp_call_function_single(i, check_supported_cpu, &rc, 1);
-		if (rc == 0)
+		smp_call_function_single(i, check_supported_cpu, &ret, 1);
+		if (!ret)
 			supported_cpus++;
 	}
 
-	if (supported_cpus != num_online_cpus())
+	if (supported_cpus != num_online_cpus()) {
+		put_online_cpus();
 		return -ENODEV;
+	}
+	put_online_cpus();
 
-	rv = cpufreq_register_driver(&cpufreq_amd64_driver);
+	ret = cpufreq_register_driver(&cpufreq_amd64_driver);
+	if (ret)
+		return ret;
 
-	if (!rv)
-		pr_info(PFX "Found %d %s (%d cpu cores) (" VERSION ")\n",
-			num_online_nodes(), boot_cpu_data.x86_model_id,
-			supported_cpus);
+	pr_info(PFX "Found %d %s (%d cpu cores) (" VERSION ")\n",
+		num_online_nodes(), boot_cpu_data.x86_model_id, supported_cpus);
 
-	return rv;
+	return ret;
 }
 
 /* driver entry point for term */
-- 
1.8.1.rc3

Re: [PATCH 0/6] acpi-cpufreq and powernow-k8 fixes

[Rafael J. Wysocki]

On Sunday, January 20, 2013 11:24:24 AM Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Hi all,
> 
> here are a couple of patches fixing acpi-cpufreq and powernow-k8 loading
> sequence and handoff.
> 
> Patch 0001 should be in Rafael's tree already but I'm adding it here for
> the sake of completeness. Basically, we want to address all possible
> cases of powernow-k8 and acpi-cpufreq:
> 
> (M: module, Y: built-in)
> 
> powernow-k8		acpi-cpufreq
> M			M		- powernow-k8 requests acpi-cpufreq
> M			Y		- powernow-k8 sees already loaded acpi-cpufreq
> Y			M		- impossible after patch 0001
> Y			Y		- patches 2,3 fix that
> 
> Anyway, please take a look and let me know.

Well, it all looks reasonable to me.

I suppose you'd like it to go into v3.8 plus v3.7-stable, right?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/6] acpi-cpufreq and powernow-k8 fixes

[Borislav Petkov]

On Sun, Jan 20, 2013 at 03:18:11PM +0100, Rafael J. Wysocki wrote:
> Well, it all looks reasonable to me.
>
> I suppose you'd like it to go into v3.8 plus v3.7-stable, right?

Well, sort of.

Patch 0001 which you've already taken and tagged for 3.7-stable and is
on its way to 3.8 is fine since it fixes a bug.

The rest are, IMHO, more or less annoyances fixes which could easily
wait for 3.9 and maybe be backported if there really is an issue. Also,
they'd see a lot more testing this way. Because in at least 99% of the
cases, acpi-cpufreq and powernow-k8 are modules and the handoff works
there.

This is at least how I see it, you tell me whether it makes sense or
not.

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 0/6] acpi-cpufreq and powernow-k8 fixes

[Rafael J. Wysocki]

On Monday, January 21, 2013 01:04:02 AM Borislav Petkov wrote:
> On Sun, Jan 20, 2013 at 03:18:11PM +0100, Rafael J. Wysocki wrote:
> > Well, it all looks reasonable to me.
> >
> > I suppose you'd like it to go into v3.8 plus v3.7-stable, right?
> 
> Well, sort of.
> 
> Patch 0001 which you've already taken and tagged for 3.7-stable and is
> on its way to 3.8 is fine since it fixes a bug.
> 
> The rest are, IMHO, more or less annoyances fixes which could easily
> wait for 3.9 and maybe be backported if there really is an issue. Also,
> they'd see a lot more testing this way. Because in at least 99% of the
> cases, acpi-cpufreq and powernow-k8 are modules and the handoff works
> there.
> 
> This is at least how I see it, you tell me whether it makes sense or
> not.

Yes, it does.

I will take [2-6/6] for v3.9, then, if there are no objections.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/6] acpi-cpufreq and powernow-k8 fixes

[Rafael J. Wysocki]

On Sunday, January 20, 2013 11:24:24 AM Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Hi all,
> 
> here are a couple of patches fixing acpi-cpufreq and powernow-k8 loading
> sequence and handoff.
> 
> Patch 0001 should be in Rafael's tree already but I'm adding it here for
> the sake of completeness. Basically, we want to address all possible
> cases of powernow-k8 and acpi-cpufreq:
> 
> (M: module, Y: built-in)
> 
> powernow-k8		acpi-cpufreq
> M			M		- powernow-k8 requests acpi-cpufreq
> M			Y		- powernow-k8 sees already loaded acpi-cpufreq
> Y			M		- impossible after patch 0001
> Y			Y		- patches 2,3 fix that
> 
> Anyway, please take a look and let me know.
> 
> Thanks.
> 
> 
> 
> 
> Borislav Petkov (5):
>   powernow-k8: Add a kconfig dependency on acpi-cpufreq
>   cpufreq: Make acpi-cpufreq link first
>   cpufreq: Add a get_current_driver helper
>   powernow-k8: Cleanup module request
>   powernow-k8: Cleanup init function
> 
> Matthew Garrett (1):
>   acpi-cpufreq: Do not load on K8

Applied.

They are in linux-pm/bleeding-edge for now, I'll move them to linux-pm/linux-next
after build testing.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 4/6] cpufreq: Add a get_current_driver helper

[Viresh Kumar]

On Sun, Jan 20, 2013 at 3:54 PM, Borislav Petkov <bp@alien8.de> wrote:
> From: Borislav Petkov <bp@suse.de>
>
> Add a helper function to return cpufreq_driver->name.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  drivers/cpufreq/cpufreq.c | 14 ++++++++++++++
>  include/linux/cpufreq.h   |  1 +
>  2 files changed, 15 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 1f93dbd72355..6ed3c1377caf 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1386,6 +1386,20 @@ static struct syscore_ops cpufreq_syscore_ops = {
>         .resume         = cpufreq_bp_resume,
>  };
>
> +/**
> + *     cpufreq_get_current_driver - return current driver's name
> + *
> + *     Return the name string of the currently loaded cpufreq driver
> + *     or NULL, if none.
> + */
> +const char *cpufreq_get_current_driver(void)
> +{
> +       if (cpufreq_driver)
> +               return cpufreq_driver->name;
> +
> +       return NULL;
> +}
> +EXPORT_SYMBOL_GPL(cpufreq_get_current_driver);
>
>  /*********************************************************************
>   *                     NOTIFIER LISTS INTERFACE                      *
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index a55b88eaf96a..a018da2d2a7c 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -407,4 +407,5 @@ void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table,
>                                       unsigned int cpu);
>
>  void cpufreq_frequency_table_put_attr(unsigned int cpu);
> +extern const char *cpufreq_get_current_driver(void);

Two minor things here:
- You placed the routine at bad place. This place is meant for
freq_table helpers.
- And you really don't need extern for function prototypes.

And after fixing these, feel free to add

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH 4/6] cpufreq: Add a get_current_driver helper

[Borislav Petkov]

On Mon, Jan 28, 2013 at 10:07:10AM +0530, Viresh Kumar wrote:
> Two minor things here:
> - You placed the routine at bad place. This place is meant for
> freq_table helpers.

Which one would you suggest: CPUFREQ DRIVER INTERFACE?

Btw, this function type splitting looks kinda silly to me. In that same
section there are two which clearly don't belong there:

struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
void   cpufreq_cpu_put(struct cpufreq_policy *data);

since they don't have anything to do with freq_table. And since they
return cpufreq_policy, they should be maybe called:

cpufreq_get_cpu_policy
cpufreq_put_cpu_policy

or something more expressive, pertaining to what they actually do. But
they actually get/put the module (inc/dec the module's refcount) *and*
return the cpufreq_policy... whatever, I don't care that much so see
below.

> - And you really don't need extern for function prototypes.

Yeah, Rafael corrected that when committing:

http://git.kernel.org/?p=linux/kernel/git/rafael/linux-pm.git;a=commitdiff;h=fcbf43dcacb0daecffca0f5c862c9d8815108e58

--
>From 830eb9ebc715d329229772d11332c50dfd1d9330 Mon Sep 17 00:00:00 2001
From: Borislav Petkov <bp@suse.de>
Date: Mon, 28 Jan 2013 10:38:33 +0100
Subject: [PATCH] cpufreq: Sort function prototypes properly

Move function prototypes to a place where they logically fit better.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 include/linux/cpufreq.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index a018da2d2a7c..f33ace57adb6 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -308,6 +308,9 @@ __ATTR(_name, 0444, show_##_name, NULL)
 static struct global_attr _name =		\
 __ATTR(_name, 0644, show_##_name, store_##_name)
 
+struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
+void   cpufreq_cpu_put(struct cpufreq_policy *data);
+const char *cpufreq_get_current_driver(void);
 
 /*********************************************************************
  *                        CPUFREQ 2.6. INTERFACE                     *
@@ -397,8 +400,6 @@ int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
 
 /* the following 3 funtions are for cpufreq core use only */
 struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu);
-struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
-void   cpufreq_cpu_put(struct cpufreq_policy *data);
 
 /* the following are really really optional */
 extern struct freq_attr cpufreq_freq_attr_scaling_available_freqs;
@@ -407,5 +408,4 @@ void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table,
 				      unsigned int cpu);
 
 void cpufreq_frequency_table_put_attr(unsigned int cpu);
-extern const char *cpufreq_get_current_driver(void);
 #endif /* _LINUX_CPUFREQ_H */
-- 
1.8.1.rc3


-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 4/6] cpufreq: Add a get_current_driver helper

[Borislav Petkov]

On Mon, Jan 28, 2013 at 10:07:10AM +0530, Viresh Kumar wrote:
> Two minor things here:
> - You placed the routine at bad place. This place is meant for
> freq_table helpers.

Which one would you suggest: CPUFREQ DRIVER INTERFACE?

Btw, this function type splitting looks kinda silly to me. In that same
section there are two which clearly don't belong there:

struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
void   cpufreq_cpu_put(struct cpufreq_policy *data);

since they don't have anything to do with freq_table. And since they
return cpufreq_policy, they should be maybe called:

cpufreq_get_cpu_policy
cpufreq_put_cpu_policy

or something more expressive, pertaining to what they actually do. But
they actually get/put the module (inc/dec the module's refcount) *and*
return the cpufreq_policy... whatever, I don't care that much so see
below.

> - And you really don't need extern for function prototypes.

Yeah, Rafael corrected that when committing:

http://git.kernel.org/?p=linux/kernel/git/rafael/linux-pm.git;a=commitdiff;h=fcbf43dcacb0daecffca0f5c862c9d8815108e58

--
From 830eb9ebc715d329229772d11332c50dfd1d9330 Mon Sep 17 00:00:00 2001
From: Borislav Petkov <bp@suse.de>
Date: Mon, 28 Jan 2013 10:38:33 +0100
Subject: [PATCH] cpufreq: Sort function prototypes properly

Move function prototypes to a place where they logically fit better.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 include/linux/cpufreq.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index a018da2d2a7c..f33ace57adb6 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -308,6 +308,9 @@ __ATTR(_name, 0444, show_##_name, NULL)
 static struct global_attr _name =		\
 __ATTR(_name, 0644, show_##_name, store_##_name)
 
+struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
+void   cpufreq_cpu_put(struct cpufreq_policy *data);
+const char *cpufreq_get_current_driver(void);
 
 /*********************************************************************
  *                        CPUFREQ 2.6. INTERFACE                     *
@@ -397,8 +400,6 @@ int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
 
 /* the following 3 funtions are for cpufreq core use only */
 struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu);
-struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
-void   cpufreq_cpu_put(struct cpufreq_policy *data);
 
 /* the following are really really optional */
 extern struct freq_attr cpufreq_freq_attr_scaling_available_freqs;
@@ -407,5 +408,4 @@ void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table,
 				      unsigned int cpu);
 
 void cpufreq_frequency_table_put_attr(unsigned int cpu);
-extern const char *cpufreq_get_current_driver(void);
 #endif /* _LINUX_CPUFREQ_H */
-- 
1.8.1.rc3


-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 4/6] cpufreq: Add a get_current_driver helper

[Viresh Kumar]

On 28 January 2013 15:19, Borislav Petkov <bp@alien8.de> wrote:
> Which one would you suggest: CPUFREQ DRIVER INTERFACE?

Yes.

> Btw, this function type splitting looks kinda silly to me. In that same
> section there are two which clearly don't belong there:
>
> struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
> void   cpufreq_cpu_put(struct cpufreq_policy *data);
>
> since they don't have anything to do with freq_table. And since they
> return cpufreq_policy, they should be maybe called:
>
> cpufreq_get_cpu_policy
> cpufreq_put_cpu_policy
>
> or something more expressive, pertaining to what they actually do. But
> they actually get/put the module (inc/dec the module's refcount) *and*
> return the cpufreq_policy... whatever, I don't care that much so see
> below.

Agree.

> --
> From 830eb9ebc715d329229772d11332c50dfd1d9330 Mon Sep 17 00:00:00 2001
> From: Borislav Petkov <bp@suse.de>
> Date: Mon, 28 Jan 2013 10:38:33 +0100
> Subject: [PATCH] cpufreq: Sort function prototypes properly
>
> Move function prototypes to a place where they logically fit better.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  include/linux/cpufreq.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index a018da2d2a7c..f33ace57adb6 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -308,6 +308,9 @@ __ATTR(_name, 0444, show_##_name, NULL)
>  static struct global_attr _name =              \
>  __ATTR(_name, 0644, show_##_name, store_##_name)
>
> +struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
> +void   cpufreq_cpu_put(struct cpufreq_policy *data);
> +const char *cpufreq_get_current_driver(void);
>
>  /*********************************************************************
>   *                        CPUFREQ 2.6. INTERFACE                     *
> @@ -397,8 +400,6 @@ int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
>
>  /* the following 3 funtions are for cpufreq core use only */
>  struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu);
> -struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
> -void   cpufreq_cpu_put(struct cpufreq_policy *data);
>
>  /* the following are really really optional */
>  extern struct freq_attr cpufreq_freq_attr_scaling_available_freqs;
> @@ -407,5 +408,4 @@ void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table,
>                                       unsigned int cpu);
>
>  void cpufreq_frequency_table_put_attr(unsigned int cpu);
> -extern const char *cpufreq_get_current_driver(void);
>  #endif /* _LINUX_CPUFREQ_H */

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>