[PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Shawn Guo]

As multiplatform build is being adopted by more and more ARM platforms,
initcall function should be used very carefully.  For example, when
GENERIC_CPUFREQ_CPU0 is built in the kernel, cpu0_cpufreq_driver_init()
will be called on all the platforms to initialize cpufreq-cpu0 driver.

To eliminate this undesired the effect, the patch changes cpufreq-cpu0
driver to have it instantiated as a platform_driver.  Then it will only
run on platforms that create the platform_device "cpufreq-cpu0".

Along with the change, it also changes cpu_dev to be &pdev->dev,
so that managed functions can start working, and module build gets
supported too.

The existing users of cpufreq-cpu0 driver highbank and am33xx are also
updated accordingly to adapt the changes.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Mark Langsdorf <mark.langsdorf@calxeda.com>
Cc: AnilKumar Ch <anilkumar@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
Changes since v1:
 * Migrate cpufreq-cpu0 users in the same patch

Rafael,

The patch is based on Mark's highbank-cpufreq series and Nishanth's
"PM / OPP : export symbol consolidation" sereis.

Mark, AnilKumar,

I only compile-tested it on highbank and omap2.  Please give it a test
no hardware to make sure cpufreq-cpu0 still works for you.  Thanks.

Shawn

 arch/arm/mach-omap2/board-generic.c   |    1 +
 arch/arm/mach-omap2/cclock33xx_data.c |    2 +-
 arch/arm/mach-omap2/common.h          |    1 +
 arch/arm/mach-omap2/io.c              |    7 +++++++
 drivers/cpufreq/Kconfig               |    2 +-
 drivers/cpufreq/cpufreq-cpu0.c        |   35 ++++++++++++++++++++++-----------
 drivers/cpufreq/highbank-cpufreq.c    |    5 +++++
 7 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 53cb380b..b945480 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -139,6 +139,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)")
 	.init_irq	= omap_intc_of_init,
 	.handle_irq	= omap3_intc_handle_irq,
 	.init_machine	= omap_generic_init,
+	.init_late	= am33xx_init_late,
 	.timer		= &omap3_am33xx_timer,
 	.dt_compat	= am33xx_boards_compat,
 MACHINE_END
diff --git a/arch/arm/mach-omap2/cclock33xx_data.c b/arch/arm/mach-omap2/cclock33xx_data.c
index ea64ad6..acb1620 100644
--- a/arch/arm/mach-omap2/cclock33xx_data.c
+++ b/arch/arm/mach-omap2/cclock33xx_data.c
@@ -850,7 +850,7 @@ static struct omap_clk am33xx_clks[] = {
 	CLK(NULL,	"dpll_core_m5_ck",	&dpll_core_m5_ck,	CK_AM33XX),
 	CLK(NULL,	"dpll_core_m6_ck",	&dpll_core_m6_ck,	CK_AM33XX),
 	CLK(NULL,	"dpll_mpu_ck",		&dpll_mpu_ck,	CK_AM33XX),
-	CLK("cpu0",	NULL,			&dpll_mpu_ck,	CK_AM33XX),
+	CLK("cpufreq-cpu0.0",	NULL,		&dpll_mpu_ck,	CK_AM33XX),
 	CLK(NULL,	"dpll_mpu_m2_ck",	&dpll_mpu_m2_ck,	CK_AM33XX),
 	CLK(NULL,	"dpll_ddr_ck",		&dpll_ddr_ck,	CK_AM33XX),
 	CLK(NULL,	"dpll_ddr_m2_ck",	&dpll_ddr_m2_ck,	CK_AM33XX),
diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index 948bcaa..e3355df5 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -106,6 +106,7 @@ void omap2430_init_late(void);
 void omap3430_init_late(void);
 void omap35xx_init_late(void);
 void omap3630_init_late(void);
+void am33xx_init_late(void);
 void am35xx_init_late(void);
 void ti81xx_init_late(void);
 void omap4430_init_late(void);
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index 2c3fdd6..7acfb8a 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -535,6 +535,13 @@ void __init omap3630_init_late(void)
 	omap2_clk_enable_autoidle_all();
 }
 
