[PATCH v2 10/10] arm: zynq: Add cpuidle support

[daniel.lezcano@linaro.org (Daniel Lezcano)]

On 03/27/2013 11:31 AM, Michal Simek wrote:
> Hi Daniel,
> 
> 2013/3/27 Daniel Lezcano <daniel.lezcano@linaro.org>:
>> On 03/26/2013 06:43 PM, Michal Simek wrote:
>>> Add support for cpuidle.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>
>> 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.
> 
> I have found this some mins ago.
> 
> I means that currently it is not zero.
> 
> zynq> cat /sys/devices/system/cpu/cpu0/cpuidle/state1/usage
> 96931
> zynq> cat /sys/devices/system/cpu/cpu0/cpuidle/state0/usage
> 8396
> 
>> 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 ?
> 
> Let me talk to Soren how this should be done on zynq.
> 
> 
>> The comments below applies for this driver based on linux-pm-next.
>>
>> https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
> 
> Appreciate that - the origin author of this code is not longer with Xilinx
> that's why hard to tell why it was done in this way. This version
> was done 2011-02. But that's not excuse just explanation.
> 
> It mean that this driver should also go through this tree, right?
> 
> 
>>
>>> ---
>>> 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 <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 <asm/proc-fns.h>
>>> +
>>> +#define XILINX_MAX_STATES    1
>>
>> You defined XILINX_MAX_STATES as 1, it should be 2.
> 
> yep. Done.
> 
>>
>>> +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.
> 
> Where is that caller function? I can trace it but this maybe faster way. :-)

In the idle loop arch/arm/kernel/process.c


>>> +     do_gettimeofday(&before);
>>
>> Remove this line, that will be taken into account by the driver's
>> en_core_tk_irqen flag.
> 
> ok
> 
>>
>>> +
>>> +     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.
> 
> ok
> 
>>
>>> +     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.
> 
> ok
> 
>>
>>> +             /* 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.
> 
> ok
> 
>>
>>> +     }
>>> +
>>> +     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.
> 
> ok
> 
>>
>>> +     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,
>> };
>>
> 
> ok
> 
>>
>>> +/* 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.
> 
> ok. Thanks you very much for your comments.
> Michal
> 


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