[PATCH v2 0/3] AT91 pm improvements for 3.20

[PATCH v2 0/3] AT91 pm improvements for 3.20

[Wenyou Yang]

Hi Nicolas,

This patch set is to add the pm support for the ARMv7 SoCs.

It is based on the following patch set,
	[PATCH v4 00/12] AT91 pm cleanup for 3.20

Changes in v2.0
 - Drop patch to enable the l2 cache controller.
 - Remove patches to disable the mpddrc and DDR clock.
 - Remove patch to change to get the memory type.
 - Fix some commments

Patrice Vilchez (1):
  pm: at91: pm_suspend: MOR register KEY was missing

Wenyou Yang (2):
  pm: at91: pm_suspend: add the WFI support for ARMv7
  pm: at91: add disable/enable the L1/L2 cache while suspend/resume

 arch/arm/mach-at91/pm.c         |   12 +++
 arch/arm/mach-at91/pm_suspend.S |  158 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 169 insertions(+), 1 deletion(-)

-- 
1.7.9.5

[PATCH v2 0/3] AT91 pm improvements for 3.20

[Wenyou Yang]

Hi Nicolas,

This patch set is to add the pm support for the ARMv7 SoCs.

It is based on the following patch set,
	[PATCH v4 00/12] AT91 pm cleanup for 3.20

Changes in v2.0
 - Drop patch to enable the l2 cache controller.
 - Remove patches to disable the mpddrc and DDR clock.
 - Remove patch to change to get the memory type.
 - Fix some commments

Patrice Vilchez (1):
  pm: at91: pm_suspend: MOR register KEY was missing

Wenyou Yang (2):
  pm: at91: pm_suspend: add the WFI support for ARMv7
  pm: at91: add disable/enable the L1/L2 cache while suspend/resume

 arch/arm/mach-at91/pm.c         |   12 +++
 arch/arm/mach-at91/pm_suspend.S |  158 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 169 insertions(+), 1 deletion(-)

-- 
1.7.9.5

[PATCH v2 1/3] pm: at91: pm_suspend: add the WFI support for ARMv7

[Wenyou Yang]

Add the WFI support to put the cpu to the sleep state.

In the meanwhile, disable the processor's clock.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 arch/arm/mach-at91/pm_suspend.S |   49 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
index 122a3f1..aed6903 100644
--- a/arch/arm/mach-at91/pm_suspend.S
+++ b/arch/arm/mach-at91/pm_suspend.S
@@ -53,6 +53,53 @@ mode	.req	r6
 	beq	1b
 	.endm
 
