ARM: EXYNOS: Refactor the pm code to use DT based lookup

Refactoring the pm.c to avoid using "soc_is_exynos" checks, instead use the DT based lookup.

While at it, consolidate the common code across SoCs and create a static helper functions.

Signed-off-by Vikas Sajjan <[email protected]>

---
changes since v1:
- Address Kukjin Kim comments to respin this patch separately from
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/272574.html
- removed panic, returned if no PMU node found and added check in exynos_wkup_irq.

Rebased on Kukjin Kim's tree, for-next branch
https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/log/?h=for-next

---
 arch/arm/mach-exynos/pm.c       |  234 ++++++++++++++++++++++++++++++++-------
 arch/arm/mach-exynos/regs-pmu.h |    1 +
 2 files changed, 192 insertions(+), 43 deletions(-)

arch/arm/mach-exynos/pm.c +191 -43

index c4c6d98..1c875e5 100644
@@ -36,6 +36,8 @@
 #include "regs-pmu.h"
 #include "regs-sys.h"
 
+#define REG_TABLE_END (-1U)
+
 /**
  * struct exynos_wkup_irq - Exynos GIC to PMU IRQ mapping
  * @hwirq: Hardware IRQ signal of the GIC

@@ -59,6 +61,19 @@ static struct sleep_save exynos_core_save[] = {
        SAVE_ITEM(S5P_SROM_BC3),
 };
 
+struct exynos_pm_data {
+       const struct exynos_wkup_irq *wkup_irq;
+       unsigned int wake_disable_mask;
+       unsigned int *release_ret_regs;
+
+       void (*pm_prepare)(void);
+       void (*pm_resume)(void);
+       int (*pm_suspend)(void);
+       int (*cpu_suspend)(unsigned long);
+};
+
+struct exynos_pm_data *pm_data;
+
 /*
  * GIC wake-up support
  */

@@ -77,14 +92,24 @@ static const struct exynos_wkup_irq exynos5250_wkup_irq[] = {
        { /* sentinel */ },
 };
 
