[RFC PATCH 1/3] ARM: imx: cpuidle: Convert imx5 driver to platform driver

[RFC PATCH 1/3] ARM: imx: cpuidle: Convert imx5 driver to platform driver

[Daniel Lezcano]

The PM low level code is tied and called from the cpuidle driver.

The platform driver approach allows to split the pm code and the
cpuidle driver, hence keeping the code encapsulated inside the pm
code.

The idle method called through the arm_pm_idle is changed by a callback
in the cpuidle driver, assigned at init time from the platform_data
field of the platform device passed in the probe function.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-imx/cpuidle-imx5.c |   20 +++++++++++++++++---
 arch/arm/mach-imx/cpuidle.h      |    5 -----
 arch/arm/mach-imx/pm-imx5.c      |   11 +++++++++--
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-imx/cpuidle-imx5.c b/arch/arm/mach-imx/cpuidle-imx5.c
index 5a47e3c..2d49846 100644
--- a/arch/arm/mach-imx/cpuidle-imx5.c
+++ b/arch/arm/mach-imx/cpuidle-imx5.c
@@ -8,12 +8,14 @@
 
 #include <linux/cpuidle.h>
 #include <linux/module.h>
-#include <asm/system_misc.h>
+#include <linux/platform_device.h>
+
+static void (*imx5_idle)(void);
 
 static int imx5_cpuidle_enter(struct cpuidle_device *dev,
 			      struct cpuidle_driver *drv, int index)
 {
-	arm_pm_idle();
+	imx5_idle();
 	return index;
 }
 
@@ -31,7 +33,19 @@ static struct cpuidle_driver imx5_cpuidle_driver = {
 	.state_count = 1,
 };
 
-int __init imx5_cpuidle_init(void)
+static int imx5_cpuidle_probe(struct platform_device *dev)
 {
+	imx5_idle = dev->dev.platform_data;
+
 	return cpuidle_register(&imx5_cpuidle_driver, NULL);
 }
+
+static struct platform_driver imx5_driver_cpuidle = {
+	.driver = {
+		.name = "cpuidle-imx5",
+		.owner = THIS_MODULE,
+	},
+	.probe = imx5_cpuidle_probe,
+};
+
+module_platform_driver(imx5_driver_cpuidle);
diff --git a/arch/arm/mach-imx/cpuidle.h b/arch/arm/mach-imx/cpuidle.h
index 786f98e..9e93ea0 100644
--- a/arch/arm/mach-imx/cpuidle.h
+++ b/arch/arm/mach-imx/cpuidle.h
@@ -11,13 +11,8 @@
  */
 
 #ifdef CONFIG_CPU_IDLE
-extern int imx5_cpuidle_init(void);
 extern int imx6q_cpuidle_init(void);
 #else
-static inline int imx5_cpuidle_init(void)
-{
-	return 0;
-}
 static inline int imx6q_cpuidle_init(void)
 {
 	return 0;
diff --git a/arch/arm/mach-imx/pm-imx5.c b/arch/arm/mach-imx/pm-imx5.c
index 58aeaf5..9eeef85 100644
--- a/arch/arm/mach-imx/pm-imx5.c
+++ b/arch/arm/mach-imx/pm-imx5.c
@@ -13,12 +13,12 @@
 #include <linux/io.h>
 #include <linux/err.h>
 #include <linux/export.h>
+#include <linux/platform_device.h>
 #include <asm/cacheflush.h>
 #include <asm/system_misc.h>
 #include <asm/tlbflush.h>
 
 #include "common.h"
-#include "cpuidle.h"
 #include "crm-regs-imx5.h"
 #include "hardware.h"
 
@@ -149,6 +149,13 @@ static void imx5_pm_idle(void)
 	imx5_cpu_do_idle();
 }
 
