ARM: cache-l2x0: add resume entry for l2 in secure mode

PATCH New Barry Song 6 participants 17 messages Sep 26, 2011 – Sep 29, 2011 Arm (32-bit)

Share & tools

Barry Song

author

2011-09-26 03:26 UTC #0 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.

Cc Shawn Guo <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Signed-off-by Barry Song <[email protected]>

---
-v2:
define the structure l2x0_regs to hold all saved registers;
make saved copy of registers non-static so that platform codes can access them; add asm_offset for l2x0 saved regs(l2x0_regs struct) so that asm codes can use

 arch/arm/include/asm/hardware/cache-l2x0.h |   13 +++++
 arch/arm/include/asm/outercache.h          |    7 +++
 arch/arm/kernel/asm-offsets.c              |    9 +++
 arch/arm/mm/cache-l2x0.c                   |   81 ++++++++++++++++++++++++---
 4 files changed, 101 insertions(+), 9 deletions(-)

arch/arm/include/asm/hardware/cache-l2x0.h +13 -0

index c48cb1e..4f9e81d 100644
@@ -96,6 +96,19 @@
 #ifndef __ASSEMBLY__
 extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
 extern int l2x0_of_init(__u32 aux_val, __u32 aux_mask);
+
+struct l2x0_regs {
+	unsigned long aux_ctrl;
+	/*
+	 * Whether the following registers need to be saved/restored
+	 * depends on platform
+	 */
+	unsigned long tag_latency;
+	unsigned long data_latency;
+	unsigned long filter_start;
+	unsigned long filter_end;
+};
+
 #endif
 
 #endif

arch/arm/include/asm/outercache.h +7 -0

index d838743..53426c6 100644
@@ -34,6 +34,7 @@ struct outer_cache_fns {
 	void (*sync)(void);
 #endif
 	void (*set_debug)(unsigned long);
+	void (*resume)(void);
 };
 
 #ifdef CONFIG_OUTER_CACHE

@@ -74,6 +75,12 @@ static inline void outer_disable(void)
 		outer_cache.disable();
 }
 
+static inline void outer_resume(void)
+{
+	if (outer_cache.resume)
+		outer_cache.resume();
+}
+
 #else
 
 static inline void outer_inv_range(phys_addr_t start, phys_addr_t end)

arch/arm/kernel/asm-offsets.c +9 -0

index 16baba2..b8d72a8 100644
@@ -20,6 +20,7 @@
 #include <asm/thread_info.h>
 #include <asm/memory.h>
 #include <asm/procinfo.h>
+#include <asm/hardware/cache-l2x0.h>
 #include <linux/kbuild.h>
 
 /*

@@ -92,6 +93,14 @@ int main(void)
   DEFINE(S_OLD_R0,		offsetof(struct pt_regs, ARM_ORIG_r0));
   DEFINE(S_FRAME_SIZE,		sizeof(struct pt_regs));
   BLANK();
+#ifdef CONFIG_CACHE_L2X0
+  DEFINE(L2X0_R_AUX_CTRL,	offsetof(struct l2x0_regs, aux_ctrl));
+  DEFINE(L2X0_R_TAG_LATENCY,	offsetof(struct l2x0_regs, tag_latency));
+  DEFINE(L2X0_R_DATA_LATENCY,	offsetof(struct l2x0_regs, data_latency));
+  DEFINE(L2X0_R_FILTER_START,	offsetof(struct l2x0_regs, filter_start));
+  DEFINE(L2X0_R_FILTER_END,	offsetof(struct l2x0_regs, filter_end));
+  BLANK();
+#endif
 #ifdef CONFIG_CPU_HAS_ASID
   DEFINE(MM_CONTEXT_ID,		offsetof(struct mm_struct, context.id));
   BLANK();

arch/arm/mm/cache-l2x0.c +72 -9

122 lines not on this page · raw message

Signature · 6 lines

1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

Santosh Shilimkar

2011-09-29 05:35 UTC #1 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Barry,

Quoted 29 lines · Barry Song

we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---
-v2:
define the structure l2x0_regs to hold all saved registers;
make saved copy of registers non-static so that platform codes can access them; add asm_offset for l2x0 saved regs(l2x0_regs struct) so that asm codes can use
 arch/arm/include/asm/hardware/cache-l2x0.h |   13 +++++
 arch/arm/include/asm/outercache.h          |    7 +++
 arch/arm/kernel/asm-offsets.c              |    9 +++
 arch/arm/mm/cache-l2x0.c                   |   81 ++++++++++++++++++++++++---
 4 files changed, 101 insertions(+), 9 deletions(-)
arch/arm/include/asm/hardware/cache-l2x0.h +3 -0
index c48cb1e..4f9e81d 100644
@@ -96,6 +96,19 @@
 #ifndef __ASSEMBLY__
 extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
 extern int l2x0_of_init(__u32 aux_val, __u32 aux_mask);
+
+struct l2x0_regs {
+	unsigned long aux_ctrl;

arch/arm/include/asm/hardware/cache-l2x0.h line 111

Quoted 10 lines

arch/arm/include/asm/hardware/cache-l2x0.h +10 -0

+	/*
+	 * Whether the following registers need to be saved/restored
+	 * depends on platform
+	 */
+	unsigned long tag_latency;
+	unsigned long data_latency;
+	unsigned long filter_start;
+	unsigned long filter_end;
+};
+

