From: Amit Daniel Kachhap amit.daniel@samsung.com

Now with common clock support added for exynos5250 it is necessary to move this code to exynos5250 common clock driver as clock registers should be handled there. This change is tested in exynos5250 based arndale platform.

Cc: Abhilash Kesavan a.kesavan@samsung.com Cc: Thomas Abraham thomas.abraham@linaro.org Acked-by: Kukjin Kim kgene.kim@samsugn.com Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com [t.figa: Rebased onto current kernel sources.] Signed-off-by: Tomasz Figa t.figa@samsung.com Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org --- arch/arm/mach-exynos/cpuidle.c | 54 ---------------------------------- drivers/clk/samsung/clk-exynos5250.c | 42 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 54 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c index b530231..3e260ba 100644 --- a/arch/arm/mach-exynos/cpuidle.c +++ b/arch/arm/mach-exynos/cpuidle.c @@ -40,25 +40,6 @@

#define S5P_CHECK_AFTR 0xFCBA0D10

-#define EXYNOS5_PWR_CTRL1 (S5P_VA_CMU + 0x01020) -#define EXYNOS5_PWR_CTRL2 (S5P_VA_CMU + 0x01024) - -#define PWR_CTRL1_CORE2_DOWN_RATIO (7 << 28) -#define PWR_CTRL1_CORE1_DOWN_RATIO (7 << 16) -#define PWR_CTRL1_DIV2_DOWN_EN (1 << 9) -#define PWR_CTRL1_DIV1_DOWN_EN (1 << 8) -#define PWR_CTRL1_USE_CORE1_WFE (1 << 5) -#define PWR_CTRL1_USE_CORE0_WFE (1 << 4) -#define PWR_CTRL1_USE_CORE1_WFI (1 << 1) -#define PWR_CTRL1_USE_CORE0_WFI (1 << 0) - -#define PWR_CTRL2_DIV2_UP_EN (1 << 25) -#define PWR_CTRL2_DIV1_UP_EN (1 << 24) -#define PWR_CTRL2_DUR_STANDBY2_VAL (1 << 16) -#define PWR_CTRL2_DUR_STANDBY1_VAL (1 << 8) -#define PWR_CTRL2_CORE2_UP_RATIO (1 << 4) -#define PWR_CTRL2_CORE1_UP_RATIO (1 << 0) - static int exynos4_enter_lowpower(struct cpuidle_device *dev, struct cpuidle_driver *drv, int index); @@ -181,46 +162,11 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev, return exynos4_enter_core0_aftr(dev, drv, new_index); }