+static struct platform_device imx5_cpuidle_pdev = {
+	.name = "cpuidle-imx5",
+	.dev = {
+		.platform_data = imx5_pm_idle,
+	},
+};
+
 static int __init imx5_pm_common_init(void)
 {
 	int ret;
@@ -166,7 +173,7 @@ static int __init imx5_pm_common_init(void)
 	/* Set the registers to the default cpu idle state. */
 	mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE);
 
-	return imx5_cpuidle_init();
+	return platform_device_register(&imx5_cpuidle_pdev);
 }
 
 void __init imx5_pm_init(void)
-- 
1.7.9.5

[RFC PATCH 2/3] ARM: imx: cpuidle: Convert imx6q driver to platform_driver

[Daniel Lezcano]

The pm code and the cpuidle driver are tied together. The platform
driver approach allows to split this code and move the pm low level
code from the driver to the pm block.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-imx/cpuidle-imx6q.c |   31 ++++++++++++++++++-------------
 arch/arm/mach-imx/cpuidle.h       |   20 --------------------
 arch/arm/mach-imx/mach-imx6q.c    |    8 --------
 arch/arm/mach-imx/pm-imx6q.c      |   30 ++++++++++++++++++++++++++++++
 4 files changed, 48 insertions(+), 41 deletions(-)
 delete mode 100644 arch/arm/mach-imx/cpuidle.h

diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
index 23ddfb6..9761278 100644
--- a/arch/arm/mach-imx/cpuidle-imx6q.c
+++ b/arch/arm/mach-imx/cpuidle-imx6q.c
@@ -7,16 +7,15 @@
  */
 
 #include <linux/cpuidle.h>
-#include <linux/module.h>
+#include <linux/platform_device.h>
 #include <asm/cpuidle.h>
 #include <asm/proc-fns.h>
 
-#include "common.h"
-#include "cpuidle.h"
-
 static atomic_t master = ATOMIC_INIT(0);
 static DEFINE_SPINLOCK(master_lock);
 