You are missing POR register here which is available on PL310 versions. You should add that.

[...]

Quoted 14 lines

arch/arm/include/asm/hardware/cache-l2x0.h +0 -0

 static inline void outer_inv_range(phys_addr_t start, phys_addr_t end)

arch/arm/kernel/asm-offsets.c +1 -0

index 16baba2..b8d72a8 100644
@@ -20,6 +20,7 @@
 #include <asm/thread_info.h>
 #include <asm/memory.h>
 #include <asm/procinfo.h>
+#include <asm/hardware/cache-l2x0.h>
 #include <linux/kbuild.h>
 
 /*

arch/arm/kernel/asm-offsets.c int main(void) · line 101

Quoted 9 lines

arch/arm/kernel/asm-offsets.c +6 -0

@@ -92,6 +93,14 @@ int main(void)
   DEFINE(S_OLD_R0,		offsetof(struct pt_regs, ARM_ORIG_r0));
   DEFINE(S_FRAME_SIZE,		sizeof(struct pt_regs));
   BLANK();
+#ifdef CONFIG_CACHE_L2X0
+  DEFINE(L2X0_R_AUX_CTRL,	offsetof(struct l2x0_regs, aux_ctrl));
+  DEFINE(L2X0_R_TAG_LATENCY,	offsetof(struct l2x0_regs, tag_latency));
+  DEFINE(L2X0_R_DATA_LATENCY,	offsetof(struct l2x0_regs, data_latency));
+  DEFINE(L2X0_R_FILTER_START,	offsetof(struct l2x0_regs, filter_start));
+  DEFINE(L2X0_R_FILTER_END,	offsetof(struct l2x0_regs, filter_end));

Add POR as commented earlier.

Rest of the patch looks good to me. I have ignored DT related changes since I don't understand them.

Shawn,
Have you tried this patch on IMX and see if you are able to use it from the asm code?

Regards
Santosh

Shawn Guo

2011-09-29 05:49 UTC #2 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Quoted 71 lines · Santosh Shilimkar

Barry,
Quoted 29 lines · Barry Song
we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---
-v2:
define the structure l2x0_regs to hold all saved registers;
make saved copy of registers non-static so that platform codes can access them; add asm_offset for l2x0 saved regs(l2x0_regs struct) so that asm codes can use
 arch/arm/include/asm/hardware/cache-l2x0.h |   13 +++++
 arch/arm/include/asm/outercache.h          |    7 +++
 arch/arm/kernel/asm-offsets.c              |    9 +++
 arch/arm/mm/cache-l2x0.c                   |   81 ++++++++++++++++++++++++---
 4 files changed, 101 insertions(+), 9 deletions(-)
arch/arm/include/asm/hardware/cache-l2x0.h +3 -0
index c48cb1e..4f9e81d 100644
@@ -96,6 +96,19 @@
 #ifndef __ASSEMBLY__
 extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
 extern int l2x0_of_init(__u32 aux_val, __u32 aux_mask);
+
+struct l2x0_regs {
+	unsigned long aux_ctrl;
arch/arm/include/asm/hardware/cache-l2x0.h line 111
Quoted 10 lines
arch/arm/include/asm/hardware/cache-l2x0.h +10 -0
+	/*
+	 * Whether the following registers need to be saved/restored
+	 * depends on platform
+	 */
+	unsigned long tag_latency;
+	unsigned long data_latency;
+	unsigned long filter_start;
+	unsigned long filter_end;
+};
+
You are missing POR register here which is available on PL310 versions. You should add that.
[...]
Quoted 14 lines
arch/arm/include/asm/hardware/cache-l2x0.h +0 -0
 static inline void outer_inv_range(phys_addr_t start, phys_addr_t end)
