| Message ID | 1366032598-23053-3-git-send-email-daniel.lezcano@linaro.org (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
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); >
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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" ?
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >>
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 --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);
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