+static void (*imx6q_idle)(void);
+
 static int imx6q_enter_wait(struct cpuidle_device *dev,
 			    struct cpuidle_driver *drv, int index)
 {
@@ -27,9 +26,9 @@ static int imx6q_enter_wait(struct cpuidle_device *dev,
 		 */
 		if (!spin_trylock(&master_lock))
 			goto idle;
-		imx6q_set_lpm(WAIT_UNCLOCKED);
-		cpu_do_idle();
-		imx6q_set_lpm(WAIT_CLOCKED);
+
+		imx6q_idle();
+		
 		spin_unlock(&master_lock);
 		goto done;
 	}
@@ -63,13 +62,19 @@ static struct cpuidle_driver imx6q_cpuidle_driver = {
 	.safe_state_index = 0,
 };
 
-int __init imx6q_cpuidle_init(void)
+static int imx6q_cpuidle_probe(struct platform_device *dev)
 {
-	/* Need to enable SCU standby for entering WAIT modes */
-	imx_scu_standby_enable();
-
-	/* Set chicken bit to get a reliable WAIT mode support */
-	imx6q_set_chicken_bit();
+	imx6q_idle = dev->dev.platform_data;
 
 	return cpuidle_register(&imx6q_cpuidle_driver, NULL);
 }
+
+static struct platform_driver imx6q_driver_cpuidle = {
+	.driver = {
+		.name = "cpuidle-imx6q",
+		.owner = THIS_MODULE,
+	},
+	.probe = imx6q_cpuidle_probe,
+};
+
+module_platform_driver(imx6q_driver_cpuidle);
diff --git a/arch/arm/mach-imx/cpuidle.h b/arch/arm/mach-imx/cpuidle.h
deleted file mode 100644
index 9e93ea0..0000000
--- a/arch/arm/mach-imx/cpuidle.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/*
- * Copyright 2012 Freescale Semiconductor, Inc.
- * Copyright 2012 Linaro Ltd.
- *
- * The code contained herein is licensed under the GNU General Public
- * License. You may obtain a copy of the GNU General Public License
- * Version 2 or later at the following locations:
- *
- * http://www.opensource.org/licenses/gpl-license.html
- * http://www.gnu.org/copyleft/gpl.html
- */
-
-#ifdef CONFIG_CPU_IDLE
-extern int imx6q_cpuidle_init(void);
-#else
-static inline int imx6q_cpuidle_init(void)
-{
-	return 0;
-}
-#endif
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 90372a2..9ca5b40 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -37,7 +37,6 @@
 #include <asm/system_misc.h>
 
 #include "common.h"
-#include "cpuidle.h"
 #include "hardware.h"
 
 static u32 chip_revision;
@@ -265,13 +264,6 @@ static struct platform_device imx6q_cpufreq_pdev = {
 
 static void __init imx6q_init_late(void)
 {
-	/*
-	 * WAIT mode is broken on TO 1.0 and 1.1, so there is no point
-	 * to run cpuidle on them.
-	 */
-	if (imx6q_revision() > IMX_CHIP_REVISION_1_1)
-		imx6q_cpuidle_init();
-
 	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
 		imx6q_opp_init();
 		platform_device_register(&imx6q_cpufreq_pdev);
diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c
index 2049427..bbb6d1c 100644
--- a/arch/arm/mach-imx/pm-imx6q.c
+++ b/arch/arm/mach-imx/pm-imx6q.c
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/suspend.h>
+#include <linux/platform_device.h>
 #include <asm/cacheflush.h>
 #include <asm/proc-fns.h>
 #include <asm/suspend.h>
@@ -50,12 +51,41 @@ static int imx6q_pm_enter(suspend_state_t state)
 	return 0;
 }
 
+static void imx6q_pm_idle(void)
+{
+	imx6q_set_lpm(WAIT_UNCLOCKED);
+	cpu_do_idle();
+	imx6q_set_lpm(WAIT_CLOCKED);
+}
+
 static const struct platform_suspend_ops imx6q_pm_ops = {
 	.enter = imx6q_pm_enter,
 	.valid = suspend_valid_only_mem,
 };
 
+static struct platform_device imx6q_cpuidle_device = {
+	.name = "cpuidle-imx6q",
+	.dev = {
+		.platform_data = imx6q_pm_idle,
+	},
+};
+
 void __init imx6q_pm_init(void)
 {
+	/*
+	 * WAIT mode is broken on TO 1.0 and 1.1, so there is no point
+	 * to run cpuidle on them.
+	 */
+	if (imx6q_revision() > IMX_CHIP_REVISION_1_1) {
+
+		/* Need to enable SCU standby for entering WAIT modes */
+		imx_scu_standby_enable();
+
+		/* Set chicken bit to get a reliable WAIT mode support */
+		imx6q_set_chicken_bit();
+
+		platform_device_register(&imx6q_cpuidle_device);
+	}
+
 	suspend_set_ops(&imx6q_pm_ops);
 }
-- 
1.7.9.5

[RFC PATCH 3/3] ARM: imx: cpuidle: Move the drivers to drivers/cpuidle

[Daniel Lezcano]

For the sake of consistency and in order to facilitate code consolidation
across cpuidle drivers, moving them to the drivers/cpuidle directory is
appreciable. The different drivers for at91, calxeda, big-little (tc2),
kirkwood, ux500 and zynq are now in this directory.

As there is no more dependency between the low level pm code and the cpuidle
backend driver we can safely move it to the directory the drivers belong to.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-imx/Makefile                         |    5 -----
 drivers/cpuidle/Kconfig.arm                        |   14 ++++++++++++++
 drivers/cpuidle/Makefile                           |    2 ++
 .../mach-imx => drivers/cpuidle}/cpuidle-imx5.c    |    0
 .../mach-imx => drivers/cpuidle}/cpuidle-imx6q.c   |    0
 5 files changed, 16 insertions(+), 5 deletions(-)
 rename {arch/arm/mach-imx => drivers/cpuidle}/cpuidle-imx5.c (100%)
 rename {arch/arm/mach-imx => drivers/cpuidle}/cpuidle-imx6q.c (100%)

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 5383c58..876c72d 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -27,11 +27,6 @@ obj-$(CONFIG_MXC_AVIC) += avic.o
 obj-$(CONFIG_MXC_USE_EPIT) += epit.o
 obj-$(CONFIG_MXC_DEBUG_BOARD) += 3ds_debugboard.o
 
