Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757946Ab3FCODl (ORCPT ); Mon, 3 Jun 2013 10:03:41 -0400 Received: from mail-bk0-f54.google.com ([209.85.214.54]:59121 "EHLO mail-bk0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755377Ab3FCODi (ORCPT ); Mon, 3 Jun 2013 10:03:38 -0400 Message-ID: <51ACA237.8070004@linaro.org> Date: Mon, 03 Jun 2013 16:03:35 +0200 From: Daniel Lezcano User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 MIME-Version: 1.0 To: Michal Simek CC: linux-kernel@vger.kernel.org, Michal Simek , Arnd Bergmann , linux-arm-kernel@lists.infradead.org, Josh Cartwright , Steffen Trumtrar , Rob Herring , Peter Crosthwaite , "Rafael J. Wysocki" , linux-pm@vger.kernel.org Subject: Re: [PATCH v3] arm: zynq: Add cpuidle support References: <2ba502c172f5374ed2e424292fb50345c9dd007a.1370266815.git.michal.simek@xilinx.com> In-Reply-To: <2ba502c172f5374ed2e424292fb50345c9dd007a.1370266815.git.michal.simek@xilinx.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5530 Lines: 205 On 06/03/2013 03:40 PM, Michal Simek wrote: > Add support for cpuidle. > > Signed-off-by: Michal Simek > --- > 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 > +#include > +#include > +#include > +#include ^^^^^ > +#include ^^^^^ > +#include ^^^^^ > +#include > +#include ^^^^ > +#include > +#include > +#include > +#include > + > +#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. > + return 0; > +} > +module_init(zynq_init_cpuidle); Do you really want it as module ? Thanks -- Daniel -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/