Re: [PATCH 3/3] ARM: EXYNOS: move cpuidle driver to drivers/cpuidle/

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 06/26/2013 12:13 PM, Bartlomiej Zolnierkiewicz wrote:
> While at it:
> - remove "common.h" include
> - remove wrong comment about file path
> 
> Tested on Exynos4210 (Universal C210 board).

I agree to move the driver to the drivers/cpuidle, but the header
#include <mach/regs-clock.h> shouldn't appear in this file.

There were several attempts to move the cpuidle drivers to this
directory [1]  and we agree to separate the pm code from the driver
code. A bit of code encapsulation should be done here before.

Thanks
  -- Daniel

ps : "use -M to generate the patch" when moving files from one directory
to another one.

[1] https://patchwork.kernel.org/patch/2445191/

> Cc: Jaecheol Lee <jc.lee@samsung.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  arch/arm/mach-exynos/Makefile    |  1 -
>  arch/arm/mach-exynos/cpuidle.c   | 95 ----------------------------------------
>  drivers/cpuidle/Makefile         |  3 ++
>  drivers/cpuidle/cpuidle-exynos.c | 92 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 95 insertions(+), 96 deletions(-)
>  delete mode 100644 arch/arm/mach-exynos/cpuidle.c
>  create mode 100644 drivers/cpuidle/cpuidle-exynos.c
> 
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index b09b027..0f91c7a 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -16,7 +16,6 @@ obj-$(CONFIG_ARCH_EXYNOS)	+= common.o
>  
>  obj-$(CONFIG_PM)		+= pm.o
>  obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
> -obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
>  
>  obj-$(CONFIG_ARCH_EXYNOS)	+= pmu.o
>  
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> deleted file mode 100644
> index d6e5115..0000000
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ /dev/null
> @@ -1,95 +0,0 @@
> -/* linux/arch/arm/mach-exynos4/cpuidle.c
> - *
> - * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> - *		http://www.samsung.com
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> -*/
> -
> -#include <linux/kernel.h>
> -#include <linux/init.h>
> -#include <linux/cpuidle.h>
> -#include <linux/cpu_pm.h>
> -#include <linux/io.h>
> -#include <linux/export.h>
> -#include <linux/time.h>
> -
> -#include <asm/proc-fns.h>
> -#include <asm/cpuidle.h>
> -#include <mach/regs-clock.h>
> -
> -#include "common.h"
> -
> -static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device);
> -
> -static struct cpuidle_driver exynos4_idle_driver = {
> -	.name			= "exynos4_idle",
> -	.owner			= THIS_MODULE,
> -	.states = {
> -		[0] = ARM_CPUIDLE_WFI_STATE,
> -	},
> -	.state_count = 1,
> -	.safe_state_index = 0,
> -};
> -
> -static void __init exynos5_core_down_clk(void)
> -{
> -	unsigned int tmp;
> -
> -	/*
> -	 * Enable arm clock down (in idle) and set arm divider
> -	 * ratios in WFI/WFE state.
> -	 */
> -	tmp = PWR_CTRL1_CORE2_DOWN_RATIO | \
> -	      PWR_CTRL1_CORE1_DOWN_RATIO | \
> -	      PWR_CTRL1_DIV2_DOWN_EN	 | \
> -	      PWR_CTRL1_DIV1_DOWN_EN	 | \
> -	      PWR_CTRL1_USE_CORE1_WFE	 | \
> -	      PWR_CTRL1_USE_CORE0_WFE	 | \
> -	      PWR_CTRL1_USE_CORE1_WFI	 | \
> -	      PWR_CTRL1_USE_CORE0_WFI;
> -	__raw_writel(tmp, EXYNOS5_PWR_CTRL1);
> -
> -	/*
> -	 * Enable arm clock up (on exiting idle). Set arm divider
> -	 * ratios when not in idle along with the standby duration
> -	 * ratios.
> -	 */
> -	tmp = PWR_CTRL2_DIV2_UP_EN	 | \
> -	      PWR_CTRL2_DIV1_UP_EN	 | \
> -	      PWR_CTRL2_DUR_STANDBY2_VAL | \
> -	      PWR_CTRL2_DUR_STANDBY1_VAL | \
> -	      PWR_CTRL2_CORE2_UP_RATIO	 | \
> -	      PWR_CTRL2_CORE1_UP_RATIO;
> -	__raw_writel(tmp, EXYNOS5_PWR_CTRL2);
> -}
> -
> -int __init exynos4_init_cpuidle(void)
> -{
> -	int cpu_id, ret;
> -	struct cpuidle_device *device;
> -
> -	if (soc_is_exynos5250())
> -		exynos5_core_down_clk();
> -
> -	ret = cpuidle_register_driver(&exynos4_idle_driver);
> -	if (ret) {
> -		printk(KERN_ERR "CPUidle failed to register driver\n");
> -		return ret;
> -	}
> -
> -	for_each_online_cpu(cpu_id) {
> -		device = &per_cpu(exynos4_cpuidle_device, cpu_id);
> -		device->cpu = cpu_id;
> -
> -		ret = cpuidle_register_device(device);
> -		if (ret) {
> -			printk(KERN_ERR "CPUidle register device failed\n");
> -			return ret;
> -		}
> -	}
> -
> -	return 0;
> -}
> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> index 0d8bd55..37c6178 100644
> --- a/drivers/cpuidle/Makefile
> +++ b/drivers/cpuidle/Makefile
> @@ -6,4 +6,7 @@ obj-y += cpuidle.o driver.o governor.o sysfs.o governors/
>  obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
>  
>  obj-$(CONFIG_CPU_IDLE_CALXEDA) += cpuidle-calxeda.o
> +ifeq ($(CONFIG_ARCH_EXYNOS),y)
> +	obj-y += cpuidle-exynos.o
> +endif
>  obj-$(CONFIG_ARCH_KIRKWOOD) += cpuidle-kirkwood.o
> diff --git a/drivers/cpuidle/cpuidle-exynos.c b/drivers/cpuidle/cpuidle-exynos.c
> new file mode 100644
> index 0000000..1c592d2
> --- /dev/null
> +++ b/drivers/cpuidle/cpuidle-exynos.c
> @@ -0,0 +1,92 @@
> +/*
> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> +*/
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/io.h>
> +#include <linux/export.h>
> +#include <linux/time.h>
> +
> +#include <asm/proc-fns.h>
> +#include <asm/cpuidle.h>
> +#include <mach/regs-clock.h>
> +
> +static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device);
> +
> +static struct cpuidle_driver exynos4_idle_driver = {
> +	.name			= "exynos4_idle",
> +	.owner			= THIS_MODULE,
> +	.states = {
> +		[0] = ARM_CPUIDLE_WFI_STATE,
> +	},
> +	.state_count = 1,
> +	.safe_state_index = 0,
> +};
> +
> +static void __init exynos5_core_down_clk(void)
> +{
> +	unsigned int tmp;
> +
> +	/*
> +	 * Enable arm clock down (in idle) and set arm divider
> +	 * ratios in WFI/WFE state.
> +	 */
> +	tmp = PWR_CTRL1_CORE2_DOWN_RATIO | \
> +	      PWR_CTRL1_CORE1_DOWN_RATIO | \
> +	      PWR_CTRL1_DIV2_DOWN_EN	 | \
> +	      PWR_CTRL1_DIV1_DOWN_EN	 | \
> +	      PWR_CTRL1_USE_CORE1_WFE	 | \
> +	      PWR_CTRL1_USE_CORE0_WFE	 | \
> +	      PWR_CTRL1_USE_CORE1_WFI	 | \
> +	      PWR_CTRL1_USE_CORE0_WFI;
> +	__raw_writel(tmp, EXYNOS5_PWR_CTRL1);
> +
> +	/*
> +	 * Enable arm clock up (on exiting idle). Set arm divider
> +	 * ratios when not in idle along with the standby duration
> +	 * ratios.
> +	 */
> +	tmp = PWR_CTRL2_DIV2_UP_EN	 | \
> +	      PWR_CTRL2_DIV1_UP_EN	 | \
> +	      PWR_CTRL2_DUR_STANDBY2_VAL | \
> +	      PWR_CTRL2_DUR_STANDBY1_VAL | \
> +	      PWR_CTRL2_CORE2_UP_RATIO	 | \
> +	      PWR_CTRL2_CORE1_UP_RATIO;
> +	__raw_writel(tmp, EXYNOS5_PWR_CTRL2);
> +}
> +
> +int __init exynos4_init_cpuidle(void)
> +{
> +	int cpu_id, ret;
> +	struct cpuidle_device *device;
> +
> +	if (soc_is_exynos5250())
> +		exynos5_core_down_clk();
> +
> +	ret = cpuidle_register_driver(&exynos4_idle_driver);
> +	if (ret) {
> +		printk(KERN_ERR "CPUidle failed to register driver\n");
> +		return ret;
> +	}
> +
> +	for_each_online_cpu(cpu_id) {
> +		device = &per_cpu(exynos4_cpuidle_device, cpu_id);
> +		device->cpu = cpu_id;
> +
> +		ret = cpuidle_register_device(device);
> +		if (ret) {
> +			printk(KERN_ERR "CPUidle register device failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> 


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