-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); -} - static int exynos_cpuidle_probe(struct platform_device *pdev) { int cpu_id, ret; struct cpuidle_device *device;

- if (soc_is_exynos5250()) - exynos5_core_down_clk(); - if (soc_is_exynos5440()) exynos4_idle_driver.state_count = 1;

diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c index e7ee442..2bb4625 100644 --- a/drivers/clk/samsung/clk-exynos5250.c +++ b/drivers/clk/samsung/clk-exynos5250.c @@ -24,6 +24,8 @@ #define APLL_CON0 0x100 #define SRC_CPU 0x200 #define DIV_CPU0 0x500 +#define PWR_CTRL1 0x1020 +#define PWR_CTRL2 0x1024 #define MPLL_LOCK 0x4000 #define MPLL_CON0 0x4100 #define SRC_CORE1 0x4204 @@ -80,6 +82,23 @@ #define SRC_CDREX 0x20200 #define PLL_DIV2_SEL 0x20a24

+/*Below definitions are used for PWR_CTRL settings*/ +#define PWR_CTRL1_CORE2_DOWN_RATIO (7 << 28) +#define PWR_CTRL1_CORE1_DOWN_RATIO (7 << 16) +#define PWR_CTRL1_DIV2_DOWN_EN (1 << 9) +#define PWR_CTRL1_DIV1_DOWN_EN (1 << 8) +#define PWR_CTRL1_USE_CORE1_WFE (1 << 5) +#define PWR_CTRL1_USE_CORE0_WFE (1 << 4) +#define PWR_CTRL1_USE_CORE1_WFI (1 << 1) +#define PWR_CTRL1_USE_CORE0_WFI (1 << 0) + +#define PWR_CTRL2_DIV2_UP_EN (1 << 25) +#define PWR_CTRL2_DIV1_UP_EN (1 << 24) +#define PWR_CTRL2_DUR_STANDBY2_VAL (1 << 16) +#define PWR_CTRL2_DUR_STANDBY1_VAL (1 << 8) +#define PWR_CTRL2_CORE2_UP_RATIO (1 << 4) +#define PWR_CTRL2_CORE1_UP_RATIO (1 << 0) + /* list of PLLs to be registered */ enum exynos5250_plls { apll, mpll, cpll, epll, vpll, gpll, bpll, @@ -98,6 +117,8 @@ static struct samsung_clk_reg_dump *exynos5250_save; static unsigned long exynos5250_clk_regs[] __initdata = { SRC_CPU, DIV_CPU0, + PWR_CTRL1, + PWR_CTRL2, SRC_CORE1, SRC_TOP0, SRC_TOP2, @@ -686,6 +707,7 @@ static struct of_device_id ext_clk_match[] __initdata = { /* register exynox5250 clocks */ static void __init exynos5250_clk_init(struct device_node *np) { + unsigned int tmp; if (np) { reg_base = of_iomap(np, 0); if (!reg_base) @@ -722,6 +744,26 @@ static void __init exynos5250_clk_init(struct device_node *np) samsung_clk_register_gate(exynos5250_gate_clks, ARRAY_SIZE(exynos5250_gate_clks));

+ /* + * 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, reg_base + 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, reg_base + PWR_CTRL2); + exynos5250_clk_sleep_init();

pr_info("Exynos5250: clock setup completed, armclk=%ld\n",

Use the cpuidle generic function 'cpuidle_register'. That saves us from some extra lines of code and unneeded variables.

A side effect of this change is a bug fix where before the cpuidle driver was registered for each_online_cpu and now it is for each_possible_cpu.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Reviewed-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com Reviewed-by: Tomasz Figa t.figa@samsung.com --- arch/arm/mach-exynos/cpuidle.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c index 17df0d8..61e1481 100644 --- a/arch/arm/mach-exynos/cpuidle.c +++ b/arch/arm/mach-exynos/cpuidle.c @@ -40,8 +40,6 @@

#define S5P_CHECK_AFTR 0xFCBA0D10

-static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device); - /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */ static void exynos4_set_wakeupmask(void) { @@ -160,29 +158,17 @@ static struct cpuidle_driver exynos4_idle_driver = {

static int exynos_cpuidle_probe(struct platform_device *pdev) { - int cpu_id, ret; - struct cpuidle_device *device; + int ret;

if (soc_is_exynos5440()) exynos4_idle_driver.state_count = 1;

- ret = cpuidle_register_driver(&exynos4_idle_driver); + ret = cpuidle_register(&exynos4_idle_driver, NULL); if (ret) { dev_err(&pdev->dev, "failed to register cpuidle 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) { - dev_err(&pdev->dev, "failed to register cpuidle device\n"); - return ret; - } - } - return 0; }

That makes the code cleaner and encapsulted. The function will be reused in the next patch.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Reviewed-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com Reviewed-by: Tomasz Figa t.figa@samsung.com --- arch/arm/mach-exynos/pm.c | 65 ++++++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c index 15af0ce..adfdf4b 100644 --- a/arch/arm/mach-exynos/pm.c +++ b/arch/arm/mach-exynos/pm.c @@ -103,6 +103,42 @@ static int exynos_irq_set_wake(struct irq_data *data, unsigned int state) /* For Cortex-A9 Diagnostic and Power control register */ static unsigned int save_arm_register[2];

+static void exynos_cpu_save_register(void) +{ + unsigned long tmp; + + /* Save Power control register */ + asm ("mrc p15, 0, %0, c15, c0, 0" + : "=r" (tmp) : : "cc"); + + save_arm_register[0] = tmp; + + /* Save Diagnostic register */ + asm ("mrc p15, 0, %0, c15, c0, 1" + : "=r" (tmp) : : "cc"); + + save_arm_register[1] = tmp; +} + +static void exynos_cpu_restore_register(void) +{ + unsigned long tmp; + + /* Restore Power control register */ + tmp = save_arm_register[0]; + + asm volatile ("mcr p15, 0, %0, c15, c0, 0" + : : "r" (tmp) + : "cc"); + + /* Restore Diagnostic register */ + tmp = save_arm_register[1]; + + asm volatile ("mcr p15, 0, %0, c15, c0, 1" + : : "r" (tmp) + : "cc"); +} + static int exynos_cpu_suspend(unsigned long arg) { #ifdef CONFIG_CACHE_L2X0 @@ -162,17 +198,8 @@ static int exynos_pm_suspend(void) tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0); __raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);

- if (!soc_is_exynos5250()) { - /* Save Power control register */ - asm ("mrc p15, 0, %0, c15, c0, 0" - : "=r" (tmp) : : "cc"); - save_arm_register[0] = tmp; - - /* Save Diagnostic register */ - asm ("mrc p15, 0, %0, c15, c0, 1" - : "=r" (tmp) : : "cc"); - save_arm_register[1] = tmp; - } + if (!soc_is_exynos5250()) + exynos_cpu_save_register();

return 0; } @@ -196,19 +223,9 @@ static void exynos_pm_resume(void) /* No need to perform below restore code */ goto early_wakeup; } - if (!soc_is_exynos5250()) { - /* Restore Power control register */ - tmp = save_arm_register[0]; - asm volatile ("mcr p15, 0, %0, c15, c0, 0" - : : "r" (tmp) - : "cc"); - - /* Restore Diagnostic register */ - tmp = save_arm_register[1]; - asm volatile ("mcr p15, 0, %0, c15, c0, 1" - : : "r" (tmp) - : "cc"); - } + + if (!soc_is_exynos5250()) + exynos_cpu_restore_register();

/* For release retention */

This function should be called only when the powerdown sequence fails.

Even if the current code does not hurt, by moving this line, we have the same code than the one in pm.c.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Reviewed-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com Reviewed-by: Tomasz Figa t.figa@samsung.com --- arch/arm/mach-exynos/cpuidle.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c index 4d8bcfd..346c2d0 100644 --- a/arch/arm/mach-exynos/cpuidle.c +++ b/arch/arm/mach-exynos/cpuidle.c @@ -115,11 +115,10 @@ static int exynos_enter_core0_aftr(struct cpuidle_device *dev, if (!(tmp & S5P_CENTRAL_LOWPWR_CFG)) { tmp |= S5P_CENTRAL_LOWPWR_CFG; __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION); + /* Clear wakeup state register */ + __raw_writel(0x0, S5P_WAKEUP_STAT); }

- /* Clear wakeup state register */ - __raw_writel(0x0, S5P_WAKEUP_STAT); - return index; }

We make the cpuidle code less arch dependent.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Reviewed-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com Reviewed-by: Tomasz Figa t.figa@samsung.com --- arch/arm/mach-exynos/cpuidle.c | 6 ------ arch/arm/mach-exynos/pm.c | 4 ++++ 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c index aa8ab1b..95826c6 100644 --- a/arch/arm/mach-exynos/cpuidle.c +++ b/arch/arm/mach-exynos/cpuidle.c @@ -18,7 +18,6 @@ #include <linux/platform_device.h>

#include <asm/proc-fns.h> -#include <asm/smp_scu.h> #include <asm/suspend.h> #include <asm/unified.h> #include <asm/cpuidle.h> @@ -74,11 +73,6 @@ static int exynos_enter_core0_aftr(struct cpuidle_device *dev,

cpu_pm_enter(); cpu_suspend(0, idle_finisher); - -#ifdef CONFIG_SMP - if (!soc_is_exynos5250()) - scu_enable(S5P_VA_SCU); -#endif cpu_pm_exit();

/* diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c index 67d75fe..aba577f 100644 --- a/arch/arm/mach-exynos/pm.c +++ b/arch/arm/mach-exynos/pm.c @@ -336,6 +336,10 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,

case CPU_PM_EXIT: if (cpu == 0) { +#ifdef CONFIG_SMP + if (!soc_is_exynos5250()) + scu_enable(S5P_VA_SCU); +#endif exynos_cpu_restore_register(); } break;

Pass the wakeup mask to 'exynos_set_wakeupmask' as this function could be used for different idle states with different mask.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org --- arch/arm/mach-exynos/cpuidle.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c index 95826c6..169db74 100644 --- a/arch/arm/mach-exynos/cpuidle.c +++ b/arch/arm/mach-exynos/cpuidle.c @@ -40,14 +40,14 @@ #define S5P_CHECK_AFTR 0xFCBA0D10

/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */ -static void exynos_set_wakeupmask(void) +static void exynos_set_wakeupmask(long mask) { - __raw_writel(0x0000ff3e, S5P_WAKEUP_MASK); + __raw_writel(mask, S5P_WAKEUP_MASK); }

static int idle_finisher(unsigned long flags) { - exynos_set_wakeupmask(); + exynos_set_wakeupmask(0x0000ff3e);

__raw_writel(virt_to_phys(s3c_cpu_resume), REG_DIRECTGO_ADDR); __raw_writel(S5P_CHECK_AFTR, REG_DIRECTGO_FLAG);

There is no point to register the cpuidle driver for the 5440 as it has only one WFI state which is the default idle function when the cpuidle driver is disabled.

By disabling cpuidle we prevent to enter to the governor computation for nothing, thus saving a lot of processing time.

The only drawback is the statistic via sysfs on this state which is lost but it is meaningless and it could be retrieved from the ftrace easily.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Reviewed-by: Tomasz Figa t.figa@samsung.com Acked-by: Amit Kucheria amit.kucheria@linaro.org --- arch/arm/mach-exynos/cpuidle.c | 3 --- arch/arm/mach-exynos/exynos.c | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c index f66ee4d..95313ea 100644 --- a/arch/arm/mach-exynos/cpuidle.c +++ b/arch/arm/mach-exynos/cpuidle.c @@ -133,9 +133,6 @@ static int exynos_cpuidle_probe(struct platform_device *pdev) { int ret;

- if (soc_is_exynos5440()) - exynos_idle_driver.state_count = 1; - ret = cpuidle_register(&exynos_idle_driver, NULL); if (ret) { dev_err(&pdev->dev, "failed to register cpuidle driver\n"); diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c index b567361..fe8dac8 100644 --- a/arch/arm/mach-exynos/exynos.c +++ b/arch/arm/mach-exynos/exynos.c @@ -227,6 +227,9 @@ static struct platform_device exynos_cpuidle = {

void __init exynos_cpuidle_init(void) { + if (soc_is_exynos5440()) + return; + platform_device_register(&exynos_cpuidle); }

In order to remove depedency on pm code, let's move the 'exynos_enter_aftr' function into the pm.c file as well as the other helper functions.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org --- arch/arm/mach-exynos/common.h | 1 + arch/arm/mach-exynos/cpuidle.c | 29 ----------------------------- arch/arm/mach-exynos/pm.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h index 9ef3f83..30123a0 100644 --- a/arch/arm/mach-exynos/common.h +++ b/arch/arm/mach-exynos/common.h @@ -62,5 +62,6 @@ struct exynos_pmu_conf { };

extern void exynos_sys_powerdown_conf(enum sys_powerdown mode); +extern void exynos_enter_aftr(void);

#endif /* __ARCH_ARM_MACH_EXYNOS_COMMON_H */ diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c index fe35fba..e6d813d 100644 --- a/arch/arm/mach-exynos/cpuidle.c +++ b/arch/arm/mach-exynos/cpuidle.c @@ -30,35 +30,6 @@ #include "common.h" #include "regs-pmu.h"

-#define REG_DIRECTGO_ADDR (samsung_rev() == EXYNOS4210_REV_1_1 ? \ - S5P_INFORM7 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \ - (S5P_VA_SYSRAM + 0x24) : S5P_INFORM0)) -#define REG_DIRECTGO_FLAG (samsung_rev() == EXYNOS4210_REV_1_1 ? \ - S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \ - (S5P_VA_SYSRAM + 0x20) : S5P_INFORM1)) - -#define S5P_CHECK_AFTR 0xFCBA0D10 - -/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */ -static void exynos_set_wakeupmask(long mask) -{ - __raw_writel(mask, S5P_WAKEUP_MASK); -} - -static void exynos_cpu_set_boot_vector(long flags) -{ - __raw_writel(virt_to_phys(exynos_cpu_resume), REG_DIRECTGO_ADDR); - __raw_writel(flags, REG_DIRECTGO_FLAG); -} - -static void exynos_enter_aftr(void) -{ - exynos_set_wakeupmask(0x0000ff3e); - exynos_cpu_set_boot_vector(S5P_CHECK_AFTR); - /* Set value of power down register for aftr mode */ - exynos_sys_powerdown_conf(SYS_AFTR); -} - static int idle_finisher(unsigned long flags) { exynos_enter_aftr(); diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c index 9773a00..50b6b4d 100644 --- a/arch/arm/mach-exynos/pm.c +++ b/arch/arm/mach-exynos/pm.c @@ -101,6 +101,35 @@ static int exynos_irq_set_wake(struct irq_data *data, unsigned int state) return -ENOENT; }

+#define EXYNOS_BOOT_VECTOR_ADDR (samsung_rev() == EXYNOS4210_REV_1_1 ? \ + S5P_INFORM7 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \ + (S5P_VA_SYSRAM + 0x24) : S5P_INFORM0)) +#define EXYNOS_BOOT_VECTOR_FLAG (samsung_rev() == EXYNOS4210_REV_1_1 ? \ + S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \ + (S5P_VA_SYSRAM + 0x20) : S5P_INFORM1)) + +#define S5P_CHECK_AFTR 0xFCBA0D10 + +/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */ +static void exynos_set_wakeupmask(long mask) +{ + __raw_writel(mask, S5P_WAKEUP_MASK); +} + +static void exynos_cpu_set_boot_vector(long flags) +{ + __raw_writel(virt_to_phys(exynos_cpu_resume), EXYNOS_BOOT_VECTOR_ADDR); + __raw_writel(flags, EXYNOS_BOOT_VECTOR_FLAG); +} + +void exynos_enter_aftr(void) +{ + exynos_set_wakeupmask(0x0000ff3e); + exynos_cpu_set_boot_vector(S5P_CHECK_AFTR); + /* Set value of power down register for aftr mode */ + exynos_sys_powerdown_conf(SYS_AFTR); +} + /* For Cortex-A9 Diagnostic and Power control register */ static unsigned int save_arm_register[2];

This macro is only used there.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org --- arch/arm/mach-exynos/pm.c | 3 ++- arch/arm/mach-exynos/regs-pmu.h | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c index 6d9ef69..b380d48 100644 --- a/arch/arm/mach-exynos/pm.c +++ b/arch/arm/mach-exynos/pm.c @@ -108,7 +108,8 @@ static int exynos_irq_set_wake(struct irq_data *data, unsigned int state) S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \ (S5P_VA_SYSRAM + 0x20) : S5P_INFORM1))

-#define S5P_CHECK_AFTR 0xFCBA0D10 +#define S5P_CHECK_AFTR 0xFCBA0D10 +#define S5P_CHECK_SLEEP 0x00000BAD

/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */ static void exynos_set_wakeupmask(long mask) diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h index 4f6a256..6c1d2db 100644 --- a/arch/arm/mach-exynos/regs-pmu.h +++ b/arch/arm/mach-exynos/regs-pmu.h @@ -119,8 +119,6 @@ #define S5P_CORE_LOCAL_PWR_EN 0x3 #define S5P_INT_LOCAL_PWR_EN 0x7

-#define S5P_CHECK_SLEEP 0x00000BAD - /* Only for EXYNOS4210 */ #define S5P_CMU_CLKSTOP_LCD1_LOWPWR S5P_PMUREG(0x1154) #define S5P_CMU_RESET_LCD1_LOWPWR S5P_PMUREG(0x1174)

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org --- arch/arm/mach-exynos/cpuidle.c | 10 ---------- 1 file changed, 10 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c index 1d1222e..1d09ebd 100644 --- a/arch/arm/mach-exynos/cpuidle.c +++ b/arch/arm/mach-exynos/cpuidle.c @@ -8,25 +8,15 @@ * 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 <linux/platform_device.h>

#include <asm/proc-fns.h> #include <asm/suspend.h> -#include <asm/unified.h> #include <asm/cpuidle.h>

-#include <plat/cpu.h> -#include <plat/pm.h> - -#include <mach/map.h> - static void (*exynos_enter_aftr)(void);

static int idle_finisher(unsigned long flags)

Hi Arnd,

On 09.05.2014 12:56, Arnd Bergmann wrote:

On Friday 11 April 2014, Daniel Lezcano wrote:

...

No more dependency on the arch code. The platform_data field is used to set the PM callback as the other cpuidle drivers.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Reviewed-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com

This has just shown up in linux-next and broken randconfig builds.

...

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c index fe8dac8..d22f0e4 100644 --- a/arch/arm/mach-exynos/exynos.c +++ b/arch/arm/mach-exynos/exynos.c @@ -221,8 +221,9 @@ void exynos_restart(enum reboot_mode mode, const char *cmd) } static struct platform_device exynos_cpuidle = {

•   .name           = "exynos_cpuidle",
  

•   .id             = -1,
  

•   .name              = "exynos_cpuidle",
  

•   .dev.platform_data = exynos_enter_aftr,
  

•   .id                = -1,
  

};

This is wrong on many levels, can we please do this properly?

• The exynos_enter_aftr function is compiled conditionally, so you can't just reference it from generic code, or you get a link error.

+1

• 'static struct platform_device ...' has been deprecated for at least a decade, stop doing that. For any platform devices that get registered, there is platform_device_register_simple().

+0.5

The missing 0.5 is because you can't pass platform data using platform_device_register_simple(). There is platform_device_register_resndata(), though.

• There shouldn't need to be a platform_device to start with, this should all come from DT. We can't do this on arm64 anyway, so any code that may be shared between arm32 and arm64 should have proper abstractions.

-1

Exynos cpuidle is not a device on the SoC, so I don't think there is any way to represent it in DT. The only thing I could see this is matching root node with a central SoC driver that instantiates specific subdevices, such as cpufreq and cpuidle, but I don't see any available infrastructure for this.

Best regards, Tomasz

Hi,

On Friday, May 09, 2014 02:02:14 PM Tomasz Figa wrote:

Hi Arnd,

On 09.05.2014 12:56, Arnd Bergmann wrote:

...

On Friday 11 April 2014, Daniel Lezcano wrote:

...

No more dependency on the arch code. The platform_data field is used to set the PM callback as the other cpuidle drivers.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Reviewed-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com

This has just shown up in linux-next and broken randconfig builds.

Could you please give some more details about these issues?

If this is a build breakage for CONFIG_PM_SLEEP=n && CONFIG_CPU_IDLE=y configuration then this is not a new problem introduced by the current patchset (please see below).

...

...

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c index fe8dac8..d22f0e4 100644 --- a/arch/arm/mach-exynos/exynos.c +++ b/arch/arm/mach-exynos/exynos.c @@ -221,8 +221,9 @@ void exynos_restart(enum reboot_mode mode, const char *cmd) } static struct platform_device exynos_cpuidle = {

•   .name           = "exynos_cpuidle",
  

•   .id             = -1,
  

•   .name              = "exynos_cpuidle",
  

•   .dev.platform_data = exynos_enter_aftr,
  

•   .id                = -1,
  

};

This is wrong on many levels, can we please do this properly?

• The exynos_enter_aftr function is compiled conditionally, so you can't just reference it from generic code, or you get a link error.

+1

Even without Daniel's patchset we are getting a link error for CONFIG_PM_SLEEP=n && CONFIG_CPU_IDLE=y configuration.

In arch/arm/mach-exynos/Makefile we have:

... obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o ... obj-$(CONFIG_CPU_IDLE) += cpuidle.o ...

and since cpuidle.c is referencing exynos_cpu_resume() from sleep.S the build results in:

arch/arm/kernel/return_address.c:63:2: warning: #warning "TODO: return_address should use unwind tables" [-Wcpp] arch/arm/mach-exynos/built-in.o: In function `exynos4_enter_core0_aftr': /home/bzolnier/linux-sprc/arch/arm/mach-exynos/cpuidle.c:141: undefined reference to `cpu_suspend' arch/arm/mach-exynos/built-in.o: In function `exynos4_enter_lowpower': /home/bzolnier/linux-sprc/arch/arm/mach-exynos/cpuidle.c:176: undefined reference to `exynos_cpu_resume' make: *** [vmlinux] Error 1

linkage errors.

[ The other error for the current code is for cpu_suspend() which is also (indirectly) dependent on CONFIG_PM_SLEEP. ]

...

• 'static struct platform_device ...' has been deprecated for at least a decade, stop doing that. For any platform devices that get registered, there is platform_device_register_simple().

+0.5

The missing 0.5 is because you can't pass platform data using platform_device_register_simple(). There is platform_device_register_resndata(), though.

Agreed but this can be fixed trivially (even in in the incremental patch).

...

• There shouldn't need to be a platform_device to start with, this should all come from DT. We can't do this on arm64 anyway, so any code that may be shared between arm32 and arm64 should have proper abstractions.

-1

Exynos cpuidle is not a device on the SoC, so I don't think there is any way to represent it in DT. The only thing I could see this is matching root node with a central SoC driver that instantiates specific subdevices, such as cpufreq and cpuidle, but I don't see any available infrastructure for this.

Moreover this code is not going to be shared between arm32 and arm64 in the near future (if ever) and doing things by using platform device has such benefit that the current code can be changed without a problem when the need arise. With DT there is no such flexibility and it also takes some time to design DT bindings properly.

The build problems should be fixed but if indeed they are not a new ones then the patchset should be brought back (Kukjin has dropped it entirely for now). This is important work which serves as a base for more cpuidle improvements from both Daniel and me therefore it would be great to have it merged in v3.16 and not delay it unnecessarily.

Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics

Arnd, Kukjin, Daniel,

On 12.05.2014 17:18, Daniel Lezcano wrote:

On 05/09/2014 02:02 PM, Tomasz Figa wrote:

...

Hi Arnd,

On 09.05.2014 12:56, Arnd Bergmann wrote:

...

On Friday 11 April 2014, Daniel Lezcano wrote:

...

No more dependency on the arch code. The platform_data field is used to set the PM callback as the other cpuidle drivers.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Reviewed-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com

This has just shown up in linux-next and broken randconfig builds.

...

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c index fe8dac8..d22f0e4 100644 --- a/arch/arm/mach-exynos/exynos.c +++ b/arch/arm/mach-exynos/exynos.c @@ -221,8 +221,9 @@ void exynos_restart(enum reboot_mode mode, const char *cmd) }

static struct platform_device exynos_cpuidle = {

•   .name           = "exynos_cpuidle",
  

•   .id             = -1,
  

•   .name              = "exynos_cpuidle",
  

•   .dev.platform_data = exynos_enter_aftr,
  

•   .id                = -1,
  

  };

This is wrong on many levels, can we please do this properly?

• The exynos_enter_aftr function is compiled conditionally, so you can't just reference it from generic code, or you get a link error.

+1

That is true but still we have a link error without this patch. We shouldn't register and declare this structure if CONFIG_PM / CONFIG_CPU_IDLE are not set.

...

...

• 'static struct platform_device ...' has been deprecated for at least a decade, stop doing that. For any platform devices that get registered, there is platform_device_register_simple().

+0.5

The missing 0.5 is because you can't pass platform data using platform_device_register_simple(). There is platform_device_register_resndata(), though.

...

• There shouldn't need to be a platform_device to start with, this should all come from DT. We can't do this on arm64 anyway, so any code that may be shared between arm32 and arm64 should have proper abstractions.

-1

Exynos cpuidle is not a device on the SoC, so I don't think there is any way to represent it in DT. The only thing I could see this is matching root node with a central SoC driver that instantiates specific subdevices, such as cpufreq and cpuidle, but I don't see any available infrastructure for this.

There is a RFC for defining generic idle states [1].

The idea behind using the platform driver framework is to unify the code across the different drivers and separate the PM / cpuidle code.

By this way, we can move the different drivers to drivers/cpuidle and store them in a single place. That make easier the tracking, the review and the maintenance.

I am ok to by using platform_device_register_resndata() but I would prefer to do that a bit later by converting the other drivers too. That will be easier if we have them grouped in a single directory (this is what does this patchset at the end).

As there are some more work based on this patchset and the link error could be fixed as an independent patch, I would recommend to re-integrate it in the tree as asked by Bartlomiej.

In general, it would be nice to have everything done properly, but I'd consider Daniel's series as a huge improvement already and a nice intermediate step towards further clean-up.

So based on the comments quoted above, instead of stalling the development, I'd suggest to accept this series and then move forward.

Best regards, Tomasz

A look at the code reveals use of S5P_VA_SYSRAM macro, in case of certain SoC revisions, which is not valid any longer, after SYSRAM started to be mapped dynamically. The new dynamic mapping is stored in sysram_base_addr variable, which is declared static in platsmp.c

This fix makes sysram_base_addr non-static, declared it in common.h and used in pm.c instead of S5P_VA_SYSRAM.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Suggested-by: Tomasz Figa t.figa@samsung.com --- arch/arm/mach-exynos/common.h | 1 + arch/arm/mach-exynos/platsmp.c | 2 +- arch/arm/mach-exynos/pm.c | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h index e2d0954..a012bc1 100644 --- a/arch/arm/mach-exynos/common.h +++ b/arch/arm/mach-exynos/common.h @@ -88,6 +88,7 @@ void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);

struct map_desc; extern void __iomem *sysram_ns_base_addr; +extern void __iomem *sysram_base_addr; void exynos_init_io(void); void exynos_restart(enum reboot_mode mode, const char *cmd); void exynos_cpuidle_init(void); diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c index 9c16da2..f2bea78 100644 --- a/arch/arm/mach-exynos/platsmp.c +++ b/arch/arm/mach-exynos/platsmp.c @@ -32,7 +32,7 @@

extern void exynos4_secondary_startup(void);

-static void __iomem *sysram_base_addr; +void __iomem *sysram_base_addr; void __iomem *sysram_ns_base_addr;

static void __init exynos_smp_prepare_sysram(void) diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c index d10c351..87c0d34 100644 --- a/arch/arm/mach-exynos/pm.c +++ b/arch/arm/mach-exynos/pm.c @@ -168,10 +168,10 @@ int exynos_cluster_power_state(int cluster)

#define EXYNOS_BOOT_VECTOR_ADDR (samsung_rev() == EXYNOS4210_REV_1_1 ? \ S5P_INFORM7 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \ - (S5P_VA_SYSRAM + 0x24) : S5P_INFORM0)) + (sysram_base_addr + 0x24) : S5P_INFORM0)) #define EXYNOS_BOOT_VECTOR_FLAG (samsung_rev() == EXYNOS4210_REV_1_1 ? \ S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \ - (S5P_VA_SYSRAM + 0x20) : S5P_INFORM1)) + (sysram_base_addr + 0x20) : S5P_INFORM1))

#define S5P_CHECK_AFTR 0xFCBA0D10 #define S5P_CHECK_SLEEP 0x00000BAD

On Fri, Apr 11, 2014 at 4:10 PM, Daniel Lezcano daniel.lezcano@linaro.org wrote:

The code to initiate and exit the powerdown sequence is the same in pm.c and cpuidle.c.

Let's split the common part in the pm.c and reuse it from the cpu_pm notifier.

That is one more step forward to make the cpuidle driver arch indenpendant.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Reviewed-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com

---

arch/arm/mach-exynos/cpuidle.c | 22 ---------------------- arch/arm/mach-exynos/pm.c | 24 ++++++++++++++++++++---- 2 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c index e6d813d..02609ac 100644 --- a/arch/arm/mach-exynos/cpuidle.c +++ b/arch/arm/mach-exynos/cpuidle.c @@ -28,7 +28,6 @@ #include <mach/map.h>

#include "common.h" -#include "regs-pmu.h"

static int idle_finisher(unsigned long flags) { @@ -42,31 +41,10 @@ static int exynos_enter_core0_aftr(struct cpuidle_device *dev, struct cpuidle_driver *drv, int index) {

•   unsigned long tmp;
  

•   /* Setting Central Sequence Register for power down mode */
  

•   tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
  

•   tmp &= ~S5P_CENTRAL_LOWPWR_CFG;
  

•   __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
  

•   cpu_pm_enter();
    cpu_suspend(0, idle_finisher);
    cpu_pm_exit();
  
  

•   /*
  

•    * If PMU failed while entering sleep mode, WFI will be
  

•    * ignored by PMU and then exiting cpu_do_idle().
  

•    * S5P_CENTRAL_LOWPWR_CFG bit will not be set automatically
  

•    * in this situation.
  

•    */
  

•   tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
  

•   if (!(tmp & S5P_CENTRAL_LOWPWR_CFG)) {
  

•           tmp |= S5P_CENTRAL_LOWPWR_CFG;
  

•           __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
  

•           /* Clear wakeup state register */
  

•           __raw_writel(0x0, S5P_WAKEUP_STAT);
  

•   }
  

•   return index;
  

}

diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c index 50b6b4d..6d9ef69 100644 --- a/arch/arm/mach-exynos/pm.c +++ b/arch/arm/mach-exynos/pm.c @@ -213,15 +213,21 @@ static void exynos_pm_prepare(void) __raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0); }

-static int exynos_pm_suspend(void) +static void exynos_pm_central_suspend(void) { unsigned long tmp;

    /* Setting Central Sequence Register for power down mode */

•   tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
    tmp &= ~S5P_CENTRAL_LOWPWR_CFG;
    __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
  

+}

+static int exynos_pm_suspend(void) +{

•   unsigned long tmp;
  

•   exynos_pm_central_suspend();
  
    /* Setting SEQ_OPTION register */
  
  

@@ -234,7 +240,7 @@ static int exynos_pm_suspend(void) return 0; }

-static void exynos_pm_resume(void) +static int exynos_pm_central_resume(void) { unsigned long tmp;

@@ -251,9 +257,17 @@ static void exynos_pm_resume(void) /* clear the wakeup state register */ __raw_writel(0x0, S5P_WAKEUP_STAT); /* No need to perform below restore code */

•           goto early_wakeup;
  

•           return -1;
    }
  
  

•   return 0;
  

+}

