All of lore.kernel.org
 help / color / mirror / Atom feed
From: daniel.lezcano@linaro.org (Daniel Lezcano)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 10/10] arm: zynq: Add cpuidle support
Date: Wed, 27 Mar 2013 11:07:16 +0100	[thread overview]
Message-ID: <5152C4D4.2020303@linaro.org> (raw)
In-Reply-To: <3395757de41837ecc1d84aac1a06ebcc23fe1367.1364319776.git.michal.simek@xilinx.com>

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.

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

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


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

  parent reply	other threads:[~2013-03-27 10:07 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-26 17:43 Zynq core changes v2 Michal Simek
2013-03-26 17:43 ` [PATCH v2 01/10] arm: zynq: Use standard timer binding Michal Simek
2013-03-26 20:14   ` Steffen Trumtrar
2013-03-27  7:40     ` Michal Simek
2013-03-27  9:04       ` Steffen Trumtrar
2013-03-27  9:25         ` Michal Simek
2013-03-27  9:37           ` Steffen Trumtrar
2013-03-27  9:54             ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 02/10] arm: zynq: Move timer to clocksource interface Michal Simek
2013-03-26 17:43 ` [PATCH v2 03/10] arm: zynq: Move timer to generic location Michal Simek
2013-03-26 17:43 ` [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time Michal Simek
2013-03-26 21:44   ` Arnd Bergmann
2013-03-27  7:49     ` Michal Simek
2013-03-27  9:29       ` Arnd Bergmann
2013-03-27  9:37         ` Michal Simek
2013-03-27 10:00           ` Arnd Bergmann
2013-03-27 10:09             ` Michal Simek
2013-03-27 10:43               ` Steffen Trumtrar
2013-03-27 10:49                 ` Michal Simek
2013-03-27 10:45               ` Arnd Bergmann
2013-03-27 10:47                 ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file Michal Simek
2013-03-26 21:43   ` Arnd Bergmann
2013-03-27  6:55     ` Steffen Trumtrar
2013-03-27  9:31       ` Arnd Bergmann
2013-03-27  9:41         ` Steffen Trumtrar
2013-03-27 14:15           ` Michal Simek
2013-03-27 13:01   ` Philip Balister
2013-03-27 13:31     ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 06/10] arm: zynq: Add support for system reset Michal Simek
2013-03-26 17:43 ` [PATCH v2 07/10] arm: zynq: Add support for pmu Michal Simek
2013-03-26 17:43 ` [PATCH v2 08/10] arm: zynq: Add smp support Michal Simek
2013-03-27  8:59   ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 09/10] arm: zynq: Add hotplug support Michal Simek
2013-03-26 17:43 ` [PATCH v2 10/10] arm: zynq: Add cpuidle support Michal Simek
2013-03-26 21:46   ` Arnd Bergmann
2013-03-27  7:56     ` Michal Simek
2013-03-27  9:27       ` Arnd Bergmann
2013-03-27 10:15         ` Daniel Lezcano
2013-03-27 10:07   ` Daniel Lezcano [this message]
2013-03-27 10:31     ` Michal Simek
2013-03-27 10:37       ` Daniel Lezcano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5152C4D4.2020303@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.