-ifeq ($(CONFIG_CPU_IDLE),y)
-obj-$(CONFIG_SOC_IMX5) += cpuidle-imx5.o
-obj-$(CONFIG_SOC_IMX6Q) += cpuidle-imx6q.o
-endif
-
 ifdef CONFIG_SND_IMX_SOC
 obj-y += ssi-fiq.o
 obj-y += ssi-fiq-ksym.o
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index f23bd75..2e1b9400 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -44,3 +44,17 @@ config ARM_AT91_CPUIDLE
 	depends on ARCH_AT91
 	help
 	  Select this to enable cpuidle for AT91 processors
+
+config ARM_IMX5_CPUIDLE
+        bool "Cpu Idle Driver for the IMX5 processors"
+	default y
+	depends on SOC_IMX5 
+	help
+	  Select this to enable cpuidle for IMX5 processors
+
+config ARM_IMX6Q_CPUIDLE
+        bool "Cpu Idle Driver for the IMX6Q processors"
+	default y
+	depends on SOC_IMX6Q
+	help
+	  Select this to enable cpuidle for IMX6Q processors
\ No newline at end of file
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 527be28..0695880 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -13,3 +13,5 @@ obj-$(CONFIG_ARM_KIRKWOOD_CPUIDLE)	+= cpuidle-kirkwood.o
 obj-$(CONFIG_ARM_ZYNQ_CPUIDLE)		+= cpuidle-zynq.o
 obj-$(CONFIG_ARM_U8500_CPUIDLE)         += cpuidle-ux500.o
 obj-$(CONFIG_ARM_AT91_CPUIDLE)          += cpuidle-at91.o
+obj-$(CONFIG_ARM_IMX5_CPUIDLE)		+= cpuidle-imx5.o
+obj-$(CONFIG_ARM_IMX6Q_CPUIDLE)		+= cpuidle-imx6q.o
diff --git a/arch/arm/mach-imx/cpuidle-imx5.c b/drivers/cpuidle/cpuidle-imx5.c
similarity index 100%
rename from arch/arm/mach-imx/cpuidle-imx5.c
rename to drivers/cpuidle/cpuidle-imx5.c
diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/drivers/cpuidle/cpuidle-imx6q.c
similarity index 100%
rename from arch/arm/mach-imx/cpuidle-imx6q.c
rename to drivers/cpuidle/cpuidle-imx6q.c
-- 
1.7.9.5

[RFC PATCH 3/3] ARM: imx: cpuidle: Move the drivers to drivers/cpuidle

[Sascha Hauer]

Daniel,

On Mon, Oct 28, 2013 at 09:49:33AM -0700, Daniel Lezcano wrote:
> +
> +config ARM_IMX5_CPUIDLE
> +        bool "Cpu Idle Driver for the IMX5 processors"
> +	default y
> +	depends on SOC_IMX5 
> +	help
> +	  Select this to enable cpuidle for IMX5 processors
> +
> +config ARM_IMX6Q_CPUIDLE
> +        bool "Cpu Idle Driver for the IMX6Q processors"
> +	default y
> +	depends on SOC_IMX6Q
> +	help
> +	  Select this to enable cpuidle for IMX6Q processors

I can't say much to this series except there's some whitespace damage
here. Please resend this series with Cc: Shawn Guo <shawn.guo@linaro.org>

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[RFC PATCH 3/3] ARM: imx: cpuidle: Move the drivers to drivers/cpuidle

[Shawn Guo]