arch/arm/kernel/asm-offsets.c +1 -0
index 16baba2..b8d72a8 100644
@@ -20,6 +20,7 @@
 #include <asm/thread_info.h>
 #include <asm/memory.h>
 #include <asm/procinfo.h>
+#include <asm/hardware/cache-l2x0.h>
 #include <linux/kbuild.h>
 
 /*
arch/arm/kernel/asm-offsets.c int main(void) · line 101
Quoted 9 lines
arch/arm/kernel/asm-offsets.c +6 -0
@@ -92,6 +93,14 @@ int main(void)
   DEFINE(S_OLD_R0,		offsetof(struct pt_regs, ARM_ORIG_r0));
   DEFINE(S_FRAME_SIZE,		sizeof(struct pt_regs));
   BLANK();
+#ifdef CONFIG_CACHE_L2X0
+  DEFINE(L2X0_R_AUX_CTRL,	offsetof(struct l2x0_regs, aux_ctrl));
+  DEFINE(L2X0_R_TAG_LATENCY,	offsetof(struct l2x0_regs, tag_latency));
+  DEFINE(L2X0_R_DATA_LATENCY,	offsetof(struct l2x0_regs, data_latency));
+  DEFINE(L2X0_R_FILTER_START,	offsetof(struct l2x0_regs, filter_start));
+  DEFINE(L2X0_R_FILTER_END,	offsetof(struct l2x0_regs, filter_end));
Add POR as commented earlier.

Quoted 7 lines

Rest of the patch looks good to me. I have ignored DT related changes since I don't understand them.
Shawn,
Have you tried this patch on IMX and see if you are able to use it from the asm code?

Will give it a test today.

Signature · 2 lines

Regards,
Shawn

Barry Song

2011-09-29 05:59 UTC #3 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

2011/9/29 Shawn Guo <[email protected]>:

Quoted 72 lines · Santosh Shilimkar

Quoted 71 lines
Barry,
Quoted 29 lines · Barry Song
we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---
?-v2:
?define the structure l2x0_regs to hold all saved registers;
?make saved copy of registers non-static so that platform codes can access them; ?add asm_offset for l2x0 saved regs(l2x0_regs struct) so that asm codes can use
?arch/arm/include/asm/hardware/cache-l2x0.h | ? 13 +++++
?arch/arm/include/asm/outercache.h ? ? ? ? ?| ? ?7 +++
?arch/arm/kernel/asm-offsets.c ? ? ? ? ? ? ?| ? ?9 +++
?arch/arm/mm/cache-l2x0.c ? ? ? ? ? ? ? ? ? | ? 81 ++++++++++++++++++++++++---
?4 files changed, 101 insertions(+), 9 deletions(-)
arch/arm/include/asm/hardware/cache-l2x0.h +0 -0
index c48cb1e..4f9e81d 100644
@@ -96,6 +96,19 @@
?#ifndef __ASSEMBLY__
?extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask); ?extern int l2x0_of_init(__u32 aux_val, __u32 aux_mask);
arch/arm/include/asm/hardware/cache-l2x0.h +3 -0
+
+struct l2x0_regs {
+ ? unsigned long aux_ctrl;
arch/arm/include/asm/hardware/cache-l2x0.h
Quoted 10 lines
arch/arm/include/asm/hardware/cache-l2x0.h +10 -0
+ ? /*
+ ? ?* Whether the following registers need to be saved/restored
+ ? ?* depends on platform
+ ? ?*/
+ ? unsigned long tag_latency;
+ ? unsigned long data_latency;
+ ? unsigned long filter_start;
+ ? unsigned long filter_end;
+};
+
You are missing POR register here which is available on PL310 versions. You should add that.
[...]
Quoted 14 lines
?static inline void outer_inv_range(phys_addr_t start, phys_addr_t end)
arch/arm/kernel/asm-offsets.c +0 -0
index 16baba2..b8d72a8 100644
@@ -20,6 +20,7 @@
?#include <asm/thread_info.h>
?#include <asm/memory.h>
?#include <asm/procinfo.h>
+#include <asm/hardware/cache-l2x0.h>
?#include <linux/kbuild.h>
?/*
arch/arm/kernel/asm-offsets.c int main(void) · line 101
Quoted 9 lines
arch/arm/kernel/asm-offsets.c +6 -0
@@ -92,6 +93,14 @@ int main(void)
 ? ?DEFINE(S_OLD_R0, ? ? ? ? offsetof(struct pt_regs, ARM_ORIG_r0));
 ? ?DEFINE(S_FRAME_SIZE, ? ? ? ? ? ? sizeof(struct pt_regs));
 ? ?BLANK();
+#ifdef CONFIG_CACHE_L2X0
+ ?DEFINE(L2X0_R_AUX_CTRL, ?offsetof(struct l2x0_regs, aux_ctrl));
+ ?DEFINE(L2X0_R_TAG_LATENCY, ? ? ? offsetof(struct l2x0_regs, tag_latency));
+ ?DEFINE(L2X0_R_DATA_LATENCY, ? ? ?offsetof(struct l2x0_regs, data_latency));
+ ?DEFINE(L2X0_R_FILTER_START, ? ? ?offsetof(struct l2x0_regs, filter_start));
+ ?DEFINE(L2X0_R_FILTER_END, ? ? ? ?offsetof(struct l2x0_regs, filter_end));
Add POR as commented earlier.

Quoted 8 lines

Quoted 7 lines
Rest of the patch looks good to me. I have ignored DT related changes since I don't understand them.
Shawn,
Have you tried this patch on IMX and see if you are able to use it from the asm code?
Will give it a test today

well, thanks. once after you test and think it is ok, i'll send v3 with fixing the lost POR.

Quoted 4 lines

--
Regards,
Shawn

-barry

Lorenzo Pieralisi

2011-09-29 10:22 UTC #4 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

In reply to Santosh Shilimkar

Quoted 10 lines · Santosh Shilimkar

Barry,
Quoted 7 lines · Barry Song
we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---

[...]

Quoted 7 lines

arch/arm/include/asm/hardware/cache-l2x0.h +7 -0

+
+struct l2x0_regs {
+	unsigned long aux_ctrl;
+	/*
+	 * Whether the following registers need to be saved/restored
+	 * depends on platform
+	 */

