[PATCH 6/6] cpufreq: probe the Integrator cpufreq driver from DT

[PATCH 6/6] cpufreq: probe the Integrator cpufreq driver from DT

[Linus Walleij]

This makes the Integrator cpufreq driver probe from the core
module device tree node through it's registered platforms
device, getting the memory base from the device tree,
remapping it and removing dependencies to <mach/platform.h>
and <mach/hardware.h> by moving the two affected CM
register offsets into the driver.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/cpufreq/integrator-cpufreq.c | 56 ++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/cpufreq/integrator-cpufreq.c b/drivers/cpufreq/integrator-cpufreq.c
index f7c99df..9faf0ca 100644
--- a/drivers/cpufreq/integrator-cpufreq.c
+++ b/drivers/cpufreq/integrator-cpufreq.c
@@ -15,18 +15,19 @@
 #include <linux/smp.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
-#include <mach/hardware.h>
-#include <mach/platform.h>
 #include <asm/mach-types.h>
 #include <asm/hardware/icst.h>
 
-static struct cpufreq_driver integrator_driver;
+static void __iomem *cm_base;
+/* The cpufreq driver only use the OSC register */
+#define INTEGRATOR_HDR_OSC_OFFSET       0x08
+#define INTEGRATOR_HDR_LOCK_OFFSET      0x14
 
