[RFC patch 0/2] ARM: at91: cpuidle: move driver to drivers/cpuidle

[RFC patch 0/2] ARM: at91: cpuidle: move driver to drivers/cpuidle

[Daniel Lezcano]

The number of cpuidle drivers is increasing more and more. Today we have
in total 24 drivers. A lot of the code is duplicated, at least for the
initialization. A work of consolidation has been done during this year:
 * a lot of code cleanup in all the drivers
 * time keeping is now part of the framework
 * timer broadcast is now part of the framework
 * a WFI state function for ARM is defined and used in the drivers
 * an init function has been proposed to factor out the initialization across
   the drivers (patchset pending)

What has been observed is a lot of code duplicationis, modification made in
the framework takes awhile before reaching the driver which uses an old API,
duplicate routine and bugs, etc ...

It appears the drivers are belonging to different trees, the cpuidle framework
is under linux-pm, the drivers are per SoC tree. The communication is made
difficult because of the different mailing lists where the cpuidle are
submitted.

After this work, it is time to prevent all these problems to occur again.

I propose to move the cpuidle drivers to the drivers/cpuidle directory, hence
having one single submission path for cpuidle in order to have the cpuidle
framework and the different drivers synced.

This series move the AT91 cpuidle driver under drivers/cpuidle. That does not
change the rule to have the patches acked-by the author of the driver.

Note the calxeda and kirkwood drivers are now in drivers/cpuidle.

Daniel Lezcano (2):
  ARM: at91: cpuidle: encapsulate the standby code
  ARM: at91: cpuidle: move the driver to drivers/cpuidle directory

 arch/arm/mach-at91/Makefile  |    1 -
 arch/arm/mach-at91/cpuidle.c |   66 ------------------------------------------
 arch/arm/mach-at91/pm.c      |    8 ++++-
 drivers/cpuidle/Makefile     |    1 +
 drivers/cpuidle/at91.c       |   55 +++++++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+), 68 deletions(-)
 delete mode 100644 arch/arm/mach-at91/cpuidle.c
 create mode 100644 drivers/cpuidle/at91.c

-- 
1.7.9.5

[RFC patch 1/2] ARM: at91: cpuidle: encapsulate the standby code

[Daniel Lezcano]

In order to split the pm code from the cpuidle driver, add an ops for the
standby function which will be initialized by the pm init functions directly,
thus no need of the SoC specific headers.

Cleanup also the headers included in this file as they are no longer needed.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-at91/cpuidle.c |   19 ++++---------------
 arch/arm/mach-at91/pm.c      |    8 +++++++-
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index 48f1228..b2bec92 100644
--- a/arch/arm/mach-at91/cpuidle.c
+++ b/arch/arm/mach-at91/cpuidle.c
@@ -13,32 +13,21 @@
  * #2 wait-for-interrupt and RAM self refresh
  */
 
-#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/init.h>
-#include <linux/platform_device.h>
 #include <linux/cpuidle.h>
-#include <linux/io.h>
-#include <linux/export.h>
-#include <asm/proc-fns.h>
 #include <asm/cpuidle.h>
-#include <mach/cpu.h>
-
-#include "pm.h"
 
 #define AT91_MAX_STATES	2
 
+extern void (*at91_standby_ops)(void);
+
 /* Actual code that puts the SoC in different idle states */
 static int at91_enter_idle(struct cpuidle_device *dev,
 			struct cpuidle_driver *drv,
 			       int index)
 {
-	if (cpu_is_at91rm9200())
-		at91rm9200_standby();
-	else if (cpu_is_at91sam9g45())
-		at91sam9g45_standby();
-	else
-		at91sam9_standby();
-
+	at91_standby_ops();
 	return index;
 }
 
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 73f1f25..f456e86 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -39,6 +39,8 @@
 #include "at91_rstc.h"
 #include "at91_shdwc.h"
 
+void (*at91_standby_ops)(void);
+
 static void __init show_reset_status(void)
 {
 	static char reset[] __initdata = "reset";
@@ -321,8 +323,12 @@ static int __init at91_pm_init(void)
 	pr_info("AT91: Power Management%s\n", (slow_clock ? " (with slow clock mode)" : ""));
 
 	/* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. */
-	if (cpu_is_at91rm9200())
+	if (cpu_is_at91rm9200()) {
 		at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0);
+		at91_standby_ops = at91rm9200_standby;
+	} else if (cpu_is_at91sam9g45())
+		at91_standby_ops= at91sam9g45_standby;
+	else at91_standby_ops = at91sam9_standby;
 
 	suspend_set_ops(&at91_pm_ops);
 
-- 
1.7.9.5

[RFC patch 2/2] ARM: at91: cpuidle: move the driver to drivers/cpuidle directory

[Daniel Lezcano]

We don't have any dependency with the SoC specific code.

Move the driver to the drivers/cpuidle directory.

Add Nicolas Ferre as author of the driver, so it will be in copy of the emails.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-at91/Makefile  |    1 -
 arch/arm/mach-at91/cpuidle.c |   55 ----------------------------------------
 drivers/cpuidle/Makefile     |    1 +
 drivers/cpuidle/at91.c       |   57 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 56 deletions(-)
 delete mode 100644 arch/arm/mach-at91/cpuidle.c
 create mode 100644 drivers/cpuidle/at91.c

diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
index 39218ca..3c7fca1 100644
--- a/arch/arm/mach-at91/Makefile
+++ b/arch/arm/mach-at91/Makefile
@@ -99,7 +99,6 @@ obj-y				+= leds.o
 # Power Management
 obj-$(CONFIG_PM)		+= pm.o
 obj-$(CONFIG_AT91_SLOW_CLOCK)	+= pm_slowclock.o
-obj-$(CONFIG_CPU_IDLE)	+= cpuidle.o
 
 ifeq ($(CONFIG_PM_DEBUG),y)
 CFLAGS_pm.o += -DDEBUG
diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
deleted file mode 100644
index b2bec92..0000000
--- a/arch/arm/mach-at91/cpuidle.c
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * based on arch/arm/mach-kirkwood/cpuidle.c
- *
- * CPU idle support for AT91 SoC
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2.  This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- *
- * The cpu idle uses wait-for-interrupt and RAM self refresh in order
- * to implement two idle states -
- * #1 wait-for-interrupt
- * #2 wait-for-interrupt and RAM self refresh
- */
-
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/cpuidle.h>
-#include <asm/cpuidle.h>
-
-#define AT91_MAX_STATES	2
-
-extern void (*at91_standby_ops)(void);
-
-/* Actual code that puts the SoC in different idle states */
-static int at91_enter_idle(struct cpuidle_device *dev,
-			struct cpuidle_driver *drv,
-			       int index)
-{
-	at91_standby_ops();
-	return index;
-}
-
-static struct cpuidle_driver at91_idle_driver = {
-	.name			= "at91_idle",
-	.owner			= THIS_MODULE,
-	.states[0]		= ARM_CPUIDLE_WFI_STATE,
-	.states[1]		= {
-		.enter			= at91_enter_idle,
-		.exit_latency		= 10,
-		.target_residency	= 100000,
-		.flags			= CPUIDLE_FLAG_TIME_VALID,
-		.name			= "RAM_SR",
-		.desc			= "WFI and DDR Self Refresh",
-	},
-	.state_count = AT91_MAX_STATES,
-};
-
-/* Initialize CPU idle by registering the idle states */
-static int __init at91_init_cpuidle(void)
-{
-	return cpuidle_register(&at91_idle_driver, NULL);
-}
-
-device_initcall(at91_init_cpuidle);
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 0d8bd55..ba4e977 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
 
 obj-$(CONFIG_CPU_IDLE_CALXEDA) += cpuidle-calxeda.o
 obj-$(CONFIG_ARCH_KIRKWOOD) += cpuidle-kirkwood.o
+obj-$(CONFIG_ARCH_AT91) += at91.o
diff --git a/drivers/cpuidle/at91.c b/drivers/cpuidle/at91.c
new file mode 100644
index 0000000..2bc745b
--- /dev/null
+++ b/drivers/cpuidle/at91.c
@@ -0,0 +1,57 @@
+/*
+ * based on arch/arm/mach-kirkwood/cpuidle.c
+ *
+ * CPU idle support for AT91 SoC
+ *
+ * Author: Nicolas Ferre <nicolas.ferre@atmel.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ *
+ * The cpu idle uses wait-for-interrupt and RAM self refresh in order
+ * to implement two idle states -
+ * #1 wait-for-interrupt
+ * #2 wait-for-interrupt and RAM self refresh
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/cpuidle.h>
+#include <asm/cpuidle.h>
+
+#define AT91_MAX_STATES	2
+
+extern void (*at91_standby_ops)(void);
+
+/* Actual code that puts the SoC in different idle states */
+static int at91_enter_idle(struct cpuidle_device *dev,
+			struct cpuidle_driver *drv,
+			       int index)
+{
+	at91_standby_ops();
+	return index;
+}
+
+static struct cpuidle_driver at91_idle_driver = {
+	.name			= "at91_idle",
+	.owner			= THIS_MODULE,
+	.states[0]		= ARM_CPUIDLE_WFI_STATE,
+	.states[1]		= {
+		.enter			= at91_enter_idle,
+		.exit_latency		= 10,
+		.target_residency	= 100000,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "RAM_SR",
+		.desc			= "WFI and DDR Self Refresh",
+	},
+	.state_count = AT91_MAX_STATES,
+};
+
+/* Initialize CPU idle by registering the idle states */
+static int __init at91_init_cpuidle(void)
+{
+	return cpuidle_register(&at91_idle_driver, NULL);
+}
+
+device_initcall(at91_init_cpuidle);
-- 
1.7.9.5

[RFC patch 1/2] ARM: at91: cpuidle: encapsulate the standby code

[Nicolas Ferre]

Hi Daniel,

On 04/15/2013 03:29 PM, Daniel Lezcano :
> In order to split the pm code from the cpuidle driver, add an ops for the
> standby function which will be initialized by the pm init functions directly,
> thus no need of the SoC specific headers.
> 
> Cleanup also the headers included in this file as they are no longer needed.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  arch/arm/mach-at91/cpuidle.c |   19 ++++---------------
>  arch/arm/mach-at91/pm.c      |    8 +++++++-
>  2 files changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
> index 48f1228..b2bec92 100644
> --- a/arch/arm/mach-at91/cpuidle.c
> +++ b/arch/arm/mach-at91/cpuidle.c
> @@ -13,32 +13,21 @@
>   * #2 wait-for-interrupt and RAM self refresh
>   */
>  
> -#include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <linux/init.h>
> -#include <linux/platform_device.h>
>  #include <linux/cpuidle.h>
> -#include <linux/io.h>
> -#include <linux/export.h>
> -#include <asm/proc-fns.h>
>  #include <asm/cpuidle.h>
> -#include <mach/cpu.h>
> -
> -#include "pm.h"
>  
>  #define AT91_MAX_STATES	2
>  
> +extern void (*at91_standby_ops)(void);
> +
>  /* Actual code that puts the SoC in different idle states */
>  static int at91_enter_idle(struct cpuidle_device *dev,
>  			struct cpuidle_driver *drv,
>  			       int index)
>  {
> -	if (cpu_is_at91rm9200())
> -		at91rm9200_standby();
> -	else if (cpu_is_at91sam9g45())
> -		at91sam9g45_standby();
> -	else
> -		at91sam9_standby();
> -
> +	at91_standby_ops();
>  	return index;
>  }
>  
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 73f1f25..f456e86 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -39,6 +39,8 @@
>  #include "at91_rstc.h"
>  #include "at91_shdwc.h"
>  
> +void (*at91_standby_ops)(void);

Is this a common pattern to have such a floating function pointer in the
pm code?

> +
>  static void __init show_reset_status(void)
>  {
>  	static char reset[] __initdata = "reset";
> @@ -321,8 +323,12 @@ static int __init at91_pm_init(void)
>  	pr_info("AT91: Power Management%s\n", (slow_clock ? " (with slow clock mode)" : ""));
>  
>  	/* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. */
> -	if (cpu_is_at91rm9200())
> +	if (cpu_is_at91rm9200()) {
>  		at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0);
> +		at91_standby_ops = at91rm9200_standby;
> +	} else if (cpu_is_at91sam9g45())

CodingStyle: ending "{" is missing.

> +		at91_standby_ops= at91sam9g45_standby;

CodingStyle: " " is missing

> +	else at91_standby_ops = at91sam9_standby;

CodingStyle: not on the same line + "{}" missing

>  
>  	suspend_set_ops(&at91_pm_ops);

I am in favor for the move. But please rewrite a new series.

Bye,
-- 
Nicolas Ferre

[RFC patch 1/2] ARM: at91: cpuidle: encapsulate the standby code

[Daniel Lezcano]

On 04/15/2013 03:54 PM, Nicolas Ferre wrote:
> Hi Daniel,
> 
> On 04/15/2013 03:29 PM, Daniel Lezcano :
>> In order to split the pm code from the cpuidle driver, add an ops for the
>> standby function which will be initialized by the pm init functions directly,
>> thus no need of the SoC specific headers.
>>
>> Cleanup also the headers included in this file as they are no longer needed.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  arch/arm/mach-at91/cpuidle.c |   19 ++++---------------
>>  arch/arm/mach-at91/pm.c      |    8 +++++++-
>>  2 files changed, 11 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
>> index 48f1228..b2bec92 100644
>> --- a/arch/arm/mach-at91/cpuidle.c
>> +++ b/arch/arm/mach-at91/cpuidle.c
>> @@ -13,32 +13,21 @@
>>   * #2 wait-for-interrupt and RAM self refresh
>>   */
>>  
>> -#include <linux/kernel.h>
>> +#include <linux/module.h>
>>  #include <linux/init.h>
>> -#include <linux/platform_device.h>
>>  #include <linux/cpuidle.h>
>> -#include <linux/io.h>
>> -#include <linux/export.h>
>> -#include <asm/proc-fns.h>
>>  #include <asm/cpuidle.h>
>> -#include <mach/cpu.h>
>> -
>> -#include "pm.h"
>>  
>>  #define AT91_MAX_STATES	2
>>  
>> +extern void (*at91_standby_ops)(void);
>> +
>>  /* Actual code that puts the SoC in different idle states */
>>  static int at91_enter_idle(struct cpuidle_device *dev,
>>  			struct cpuidle_driver *drv,
>>  			       int index)
>>  {
>> -	if (cpu_is_at91rm9200())
>> -		at91rm9200_standby();
>> -	else if (cpu_is_at91sam9g45())
>> -		at91sam9g45_standby();
>> -	else
>> -		at91sam9_standby();
>> -
>> +	at91_standby_ops();
>>  	return index;
>>  }
>>  
>> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
>> index 73f1f25..f456e86 100644
>> --- a/arch/arm/mach-at91/pm.c
>> +++ b/arch/arm/mach-at91/pm.c
>> @@ -39,6 +39,8 @@
>>  #include "at91_rstc.h"
>>  #include "at91_shdwc.h"
>>  
>> +void (*at91_standby_ops)(void);
> 
> Is this a common pattern to have such a floating function pointer in the
> pm code?

Well, already seen but I agree it is not really nice.

The idea I had was to convert little by little all these pm functions
into ops, then order, group and use them to initialize the cpuidle
drivers through a single ARM driver. The correct order would have been
to convert first these to ops then move the driver but there are so many
drivers, so many code, I don't know where do I start.

Do you have a suggestion ?

>> +
>>  static void __init show_reset_status(void)
>>  {
>>  	static char reset[] __initdata = "reset";
>> @@ -321,8 +323,12 @@ static int __init at91_pm_init(void)
>>  	pr_info("AT91: Power Management%s\n", (slow_clock ? " (with slow clock mode)" : ""));
>>  
>>  	/* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. */
>> -	if (cpu_is_at91rm9200())
>> +	if (cpu_is_at91rm9200()) {
>>  		at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0);
>> +		at91_standby_ops = at91rm9200_standby;
>> +	} else if (cpu_is_at91sam9g45())
> 
> CodingStyle: ending "{" is missing.
> 
>> +		at91_standby_ops= at91sam9g45_standby;
> 
> CodingStyle: " " is missing
> 
>> +	else at91_standby_ops = at91sam9_standby;
> 
> CodingStyle: not on the same line + "{}" missing
> 
>>  
>>  	suspend_set_ops(&at91_pm_ops);
> 
> I am in favor for the move. 

Ok, cool.

> But please rewrite a new series.

Yes, sure, that was a draft.

Thanks for the review.

  -- Daniel


-- 
 <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/2] ARM: at91: cpuidle: move the driver to drivers/cpuidle directory

[Nicolas Ferre]

On 04/15/2013 03:29 PM, Daniel Lezcano :
> We don't have any dependency with the SoC specific code.
> 
> Move the driver to the drivers/cpuidle directory.
> 
> Add Nicolas Ferre as author of the driver, so it will be in copy of the emails.

Unfortunately, I am not the author of this driver. It is the work of
Albin Tonnerre <albin.tonnerre@free-electrons.com>. So, for sure, I (and
Jean-Christohpe) can babysit this code but I cannot be awarded for it...

You can certainly add to the header of this file:
Maintained by the AT91 crew: Nicolas Ferre, Jean-Christophe
Plagniol-Villard (together with email addresses).



> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  arch/arm/mach-at91/Makefile  |    1 -
>  arch/arm/mach-at91/cpuidle.c |   55 ----------------------------------------
>  drivers/cpuidle/Makefile     |    1 +
>  drivers/cpuidle/at91.c       |   57 ++++++++++++++++++++++++++++++++++++++++++

No, sorry this file name is not appropriate: I prefer cpuidle-at91.c

