diff mbox

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

Message ID 1366032598-23053-3-git-send-email-daniel.lezcano@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Lezcano April 15, 2013, 1:29 p.m. UTC
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

Comments

Nicolas Ferre April 15, 2013, 2:14 p.m. UTC | #1
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);
>
Jean-Christophe PLAGNIOL-VILLARD April 15, 2013, 2:20 p.m. UTC | #2
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
>
Daniel Lezcano April 15, 2013, 2:29 p.m. UTC | #3
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
Daniel Lezcano April 15, 2013, 2:37 p.m. UTC | #4
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" ?
Jean-Christophe PLAGNIOL-VILLARD April 15, 2013, 4 p.m. UTC | #5
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
>
Daniel Lezcano April 16, 2013, 10:39 p.m. UTC | #6
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
>>
Daniel Lezcano June 21, 2013, 10:51 a.m. UTC | #7
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
>>>
> 
>
diff mbox

Patch

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