Quoted 9 lines

arch/arm/include/asm/hardware/cache-l2x0.h line 111
Quoted 6 lines
arch/arm/include/asm/hardware/cache-l2x0.h +6 -0
+	unsigned long tag_latency;
+	unsigned long data_latency;
+	unsigned long filter_start;
+	unsigned long filter_end;
+};
+
You are missing POR register here which is available on PL310 versions. You should add that.

In latest versions there are Prefetch and Power control registers, but it depends on the revision. What should we do in this case Santosh ? PCR is a superset of POR from r3p0 onwards, but Power control ? Leave it to platform code ?

Quoted 13 lines

[...]
Quoted 12 lines
static inline void outer_inv_range(phys_addr_t start, phys_addr_t end)
arch/arm/kernel/asm-offsets.c +1 -0
index 16baba2..b8d72a8 100644
@@ -20,6 +20,7 @@
 #include <asm/thread_info.h>
 #include <asm/memory.h>
 #include <asm/procinfo.h>
+#include <asm/hardware/cache-l2x0.h>
 #include <linux/kbuild.h>
 

Quoted 12 lines

arch/arm/kernel/asm-offsets.c int main(void) · line 101

Quoted 11 lines

arch/arm/kernel/asm-offsets.c +6 -0

/*

@@ -92,6 +93,14 @@ int main(void)
   DEFINE(S_OLD_R0,		offsetof(struct pt_regs, ARM_ORIG_r0));
   DEFINE(S_FRAME_SIZE,		sizeof(struct pt_regs));
   BLANK();
+#ifdef CONFIG_CACHE_L2X0
+  DEFINE(L2X0_R_AUX_CTRL,	offsetof(struct l2x0_regs, aux_ctrl));
+  DEFINE(L2X0_R_TAG_LATENCY,	offsetof(struct l2x0_regs, tag_latency));
+  DEFINE(L2X0_R_DATA_LATENCY,	offsetof(struct l2x0_regs, data_latency));
+  DEFINE(L2X0_R_FILTER_START,	offsetof(struct l2x0_regs, filter_start));
+  DEFINE(L2X0_R_FILTER_END,	offsetof(struct l2x0_regs, filter_end));

Add POR as commented earlier.

See above.

Thanks,
Lorenzo

Santosh Shilimkar

2011-09-29 10:36 UTC #5 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Quoted 31 lines · Lorenzo Pieralisi

Quoted 10 lines · Santosh Shilimkar
Barry,
Quoted 7 lines · Barry Song
we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---
[...]
Quoted 7 lines
Quoted 7 lines
arch/arm/include/asm/hardware/cache-l2x0.h +7 -0
+
+struct l2x0_regs {
+	unsigned long aux_ctrl;
+	/*
+	 * Whether the following registers need to be saved/restored
+	 * depends on platform
+	 */
Quoted 9 lines
arch/arm/include/asm/hardware/cache-l2x0.h line 111
Quoted 6 lines
arch/arm/include/asm/hardware/cache-l2x0.h +6 -0
+	unsigned long tag_latency;
+	unsigned long data_latency;
+	unsigned long filter_start;
+	unsigned long filter_end;
+};
+
You are missing POR register here which is available on PL310 versions. You should add that.