>  4 files changed, 58 insertions(+), 56 deletions(-)
>  delete mode 100644 arch/arm/mach-at91/cpuidle.c
>  create mode 100644 drivers/cpuidle/at91.c
> 
> diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
> index 39218ca..3c7fca1 100644
> --- a/arch/arm/mach-at91/Makefile
> +++ b/arch/arm/mach-at91/Makefile
> @@ -99,7 +99,6 @@ obj-y				+= leds.o
>  # Power Management
>  obj-$(CONFIG_PM)		+= pm.o
>  obj-$(CONFIG_AT91_SLOW_CLOCK)	+= pm_slowclock.o
> -obj-$(CONFIG_CPU_IDLE)	+= cpuidle.o
>  
>  ifeq ($(CONFIG_PM_DEBUG),y)
>  CFLAGS_pm.o += -DDEBUG
> diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
> deleted file mode 100644
> index b2bec92..0000000
> --- a/arch/arm/mach-at91/cpuidle.c
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -/*
> - * based on arch/arm/mach-kirkwood/cpuidle.c
> - *
> - * CPU idle support for AT91 SoC
> - *
> - * This file is licensed under the terms of the GNU General Public
> - * License version 2.  This program is licensed "as is" without any
> - * warranty of any kind, whether express or implied.
> - *
> - * The cpu idle uses wait-for-interrupt and RAM self refresh in order
> - * to implement two idle states -
> - * #1 wait-for-interrupt
> - * #2 wait-for-interrupt and RAM self refresh
> - */
> -
> -#include <linux/module.h>
> -#include <linux/init.h>
> -#include <linux/cpuidle.h>
> -#include <asm/cpuidle.h>
> -
> -#define AT91_MAX_STATES	2
> -
> -extern void (*at91_standby_ops)(void);
> -
> -/* Actual code that puts the SoC in different idle states */
> -static int at91_enter_idle(struct cpuidle_device *dev,
> -			struct cpuidle_driver *drv,
> -			       int index)
> -{
> -	at91_standby_ops();
> -	return index;
> -}
> -
> -static struct cpuidle_driver at91_idle_driver = {
> -	.name			= "at91_idle",
> -	.owner			= THIS_MODULE,
> -	.states[0]		= ARM_CPUIDLE_WFI_STATE,
> -	.states[1]		= {
> -		.enter			= at91_enter_idle,
> -		.exit_latency		= 10,
> -		.target_residency	= 100000,
> -		.flags			= CPUIDLE_FLAG_TIME_VALID,
> -		.name			= "RAM_SR",
> -		.desc			= "WFI and DDR Self Refresh",
> -	},
> -	.state_count = AT91_MAX_STATES,
> -};
> -
> -/* Initialize CPU idle by registering the idle states */
> -static int __init at91_init_cpuidle(void)
> -{
> -	return cpuidle_register(&at91_idle_driver, NULL);
> -}
> -
> -device_initcall(at91_init_cpuidle);
> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> index 0d8bd55..ba4e977 100644
> --- a/drivers/cpuidle/Makefile
> +++ b/drivers/cpuidle/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
>  
>  obj-$(CONFIG_CPU_IDLE_CALXEDA) += cpuidle-calxeda.o
>  obj-$(CONFIG_ARCH_KIRKWOOD) += cpuidle-kirkwood.o
> +obj-$(CONFIG_ARCH_AT91) += at91.o
> diff --git a/drivers/cpuidle/at91.c b/drivers/cpuidle/at91.c
> new file mode 100644
> index 0000000..2bc745b
> --- /dev/null
> +++ b/drivers/cpuidle/at91.c
> @@ -0,0 +1,57 @@
> +/*
> + * based on arch/arm/mach-kirkwood/cpuidle.c
> + *
> + * CPU idle support for AT91 SoC
> + *
> + * Author: Nicolas Ferre <nicolas.ferre@atmel.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + *
> + * The cpu idle uses wait-for-interrupt and RAM self refresh in order
> + * to implement two idle states -
> + * #1 wait-for-interrupt
> + * #2 wait-for-interrupt and RAM self refresh
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/cpuidle.h>
> +#include <asm/cpuidle.h>
> +
> +#define AT91_MAX_STATES	2
> +
> +extern void (*at91_standby_ops)(void);
> +
> +/* Actual code that puts the SoC in different idle states */
> +static int at91_enter_idle(struct cpuidle_device *dev,
> +			struct cpuidle_driver *drv,
> +			       int index)
> +{
> +	at91_standby_ops();
> +	return index;
> +}
> +
> +static struct cpuidle_driver at91_idle_driver = {
> +	.name			= "at91_idle",
> +	.owner			= THIS_MODULE,
> +	.states[0]		= ARM_CPUIDLE_WFI_STATE,
> +	.states[1]		= {
> +		.enter			= at91_enter_idle,
> +		.exit_latency		= 10,
> +		.target_residency	= 100000,
> +		.flags			= CPUIDLE_FLAG_TIME_VALID,
> +		.name			= "RAM_SR",
> +		.desc			= "WFI and DDR Self Refresh",
> +	},
> +	.state_count = AT91_MAX_STATES,
> +};
> +
> +/* Initialize CPU idle by registering the idle states */
> +static int __init at91_init_cpuidle(void)
> +{
> +	return cpuidle_register(&at91_idle_driver, NULL);
> +}
> +
> +device_initcall(at91_init_cpuidle);
> 


-- 
Nicolas Ferre

[RFC patch 1/2] ARM: at91: cpuidle: encapsulate the standby code

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:29 Mon 15 Apr     , Daniel Lezcano wrote:
> In order to split the pm code from the cpuidle driver, add an ops for the
> standby function which will be initialized by the pm init functions directly,
> thus no need of the SoC specific headers.
> 
> Cleanup also the headers included in this file as they are no longer needed.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  arch/arm/mach-at91/cpuidle.c |   19 ++++---------------
>  arch/arm/mach-at91/pm.c      |    8 +++++++-
>  2 files changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
> index 48f1228..b2bec92 100644
> --- a/arch/arm/mach-at91/cpuidle.c
> +++ b/arch/arm/mach-at91/cpuidle.c
> @@ -13,32 +13,21 @@
>   * #2 wait-for-interrupt and RAM self refresh
>   */
>  
> -#include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <linux/init.h>
> -#include <linux/platform_device.h>
>  #include <linux/cpuidle.h>
> -#include <linux/io.h>
> -#include <linux/export.h>
> -#include <asm/proc-fns.h>
>  #include <asm/cpuidle.h>
> -#include <mach/cpu.h>
> -
> -#include "pm.h"
>  
>  #define AT91_MAX_STATES	2
>  
> +extern void (*at91_standby_ops)(void);
> +
>  /* Actual code that puts the SoC in different idle states */
>  static int at91_enter_idle(struct cpuidle_device *dev,
>  			struct cpuidle_driver *drv,
>  			       int index)
>  {
> -	if (cpu_is_at91rm9200())
> -		at91rm9200_standby();
> -	else if (cpu_is_at91sam9g45())
> -		at91sam9g45_standby();
> -	else
> -		at91sam9_standby();
> -
> +	at91_standby_ops();
>  	return index;
>  }
>  
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 73f1f25..f456e86 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -39,6 +39,8 @@
>  #include "at91_rstc.h"
>  #include "at91_shdwc.h"
>  
> +void (*at91_standby_ops)(void);
> +
>  static void __init show_reset_status(void)
>  {
>  	static char reset[] __initdata = "reset";
> @@ -321,8 +323,12 @@ static int __init at91_pm_init(void)
>  	pr_info("AT91: Power Management%s\n", (slow_clock ? " (with slow clock mode)" : ""));
>  
>  	/* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. */
> -	if (cpu_is_at91rm9200())
> +	if (cpu_is_at91rm9200()) {
>  		at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0);
> +		at91_standby_ops = at91rm9200_standby;
> +	} else if (cpu_is_at91sam9g45())
> +		at91_standby_ops= at91sam9g45_standby;
keep the {} for codying stye rules

> +	else at91_standby_ops = at91sam9_standby;
I'm not a fan of global symbol but ok

Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Best Regards,
J.