+void __init am33xx_init_late(void)
+{
+	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", };
+
+	platform_device_register_full(&devinfo);
+}
+
 void __init am35xx_init_late(void)
 {
 	omap_mux_late_init();
diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index ea512f4..774dc1c 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -180,7 +180,7 @@ config CPU_FREQ_GOV_CONSERVATIVE
 	  If in doubt, say N.
 
 config GENERIC_CPUFREQ_CPU0
-	bool "Generic CPU0 cpufreq driver"
+	tristate "Generic CPU0 cpufreq driver"
 	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
 	select CPU_FREQ_TABLE
 	help
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 90e9d73..519c2f7 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -12,12 +12,12 @@
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
 #include <linux/clk.h>
-#include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/opp.h>
+#include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
@@ -174,7 +174,7 @@ static struct cpufreq_driver cpu0_cpufreq_driver = {
 	.attr = cpu0_cpufreq_attr,
 };
 
-static int cpu0_cpufreq_driver_init(void)
+static int cpu0_cpufreq_probe(struct platform_device *pdev)
 {
 	struct device_node *np;
 	int ret;
@@ -189,23 +189,17 @@ static int cpu0_cpufreq_driver_init(void)
 		return -ENOENT;
 	}
 
-	cpu_dev = get_cpu_device(0);
-	if (!cpu_dev) {
-		pr_err("failed to get cpu0 device\n");
-		ret = -ENODEV;
-		goto out_put_node;
-	}
-
+	cpu_dev = &pdev->dev;
 	cpu_dev->of_node = np;
 
-	cpu_clk = clk_get(cpu_dev, NULL);
+	cpu_clk = devm_clk_get(cpu_dev, NULL);
 	if (IS_ERR(cpu_clk)) {
 		ret = PTR_ERR(cpu_clk);
 		pr_err("failed to get cpu0 clock: %d\n", ret);
 		goto out_put_node;
 	}
 
-	cpu_reg = regulator_get(cpu_dev, "cpu0");
+	cpu_reg = devm_regulator_get(cpu_dev, "cpu0");
 	if (IS_ERR(cpu_reg)) {
 		pr_warn("failed to get cpu0 regulator\n");
 		cpu_reg = NULL;
@@ -266,7 +260,24 @@ out_put_node:
 	of_node_put(np);
 	return ret;
 }
-late_initcall(cpu0_cpufreq_driver_init);
+
+static int cpu0_cpufreq_remove(struct platform_device *pdev)
+{
+	cpufreq_unregister_driver(&cpu0_cpufreq_driver);
+	opp_free_cpufreq_table(cpu_dev, &freq_table);
+
+	return 0;
+}
+
+static struct platform_driver cpu0_cpufreq_platdrv = {
+	.driver = {
+		.name	= "cpufreq-cpu0",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= cpu0_cpufreq_probe,
+	.remove		= cpu0_cpufreq_remove,
+};
+module_platform_driver(cpu0_cpufreq_platdrv);
 
 MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
 MODULE_DESCRIPTION("Generic CPU0 cpufreq driver");
diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c
index 2ea6276..0491f1f 100644
--- a/drivers/cpufreq/highbank-cpufreq.c
+++ b/drivers/cpufreq/highbank-cpufreq.c
@@ -20,6 +20,7 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/mailbox.h>
+#include <linux/platform_device.h>
 
 #define HB_CPUFREQ_CHANGE_NOTE	0x80000001
 #define HB_CPUFREQ_IPC_LEN	7
@@ -65,6 +66,7 @@ static struct notifier_block hb_cpufreq_clk_nb = {
 
 static int hb_cpufreq_driver_init(void)
 {
+	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", };
 	struct device *cpu_dev;
 	struct clk *cpu_clk;
 	struct device_node *np;
@@ -104,6 +106,9 @@ static int hb_cpufreq_driver_init(void)
 		goto out_put_node;
 	}
 
+	/* Instantiate cpufreq-cpu0 */
+	platform_device_register_full(&devinfo);
+
 out_put_node:
 	of_node_put(np);
 	return ret;
-- 
1.7.9.5

Re: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Viresh Kumar]

On Tue, Jan 29, 2013 at 8:23 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> As multiplatform build is being adopted by more and more ARM platforms,
> initcall function should be used very carefully.  For example, when
> GENERIC_CPUFREQ_CPU0 is built in the kernel, cpu0_cpufreq_driver_init()
> will be called on all the platforms to initialize cpufreq-cpu0 driver.
>
> To eliminate this undesired the effect, the patch changes cpufreq-cpu0
> driver to have it instantiated as a platform_driver.  Then it will only
> run on platforms that create the platform_device "cpufreq-cpu0".
>
> Along with the change, it also changes cpu_dev to be &pdev->dev,
> so that managed functions can start working, and module build gets
> supported too.
>
> The existing users of cpufreq-cpu0 driver highbank and am33xx are also
> updated accordingly to adapt the changes.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Mark Langsdorf <mark.langsdorf@calxeda.com>
> Cc: AnilKumar Ch <anilkumar@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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

RE: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[AnilKumar, Chimata]

On Tue, Jan 29, 2013 at 08:23:40, Shawn Guo wrote:
> As multiplatform build is being adopted by more and more ARM platforms,
> initcall function should be used very carefully.  For example, when
> GENERIC_CPUFREQ_CPU0 is built in the kernel, cpu0_cpufreq_driver_init()
> will be called on all the platforms to initialize cpufreq-cpu0 driver.
> 
> To eliminate this undesired the effect, the patch changes cpufreq-cpu0
> driver to have it instantiated as a platform_driver.  Then it will only
> run on platforms that create the platform_device "cpufreq-cpu0".
> 
> Along with the change, it also changes cpu_dev to be &pdev->dev,
> so that managed functions can start working, and module build gets
> supported too.
> 
> The existing users of cpufreq-cpu0 driver highbank and am33xx are also
> updated accordingly to adapt the changes.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Mark Langsdorf <mark.langsdorf@calxeda.com>
> Cc: AnilKumar Ch <anilkumar@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> Changes since v1:
>  * Migrate cpufreq-cpu0 users in the same patch
> 
> Rafael,
> 
> The patch is based on Mark's highbank-cpufreq series and Nishanth's
> "PM / OPP : export symbol consolidation" sereis.
> 
> Mark, AnilKumar,
> 
> I only compile-tested it on highbank and omap2.  Please give it a test
> no hardware to make sure cpufreq-cpu0 still works for you.  Thanks.

Hi Shawn,

I hope this is based on linux-omap/master, to test the driver I have
to add some patches on top of this patch, because of recent changes.
I will provide the test details once I am done.

Thanks
AnilKumar

Re: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Rafael J. Wysocki]

On Wednesday, January 30, 2013 07:10:20 AM AnilKumar, Chimata wrote:
> On Tue, Jan 29, 2013 at 08:23:40, Shawn Guo wrote:
> > As multiplatform build is being adopted by more and more ARM platforms,
> > initcall function should be used very carefully.  For example, when
> > GENERIC_CPUFREQ_CPU0 is built in the kernel, cpu0_cpufreq_driver_init()
> > will be called on all the platforms to initialize cpufreq-cpu0 driver.
> > 
> > To eliminate this undesired the effect, the patch changes cpufreq-cpu0
> > driver to have it instantiated as a platform_driver.  Then it will only
> > run on platforms that create the platform_device "cpufreq-cpu0".
> > 
> > Along with the change, it also changes cpu_dev to be &pdev->dev,
> > so that managed functions can start working, and module build gets
> > supported too.
> > 
> > The existing users of cpufreq-cpu0 driver highbank and am33xx are also
> > updated accordingly to adapt the changes.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Mark Langsdorf <mark.langsdorf@calxeda.com>
> > Cc: AnilKumar Ch <anilkumar@ti.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > Changes since v1:
> >  * Migrate cpufreq-cpu0 users in the same patch
> > 
> > Rafael,
> > 
> > The patch is based on Mark's highbank-cpufreq series and Nishanth's
> > "PM / OPP : export symbol consolidation" sereis.
> > 
> > Mark, AnilKumar,
> > 
> > I only compile-tested it on highbank and omap2.  Please give it a test
> > no hardware to make sure cpufreq-cpu0 still works for you.  Thanks.
> 
> Hi Shawn,
> 
> I hope this is based on linux-omap/master, to test the driver I have
> to add some patches on top of this patch, because of recent changes.
> I will provide the test details once I am done.

Shawn, does it mean I need to wait with applying the patch or will OMAP2
work as is?

Rafael


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

Re: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Shawn Guo]