Quoted 5 lines

In latest versions there are Prefetch and Power control registers, but it depends on the revision. What should we do in this case Santosh ? PCR is a superset of POR from r3p0 onwards, but Power control ? Leave it to platform code ?

Good point Lorenzo.
C code resume can be fixed by marking the validity based on PL310 revision register.
The asm code restore with MMU OFF code, would be handled with platform code with this patch and platform code can choose restore of only supported registers.

Regards
Santosh

Shawn Guo

2011-09-29 09:28 UTC #6 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

In reply to Barry Song

Quoted 12 lines · Barry Song

we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---
-v2:
define the structure l2x0_regs to hold all saved registers;
make saved copy of registers non-static so that platform codes can access them; add asm_offset for l2x0 saved regs(l2x0_regs struct) so that asm codes can use

I'm trying to migrate imx6q to this infrastructure, but seeing some facts that might stop me from the effort. Please help me understand these facts are correct.

It seems to me that the only benefit that imx6q gains from the infrastructure is the register saving, and I fail to see any benefit on the restoring.

* I still have to save physical base of l2x0 for register restoring.
* I do not have the physical address of l2x0_saved_regs, and I have to manage to get it somehow.

Signature · 2 lines

Regards,
Shawn

Russell King - ARM Linux

2011-09-29 09:24 UTC #7 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Quoted 14 lines · Shawn Guo

Quoted 12 lines · Barry Song
we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---
-v2:
define the structure l2x0_regs to hold all saved registers;
make saved copy of registers non-static so that platform codes can access them; add asm_offset for l2x0 saved regs(l2x0_regs struct) so that asm codes can use

Quoted 7 lines

I'm trying to migrate imx6q to this infrastructure, but seeing some facts that might stop me from the effort. Please help me understand these facts are correct.
It seems to me that the only benefit that imx6q gains from the infrastructure is the register saving, and I fail to see any benefit on the restoring.

That's correct - because I believe that's the only part which is worth doing in common code. The restoring tends to be platform dependent so I don't see the point of having a bunch of code in common files to do that which almost no one uses.

Quoted 1 line

* I still have to save physical base of l2x0 for register restoring.

That could be added to the structure.

Quoted 2 lines