[RFC patch 2/2] ARM: at91: cpuidle: move the driver to drivers/cpuidle directory

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:29 Mon 15 Apr     , Daniel Lezcano wrote:
> We don't have any dependency with the SoC specific code.
> 
> Move the driver to the drivers/cpuidle directory.
> 
> Add Nicolas Ferre as author of the driver, so it will be in copy of the emails.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
please use -M when generating the patch
> ---
>  arch/arm/mach-at91/Makefile  |    1 -
>  arch/arm/mach-at91/cpuidle.c |   55 ----------------------------------------
>  drivers/cpuidle/Makefile     |    1 +
>  drivers/cpuidle/at91.c       |   57 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 58 insertions(+), 56 deletions(-)
>  delete mode 100644 arch/arm/mach-at91/cpuidle.c
>  create mode 100644 drivers/cpuidle/at91.c
> 
> diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
> index 39218ca..3c7fca1 100644
> --- a/arch/arm/mach-at91/Makefile
> +++ b/arch/arm/mach-at91/Makefile
> @@ -99,7 +99,6 @@ obj-y				+= leds.o
>  # Power Management
>  obj-$(CONFIG_PM)		+= pm.o
>  obj-$(CONFIG_AT91_SLOW_CLOCK)	+= pm_slowclock.o
> -obj-$(CONFIG_CPU_IDLE)	+= cpuidle.o
>  
>  ifeq ($(CONFIG_PM_DEBUG),y)
>  CFLAGS_pm.o += -DDEBUG
> diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
> deleted file mode 100644
> index b2bec92..0000000
> --- a/arch/arm/mach-at91/cpuidle.c
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -/*
> - * based on arch/arm/mach-kirkwood/cpuidle.c
> - *
> - * CPU idle support for AT91 SoC
> - *
> - * This file is licensed under the terms of the GNU General Public
> - * License version 2.  This program is licensed "as is" without any
> - * warranty of any kind, whether express or implied.
> - *
> - * The cpu idle uses wait-for-interrupt and RAM self refresh in order
> - * to implement two idle states -
> - * #1 wait-for-interrupt
> - * #2 wait-for-interrupt and RAM self refresh
> - */
> -
> -#include <linux/module.h>
> -#include <linux/init.h>
> -#include <linux/cpuidle.h>
> -#include <asm/cpuidle.h>
> -
> -#define AT91_MAX_STATES	2
> -
> -extern void (*at91_standby_ops)(void);
> -
> -/* Actual code that puts the SoC in different idle states */
> -static int at91_enter_idle(struct cpuidle_device *dev,
> -			struct cpuidle_driver *drv,
> -			       int index)
> -{
> -	at91_standby_ops();
> -	return index;
> -}
> -
> -static struct cpuidle_driver at91_idle_driver = {
> -	.name			= "at91_idle",
> -	.owner			= THIS_MODULE,
> -	.states[0]		= ARM_CPUIDLE_WFI_STATE,
> -	.states[1]		= {
> -		.enter			= at91_enter_idle,
> -		.exit_latency		= 10,
> -		.target_residency	= 100000,
> -		.flags			= CPUIDLE_FLAG_TIME_VALID,
> -		.name			= "RAM_SR",
> -		.desc			= "WFI and DDR Self Refresh",
> -	},
> -	.state_count = AT91_MAX_STATES,
> -};
> -
> -/* Initialize CPU idle by registering the idle states */
> -static int __init at91_init_cpuidle(void)
> -{
> -	return cpuidle_register(&at91_idle_driver, NULL);
> -}
> -
> -device_initcall(at91_init_cpuidle);
> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> index 0d8bd55..ba4e977 100644
> --- a/drivers/cpuidle/Makefile
> +++ b/drivers/cpuidle/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
>  
>  obj-$(CONFIG_CPU_IDLE_CALXEDA) += cpuidle-calxeda.o
>  obj-$(CONFIG_ARCH_KIRKWOOD) += cpuidle-kirkwood.o
> +obj-$(CONFIG_ARCH_AT91) += at91.o
> diff --git a/drivers/cpuidle/at91.c b/drivers/cpuidle/at91.c
> new file mode 100644
> index 0000000..2bc745b
> --- /dev/null
> +++ b/drivers/cpuidle/at91.c
> @@ -0,0 +1,57 @@
> +/*
> + * based on arch/arm/mach-kirkwood/cpuidle.c
> + *
> + * CPU idle support for AT91 SoC
> + *
> + * Author: Nicolas Ferre <nicolas.ferre@atmel.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + *
> + * The cpu idle uses wait-for-interrupt and RAM self refresh in order
> + * to implement two idle states -
> + * #1 wait-for-interrupt
> + * #2 wait-for-interrupt and RAM self refresh
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/cpuidle.h>
> +#include <asm/cpuidle.h>
> +
> +#define AT91_MAX_STATES	2
> +
> +extern void (*at91_standby_ops)(void);
really don't like can we pass it via the pm?
> +
> +/* Actual code that puts the SoC in different idle states */
> +static int at91_enter_idle(struct cpuidle_device *dev,
> +			struct cpuidle_driver *drv,
> +			       int index)
> +{
> +	at91_standby_ops();
> +	return index;
> +}
> +
> +static struct cpuidle_driver at91_idle_driver = {
> +	.name			= "at91_idle",
> +	.owner			= THIS_MODULE,
> +	.states[0]		= ARM_CPUIDLE_WFI_STATE,
> +	.states[1]		= {
> +		.enter			= at91_enter_idle,
> +		.exit_latency		= 10,
> +		.target_residency	= 100000,
> +		.flags			= CPUIDLE_FLAG_TIME_VALID,
> +		.name			= "RAM_SR",
> +		.desc			= "WFI and DDR Self Refresh",
> +	},
> +	.state_count = AT91_MAX_STATES,
> +};
> +
> +/* Initialize CPU idle by registering the idle states */
> +static int __init at91_init_cpuidle(void)
> +{
> +	return cpuidle_register(&at91_idle_driver, NULL);
> +}
> +
> +device_initcall(at91_init_cpuidle);
> -- 
> 1.7.9.5
> 

[RFC patch 2/2] ARM: at91: cpuidle: move the driver to drivers/cpuidle directory

[Daniel Lezcano]

On 04/15/2013 04:14 PM, Nicolas Ferre wrote:
> On 04/15/2013 03:29 PM, Daniel Lezcano :
>> We don't have any dependency with the SoC specific code.
>>
>> Move the driver to the drivers/cpuidle directory.
>>
>> Add Nicolas Ferre as author of the driver, so it will be in copy of the emails.
> 
> Unfortunately, I am not the author of this driver. It is the work of
> Albin Tonnerre <albin.tonnerre@free-electrons.com>. So, for sure, I (and
> Jean-Christohpe) can babysit this code but I cannot be awarded for it...
> You can certainly add to the header of this file:
> Maintained by the AT91 crew: Nicolas Ferre, Jean-Christophe
> Plagniol-Villard (together with email addresses).

