[PATCH v3 0/2] Refactor the pm code to use DT based lookup

[PATCH v3 0/2] Refactor the pm code to use DT based lookup

[Vikas Sajjan]

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

While at it, move exynos5250 specific disabling of JPEG USE_RETENTION to pmu.c

changes since v2:
	- Addressed Tomasz Figa's comments to unify the functions for exynos4
	and exynos5250.
	- Added new patch to move exynos5250 specific disabling of 
		JPEG USE_RETENTION to pmu.c 
	- Addressed comment from Thomas Abraham.
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

Vikas Sajjan (2):
  ARM: EXYNOS: Move Disabling of JPEG USE_RETENTION for exynos5250 to
    pmu.c
  ARM: EXYNOS: Refactor the pm code to use DT based lookup

 arch/arm/mach-exynos/pm.c       |  173 ++++++++++++++++++++++++++++-----------
 arch/arm/mach-exynos/pmu.c      |    6 ++
 arch/arm/mach-exynos/regs-pmu.h |    1 +
 3 files changed, 132 insertions(+), 48 deletions(-)

-- 
1.7.9.5

[PATCH v3 1/2] ARM: EXYNOS: Move Disabling of JPEG USE_RETENTION for exynos5250 to pmu.c

[Vikas Sajjan]

Move the Disabling of JPEG USE_RETENTION for exynos5250 to pmu.c to make way for
refactoring of pm.c and to create common functions across exynos4 and
exynos5250.

Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com>
---
 arch/arm/mach-exynos/pm.c  |    7 +------
 arch/arm/mach-exynos/pmu.c |    6 ++++++
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index c4c6d98..fdd68c2 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -265,13 +265,8 @@ static void exynos_pm_prepare(void)
 
 	s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
-	if (soc_is_exynos5250()) {
+	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);
-	}
 
 	/* Set value of power down register for sleep mode */
 
diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
index ff9d23f..6021adb 100644
--- a/arch/arm/mach-exynos/pmu.c
+++ b/arch/arm/mach-exynos/pmu.c
@@ -389,6 +389,7 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode)
 static int __init exynos_pmu_init(void)
 {
 	unsigned int value;
+	unsigned int tmp;
 
 	exynos_pmu_config = exynos4210_pmu_config;
 
@@ -411,6 +412,11 @@ static int __init exynos_pmu_init(void)
 		value &= ~EXYNOS5_SYS_WDTRESET;
 		pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST);
 
+		/* 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_pmu_config = exynos5250_pmu_config;
 		pr_info("EXYNOS5250 PMU Initialize\n");
 	} else {
-- 
1.7.9.5

[PATCH v3 2/2] ARM: EXYNOS: Refactor the pm code to use DT based lookup

[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>
---
 arch/arm/mach-exynos/pm.c       |  168 +++++++++++++++++++++++++++++----------
 arch/arm/mach-exynos/regs-pmu.h |    1 +
 2 files changed, 126 insertions(+), 43 deletions(-)

diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index fdd68c2..b6690c4 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,21 @@ static struct sleep_save exynos_core_save[] = {
 	SAVE_ITEM(S5P_SROM_BC3),
 };
 
+struct exynos_pm_data {
+	const struct exynos_wkup_irq *wkup_irq;
+	struct sleep_save *extra_save;
+	int num_extra_save;
+	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 +94,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) {
@@ -239,15 +266,8 @@ static void exynos_cpu_restore_register(void)
 		      : "cc");
 }
 
-static int exynos_cpu_suspend(unsigned long arg)
+static int exynos_cpu_do_idle(void)
 {
-#ifdef CONFIG_CACHE_L2X0
-	outer_flush_all();
-#endif
-
-	if (soc_is_exynos5250())
-		flush_cache_all();
-
 	/* issue the standby signal into the pm unit. */
 	cpu_do_idle();
 
@@ -255,29 +275,43 @@ static int exynos_cpu_suspend(unsigned long arg)
 	return 1; /* Aborting suspend */
 }
 
-static void exynos_pm_prepare(void)
+static int exynos_cpu_suspend(unsigned long arg)
+{
+	flush_cache_all();
+	outer_flush_all();
+	return exynos_cpu_do_idle();
+}
+static void exynos_pm_set_wakeup_mask(void)
 {
-	unsigned int tmp;
-
 	/* 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));
-
+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 exynos_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));
+
+	 if (pm_data->extra_save)
+		s3c_pm_do_save(pm_data->extra_save,
+				pm_data->num_extra_save);
+
+	exynos_pm_enter_sleep_mode();
+}
+
 static void exynos_pm_central_suspend(void)
 {
 	unsigned long tmp;
@@ -328,6 +362,15 @@ static int exynos_pm_central_resume(void)
 	return 0;
 }
 
+static void exynos_pm_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 (exynos_pm_central_resume())
@@ -337,18 +380,11 @@ static void exynos_pm_resume(void)
 		exynos_cpu_restore_register();
 
 	/* For release retention */
+	exynos_pm_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));
+	if (pm_data->extra_save)
+		s3c_pm_do_restore_core(pm_data->extra_save,
+					pm_data->num_extra_save);
 
 	s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
@@ -359,15 +395,8 @@ early_wakeup:
 
 	/* Clear SLEEP mode set in INFORM1 */
 	pmu_raw_writel(0x0, S5P_INFORM1);
-
-	return;
 }
 
-static struct syscore_ops exynos_pm_syscore_ops = {
-	.suspend	= exynos_pm_suspend,
-	.resume		= exynos_pm_resume,
-};
-
 /*
  * Suspend Ops
  */
@@ -391,11 +420,12 @@ static int exynos_suspend_enter(suspend_state_t state)
 	}
 
 	s3c_pm_save_uarts();
-	exynos_pm_prepare();
+	if (pm_data->pm_prepare)
+		pm_data->pm_prepare();
 	flush_cache_all();
 	s3c_pm_check_store();
 
-	ret = cpu_suspend(0, exynos_cpu_suspend);
+	ret = cpu_suspend(0, pm_data->cpu_suspend);
 	if (ret)
 		return ret;
 
@@ -463,20 +493,72 @@ static struct notifier_block exynos_cpu_pm_notifier_block = {
 	.notifier_call = exynos_cpu_pm_notifier,
 };
 
+static const 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	= exynos_pm_suspend,
+	.pm_resume	= exynos_pm_resume,
+	.pm_prepare	= exynos_pm_prepare,
+	.cpu_suspend	= exynos_cpu_suspend,
+};
+
+static const 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,
+	.extra_save	= exynos5_sys_save,
+	.num_extra_save	= ARRAY_SIZE(exynos5_sys_save),
+	.pm_suspend	= exynos_pm_suspend,
+	.pm_resume	= exynos_pm_resume,
+	.pm_prepare	= exynos_pm_prepare,
+	.cpu_suspend	= exynos_cpu_suspend,
+};
+
+static struct of_device_id exynos_pmu_of_device_ids[] = {
+	{
+		.compatible = "samsung,exynos4210-pmu",
+		.data = &exynos4_pm_data,
+	}, {
+		.compatible = "samsung,exynos4212-pmu",
+		.data = &exynos4_pm_data,
+	}, {
+		.compatible = "samsung,exynos4412-pmu",
+		.data = &exynos4_pm_data,
+	}, {
+		.compatible = "samsung,exynos5250-pmu",
+		.data = &exynos5250_pm_data,
+	},
+	{ /*sentinel*/ },
+};
+
+static struct syscore_ops exynos_pm_syscore_ops;
+
 void __init exynos_pm_init(void)
 {
+	const struct of_device_id *match;
 	u32 tmp;
 
 	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);
 
