From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel.lezcano@linaro.org (Daniel Lezcano) Date: Wed, 27 Mar 2013 11:07:16 +0100 Subject: [PATCH v2 10/10] arm: zynq: Add cpuidle support In-Reply-To: <3395757de41837ecc1d84aac1a06ebcc23fe1367.1364319776.git.michal.simek@xilinx.com> References: <1364319822-5504-1-git-send-email-michal.simek@xilinx.com> <3395757de41837ecc1d84aac1a06ebcc23fe1367.1364319776.git.michal.simek@xilinx.com> Message-ID: <5152C4D4.2020303@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 03/26/2013 06:43 PM, Michal Simek wrote: > Add support for cpuidle. > > Signed-off-by: Michal Simek Hi Michal, at the first glance, the driver won't enter in the self refresh state. If I am right, by checking: /sys/devices/system/cpu/cpu0/cpuidle/state1/usage, it should be always zero. Also, I see there is self refresh but you save the cpu context, so it is cpu shutdown no ? In this case, 3 states would be needed, WFI, self-refresh, powerdown, no ? The comments below applies for this driver based on linux-pm-next. https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/ > --- > v2: Fix file header > --- > arch/arm/mach-zynq/Makefile | 1 + > arch/arm/mach-zynq/cpuidle.c | 133 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 134 insertions(+) > create mode 100644 arch/arm/mach-zynq/cpuidle.c > > diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile > index 1b25d92..238b8f9 100644 > --- a/arch/arm/mach-zynq/Makefile > +++ b/arch/arm/mach-zynq/Makefile > @@ -7,4 +7,5 @@ obj-y := common.o slcr.o > CFLAGS_REMOVE_hotplug.o =-march=armv6k > CFLAGS_hotplug.o =-Wa,-march=armv7-a -mcpu=cortex-a9 > obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o > +obj-$(CONFIG_CPU_IDLE) += cpuidle.o > obj-$(CONFIG_SMP) += headsmp.o platsmp.o > diff --git a/arch/arm/mach-zynq/cpuidle.c b/arch/arm/mach-zynq/cpuidle.c > new file mode 100644 > index 0000000..4ed8855 > --- /dev/null > +++ b/arch/arm/mach-zynq/cpuidle.c > @@ -0,0 +1,133 @@ > +/* > + * Copyright (C) 2012-2013 Xilinx > + * > + * CPU idle support for Xilinx > + * > + * 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 > + * > + * Note that this code is only intended as a prototype and is not tested > + * well yet, or tuned for the Xilinx Cortex A9. Also note that for a > + * tickless kernel, high res timers must not be turned on. The cpuidle > + * framework must also be turned on in the kernel. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define XILINX_MAX_STATES 1 You defined XILINX_MAX_STATES as 1, it should be 2. > +static DEFINE_PER_CPU(struct cpuidle_device, xilinx_cpuidle_device); > + > +/* Actual code that puts the SoC in different idle states */ > +static int xilinx_enter_idle(struct cpuidle_device *dev, > + struct cpuidle_driver *drv, int index) > +{ > + struct timeval before, after; > + int idle_time; > + > + local_irq_disable(); Remove this line, it is done by the caller. > + do_gettimeofday(&before); Remove this line, that will be taken into account by the driver's en_core_tk_irqen flag. > + > + if (index == 0) > + /* Wait for interrupt state */ > + cpu_do_idle(); Don't check the index, see in the driver initialization comment below the defaul wfi state. You will have one state entering WFI and the xilinx_enter_idle function entering with always index 1. > + else if (index == 1) { > + unsigned int cpu_id = smp_processor_id(); > + > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id); Remove this line, CPUIDLE_FLAG_TIMER_STOP will tell the cpuidle framework to handle that. > + /* Devices must be stopped here */ > + cpu_pm_enter(); > + > + /* Add code for DDR self refresh start */ > + > + cpu_do_idle(); > + /*cpu_suspend(foo, bar);*/ > + > + /* Add code for DDR self refresh stop */ > + > + cpu_pm_exit(); > + > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id); Remove this line, CPUIDLE_FLAG_TIMER_STOP will tell the cpuidle framework to handle that. > + } > + > + do_gettimeofday(&after); > + local_irq_enable(); > + idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC + > + (after.tv_usec - before.tv_usec); > + > + dev->last_residency = idle_time; Remove these lines about time computation, that is handled by the caller. > + return index; > +} > + > +static struct cpuidle_driver xilinx_idle_driver = { > + .name = "xilinx_idle", > + .owner = THIS_MODULE, > + .state_count = XILINX_MAX_STATES, > + /* Wait for interrupt state */ > + .states[0] = { > + .enter = xilinx_enter_idle, > + .exit_latency = 1, > + .target_residency = 10000, > + .flags = CPUIDLE_FLAG_TIME_VALID, > + .name = "WFI", > + .desc = "Wait for interrupt", > + }, > + /* Wait for interrupt and RAM self refresh state */ > + .states[1] = { > + .enter = xilinx_enter_idle, > + .exit_latency = 10, > + .target_residency = 10000, > + .flags = CPUIDLE_FLAG_TIME_VALID, > + .name = "RAM_SR", > + .desc = "WFI and RAM Self Refresh", > + }, > +}; static struct cpuidle_driver xilinx_idle_driver = { .name = "xilinx_idle", .owner = THIS_MODULE, .states = { ARM_CPUIDLE_WFI_STATE, { .enter = xilinx_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 = XILINX_MAX_STATES, }; > +/* Initialize CPU idle by registering the idle states */ > +static int xilinx_init_cpuidle(void) > +{ > + unsigned int cpu; > + struct cpuidle_device *device; > + int ret; > + > + ret = cpuidle_register_driver(&xilinx_idle_driver); > + if (ret) { > + pr_err("Registering Xilinx CpuIdle Driver failed.\n"); > + return ret; > + } > + > + for_each_possible_cpu(cpu) { > + device = &per_cpu(xilinx_cpuidle_device, cpu); > + device->state_count = XILINX_MAX_STATES; This is initialization is done from the cpuidle_register_device. As drv->state_count == device->state_count. > + device->cpu = cpu; > + ret = cpuidle_register_device(device); > + if (ret) { > + pr_err("xilinx_init_cpuidle: Failed registering\n"); > + return ret; > + } > + } > + > + pr_info("Xilinx CpuIdle Driver started\n"); > + return 0; > +} > +device_initcall(xilinx_init_cpuidle); > -- Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog