On 06/03/2013 04:03 PM, Daniel Lezcano wrote: > On 06/03/2013 03:40 PM, Michal Simek wrote: >> Add support for cpuidle. >> >> Signed-off-by: Michal Simek <[email protected]> >> --- >> Changes in v3: >> - Move driver to drivers/cpuidle/ >> - Check zynq compatible string suggested by Arnd >> - Use zynq_ function prefix because of multiplatform kernel >> - Incorporate comments from Daniel Lezcano >> - Rebase on the top of >> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next >> >> Changes in v2: >> - Fix file header >> >> Changes in v1: >> - It was the part of Zynq core changes >> https://patchwork.kernel.org/patch/2342511/ >> >> drivers/cpuidle/Kconfig | 6 +++ >> drivers/cpuidle/Makefile | 1 + >> drivers/cpuidle/cpuidle-zynq.c | 100 >> +++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 107 insertions(+) >> create mode 100644 drivers/cpuidle/cpuidle-zynq.c >> >> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig >> index c4cc27e..8272a08 100644 >> --- a/drivers/cpuidle/Kconfig >> +++ b/drivers/cpuidle/Kconfig >> @@ -39,4 +39,10 @@ config CPU_IDLE_CALXEDA >> help >> Select this to enable cpuidle on Calxeda processors. >> >> +config CPU_IDLE_ZYNQ >> + bool "CPU Idle Driver for Xilinx Zynq processors" >> + depends on ARCH_ZYNQ >> + help >> + Select this to enable cpuidle on Xilinx Zynq processors. >> + >> endif >> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile >> index 0d8bd55..8767a7b 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_CPU_IDLE_ZYNQ) += cpuidle-zynq.o >> diff --git a/drivers/cpuidle/cpuidle-zynq.c b/drivers/cpuidle/cpuidle-zynq.c >> new file mode 100644 >> index 0000000..afe6af3 >> --- /dev/null >> +++ b/drivers/cpuidle/cpuidle-zynq.c >> @@ -0,0 +1,100 @@ >> +/* >> + * Copyright (C) 2012-2013 Xilinx >> + * >> + * CPU idle support for Xilinx Zynq >> + * >> + * based on arch/arm/mach-at91/cpuidle.c >> + * >> + * 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 >> + */ > > Please check you headers, it seems you are including unused headers. > >> +#include <linux/kernel.h> >> +#include <linux/init.h> >> +#include <linux/platform_device.h> >> +#include <linux/cpuidle.h> >> +#include <linux/io.h> > > ^^^^^ > >> +#include <linux/export.h> > > ^^^^^ > >> +#include <linux/clockchips.h> > > ^^^^^ > >> +#include <linux/cpu_pm.h> >> +#include <linux/clk.h> > > ^^^^ > >> +#include <linux/err.h> >> +#include <linux/of.h> >> +#include <asm/proc-fns.h> >> +#include <asm/cpuidle.h> >> + >> +#define ZYNQ_MAX_STATES 2 >> + >> +static DEFINE_PER_CPU(struct cpuidle_device, zynq_cpuidle_device); > > see below. > >> + >> +/* Actual code that puts the SoC in different idle states */ >> +static int zynq_enter_idle(struct cpuidle_device *dev, >> + struct cpuidle_driver *drv, int index) > > Indentation. > >> +{ >> + /* Devices must be stopped here */ >> + cpu_pm_enter(); >> + >> + /* Add code for DDR self refresh start */ >> + cpu_do_idle(); >> + >> + /* Add code for DDR self refresh stop */ >> + cpu_pm_exit(); >> + >> + return index; >> +} >> + >> +static struct cpuidle_driver zynq_idle_driver = { >> + .name = "zynq_idle", >> + .owner = THIS_MODULE, >> + .states = { >> + ARM_CPUIDLE_WFI_STATE, >> + { >> + .enter = zynq_enter_idle, >> + .exit_latency = 10, >> + .target_residency = 10000, >> + .flags = CPUIDLE_FLAG_TIME_VALID, >> + CPUIDLE_FLAG_TIMER_STOP, >> + .name = "RAM_SR", >> + .desc = "WFI and RAM Self Refresh", >> + }, >> + }, >> + .safe_state_index = 0, >> + .state_count = ZYNQ_MAX_STATES, >> +}; >> + >> +/* Initialize CPU idle by registering the idle states */ >> +static int zynq_init_cpuidle(void) >> +{ >> + unsigned int cpu; >> + struct cpuidle_device *device; >> + int ret; >> + >> + if (!of_machine_is_compatible("xlnx,zynq-7000")) >> + return -ENODEV; >> + >> + ret = cpuidle_register_driver(&zynq_idle_driver); >> + if (ret) { >> + pr_err("%s: Driver registration failed\n", __func__); >> + return ret; >> + } >> + >> + for_each_possible_cpu(cpu) { >> + device = &per_cpu(zynq_cpuidle_device, cpu); >> + device->cpu = cpu; >> + ret = cpuidle_register_device(device); >> + if (ret) { >> + pr_err("%s: Device registration failed\n", __func__); >> + return ret; >> + } >> + } >> + >> + pr_info("Xilinx Zynq CpuIdle Driver started\n"); > > Hi Michal, > > actually you can replace this code by > > cpuidle_register(&zynq_idle_driver, NULL); > > and remove the zynq_cpuidle_device. > > The framework will take care of registering the driver and the devices.
Will change. >> + return 0; >> +} >> +module_init(zynq_init_cpuidle); > > Do you really want it as module ? kirkwood is a module but in Makefile depends directly on ARCH obj-$(CONFIG_ARCH_KIRKWOOD) += cpuidle-kirkwood.o Calxeda uses module_init() which is in this configuration device_initcall. Any advantage to write it as module? Maybe for testing purpose will be helpful to have it as module too. What do you think? BTW: I will also add entry to MAINTAINERS file and list myself in this header. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
signature.asc
Description: OpenPGP digital signature