+	exynos_pm_syscore_ops.suspend	= pm_data->pm_suspend;
+	exynos_pm_syscore_ops.resume	= pm_data->pm_resume;
+
 	register_syscore_ops(&exynos_pm_syscore_ops);
 	suspend_set_ops(&exynos_suspend_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

[PATCH v3 0/2] Refactor the pm code to use DT based lookup

[Vikas Sajjan]

Hi All,

On Thu, Aug 7, 2014 at 5:29 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, move exynos5250 specific disabling of JPEG USE_RETENTION to pmu.c
>
> changes since v2:
>         - Addressed Tomasz Figa's comments to unify the functions for exynos4
>         and exynos5250.
>         - Added new patch to move exynos5250 specific disabling of
>                 JPEG USE_RETENTION to pmu.c
>         - Addressed comment from Thomas Abraham.
> 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
>
> Vikas Sajjan (2):
>   ARM: EXYNOS: Move Disabling of JPEG USE_RETENTION for exynos5250 to
>     pmu.c
>   ARM: EXYNOS: Refactor the pm code to use DT based lookup
>
>  arch/arm/mach-exynos/pm.c       |  173 ++++++++++++++++++++++++++++-----------
>  arch/arm/mach-exynos/pmu.c      |    6 ++
>  arch/arm/mach-exynos/regs-pmu.h |    1 +
>  3 files changed, 132 insertions(+), 48 deletions(-)


Anymore  comments on this series...


>
> --
> 1.7.9.5
>

[PATCH v3 1/2] ARM: EXYNOS: Move Disabling of JPEG USE_RETENTION for exynos5250 to pmu.c

[Tomasz Figa]

Hi Vikas,

On 07.08.2014 13:59, Vikas Sajjan wrote:
> Move the Disabling of JPEG USE_RETENTION for exynos5250 to pmu.c to make way for
> refactoring of pm.c and to create common functions across exynos4 and
> exynos5250.
> 
> Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com>
> ---
>  arch/arm/mach-exynos/pm.c  |    7 +------
>  arch/arm/mach-exynos/pmu.c |    6 ++++++
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index c4c6d98..fdd68c2 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -265,13 +265,8 @@ static void exynos_pm_prepare(void)
>  
>  	s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
>  
> -	if (soc_is_exynos5250()) {
> +	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);
> -	}
>  
>  	/* Set value of power down register for sleep mode */
>  
> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
> index ff9d23f..6021adb 100644
> --- a/arch/arm/mach-exynos/pmu.c
> +++ b/arch/arm/mach-exynos/pmu.c
> @@ -389,6 +389,7 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode)
>  static int __init exynos_pmu_init(void)
>  {
>  	unsigned int value;
> +	unsigned int tmp;
>  
>  	exynos_pmu_config = exynos4210_pmu_config;
>  
> @@ -411,6 +412,11 @@ static int __init exynos_pmu_init(void)
>  		value &= ~EXYNOS5_SYS_WDTRESET;
>  		pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST);
>  
> +		/* 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);
> +

Note that this code is executed once at system bootup. Is this register
preserved across a suspend/resume cycle?

Anyway, what I suggested in my comment to previous revision of this
series was stuffing this thing into the PMU configuration array. As you
can see in the arrays for Exynos4 SoCs, they just hardcode the
MEM_OPTION registers to constant values, because I believe the only
field from those registers with non-zero value in practice is
EXYNOS5_OPTION_USE_RETENTION (or 0x10 used directly in PMU arrays for
Exynos4 SoCs). This would also cover my comment above, because the
arrays are written to the PMU every time a low power state is being entered.

Best regards,
Tomasz

[PATCH v3 2/2] ARM: EXYNOS: Refactor the pm code to use DT based lookup

[Tomasz Figa]

Hi Vikas,

On 07.08.2014 13:59, 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.
> 
> Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com>
> ---
>  arch/arm/mach-exynos/pm.c       |  168 +++++++++++++++++++++++++++++----------
>  arch/arm/mach-exynos/regs-pmu.h |    1 +
>  2 files changed, 126 insertions(+), 43 deletions(-)
> 

Reviewed-by: Tomasz Figa <t.figa@samsung.com>

Best regards,
Tomas

[PATCH v3 1/2] ARM: EXYNOS: Move Disabling of JPEG USE_RETENTION for exynos5250 to pmu.c

[Vikas Sajjan]

Hi Tomasz,

On Mon, Aug 18, 2014 at 5:42 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Vikas,
>
> On 07.08.2014 13:59, Vikas Sajjan wrote:
>> Move the Disabling of JPEG USE_RETENTION for exynos5250 to pmu.c to make way for
>> refactoring of pm.c and to create common functions across exynos4 and
>> exynos5250.
>>
>> Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com>
>> ---
>>  arch/arm/mach-exynos/pm.c  |    7 +------
>>  arch/arm/mach-exynos/pmu.c |    6 ++++++
>>  2 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
>> index c4c6d98..fdd68c2 100644
>> --- a/arch/arm/mach-exynos/pm.c
>> +++ b/arch/arm/mach-exynos/pm.c
>> @@ -265,13 +265,8 @@ static void exynos_pm_prepare(void)
>>
>>       s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
>>
>> -     if (soc_is_exynos5250()) {
>> +     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);
>> -     }
>>
>>       /* Set value of power down register for sleep mode */
>>
>> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
>> index ff9d23f..6021adb 100644
>> --- a/arch/arm/mach-exynos/pmu.c
>> +++ b/arch/arm/mach-exynos/pmu.c
>> @@ -389,6 +389,7 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode)
>>  static int __init exynos_pmu_init(void)
>>  {
>>       unsigned int value;
>> +     unsigned int tmp;
>>
>>       exynos_pmu_config = exynos4210_pmu_config;
>>
>> @@ -411,6 +412,11 @@ static int __init exynos_pmu_init(void)
>>               value &= ~EXYNOS5_SYS_WDTRESET;
>>               pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST);
>>
>> +             /* 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);
>> +
>
> Note that this code is executed once at system bootup. Is this register
> preserved across a suspend/resume cycle?
>
> Anyway, what I suggested in my comment to previous revision of this
> series was stuffing this thing into the PMU configuration array. As you
> can see in the arrays for Exynos4 SoCs, they just hardcode the
> MEM_OPTION registers to constant values, because I believe the only
> field from those registers with non-zero value in practice is
> EXYNOS5_OPTION_USE_RETENTION (or 0x10 used directly in PMU arrays for
> Exynos4 SoCs). This would also cover my comment above, because the
> arrays are written to the PMU every time a low power state is being entered.
>

Is this what you meant,

diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
index 6021adb..67b70fd 100644
--- a/arch/arm/mach-exynos/pmu.c
+++ b/arch/arm/mach-exynos/pmu.c
@@ -264,6 +264,7 @@ static const struct exynos_pmu_conf
exynos5250_pmu_config[] = {
        { EXYNOS5_INTRAM_MEM_SYS_PWR_REG,               { 0x3, 0x0, 0x0} },
        { EXYNOS5_INTROM_MEM_SYS_PWR_REG,               { 0x3, 0x0, 0x0} },
        { EXYNOS5_JPEG_MEM_SYS_PWR_REG,                 { 0x3, 0x0, 0x0} },
+       { EXYNOS5_JPEG_MEM_OPTION,                      { 0x10, 0x10, 0x0} },
        { EXYNOS5_HSI_MEM_SYS_PWR_REG,                  { 0x3, 0x0, 0x0} },
        { EXYNOS5_MCUIOP_MEM_SYS_PWR_REG,               { 0x3, 0x0, 0x0} },
        { EXYNOS5_SATA_MEM_SYS_PWR_REG,                 { 0x3, 0x0, 0x0} },
@@ -412,11 +413,6 @@ static int __init exynos_pmu_init(void)
                value &= ~EXYNOS5_SYS_WDTRESET;
                pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST);

-               /* 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_pmu_config = exynos5250_pmu_config;
                pr_info("EXYNOS5250 PMU Initialize\n");
        } else {

for AFTR and LPA mode, i still maintain 0x10 like any other
XXX_XXX_MEM_OPTION register,
but for sleep mode, I am disabling USE_RETENTION of JPEG_MEM_OPTION as required.


Hope this is fine.

I tested with the above change on snow, S2R works well.



> Best regards,
> Tomasz

[PATCH v3 1/2] ARM: EXYNOS: Move Disabling of JPEG USE_RETENTION for exynos5250 to pmu.c

[Tomasz Figa]

On 18.08.2014 15:21, Vikas Sajjan wrote:
> Hi Tomasz,
> 
> On Mon, Aug 18, 2014 at 5:42 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> Hi Vikas,
>>
>> On 07.08.2014 13:59, Vikas Sajjan wrote:
>>> Move the Disabling of JPEG USE_RETENTION for exynos5250 to pmu.c to make way for
>>> refactoring of pm.c and to create common functions across exynos4 and
>>> exynos5250.
>>>
>>> Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com>
>>> ---
>>>  arch/arm/mach-exynos/pm.c  |    7 +------
>>>  arch/arm/mach-exynos/pmu.c |    6 ++++++
>>>  2 files changed, 7 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
>>> index c4c6d98..fdd68c2 100644
>>> --- a/arch/arm/mach-exynos/pm.c
>>> +++ b/arch/arm/mach-exynos/pm.c
>>> @@ -265,13 +265,8 @@ static void exynos_pm_prepare(void)
>>>
>>>       s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
>>>
>>> -     if (soc_is_exynos5250()) {
>>> +     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);
>>> -     }
>>>
>>>       /* Set value of power down register for sleep mode */
>>>
>>> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
>>> index ff9d23f..6021adb 100644
>>> --- a/arch/arm/mach-exynos/pmu.c
>>> +++ b/arch/arm/mach-exynos/pmu.c
>>> @@ -389,6 +389,7 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode)
>>>  static int __init exynos_pmu_init(void)
>>>  {
>>>       unsigned int value;
>>> +     unsigned int tmp;
>>>
>>>       exynos_pmu_config = exynos4210_pmu_config;
>>>
>>> @@ -411,6 +412,11 @@ static int __init exynos_pmu_init(void)
>>>               value &= ~EXYNOS5_SYS_WDTRESET;
>>>               pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST);
>>>
>>> +             /* 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);
>>> +
>>
>> Note that this code is executed once at system bootup. Is this register
>> preserved across a suspend/resume cycle?
>>
>> Anyway, what I suggested in my comment to previous revision of this
>> series was stuffing this thing into the PMU configuration array. As you
>> can see in the arrays for Exynos4 SoCs, they just hardcode the
>> MEM_OPTION registers to constant values, because I believe the only
>> field from those registers with non-zero value in practice is
>> EXYNOS5_OPTION_USE_RETENTION (or 0x10 used directly in PMU arrays for
>> Exynos4 SoCs). This would also cover my comment above, because the
>> arrays are written to the PMU every time a low power state is being entered.
>>
> 
> Is this what you meant,
> 
> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
> index 6021adb..67b70fd 100644
> --- a/arch/arm/mach-exynos/pmu.c
> +++ b/arch/arm/mach-exynos/pmu.c
> @@ -264,6 +264,7 @@ static const struct exynos_pmu_conf
> exynos5250_pmu_config[] = {
>         { EXYNOS5_INTRAM_MEM_SYS_PWR_REG,               { 0x3, 0x0, 0x0} },
>         { EXYNOS5_INTROM_MEM_SYS_PWR_REG,               { 0x3, 0x0, 0x0} },
>         { EXYNOS5_JPEG_MEM_SYS_PWR_REG,                 { 0x3, 0x0, 0x0} },
> +       { EXYNOS5_JPEG_MEM_OPTION,                      { 0x10, 0x10, 0x0} },
>         { EXYNOS5_HSI_MEM_SYS_PWR_REG,                  { 0x3, 0x0, 0x0} },
>         { EXYNOS5_MCUIOP_MEM_SYS_PWR_REG,               { 0x3, 0x0, 0x0} },
>         { EXYNOS5_SATA_MEM_SYS_PWR_REG,                 { 0x3, 0x0, 0x0} },
> @@ -412,11 +413,6 @@ static int __init exynos_pmu_init(void)
>                 value &= ~EXYNOS5_SYS_WDTRESET;
>                 pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST);
> 
> -               /* 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_pmu_config = exynos5250_pmu_config;
>                 pr_info("EXYNOS5250 PMU Initialize\n");
>         } else {
> 
> for AFTR and LPA mode, i still maintain 0x10 like any other
> XXX_XXX_MEM_OPTION register,
> but for sleep mode, I am disabling USE_RETENTION of JPEG_MEM_OPTION as required.
> 
> 
> Hope this is fine.
> 
> I tested with the above change on snow, S2R works well.

Yes, looks good, thanks.

Best regards,
Tomasz