* I do not have the physical address of l2x0_saved_regs, and I have to manage to get it somehow.

That's true no matter what - we could store the physical address of the struct somewhere, but then in order for you to obtain that, you'd somehow need to know the physical address of _that_ location too - so there's no benefit there.

It can be obtained simply by: __pa(l2x0_saved_regs) in C code, and saving that in a location known to the platform prior to suspend.

Shawn Guo

2011-09-29 12:44 UTC #8 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Quoted 35 lines · Russell King - ARM Linux

Quoted 14 lines · Shawn Guo
Quoted 12 lines · Barry Song
we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---
-v2:
define the structure l2x0_regs to hold all saved registers;
make saved copy of registers non-static so that platform codes can access them; add asm_offset for l2x0 saved regs(l2x0_regs struct) so that asm codes can use
Quoted 7 lines
I'm trying to migrate imx6q to this infrastructure, but seeing some facts that might stop me from the effort. Please help me understand these facts are correct.
It seems to me that the only benefit that imx6q gains from the infrastructure is the register saving, and I fail to see any benefit on the restoring.
That's correct - because I believe that's the only part which is worth doing in common code. The restoring tends to be platform dependent so I don't see the point of having a bunch of code in common files to do that which almost no one uses.
Quoted 1 line
* I still have to save physical base of l2x0 for register restoring.
That could be added to the structure.
Quoted 2 lines
* I do not have the physical address of l2x0_saved_regs, and I have to manage to get it somehow.

Quoted 8 lines

That's true no matter what - we could store the physical address of the struct somewhere, but then in order for you to obtain that, you'd somehow need to know the physical address of _that_ location too - so there's no benefit there.
It can be obtained simply by: __pa(l2x0_saved_regs) in C code, and saving that in a location known to the platform prior to suspend.

Yeah, that's why I want to get imx6q stay away from this infrastructure right now. I do not see any simplicity and cleanup on imx6q current code by migrating to this infrastructure.

Signature · 2 lines

Regards,
Shawn

Russell King - ARM Linux

2011-09-29 12:50 UTC #9 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Quoted 3 lines · Shawn Guo

Yeah, that's why I want to get imx6q stay away from this infrastructure right now. I do not see any simplicity and cleanup on imx6q current code by migrating to this infrastructure.

Why? If the data is already saved for you, then there's no reason not to use it. The fact that some generic code doesn't give you _exactly_ everything you'd want is not a reason to avoid it.

The cleanup for imx6q is that it would no longer have to have its own distinct code for saving the register values - and that's a danmed good thing.

The idea here is that we consolidate what _can_ be consolidated (which is the register saving.)

If you feel soo strongly that it's not worth doing, then let's stop wasting time and review effort on this, and instead have _every_ SoC implementing their own private L2 cache handling on resume.

Shawn Guo

2011-09-29 13:12 UTC #10 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Shawn Guo
Yeah, that's why I want to get imx6q stay away from this infrastructure right now. I do not see any simplicity and cleanup on imx6q current code by migrating to this infrastructure.

Quoted 8 lines

Why? If the data is already saved for you, then there's no reason not to use it. The fact that some generic code doesn't give you _exactly_ everything you'd want is not a reason to avoid it.
The cleanup for imx6q is that it would no longer have to have its own distinct code for saving the register values - and that's a danmed good thing.

It cleans up the register saving but requires additional code handling physical address of l2x0_saved_regs.

Quoted 7 lines

The idea here is that we consolidate what _can_ be consolidated (which is the register saving.)
If you feel soo strongly that it's not worth doing, then let's stop wasting time and review effort on this, and instead have _every_ SoC implementing their own private L2 cache handling on resume.

This infrastructure is definitely good thing for platform that L2 will be lost during suspend. But for imx6q which retains L2, I would not migrate it until the physical base of L2 and l2x0_saved_regs itself can be retrieved from infrastructure too.

Signature · 2 lines

Regards,
Shawn

Russell King - ARM Linux

2011-09-29 13:26 UTC #11 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Quoted 20 lines · Shawn Guo

Quoted 5 lines · Russell King - ARM Linux
Quoted 3 lines · Shawn Guo
Yeah, that's why I want to get imx6q stay away from this infrastructure right now. I do not see any simplicity and cleanup on imx6q current code by migrating to this infrastructure.
Quoted 8 lines
Why? If the data is already saved for you, then there's no reason not to use it. The fact that some generic code doesn't give you _exactly_ everything you'd want is not a reason to avoid it.
The cleanup for imx6q is that it would no longer have to have its own distinct code for saving the register values - and that's a danmed good thing.
It cleans up the register saving but requires additional code handling physical address of l2x0_saved_regs.
Quoted 3 lines
The idea here is that we consolidate what _can_ be consolidated (which is the register saving.)

Quoted 8 lines

Quoted 4 lines
If you feel soo strongly that it's not worth doing, then let's stop wasting time and review effort on this, and instead have _every_ SoC implementing their own private L2 cache handling on resume.
This infrastructure is definitely good thing for platform that L2 will be lost during suspend. But for imx6q which retains L2, I would not migrate it until the physical base of L2 and l2x0_saved_regs itself can be retrieved from infrastructure too.

Putting the physical base of the L2 cache inside l2x0_saved_regs is trivial (and I think will probably be done.)

What _can't_ be done - because it's _totally_ idiotic as I previously described - is to do anything about the physical address of l2x0_saved_regs itself.

Look - think about it for a moment. Let's say we do:

struct l2x0_saved_regs l2x0_saved_regs;
unsigned long phys_l2x0_saved_regs;

l2x0_init()

{
	phys_l2x0_saved_regs = __pa(&l2x0_saved_regs);
}

Does this buy us anything? It most certainly does not - it actually buys us additional complexity, because now we need to have platform code knowing the _physical_ address of phys_l2x0_saved_regs in order to get the _physical_ address of l2x0_saved_regs. Totally idiotic and pointless - platform code might as well just use __pa(&l2x0_saved_regs) directly in its suspend path to place it _somewhere_ that its own L2 resume code (which common code has no knowledge of) can access.

It's not that big a deal in any case - you can do this in your platform code:

.data
.globl phys_l2x0_saved_regs
phys_l2x0_saved_regs:

	.long	0

which the resume code can then access directly - and then obtain the values directly from the l2x0_saved_reg structure.

And in your suspend initialization:

extern unsigned long phys_l2x0_saved_regs;

int my_suspend_init()
{
...

	phys_l2x0_saved_regs = __pa(&l2x0_saved_regs);
}

So I think you're barking up the wrong tree if you think there's *anything* which generic code can do to make access to l2x0_saved_regs any easier than it is in this patch.

Shawn Guo

2011-09-29 13:50 UTC #12 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Quoted 73 lines · Russell King - ARM Linux

Quoted 20 lines · Shawn Guo
Quoted 5 lines · Russell King - ARM Linux
Quoted 3 lines · Shawn Guo
Yeah, that's why I want to get imx6q stay away from this infrastructure right now. I do not see any simplicity and cleanup on imx6q current code by migrating to this infrastructure.
Quoted 8 lines
Why? If the data is already saved for you, then there's no reason not to use it. The fact that some generic code doesn't give you _exactly_ everything you'd want is not a reason to avoid it.
The cleanup for imx6q is that it would no longer have to have its own distinct code for saving the register values - and that's a danmed good thing.
It cleans up the register saving but requires additional code handling physical address of l2x0_saved_regs.
Quoted 3 lines
The idea here is that we consolidate what _can_ be consolidated (which is the register saving.)
Quoted 8 lines
Quoted 4 lines
If you feel soo strongly that it's not worth doing, then let's stop wasting time and review effort on this, and instead have _every_ SoC implementing their own private L2 cache handling on resume.
This infrastructure is definitely good thing for platform that L2 will be lost during suspend. But for imx6q which retains L2, I would not migrate it until the physical base of L2 and l2x0_saved_regs itself can be retrieved from infrastructure too.
Putting the physical base of the L2 cache inside l2x0_saved_regs is trivial (and I think will probably be done.)
What _can't_ be done - because it's _totally_ idiotic as I previously described - is to do anything about the physical address of l2x0_saved_regs itself.
Look - think about it for a moment. Let's say we do:
struct l2x0_saved_regs l2x0_saved_regs;
unsigned long phys_l2x0_saved_regs;
l2x0_init()
{
	phys_l2x0_saved_regs = __pa(&l2x0_saved_regs);
}
Does this buy us anything? It most certainly does not - it actually buys us additional complexity, because now we need to have platform code knowing the _physical_ address of phys_l2x0_saved_regs in order to get the _physical_ address of l2x0_saved_regs. Totally idiotic and pointless - platform code might as well just use __pa(&l2x0_saved_regs) directly in its suspend path to place it _somewhere_ that its own L2 resume code (which common code has no knowledge of) can access.
It's not that big a deal in any case - you can do this in your platform code:
.data
.globl phys_l2x0_saved_regs
phys_l2x0_saved_regs:
	.long	0