On Wed, Nov 06, 2013 at 08:21:33AM +0100, Sascha Hauer wrote:
> Daniel,
> 
> On Mon, Oct 28, 2013 at 09:49:33AM -0700, Daniel Lezcano wrote:
> > +
> > +config ARM_IMX5_CPUIDLE
> > +        bool "Cpu Idle Driver for the IMX5 processors"
> > +	default y
> > +	depends on SOC_IMX5 
> > +	help
> > +	  Select this to enable cpuidle for IMX5 processors
> > +
> > +config ARM_IMX6Q_CPUIDLE
> > +        bool "Cpu Idle Driver for the IMX6Q processors"
> > +	default y
> > +	depends on SOC_IMX6Q
> > +	help
> > +	  Select this to enable cpuidle for IMX6Q processors
> 
> I can't say much to this series except there's some whitespace damage
> here. Please resend this series with Cc: Shawn Guo <shawn.guo@linaro.org>

I like it in general.  Will take a deep look and test it on hardware
tomorrow.

Shawn

[RFC PATCH 1/3] ARM: imx: cpuidle: Convert imx5 driver to platform driver

[Shawn Guo]

On Mon, Oct 28, 2013 at 09:49:31AM -0700, Daniel Lezcano wrote:
> @@ -149,6 +149,13 @@ static void imx5_pm_idle(void)
>  	imx5_cpu_do_idle();
>  }
>  
> +static struct platform_device imx5_cpuidle_pdev = {
> +	.name = "cpuidle-imx5",
> +	.dev = {
> +		.platform_data = imx5_pm_idle,

This is a little bit hackish and less future proof.  We should probably
create a data structure with the function hook as a field in it.

Shawn

> +	},
> +};
> +
>  static int __init imx5_pm_common_init(void)
>  {
>  	int ret;
> @@ -166,7 +173,7 @@ static int __init imx5_pm_common_init(void)
>  	/* Set the registers to the default cpu idle state. */
>  	mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE);
>  
> -	return imx5_cpuidle_init();
> +	return platform_device_register(&imx5_cpuidle_pdev);
>  }
>  
>  void __init imx5_pm_init(void)
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH 2/3] ARM: imx: cpuidle: Convert imx6q driver to platform_driver

[Shawn Guo]

On Mon, Oct 28, 2013 at 09:49:32AM -0700, Daniel Lezcano wrote:
> +static struct platform_device imx6q_cpuidle_device = {
> +	.name = "cpuidle-imx6q",
> +	.dev = {
> +		.platform_data = imx6q_pm_idle,
> +	},
> +};
> +
>  void __init imx6q_pm_init(void)
>  {
> +	/*
> +	 * WAIT mode is broken on TO 1.0 and 1.1, so there is no point
> +	 * to run cpuidle on them.
> +	 */
> +	if (imx6q_revision() > IMX_CHIP_REVISION_1_1) {
> +
> +		/* Need to enable SCU standby for entering WAIT modes */
> +		imx_scu_standby_enable();
> +
> +		/* Set chicken bit to get a reliable WAIT mode support */
> +		imx6q_set_chicken_bit();

So this is a behavior change.  Before the patch, these two functions
will not get called if CONFIG_CPU_IDLE is not enabled.  We only want to
call them when cpuidle driver is instantiated.

Shawn

> +
> +		platform_device_register(&imx6q_cpuidle_device);
> +	}
> +
>  	suspend_set_ops(&imx6q_pm_ops);
>  }
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH 1/3] ARM: imx: cpuidle: Convert imx5 driver to platform driver

[Daniel Lezcano]