On Wed, Jan 30, 2013 at 02:40:35PM +0100, Rafael J. Wysocki wrote:
> > > Mark, AnilKumar,
> > > 
> > > I only compile-tested it on highbank and omap2.  Please give it a test
> > > no hardware to make sure cpufreq-cpu0 still works for you.  Thanks.
> > 
> > Hi Shawn,
> > 
> > I hope this is based on linux-omap/master, to test the driver I have
> > to add some patches on top of this patch, because of recent changes.
> > I will provide the test details once I am done.
> 
> Shawn, does it mean I need to wait with applying the patch or will OMAP2
> work as is?
> 
I would expect that omap2/am33xx should work as it is, but I do not
have hardware to confirm that.

Also as the patch touches a few files in arch/arm/mach-omap2, we may
need an ACK from Tony.

Actually, I prefer to move omap2 bits into a separate patch and have
it go via omap tree to avoid the possible conflict with cpufreq and
arm-soc tree.  What do you say?  If you agree, I will re-iterate the
patch to do so.

Shawn

Re: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Rafael J. Wysocki]

On Wednesday, January 30, 2013 09:49:41 PM Shawn Guo wrote:
> On Wed, Jan 30, 2013 at 02:40:35PM +0100, Rafael J. Wysocki wrote:
> > > > Mark, AnilKumar,
> > > > 
> > > > I only compile-tested it on highbank and omap2.  Please give it a test
> > > > no hardware to make sure cpufreq-cpu0 still works for you.  Thanks.
> > > 
> > > Hi Shawn,
> > > 
> > > I hope this is based on linux-omap/master, to test the driver I have
> > > to add some patches on top of this patch, because of recent changes.
> > > I will provide the test details once I am done.
> > 
> > Shawn, does it mean I need to wait with applying the patch or will OMAP2
> > work as is?
> > 
> I would expect that omap2/am33xx should work as it is, but I do not
> have hardware to confirm that.
> 
> Also as the patch touches a few files in arch/arm/mach-omap2, we may
> need an ACK from Tony.
> 
> Actually, I prefer to move omap2 bits into a separate patch and have
> it go via omap tree to avoid the possible conflict with cpufreq and
> arm-soc tree.  What do you say?  If you agree, I will re-iterate the
> patch to do so.

Yes, please do as you think is appropriate.

Thanks,
Rafael


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

Re: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Guennadi Liakhovetski]

On Tue, 29 Jan 2013, Shawn Guo wrote:

> As multiplatform build is being adopted by more and more ARM platforms,
> initcall function should be used very carefully.  For example, when
> GENERIC_CPUFREQ_CPU0 is built in the kernel, cpu0_cpufreq_driver_init()
> will be called on all the platforms to initialize cpufreq-cpu0 driver.
> 
> To eliminate this undesired the effect, the patch changes cpufreq-cpu0
> driver to have it instantiated as a platform_driver.  Then it will only
> run on platforms that create the platform_device "cpufreq-cpu0".

Sorry, confused. Before this used to be a generic cpufreq driver, usable 
on all (DT-enabled only) platforms. You just had to provide an OPP table, 
a clock, a regulator, similar to this

http://thread.gmane.org/gmane.linux.kernel.cpufreq/9510/focus=9509

(also see the complete thread for more information). Now this won't work 
obviously. Instead we now need a pseudo platform device to instantiate 
cpufreq-cpu0. This device cannot be put in DT, because it doesn't describe 
real hardware. So, we have to add register it from the board specific .c 
code, which we actually want to get rid of... Is this really what we want?

What about other cpufreq drivers? They have the same problem on 
multiplatform builds, right? Say, s3c2416-cpufreq.c. It also just 
initialises itself and starts looking for a clock, names "msysclk" with a 
NULL device pointer etc. Don't we need a common approach for cpufreq 
driver initialisation?

The decision which cpufreq driver to use is SoC-specific, right? We're 
unlikely to have several boards, using the same SoC, wishing to use 
different cpufreq drivers? The decision _whether_ or not to enable it and 
_which_ resources to use might be board-specific. So, how about adding a 
cpufreq call something like

cpufreq_driver_request("cpufreq-driver-name");

to be called by SoC-specific code. You can say it is not much different 
from adding a virtual device, but firstly I think such a use of a platform 
device is really an overkill. Secondly you still run a danger, that 
several platforms, built into a single image, register several devices for 
different cpufreq drivers, or even for one... With a special call you know 
there can be only one and you return -EBUSY to all further calls to that 
function.

Thanks
Guennadi