+unsigned int exynos_release_ret_regs[] = {
+       S5P_PAD_RET_MAUDIO_OPTION,
+       S5P_PAD_RET_GPIO_OPTION,
+       S5P_PAD_RET_UART_OPTION,
+       S5P_PAD_RET_MMCA_OPTION,
+       S5P_PAD_RET_MMCB_OPTION,
+       S5P_PAD_RET_EBIA_OPTION,
+       S5P_PAD_RET_EBIB_OPTION,
+       REG_TABLE_END,
+};
+
 static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
 {
        const struct exynos_wkup_irq *wkup_irq;
 
-       if (soc_is_exynos5250())
-               wkup_irq = exynos5250_wkup_irq;
-       else
-               wkup_irq = exynos4_wkup_irq;
+       if (!pm_data->wkup_irq)
+               return -ENOENT;
+       wkup_irq = pm_data->wkup_irq;
 
        while (wkup_irq->mask) {
                if (wkup_irq->hwirq == data->hwirq) {

@@ -241,13 +266,13 @@ static void exynos_cpu_restore_register(void)
 
 static int exynos_cpu_suspend(unsigned long arg)
 {
-#ifdef CONFIG_CACHE_L2X0
-       outer_flush_all();
-#endif
-
-       if (soc_is_exynos5250())
-               flush_cache_all();
+       if (pm_data->cpu_suspend)
+               return pm_data->cpu_suspend(arg);
+       return -1;
+}
 
+static int exynos_cpu_do_idle(void)
+{
        /* issue the standby signal into the pm unit. */
        cpu_do_idle();
 

@@ -257,32 +282,73 @@ static int exynos_cpu_suspend(unsigned long arg)
 
 static void exynos_pm_prepare(void)
 {
-       unsigned int tmp;
+       if (pm_data->pm_prepare)
+               pm_data->pm_prepare();
+}
 
+static int exynos4_cpu_suspend(unsigned long arg)
+{
+#ifdef CONFIG_CACHE_L2X0
+       outer_flush_all();
+#endif
+       return exynos_cpu_do_idle();
+}
+
+static int exynos5250_cpu_suspend(unsigned long arg)
+{
+#ifdef CONFIG_CACHE_L2X0
+       outer_flush_all();
+#endif
+       flush_cache_all();
+       return exynos_cpu_do_idle();
+}
+
+static void exynos_pm_set_wakeup_mask(void)
+{
        /* Set wake-up mask registers */
        pmu_raw_writel(exynos_get_eint_wake_mask(), S5P_EINT_WAKEUP_MASK);
        pmu_raw_writel(exynos_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
+}
 
-       s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
-
-       if (soc_is_exynos5250()) {
-               s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
-               /* Disable USE_RETENTION of JPEG_MEM_OPTION */
-               tmp = pmu_raw_readl(EXYNOS5_JPEG_MEM_OPTION);
-               tmp &= ~EXYNOS5_OPTION_USE_RETENTION;
-               pmu_raw_writel(tmp, EXYNOS5_JPEG_MEM_OPTION);
-       }
-
+static void exynos_pm_enter_sleep_mode(void)
+{
        /* Set value of power down register for sleep mode */
-
        exynos_sys_powerdown_conf(SYS_SLEEP);
        pmu_raw_writel(S5P_CHECK_SLEEP, S5P_INFORM1);
 
        /* ensure at least INFORM0 has the resume address */
-
        pmu_raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0);
 }
 
+static void exynos5250_pm_prepare(void)
+{
+       unsigned int tmp;
+
+       /* Set wake-up mask registers */
+       exynos_pm_set_wakeup_mask();
+
+       s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+       s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
+
+       /* Disable USE_RETENTION of JPEG_MEM_OPTION */
+       tmp = pmu_raw_readl(EXYNOS5_JPEG_MEM_OPTION);
+       tmp &= ~EXYNOS5_OPTION_USE_RETENTION;
+       pmu_raw_writel(tmp, EXYNOS5_JPEG_MEM_OPTION);
+
+       exynos_pm_enter_sleep_mode();
+}
+
+static void exynos4_pm_prepare(void)
+{
+       /* Set wake-up mask registers */
+       exynos_pm_set_wakeup_mask();
+
+       s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+       exynos_pm_enter_sleep_mode();
+}
+
 static void exynos_pm_central_suspend(void)
 {
        unsigned long tmp;

@@ -295,6 +361,13 @@ static void exynos_pm_central_suspend(void)
 
 static int exynos_pm_suspend(void)
 {
+       if (pm_data->pm_suspend)
+               return pm_data->pm_suspend();
+       return -1;
+}
+
+static int exynos4_pm_suspend(void)
+{
        unsigned long tmp;
 
        exynos_pm_central_suspend();

@@ -304,9 +377,20 @@ static int exynos_pm_suspend(void)
        tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
        pmu_raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);
 
-       if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-               exynos_cpu_save_register();
+       exynos_cpu_save_register();
+       return 0;
+}
+
+static int exynos5250_pm_suspend(void)
+{
+       unsigned long tmp;
+
+       exynos_pm_central_suspend();
+
+       /* Setting SEQ_OPTION register */
 
+       tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
+       pmu_raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);
        return 0;
 }
 

@@ -333,39 +417,57 @@ static int exynos_pm_central_resume(void)
        return 0;
 }
 
+static void exynos_pm_set_release_retention(void)
+{
+       unsigned int i;
+
+       for (i = 0; (pm_data->release_ret_regs[i] != REG_TABLE_END); i++)
+               pmu_raw_writel(EXYNOS_WAKEUP_FROM_LOWPWR,
+                               pm_data->release_ret_regs[i]);
+}
+
 static void exynos_pm_resume(void)
 {
+       if (pm_data->pm_resume)
+               pm_data->pm_resume();
+}
+
+static void exynos4_pm_resume(void)
+{
        if (exynos_pm_central_resume())
                goto early_wakeup;
 
-       if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-               exynos_cpu_restore_register();
+       exynos_cpu_restore_register();
 
        /* For release retention */
-
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_MAUDIO_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_GPIO_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_UART_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_MMCA_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_MMCB_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_EBIA_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_EBIB_OPTION);
-
-       if (soc_is_exynos5250())
-               s3c_pm_do_restore(exynos5_sys_save,
-                       ARRAY_SIZE(exynos5_sys_save));
+       exynos_pm_set_release_retention();
 
        s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
-       if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-               scu_enable(S5P_VA_SCU);
+       scu_enable(S5P_VA_SCU);
 
 early_wakeup:
 
        /* Clear SLEEP mode set in INFORM1 */
        pmu_raw_writel(0x0, S5P_INFORM1);
+}
+
+static void exynos5250_pm_resume(void)
+{
+       if (exynos_pm_central_resume())
+               goto early_wakeup;
 
-       return;
+       /* For release retention */
+       exynos_pm_set_release_retention();
+
+       s3c_pm_do_restore(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
+
+       s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+early_wakeup:
+
+       /* Clear SLEEP mode set in INFORM1 */
+       pmu_raw_writel(0x0, S5P_INFORM1);
 }
 
 static struct syscore_ops exynos_pm_syscore_ops = {

@@ -468,18 +570,64 @@ static struct notifier_block exynos_cpu_pm_notifier_block = {
        .notifier_call = exynos_cpu_pm_notifier,
 };
 
+static struct exynos_pm_data exynos4_pm_data = {
+       .wkup_irq       = exynos4_wkup_irq,
+       .wake_disable_mask = ((0xFF << 8) | (0x1F << 1)),
+       .release_ret_regs = exynos_release_ret_regs,
+       .pm_suspend     = exynos4_pm_suspend,
+       .pm_resume      = exynos4_pm_resume,
+       .pm_prepare     = exynos4_pm_prepare,
+       .cpu_suspend    = exynos4_cpu_suspend,
+};
+
+static struct exynos_pm_data exynos5250_pm_data = {
+       .wkup_irq       = exynos5250_wkup_irq,
+       .wake_disable_mask = ((0xFF << 8) | (0x1F << 1)),
+       .release_ret_regs = exynos_release_ret_regs,
+       .pm_suspend     = exynos5250_pm_suspend,
+       .pm_resume      = exynos5250_pm_resume,
+       .pm_prepare     = exynos5250_pm_prepare,
+       .cpu_suspend    = exynos5250_cpu_suspend,
+};
+
+static struct of_device_id exynos_pmu_of_device_ids[] = {
+       {
+               .compatible = "samsung,exynos4210-pmu",
+               .data = (void *)&exynos4_pm_data,
+       }, {
+               .compatible = "samsung,exynos4212-pmu",
+               .data = (void *)&exynos4_pm_data,
+       }, {
+               .compatible = "samsung,exynos4412-pmu",
+               .data = (void *)&exynos4_pm_data,
+       }, {
+               .compatible = "samsung,exynos5250-pmu",
+               .data = (void *)&exynos5250_pm_data,
+       },
+       { /*sentinel*/ },
+};
+
 void __init exynos_pm_init(void)
 {
        u32 tmp;
+       const struct of_device_id *match;
 
        cpu_pm_register_notifier(&exynos_cpu_pm_notifier_block);
 
+       of_find_matching_node_and_match(NULL, exynos_pmu_of_device_ids, &match);
+
+       if (!match) {
+               pr_err("Failed to find PMU node\n");
+               return;
+       }
+       pm_data = (struct exynos_pm_data *) match->data;
+
        /* Platform-specific GIC callback */
        gic_arch_extn.irq_set_wake = exynos_irq_set_wake;
 
        /* All wakeup disable */
        tmp = pmu_raw_readl(S5P_WAKEUP_MASK);
-       tmp |= ((0xFF << 8) | (0x1F << 1));
+       tmp |= pm_data->wake_disable_mask;
        pmu_raw_writel(tmp, S5P_WAKEUP_MASK);
 
        register_syscore_ops(&exynos_pm_syscore_ops);

arch/arm/mach-exynos/regs-pmu.h +1 -1

index 96a1569..30c0301 100644
@@ -21,6 +21,7 @@
 #define S5P_USE_STANDBY_WFI0                   (1 << 16)
 #define S5P_USE_STANDBY_WFE0                   (1 << 24)
 
+#define EXYNOS_WAKEUP_FROM_LOWPWR              (1 << 28)
 #define EXYNOS_SWRESET                         0x0400
 #define EXYNOS5440_SWRESET                     0x00C4
 
--

1.7.9.5

Refactoring the pm.c to avoid using "soc_is_exynos" checks, instead use the DT based lookup.

While at it, consolidate the common code across SoCs and create a static helper functions.

Signed-off-by Vikas Sajjan <[email protected]>

---
changes since v1:
- Address Kukjin Kim comments to respin this patch separately from
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/272574.html
- removed panic, returned if no PMU node found and added check in exynos_wkup_irq.

Rebased on Kukjin Kim's tree, for-next branch
https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/log/?h=for-next

---
 arch/arm/mach-exynos/pm.c       |  234 ++++++++++++++++++++++++++++++++-------
 arch/arm/mach-exynos/regs-pmu.h |    1 +
 2 files changed, 192 insertions(+), 43 deletions(-)

arch/arm/mach-exynos/pm.c +41 -11

index c4c6d98..1c875e5 100644
@@ -36,6 +36,8 @@
 #include "regs-pmu.h"
 #include "regs-sys.h"
 
+#define REG_TABLE_END (-1U)
+
 /**
  * struct exynos_wkup_irq - Exynos GIC to PMU IRQ mapping
  * @hwirq: Hardware IRQ signal of the GIC

@@ -59,6 +61,19 @@ static struct sleep_save exynos_core_save[] = {
        SAVE_ITEM(S5P_SROM_BC3),
 };
 
+struct exynos_pm_data {
+       const struct exynos_wkup_irq *wkup_irq;
+       unsigned int wake_disable_mask;
+       unsigned int *release_ret_regs;
+
+       void (*pm_prepare)(void);
+       void (*pm_resume)(void);
+       int (*pm_suspend)(void);
+       int (*cpu_suspend)(unsigned long);
+};
+
+struct exynos_pm_data *pm_data;
+
 /*
  * GIC wake-up support
  */

@@ -77,14 +92,24 @@ static const struct exynos_wkup_irq exynos5250_wkup_irq[] = {
        { /* sentinel */ },
 };
 
+unsigned int exynos_release_ret_regs[] = {
+       S5P_PAD_RET_MAUDIO_OPTION,
+       S5P_PAD_RET_GPIO_OPTION,
+       S5P_PAD_RET_UART_OPTION,
+       S5P_PAD_RET_MMCA_OPTION,
+       S5P_PAD_RET_MMCB_OPTION,
+       S5P_PAD_RET_EBIA_OPTION,
+       S5P_PAD_RET_EBIB_OPTION,
+       REG_TABLE_END,
+};
+
 static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
 {
        const struct exynos_wkup_irq *wkup_irq;
 
-       if (soc_is_exynos5250())
-               wkup_irq = exynos5250_wkup_irq;
-       else
-               wkup_irq = exynos4_wkup_irq;
+       if (!pm_data->wkup_irq)
+               return -ENOENT;
+       wkup_irq = pm_data->wkup_irq;
 
        while (wkup_irq->mask) {
                if (wkup_irq->hwirq == data->hwirq) {

@@ -241,13 +266,13 @@ static void exynos_cpu_restore_register(void)
 
 static int exynos_cpu_suspend(unsigned long arg)
 {
-#ifdef CONFIG_CACHE_L2X0
-       outer_flush_all();
-#endif
-
-       if (soc_is_exynos5250())
-               flush_cache_all();
+       if (pm_data->cpu_suspend)
+               return pm_data->cpu_suspend(arg);
+       return -1;
+}
 
+static int exynos_cpu_do_idle(void)
+{
        /* issue the standby signal into the pm unit. */
        cpu_do_idle();
 

@@ -257,32 +282,73 @@ static int exynos_cpu_suspend(unsigned long arg)
 
 static void exynos_pm_prepare(void)
 {
-       unsigned int tmp;
+       if (pm_data->pm_prepare)
+               pm_data->pm_prepare();
+}
 
+static int exynos4_cpu_suspend(unsigned long arg)
+{
+#ifdef CONFIG_CACHE_L2X0

arch/arm/mach-exynos/pm.c +140 -32

+       flush_cache_all();
+       return exynos_cpu_do_idle();
+}
+
+static void exynos_pm_set_wakeup_mask(void)
+{
        /* Set wake-up mask registers */
        pmu_raw_writel(exynos_get_eint_wake_mask(), S5P_EINT_WAKEUP_MASK);
        pmu_raw_writel(exynos_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
+}
 
-       s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
-
-       if (soc_is_exynos5250()) {
-               s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
-               /* Disable USE_RETENTION of JPEG_MEM_OPTION */
-               tmp = pmu_raw_readl(EXYNOS5_JPEG_MEM_OPTION);
-               tmp &= ~EXYNOS5_OPTION_USE_RETENTION;
-               pmu_raw_writel(tmp, EXYNOS5_JPEG_MEM_OPTION);
-       }
-
+static void exynos_pm_enter_sleep_mode(void)
+{
        /* Set value of power down register for sleep mode */
-
        exynos_sys_powerdown_conf(SYS_SLEEP);
        pmu_raw_writel(S5P_CHECK_SLEEP, S5P_INFORM1);
 
        /* ensure at least INFORM0 has the resume address */
-
        pmu_raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0);
 }
 
+static void exynos5250_pm_prepare(void)
+{
+       unsigned int tmp;
+
+       /* Set wake-up mask registers */
+       exynos_pm_set_wakeup_mask();
+
+       s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+       s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
+
+       /* Disable USE_RETENTION of JPEG_MEM_OPTION */
+       tmp = pmu_raw_readl(EXYNOS5_JPEG_MEM_OPTION);
+       tmp &= ~EXYNOS5_OPTION_USE_RETENTION;
+       pmu_raw_writel(tmp, EXYNOS5_JPEG_MEM_OPTION);
+
+       exynos_pm_enter_sleep_mode();
+}
+
+static void exynos4_pm_prepare(void)
+{
+       /* Set wake-up mask registers */
+       exynos_pm_set_wakeup_mask();
+
+       s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+       exynos_pm_enter_sleep_mode();
+}
+
 static void exynos_pm_central_suspend(void)
 {
        unsigned long tmp;

@@ -295,6 +361,13 @@ static void exynos_pm_central_suspend(void)
 
 static int exynos_pm_suspend(void)
 {
+       if (pm_data->pm_suspend)
+               return pm_data->pm_suspend();
+       return -1;
+}
+
+static int exynos4_pm_suspend(void)
+{
        unsigned long tmp;
 
        exynos_pm_central_suspend();

@@ -304,9 +377,20 @@ static int exynos_pm_suspend(void)
        tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
        pmu_raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);
 
-       if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-               exynos_cpu_save_register();
+       exynos_cpu_save_register();
+       return 0;
+}
+
+static int exynos5250_pm_suspend(void)
+{
+       unsigned long tmp;
+
+       exynos_pm_central_suspend();
+
+       /* Setting SEQ_OPTION register */
 
+       tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
+       pmu_raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);
        return 0;
 }
 

@@ -333,39 +417,57 @@ static int exynos_pm_central_resume(void)
        return 0;
 }
 
+static void exynos_pm_set_release_retention(void)
+{
+       unsigned int i;
+
+       for (i = 0; (pm_data->release_ret_regs[i] != REG_TABLE_END); i++)
+               pmu_raw_writel(EXYNOS_WAKEUP_FROM_LOWPWR,
+                               pm_data->release_ret_regs[i]);
+}
+
 static void exynos_pm_resume(void)
 {
+       if (pm_data->pm_resume)
+               pm_data->pm_resume();
+}
+
+static void exynos4_pm_resume(void)
+{
        if (exynos_pm_central_resume())
                goto early_wakeup;
 
-       if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-               exynos_cpu_restore_register();
+       exynos_cpu_restore_register();
 
        /* For release retention */
-
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_MAUDIO_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_GPIO_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_UART_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_MMCA_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_MMCB_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_EBIA_OPTION);
-       pmu_raw_writel((1 << 28), S5P_PAD_RET_EBIB_OPTION);
-
-       if (soc_is_exynos5250())
-               s3c_pm_do_restore(exynos5_sys_save,
-                       ARRAY_SIZE(exynos5_sys_save));
+       exynos_pm_set_release_retention();
 
        s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
-       if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-               scu_enable(S5P_VA_SCU);
+       scu_enable(S5P_VA_SCU);
 
 early_wakeup:
 
        /* Clear SLEEP mode set in INFORM1 */
        pmu_raw_writel(0x0, S5P_INFORM1);
+}
+
+static void exynos5250_pm_resume(void)
+{
+       if (exynos_pm_central_resume())
+               goto early_wakeup;
 
-       return;
+       /* For release retention */
+       exynos_pm_set_release_retention();
+
+       s3c_pm_do_restore(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
+
+       s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+early_wakeup:
+
+       /* Clear SLEEP mode set in INFORM1 */
+       pmu_raw_writel(0x0, S5P_INFORM1);
 }
 
 static struct syscore_ops exynos_pm_syscore_ops = {

@@ -468,18 +570,64 @@ static struct notifier_block exynos_cpu_pm_notifier_block = {
        .notifier_call = exynos_cpu_pm_notifier,
 };
 
+static struct exynos_pm_data exynos4_pm_data = {
+       .wkup_irq       = exynos4_wkup_irq,
+       .wake_disable_mask = ((0xFF << 8) | (0x1F << 1)),
+       .release_ret_regs = exynos_release_ret_regs,
+       .pm_suspend     = exynos4_pm_suspend,
+       .pm_resume      = exynos4_pm_resume,
+       .pm_prepare     = exynos4_pm_prepare,
+       .cpu_suspend    = exynos4_cpu_suspend,
+};
+
+static struct exynos_pm_data exynos5250_pm_data = {
+       .wkup_irq       = exynos5250_wkup_irq,
+       .wake_disable_mask = ((0xFF << 8) | (0x1F << 1)),
+       .release_ret_regs = exynos_release_ret_regs,
+       .pm_suspend     = exynos5250_pm_suspend,
+       .pm_resume      = exynos5250_pm_resume,
+       .pm_prepare     = exynos5250_pm_prepare,
+       .cpu_suspend    = exynos5250_cpu_suspend,
+};
+
+static struct of_device_id exynos_pmu_of_device_ids[] = {
+       {
+               .compatible = "samsung,exynos4210-pmu",
+               .data = (void *)&exynos4_pm_data,
+       }, {
+               .compatible = "samsung,exynos4212-pmu",
+               .data = (void *)&exynos4_pm_data,
+       }, {
+               .compatible = "samsung,exynos4412-pmu",
+               .data = (void *)&exynos4_pm_data,
+       }, {
+               .compatible = "samsung,exynos5250-pmu",
+               .data = (void *)&exynos5250_pm_data,
+       },
+       { /*sentinel*/ },
+};
+
 void __init exynos_pm_init(void)
 {
        u32 tmp;
+       const struct of_device_id *match;
 
        cpu_pm_register_notifier(&exynos_cpu_pm_notifier_block);
 
+       of_find_matching_node_and_match(NULL, exynos_pmu_of_device_ids, &match);
+
+       if (!match) {
+               pr_err("Failed to find PMU node\n");
+               return;
+       }
+       pm_data = (struct exynos_pm_data *) match->data;
+
        /* Platform-specific GIC callback */
        gic_arch_extn.irq_set_wake = exynos_irq_set_wake;
 
        /* All wakeup disable */
        tmp = pmu_raw_readl(S5P_WAKEUP_MASK);
-       tmp |= ((0xFF << 8) | (0x1F << 1));
+       tmp |= pm_data->wake_disable_mask;
        pmu_raw_writel(tmp, S5P_WAKEUP_MASK);
 
        register_syscore_ops(&exynos_pm_syscore_ops);

arch/arm/mach-exynos/regs-pmu.h +0 -0

index 96a1569..30c0301 100644

--
1.7.9.5

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

Refactoring the pm.c to avoid using "soc_is_exynos" checks, instead use the DT based lookup.

While at it, consolidate the common code across SoCs and create a static helper functions.

arch/arm/mach-exynos/pm.c +0 -0

@@ -241,13 +266,13 @@ static void exynos_cpu_restore_register(void)
 
 static int exynos_cpu_suspend(unsigned long arg)
 {

arch/arm/mach-exynos/pm.c +7 -0

+}
 
+static int exynos4_cpu_suspend(unsigned long arg)
+{
+#ifdef CONFIG_CACHE_L2X0
+	outer_flush_all();
+#endif
+	return exynos_cpu_do_idle();

arch/arm/mach-exynos/pm.c +6 -12

+
+static void exynos_pm_set_wakeup_mask(void)
+{
 	/* Set wake-up mask registers */
 	pmu_raw_writel(exynos_get_eint_wake_mask(), S5P_EINT_WAKEUP_MASK);
 	pmu_raw_writel(exynos_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
+}
 
-	s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
-
-	if (soc_is_exynos5250()) {
-		s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
-		/* Disable USE_RETENTION of JPEG_MEM_OPTION */
-		tmp = pmu_raw_readl(EXYNOS5_JPEG_MEM_OPTION);
-		tmp &= ~EXYNOS5_OPTION_USE_RETENTION;
-		pmu_raw_writel(tmp, EXYNOS5_JPEG_MEM_OPTION);
-	}
-
+static void exynos_pm_enter_sleep_mode(void)
+{
 	/* Set value of power down register for sleep mode */
-
 	exynos_sys_powerdown_conf(SYS_SLEEP);
 	pmu_raw_writel(S5P_CHECK_SLEEP, S5P_INFORM1);
 
 	/* ensure at least INFORM0 has the resume address */
-
 	pmu_raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0);
 }
 

arch/arm/mach-exynos/pm.c +9 -2

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

@@ -304,9 +377,20 @@ static int exynos_pm_suspend(void)
 	tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
 	pmu_raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);
 
-	if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-		exynos_cpu_save_register();
+	exynos_cpu_save_register();
+	return 0;
+}
+
+static int exynos5250_pm_suspend(void)
+{

arch/arm/mach-exynos/pm.c +14 -17

+
+static void exynos4_pm_resume(void)
+{
 	if (exynos_pm_central_resume())
 		goto early_wakeup;
 
-	if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-		exynos_cpu_restore_register();
+	exynos_cpu_restore_register();
 
 	/* For release retention */
-
-	pmu_raw_writel((1 << 28), S5P_PAD_RET_MAUDIO_OPTION);
-	pmu_raw_writel((1 << 28), S5P_PAD_RET_GPIO_OPTION);
-	pmu_raw_writel((1 << 28), S5P_PAD_RET_UART_OPTION);
-	pmu_raw_writel((1 << 28), S5P_PAD_RET_MMCA_OPTION);
-	pmu_raw_writel((1 << 28), S5P_PAD_RET_MMCB_OPTION);
-	pmu_raw_writel((1 << 28), S5P_PAD_RET_EBIA_OPTION);
-	pmu_raw_writel((1 << 28), S5P_PAD_RET_EBIB_OPTION);
-
-	if (soc_is_exynos5250())
-		s3c_pm_do_restore(exynos5_sys_save,
-			ARRAY_SIZE(exynos5_sys_save));
+	exynos_pm_set_release_retention();
 
 	s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
-	if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-		scu_enable(S5P_VA_SCU);
+	scu_enable(S5P_VA_SCU);
 
 early_wakeup:
 
 	/* Clear SLEEP mode set in INFORM1 */
 	pmu_raw_writel(0x0, S5P_INFORM1);
+}
+
+static void exynos5250_pm_resume(void)
+{
+	if (exynos_pm_central_resume())
+		goto early_wakeup;
 
-	return;
+	/* For release retention */
+	exynos_pm_set_release_retention();

arch/arm/mach-exynos/pm.c +7 -0

+	}, {
+		.compatible = "samsung,exynos4212-pmu",
+		.data = (void *)&exynos4_pm_data,
+	}, {
+		.compatible = "samsung,exynos4412-pmu",
+		.data = (void *)&exynos4_pm_data,
+	}, {

Hi Vikas,

Please see my comments inline.

Quoted 5 lines · Vikas Sajjan

Refactoring the pm.c to avoid using "soc_is_exynos" checks, instead use the DT based lookup.

While at it, consolidate the common code across SoCs and create a static helper functions.

[snip]

Quoted 2 lines

(quoted diff) +0 -0

@@ -241,13 +266,13 @@ static void exynos_cpu_restore_register(void)
 

arch/arm/mach-exynos/pm.c static void exynos_cpu_restore_register(void) · line 272

Quoted 12 lines

arch/arm/mach-exynos/pm.c +4 -6

 static int exynos_cpu_suspend(unsigned long arg)
 {
-#ifdef CONFIG_CACHE_L2X0
-     outer_flush_all();
-#endif
-
-     if (soc_is_exynos5250())
-             flush_cache_all();
+     if (pm_data->cpu_suspend)
+             return pm_data->cpu_suspend(arg);
+     return -1;
+}

I believe you could just pass pm_data->cpu_suspend to cpu_suspend(), without this three-liner.

Quoted 8 lines

arch/arm/mach-exynos/pm.c +2 -0

+static int exynos_cpu_do_idle(void)
+{
      /* issue the standby signal into the pm unit. */
      cpu_do_idle();
 

@@ -257,32 +282,73 @@ static int exynos_cpu_suspend(unsigned long arg)
 

arch/arm/mach-exynos/pm.c static int exynos_cpu_suspend(unsigned long arg) · line 286

Quoted 5 lines

arch/arm/mach-exynos/pm.c +2 -1

 static void exynos_pm_prepare(void)
 {
-     unsigned int tmp;
+     if (pm_data->pm_prepare)
+             pm_data->pm_prepare();

You could just directly insert this code into exynos_suspend_enter() instead of adding this two-liner.

Quoted 9 lines

arch/arm/mach-exynos/pm.c +7 -0

+}
 
+static int exynos4_cpu_suspend(unsigned long arg)
+{
+#ifdef CONFIG_CACHE_L2X0
+     outer_flush_all();
+#endif
+     return exynos_cpu_do_idle();

arch/arm/mach-exynos/pm.c static int exynos_cpu_suspend(unsigned long arg) · line 304

Quoted 10 lines

arch/arm/mach-exynos/pm.c +10 -0

+}
+
+static int exynos5250_cpu_suspend(unsigned long arg)
+{
+#ifdef CONFIG_CACHE_L2X0
+     outer_flush_all();
+#endif
+     flush_cache_all();
+     return exynos_cpu_do_idle();
+}

I believe both can be merged safely into the same single function. A call to flush_cache_all() will not hurt on Exynos4, but it should be moved before outer_flush_all().

Moreover, the #ifdef CONFIG_CACHE_L2X0 is superfluous, because the existence of outer_flush_all() symbol doesn't depend on this Kconfig symbol (it depends on CONFIG_OUTER_CACHE_SYNC, but a stub is provided if it is disabled).

Quoted 30 lines

arch/arm/mach-exynos/pm.c +6 -12

+
+static void exynos_pm_set_wakeup_mask(void)
+{
      /* Set wake-up mask registers */
      pmu_raw_writel(exynos_get_eint_wake_mask(), S5P_EINT_WAKEUP_MASK);
      pmu_raw_writel(exynos_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
+}
 
-     s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
-
-     if (soc_is_exynos5250()) {
-             s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
-             /* Disable USE_RETENTION of JPEG_MEM_OPTION */
-             tmp = pmu_raw_readl(EXYNOS5_JPEG_MEM_OPTION);
-             tmp &= ~EXYNOS5_OPTION_USE_RETENTION;
-             pmu_raw_writel(tmp, EXYNOS5_JPEG_MEM_OPTION);
-     }
-
+static void exynos_pm_enter_sleep_mode(void)
+{
      /* Set value of power down register for sleep mode */
-
      exynos_sys_powerdown_conf(SYS_SLEEP);
      pmu_raw_writel(S5P_CHECK_SLEEP, S5P_INFORM1);
 
      /* ensure at least INFORM0 has the resume address */
-
      pmu_raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0);
 }
 

arch/arm/mach-exynos/pm.c static int exynos_cpu_suspend(unsigned long arg) · line 332

Quoted 10 lines

arch/arm/mach-exynos/pm.c +10 -0

+static void exynos5250_pm_prepare(void)
+{
+     unsigned int tmp;
+
+     /* Set wake-up mask registers */
+     exynos_pm_set_wakeup_mask();
+
+     s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+     s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));

You could store a pointer to this array in new .extra_save field of the variant struct and handle this with generic code, like below:

        if (pm_data->extra_save)
                s3c_pm_do_save(pm_data->extra_save,
                                pm_data->num_extra_save);

arch/arm/mach-exynos/pm.c static int exynos_cpu_suspend(unsigned long arg) · line 337

Quoted 5 lines

arch/arm/mach-exynos/pm.c +5 -0

+
+     /* Disable USE_RETENTION of JPEG_MEM_OPTION */
+     tmp = pmu_raw_readl(EXYNOS5_JPEG_MEM_OPTION);
+     tmp &= ~EXYNOS5_OPTION_USE_RETENTION;
+     pmu_raw_writel(tmp, EXYNOS5_JPEG_MEM_OPTION);

This looks completely like a copy paste from a vendor tree needed to workaround some issues in early revisions of the SoC. Are you sure this is still needed in production versions of the silicon used on boards supported in mainline?

Even if yes, Exynos4 handles such registers in PMU register array in pmu.c, so I wonder whether it shouldn't be moved there and allow handling of all Exynos SoCs uniformly in this file.

Quoted 9 lines

arch/arm/mach-exynos/pm.c +9 -0

+
+     exynos_pm_enter_sleep_mode();
+}
+
+static void exynos4_pm_prepare(void)
+{
+     /* Set wake-up mask registers */
+     exynos_pm_set_wakeup_mask();
+

arch/arm/mach-exynos/pm.c static int exynos_cpu_suspend(unsigned long arg) · line 340

Quoted 4 lines

arch/arm/mach-exynos/pm.c +4 -0

+     s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+     exynos_pm_enter_sleep_mode();
+}

In fact, exynos4_pm_prepare() is a direct subset of exynos5250_pm_prepare(). This just confirms that it might be a good idea to just merge both functions into a single generic one.

Quoted 7 lines

arch/arm/mach-exynos/pm.c +1 -0

+
 static void exynos_pm_central_suspend(void)
 {
      unsigned long tmp;

@@ -295,6 +361,13 @@ static void exynos_pm_central_suspend(void)
 

arch/arm/mach-exynos/pm.c static void exynos_pm_central_suspend(void) · line 367

Quoted 6 lines

arch/arm/mach-exynos/pm.c +4 -0

 static int exynos_pm_suspend(void)
 {
+     if (pm_data->pm_suspend)
+             return pm_data->pm_suspend();
+     return -1;
+}

Do you need this three-liner? I believe you could just assign pm_data->pm_suspend directly to exynos_pm_syscore_ops.

Quoted 24 lines

arch/arm/mach-exynos/pm.c +14 -2

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

@@ -304,9 +377,20 @@ static int exynos_pm_suspend(void)
      tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
      pmu_raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);
 
-     if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-             exynos_cpu_save_register();
+     exynos_cpu_save_register();
+     return 0;
+}
+
+static int exynos5250_pm_suspend(void)
+{
+     unsigned long tmp;
+
+     exynos_pm_central_suspend();
+
+     /* Setting SEQ_OPTION register */
 

arch/arm/mach-exynos/pm.c static int exynos_pm_suspend(void) · line 395

Quoted 4 lines

arch/arm/mach-exynos/pm.c +2 -0

+     tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
+     pmu_raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);
      return 0;
 }

This function is exactly the same as exynos4_pm_suspend, but without the call to exynos_cpu_save_register(). Since originally this call was conditional, depending on CPU part number, is there any reason to split this function into SoC specific ones?

arch/arm/mach-exynos/pm.c static int exynos_pm_central_resume(void) · line 420

Quoted 7 lines

arch/arm/mach-exynos/pm.c +1 -0

@@ -333,39 +417,57 @@ static int exynos_pm_central_resume(void)
      return 0;
 }
 
+static void exynos_pm_set_release_retention(void)

nit: exynos_pm_release_retention() would sound better.

arch/arm/mach-exynos/pm.c static int exynos_pm_central_resume(void) · line 433

Quoted 13 lines

arch/arm/mach-exynos/pm.c +11 -0

+{
+     unsigned int i;
+
+     for (i = 0; (pm_data->release_ret_regs[i] != REG_TABLE_END); i++)
+             pmu_raw_writel(EXYNOS_WAKEUP_FROM_LOWPWR,
+                             pm_data->release_ret_regs[i]);
+}
+
 static void exynos_pm_resume(void)
 {
+     if (pm_data->pm_resume)
+             pm_data->pm_resume();
+}

Similarly as with exynos_pm_suspend().

Quoted 53 lines

arch/arm/mach-exynos/pm.c +22 -17

+
+static void exynos4_pm_resume(void)
+{
      if (exynos_pm_central_resume())
              goto early_wakeup;
 
-     if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-             exynos_cpu_restore_register();
+     exynos_cpu_restore_register();
 
      /* For release retention */
-
-     pmu_raw_writel((1 << 28), S5P_PAD_RET_MAUDIO_OPTION);
-     pmu_raw_writel((1 << 28), S5P_PAD_RET_GPIO_OPTION);
-     pmu_raw_writel((1 << 28), S5P_PAD_RET_UART_OPTION);
-     pmu_raw_writel((1 << 28), S5P_PAD_RET_MMCA_OPTION);
-     pmu_raw_writel((1 << 28), S5P_PAD_RET_MMCB_OPTION);
-     pmu_raw_writel((1 << 28), S5P_PAD_RET_EBIA_OPTION);
-     pmu_raw_writel((1 << 28), S5P_PAD_RET_EBIB_OPTION);
-
-     if (soc_is_exynos5250())
-             s3c_pm_do_restore(exynos5_sys_save,
-                     ARRAY_SIZE(exynos5_sys_save));
+     exynos_pm_set_release_retention();
 
      s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
-     if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
-             scu_enable(S5P_VA_SCU);
+     scu_enable(S5P_VA_SCU);
 
 early_wakeup:
 
      /* Clear SLEEP mode set in INFORM1 */
      pmu_raw_writel(0x0, S5P_INFORM1);
+}
+
+static void exynos5250_pm_resume(void)
+{
+     if (exynos_pm_central_resume())
+             goto early_wakeup;
 
-     return;
+     /* For release retention */
+     exynos_pm_set_release_retention();
+
+     s3c_pm_do_restore(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
+
+     s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+early_wakeup:
+
+     /* Clear SLEEP mode set in INFORM1 */

arch/arm/mach-exynos/pm.c static int exynos_pm_central_resume(void) · line 471

Quoted 2 lines

arch/arm/mach-exynos/pm.c +1 -0

+     pmu_raw_writel(0x0, S5P_INFORM1);
 }

Similar to exynos{4,5250}_pm_suspend(), this could be kept as a single generic function. exynos5_sys_save would be handled by .extra_save field of the pm_data struct and scu_enable() and exynos_cpu_restore_register() was already handled in a generic way, based on ARM core type, not soc_is_*().

arch/arm/mach-exynos/pm.c static struct notifier_block exynos_cpu_pm_notifier_block = { · line 573

Quoted 8 lines

arch/arm/mach-exynos/pm.c +1 -0

 static struct syscore_ops exynos_pm_syscore_ops = {

@@ -468,18 +570,64 @@ static struct notifier_block exynos_cpu_pm_notifier_block = {
      .notifier_call = exynos_cpu_pm_notifier,
 };
 
+static struct exynos_pm_data exynos4_pm_data = {

static const

arch/arm/mach-exynos/pm.c static struct notifier_block exynos_cpu_pm_notifier_block = { · line 583

Quoted 10 lines

arch/arm/mach-exynos/pm.c +10 -0

+     .wkup_irq       = exynos4_wkup_irq,
+     .wake_disable_mask = ((0xFF << 8) | (0x1F << 1)),
+     .release_ret_regs = exynos_release_ret_regs,
+     .pm_suspend     = exynos4_pm_suspend,
+     .pm_resume      = exynos4_pm_resume,
+     .pm_prepare     = exynos4_pm_prepare,
+     .cpu_suspend    = exynos4_cpu_suspend,
+};
+
+static struct exynos_pm_data exynos5250_pm_data = {

static const

arch/arm/mach-exynos/pm.c static struct notifier_block exynos_cpu_pm_notifier_block = { · line 596

Quoted 13 lines

arch/arm/mach-exynos/pm.c +13 -0

+     .wkup_irq       = exynos5250_wkup_irq,
+     .wake_disable_mask = ((0xFF << 8) | (0x1F << 1)),
+     .release_ret_regs = exynos_release_ret_regs,
+     .pm_suspend     = exynos5250_pm_suspend,
+     .pm_resume      = exynos5250_pm_resume,
+     .pm_prepare     = exynos5250_pm_prepare,
+     .cpu_suspend    = exynos5250_cpu_suspend,
+};
+
+static struct of_device_id exynos_pmu_of_device_ids[] = {
+     {
+             .compatible = "samsung,exynos4210-pmu",
+             .data = (void *)&exynos4_pm_data,

No need to cast if const pointers are used. + 3 more below.

Quoted 7 lines

arch/arm/mach-exynos/pm.c +7 -0

+     }, {
+             .compatible = "samsung,exynos4212-pmu",
+             .data = (void *)&exynos4_pm_data,
+     }, {
+             .compatible = "samsung,exynos4412-pmu",
+             .data = (void *)&exynos4_pm_data,
+     }, {

arch/arm/mach-exynos/pm.c static struct notifier_block exynos_cpu_pm_notifier_block = { · line 613

Quoted 10 lines

arch/arm/mach-exynos/pm.c +7 -0

+             .compatible = "samsung,exynos5250-pmu",
+             .data = (void *)&exynos5250_pm_data,
+     },
+     { /*sentinel*/ },
+};
+
 void __init exynos_pm_init(void)
 {
      u32 tmp;
+     const struct of_device_id *match;

nit: it would look better if this variable was added above tmp.

arch/arm/mach-exynos/pm.c static struct notifier_block exynos_cpu_pm_notifier_block = { · line 618

Quoted 5 lines

arch/arm/mach-exynos/pm.c +2 -0

      cpu_pm_register_notifier(&exynos_cpu_pm_notifier_block);
 
+     of_find_matching_node_and_match(NULL, exynos_pmu_of_device_ids, &match);
+

nit: superfluous blank line

arch/arm/mach-exynos/pm.c static struct notifier_block exynos_cpu_pm_notifier_block = { · line 623

Quoted 5 lines

arch/arm/mach-exynos/pm.c +5 -0

+     if (!match) {
+             pr_err("Failed to find PMU node\n");
+             return;
+     }
+     pm_data = (struct exynos_pm_data *) match->data;

No need to cast if const pointers are used.

Quoted 4 lines

arch/arm/mach-exynos/pm.c +1 -0

+
      /* Platform-specific GIC callback */
      gic_arch_extn.irq_set_wake = exynos_irq_set_wake;
 

arch/arm/mach-exynos/pm.c static struct notifier_block exynos_cpu_pm_notifier_block = { · line 630

Quoted 4 lines

arch/arm/mach-exynos/pm.c +1 -1

      /* All wakeup disable */
      tmp = pmu_raw_readl(S5P_WAKEUP_MASK);
-     tmp |= ((0xFF << 8) | (0x1F << 1));
+     tmp |= pm_data->wake_disable_mask;

Does this vary between SoCs?

Quoted 12 lines

arch/arm/mach-exynos/pm.c +0 -0

      pmu_raw_writel(tmp, S5P_WAKEUP_MASK);
 
      register_syscore_ops(&exynos_pm_syscore_ops);

arch/arm/mach-exynos/regs-pmu.h +0 -0

index 96a1569..30c0301 100644
@@ -21,6 +21,7 @@
 #define S5P_USE_STANDBY_WFI0                 (1 << 16)
 #define S5P_USE_STANDBY_WFE0                 (1 << 24)
 

arch/arm/mach-exynos/regs-pmu.h line 24

Quoted 1 line

arch/arm/mach-exynos/regs-pmu.h +1 -0

+#define EXYNOS_WAKEUP_FROM_LOWPWR            (1 << 28)

Is this really the real name of this bit, as specified in the datasheet?

Best regards,
Tomasz

Quoted 1 line · Tomasz Figa

On 25.07.2014 13:49, Vikas Sajjan wrote:

Quoted 7 lines

Quoted 6 lines

arch/arm/mach-exynos/pm.c +5 -0

+
+     /* Disable USE_RETENTION of JPEG_MEM_OPTION */
+     tmp = pmu_raw_readl(EXYNOS5_JPEG_MEM_OPTION);
+     tmp &= ~EXYNOS5_OPTION_USE_RETENTION;
+     pmu_raw_writel(tmp, EXYNOS5_JPEG_MEM_OPTION);

Quoted 4 lines

This looks completely like a copy paste from a vendor tree needed to workaround some issues in early revisions of the SoC. Are you sure this is still needed in production versions of the silicon used on boards supported in mainline?

This piece of code is NOT copy paste from my side, it is an already existing code. I just moved it into the function exynos5250_pm_prepare().

However I removed this piece of code and
made a common function for exynos4 and exynos5250, S2R works on 5250 well even without this piece of code.

Quoted 15 lines

Even if yes, Exynos4 handles such registers in PMU register array in pmu.c, so I wonder whether it shouldn't be moved there and allow handling of all Exynos SoCs uniformly in this file.

Quoted 9 lines

arch/arm/mach-exynos/pm.c +9 -0

+
+     exynos_pm_enter_sleep_mode();
+}
+
+static void exynos4_pm_prepare(void)
+{
+     /* Set wake-up mask registers */
+     exynos_pm_set_wakeup_mask();
+

Quoted 8 lines

arch/arm/mach-exynos/pm.c static int exynos_cpu_suspend(unsigned long arg) · line 340

Quoted 4 lines

arch/arm/mach-exynos/pm.c +4 -0

+     s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+     exynos_pm_enter_sleep_mode();
+}

In fact, exynos4_pm_prepare() is a direct subset of exynos5250_pm_prepare(). This just confirms that it might be a good idea to just merge both functions into a single generic one.

Right, can be merged into a common function which can be used for exynos4 and exynos5250.
But I am afraid we still need specific functions in case of 5420.

Quoted 6 lines

Quoted 6 lines

+
static void exynos_pm_central_suspend(void)

 {
      unsigned long tmp;

Quoted 4 lines

Quoted 4 lines

+

      /* Platform-specific GIC callback */
      gic_arch_extn.irq_set_wake = exynos_irq_set_wake;

Quoted 6 lines

arch/arm/mach-exynos/pm.c static struct notifier_block exynos_cpu_pm_notifier_block = { · line 630

Quoted 4 lines

      /* All wakeup disable */
      tmp = pmu_raw_readl(S5P_WAKEUP_MASK);

arch/arm/mach-exynos/pm.c +1 -1

-     tmp |= ((0xFF << 8) | (0x1F << 1));
+     tmp |= pm_data->wake_disable_mask;

Does this vary between SoCs?

Yes, It is different in case of 5420.

Quoted 2 lines · Vikas Sajjan

Quoted 1 line · Tomasz Figa

On 25.07.2014 13:49, Vikas Sajjan wrote:

[snip]

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

arch/arm/mach-exynos/pm.c +5 -0

+
+     /* Disable USE_RETENTION of JPEG_MEM_OPTION */
+     tmp = pmu_raw_readl(EXYNOS5_JPEG_MEM_OPTION);
+     tmp &= ~EXYNOS5_OPTION_USE_RETENTION;
+     pmu_raw_writel(tmp, EXYNOS5_JPEG_MEM_OPTION);

Quoted 8 lines

Quoted 4 lines

This looks completely like a copy paste from a vendor tree needed to workaround some issues in early revisions of the SoC. Are you sure this is still needed in production versions of the silicon used on boards supported in mainline?

This piece of code is NOT copy paste from my side, it is an already existing code. I just moved it into the function exynos5250_pm_prepare().

Sure, I'm aware of this. It might be good to know though if this is really necessary, as I don't think we want useless code.

Quoted 3 lines

However I removed this piece of code and
made a common function for exynos4 and exynos5250, S2R works on 5250 well even without this piece of code.

Thanks for checking this. I wonder if we could get some clarification about this from hardware guys. Kukjin, Jingoo, any ideas or people that might know what this is about?

For now, I believe it could be handled the same way as Exynos4, in PMU register array as I suggested in my previous reply (quoted below).

Quoted 24 lines

Quoted 23 lines

Even if yes, Exynos4 handles such registers in PMU register array in pmu.c, so I wonder whether it shouldn't be moved there and allow handling of all Exynos SoCs uniformly in this file.

arch/arm/mach-exynos/pm.c static int exynos_cpu_suspend(unsigned long arg) · line 350

Quoted 13 lines

arch/arm/mach-exynos/pm.c +13 -0

+
+     exynos_pm_enter_sleep_mode();
+}
+
+static void exynos4_pm_prepare(void)
+{
+     /* Set wake-up mask registers */
+     exynos_pm_set_wakeup_mask();
+
+     s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
+
+     exynos_pm_enter_sleep_mode();
+}

In fact, exynos4_pm_prepare() is a direct subset of exynos5250_pm_prepare(). This just confirms that it might be a good idea to just merge both functions into a single generic one.

Quoted 3 lines

Right, can be merged into a common function which can be used for exynos4 and exynos5250.
But I am afraid we still need specific functions in case of 5420.

Could you provide some details about Exynos5420 specific things?

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

+
static void exynos_pm_central_suspend(void)

 {
      unsigned long tmp;

[snip]

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

arch/arm/mach-exynos/pm.c +2 -1

+
      /* Platform-specific GIC callback */
      gic_arch_extn.irq_set_wake = exynos_irq_set_wake;
 
      /* All wakeup disable */
      tmp = pmu_raw_readl(S5P_WAKEUP_MASK);
-     tmp |= ((0xFF << 8) | (0x1F << 1));
+     tmp |= pm_data->wake_disable_mask;

Quoted 3 lines

Quoted 1 line

Does this vary between SoCs?

Yes, It is different in case of 5420.

Could you provide more information about the difference?

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