which the resume code can then access directly - and then obtain the values directly from the l2x0_saved_reg structure.
And in your suspend initialization:
extern unsigned long phys_l2x0_saved_regs;
int my_suspend_init()
{
...

Quoted 7 lines

	phys_l2x0_saved_regs = __pa(&l2x0_saved_regs);
}
So I think you're barking up the wrong tree if you think there's *anything* which generic code can do to make access to l2x0_saved_regs any easier than it is in this patch.

Yes, you are right. __pa(&l2x0_saved_regs) can be handled in imx6q platform code.

Signature · 2 lines

Regards,
Shawn

Barry Song

2011-09-29 09:44 UTC #13 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

In reply to Shawn Guo

2011/9/29 Shawn Guo <[email protected]>:

Quoted 18 lines · Barry Song

Quoted 12 lines
we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---
?-v2:
?define the structure l2x0_regs to hold all saved registers;
?make saved copy of registers non-static so that platform codes can access them; ?add asm_offset for l2x0 saved regs(l2x0_regs struct) so that asm codes can use
I'm trying to migrate imx6q to this infrastructure, but seeing some facts that might stop me from the effort. ?Please help me understand these facts are correct.

Quoted 7 lines

It seems to me that the only benefit that imx6q gains from the infrastructure is the register saving, and I fail to see any benefit on the restoring.
?* I still have to save physical base of l2x0 for register restoring. ?* I do not have the physical address of l2x0_saved_regs, and I have ? to manage to get it somehow.

well. the phyical address should be added so that mmu-off asm codes can get it.

Quoted 4 lines

--
Regards,
Shawn

-barry

Shawn Guo

2011-09-29 13:57 UTC #14 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

In reply to Barry Song

Quoted 7 lines · Barry Song

we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---

Barry,

You may want to fix those checkpatch error and warnings in v3.

total: 1 errors, 15 warnings, 181 lines checked

Signature · 2 lines

Regards,
Shawn

Barry Song

2011-09-29 14:55 UTC #15 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Hi Shawn,

2011/9/29 Shawn Guo <[email protected]>:

Quoted 12 lines · Barry Song

Quoted 7 lines
we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---
Barry,
You may want to fix those checkpatch error and warnings in v3.

Thanks. i will.
since people have basically agreen about what should be fixed and what should not be, i'd like to send v3 tomorrow. you might help to test on imx6q as well.

Quoted 6 lines

total: 1 errors, 15 warnings, 181 lines checked
--
Regards,
Shawn

-barry

Shawn Guo

2011-09-29 15:07 UTC #16 lore raw

PATCH v2 1/1 ARM: cache-l2x0: add resume entry for l2 in secure mode

Quoted 12 lines · Barry Song

Hi Shawn,
2011/9/29 Shawn Guo <[email protected]>:
Quoted 9 lines · Barry Song
Quoted 7 lines
we save the l2x0 registers at the first initialization, and platform codes can get them to restore l2x0 status after wakeup.
Cc Shawn Guo <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Signed-off-by Barry Song <[email protected]>
---

Quoted 9 lines

Quoted 3 lines
Barry,
You may want to fix those checkpatch error and warnings in v3.
Thanks. i will.
since people have basically agreen about what should be fixed and what should not be, i'd like to send v3 tomorrow. you might help to test on imx6q as well.

Sure.

Signature · 2 lines

Regards,
Shawn