> 
> Along with the change, it also changes cpu_dev to be &pdev->dev,
> so that managed functions can start working, and module build gets
> supported too.
> 
> The existing users of cpufreq-cpu0 driver highbank and am33xx are also
> updated accordingly to adapt the changes.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Mark Langsdorf <mark.langsdorf@calxeda.com>
> Cc: AnilKumar Ch <anilkumar@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> Changes since v1:
>  * Migrate cpufreq-cpu0 users in the same patch
> 
> Rafael,
> 
> The patch is based on Mark's highbank-cpufreq series and Nishanth's
> "PM / OPP : export symbol consolidation" sereis.
> 
> Mark, AnilKumar,
> 
> I only compile-tested it on highbank and omap2.  Please give it a test
> no hardware to make sure cpufreq-cpu0 still works for you.  Thanks.
> 
> Shawn
> 
>  arch/arm/mach-omap2/board-generic.c   |    1 +
>  arch/arm/mach-omap2/cclock33xx_data.c |    2 +-
>  arch/arm/mach-omap2/common.h          |    1 +
>  arch/arm/mach-omap2/io.c              |    7 +++++++
>  drivers/cpufreq/Kconfig               |    2 +-
>  drivers/cpufreq/cpufreq-cpu0.c        |   35 ++++++++++++++++++++++-----------
>  drivers/cpufreq/highbank-cpufreq.c    |    5 +++++
>  7 files changed, 39 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> index 53cb380b..b945480 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -139,6 +139,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)")
>  	.init_irq	= omap_intc_of_init,
>  	.handle_irq	= omap3_intc_handle_irq,
>  	.init_machine	= omap_generic_init,
> +	.init_late	= am33xx_init_late,
>  	.timer		= &omap3_am33xx_timer,
>  	.dt_compat	= am33xx_boards_compat,
>  MACHINE_END
> diff --git a/arch/arm/mach-omap2/cclock33xx_data.c b/arch/arm/mach-omap2/cclock33xx_data.c
> index ea64ad6..acb1620 100644
> --- a/arch/arm/mach-omap2/cclock33xx_data.c
> +++ b/arch/arm/mach-omap2/cclock33xx_data.c
> @@ -850,7 +850,7 @@ static struct omap_clk am33xx_clks[] = {
>  	CLK(NULL,	"dpll_core_m5_ck",	&dpll_core_m5_ck,	CK_AM33XX),
>  	CLK(NULL,	"dpll_core_m6_ck",	&dpll_core_m6_ck,	CK_AM33XX),
>  	CLK(NULL,	"dpll_mpu_ck",		&dpll_mpu_ck,	CK_AM33XX),
> -	CLK("cpu0",	NULL,			&dpll_mpu_ck,	CK_AM33XX),
> +	CLK("cpufreq-cpu0.0",	NULL,		&dpll_mpu_ck,	CK_AM33XX),
>  	CLK(NULL,	"dpll_mpu_m2_ck",	&dpll_mpu_m2_ck,	CK_AM33XX),
>  	CLK(NULL,	"dpll_ddr_ck",		&dpll_ddr_ck,	CK_AM33XX),
>  	CLK(NULL,	"dpll_ddr_m2_ck",	&dpll_ddr_m2_ck,	CK_AM33XX),
> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index 948bcaa..e3355df5 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -106,6 +106,7 @@ void omap2430_init_late(void);
>  void omap3430_init_late(void);
>  void omap35xx_init_late(void);
>  void omap3630_init_late(void);
> +void am33xx_init_late(void);
>  void am35xx_init_late(void);
>  void ti81xx_init_late(void);
>  void omap4430_init_late(void);
> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
> index 2c3fdd6..7acfb8a 100644
> --- a/arch/arm/mach-omap2/io.c
> +++ b/arch/arm/mach-omap2/io.c
> @@ -535,6 +535,13 @@ void __init omap3630_init_late(void)
>  	omap2_clk_enable_autoidle_all();
>  }
>  
> +void __init am33xx_init_late(void)
> +{
> +	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", };
> +
> +	platform_device_register_full(&devinfo);
> +}
> +
>  void __init am35xx_init_late(void)
>  {
>  	omap_mux_late_init();
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index ea512f4..774dc1c 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -180,7 +180,7 @@ config CPU_FREQ_GOV_CONSERVATIVE
>  	  If in doubt, say N.
>  
>  config GENERIC_CPUFREQ_CPU0
> -	bool "Generic CPU0 cpufreq driver"
> +	tristate "Generic CPU0 cpufreq driver"
>  	depends on HAVE_CLK && REGULATOR && PM_OPP && OF
>  	select CPU_FREQ_TABLE
>  	help
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index 90e9d73..519c2f7 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -12,12 +12,12 @@
>  #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
>  
>  #include <linux/clk.h>
> -#include <linux/cpu.h>
>  #include <linux/cpufreq.h>
>  #include <linux/err.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/opp.h>
> +#include <linux/platform_device.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  
> @@ -174,7 +174,7 @@ static struct cpufreq_driver cpu0_cpufreq_driver = {
>  	.attr = cpu0_cpufreq_attr,
>  };
>  
> -static int cpu0_cpufreq_driver_init(void)
> +static int cpu0_cpufreq_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np;
>  	int ret;
> @@ -189,23 +189,17 @@ static int cpu0_cpufreq_driver_init(void)
>  		return -ENOENT;
>  	}
>  
> -	cpu_dev = get_cpu_device(0);
> -	if (!cpu_dev) {
> -		pr_err("failed to get cpu0 device\n");
> -		ret = -ENODEV;
> -		goto out_put_node;
> -	}
> -
> +	cpu_dev = &pdev->dev;
>  	cpu_dev->of_node = np;
>  
> -	cpu_clk = clk_get(cpu_dev, NULL);
> +	cpu_clk = devm_clk_get(cpu_dev, NULL);
>  	if (IS_ERR(cpu_clk)) {
>  		ret = PTR_ERR(cpu_clk);
>  		pr_err("failed to get cpu0 clock: %d\n", ret);
>  		goto out_put_node;
>  	}
>  
> -	cpu_reg = regulator_get(cpu_dev, "cpu0");
> +	cpu_reg = devm_regulator_get(cpu_dev, "cpu0");
>  	if (IS_ERR(cpu_reg)) {
>  		pr_warn("failed to get cpu0 regulator\n");
>  		cpu_reg = NULL;
> @@ -266,7 +260,24 @@ out_put_node:
>  	of_node_put(np);
>  	return ret;
>  }
> -late_initcall(cpu0_cpufreq_driver_init);
> +
> +static int cpu0_cpufreq_remove(struct platform_device *pdev)
> +{
> +	cpufreq_unregister_driver(&cpu0_cpufreq_driver);
> +	opp_free_cpufreq_table(cpu_dev, &freq_table);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver cpu0_cpufreq_platdrv = {
> +	.driver = {
> +		.name	= "cpufreq-cpu0",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= cpu0_cpufreq_probe,
> +	.remove		= cpu0_cpufreq_remove,
> +};
> +module_platform_driver(cpu0_cpufreq_platdrv);
>  
>  MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
>  MODULE_DESCRIPTION("Generic CPU0 cpufreq driver");
> diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c
> index 2ea6276..0491f1f 100644
> --- a/drivers/cpufreq/highbank-cpufreq.c
> +++ b/drivers/cpufreq/highbank-cpufreq.c
> @@ -20,6 +20,7 @@
>  #include <linux/err.h>
>  #include <linux/of.h>
>  #include <linux/mailbox.h>
> +#include <linux/platform_device.h>
>  
>  #define HB_CPUFREQ_CHANGE_NOTE	0x80000001
>  #define HB_CPUFREQ_IPC_LEN	7
> @@ -65,6 +66,7 @@ static struct notifier_block hb_cpufreq_clk_nb = {
>  
>  static int hb_cpufreq_driver_init(void)
>  {
> +	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", };
>  	struct device *cpu_dev;
>  	struct clk *cpu_clk;
>  	struct device_node *np;
> @@ -104,6 +106,9 @@ static int hb_cpufreq_driver_init(void)
>  		goto out_put_node;
>  	}
>  
> +	/* Instantiate cpufreq-cpu0 */
> +	platform_device_register_full(&devinfo);
> +
>  out_put_node:
>  	of_node_put(np);
>  	return ret;
> -- 
> 1.7.9.5
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Nishanth Menon]

-Anil as the mail id does not seem to be accepted by TI mail server
anymore.

On 16:47-20130322, Guennadi Liakhovetski wrote:
> On Tue, 29 Jan 2013, Shawn Guo wrote:
> 
> > As multiplatform build is being adopted by more and more ARM platforms,
> > initcall function should be used very carefully.  For example, when
> > GENERIC_CPUFREQ_CPU0 is built in the kernel, cpu0_cpufreq_driver_init()
> > will be called on all the platforms to initialize cpufreq-cpu0 driver.
> > 
> > To eliminate this undesired the effect, the patch changes cpufreq-cpu0
> > driver to have it instantiated as a platform_driver.  Then it will only
> > run on platforms that create the platform_device "cpufreq-cpu0".
> 
> Sorry, confused. Before this used to be a generic cpufreq driver, usable 
> on all (DT-enabled only) platforms. You just had to provide an OPP table, 
> a clock, a regulator, similar to this
> 
> http://thread.gmane.org/gmane.linux.kernel.cpufreq/9510/focus=9509
> 
> (also see the complete thread for more information). Now this won't work 
> obviously. Instead we now need a pseudo platform device to instantiate 
> cpufreq-cpu0. This device cannot be put in DT, because it doesn't describe 
> real hardware. So, we have to add register it from the board specific .c 
> code, which we actually want to get rid of... Is this really what we want?
> 
> What about other cpufreq drivers? They have the same problem on 
> multiplatform builds, right? Say, s3c2416-cpufreq.c. It also just 
> initialises itself and starts looking for a clock, names "msysclk" with a 
> NULL device pointer etc. Don't we need a common approach for cpufreq 
> driver initialisation?
> 
> The decision which cpufreq driver to use is SoC-specific, right? We're 
> unlikely to have several boards, using the same SoC, wishing to use 
> different cpufreq drivers? The decision _whether_ or not to enable it and 
> _which_ resources to use might be board-specific. So, how about adding a 
> cpufreq call something like
> 
> cpufreq_driver_request("cpufreq-driver-name");
> 
> to be called by SoC-specific code. You can say it is not much different 
> from adding a virtual device, but firstly I think such a use of a platform 
> device is really an overkill. Secondly you still run a danger, that 
> several platforms, built into a single image, register several devices for 
> different cpufreq drivers, or even for one... With a special call you know 
> there can be only one and you return -EBUSY to all further calls to that 
> function.

Just an note on this specific patch - I made this generic handling for
OMAP OPP handling in my series:
http://marc.info/?l=linux-omap&m=136371580826031&w=2
Handling non-DT and DT enabled boots are key for us as well here.

-- 
Regards,
Nishanth Menon

Re: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Shawn Guo]

On Fri, Mar 22, 2013 at 04:47:17PM +0100, Guennadi Liakhovetski wrote:
> Sorry, confused. Before this used to be a generic cpufreq driver, usable 
> on all (DT-enabled only) platforms. You just had to provide an OPP table, 
> a clock, a regulator, similar to this
> 
> http://thread.gmane.org/gmane.linux.kernel.cpufreq/9510/focus=9509
> 
> (also see the complete thread for more information). Now this won't work 
> obviously. Instead we now need a pseudo platform device to instantiate 
> cpufreq-cpu0. This device cannot be put in DT, because it doesn't describe 
> real hardware. So, we have to add register it from the board specific .c 
> code, which we actually want to get rid of... Is this really what we want?
> 
Yes, that's what we want.  We are moving to multiplatform build, and we
do not want the driver get instantiated on very single platform that is
part of multiplatform.  Some of them may not support cpufreq at all and
others may have operating-points defined in device tree but need to work
with platform specific cpufreq driver.

In an ideal DT world, there shouldn't be any board specific .c code but
only SoC specific code.  Whether or not support cpufreq is a SoC
specific decision, and it's perfectly fine to make the decision in SoC
specific code.

> What about other cpufreq drivers? They have the same problem on 
> multiplatform builds, right? Say, s3c2416-cpufreq.c. It also just 
> initialises itself and starts looking for a clock, names "msysclk" with a 
> NULL device pointer etc. Don't we need a common approach for cpufreq 
> driver initialisation?
> 
The s3c2416 is not part of multiplatform build yet.  And any platform
that wants to be part of multiplatform build needs to fix those global
initcalls.  Two approaches are being used to do that.

- Call of_machine_is_compatible() to check target platform at the
  beginning of the initcall, and return immediately if it's not the
  target platform.  See drivers/cpufreq/highbank-cpufreq.c for example.

- Remove global initcalls and use platform_device/driver mechanism for
  instantiation.  See drivers/cpufreq/imx6q-cpufreq.c for example.

I like the second one which is more scalable, and choose to use it for
cpufreq-cpu0 driver.

> The decision which cpufreq driver to use is SoC-specific, right? We're 
> unlikely to have several boards, using the same SoC, wishing to use 
> different cpufreq drivers? The decision _whether_ or not to enable it and 
> _which_ resources to use might be board-specific. So, how about adding a 
> cpufreq call something like
> 
> cpufreq_driver_request("cpufreq-driver-name");
> 
> to be called by SoC-specific code. You can say it is not much different 
> from adding a virtual device, but firstly I think such a use of a platform 
> device is really an overkill.

I do not see why this is an overkill.  It's pretty common to use the
approach for instantiating particular driver on particular target.

> Secondly you still run a danger, that 
> several platforms, built into a single image, register several devices for 
> different cpufreq drivers, or even for one... With a special call you know 
> there can be only one and you return -EBUSY to all further calls to that 
> function.

I do not see how this could happen.  The cpufreq device gets added in
target specific init function which will only be invoked when the kernel
is running on this target.  Check arch/arm/mach-imx/mach-imx6q.c or the
OMAP example given by Nishanth to see how this should be done.

Shawn

Re: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Guennadi Liakhovetski]