+static void exynos_pm_resume(void) +{

•   if (exynos_pm_central_resume())
  

•           goto early_wakeup;
  

•   if (!soc_is_exynos5250())
            exynos_cpu_restore_register();
  
  

@@ -359,6 +373,7 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self, switch (cmd) { case CPU_PM_ENTER: if (cpu == 0) {

•                   exynos_pm_central_suspend();
                    exynos_cpu_save_register();
            }
            break;
  

@@ -368,6 +383,7 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self, if (!soc_is_exynos5250()) scu_enable(S5P_VA_SCU); exynos_cpu_restore_register();

•                   exynos_pm_central_resume();
  

This notifier is called for system wide suspend and cpuidle.

In case of Exynos cpuidle only AFTR and LPA state need to program central_sequencer and store/restore the registers.

But in 5420 (core-power-down), this is not required, and causing the regression.

Hence need to remove this notifier, or need to find a way to differentiate the cpuidle state.

            }
            break;
    }

-- 1.7.9.5

-- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

On Thu, Jun 26, 2014 at 3:18 PM, Tomasz Figa t.figa@samsung.com wrote:

Hi Chander,

On 26.06.2014 11:07, Chander Kashyap wrote:

...

On Fri, Apr 11, 2014 at 4:10 PM, Daniel Lezcano daniel.lezcano@linaro.org wrote:

[snip]

...

...

@@ -359,6 +373,7 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self, switch (cmd) { case CPU_PM_ENTER: if (cpu == 0) {

•                   exynos_pm_central_suspend();
                    exynos_cpu_save_register();
            }
            break;
  

@@ -368,6 +383,7 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self, if (!soc_is_exynos5250()) scu_enable(S5P_VA_SCU); exynos_cpu_restore_register();

•                   exynos_pm_central_resume();
  

This notifier is called for system wide suspend and cpuidle.

In case of Exynos cpuidle only AFTR and LPA state need to program central_sequencer and store/restore the registers.

But in 5420 (core-power-down), this is not required, and causing the regression.

Hence need to remove this notifier, or need to find a way to differentiate the cpuidle state.

This patch is already present in v3.16. Moreover, Exynos5420 cpuidle has not been merged yet. This means that this issue is not a regression and I believe any further work on this should be carried out as further patches on top of this change.

Anyway, this change has introduced a regression, though, but in another area - it broke suspend, at least on Exynos4-based devices, because now certain steps are performed twice. I've sent a patch for 3.16-rc3 to fix this by dropping custom suspend-specific syscore ops, effectively moving most of the handling to CPU PM notifier, which also matches requirements of AFTR and lower power states. See [1].

[1] http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg32935.html

However, in this case, moving back to suspend-specific syscore_ops and simply duplicating some code for lower cpuidle states might be a better option. Care to send a patch (fix for 3.16, replacing mine) or I should do it?

Yes need to move the common code to lower cpuildle implementation and removing the notifier all together. I will do it and send the patch.

Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html