Ok, will do.

>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  arch/arm/mach-at91/Makefile  |    1 -
>>  arch/arm/mach-at91/cpuidle.c |   55 ----------------------------------------
>>  drivers/cpuidle/Makefile     |    1 +
>>  drivers/cpuidle/at91.c       |   57 ++++++++++++++++++++++++++++++++++++++++++
> 
> No, sorry this file name is not appropriate: I prefer cpuidle-at91.c

Sure ? :)

It is a bit redundant drivers/cpuidle/cpuidle-at91.c, no ? and if all
the cpuidle drivers are there, the directory content will look:

cpuidle/cpuidle-tegra114.c
cpuidle/cpuidle-tegra20.c
cpuidle/cpuidle-tegra30.c
cpuidle/cpuidle-imx5.c
cpuidle/cpuidle-imx6.c
cpuidle/cpuidle-kirkwood.c
cpuidle/cpuidle-calxeda.c
cpuidle/cpuidle-shmobile.c
cpuidle/cpuidle-intel-idle.c
cpuidle/cpuidle-at91.c
cpuidle/cpuidle-omap34xx.c
cpuidle/cpuidle-omap44xx.c
cpuidle/cpuidle-ux500.c
cpuidle/cpuidle-exynos4.c
cpuidle/cpuidle-acpi.c
cpuidle/cpuidle-pseries.c
cpuidle/cpuidle-sh.c
cpuidle/cpuidle-s3c64xx.c
cpuidle/cpuidle-davinci.c

etc ...

But I won't argue if you really want cpuidle-at91.c, it is a detail IMO.

Thanks
  -- Daniel

-- 
 <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/2] ARM: at91: cpuidle: move the driver to drivers/cpuidle directory

[Daniel Lezcano]

On 04/15/2013 04:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 15:29 Mon 15 Apr     , Daniel Lezcano wrote:
>> We don't have any dependency with the SoC specific code.
>>
>> Move the driver to the drivers/cpuidle directory.
>>
>> Add Nicolas Ferre as author of the driver, so it will be in copy of the emails.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> please use -M when generating the patch

Oh, right. Thanks for reminding me the option.

[ ... ]

>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/cpuidle.h>
>> +#include <asm/cpuidle.h>
>> +
>> +#define AT91_MAX_STATES	2
>> +
>> +extern void (*at91_standby_ops)(void);
> really don't like can we pass it via the pm?

I agree, it is hackish. Can you elaborate when you say "pass it via the
pm" ?



-- 
 <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/2] ARM: at91: cpuidle: move the driver to drivers/cpuidle directory

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:37 Mon 15 Apr     , Daniel Lezcano wrote:
> On 04/15/2013 04:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 15:29 Mon 15 Apr     , Daniel Lezcano wrote:
> >> We don't have any dependency with the SoC specific code.
> >>
> >> Move the driver to the drivers/cpuidle directory.
> >>
> >> Add Nicolas Ferre as author of the driver, so it will be in copy of the emails.
> >>
> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > please use -M when generating the patch
> 
> Oh, right. Thanks for reminding me the option.
> 
> [ ... ]
> 
> >> +#include <linux/module.h>
> >> +#include <linux/init.h>
> >> +#include <linux/cpuidle.h>
> >> +#include <asm/cpuidle.h>
> >> +
> >> +#define AT91_MAX_STATES	2
> >> +
> >> +extern void (*at91_standby_ops)(void);
> > really don't like can we pass it via the pm?
> 
> I agree, it is hackish. Can you elaborate when you say "pass it via the
> pm" ?

I was thinking about it

I would even prefer to have a platfrom driver that pass it via platform data
even in DT

but not something that can touch globally

and for drivers I prefer to do not use cpu_is only for the core

Best Regards,
J.
> 
> 
> 
> -- 
>  <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/2] ARM: at91: cpuidle: move the driver to drivers/cpuidle directory

[Daniel Lezcano]

On 04/15/2013 06:00 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 16:37 Mon 15 Apr     , Daniel Lezcano wrote:
>> On 04/15/2013 04:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 15:29 Mon 15 Apr     , Daniel Lezcano wrote:
>>>> We don't have any dependency with the SoC specific code.
>>>>
>>>> Move the driver to the drivers/cpuidle directory.
>>>>
>>>> Add Nicolas Ferre as author of the driver, so it will be in copy of the emails.
>>>>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> please use -M when generating the patch
>>
>> Oh, right. Thanks for reminding me the option.
>>
>> [ ... ]
>>
>>>> +#include <linux/module.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/cpuidle.h>
>>>> +#include <asm/cpuidle.h>
>>>> +
>>>> +#define AT91_MAX_STATES	2
>>>> +
>>>> +extern void (*at91_standby_ops)(void);
>>> really don't like can we pass it via the pm?
>>
>> I agree, it is hackish. Can you elaborate when you say "pass it via the
>> pm" ?
> 
> I was thinking about it
> 
> I would even prefer to have a platfrom driver that pass it via platform data
> even in DT
> 
> but not something that can touch globally
> 
> and for drivers I prefer to do not use cpu_is only for the core

Is the following code what you had in mind ?

Please, be lenient, it is the first time I write something based on
'platform' :)


Index: cpuidle-next/arch/arm/mach-at91/cpuidle.c
===================================================================
--- cpuidle-next.orig/arch/arm/mach-at91/cpuidle.c      2013-04-16
10:40:21.058315042 +0200
+++ cpuidle-next/arch/arm/mach-at91/cpuidle.c   2013-04-17
00:35:42.836314931 +0200
@@ -13,32 +13,22 @@
  * #2 wait-for-interrupt and RAM self refresh
  */

-#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/init.h>
-#include <linux/platform_device.h>
 #include <linux/cpuidle.h>
-#include <linux/io.h>
-#include <linux/export.h>
-#include <asm/proc-fns.h>
+#include <linux/platform_device.h>
 #include <asm/cpuidle.h>
-#include <mach/cpu.h>
-
-#include "pm.h"

 #define AT91_MAX_STATES        2

+static void (*at91_standby_ops)(void);
+
 /* Actual code that puts the SoC in different idle states */
 static int at91_enter_idle(struct cpuidle_device *dev,
                        struct cpuidle_driver *drv,
                               int index)
 {
-       if (cpu_is_at91rm9200())
-               at91rm9200_standby();
-       else if (cpu_is_at91sam9g45())
-               at91sam9g45_standby();
-       else
-               at91sam9_standby();
-
+       at91_standby_ops();
        return index;
 }