-#define CM_ID  	__io_address(INTEGRATOR_HDR_ID)
-#define CM_OSC	__io_address(INTEGRATOR_HDR_OSC)
-#define CM_STAT __io_address(INTEGRATOR_HDR_STAT)
-#define CM_LOCK __io_address(INTEGRATOR_HDR_LOCK)
+static struct cpufreq_driver integrator_driver;
 
 static const struct icst_params lclk_params = {
 	.ref		= 24000000,
@@ -100,7 +101,7 @@ static int integrator_set_target(struct cpufreq_policy *policy,
 	BUG_ON(cpu != smp_processor_id());
 
 	/* get current setting */
-	cm_osc = __raw_readl(CM_OSC);
+	cm_osc = __raw_readl(cm_base + INTEGRATOR_HDR_OSC_OFFSET);
 
 	if (machine_is_integrator()) {
 		vco.s = (cm_osc >> 8) & 7;
@@ -128,7 +129,7 @@ static int integrator_set_target(struct cpufreq_policy *policy,
 
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
 
-	cm_osc = __raw_readl(CM_OSC);
+	cm_osc = __raw_readl(cm_base + INTEGRATOR_HDR_OSC_OFFSET);
 
 	if (machine_is_integrator()) {
 		cm_osc &= 0xfffff800;
@@ -138,9 +139,9 @@ static int integrator_set_target(struct cpufreq_policy *policy,
 	}
 	cm_osc |= vco.v;
 
-	__raw_writel(0xa05f, CM_LOCK);
-	__raw_writel(cm_osc, CM_OSC);
-	__raw_writel(0, CM_LOCK);
+	__raw_writel(0xa05f, cm_base + INTEGRATOR_HDR_LOCK_OFFSET);
+	__raw_writel(cm_osc, cm_base + INTEGRATOR_HDR_OSC_OFFSET);
+	__raw_writel(0, cm_base + INTEGRATOR_HDR_LOCK_OFFSET);
 
 	/*
 	 * Restore the CPUs allowed mask.
@@ -165,7 +166,7 @@ static unsigned int integrator_get(unsigned int cpu)
 	BUG_ON(cpu != smp_processor_id());
 
 	/* detect memory etc. */
-	cm_osc = __raw_readl(CM_OSC);
+	cm_osc = __raw_readl(cm_base + INTEGRATOR_HDR_OSC_OFFSET);
 
 	if (machine_is_integrator()) {
 		vco.s = (cm_osc >> 8) & 7;
@@ -202,19 +203,38 @@ static struct cpufreq_driver integrator_driver = {
 	.name		= "integrator",
 };
 
-static int __init integrator_cpu_init(void)
+static int __init integrator_cpufreq_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
+
+	cm_base = of_iomap(np, 0);
+	if (!cm_base)
+		return -ENODEV;
 	return cpufreq_register_driver(&integrator_driver);
 }
 
-static void __exit integrator_cpu_exit(void)
+static void __exit integrator_cpufreq_remove(struct platform_device *pdev)
 {
 	cpufreq_unregister_driver(&integrator_driver);
 }
 
+static const struct of_device_id integrator_cpufreq_match[] = {
+	{ .compatible = "arm,core-module-integrator"},
+	{ },
+};
+
+static struct platform_driver integrator_cpufreq_driver = {
+	.driver = {
+		.name = "integrator-cpufreq",
+		.owner = THIS_MODULE,
+		.of_match_table = integrator_cpufreq_match,
+	},
+	.remove = __exit_p(integrator_cpufreq_remove),
+};
+
+module_platform_driver_probe(integrator_cpufreq_driver,
+			     integrator_cpufreq_probe);
+
 MODULE_AUTHOR ("Russell M. King");
 MODULE_DESCRIPTION ("cpufreq driver for ARM Integrator CPUs");
 MODULE_LICENSE ("GPL");
-
-module_init(integrator_cpu_init);
-module_exit(integrator_cpu_exit);
-- 
1.8.3.1

[PATCH 6/6] cpufreq: probe the Integrator cpufreq driver from DT

[Mark Rutland]

[adding devicetree to Cc]

Hi Linus,

[...]

> +static int __init integrator_cpufreq_probe(struct platform_device *pdev)
>  {
> +	struct device_node *np = pdev->dev.of_node;
> +
> +	cm_base = of_iomap(np, 0);
> +	if (!cm_base)
> +		return -ENODEV;
>  	return cpufreq_register_driver(&integrator_driver);

If we fail to register the driver we could unmap cm_base, no?

>  }
>  
> -static void __exit integrator_cpu_exit(void)
> +static void __exit integrator_cpufreq_remove(struct platform_device *pdev)
>  {
>  	cpufreq_unregister_driver(&integrator_driver);

Unmap here?

>  }
>  
> +static const struct of_device_id integrator_cpufreq_match[] = {
> +	{ .compatible = "arm,core-module-integrator"},
> +	{ },
> +};

It feels a little scary that this also got handled by some other code in
the previous patch. Is no arbitrarion required between the two when
accessing the core module registers?

Cheers,
Mark.

[PATCH 6/6] cpufreq: probe the Integrator cpufreq driver from DT

[Linus Walleij]

On Tue, Oct 8, 2013 at 12:00 PM, Mark Rutland <mark.rutland@arm.com> wrote:

>> +static int __init integrator_cpufreq_probe(struct platform_device *pdev)
>>  {
>> +     struct device_node *np = pdev->dev.of_node;
>> +
>> +     cm_base = of_iomap(np, 0);
>> +     if (!cm_base)
>> +             return -ENODEV;
>>       return cpufreq_register_driver(&integrator_driver);
>
> If we fail to register the driver we could unmap cm_base, no?

Using devm_* is better so I'll revise it to do this.

>> -static void __exit integrator_cpu_exit(void)
>> +static void __exit integrator_cpufreq_remove(struct platform_device *pdev)
>>  {
>>       cpufreq_unregister_driver(&integrator_driver);
>
> Unmap here?

Dito. Check v2.

>> +static const struct of_device_id integrator_cpufreq_match[] = {
>> +     { .compatible = "arm,core-module-integrator"},
>> +     { },
>> +};
>
> It feels a little scary that this also got handled by some other code in
> the previous patch. Is no arbitrarion required between the two when
> accessing the core module registers?

They are using totally different registers, so it should not be an
issue. Inside the mach folder there is some arbitration for the
register that is actually accessed by several consumers.

Yours,
Linus Walleij