* [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
@ 2014-07-25 11:49 ` Vikas Sajjan
0 siblings, 0 replies; 14+ messages in thread
From: Vikas Sajjan @ 2014-07-25 11:49 UTC (permalink / raw)
To: linux-arm-kernel, linux-samsung-soc
Cc: kgene.kim, tomasz.figa, joshi, sajjan.linux, dianders, 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.
Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com>
---
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(-)
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index c4c6d98..1c875e5 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -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);
diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
index 96a1569..30c0301 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/arch/arm/mach-exynos/regs-pmu.h
@@ -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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
@ 2014-07-25 11:49 ` Vikas Sajjan
0 siblings, 0 replies; 14+ messages in thread
From: Vikas Sajjan @ 2014-07-25 11:49 UTC (permalink / raw)
To: linux-arm-kernel
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 <vikas.sajjan@samsung.com>
---
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(-)
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index c4c6d98..1c875e5 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -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@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);
diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
index 96a1569..30c0301 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/arch/arm/mach-exynos/regs-pmu.h
@@ -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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
2014-07-25 11:49 ` Vikas Sajjan
@ 2014-08-04 4:54 ` Vikas Sajjan
-1 siblings, 0 replies; 14+ messages in thread
From: Vikas Sajjan @ 2014-08-04 4:54 UTC (permalink / raw)
To: Kukjin Kim
Cc: Tomasz Figa, Vikas Sajjan, Doug Anderson, Vikas Sajjan,
linux-arm-kernel, linux-samsung-soc, Olof Johansson
Hi Kukjin,
On Fri, Jul 25, 2014 at 5:34 PM, Vikas Sajjan <vikas.sajjan@samsung.com> wrote:
> 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 <vikas.sajjan@samsung.com>
> ---
> 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.
>
Do you have any more comments.
> 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(-)
>
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index c4c6d98..1c875e5 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -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);
> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
> index 96a1569..30c0301 100644
> --- a/arch/arm/mach-exynos/regs-pmu.h
> +++ b/arch/arm/mach-exynos/regs-pmu.h
> @@ -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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
@ 2014-08-04 4:54 ` Vikas Sajjan
0 siblings, 0 replies; 14+ messages in thread
From: Vikas Sajjan @ 2014-08-04 4:54 UTC (permalink / raw)
To: linux-arm-kernel
Hi Kukjin,
On Fri, Jul 25, 2014 at 5:34 PM, Vikas Sajjan <vikas.sajjan@samsung.com> wrote:
> 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 <vikas.sajjan@samsung.com>
> ---
> 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.
>
Do you have any more comments.
> 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(-)
>
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index c4c6d98..1c875e5 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -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);
> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
> index 96a1569..30c0301 100644
> --- a/arch/arm/mach-exynos/regs-pmu.h
> +++ b/arch/arm/mach-exynos/regs-pmu.h
> @@ -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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
2014-07-25 11:49 ` Vikas Sajjan
@ 2014-08-05 12:24 ` Thomas Abraham
-1 siblings, 0 replies; 14+ messages in thread
From: Thomas Abraham @ 2014-08-05 12:24 UTC (permalink / raw)
To: Vikas Sajjan
Cc: linux-arm-kernel, linux-samsung-soc, Kukjin Kim, Tomasz Figa,
joshi, sajjan.linux, Doug Anderson
Hi Vikas,
On Fri, Jul 25, 2014 at 5:19 PM, Vikas Sajjan <vikas.sajjan@samsung.com> wrote:
> 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 <vikas.sajjan@samsung.com>
> ---
> 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(-)
>
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index c4c6d98..1c875e5 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -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
Exynos5 SoCs do not use an additional outer cache controller. So the
above #ifdef block can be dropped.
> + 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);
> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
> index 96a1569..30c0301 100644
> --- a/arch/arm/mach-exynos/regs-pmu.h
> +++ b/arch/arm/mach-exynos/regs-pmu.h
> @@ -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
>
Rest of the patch looks fine. With the fix for comment above,
Reviewed-by: Thomas Abraham <thomas.ab@samsung.com>
> --
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
@ 2014-08-05 12:24 ` Thomas Abraham
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Abraham @ 2014-08-05 12:24 UTC (permalink / raw)
To: linux-arm-kernel
Hi Vikas,
On Fri, Jul 25, 2014 at 5:19 PM, Vikas Sajjan <vikas.sajjan@samsung.com> wrote:
> 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 <vikas.sajjan@samsung.com>
> ---
> 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(-)
>
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index c4c6d98..1c875e5 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -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
Exynos5 SoCs do not use an additional outer cache controller. So the
above #ifdef block can be dropped.
> + 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);
> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
> index 96a1569..30c0301 100644
> --- a/arch/arm/mach-exynos/regs-pmu.h
> +++ b/arch/arm/mach-exynos/regs-pmu.h
> @@ -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
>
Rest of the patch looks fine. With the fix for comment above,
Reviewed-by: Thomas Abraham <thomas.ab@samsung.com>
> --
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
2014-07-25 11:49 ` Vikas Sajjan
@ 2014-08-05 19:50 ` Tomasz Figa
-1 siblings, 0 replies; 14+ messages in thread
From: Tomasz Figa @ 2014-08-05 19:50 UTC (permalink / raw)
To: Vikas Sajjan, linux-arm-kernel, linux-samsung-soc
Cc: kgene.kim, joshi, sajjan.linux, dianders
Hi Vikas,
Please see my comments inline.
On 25.07.2014 13:49, Vikas Sajjan wrote:
> 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]
> @@ -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;
> +}
I believe you could just pass pm_data->cpu_suspend to cpu_suspend(),
without this three-liner.
>
> +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();
You could just directly insert this code into exynos_suspend_enter()
instead of adding this two-liner.
> +}
>
> +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();
> +}
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).
> +
> +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));
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);
> +
> + /* 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.
> +
> + 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.
> +
> 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;
> +}
Do you need this three-liner? I believe you could just assign
pm_data->pm_suspend directly to exynos_pm_syscore_ops.
> +
> +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;
> }
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?
>
> @@ -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.
> +{
> + 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().
> +
> +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);
> }
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_*().
>
> 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
> + .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
> + .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.
> + }, {
> + .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;
nit: it would look better if this variable was added above tmp.
>
> 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
> + 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.
> +
> /* 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;
Does this vary between SoCs?
> pmu_raw_writel(tmp, S5P_WAKEUP_MASK);
>
> register_syscore_ops(&exynos_pm_syscore_ops);
> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
> index 96a1569..30c0301 100644
> --- a/arch/arm/mach-exynos/regs-pmu.h
> +++ b/arch/arm/mach-exynos/regs-pmu.h
> @@ -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)
Is this really the real name of this bit, as specified in the datasheet?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
@ 2014-08-05 19:50 ` Tomasz Figa
0 siblings, 0 replies; 14+ messages in thread
From: Tomasz Figa @ 2014-08-05 19:50 UTC (permalink / raw)
To: linux-arm-kernel
Hi Vikas,
Please see my comments inline.
On 25.07.2014 13:49, Vikas Sajjan wrote:
> 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]
> @@ -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;
> +}
I believe you could just pass pm_data->cpu_suspend to cpu_suspend(),
without this three-liner.
>
> +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();
You could just directly insert this code into exynos_suspend_enter()
instead of adding this two-liner.
> +}
>
> +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();
> +}
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).
> +
> +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));
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);
> +
> + /* 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.
> +
> + 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.
> +
> 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;
> +}
Do you need this three-liner? I believe you could just assign
pm_data->pm_suspend directly to exynos_pm_syscore_ops.
> +
> +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;
> }
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?
>
> @@ -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.
> +{
> + 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().
> +
> +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);
> }
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_*().
>
> 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
> + .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
> + .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.
> + }, {
> + .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;
nit: it would look better if this variable was added above tmp.
>
> 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
> + 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.
> +
> /* 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;
Does this vary between SoCs?
> pmu_raw_writel(tmp, S5P_WAKEUP_MASK);
>
> register_syscore_ops(&exynos_pm_syscore_ops);
> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
> index 96a1569..30c0301 100644
> --- a/arch/arm/mach-exynos/regs-pmu.h
> +++ b/arch/arm/mach-exynos/regs-pmu.h
> @@ -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)
Is this really the real name of this bit, as specified in the datasheet?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
2014-08-05 19:50 ` Tomasz Figa
@ 2014-08-06 12:58 ` Vikas Sajjan
-1 siblings, 0 replies; 14+ messages in thread
From: Vikas Sajjan @ 2014-08-06 12:58 UTC (permalink / raw)
To: Tomasz Figa
Cc: linux-arm-kernel, linux-samsung-soc, Kukjin Kim, sunil joshi,
Doug Anderson
Hi Tomasz,
On Wed, Aug 6, 2014 at 1:35 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Vikas,
>
> Please see my comments inline.
>
> On 25.07.2014 13:49, Vikas Sajjan wrote:
>> 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]
>
>> @@ -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;
>> +}
>
> I believe you could just pass pm_data->cpu_suspend to cpu_suspend(),
> without this three-liner.
>
OK.
>>
>> +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();
>
> You could just directly insert this code into exynos_suspend_enter()
> instead of adding this two-liner.
OK.
>
>> +}
>>
>> +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();
>> +}
>
> 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).
>
>> +
>> +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));
>
> 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);
>
OK.
>> +
>> + /* 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?
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.
>
> 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.
>
>> +
>> + 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.
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.
>
>> +
>> 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;
>> +}
>
> Do you need this three-liner? I believe you could just assign
> pm_data->pm_suspend directly to exynos_pm_syscore_ops.
>
OK.
>> +
>> +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;
>> }
>
> 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?
Can be made as single common function.
>
>>
>> @@ -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.
>
>> +{
>> + 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().
>
>> +
>> +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);
>> }
>
> 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_*().
OK.
>
>>
>> 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
>
>> + .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
>
>> + .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.
>
>> + }, {
>> + .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;
>
> nit: it would look better if this variable was added above tmp.
>
>>
>> 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
>
>> + 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.
>
>> +
>> /* 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;
>
> Does this vary between SoCs?
Yes, It is different in case of 5420.
>
>> pmu_raw_writel(tmp, S5P_WAKEUP_MASK);
>>
>> register_syscore_ops(&exynos_pm_syscore_ops);
>> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
>> index 96a1569..30c0301 100644
>> --- a/arch/arm/mach-exynos/regs-pmu.h
>> +++ b/arch/arm/mach-exynos/regs-pmu.h
>> @@ -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)
>
> Is this really the real name of this bit, as specified in the datasheet?
Yes, it is the real name. I referred exynos4 UM for this.
>
> Best regards,
> Tomasz
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
@ 2014-08-06 12:58 ` Vikas Sajjan
0 siblings, 0 replies; 14+ messages in thread
From: Vikas Sajjan @ 2014-08-06 12:58 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On Wed, Aug 6, 2014 at 1:35 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Vikas,
>
> Please see my comments inline.
>
> On 25.07.2014 13:49, Vikas Sajjan wrote:
>> 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]
>
>> @@ -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;
>> +}
>
> I believe you could just pass pm_data->cpu_suspend to cpu_suspend(),
> without this three-liner.
>
OK.
>>
>> +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();
>
> You could just directly insert this code into exynos_suspend_enter()
> instead of adding this two-liner.
OK.
>
>> +}
>>
>> +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();
>> +}
>
> 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).
>
>> +
>> +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));
>
> 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);
>
OK.
>> +
>> + /* 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?
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.
>
> 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.
>
>> +
>> + 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.
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.
>
>> +
>> 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;
>> +}
>
> Do you need this three-liner? I believe you could just assign
> pm_data->pm_suspend directly to exynos_pm_syscore_ops.
>
OK.
>> +
>> +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;
>> }
>
> 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?
Can be made as single common function.
>
>>
>> @@ -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.
>
>> +{
>> + 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().
>
>> +
>> +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);
>> }
>
> 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_*().
OK.
>
>>
>> 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
>
>> + .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
>
>> + .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.
>
>> + }, {
>> + .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;
>
> nit: it would look better if this variable was added above tmp.
>
>>
>> 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
>
>> + 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.
>
>> +
>> /* 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;
>
> Does this vary between SoCs?
Yes, It is different in case of 5420.
>
>> pmu_raw_writel(tmp, S5P_WAKEUP_MASK);
>>
>> register_syscore_ops(&exynos_pm_syscore_ops);
>> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
>> index 96a1569..30c0301 100644
>> --- a/arch/arm/mach-exynos/regs-pmu.h
>> +++ b/arch/arm/mach-exynos/regs-pmu.h
>> @@ -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)
>
> Is this really the real name of this bit, as specified in the datasheet?
Yes, it is the real name. I referred exynos4 UM for this.
>
> Best regards,
> Tomasz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
2014-08-06 12:58 ` Vikas Sajjan
@ 2014-08-06 13:12 ` Tomasz Figa
-1 siblings, 0 replies; 14+ messages in thread
From: Tomasz Figa @ 2014-08-06 13:12 UTC (permalink / raw)
To: Vikas Sajjan, Tomasz Figa
Cc: linux-arm-kernel, linux-samsung-soc, Kukjin Kim, sunil joshi,
Doug Anderson, Jingoo Han
On 06.08.2014 14:58, Vikas Sajjan wrote:
> On Wed, Aug 6, 2014 at 1:35 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> On 25.07.2014 13:49, Vikas Sajjan wrote:
[snip]
>
>>> +
>>> + /* 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?
>
> 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.
> 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).
>
>>
>> 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.
>>
>>> +
>>> + 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.
>
> 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?
>
>>
>>> +
>>> static void exynos_pm_central_suspend(void)
>>> {
>>> unsigned long tmp;
[snip]
>>> +
>>> /* 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;
>>
>> Does this vary between SoCs?
>
> Yes, It is different in case of 5420.
Could you provide more information about the difference?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
@ 2014-08-06 13:12 ` Tomasz Figa
0 siblings, 0 replies; 14+ messages in thread
From: Tomasz Figa @ 2014-08-06 13:12 UTC (permalink / raw)
To: linux-arm-kernel
On 06.08.2014 14:58, Vikas Sajjan wrote:
> On Wed, Aug 6, 2014 at 1:35 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> On 25.07.2014 13:49, Vikas Sajjan wrote:
[snip]
>
>>> +
>>> + /* 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?
>
> 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.
> 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).
>
>>
>> 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.
>>
>>> +
>>> + 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.
>
> 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?
>
>>
>>> +
>>> static void exynos_pm_central_suspend(void)
>>> {
>>> unsigned long tmp;
[snip]
>>> +
>>> /* 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;
>>
>> Does this vary between SoCs?
>
> Yes, It is different in case of 5420.
Could you provide more information about the difference?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
2014-08-06 13:12 ` Tomasz Figa
@ 2014-08-06 13:19 ` Vikas Sajjan
-1 siblings, 0 replies; 14+ messages in thread
From: Vikas Sajjan @ 2014-08-06 13:19 UTC (permalink / raw)
To: Tomasz Figa
Cc: Tomasz Figa, linux-arm-kernel, linux-samsung-soc, Kukjin Kim,
sunil joshi, Doug Anderson, Jingoo Han
Hi Tomasz,
On Wed, Aug 6, 2014 at 6:57 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> On 06.08.2014 14:58, Vikas Sajjan wrote:
>> On Wed, Aug 6, 2014 at 1:35 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>>> On 25.07.2014 13:49, Vikas Sajjan wrote:
>
> [snip]
>
>>
>>>> +
>>>> + /* 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?
>>
>> 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.
>
>> 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).
>
>>
>>>
>>> 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.
>>>
>>>> +
>>>> + 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.
>>
>> 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?
Please refer my post [1] for 5420 PMU and S2R Support
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/269956.html
>
>>
>>>
>>>> +
>>>> static void exynos_pm_central_suspend(void)
>>>> {
>>>> unsigned long tmp;
>
> [snip]
>
>>>> +
>>>> /* 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;
>>>
>>> Does this vary between SoCs?
>>
>> Yes, It is different in case of 5420.
>
> Could you provide more information about the difference?
In case of 5420, it is wake_disable_mask = (0x7F << 7) | (0x1F << 1)
>
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
@ 2014-08-06 13:19 ` Vikas Sajjan
0 siblings, 0 replies; 14+ messages in thread
From: Vikas Sajjan @ 2014-08-06 13:19 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On Wed, Aug 6, 2014 at 6:57 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> On 06.08.2014 14:58, Vikas Sajjan wrote:
>> On Wed, Aug 6, 2014 at 1:35 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>>> On 25.07.2014 13:49, Vikas Sajjan wrote:
>
> [snip]
>
>>
>>>> +
>>>> + /* 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?
>>
>> 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.
>
>> 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).
>
>>
>>>
>>> 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.
>>>
>>>> +
>>>> + 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.
>>
>> 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?
Please refer my post [1] for 5420 PMU and S2R Support
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/269956.html
>
>>
>>>
>>>> +
>>>> static void exynos_pm_central_suspend(void)
>>>> {
>>>> unsigned long tmp;
>
> [snip]
>
>>>> +
>>>> /* 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;
>>>
>>> Does this vary between SoCs?
>>
>> Yes, It is different in case of 5420.
>
> Could you provide more information about the difference?
In case of 5420, it is wake_disable_mask = (0x7F << 7) | (0x1F << 1)
>
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-08-06 13:19 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-25 11:49 [PATCH v2] ARM: EXYNOS: Refactor the pm code to use DT based lookup Vikas Sajjan
2014-07-25 11:49 ` Vikas Sajjan
2014-08-04 4:54 ` Vikas Sajjan
2014-08-04 4:54 ` Vikas Sajjan
2014-08-05 12:24 ` Thomas Abraham
2014-08-05 12:24 ` Thomas Abraham
2014-08-05 19:50 ` Tomasz Figa
2014-08-05 19:50 ` Tomasz Figa
2014-08-06 12:58 ` Vikas Sajjan
2014-08-06 12:58 ` Vikas Sajjan
2014-08-06 13:12 ` Tomasz Figa
2014-08-06 13:12 ` Tomasz Figa
2014-08-06 13:19 ` Vikas Sajjan
2014-08-06 13:19 ` Vikas Sajjan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.