@@ -58,9 +48,16 @@ static struct cpuidle_driver at91_idle_d
 };

 /* Initialize CPU idle by registering the idle states */
-static int __init at91_init_cpuidle(void)
+int __init at91_init_cpuidle(struct platform_device *pdev)
 {
-       return cpuidle_register(&at91_idle_driver, NULL);
+       at91_standby_ops = pdata;
+       return 0;
 }

-device_initcall(at91_init_cpuidle);
+static int __init at91_cpuidle_init(void)
+{
+
+
+       return cpuidle_register(&at91_idle_driver, NULL);
+}
+device_initcall(at91_cpuidle_init);
Index: cpuidle-next/arch/arm/mach-at91/pm.c
===================================================================
--- cpuidle-next.orig/arch/arm/mach-at91/pm.c   2013-04-16
09:52:33.048426618 +0200
+++ cpuidle-next/arch/arm/mach-at91/pm.c        2013-04-16
18:22:03.207420375 +0200
@@ -312,21 +312,40 @@ static const struct platform_suspend_ops
        .end    = at91_pm_end,
 };

-static int __init at91_pm_init(void)
+static int __init at91_pm_probe(struct platform_device *pdev)
 {
-#ifdef CONFIG_AT91_SLOW_CLOCK
-       slow_clock = (void *) (AT91_IO_VIRT_BASE - at91_slow_clock_sz);
-#endif
-
-       pr_info("AT91: Power Management%s\n", (slow_clock ? " (with slow
clock mode)" : ""));
-
        /* AT91RM9200 SDRAM low-power mode cannot be used with
self-refresh. */
-       if (cpu_is_at91rm9200())
+       if (of_machine_is_compatible("atmel,at91rm9200")) {
                at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0);
+               pdev->dev.platform_data = at91rm9200_standby;
+       } else if (of_machine_is_compatible("atmel,atmel,at91sam9g45")) {
+               pdev->dev.platform_data = at91sam9g45_standby;
+       } else {
+               pdev->dev.platform_data = at91sam9_standby;
+       }

        suspend_set_ops(&at91_pm_ops);

        show_reset_status();
+
        return 0;
 }
+
+static struct platform_driver at91_pm_driver = {
+       .driver = {
+               .name    = "pm-at91",
+               .owner   = THIS_MODULE,
+       },
+};
+
+static int __init at91_pm_init(void)
+{
+#ifdef CONFIG_AT91_SLOW_CLOCK
+       slow_clock = (void *) (AT91_IO_VIRT_BASE - at91_slow_clock_sz);
+#endif
+       pr_info("AT91: Power Management%s\n",
+               (slow_clock ? " (with slow clock mode)" : ""));
+
+       return platform_driver_probe(&at91_pm_driver, at91_pm_probe);
+}
 arch_initcall(at91_pm_init);
Index: cpuidle-next/arch/arm/mach-at91/board-usb-a926x.c
===================================================================
--- cpuidle-next.orig/arch/arm/mach-at91/board-usb-a926x.c
2013-04-15 10:31:45.503051053 +0200
+++ cpuidle-next/arch/arm/mach-at91/board-usb-a926x.c   2013-04-17
00:35:38.480314758 +0200
@@ -49,7 +49,6 @@
 #include "sam9_smc.h"
 #include "generic.h"

-
 static void __init ek_init_early(void)
 {
        /* Initialize processor: 12.00 MHz crystal */
@@ -319,6 +318,19 @@ static void __init ek_add_device_leds(vo
        at91_gpio_leds(ek_leds, ARRAY_SIZE(ek_leds));
 }

+#ifdef CONFIG_PM
+static struct platform_device at91_pm_device = {
+       .name = "pm-at91",
+       .id = -1,
+};
+#endif
+
+static struct platform_device *platform_devices[] __initdata = {
+#ifdef CONFIG_PM
+       &at91_pm_device,
+#endif
+}
+
 static void __init ek_board_init(void)
 {
        /* Serial */
@@ -351,6 +363,9 @@ static void __init ek_board_init(void)
                                | AT91_SHDW_WKMODE0_LOW
                                | AT91_SHDW_RTTWKEN);
        }
+
+       /* Platform devices */
+       platform_add_devices(&platform_devices,
ARRAY_SIZE(platform_devices));
 }

 MACHINE_START(USB_A9263, "CALAO USB_A9263")