+/*
+ * Put the processor to enter the WFI state
+ */
+	.macro _do_wfi
+
+#if defined(CONFIG_CPU_V7)
+	/*
+	 * Execute an ISB instruction to flush the pipeline to ensure
+	 * that all of operations have beem completed.
+	 */
+	isb
+
+	dsb
+	dmb
+
+	/* Disable the processor's clock */
+	mov	tmp1, #AT91_PMC_PCK
+	str	tmp1, [pmc, #AT91_PMC_SCDR]
+
+	wfi
+
+	/*
+	 * CPU can speculatively prefetch the instructions
+	 * so add NOPs after WFI. Sixteen NOPs as Cortex-A5 pipeline.
+	 */
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+#else
+	mcr	p15, 0, tmp1, c7, c0, 4
+#endif
+
+	.endm
+
 	.text
 
 /*
@@ -181,7 +228,7 @@ sdr_sr_done:
 
 skip_disable_main_clock:
 	/* Wait for interrupt */
-	mcr	p15, 0, tmp1, c7, c0, 4
+	_do_wfi
 
 	tst	mode, #AT91_PM_SLOW_CLOCK
 	beq	skip_enable_main_clock
-- 
1.7.9.5

[PATCH v2 1/3] pm: at91: pm_suspend: add the WFI support for ARMv7

[Wenyou Yang]

Add the WFI support to put the cpu to the sleep state.

In the meanwhile, disable the processor's clock.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 arch/arm/mach-at91/pm_suspend.S |   49 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
index 122a3f1..aed6903 100644
--- a/arch/arm/mach-at91/pm_suspend.S
+++ b/arch/arm/mach-at91/pm_suspend.S
@@ -53,6 +53,53 @@ mode	.req	r6
 	beq	1b
 	.endm
 
+/*
+ * Put the processor to enter the WFI state
+ */
+	.macro _do_wfi
+
+#if defined(CONFIG_CPU_V7)
+	/*
+	 * Execute an ISB instruction to flush the pipeline to ensure
+	 * that all of operations have beem completed.
+	 */
+	isb
+
+	dsb
+	dmb
+
+	/* Disable the processor's clock */
+	mov	tmp1, #AT91_PMC_PCK
+	str	tmp1, [pmc, #AT91_PMC_SCDR]
+
+	wfi
+
+	/*
+	 * CPU can speculatively prefetch the instructions
+	 * so add NOPs after WFI. Sixteen NOPs as Cortex-A5 pipeline.
+	 */
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+	nop
+#else
+	mcr	p15, 0, tmp1, c7, c0, 4
+#endif
+
+	.endm
+
 	.text
 
 /*
@@ -181,7 +228,7 @@ sdr_sr_done:
 
 skip_disable_main_clock:
 	/* Wait for interrupt */
-	mcr	p15, 0, tmp1, c7, c0, 4
+	_do_wfi
 
 	tst	mode, #AT91_PM_SLOW_CLOCK
 	beq	skip_enable_main_clock
-- 
1.7.9.5

[PATCH v2 2/3] pm: at91: pm_suspend: MOR register KEY was missing

[Wenyou Yang]

From: Patrice Vilchez <patrice.vilchez@atmel.com>

Because writing the MOR register requires the PASSWD(0x37),
if missed, the write operation will be aborted.

Signed-off-by: Patrice Vilchez <patrice.vilchez@atmel.com>
---
 arch/arm/mach-at91/pm_suspend.S |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
index aed6903..311cc23 100644
--- a/arch/arm/mach-at91/pm_suspend.S
+++ b/arch/arm/mach-at91/pm_suspend.S
@@ -224,6 +224,7 @@ sdr_sr_done:
 	/* Turn off the main oscillator */
 	ldr	tmp1, [pmc, #AT91_CKGR_MOR]
 	bic	tmp1, tmp1, #AT91_PMC_MOSCEN
+	orr	tmp1, tmp1, #AT91_PMC_KEY
 	str	tmp1, [pmc, #AT91_CKGR_MOR]
 
 skip_disable_main_clock:
@@ -236,6 +237,7 @@ skip_disable_main_clock:
 	/* Turn on the main oscillator */
 	ldr	tmp1, [pmc, #AT91_CKGR_MOR]
 	orr	tmp1, tmp1, #AT91_PMC_MOSCEN
+	orr	tmp1, tmp1, #AT91_PMC_KEY
 	str	tmp1, [pmc, #AT91_CKGR_MOR]
 
 	wait_moscrdy
-- 
1.7.9.5

[PATCH v2 2/3] pm: at91: pm_suspend: MOR register KEY was missing

[Wenyou Yang]

From: Patrice Vilchez <patrice.vilchez@atmel.com>

Because writing the MOR register requires the PASSWD(0x37),
if missed, the write operation will be aborted.

Signed-off-by: Patrice Vilchez <patrice.vilchez@atmel.com>
---
 arch/arm/mach-at91/pm_suspend.S |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
index aed6903..311cc23 100644
--- a/arch/arm/mach-at91/pm_suspend.S
+++ b/arch/arm/mach-at91/pm_suspend.S
@@ -224,6 +224,7 @@ sdr_sr_done:
 	/* Turn off the main oscillator */
 	ldr	tmp1, [pmc, #AT91_CKGR_MOR]
 	bic	tmp1, tmp1, #AT91_PMC_MOSCEN
+	orr	tmp1, tmp1, #AT91_PMC_KEY
 	str	tmp1, [pmc, #AT91_CKGR_MOR]
 
 skip_disable_main_clock:
@@ -236,6 +237,7 @@ skip_disable_main_clock:
 	/* Turn on the main oscillator */
 	ldr	tmp1, [pmc, #AT91_CKGR_MOR]
 	orr	tmp1, tmp1, #AT91_PMC_MOSCEN
+	orr	tmp1, tmp1, #AT91_PMC_KEY
 	str	tmp1, [pmc, #AT91_CKGR_MOR]
 
 	wait_moscrdy
-- 
1.7.9.5

[PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Wenyou Yang]

For the sama5, disable L1 D-cache and L2 cache before the cpu go to wfi,
after wakeing up, enable L1 D-cache and L2 cache.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 arch/arm/mach-at91/pm.c         |   12 +++++
 arch/arm/mach-at91/pm_suspend.S |  107 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index c547038..3442f80e 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -40,6 +40,12 @@ static struct {
 	int memctrl;
 } at91_pm_data;
 
+void __weak at91_disable_l1_l2_cache(void) {}
+void __weak at91_enable_l1_l2_cache(void) {}
+
+void __iomem *at91_l2cc_base;
+EXPORT_SYMBOL_GPL(at91_l2cc_base);
+
 static int at91_pm_valid_state(suspend_state_t state)
 {
 	switch (state) {
@@ -141,8 +147,14 @@ static void at91_pm_suspend(suspend_state_t state)
 	pm_data |= (state == PM_SUSPEND_MEM) ?
 				AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
 
+	/* Disable L1 D-cache and L2 cache */
+	at91_disable_l1_l2_cache();
+
 	at91_suspend_sram_fn(at91_pmc_base, at91_ramc_base[0],
 				at91_ramc_base[1], pm_data);
+
+	/* Enable L1 D-cache and L2 cache */
+	at91_enable_l1_l2_cache();
 }
 
 static int at91_pm_enter(suspend_state_t state)
diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
index 311cc23..02d4e56 100644
--- a/arch/arm/mach-at91/pm_suspend.S
+++ b/arch/arm/mach-at91/pm_suspend.S
@@ -15,6 +15,7 @@
 #include <linux/clk/at91_pmc.h>
 #include <mach/hardware.h>
 #include <mach/at91_ramc.h>
+#include <asm/hardware/cache-l2x0.h>
 
 #include "pm.h"
 
@@ -324,3 +325,109 @@ ram_restored:
 
 ENTRY(at91_pm_suspend_in_sram_sz)
 	.word .-at91_pm_suspend_in_sram
+
+/*---------------------------------------*/
+
+#if defined(CONFIG_CPU_V7)
+
+/*
+ * void at91_disable_l1_l2_cache(void)
+ *
+ * This function code disables, cleans & invalidates the L1 D-cache
+ * and cleans, invalidates & disable the L2 cache.
+ */
+ENTRY(at91_disable_l1_l2_cache)
+	stmfd	sp!, {r4 - r12, lr}
+
+	/*
+	 * Flush all data from the L1 D-cache before disabling
+	 * SCTLR.C bit.
+	 */
+	bl	v7_flush_dcache_all
+
+	/*
+	 * Clear the SCTLR.C bit to prevent further data cache
+	 * allocation. Clearing SCTLR.C would make all the data accesses
+	 * strongly ordered and would not hit the cache.
+	 */
+	mrc	p15, 0, r0, c1, c0, 0
+	bic	r0, r0, #(1 << 2)	@ Disable the C bit
+	mcr	p15, 0, r0, c1, c0, 0
+	isb
+
+	/*
+	 * Invalidate L1 D-cache. Even though only invalidate is
+	 * necessary exported flush API is used here. Doing clean
+	 * on already clean cache would be almost NOP.
+	 */
+	bl	v7_flush_dcache_all
+
+	/*
+	 * Clean and invalidate the L2 cache.
+	 * Common cache-l2x0.c functions can't be used here since it
+	 * uses spinlocks. We are out of coherency here with data cache
+	 * disabled. The spinlock implementation uses exclusive load/store
+	 * instruction which can fail without data cache being enabled.
+	 * Because of this, CPU can lead to deadlock.
+	 */
+	ldr	r1, at91_l2cc_base_addr
+	ldr	r2, [r1]
+	cmp	r2, #0
+	beq	skip_l2disable
+	mov	r0, #0xff
+	str	r0, [r2, #L2X0_CLEAN_INV_WAY]
+wait:
+	ldr	r0, [r2, #L2X0_CLEAN_INV_WAY]
+	mov	r1, #0xff
+	ands	r0, r0, r1
+	bne	wait
+
+	mov	r0, #0
+	str	r0, [r2, #L2X0_CTRL]
+
+l2x_sync:
+	ldr	r0, [r2, #L2X0_CACHE_SYNC]
+	bic	r0, r0, #0x1
+	str	r0, [r2, #L2X0_CACHE_SYNC]
+sync:
+	ldr	r0, [r2, #L2X0_CACHE_SYNC]
+	ands	r0, r0, #0x1
+	bne	sync
+
+skip_l2disable:
+	ldmfd	sp!, {r4 - r12, pc}
+ENDPROC(at91_disable_l1_l2_cache)
+
+/*
+ * void at91_enable_l1_l2_cache(void)
+ *
+ * This function code enables the L1 D-cache and the L2 cache.
+ */
+ENTRY(at91_enable_l1_l2_cache)
+	stmfd	sp!, {r4 - r12, lr}
+
+	/* Enable the L2 cache */
+	ldr	r1, at91_l2cc_base_addr
+	ldr	r2, [r1]
+	cmp	r2, #0
+	beq	skip_l2en
+	ldr	r0, [r2, #L2X0_CTRL]
+	ands	r0, r0, #L2X0_CTRL_EN
+	bne	skip_l2en	@ Skip if already enabled
+	mov	r0, #L2X0_CTRL_EN
+	str	r0, [r2, #L2X0_CTRL]
+skip_l2en:
+
+	/* Enable the L1 D-cache */
+	mrc	p15, 0, r0, c1, c0, 0
+	tst	r0, #(1 << 2)		@ Check C bit enabled?
+	orreq	r0, r0, #(1 << 2)	@ Enable the C bit
+	mcreq	p15, 0, r0, c1, c0, 0
+	isb
+
+	ldmfd	sp!, {r4 - r12, pc}
+ENDPROC(at91_enable_l1_l2_cache)
+
+at91_l2cc_base_addr:
+	.word	at91_l2cc_base
+#endif
-- 
1.7.9.5

[PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Wenyou Yang]

For the sama5, disable L1 D-cache and L2 cache before the cpu go to wfi,
after wakeing up, enable L1 D-cache and L2 cache.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 arch/arm/mach-at91/pm.c         |   12 +++++
 arch/arm/mach-at91/pm_suspend.S |  107 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index c547038..3442f80e 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -40,6 +40,12 @@ static struct {
 	int memctrl;
 } at91_pm_data;
 
+void __weak at91_disable_l1_l2_cache(void) {}
+void __weak at91_enable_l1_l2_cache(void) {}
+
+void __iomem *at91_l2cc_base;
+EXPORT_SYMBOL_GPL(at91_l2cc_base);
+
 static int at91_pm_valid_state(suspend_state_t state)
 {
 	switch (state) {
@@ -141,8 +147,14 @@ static void at91_pm_suspend(suspend_state_t state)
 	pm_data |= (state == PM_SUSPEND_MEM) ?
 				AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
 
+	/* Disable L1 D-cache and L2 cache */
+	at91_disable_l1_l2_cache();
+
 	at91_suspend_sram_fn(at91_pmc_base, at91_ramc_base[0],
 				at91_ramc_base[1], pm_data);
+
+	/* Enable L1 D-cache and L2 cache */
+	at91_enable_l1_l2_cache();
 }
 
 static int at91_pm_enter(suspend_state_t state)
diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
index 311cc23..02d4e56 100644
--- a/arch/arm/mach-at91/pm_suspend.S
+++ b/arch/arm/mach-at91/pm_suspend.S
@@ -15,6 +15,7 @@
 #include <linux/clk/at91_pmc.h>
 #include <mach/hardware.h>
 #include <mach/at91_ramc.h>
+#include <asm/hardware/cache-l2x0.h>
 
 #include "pm.h"
 
@@ -324,3 +325,109 @@ ram_restored:
 
 ENTRY(at91_pm_suspend_in_sram_sz)
 	.word .-at91_pm_suspend_in_sram
+
+/*---------------------------------------*/
+
+#if defined(CONFIG_CPU_V7)
+
+/*
+ * void at91_disable_l1_l2_cache(void)
+ *
+ * This function code disables, cleans & invalidates the L1 D-cache
+ * and cleans, invalidates & disable the L2 cache.
+ */
+ENTRY(at91_disable_l1_l2_cache)
+	stmfd	sp!, {r4 - r12, lr}
+
+	/*
+	 * Flush all data from the L1 D-cache before disabling
+	 * SCTLR.C bit.
+	 */
+	bl	v7_flush_dcache_all
+
+	/*
+	 * Clear the SCTLR.C bit to prevent further data cache
+	 * allocation. Clearing SCTLR.C would make all the data accesses
+	 * strongly ordered and would not hit the cache.
+	 */
+	mrc	p15, 0, r0, c1, c0, 0
+	bic	r0, r0, #(1 << 2)	@ Disable the C bit
+	mcr	p15, 0, r0, c1, c0, 0
+	isb
+
+	/*
+	 * Invalidate L1 D-cache. Even though only invalidate is
+	 * necessary exported flush API is used here. Doing clean
+	 * on already clean cache would be almost NOP.
+	 */
+	bl	v7_flush_dcache_all
+
+	/*
+	 * Clean and invalidate the L2 cache.
+	 * Common cache-l2x0.c functions can't be used here since it
+	 * uses spinlocks. We are out of coherency here with data cache
+	 * disabled. The spinlock implementation uses exclusive load/store
+	 * instruction which can fail without data cache being enabled.
+	 * Because of this, CPU can lead to deadlock.
+	 */
+	ldr	r1, at91_l2cc_base_addr
+	ldr	r2, [r1]
+	cmp	r2, #0
+	beq	skip_l2disable
+	mov	r0, #0xff
+	str	r0, [r2, #L2X0_CLEAN_INV_WAY]
+wait:
+	ldr	r0, [r2, #L2X0_CLEAN_INV_WAY]
+	mov	r1, #0xff
+	ands	r0, r0, r1
+	bne	wait
+
+	mov	r0, #0
+	str	r0, [r2, #L2X0_CTRL]
+
+l2x_sync:
+	ldr	r0, [r2, #L2X0_CACHE_SYNC]
+	bic	r0, r0, #0x1
+	str	r0, [r2, #L2X0_CACHE_SYNC]
+sync:
+	ldr	r0, [r2, #L2X0_CACHE_SYNC]
+	ands	r0, r0, #0x1
+	bne	sync
+
+skip_l2disable:
+	ldmfd	sp!, {r4 - r12, pc}
+ENDPROC(at91_disable_l1_l2_cache)
+
+/*
+ * void at91_enable_l1_l2_cache(void)
+ *
+ * This function code enables the L1 D-cache and the L2 cache.
+ */
+ENTRY(at91_enable_l1_l2_cache)
+	stmfd	sp!, {r4 - r12, lr}
+
+	/* Enable the L2 cache */
+	ldr	r1, at91_l2cc_base_addr
+	ldr	r2, [r1]
+	cmp	r2, #0
+	beq	skip_l2en
+	ldr	r0, [r2, #L2X0_CTRL]
+	ands	r0, r0, #L2X0_CTRL_EN
+	bne	skip_l2en	@ Skip if already enabled
+	mov	r0, #L2X0_CTRL_EN
+	str	r0, [r2, #L2X0_CTRL]
+skip_l2en:
+
+	/* Enable the L1 D-cache */
+	mrc	p15, 0, r0, c1, c0, 0
+	tst	r0, #(1 << 2)		@ Check C bit enabled?
+	orreq	r0, r0, #(1 << 2)	@ Enable the C bit
+	mcreq	p15, 0, r0, c1, c0, 0
+	isb
+
+	ldmfd	sp!, {r4 - r12, pc}
+ENDPROC(at91_enable_l1_l2_cache)
+
+at91_l2cc_base_addr:
+	.word	at91_l2cc_base
+#endif
-- 
1.7.9.5

[PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Sergei Shtylyov]

Hello.

On 1/28/2015 5:24 AM, Wenyou Yang wrote:

> For the sama5, disable L1 D-cache and L2 cache before the cpu go to wfi,
> after wakeing up, enable L1 D-cache and L2 cache.

    Waking.

> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
>   arch/arm/mach-at91/pm.c         |   12 +++++
>   arch/arm/mach-at91/pm_suspend.S |  107 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 119 insertions(+)

[...]
> diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> index 311cc23..02d4e56 100644
> --- a/arch/arm/mach-at91/pm_suspend.S
> +++ b/arch/arm/mach-at91/pm_suspend.S
[...]
> @@ -324,3 +325,109 @@ ram_restored:
[...]
> +l2x_sync:

    I don't see where this label is used.

> +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> +	bic	r0, r0, #0x1
> +	str	r0, [r2, #L2X0_CACHE_SYNC]
> +sync:
> +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> +	ands	r0, r0, #0x1
> +	bne	sync
> +
> +skip_l2disable:
> +	ldmfd	sp!, {r4 - r12, pc}
> +ENDPROC(at91_disable_l1_l2_cache)
[...]

WBR, Sergei

Re: [PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Sergei Shtylyov]

Hello.

On 1/28/2015 5:24 AM, Wenyou Yang wrote:

> For the sama5, disable L1 D-cache and L2 cache before the cpu go to wfi,
> after wakeing up, enable L1 D-cache and L2 cache.

    Waking.

> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
>   arch/arm/mach-at91/pm.c         |   12 +++++
>   arch/arm/mach-at91/pm_suspend.S |  107 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 119 insertions(+)

[...]
> diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> index 311cc23..02d4e56 100644
> --- a/arch/arm/mach-at91/pm_suspend.S
> +++ b/arch/arm/mach-at91/pm_suspend.S
[...]
> @@ -324,3 +325,109 @@ ram_restored:
[...]
> +l2x_sync:

    I don't see where this label is used.

> +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> +	bic	r0, r0, #0x1
> +	str	r0, [r2, #L2X0_CACHE_SYNC]
> +sync:
> +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> +	ands	r0, r0, #0x1
> +	bne	sync
> +
> +skip_l2disable:
> +	ldmfd	sp!, {r4 - r12, pc}
> +ENDPROC(at91_disable_l1_l2_cache)
[...]

WBR, Sergei

[PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Yang, Wenyou]

Hi Sergei,

Thank you for your review.

> -----Original Message-----
> From: Sergei Shtylyov [mailto:sergei.shtylyov at cogentembedded.com]
> Sent: Wednesday, January 28, 2015 6:09 PM
> To: Yang, Wenyou; Ferre, Nicolas; linux at arm.linux.org.uk
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> alexandre.belloni at free-electrons.com; sylvain.rochet at finsecur.com;
> peda at axentia.se; linux at maxim.org.za
> Subject: Re: [PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while
> suspend/resume
> 
> Hello.
> 
> On 1/28/2015 5:24 AM, Wenyou Yang wrote:
> 
> > For the sama5, disable L1 D-cache and L2 cache before the cpu go to
> > wfi, after wakeing up, enable L1 D-cache and L2 cache.
> 
>     Waking.
> 
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> >   arch/arm/mach-at91/pm.c         |   12 +++++
> >   arch/arm/mach-at91/pm_suspend.S |  107
> +++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 119 insertions(+)
> 
> [...]
> > diff --git a/arch/arm/mach-at91/pm_suspend.S
> > b/arch/arm/mach-at91/pm_suspend.S index 311cc23..02d4e56 100644
> > --- a/arch/arm/mach-at91/pm_suspend.S
> > +++ b/arch/arm/mach-at91/pm_suspend.S
> [...]
> > @@ -324,3 +325,109 @@ ram_restored:
> [...]
> > +l2x_sync:
> 
>     I don't see where this label is used.
I thought it is an indication the following is for L2 cache synchronization. 

It is redundant, remove it.

> 
> > +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> > +	bic	r0, r0, #0x1
> > +	str	r0, [r2, #L2X0_CACHE_SYNC]
> > +sync:
> > +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> > +	ands	r0, r0, #0x1
> > +	bne	sync
> > +
> > +skip_l2disable:
> > +	ldmfd	sp!, {r4 - r12, pc}
> > +ENDPROC(at91_disable_l1_l2_cache)
> [...]
> 
> WBR, Sergei

Best Regards,
Wenyou Yang

RE: [PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Yang, Wenyou]

Hi Sergei,

Thank you for your review.

> -----Original Message-----
> From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com]
> Sent: Wednesday, January 28, 2015 6:09 PM
> To: Yang, Wenyou; Ferre, Nicolas; linux@arm.linux.org.uk
> Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> alexandre.belloni@free-electrons.com; sylvain.rochet@finsecur.com;
> peda@axentia.se; linux@maxim.org.za
> Subject: Re: [PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while
> suspend/resume
> 
> Hello.
> 
> On 1/28/2015 5:24 AM, Wenyou Yang wrote:
> 
> > For the sama5, disable L1 D-cache and L2 cache before the cpu go to
> > wfi, after wakeing up, enable L1 D-cache and L2 cache.
> 
>     Waking.
> 
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> >   arch/arm/mach-at91/pm.c         |   12 +++++
> >   arch/arm/mach-at91/pm_suspend.S |  107
> +++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 119 insertions(+)
> 
> [...]
> > diff --git a/arch/arm/mach-at91/pm_suspend.S
> > b/arch/arm/mach-at91/pm_suspend.S index 311cc23..02d4e56 100644
> > --- a/arch/arm/mach-at91/pm_suspend.S
> > +++ b/arch/arm/mach-at91/pm_suspend.S
> [...]
> > @@ -324,3 +325,109 @@ ram_restored:
> [...]
> > +l2x_sync:
> 
>     I don't see where this label is used.
I thought it is an indication the following is for L2 cache synchronization. 

It is redundant, remove it.

> 
> > +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> > +	bic	r0, r0, #0x1
> > +	str	r0, [r2, #L2X0_CACHE_SYNC]
> > +sync:
> > +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> > +	ands	r0, r0, #0x1
> > +	bne	sync
> > +
> > +skip_l2disable:
> > +	ldmfd	sp!, {r4 - r12, pc}
> > +ENDPROC(at91_disable_l1_l2_cache)
> [...]
> 
> WBR, Sergei

Best Regards,
Wenyou Yang

[PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Russell King - ARM Linux]

On Wed, Jan 28, 2015 at 10:24:04AM +0800, Wenyou Yang wrote:
> +	/*
> +	 * Clean and invalidate the L2 cache.
> +	 * Common cache-l2x0.c functions can't be used here since it
> +	 * uses spinlocks. We are out of coherency here with data cache
> +	 * disabled. The spinlock implementation uses exclusive load/store
> +	 * instruction which can fail without data cache being enabled.
> +	 * Because of this, CPU can lead to deadlock.

We really need to stop needing platforms to create their own L2 handling
code.  Please move this to a helper function in arch/arm/mm/l2c-l...-clean.S,
replacing ... with the appropriate part for the code fragment.

> +	 */
> +	ldr	r1, at91_l2cc_base_addr
> +	ldr	r2, [r1]
> +	cmp	r2, #0
> +	beq	skip_l2disable
> +	mov	r0, #0xff
> +	str	r0, [r2, #L2X0_CLEAN_INV_WAY]
> +wait:
> +	ldr	r0, [r2, #L2X0_CLEAN_INV_WAY]
> +	mov	r1, #0xff
> +	ands	r0, r0, r1
> +	bne	wait
> +
> +	mov	r0, #0
> +	str	r0, [r2, #L2X0_CTRL]
> +
> +l2x_sync:
> +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> +	bic	r0, r0, #0x1
> +	str	r0, [r2, #L2X0_CACHE_SYNC]

I wonder whether you've actually read the documentation for this.  You
don't need to read-modify-write this register.  The C code doesn't even
do this.  A write to this register is sufficient - a write issues the
sync, a read returns the completion status.

> +sync:
> +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> +	ands	r0, r0, #0x1
> +	bne	sync

Moreover, do you actually need this - it depends on the L2C model.  Only
L2C220 needs to spin waiting for the sync operation to complete.

Also, are you sure the "clean+invalidate, disable, sync" sequence is
correct?  Should it not be "clean+invalidate, sync, disable" ?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Russell King - ARM Linux]

On Wed, Jan 28, 2015 at 10:24:04AM +0800, Wenyou Yang wrote:
> +	/*
> +	 * Clean and invalidate the L2 cache.
> +	 * Common cache-l2x0.c functions can't be used here since it
> +	 * uses spinlocks. We are out of coherency here with data cache
> +	 * disabled. The spinlock implementation uses exclusive load/store
> +	 * instruction which can fail without data cache being enabled.
> +	 * Because of this, CPU can lead to deadlock.

We really need to stop needing platforms to create their own L2 handling
code.  Please move this to a helper function in arch/arm/mm/l2c-l...-clean.S,
replacing ... with the appropriate part for the code fragment.

> +	 */
> +	ldr	r1, at91_l2cc_base_addr
> +	ldr	r2, [r1]
> +	cmp	r2, #0
> +	beq	skip_l2disable
> +	mov	r0, #0xff
> +	str	r0, [r2, #L2X0_CLEAN_INV_WAY]
> +wait:
> +	ldr	r0, [r2, #L2X0_CLEAN_INV_WAY]
> +	mov	r1, #0xff
> +	ands	r0, r0, r1
> +	bne	wait
> +
> +	mov	r0, #0
> +	str	r0, [r2, #L2X0_CTRL]
> +
> +l2x_sync:
> +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> +	bic	r0, r0, #0x1
> +	str	r0, [r2, #L2X0_CACHE_SYNC]

I wonder whether you've actually read the documentation for this.  You
don't need to read-modify-write this register.  The C code doesn't even
do this.  A write to this register is sufficient - a write issues the
sync, a read returns the completion status.

> +sync:
> +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> +	ands	r0, r0, #0x1
> +	bne	sync

Moreover, do you actually need this - it depends on the L2C model.  Only
L2C220 needs to spin waiting for the sync operation to complete.

Also, are you sure the "clean+invalidate, disable, sync" sequence is
correct?  Should it not be "clean+invalidate, sync, disable" ?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Yang, Wenyou]

Hi Russell,

Thank you very much for your suggestion.

I will redo this patch to use the cache helper functions ASAP.

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: Thursday, January 29, 2015 7:35 PM
> To: Yang, Wenyou
> Cc: Ferre, Nicolas; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; alexandre.belloni at free-electrons.com;
> sylvain.rochet at finsecur.com; peda at axentia.se;
> sergei.shtylyov at cogentembedded.com; linux at maxim.org.za
> Subject: Re: [PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while
> suspend/resume
> 
> On Wed, Jan 28, 2015 at 10:24:04AM +0800, Wenyou Yang wrote:
> > +	/*
> > +	 * Clean and invalidate the L2 cache.
> > +	 * Common cache-l2x0.c functions can't be used here since it
> > +	 * uses spinlocks. We are out of coherency here with data cache
> > +	 * disabled. The spinlock implementation uses exclusive load/store
> > +	 * instruction which can fail without data cache being enabled.
> > +	 * Because of this, CPU can lead to deadlock.
> 
> We really need to stop needing platforms to create their own L2 handling code.
> Please move this to a helper function in arch/arm/mm/l2c-l...-clean.S, replacing ...
> with the appropriate part for the code fragment.
> 
> > +	 */
> > +	ldr	r1, at91_l2cc_base_addr
> > +	ldr	r2, [r1]
> > +	cmp	r2, #0
> > +	beq	skip_l2disable
> > +	mov	r0, #0xff
> > +	str	r0, [r2, #L2X0_CLEAN_INV_WAY]
> > +wait:
> > +	ldr	r0, [r2, #L2X0_CLEAN_INV_WAY]
> > +	mov	r1, #0xff
> > +	ands	r0, r0, r1
> > +	bne	wait
> > +
> > +	mov	r0, #0
> > +	str	r0, [r2, #L2X0_CTRL]
> > +
> > +l2x_sync:
> > +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> > +	bic	r0, r0, #0x1
> > +	str	r0, [r2, #L2X0_CACHE_SYNC]
> 
> I wonder whether you've actually read the documentation for this.  You don't need
> to read-modify-write this register.  The C code doesn't even do this.  A write to this
> register is sufficient - a write issues the sync, a read returns the completion status.
> 
> > +sync:
> > +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> > +	ands	r0, r0, #0x1
> > +	bne	sync
> 
> Moreover, do you actually need this - it depends on the L2C model.  Only
> L2C220 needs to spin waiting for the sync operation to complete.
> 
> Also, are you sure the "clean+invalidate, disable, sync" sequence is correct?
> Should it not be "clean+invalidate, sync, disable" ?
> 
> --
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.

Best Regards,
Wenyou Yang

RE: [PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while suspend/resume

[Yang, Wenyou]

Hi Russell,

Thank you very much for your suggestion.

I will redo this patch to use the cache helper functions ASAP.

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk]
> Sent: Thursday, January 29, 2015 7:35 PM
> To: Yang, Wenyou
> Cc: Ferre, Nicolas; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; alexandre.belloni@free-electrons.com;
> sylvain.rochet@finsecur.com; peda@axentia.se;
> sergei.shtylyov@cogentembedded.com; linux@maxim.org.za
> Subject: Re: [PATCH v2 3/3] pm: at91: add disable/enable the L1/L2 cache while
> suspend/resume
> 
> On Wed, Jan 28, 2015 at 10:24:04AM +0800, Wenyou Yang wrote:
> > +	/*
> > +	 * Clean and invalidate the L2 cache.
> > +	 * Common cache-l2x0.c functions can't be used here since it
> > +	 * uses spinlocks. We are out of coherency here with data cache
> > +	 * disabled. The spinlock implementation uses exclusive load/store
> > +	 * instruction which can fail without data cache being enabled.
> > +	 * Because of this, CPU can lead to deadlock.
> 
> We really need to stop needing platforms to create their own L2 handling code.
> Please move this to a helper function in arch/arm/mm/l2c-l...-clean.S, replacing ...
> with the appropriate part for the code fragment.
> 
> > +	 */
> > +	ldr	r1, at91_l2cc_base_addr
> > +	ldr	r2, [r1]
> > +	cmp	r2, #0
> > +	beq	skip_l2disable
> > +	mov	r0, #0xff
> > +	str	r0, [r2, #L2X0_CLEAN_INV_WAY]
> > +wait:
> > +	ldr	r0, [r2, #L2X0_CLEAN_INV_WAY]
> > +	mov	r1, #0xff
> > +	ands	r0, r0, r1
> > +	bne	wait
> > +
> > +	mov	r0, #0
> > +	str	r0, [r2, #L2X0_CTRL]
> > +
> > +l2x_sync:
> > +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> > +	bic	r0, r0, #0x1
> > +	str	r0, [r2, #L2X0_CACHE_SYNC]
> 
> I wonder whether you've actually read the documentation for this.  You don't need
> to read-modify-write this register.  The C code doesn't even do this.  A write to this
> register is sufficient - a write issues the sync, a read returns the completion status.
> 
> > +sync:
> > +	ldr	r0, [r2, #L2X0_CACHE_SYNC]
> > +	ands	r0, r0, #0x1
> > +	bne	sync
> 
> Moreover, do you actually need this - it depends on the L2C model.  Only
> L2C220 needs to spin waiting for the sync operation to complete.
> 
> Also, are you sure the "clean+invalidate, disable, sync" sequence is correct?
> Should it not be "clean+invalidate, sync, disable" ?
> 
> --
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.

Best Regards,
Wenyou Yang