On Sat, 23 Mar 2013, Shawn Guo wrote:

> On Fri, Mar 22, 2013 at 04:47:17PM +0100, Guennadi Liakhovetski wrote:

[snip]

> > Secondly you still run a danger, that 
> > several platforms, built into a single image, register several devices for 
> > different cpufreq drivers, or even for one... With a special call you know 
> > there can be only one and you return -EBUSY to all further calls to that 
> > function.
> 
> I do not see how this could happen.  The cpufreq device gets added in
> target specific init function which will only be invoked when the kernel
> is running on this target.  Check arch/arm/mach-imx/mach-imx6q.c or the
> OMAP example given by Nishanth to see how this should be done.

Sorry, I meant buggy implementations, where an initcall is added without 
checking, whether it's running on supported hardware.

Already before your patch for cpufreq-cpu0 to instantiate "mistakenly" on 
unsupported hardware you had to have an "operating-points" property in 
your "cpus" node, and you needed a clock attached to your cpu0 device. Do 
you really think this was likely?

Anyway, I do find this an overkill and an abuse, but I'm not going to 
fight over it. If I'm the only one with this impression - no problem, just 
forget my ranting :)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v2] cpufreq: instantiate cpufreq-cpu0 as a platform_driver

[Guennadi Liakhovetski]