if (of_machine_is_compatible("atmel,at91rm9200")) {
	printk("at91rm9200\n");
} else if (of_machine_is_compatible("atmel,atmel,at91sam9g45")) {
	printk(at91sam9g45\n");
} else {
	printk(other at91 cpu\n");
}

> 
> Best Regards,
> J.
>>
>>
>>
>> -- 
>>  <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
>>


-- 
 <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/2] ARM: at91: cpuidle: move the driver to drivers/cpuidle directory

[Daniel Lezcano]

On 04/17/2013 12:39 AM, Daniel Lezcano wrote:
> On 04/15/2013 06:00 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 16:37 Mon 15 Apr     , Daniel Lezcano wrote:
>>> On 04/15/2013 04:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>> On 15:29 Mon 15 Apr     , Daniel Lezcano wrote:
>>>>> We don't have any dependency with the SoC specific code.
>>>>>
>>>>> Move the driver to the drivers/cpuidle directory.
>>>>>
>>>>> Add Nicolas Ferre as author of the driver, so it will be in copy of the emails.
>>>>>
>>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> please use -M when generating the patch
>>>
>>> Oh, right. Thanks for reminding me the option.
>>>
>>> [ ... ]
>>>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/init.h>
>>>>> +#include <linux/cpuidle.h>
>>>>> +#include <asm/cpuidle.h>
>>>>> +
>>>>> +#define AT91_MAX_STATES	2
>>>>> +
>>>>> +extern void (*at91_standby_ops)(void);
>>>> really don't like can we pass it via the pm?
>>>
>>> I agree, it is hackish. Can you elaborate when you say "pass it via the
>>> pm" ?
>>
>> I was thinking about it
>>
>> I would even prefer to have a platfrom driver that pass it via platform data
>> even in DT
>>
>> but not something that can touch globally
>>
>> and for drivers I prefer to do not use cpu_is only for the core
> 
> Is the following code what you had in mind ?

Hi Jean-Christophe,

coming back to move the cpuidle driver to drivers/cpuidle.

I was wondering if the code below would be ok for you ?

Thanks
  -- Daniel


> Please, be lenient, it is the first time I write something based on
> 'platform' :)
> 
> 
> Index: cpuidle-next/arch/arm/mach-at91/cpuidle.c
> ===================================================================
> --- cpuidle-next.orig/arch/arm/mach-at91/cpuidle.c      2013-04-16
> 10:40:21.058315042 +0200
> +++ cpuidle-next/arch/arm/mach-at91/cpuidle.c   2013-04-17
> 00:35:42.836314931 +0200
> @@ -13,32 +13,22 @@
>   * #2 wait-for-interrupt and RAM self refresh
>   */
> 
> -#include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <linux/init.h>
> -#include <linux/platform_device.h>
>  #include <linux/cpuidle.h>
> -#include <linux/io.h>
> -#include <linux/export.h>
> -#include <asm/proc-fns.h>
> +#include <linux/platform_device.h>
>  #include <asm/cpuidle.h>
> -#include <mach/cpu.h>
> -
> -#include "pm.h"
> 
>  #define AT91_MAX_STATES        2
> 
> +static void (*at91_standby_ops)(void);
> +
>  /* Actual code that puts the SoC in different idle states */
>  static int at91_enter_idle(struct cpuidle_device *dev,
>                         struct cpuidle_driver *drv,
>                                int index)
>  {
> -       if (cpu_is_at91rm9200())
> -               at91rm9200_standby();
> -       else if (cpu_is_at91sam9g45())
> -               at91sam9g45_standby();
> -       else
> -               at91sam9_standby();
> -
> +       at91_standby_ops();
>         return index;
>  }
> 
> @@ -58,9 +48,16 @@ static struct cpuidle_driver at91_idle_d
>  };
> 
>  /* Initialize CPU idle by registering the idle states */
> -static int __init at91_init_cpuidle(void)
> +int __init at91_init_cpuidle(struct platform_device *pdev)
>  {
> -       return cpuidle_register(&at91_idle_driver, NULL);
> +       at91_standby_ops = pdata;
> +       return 0;
>  }
> 
> -device_initcall(at91_init_cpuidle);
> +static int __init at91_cpuidle_init(void)
> +{
> +
> +
> +       return cpuidle_register(&at91_idle_driver, NULL);
> +}
> +device_initcall(at91_cpuidle_init);
> Index: cpuidle-next/arch/arm/mach-at91/pm.c
> ===================================================================
> --- cpuidle-next.orig/arch/arm/mach-at91/pm.c   2013-04-16
> 09:52:33.048426618 +0200
> +++ cpuidle-next/arch/arm/mach-at91/pm.c        2013-04-16
> 18:22:03.207420375 +0200
> @@ -312,21 +312,40 @@ static const struct platform_suspend_ops
>         .end    = at91_pm_end,
>  };
> 
> -static int __init at91_pm_init(void)
> +static int __init at91_pm_probe(struct platform_device *pdev)
>  {
> -#ifdef CONFIG_AT91_SLOW_CLOCK
> -       slow_clock = (void *) (AT91_IO_VIRT_BASE - at91_slow_clock_sz);
> -#endif
> -
> -       pr_info("AT91: Power Management%s\n", (slow_clock ? " (with slow
> clock mode)" : ""));
> -
>         /* AT91RM9200 SDRAM low-power mode cannot be used with
> self-refresh. */
> -       if (cpu_is_at91rm9200())
> +       if (of_machine_is_compatible("atmel,at91rm9200")) {
>                 at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0);
> +               pdev->dev.platform_data = at91rm9200_standby;
> +       } else if (of_machine_is_compatible("atmel,atmel,at91sam9g45")) {
> +               pdev->dev.platform_data = at91sam9g45_standby;
> +       } else {
> +               pdev->dev.platform_data = at91sam9_standby;
> +       }
> 
>         suspend_set_ops(&at91_pm_ops);
> 
>         show_reset_status();
> +
>         return 0;
>  }
> +
> +static struct platform_driver at91_pm_driver = {
> +       .driver = {
> +               .name    = "pm-at91",
> +               .owner   = THIS_MODULE,
> +       },
> +};
> +
> +static int __init at91_pm_init(void)
> +{
> +#ifdef CONFIG_AT91_SLOW_CLOCK
> +       slow_clock = (void *) (AT91_IO_VIRT_BASE - at91_slow_clock_sz);
> +#endif
> +       pr_info("AT91: Power Management%s\n",
> +               (slow_clock ? " (with slow clock mode)" : ""));
> +
> +       return platform_driver_probe(&at91_pm_driver, at91_pm_probe);
> +}
>  arch_initcall(at91_pm_init);
> Index: cpuidle-next/arch/arm/mach-at91/board-usb-a926x.c
> ===================================================================
> --- cpuidle-next.orig/arch/arm/mach-at91/board-usb-a926x.c
> 2013-04-15 10:31:45.503051053 +0200
> +++ cpuidle-next/arch/arm/mach-at91/board-usb-a926x.c   2013-04-17
> 00:35:38.480314758 +0200
> @@ -49,7 +49,6 @@
>  #include "sam9_smc.h"
>  #include "generic.h"
> 
> -
>  static void __init ek_init_early(void)
>  {
>         /* Initialize processor: 12.00 MHz crystal */
> @@ -319,6 +318,19 @@ static void __init ek_add_device_leds(vo
>         at91_gpio_leds(ek_leds, ARRAY_SIZE(ek_leds));
>  }
> 
> +#ifdef CONFIG_PM
> +static struct platform_device at91_pm_device = {
> +       .name = "pm-at91",
> +       .id = -1,
> +};
> +#endif
> +
> +static struct platform_device *platform_devices[] __initdata = {
> +#ifdef CONFIG_PM
> +       &at91_pm_device,
> +#endif
> +}
> +
>  static void __init ek_board_init(void)
>  {
>         /* Serial */
> @@ -351,6 +363,9 @@ static void __init ek_board_init(void)
>                                 | AT91_SHDW_WKMODE0_LOW
>                                 | AT91_SHDW_RTTWKEN);
>         }
> +
> +       /* Platform devices */
> +       platform_add_devices(&platform_devices,
> ARRAY_SIZE(platform_devices));
>  }
> 
>  MACHINE_START(USB_A9263, "CALAO USB_A9263")
> 
> 
> if (of_machine_is_compatible("atmel,at91rm9200")) {
> 	printk("at91rm9200\n");
> } else if (of_machine_is_compatible("atmel,atmel,at91sam9g45")) {
> 	printk(at91sam9g45\n");
> } else {
> 	printk(other at91 cpu\n");
> }
> 
>>
>> Best Regards,
>> J.
>>>
>>>
>>>
>>> -- 
>>>  <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
>>>
> 
> 


-- 
 <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