On 11/07/2013 08:56 AM, Shawn Guo wrote:
> On Mon, Oct 28, 2013 at 09:49:31AM -0700, Daniel Lezcano wrote:
>> @@ -149,6 +149,13 @@ static void imx5_pm_idle(void)
>>   	imx5_cpu_do_idle();
>>   }
>>
>> +static struct platform_device imx5_cpuidle_pdev = {
>> +	.name = "cpuidle-imx5",
>> +	.dev = {
>> +		.platform_data = imx5_pm_idle,
>
> This is a little bit hackish and less future proof.  We should probably
> create a data structure with the function hook as a field in it.

Yeah, I agree that is what I was planning for the near future as soon as 
the driver is moved into the drivers/cpuidle directory. As the other 
drivers are following the same scheme I want to define a common ops 
structure to be shared across the different driver. But I need to have 
several drivers in the same place in order to define the different idle 
callback.

Is it acceptable we keep this for the moment as the other cpuidle driver 
like cpuidle-at91 and then consolidate with a structure with an 
additional patchset addressing several drivers at once ?

>> +	},
>> +};
>> +
>>   static int __init imx5_pm_common_init(void)
>>   {
>>   	int ret;
>> @@ -166,7 +173,7 @@ static int __init imx5_pm_common_init(void)
>>   	/* Set the registers to the default cpu idle state. */
>>   	mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE);
>>
>> -	return imx5_cpuidle_init();
>> +	return platform_device_register(&imx5_cpuidle_pdev);
>>   }
>>
>>   void __init imx5_pm_init(void)
>> --
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[RFC PATCH 2/3] ARM: imx: cpuidle: Convert imx6q driver to platform_driver

[Daniel Lezcano]

On 11/07/2013 09:07 AM, Shawn Guo wrote:
> On Mon, Oct 28, 2013 at 09:49:32AM -0700, Daniel Lezcano wrote:
>> +static struct platform_device imx6q_cpuidle_device = {
>> +	.name = "cpuidle-imx6q",
>> +	.dev = {
>> +		.platform_data = imx6q_pm_idle,
>> +	},
>> +};
>> +
>>   void __init imx6q_pm_init(void)
>>   {
>> +	/*
>> +	 * WAIT mode is broken on TO 1.0 and 1.1, so there is no point
>> +	 * to run cpuidle on them.
>> +	 */
>> +	if (imx6q_revision() > IMX_CHIP_REVISION_1_1) {
>> +
>> +		/* Need to enable SCU standby for entering WAIT modes */
>> +		imx_scu_standby_enable();
>> +
>> +		/* Set chicken bit to get a reliable WAIT mode support */
>> +		imx6q_set_chicken_bit();
>
> So this is a behavior change.  Before the patch, these two functions
> will not get called if CONFIG_CPU_IDLE is not enabled.  We only want to
> call them when cpuidle driver is instantiated.

Yep, I will look at changing that.

Thanks for the review !

  -- Daniel

>> +
>> +		platform_device_register(&imx6q_cpuidle_device);
>> +	}
>> +
>>   	suspend_set_ops(&imx6q_pm_ops);
>>   }
>> --
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[RFC PATCH 1/3] ARM: imx: cpuidle: Convert imx5 driver to platform driver

[Shawn Guo]

On Thu, Nov 07, 2013 at 09:33:34AM +0100, Daniel Lezcano wrote:
> On 11/07/2013 08:56 AM, Shawn Guo wrote:
> >On Mon, Oct 28, 2013 at 09:49:31AM -0700, Daniel Lezcano wrote:
> >>@@ -149,6 +149,13 @@ static void imx5_pm_idle(void)
> >>  	imx5_cpu_do_idle();
> >>  }
> >>
> >>+static struct platform_device imx5_cpuidle_pdev = {
> >>+	.name = "cpuidle-imx5",
> >>+	.dev = {
> >>+		.platform_data = imx5_pm_idle,
> >
> >This is a little bit hackish and less future proof.  We should probably
> >create a data structure with the function hook as a field in it.
> 
> Yeah, I agree that is what I was planning for the near future as
> soon as the driver is moved into the drivers/cpuidle directory. As
> the other drivers are following the same scheme I want to define a
> common ops structure to be shared across the different driver. But I
> need to have several drivers in the same place in order to define
> the different idle callback.
> 
> Is it acceptable we keep this for the moment as the other cpuidle
> driver like cpuidle-at91 and then consolidate with a structure with
> an additional patchset addressing several drivers at once ?

Okay.

Shawn