On Sun, 24 Mar 2013, Guennadi Liakhovetski wrote:

> On Sat, 23 Mar 2013, Shawn Guo wrote:
> 
> > On Fri, Mar 22, 2013 at 04:47:17PM +0100, Guennadi Liakhovetski wrote:
> 
> [snip]
> 
> > > Secondly you still run a danger, that 
> > > several platforms, built into a single image, register several devices for 
> > > different cpufreq drivers, or even for one... With a special call you know 
> > > there can be only one and you return -EBUSY to all further calls to that 
> > > function.
> > 
> > I do not see how this could happen.  The cpufreq device gets added in
> > target specific init function which will only be invoked when the kernel
> > is running on this target.  Check arch/arm/mach-imx/mach-imx6q.c or the
> > OMAP example given by Nishanth to see how this should be done.
> 
> Sorry, I meant buggy implementations, where an initcall is added without 
> checking, whether it's running on supported hardware.
> 
> Already before your patch for cpufreq-cpu0 to instantiate "mistakenly" on 
> unsupported hardware you had to have an "operating-points" property in 
> your "cpus" node, and you needed a clock attached to your cpu0 device. Do 
> you really think this was likely?
> 
> Anyway, I do find this an overkill and an abuse,

Look at this: in your now platform driver

static int cpu0_cpufreq_probe(struct platform_device *pdev)
{
	...

you get a DT node from "/cpus" with an "operating-points" property:

	for_each_child_of_node(of_find_node_by_path("/cpus"), np) {
		if (of_get_property(np, "operating-points", NULL))
			break;
	}

and then you patch your pseudo-, virtual platform device, that you're 
currently probing with this found DT node:

	cpu_dev = &pdev->dev;
	cpu_dev->of_node = np;

huh...

> but I'm not going to 
> fight over it. If I'm the only one with this impression - no problem, just 
> forget my ranting :)

Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/