OMAP4: PM: suspend, CPU-hotplug and CPUilde support

On certain architectures, there might be a need to mark certain addresses with strongly ordered memory attributes to avoid ordering issues at the interconnect level.

Quoted 3 lines · Santosh Shilimkar

On certain architectures, there might be a need to mark certain addresses with strongly ordered memory attributes to avoid ordering issues at the interconnect level.

This is something Russell needs to look.

You might want to also read the mailing list archives regarding the strongly ordered access.

Basically it still won't guarantee that the write gets to the device, only a read back from the device in question guarantees that at the bus level.

On OMAP4 SOC intecronnects has many write buffers in the async bridges and they can be drained only with stongly ordered accesses.

There are two ports as below from MPU and both needs to be drained.
- MPU --> L3 T2ASYNC FIFO
- MPU --> DDR T2ASYNC FIFO

Without the interconnect barriers, many issues have been observed leading to system freeze, CPU deadlocks, random crashes with register accesses, synchronization loss on initiators operating on both interconnect port simultaneously.

Quoted 2 lines · Santosh Shilimkar

On OMAP4 SOC intecronnects has many write buffers in the async bridges and they can be drained only with stongly ordered accesses.

This is not correct, strongly ordered access does not guarantee anything here. If it fixes issues, it's because it makes the writes to reach the device faster. Strongly ordered does not affect anything outside ARM, so the bus access won't change.

The only real fix here is to do a read back of the device in question to guarantee the write got to the device.

Quoted 4 lines

There are two ports as below from MPU and both needs to be drained. ? ? ? - MPU --> L3 T2ASYNC FIFO
? ? ? - MPU --> DDR T2ASYNC FIFO

Quoted 4 lines

Without the interconnect barriers, many issues have been observed leading to system freeze, CPU deadlocks, random crashes with register accesses, synchronization loss on initiators operating on both interconnect port simultaneously.

We had these issues for omap3 too. Adding a few read backs solved those kinds of issues.

Quoted 2 lines · Santosh Shilimkar

On OMAP4 SOC intecronnects has many write buffers in the async bridges and they can be drained only with stongly ordered accesses.

This is not correct, strongly ordered access does not guarantee anything here. If it fixes issues, it's because it makes the writes to reach the device faster. Strongly ordered does not affect anything outside ARM, so the bus access won't change.

Quoted 9 lines · Tony Lindgren

Quoted 2 lines · Santosh Shilimkar

On OMAP4 SOC intecronnects has many write buffers in the async bridges and they can be drained only with stongly ordered accesses.

This is not correct, strongly ordered access does not guarantee anything here. If it fixes issues, it's because it makes the writes to reach the device faster. Strongly ordered does not affect anything outside ARM, so the bus access won't change.

What I said is the aync bridges WB and what is said is correct from MPU accesses point of view.

It's not about faster or slower. With device memory the, writes can get stuck into write buffers where as with SO, the write buffers will be bypassed.

The behaviours is limited to the MPU side async bridge boundary which is the problem. The statement is not for l3 and l4 interconnect which probably you mean.

There is always a hardware signal to communicate CPU at async bridges to ensure that data is not stuck in these bridges before CPU clock/voltage is cut. Unfortunately we have a BUG on OMAP44XX devices and the dual channel makes it even worst since both pipes have the same BUG. So what we are doing is issuing SO write/read accesses on these pipes so that there is nothing stuck there before MPU hits low power states and also avoids any race conditions when both channels are used together by some initiators. The behaviour is validated at RTL level and there is no ambiguity about it.

May be you have mistaken the L3 and L4 as the interconnect levels in this case.

Quoted 31 lines · Santosh

Quoted 9 lines · Tony Lindgren

Quoted 2 lines · Santosh Shilimkar

On OMAP4 SOC intecronnects has many write buffers in the async bridges and they can be drained only with stongly ordered accesses.

This is not correct, strongly ordered access does not guarantee anything here. If it fixes issues, it's because it makes the writes to reach the device faster. Strongly ordered does not affect anything outside ARM, so the bus access won't change.

What I said is the aync bridges WB and what is said is correct from MPU accesses point of view.

It's not about faster or slower. With device memory the, writes can get stuck into write buffers where as with SO, the write buffers will be bypassed.

The behaviours is limited to the MPU side async bridge boundary which is the problem. The statement is not for l3 and l4 interconnect which probably you mean.

There is always a hardware signal to communicate CPU at async bridges to ensure that data is not stuck in these bridges before CPU clock/voltage is cut. Unfortunately we have a BUG on OMAP44XX devices and the dual channel makes it even worst since both pipes have the same BUG. So what we are doing is issuing SO write/read accesses on these pipes so that there is nothing stuck there before MPU hits low power states and also avoids any race conditions when both channels are used together by some initiators. The behaviour is validated at RTL level and there is no ambiguity about it.

Quoted 2 lines

May be you have mistaken the L3 and L4 as the interconnect levels in this case.

Sounds to me like the changelog needs to be a bit more verbose.

Remember, we're all probably going to forget the gory details of this in a few months and want to be able to go back to the code w/changelog to refresh our memories.

Quoted 36 lines · Kevin Hilman

Quoted 33 lines · Santosh

Quoted 9 lines · Tony Lindgren

Quoted 2 lines · Santosh Shilimkar

On OMAP4 SOC intecronnects has many write buffers in the async bridges and they can be drained only with stongly ordered accesses.

This is not correct, strongly ordered access does not guarantee anything here. If it fixes issues, it's because it makes the writes to reach the device faster. Strongly ordered does not affect anything outside ARM, so the bus access won't change.

What I said is the aync bridges WB and what is said is correct from MPU accesses point of view.

It's not about faster or slower. With device memory the, writes can get stuck into write buffers where as with SO, the write buffers will be bypassed.

The behaviours is limited to the MPU side async bridge boundary which is the problem. The statement is not for l3 and l4 interconnect which probably you mean.

There is always a hardware signal to communicate CPU at async bridges to ensure that data is not stuck in these bridges before CPU clock/voltage is cut. Unfortunately we have a BUG on OMAP44XX devices and the dual channel makes it even worst since both pipes have the same BUG. So what we are doing is issuing SO write/read accesses on these pipes so that there is nothing stuck there before MPU hits low power states and also avoids any race conditions when both channels are used together by some initiators. The behaviour is validated at RTL level and there is no ambiguity about it.

May be you have mistaken the L3 and L4 as the interconnect levels in this case.

Quoted 6 lines

Sounds to me like the changelog needs to be a bit more verbose.

Remember, we're all probably going to forget the gory details of this in a few months and want to be able to go back to the code w/changelog to refresh our memories.

Change log was accurate but I agree it needs more description to make it clear. I plan to update it.

Quoted 43 lines · Shilimkar, Santosh

Quoted 42 lines · Kevin Hilman

Quoted 31 lines · Santosh

Quoted 9 lines · Tony Lindgren

Quoted 2 lines · Santosh Shilimkar

On OMAP4 SOC intecronnects has many write buffers in the async bridges and they can be drained only with stongly ordered accesses.

This is not correct, strongly ordered access does not guarantee anything here. If it fixes issues, it's because it makes the writes to reach the device faster. Strongly ordered does not affect anything outside ARM, so the bus access won't change.

What I said is the aync bridges WB and what is said is correct from MPU accesses point of view.

It's not about faster or slower. With device memory the, writes can get stuck into write buffers where as with SO, the write buffers will be bypassed.

The behaviours is limited to the MPU side async bridge boundary which is the problem. The statement is not for l3 and l4 interconnect which probably you mean.

There is always a hardware signal to communicate CPU at async bridges to ensure that data is not stuck in these bridges before CPU clock/voltage is cut. Unfortunately we have a BUG on OMAP44XX devices and the dual channel makes it even worst since both pipes have the same BUG. So what we are doing is issuing SO write/read accesses on these pipes so that there is nothing stuck there before MPU hits low power states and also avoids any race conditions when both channels are used together by some initiators. The behaviour is validated at RTL level and there is no ambiguity about it.

Quoted 2 lines

May be you have mistaken the L3 and L4 as the interconnect levels in this case.

Sounds to me like the changelog needs to be a bit more verbose.

Remember, we're all probably going to forget the gory details of this in a few months and want to be able to go back to the code w/changelog to refresh our memories.

Quoted 2 lines

Change log was accurate but I agree it needs more description to make it clear. I plan to update it.

Please also include the errata in the description and set it up with a Kconfig entry with something like ARM_ERRATA_XXXXXX or TI_ERRATA_XXXXXX.

Also it would be best to apply this fix at the end of the PM series so it is easier to verify the bug and potentially remove the hack if a better fix is ever available.

Quoted 4 lines · Tony Lindgren

Please also include the errata in the description and set it up with a Kconfig entry with something like ARM_ERRATA_XXXXXX or TI_ERRATA_XXXXXX.

Sure.
It's a TI ERRATA.

Quoted 4 lines

Also it would be best to apply this fix at the end of the PM series so it is easier to verify the bug and potentially remove the hack if a better fix is ever available.

As such order doesn't matter much because it can be removed either way even if it is in the beginning of the series with KCONFIG entry.

But I can change the order if you prefer that way.

Quoted 7 lines · Shilimkar, Santosh

Quoted 4 lines · Tony Lindgren

Please also include the errata in the description and set it up with a Kconfig entry with something like ARM_ERRATA_XXXXXX or TI_ERRATA_XXXXXX.

Sure.
It's a ?TI ERRATA.

OK

Quoted 7 lines

Quoted 4 lines

Also it would be best to apply this fix at the end of the PM series so it is easier to verify the bug and potentially remove the hack if a better fix is ever available.

As such order doesn't matter much because it can be removed either way even if it is in the beginning of the series with KCONFIG entry.

Quoted 1 line

But I can change the order if you prefer that way.

Well it seems that it's the omap4_finish_suspend in this series that can be used to trigger the bug, although the bug has been around for few years. So then instead of mentioning random hangups you can have a better description with something like "Patch xyz added PM idle support for omap4, however bug 123 causes so and so. Fix the issue with blah blah".

Also, if you have some easy way to reproduce the bug maybe mention that too? That would make it easy to verify if issue has been fixed.

Quoted 20 lines · Tony Lindgren

Quoted 7 lines · Shilimkar, Santosh

Quoted 4 lines · Tony Lindgren

Please also include the errata in the description and set it up with a Kconfig entry with something like ARM_ERRATA_XXXXXX or TI_ERRATA_XXXXXX.

Sure.
It's a ?TI ERRATA.

OK

Quoted 8 lines

Quoted 4 lines

Also it would be best to apply this fix at the end of the PM series so it is easier to verify the bug and potentially remove the hack if a better fix is ever available.

As such order doesn't matter much because it can be removed either way even if it is in the beginning of the series with KCONFIG entry.

But I can change the order if you prefer that way.

Quoted 7 lines

Well it seems that it's the omap4_finish_suspend in this series that can be used to trigger the bug, although the bug has been around for few years. So then instead of mentioning random hangups you can have a better description with something like "Patch xyz added PM idle support for omap4, however bug 123 causes so and so. Fix the issue with blah blah".

Sounds good .

Quoted 3 lines

Also, if you have some easy way to reproduce the bug maybe mention that too? That would make it easy to verify if issue has been fixed.

There are some multimedia usecases where the bug was discovered but on mainline obviously we don't have that support.

I will check with IP folks if any other simple test-case is possible to reproduce the issue and If I find one, will mention that.

OMAP4 L2X0 initialisation code uses BUG_ON() for the ioremap() failure scenarios.

Use WARN_ON() instead and allow graceful function exits.

This was suggsted by Kevin Hilman<[email protected]> during OMAP4 PM code review.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

---
  arch/arm/mach-omap2/omap4-common.c |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/omap4-common.c +0 -0

index 4791370..4904025 100644

Hello.

Quoted 21 lines · Santosh Shilimkar

OMAP4 L2X0 initialisation code uses BUG_ON() for the ioremap() failure scenarios.

Use WARN_ON() instead and allow graceful function exits.

This was suggsted by Kevin Hilman<[email protected]> during OMAP4 PM code review.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

---
arch/arm/mach-omap2/omap4-common.c | 3 ++-

1 files changed, 2 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/omap4-common.c +0 -0

index 4791370..4904025 100644
@@ -121,7 +121,8 @@ static int __init omap_l2_cache_init(void)
 

arch/arm/mach-omap2/omap4-common.c static int __init omap_l2_cache_init(void) · line 125

Quoted 5 lines

arch/arm/mach-omap2/omap4-common.c +2 -1

 /* Static mapping, never released */
 l2cache_base = ioremap(OMAP44XX_L2CACHE_BASE, SZ_4K);
- BUG_ON(!l2cache_base);
+ if (WARN_ON(!l2cache_base))
+ return -ENODEV;

Rather ENOMEM...

Quoted 25 lines · Sergei Shtylyov

Hello.

Quoted 21 lines · Santosh Shilimkar

OMAP4 L2X0 initialisation code uses BUG_ON() for the ioremap() failure scenarios.

Use WARN_ON() instead and allow graceful function exits.

This was suggsted by Kevin Hilman<[email protected]> during OMAP4 PM code review.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

---
arch/arm/mach-omap2/omap4-common.c | 3 ++-

1 files changed, 2 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/omap4-common.c +0 -0

index 4791370..4904025 100644
@@ -121,7 +121,8 @@ static int __init omap_l2_cache_init(void)
 

Quoted 8 lines

arch/arm/mach-omap2/omap4-common.c static int __init omap_l2_cache_init(void) · line 125

Quoted 5 lines

arch/arm/mach-omap2/omap4-common.c +2 -1

 /* Static mapping, never released */
 l2cache_base = ioremap(OMAP44XX_L2CACHE_BASE, SZ_4K);
- BUG_ON(!l2cache_base);
+ if (WARN_ON(!l2cache_base))
+ return -ENODEV;

Rather ENOMEM...

Nope. Even though it's related to memory, it's
a memory controller so DEV is right.

Quoted 1 line · Santosh

On Monday 05 September 2011 03:41 PM, Sergei Shtylyov wrote:

Quoted 10 lines

Quoted 8 lines

Quoted 5 lines

/* Static mapping, never released */ l2cache_base = ioremap(OMAP44XX_L2CACHE_BASE, SZ_4K);
- BUG_ON(!l2cache_base);
+ if (WARN_ON(!l2cache_base))
+ return -ENODEV;

Rather ENOMEM...

Nope. Even though it's related to memory, it's
a memory controller so DEV is right.

No it's not. The most likely reason ioremap() fails is because:

1. it's run out of memory for the metadata or page tables.
2. you passed it an invalid memory type (unlikely, you're using ioremap which uses a fixed known good type).
3. the address was>4GB but was not section aligned (also unlikely).

Therefore, the error code should be "out of memory" not "no such device".

Quoted 4 lines · Russell King - ARM Linux

Quoted 2 lines · Santosh

On Monday 05 September 2011 03:41 PM, Sergei Shtylyov wrote:

[..]

Quoted 13 lines

Quoted 10 lines

Quoted 8 lines

Quoted 5 lines

/* Static mapping, never released */ l2cache_base = ioremap(OMAP44XX_L2CACHE_BASE, SZ_4K);
- BUG_ON(!l2cache_base);
+ if (WARN_ON(!l2cache_base))
+ return -ENODEV;

Rather ENOMEM...

Nope. Even though it's related to memory, it's
a memory controller so DEV is right.

No it's not. ?The most likely reason ioremap() fails is because:

Quoted 6 lines

1. it's run out of memory for the metadata or page tables.
2. you passed it an invalid memory type (unlikely, you're using ioremap ? ?which uses a fixed known good type).
3. the address was>4GB but was not section aligned (also unlikely).

Therefore, the error code should be "out of memory" not "no such device".

Got it. Will fix that.

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

As per OMAP4430 TRM, the dynamic dependency between MPUSS -> EMIF and MPUSS -> L3* and DUCATI -> L3 *clockdomains is enable by default. Refer register CM_MPU_DYNAMICDEP description for details.

But these dynamic dependencies doesn't work as expected. The hardware recommendation is to keep above dependencies.

Without this system locks up or randomly crashesh.

Signed-off-by Rajendra Nayak <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

Acked-by Paul Walmsley <[email protected]>

Cc Kevin Hilman <[email protected]>

---
 arch/arm/mach-omap2/pm44xx.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/pm44xx.c +2 -0

index 4bbc6fe..35ba028 100644
@@ -116,6 +116,8 @@ static void omap_default_idle(void)
 static int __init omap4_pm_init(void)
 {
 	int ret;
+	struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm;
+	struct clockdomain *ducati_clkdm, *l3_2_clkdm;
 
 	if (!cpu_is_omap44xx())
 		return -ENODEV;

arch/arm/mach-omap2/pm44xx.c +15 -0

+	 * Without this system locks up or randomly crashesh.
+	 */
+	mpuss_clkdm = clkdm_lookup("mpuss_clkdm");
+	emif_clkdm = clkdm_lookup("l3_emif_clkdm");
+	l3_1_clkdm = clkdm_lookup("l3_1_clkdm");
+	l3_2_clkdm = clkdm_lookup("l3_2_clkdm");
+	ducati_clkdm = clkdm_lookup("ducati_clkdm");
+	if ((!mpuss_clkdm) || (!emif_clkdm) || (!l3_1_clkdm) ||
+			(!l3_2_clkdm) || (!ducati_clkdm))
+		goto err2;
+
+	ret = clkdm_add_wkdep(mpuss_clkdm, emif_clkdm);
+	ret |= clkdm_add_wkdep(mpuss_clkdm, l3_1_clkdm);
+	ret |= clkdm_add_wkdep(mpuss_clkdm, l3_2_clkdm);
+	ret |= clkdm_add_wkdep(ducati_clkdm, l3_1_clkdm);

Quoted 6 lines · Santosh Shilimkar

As per OMAP4430 TRM, the dynamic dependency between MPUSS -> EMIF and MPUSS -> L3* and DUCATI -> L3 *clockdomains is enable by default. Refer register CM_MPU_DYNAMICDEP description for details.

But these dynamic dependencies doesn't work as expected. The hardware recommendation is to keep above dependencies.

Minor nit on changelog wording...

The use of "above dependencies" here is a bit confusing, since the only thing refered to above are dynamic dependencies.

I think this should be worked to be clear that what is being kept are the *static* dependencies (assuming I understand it correctly.)

Quoted 2 lines

Without this system locks up or randomly crashesh.

Is 'crashesh' a particular kind of crash. maybe a silent one? ;)

On OMAP secure/emulation devices, certain APIs are exported by secure code. Add an infrastructure so that relevant operations on secure devices can be implemented using it.

While at this, omap44xx-smc.S to omap-smc.S since the common APIs can be used on other OMAP's too.

Santosh,

On Sun, Sep 4, 2011 at 3:54 PM, Santosh Shilimkar
<[email protected]> wrote:

Quoted 6 lines

On OMAP secure/emulation devices, certain APIs are exported by secure code. Add an infrastructure so that relevant operations on secure devices can be implemented using it.

While at this, omap44xx-smc.S to omap-smc.S since the common APIs can be used on other OMAP's too.

'move' or 'rename' is missing in the sentence.

Allocate the memory to save secure ram context which needs to be done when MPU is hitting off mode.

The ROM code expects a physical address to this memory and hence use memblock APIs to reserve this memory as part of .reserve() callback.

Maximum possible size is allocated and can cater to OMAP3XXX / OMAP4XXX secure RAM size requirements.

Signed-off-by Santosh Shilimkar <[email protected]>

---

?arch/arm/mach-omap2/include/mach/omap-secure.h | ? ?4 +++
?arch/arm/mach-omap2/omap-secure.c ? ? ? ? ? ? ?| ? 29 ++++++++++++++++++++++++
?arch/arm/plat-omap/common.c ? ? ? ? ? ? ? ? ? ?| ? ?3 ++

?3 files changed, 36 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/include/mach/omap-secure.h +0 -0

index 26e7bcc..e2f95a0 100644
@@ -26,6 +26,8 @@

?#define FLAG_FIQ_ENABLE ? ? ? ? ? ? ? ? ? ? ? ?0x1 ?#define NO_FLAG ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0x0

Quoted 25 lines · Santosh Shilimkar

Allocate the memory to save secure ram context which needs to be done when MPU is hitting off mode.

The ROM code expects a physical address to this memory and hence use memblock APIs to reserve this memory as part of .reserve() callback.

Maximum possible size is allocated and can cater to OMAP3XXX / OMAP4XXX secure RAM size requirements.

Signed-off-by Santosh Shilimkar<[email protected]>

---
  arch/arm/mach-omap2/include/mach/omap-secure.h |    4 +++
  arch/arm/mach-omap2/omap-secure.c              |   29 ++++++++++++++++++++++++
  arch/arm/plat-omap/common.c                    |    3 ++
  3 files changed, 36 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/include/mach/omap-secure.h +0 -0

index 26e7bcc..e2f95a0 100644
@@ -26,6 +26,8 @@
  #define FLAG_FIQ_ENABLE                        0x1
  #define NO_FLAG                                0x0
 

arch/arm/mach-omap2/include/mach/omap-secure.h line 30

Quoted 2 lines

arch/arm/mach-omap2/include/mach/omap-secure.h +2 -0

+/* Maximum Secure memory storage size */
+#define OMAP_SECURE_RAM_STORAGE        (88 * SZ_1K)

Is this valid for all supported devices? How to differentiate variations in the size for new chips variants in the future?

Quoted 31 lines · Jean Pihet

Quoted 28 lines · On Sun, Sep 4, 2011 at 3:54 PM, Santosh Shilimkar <[email protected]>…

Allocate the memory to save secure ram context which needs to be done when MPU is hitting off mode.

The ROM code expects a physical address to this memory and hence use memblock APIs to reserve this memory as part of .reserve() callback.

Maximum possible size is allocated and can cater to OMAP3XXX / OMAP4XXX secure RAM size requirements.

Signed-off-by Santosh Shilimkar<[email protected]>

---
?arch/arm/mach-omap2/include/mach/omap-secure.h | ? ?4 +++ ?arch/arm/mach-omap2/omap-secure.c ? ? ? ? ? ? ?| ? 29
++++++++++++++++++++++++
?arch/arm/plat-omap/common.c ? ? ? ? ? ? ? ? ? ?| ? ?3 ++ ?3 files changed, 36 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/include/mach/omap-secure.h +0 -0

index 26e7bcc..e2f95a0 100644
@@ -26,6 +26,8 @@

?#define FLAG_FIQ_ENABLE ? ? ? ? ? ? ? ? ? ? ? ?0x1 ?#define NO_FLAG ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0x0

Quoted 6 lines

arch/arm/mach-omap2/include/mach/omap-secure.h

Quoted 2 lines

arch/arm/mach-omap2/include/mach/omap-secure.h +2 -0

+/* Maximum Secure memory storage size */
+#define OMAP_SECURE_RAM_STORAGE ? ? ? ?(88 * SZ_1K)

Is this valid for all supported devices? How to differentiate variations in the size for new chips variants in the future?

You don't have to. ROM code does that job. We should just ensure that maximum needed memory is made available to secure code.

Regards
Santosh

Santosh,

Quoted 38 lines · Santosh

Quoted 37 lines · Jean Pihet

Quoted 28 lines · On Sun, Sep 4, 2011 at 3:54 PM, Santosh Shilimkar <[email protected]>…

Allocate the memory to save secure ram context which needs to be done when MPU is hitting off mode.

The ROM code expects a physical address to this memory and hence use memblock APIs to reserve this memory as part of .reserve() callback.

Maximum possible size is allocated and can cater to OMAP3XXX / OMAP4XXX secure RAM size requirements.

Signed-off-by Santosh Shilimkar<[email protected]>

---
?arch/arm/mach-omap2/include/mach/omap-secure.h | ? ?4 +++ ?arch/arm/mach-omap2/omap-secure.c ? ? ? ? ? ? ?| ? 29
++++++++++++++++++++++++
?arch/arm/plat-omap/common.c ? ? ? ? ? ? ? ? ? ?| ? ?3 ++ ?3 files changed, 36 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/include/mach/omap-secure.h +0 -0

index 26e7bcc..e2f95a0 100644
@@ -26,6 +26,8 @@

?#define FLAG_FIQ_ENABLE ? ? ? ? ? ? ? ? ? ? ? ?0x1 ?#define NO_FLAG ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0x0

arch/arm/mach-omap2/include/mach/omap-secure.h

Quoted 2 lines

arch/arm/mach-omap2/include/mach/omap-secure.h +2 -0

+/* Maximum Secure memory storage size */
+#define OMAP_SECURE_RAM_STORAGE ? ? ? ?(88 * SZ_1K)

Is this valid for all supported devices? How to differentiate variations in the size for new chips variants in the future?

Quoted 2 lines

You don't have to. ROM code does that job. We should just ensure that maximum needed memory is made available to secure code.

The question was: what would be the code if tomorrow -for example- we have OMAP4460 with 88K of secure RAM, OMAP5432 with 128K and OMAP6667 with 132K? How to code the differences?

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

The WakeupGen unit is responsible for generating wakeup event from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation, WakeupGen delivers external interrupts directly to the GIC.

Signed-off-by Santosh Shilimkar <[email protected]>

Cc Kevin Hilman <[email protected]>

Quoted 11 lines · Santosh Shilimkar

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

The WakeupGen unit is responsible for generating wakeup event from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation, WakeupGen delivers external interrupts directly to the GIC.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

[...]

arch/arm/mach-omap2/omap-wakeupgen.c line 166

Quoted 7 lines

arch/arm/mach-omap2/omap-wakeupgen.c new file +7 -0

+#ifdef CONFIG_PM
+/*
+ * Masking wakeup irqs is handled by the IRQCHIP_MASK_ON_SUSPEND flag,
+ * so no action is necessary in set_wake, but implement an empty handler
+ * here to prevent enable_irq_wake() returning an error.
+ * FIXME: Remove the dummy handler once gen irq code fix above.
+ */

Just curious... is there a fix for this pending for v3.2?

Quoted 16 lines · Kevin Hilman

Quoted 11 lines · Santosh Shilimkar

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

The WakeupGen unit is responsible for generating wakeup event from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation, WakeupGen delivers external interrupts directly to the GIC.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

[...]

Quoted 10 lines

Review

Quoted 7 lines

(quoted diff) +7 -0

+#ifdef CONFIG_PM
+/*
+ * Masking wakeup irqs is handled by the IRQCHIP_MASK_ON_SUSPEND flag,
+ * so no action is necessary in set_wake, but implement an empty handler
+ * here to prevent enable_irq_wake() returning an error.
+ * FIXME: Remove the dummy handler once gen irq code fix above.
+ */

Just curious... is there a fix for this pending for v3.2?

I have sent a proposed change to Thomas but have not seen any response yet from him.

http://www.spinics.net/lists/arm-kernel/msg134297.html

Quoted 31 lines · Santosh

Quoted 16 lines · Kevin Hilman

Quoted 11 lines · Santosh Shilimkar

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

The WakeupGen unit is responsible for generating wakeup event from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation, WakeupGen delivers external interrupts directly to the GIC.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

[...]

Quoted 10 lines

Review

Quoted 7 lines

(quoted diff) +7 -0

+#ifdef CONFIG_PM
+/*
+ * Masking wakeup irqs is handled by the IRQCHIP_MASK_ON_SUSPEND flag,
+ * so no action is necessary in set_wake, but implement an empty handler
+ * here to prevent enable_irq_wake() returning an error.
+ * FIXME: Remove the dummy handler once gen irq code fix above.
+ */

Just curious... is there a fix for this pending for v3.2?

I have sent a proposed change to Thomas but have not seen any response yet from him.

http://www.spinics.net/lists/arm-kernel/msg134297.html

kernel/irq/manage.c +3 -0

index 0a7840a..cd4bc01 100644
@@ -467,6 +467,9 @@ static int set_irq_wake_real(unsigned int irq, unsigned int on)
 
    struct irq_desc *desc = irq_to_desc(irq);
    int ret = -ENXIO;
 
+  if (irq_desc_get_chip(desc)->flags&  IRQCHIP_MASK_ON_SUSPEND)
+     return 0;
+
 	if (desc->irq_data.chip->irq_set_wake)
  	   ret = desc->irq_data.chip->irq_set_wake(&desc->irq_data, on);

The flag says: MASK ON SUSPEND and it does not imply that you don't need a wake function. There might be cases where you want to setup stuff in that function in order to have the wakeup happen on that interrupt line despite of the mask on suspend.

We either need a separate flag or a global dummy set_wake function in the core to avoid empty copies all over the place.

Quoted 6 lines · Thomas Gleixner

The flag says: MASK ON SUSPEND and it does not imply that you don't need a wake function. There might be cases where you want to setup stuff in that function in order to have the wakeup happen on that interrupt line despite of the mask on suspend.

I see your point.

Quoted 3 lines

We either need a separate flag or a global dummy set_wake function in the core to avoid empty copies all over the place.

A flag is probably better since you mentioned that on some arch, there might be need to have actual set_wake() handler. Or if the global dummy can be over-ridden by platform, that's fine too.

Quoted 7 lines · Santosh

Quoted 6 lines · Thomas Gleixner

The flag says: MASK ON SUSPEND and it does not imply that you don't need a wake function. There might be cases where you want to setup stuff in that function in order to have the wakeup happen on that interrupt line despite of the mask on suspend.

Quoted 8 lines

I see your point.

Quoted 3 lines

We either need a separate flag or a global dummy set_wake function in the core to avoid empty copies all over the place.

A flag is probably better since you mentioned that on some arch, there might be need to have actual set_wake() handler. Or if the global dummy can be over-ridden by platform, that's fine too.

Global dummy would mean:

int irq_set_wake_dummy(...)

{
	return 0;
}

And you just add it to your chip, but either way I don't care whether it's a dummy function or a flag.

Quoted 24 lines · Thomas Gleixner

Quoted 7 lines · Santosh

Quoted 6 lines · Thomas Gleixner

The flag says: MASK ON SUSPEND and it does not imply that you don't need a wake function. There might be cases where you want to setup stuff in that function in order to have the wakeup happen on that interrupt line despite of the mask on suspend.

Quoted 8 lines

I see your point.

Quoted 3 lines

We either need a separate flag or a global dummy set_wake function in the core to avoid empty copies all over the place.

A flag is probably better since you mentioned that on some arch, there might be need to have actual set_wake() handler. Or if the global dummy can be over-ridden by platform, that's fine too.

Global dummy would mean:

int irq_set_wake_dummy(...)

{
	return 0;
}

Quoted 3 lines

And you just add it to your chip, but either way I don't care whether it's a dummy function or a flag.

Will below patch work for you then ?
Attaching the same in case mailer damages it.

Regards,
Santosh

From d63d4347dc8fb144b19f4d4e7c0621397cccea94 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 9 Sep 2011 13:59:35 +0530
Subject: [PATCH] irq: Add IRQCHIP_SKIP_SET_WAKE flag to avoid need of dummy set_wake() handler.

Certain IRQCHIP's may not need to install the irq_set_wake() handler if the IRQCHIP_MASK_ON_SUSPEND flag is set. But but if it's not implemented, enable_irq_wake() will return an error. That needs the IRQCHIP to install an empty set_wake handler.

Add an 'IRQCHIP_SKIP_SET_WAKE' flag so that IRQCHIP can inform the core irq code that irq_set_wake() handler is not necessary.

Signed-off-by Santosh Shilimkar <[email protected]>

Cc Thomas Gleixner <[email protected]>

On Fri, 9 Sep 2011, Santosh wrote:

Quoted 11 lines

From d63d4347dc8fb144b19f4d4e7c0621397cccea94 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar<[email protected]>
Date: Fri, 9 Sep 2011 13:59:35 +0530
Subject: [PATCH] irq: Add IRQCHIP_SKIP_SET_WAKE flag to avoid need of dummy set_wake() handler.

Certain IRQCHIP's may not need to install the irq_set_wake() handler if the IRQCHIP_MASK_ON_SUSPEND flag is set. But but if it's not implemented, enable_irq_wake() will return an error. That needs the IRQCHIP to install an empty set_wake handler.

Quoted 5 lines

Add an 'IRQCHIP_SKIP_SET_WAKE' flag so that IRQCHIP can inform the core irq code that irq_set_wake() handler is not necessary.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Thomas Gleixner<[email protected]>

Queued to irq/core for 3.2

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

The WakeupGen unit is responsible for generating wakeup event

from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation,

WakeupGen delivers external interrupts directly to the GIC.

Signed-off-by Santosh Shilimkar <[email protected]>

Cc Kevin Hilman <[email protected]>

---

?arch/arm/mach-omap2/Makefile ? ? ? ? ? ? ? ? ? ? ?| ? ?2 +-
?arch/arm/mach-omap2/include/mach/omap-wakeupgen.h | ? 39 ++++
?arch/arm/mach-omap2/omap-wakeupgen.c ? ? ? ? ? ? ?| ?241 +++++++++++++++++++++
?arch/arm/mach-omap2/omap4-common.c ? ? ? ? ? ? ? ?| ? ?3 +

?4 files changed, 284 insertions(+), 1 deletions(-) ?create mode 100644 arch/arm/mach-omap2/include/mach/omap-wakeupgen.h ?create mode 100644 arch/arm/mach-omap2/omap-wakeupgen.c

+ */

(quoted diff) +5 -0

+int __init omap_wakeupgen_init(void)
+{
+ ? ? ? int i;
+ ? ? ? unsigned int boot_cpu = smp_processor_id();
+

Quoted 4 lines · Santosh Shilimkar

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

Quoted 1 line

The WakeupGen unit is responsible for generating wakeup event

... for generating the wakeup events

Quoted 2 lines

from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation,

in the MPU...

Quoted 1 line

WakeupGen delivers external interrupts directly to the GIC.

... delivers the external interrupts

Quoted 11 lines

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

---
  arch/arm/mach-omap2/Makefile                      |    2 +-
  arch/arm/mach-omap2/include/mach/omap-wakeupgen.h |   39 ++++
  arch/arm/mach-omap2/omap-wakeupgen.c              |  241 +++++++++++++++++++++
  arch/arm/mach-omap2/omap4-common.c                |    3 +
  4 files changed, 284 insertions(+), 1 deletions(-)

create mode 100644 arch/arm/mach-omap2/include/mach/omap-wakeupgen.h create mode 100644 arch/arm/mach-omap2/omap-wakeupgen.c

...

Quoted 9 lines

+ */

arch/arm/mach-omap2/omap-wakeupgen.c new file +8 -0

+int __init omap_wakeupgen_init(void)
+{
+       int i;
+       unsigned int boot_cpu = smp_processor_id();
+
+       /* Not supported on OMAP4 ES1.0 silicon */
+       if (omap_rev() == OMAP4430_REV_ES1_0) {
+               WARN(1, "WakeupGen: Not supported on OMAP4430 ES1.0\n");

arch/arm/mach-omap2/omap-wakeupgen.c line 214

Quoted 7 lines

arch/arm/mach-omap2/omap-wakeupgen.c new file +7 -0

+               return -EPERM;
+       }
+
+       /* Static mapping, never released */
+       wakeupgen_base = ioremap(OMAP44XX_WKUPGEN_BASE, SZ_4K);
+       if (WARN_ON(!wakeupgen_base))
+               return -ENODEV;

-ENOMEM os required as discussed in the other patches of the series.

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

The WakeupGen unit is responsible for generating wakeup event from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation, WakeupGen delivers external interrupts directly to the GIC.

Quoted 8 lines · Santosh Shilimkar

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

The WakeupGen unit is responsible for generating wakeup event from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation, WakeupGen delivers external interrupts directly to the GIC.

...

Quoted 8 lines

+ ? ? /*
+ ? ? ?* Override GIC architecture specific functions to add
+ ? ? ?* OMAP WakeupGen interrupt controller along with GIC
+ ? ? ?*/
+ ? ? gic_arch_extn.irq_mask = wakeupgen_mask;
+ ? ? gic_arch_extn.irq_unmask = wakeupgen_unmask;
+ ? ? gic_arch_extn.irq_set_wake = wakeupgen_set_wake;
+ ? ? gic_arch_extn.flags = IRQCHIP_MASK_ON_SUSPEND;

As I've commented before, there should not be any need to tweak the wakeupgen registers for each interrupt during the runtime.

AFAIK the wakeupgen registers only need to be armed every time before entering idle.

Tony,

Quoted 20 lines · Tony Lindgren

Quoted 8 lines · Santosh Shilimkar

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

The WakeupGen unit is responsible for generating wakeup event from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation, WakeupGen delivers external interrupts directly to the GIC.

...

Quoted 8 lines

+ ? ? /*
+ ? ? ?* Override GIC architecture specific functions to add
+ ? ? ?* OMAP WakeupGen interrupt controller along with GIC
+ ? ? ?*/
+ ? ? gic_arch_extn.irq_mask = wakeupgen_mask;
+ ? ? gic_arch_extn.irq_unmask = wakeupgen_unmask;
+ ? ? gic_arch_extn.irq_set_wake = wakeupgen_set_wake;
+ ? ? gic_arch_extn.flags = IRQCHIP_MASK_ON_SUSPEND;

Quoted 3 lines

As I've commented before, there should not be any need to tweak the wakeupgen registers for each interrupt during the runtime.

And I gave you all the reasons why it needs to be done this way.

Quoted 3 lines

AFAIK the wakeupgen registers only need to be armed every time before entering idle.

No that doesn't work and it completely hacky approach.

This problem is for all SOC's using A9 SMP and GIC and every soc has an architecture specific interrupt controller extension. And that was the reason the GIC arch_extn was proposed.
It's just another IRQCHIP and works seamlessly being part of the framework. And it will also initialized with primary IRQCHIP( GIC).

Quoted 25 lines · Shilimkar, Santosh

Tony,

Quoted 23 lines · Tony Lindgren

Quoted 8 lines · Santosh Shilimkar

OMAP WakeupGen is the interrupt controller extension used along with ARM GIC to wake the CPU out from low power states on external interrupts.

The WakeupGen unit is responsible for generating wakeup event from the incoming interrupts and enable bits. It is implemented in MPU always ON power domain. During normal operation, WakeupGen delivers external interrupts directly to the GIC.

...

arch/arm/mach-omap2/omap-wakeupgen.c line 229

Quoted 8 lines

arch/arm/mach-omap2/omap-wakeupgen.c new file +8 -0

+     /*
+      * Override GIC architecture specific functions to add
+      * OMAP WakeupGen interrupt controller along with GIC
+      */
+     gic_arch_extn.irq_mask = wakeupgen_mask;
+     gic_arch_extn.irq_unmask = wakeupgen_unmask;
+     gic_arch_extn.irq_set_wake = wakeupgen_set_wake;
+     gic_arch_extn.flags = IRQCHIP_MASK_ON_SUSPEND;

As I've commented before, there should not be any need to tweak the wakeupgen registers for each interrupt during the runtime.

Quoted 1 line

And I gave you all the reasons why it needs to be done this way.

Hmm, I don't think you ever answered the main question:

Why would you need to write the wakeupgen registers for every interrupt during the runtime instead of arming them only when entering idle?

First and foremost, I have to go with the approach here because MPUSS hardware team put a requirement that GIC and wakeupgen should always be kept in sync. If needed we can discuss this off-the list with Richard.

Below is the extract from the veyron func specs.
-------------------------------------
Version 1.6 of veyron spec has this.

From page 95, paragraph 2 on version 1.6:

"It is SW responsibility to program interrupt enabling/disabling coherently in the GIC and in the Wugen enable registers. That is, a given interrupt for a given CPU is either enable at both GIC and Wugen, or disable at both, but no mix."
-------------------------------------

The way understand this IP is, even in normal scenario's every IRQ will come to wakeupgen and then it will pass that to GIC. CPU clock domains are kept under HW supervised always and they can enter inactive any time without WFI. Only wakeup gen can bring the CPU out of inactive state.

That's requirement really lead to this design choice. Just to add all ARM SOC's using GIC has a gic extension interrupt controller and follow the same approach for the secondary IRQCHIPO.

Below points as such don't matter after the strict hardware requirement. Still .....

Let's say, we ignore the hardware recommendation and try to do what you are suggesting.

How will you know while entering in idle which IRQ's to be masked and which are to be unmasked ?
The only way is to run though entire 1024 possible IRQ's from GIC and then check the state of each IRQ and set things accordingly. At GIC level, mask and unmask registers are different so you will end up reading those many registers. That also means you need to export some non-standard APIs from GIC driver.

In system wide suspend, the core irq code, communicates the wakeup and non-wakeup functionality using standard mask/ unmask APIs when used with IRQCHIP_MASK_ON_SUSPEND. With what you are suggesting it won't work
as desired. Because that information is only passed to the IRQ chips. So you will still need IRQCHIP and mask/unmask APIs. That can be done as part of set_wake() handler as well though.

The wakeupgen is within CPU cluster and the accesses to it are not as expensive as like accessing 32 K timer or GP timer.

By making the wakeupgen as an IRQCHIP, we meet the hardware requirement and also make use of this IP properly for the desired functionality using standard IRQCHIP interfaces No need of non-standard hacking.

It also avoid platform code monkeing with common GIC code and irq subsystem to hack the stuff.

Btw, not exactly related here, but because of common code consolidation, we need to actually use GIC common save/restore hooks, even though OMAP has very optimal software save and hardware restore mechanism for GIC.

Hope this email summarise all previous multiple discussions in one place.

Quoted 23 lines · Santosh

First and foremost, I have to go with the approach here because MPUSS hardware team put a requirement that GIC and wakeupgen should always be kept in sync. If needed we can discuss this off-the list with Richard.

Below is the extract from the veyron func specs.
-------------------------------------
Version 1.6 of veyron spec has this.

From page 95, paragraph 2 on version 1.6:

"It is SW responsibility to program interrupt enabling/disabling coherently in the GIC and in the Wugen enable registers. That is, a given interrupt for a given CPU is either enable at both GIC and Wugen, or disable at both, but no mix."
-------------------------------------

The way understand this IP is, even in normal scenario's every IRQ will come to wakeupgen and then it will pass that to GIC. CPU clock domains are kept under HW supervised always and they can enter inactive any time without WFI. Only wakeup gen can bring the CPU out of inactive state.

Quoted 3 lines

That's requirement really lead to this design choice. Just to add all ARM SOC's using GIC has a gic extension interrupt controller and follow the same approach for the secondary IRQCHIPO.

Thanks for the clarification. It seems to me the spec is most likely wrong as we've had the GIC working for over two years now without doing anything with the wakeup gen registers :)

Quoted 6 lines · Tony Lindgren

* Santosh<[email protected]> [110914 09:16]:

Thanks for the clarification. It seems to me the spec is most likely wrong as we've had the GIC working for over two years now without doing anything with the wakeup gen registers :)

It's working because CPU clockdomain are never put under HW supervised mode and they are kept in force wakeup. Clock-domain never idles on mainline code. PM series will put the clock-domains under HW supervison as needed to achieve any low power states and then all sorts of corner cases will come out.

Quoted 7 lines · Santosh

Quoted 6 lines · Tony Lindgren

* Santosh<[email protected]> [110914 09:16]:

Thanks for the clarification. It seems to me the spec is most likely wrong as we've had the GIC working for over two years now without doing anything with the wakeup gen registers :)

Quoted 5 lines

It's working because CPU clockdomain are never put under HW supervised mode and they are kept in force wakeup. Clock-domain never idles on mainline code. PM series will put the clock-domains under HW supervison as needed to achieve any low power states and then all sorts of corner cases will come out.

But again the wakeup gen triggers only do something when hitting idle. There should be no use for them during runtime, right?

Quoted 7 lines · Santosh

Quoted 6 lines · Tony Lindgren

* Santosh<[email protected]> [110914 09:16]:

Thanks for the clarification. It seems to me the spec is most likely wrong as we've had the GIC working for over two years now without doing anything with the wakeup gen registers :)

Quoted 5 lines

It's working because CPU clockdomain are never put under HW supervised mode and they are kept in force wakeup. Clock-domain never idles on mainline code. PM series will put the clock-domains under HW supervison as needed to achieve any low power states and then all sorts of corner cases will come out.

But again the wakeup gen triggers only do something when hitting idle. There should be no use for them during runtime, right?

Quoted 14 lines · Tony Lindgren

Quoted 12 lines · Santosh

Quoted 6 lines · Tony Lindgren

* Santosh<[email protected]> [110914 09:16]:

Thanks for the clarification. It seems to me the spec is most likely wrong as we've had the GIC working for over two years now without doing anything with the wakeup gen registers :)

It's working because CPU clockdomain are never put under HW supervised mode and they are kept in force wakeup. Clock-domain never idles on mainline code. PM series will put the clock-domains under HW supervison as needed to achieve any low power states and then all sorts of corner cases will come out.

Quoted 3 lines

But again the wakeup gen triggers only do something when hitting idle. There should be no use for them during runtime, right?

You missed my point in the description. Clockdomain inactive doesn't depend on idle or WFI execution. Under HW supervison CPU clock domain can get into inactive when CPU is stalled and waiting for a read response from slow interconnect.

One thing for sure. Designers has chosen a wrong name to this IP. Wakeugen apears like needed only for low power wakeup which not seems to be entirely correct as per specs

Quoted 15 lines · Santosh

Quoted 14 lines · Tony Lindgren

Quoted 12 lines · Santosh

Quoted 6 lines · Tony Lindgren

* Santosh<[email protected]> [110914 09:16]:

Thanks for the clarification. It seems to me the spec is most likely wrong as we've had the GIC working for over two years now without doing anything with the wakeup gen registers :)

It's working because CPU clockdomain are never put under HW supervised mode and they are kept in force wakeup. Clock-domain never idles on mainline code. PM series will put the clock-domains under HW supervison as needed to achieve any low power states and then all sorts of corner cases will come out.

Quoted 7 lines

Quoted 3 lines

But again the wakeup gen triggers only do something when hitting idle. There should be no use for them during runtime, right?

You missed my point in the description. Clockdomain inactive doesn't depend on idle or WFI execution. Under HW supervison CPU clock domain can get into inactive when CPU is stalled and waiting for a read response from slow interconnect.

Ah OK. If it's needed during runtime too then that explains why the registers need to be kept in sync.

Quoted 3 lines

One thing for sure. Designers has chosen a wrong name to this IP. Wakeugen apears like needed only for low power wakeup which not seems to be entirely correct as per specs

Yes it's not obvious reading the TRM either. Maybe add some comment about that to the patch?

Quoted 14 lines · Tony Lindgren

Quoted 12 lines · Santosh

Quoted 6 lines · Tony Lindgren

* Santosh<[email protected]> [110914 09:16]:

Thanks for the clarification. It seems to me the spec is most likely wrong as we've had the GIC working for over two years now without doing anything with the wakeup gen registers :)

It's working because CPU clockdomain are never put under HW supervised mode and they are kept in force wakeup. Clock-domain never idles on mainline code. PM series will put the clock-domains under HW supervison as needed to achieve any low power states and then all sorts of corner cases will come out.

Quoted 3 lines

But again the wakeup gen triggers only do something when hitting idle. There should be no use for them during runtime, right?

You missed my point in the description. Clockdomain inactive doesn't depend on idle or WFI execution. Under HW supervison CPU clock domain can get into inactive when CPU is stalled and waiting for a read response from slow interconnect.

Hi Santosh,

Quoted 19 lines · Santosh

Quoted 17 lines · Tony Lindgren

Quoted 14 lines · Santosh

Quoted 7 lines · Tony Lindgren

* Santosh<[email protected]> ? ?[110914 09:16]:

Thanks for the clarification. It seems to me the spec is most likely wrong as we've had the GIC working for over two years now without doing anything with the wakeup gen registers :)

It's working because CPU clockdomain are never put under HW supervised mode and they are kept in force wakeup. Clock-domain never idles on mainline code. PM series will put the clock-domains under HW supervison as needed to achieve any low power states and then all sorts of corner cases will come out.

Quoted 7 lines

Quoted 3 lines

But again the wakeup gen triggers only do something when hitting idle. There should be no use for them during runtime, right?

You missed my point in the description. Clockdomain inactive doesn't depend on idle or WFI execution. Under HW supervison CPU clock domain can get into inactive when CPU is stalled and waiting for a read response from slow interconnect.

I'm a little bit confused by that statement...

I'm wondering how the PRCM can gate the CPU/MPU clock and re-enable it if the MPU is gated? What kind of event can wakeup the CPU in case of CPU stalled?

The spec seems to indicate that wakeupgen is needed only if CPU are in WFI. It is even written: "CPUx will change power state only when StandbyWFI is asserted.". Even a WFE is not supposed to trigger a standby. How can the CPU be inactive at clock domain level without a WFI?

From: Shilimkar, Santosh
Sent: Thursday, September 15, 2011 7:02 AM

Quoted 4 lines

Quoted 4 lines

You missed my point in the description. Clockdomain inactive doesn't depend on idle or WFI execution. Under HW supervison CPU clock domain can get into inactive when CPU is stalled and waiting for a read response from slow interconnect.

This patch adds the CPU0 and CPU1 off mode support. CPUX close switch retention (CSWR) is not supported by hardware design.

The CPUx OFF mode isn't supported on OMAP4430 ES1.0

CPUx sleep code is common for hotplug, suspend and CPUilde.

Signed-off-by Santosh Shilimkar <[email protected]>

Cc Kevin Hilman <[email protected]>

---

?arch/arm/mach-omap2/Makefile ? ? ? ? ? ? ? ? ? ?| ? ?3 +-
?arch/arm/mach-omap2/include/mach/omap-secure.h ?| ? ?8 +
?arch/arm/mach-omap2/include/mach/omap4-common.h | ? 25 +++
?arch/arm/mach-omap2/omap-mpuss-lowpower.c ? ? ? | ?249 +++++++++++++++++++++++
?arch/arm/mach-omap2/omap-smp.c ? ? ? ? ? ? ? ? ?| ? ?6 +
?arch/arm/mach-omap2/omap4-sar-layout.h ? ? ? ? ?| ? ?9 +
?arch/arm/mach-omap2/pm44xx.c ? ? ? ? ? ? ? ? ? ?| ? ?6 +
?arch/arm/mach-omap2/sleep44xx.S ? ? ? ? ? ? ? ? | ?213 +++++++++++++++++++

?8 files changed, 518 insertions(+), 1 deletions(-) ?create mode 100644 arch/arm/mach-omap2/omap-mpuss-lowpower.c

arch/arm/mach-omap2/Makefile +0 -0

index b032c21..b4f2eeb 100644
@@ -63,7 +63,8 @@ obj-$(CONFIG_ARCH_OMAP2) ? ? ? ? ? ? ?+= pm24xx.o

?obj-$(CONFIG_ARCH_OMAP2) ? ? ? ? ? ? ? += sleep24xx.o ?obj-$(CONFIG_ARCH_OMAP3) ? ? ? ? ? ? ? += pm34xx.o sleep34xx.o \
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cpuidle34xx.o

arch/arm/mach-omap2/Makefile +2 -1

-obj-$(CONFIG_ARCH_OMAP4) ? ? ? ? ? ? ? += pm44xx.o sleep44xx.o
+obj-$(CONFIG_ARCH_OMAP4) ? ? ? ? ? ? ? += pm44xx.o sleep44xx.o \
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?omap-mpuss-lowpower.o

?obj-$(CONFIG_PM_DEBUG) ? ? ? ? ? ? ? ? += pm-debug.o ?obj-$(CONFIG_OMAP_SMARTREFLEX) ? ? ? ? ?+= sr_device.o smartreflex.o ?obj-$(CONFIG_OMAP_SMARTREFLEX_CLASS3) ?+= smartreflex-class3.o

arch/arm/mach-omap2/include/mach/omap-secure.h +0 -0

index e2f95a0..0062d49 100644
@@ -35,10 +35,18 @@

?#define OMAP4_HAL_SAVEALL_INDEX ? ? ? ? ? ? ? ?0x1c ?#define OMAP4_HAL_SAVEGIC_INDEX ? ? ? ? ? ? ? ?0x1d

arch/arm/mach-omap2/include/mach/omap-secure.h +7 -0

+/* Secure Monitor mode APIs */
+#define OMAP4_MON_SCU_PWR_INDEX ? ? ? ? ? ? ? ?0x108
+
+/* Secure PPA(Primary Protected Application) APIs */
+#define OMAP4_PPA_CPU_ACTRL_SMP_INDEX ?0x25
+
+#ifndef __ASSEMBLER__

?extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?u32 arg1, u32 arg2, u32 arg3, u32 arg4); ?extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
?extern phys_addr_t omap_secure_ram_mempool_base(void); ?extern int omap_secure_ram_reserve_memblock(void);

+#endif /* __ASSEMBLER__ */
?#endif /* OMAP_ARCH_OMAP_SECURE_H */

arch/arm/mach-omap2/include/mach/omap4-common.h +0 -0

index 040dcf6..662f37c 100644
@@ -45,5 +45,30 @@ extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
 ?extern void omap_auxcoreboot_addr(u32 cpu_addr);
 ?extern u32 omap_read_auxcoreboot0(void);

?#endif

arch/arm/mach-omap2/include/mach/omap4-common.h +11 -0

+
+#if defined(CONFIG_SMP) && defined(CONFIG_PM)
+extern int omap4_mpuss_init(void);
+extern int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state);
+extern int omap4_finish_suspend(unsigned long cpu_state);
+extern void omap4_cpu_resume(void);
+#else
+static inline int omap4_enter_lowpower(unsigned int cpu,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int power_state)
+{
+ ? ? ? omap_do_wfi();

arch/arm/mach-omap2/include/mach/omap4-common.h +4 -0

+static inline void omap4_cpu_resume(void)
+{}
+#endif
+

?#endif /* __ASSEMBLER__ */
?#endif /* OMAP_ARCH_OMAP4_COMMON_H */

arch/arm/mach-omap2/omap-mpuss-lowpower.c +0 -0

arch/arm/mach-omap2/omap-mpuss-lowpower.c +35 -0

+/**
+ * omap4_enter_lowpower: OMAP4 MPUSS Low Power Entry Function
+ * The purpose of this function is to manage low power programming
+ * of OMAP4 MPUSS subsystem
+ * @cpu : CPU ID
+ * @power_state: Low power state.
+ */
+int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
+{
+ ? ? ? unsigned int save_state = 0;
+ ? ? ? unsigned int wakeup_cpu;
+
+ ? ? ? if (omap_rev() == OMAP4430_REV_ES1_0)
+ ? ? ? ? ? ? ? return -ENXIO;
+
+ ? ? ? switch (power_state) {
+ ? ? ? case PWRDM_POWER_ON:
+ ? ? ? case PWRDM_POWER_INACTIVE:
+ ? ? ? ? ? ? ? save_state = 0;
+ ? ? ? ? ? ? ? break;
+ ? ? ? case PWRDM_POWER_OFF:
+ ? ? ? ? ? ? ? save_state = 1;
+ ? ? ? ? ? ? ? break;
+ ? ? ? case PWRDM_POWER_RET:
+ ? ? ? default:
+ ? ? ? ? ? ? ? /*
+ ? ? ? ? ? ? ? ?* CPUx CSWR is invalid hardware state. Also CPUx OSWR
+ ? ? ? ? ? ? ? ?* doesn't make much scense, since logic is lost and $L1
+ ? ? ? ? ? ? ? ?* needs to be cleaned because of coherency. This makes
+ ? ? ? ? ? ? ? ?* CPUx OSWR equivalent to CPUX OFF and hence not supported
+ ? ? ? ? ? ? ? ?*/
+ ? ? ? ? ? ? ? WARN_ON(1);
+ ? ? ? ? ? ? ? return -ENXIO;
+ ? ? ? }
+

arch/arm/mach-omap2/sleep44xx.S +0 -0

+/*
+ * =============================
+ * == CPU suspend finisher ==
+ * =============================
+ *
+ * void omap4_finish_suspend(unsigned long cpu_state)
+ *
+ * This function code saves the CPU context and performs the CPU
+ * power down sequence. Calling WFI effectively changes the CPU
+ * power domains states to the desired target power state.
+ *

+ * @cpu_state : contains context save state (r0)
+ * ? ? 0 - No context lost
+ * ? ? 1 - CPUx L1 and logic lost: MPUSS CSWR
+ * ? ? 2 - CPUx L1 and logic lost + GIC lost: MPUSS OSWR
+ * ? ? 3 - CPUx L1 and logic lost + GIC + L2 lost: MPUSS OFF
+ * @return: This function never returns for CPU OFF and DORMANT power states.
+ * Post WFI, CPU transitions to DORMANT or OFF power state and on wake-up
+ * from this follows a full CPU reset path via ROM code to CPU restore code.

+/*
+ * ============================
+ * == CPU resume entry point ==
+ * ============================
+ *
+ * void omap4_cpu_resume(void)
+ *
+ * ROM code jumps to this function while waking up from CPU
+ * OFF or DORMANT state. Physical address of the function is
+ * stored in the SAR RAM while entering to OFF or DORMANT mode.

+ */
+ENTRY(omap4_cpu_resume)
+ ? ? ? /*

Quoted 77 lines · Santosh Shilimkar

This patch adds the CPU0 and CPU1 off mode support. CPUX close switch retention (CSWR) is not supported by hardware design.

The CPUx OFF mode isn't supported on OMAP4430 ES1.0

CPUx sleep code is common for hotplug, suspend and CPUilde.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

---
  arch/arm/mach-omap2/Makefile                    |    3 +-
  arch/arm/mach-omap2/include/mach/omap-secure.h  |    8 +
  arch/arm/mach-omap2/include/mach/omap4-common.h |   25 +++
  arch/arm/mach-omap2/omap-mpuss-lowpower.c       |  249 +++++++++++++++++++++++
  arch/arm/mach-omap2/omap-smp.c                  |    6 +
  arch/arm/mach-omap2/omap4-sar-layout.h          |    9 +
  arch/arm/mach-omap2/pm44xx.c                    |    6 +
  arch/arm/mach-omap2/sleep44xx.S                 |  213 +++++++++++++++++++
  8 files changed, 518 insertions(+), 1 deletions(-)

create mode 100644 arch/arm/mach-omap2/omap-mpuss-lowpower.c

arch/arm/mach-omap2/Makefile +2 -1

index b032c21..b4f2eeb 100644
@@ -63,7 +63,8 @@ obj-$(CONFIG_ARCH_OMAP2)              += pm24xx.o
  obj-$(CONFIG_ARCH_OMAP2)               += sleep24xx.o
  obj-$(CONFIG_ARCH_OMAP3)               += pm34xx.o sleep34xx.o \
                                           cpuidle34xx.o
-obj-$(CONFIG_ARCH_OMAP4)               += pm44xx.o sleep44xx.o
+obj-$(CONFIG_ARCH_OMAP4)               += pm44xx.o sleep44xx.o \
+                                          omap-mpuss-lowpower.o
  obj-$(CONFIG_PM_DEBUG)                 += pm-debug.o
  obj-$(CONFIG_OMAP_SMARTREFLEX)          += sr_device.o smartreflex.o
  obj-$(CONFIG_OMAP_SMARTREFLEX_CLASS3)  += smartreflex-class3.o

arch/arm/mach-omap2/include/mach/omap-secure.h +8 -0

index e2f95a0..0062d49 100644
@@ -35,10 +35,18 @@
  #define OMAP4_HAL_SAVEALL_INDEX                0x1c
  #define OMAP4_HAL_SAVEGIC_INDEX                0x1d
 
+/* Secure Monitor mode APIs */
+#define OMAP4_MON_SCU_PWR_INDEX                0x108
+
+/* Secure PPA(Primary Protected Application) APIs */
+#define OMAP4_PPA_CPU_ACTRL_SMP_INDEX  0x25
+
+#ifndef __ASSEMBLER__
  extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
                                u32 arg1, u32 arg2, u32 arg3, u32 arg4);
  extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
  extern phys_addr_t omap_secure_ram_mempool_base(void);
  extern int omap_secure_ram_reserve_memblock(void);
 
+#endif /* __ASSEMBLER__ */
  #endif /* OMAP_ARCH_OMAP_SECURE_H */

arch/arm/mach-omap2/include/mach/omap4-common.h +11 -0

index 040dcf6..662f37c 100644
@@ -45,5 +45,30 @@ extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
  extern void omap_auxcoreboot_addr(u32 cpu_addr);
  extern u32 omap_read_auxcoreboot0(void);
  #endif
+
+#if defined(CONFIG_SMP)&&  defined(CONFIG_PM)
+extern int omap4_mpuss_init(void);
+extern int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state);
+extern int omap4_finish_suspend(unsigned long cpu_state);
+extern void omap4_cpu_resume(void);
+#else
+static inline int omap4_enter_lowpower(unsigned int cpu,
+                                       unsigned int power_state)
+{
+       omap_do_wfi();

arch/arm/mach-omap2/include/mach/omap4-common.h extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask); · line 68

Quoted 10 lines

arch/arm/mach-omap2/include/mach/omap4-common.h +10 -0

+       return 0;
+}
+
+static inline int omap4_mpuss_init(void)
+{
+       return 0;
+}
+
+static inline int omap4_finish_suspend(unsigned long cpu_state)
+{}

The int function should return something (0?) Has the code been test compiled without SMP and/or PM?

Quoted 7 lines

arch/arm/mach-omap2/include/mach/omap4-common.h +4 -0

+static inline void omap4_cpu_resume(void)
+{}
+#endif
+
  #endif /* __ASSEMBLER__ */
  #endif /* OMAP_ARCH_OMAP4_COMMON_H */

arch/arm/mach-omap2/omap-mpuss-lowpower.c +0 -0

...

Quoted 35 lines

arch/arm/mach-omap2/omap-mpuss-lowpower.c +35 -0

+/**
+ * omap4_enter_lowpower: OMAP4 MPUSS Low Power Entry Function
+ * The purpose of this function is to manage low power programming
+ * of OMAP4 MPUSS subsystem
+ * @cpu : CPU ID
+ * @power_state: Low power state.
+ */
+int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
+{
+       unsigned int save_state = 0;
+       unsigned int wakeup_cpu;
+
+       if (omap_rev() == OMAP4430_REV_ES1_0)
+               return -ENXIO;
+
+       switch (power_state) {
+       case PWRDM_POWER_ON:
+       case PWRDM_POWER_INACTIVE:
+               save_state = 0;
+               break;
+       case PWRDM_POWER_OFF:
+               save_state = 1;
+               break;
+       case PWRDM_POWER_RET:
+       default:
+               /*
+                * CPUx CSWR is invalid hardware state. Also CPUx OSWR
+                * doesn't make much scense, since logic is lost and $L1
+                * needs to be cleaned because of coherency. This makes
+                * CPUx OSWR equivalent to CPUX OFF and hence not supported
+                */
+               WARN_ON(1);
+               return -ENXIO;
+       }
+

arch/arm/mach-omap2/omap-mpuss-lowpower.c line 181

Quoted 10 lines

arch/arm/mach-omap2/omap-mpuss-lowpower.c +10 -0

+       clear_cpu_prev_pwrst(cpu);
+       set_cpu_next_pwrst(cpu, power_state);
+       set_cpu_wakeup_addr(cpu, virt_to_phys(omap4_cpu_resume));
+       scu_pwrst_prepare(cpu, power_state);
+
+       /*
+        * Call low level function  with targeted CPU id
+        * and its low power state.
+        */
+       cpu_suspend(save_state, omap4_finish_suspend);

There is no CPU id parameter to the call although the above comment says so.

...

Quoted 1 line

arch/arm/mach-omap2/sleep44xx.S +0 -0

...

Quoted 12 lines

+/*
+ * =============================
+ * == CPU suspend finisher ==
+ * =============================
+ *
+ * void omap4_finish_suspend(unsigned long cpu_state)
+ *
+ * This function code saves the CPU context and performs the CPU
+ * power down sequence. Calling WFI effectively changes the CPU
+ * power domains states to the desired target power state.
+ *
+ * @cpu_state : contains context save state (r0)

Quoted 7 lines

+ *     0 - No context lost
+ *     1 - CPUx L1 and logic lost: MPUSS CSWR
+ *     2 - CPUx L1 and logic lost + GIC lost: MPUSS OSWR
+ *     3 - CPUx L1 and logic lost + GIC + L2 lost: MPUSS OFF

+ * @return: This function never returns for CPU OFF and DORMANT power states.
+ * Post WFI, CPU transitions to DORMANT or OFF power state and on wake-up
+ * from this follows a full CPU reset path via ROM code to CPU restore code.

Is the ROM code jumping to the physical address of omap4_cpu_resume? If so please add it in the comment.

...

Quoted 10 lines

+/*
+ * ============================
+ * == CPU resume entry point ==
+ * ============================
+ *
+ * void omap4_cpu_resume(void)
+ *
+ * ROM code jumps to this function while waking up from CPU
+ * OFF or DORMANT state. Physical address of the function is
+ * stored in the SAR RAM while entering to OFF or DORMANT mode.

Which code stores the address in the SAR RAM? Please describe it here.

This patch adds the CPU0 and CPU1 off mode support. CPUX close switch retention (CSWR) is not supported by hardware design.

The CPUx OFF mode isn't supported on OMAP4430 ES1.0

CPUx sleep code is common for hotplug, suspend and CPUilde.

Signed-off-by Santosh Shilimkar <[email protected]>

Cc Kevin Hilman <[email protected]>

---
 arch/arm/mach-omap2/Makefile                    |    3 +-
 arch/arm/mach-omap2/include/mach/omap-secure.h  |    8 +
 arch/arm/mach-omap2/include/mach/omap4-common.h |   25 +++
 arch/arm/mach-omap2/omap-mpuss-lowpower.c       |  249 +++++++++++++++++++++++
 arch/arm/mach-omap2/omap-smp.c                  |    6 +
 arch/arm/mach-omap2/omap4-sar-layout.h          |    9 +
 arch/arm/mach-omap2/pm44xx.c                    |    6 +
 arch/arm/mach-omap2/sleep44xx.S                 |  213 +++++++++++++++++++
 8 files changed, 518 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap-mpuss-lowpower.c

arch/arm/mach-omap2/sleep44xx.S +25 -0

index 049f426..230ab8c 100644
@@ -11,8 +11,221 @@
 
 #include <linux/linkage.h>
 #include <asm/system.h>
+#include <asm/smp_scu.h>
+#include <asm/memory.h>
+#include <asm/hardware/cache-l2x0.h>
 
+#include <plat/omap44xx.h>
 #include <mach/omap4-common.h>
+#include <mach/omap-secure.h>
+
+#include "omap4-sar-layout.h"
+
+#ifdef CONFIG_SMP
+
+.macro	DO_SMC
+	dsb
+	smc	#0
+	dsb
+.endm
+
+ppa_zero_params:
+	.word		0x0
+
+/*
+ * =============================
+ * == CPU suspend finisher ==
+ * =============================
+ *
+ * void omap4_finish_suspend(unsigned long cpu_state)

Hi Santosh,

Quoted 21 lines · Santosh Shilimkar

This patch adds the CPU0 and CPU1 off mode support. CPUX close switch retention (CSWR) is not supported by hardware design.

The CPUx OFF mode isn't supported on OMAP4430 ES1.0

CPUx sleep code is common for hotplug, suspend and CPUilde.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

---
  arch/arm/mach-omap2/Makefile                    |    3 +-
  arch/arm/mach-omap2/include/mach/omap-secure.h  |    8 +
  arch/arm/mach-omap2/include/mach/omap4-common.h |   25 +++
  arch/arm/mach-omap2/omap-mpuss-lowpower.c       |  249 +++++++++++++++++++++++
  arch/arm/mach-omap2/omap-smp.c                  |    6 +
  arch/arm/mach-omap2/omap4-sar-layout.h          |    9 +
  arch/arm/mach-omap2/pm44xx.c                    |    6 +
  arch/arm/mach-omap2/sleep44xx.S                 |  213 +++++++++++++++++++
  8 files changed, 518 insertions(+), 1 deletions(-)

create mode 100644 arch/arm/mach-omap2/omap-mpuss-lowpower.c

[...]

Quoted 41 lines

arch/arm/mach-omap2/sleep44xx.S +31 -0

index 049f426..230ab8c 100644
@@ -11,8 +11,221 @@
 
  #include<linux/linkage.h>
  #include<asm/system.h>
+#include<asm/smp_scu.h>
+#include<asm/memory.h>
+#include<asm/hardware/cache-l2x0.h>
 
+#include<plat/omap44xx.h>
  #include<mach/omap4-common.h>
+#include<mach/omap-secure.h>
+
+#include "omap4-sar-layout.h"
+
+#ifdef CONFIG_SMP
+
+.macro	DO_SMC
+	dsb
+	smc	#0
+	dsb
+.endm
+
+ppa_zero_params:
+	.word		0x0
+
+/*
+ * =============================
+ * == CPU suspend finisher ==
+ * =============================
+ *
+ * void omap4_finish_suspend(unsigned long cpu_state)
+ *
+ * This function code saves the CPU context and performs the CPU
+ * power down sequence. Calling WFI effectively changes the CPU
+ * power domains states to the desired target power state.
+ *
+ * @cpu_state : contains context save state (r0)

arch/arm/mach-omap2/sleep44xx.S line 50

Quoted 4 lines

arch/arm/mach-omap2/sleep44xx.S +4 -0

+ *	0 - No context lost
+ * 	1 - CPUx L1 and logic lost: MPUSS CSWR
+ * 	2 - CPUx L1 and logic lost + GIC lost: MPUSS OSWR
+ *	3 - CPUx L1 and logic lost + GIC + L2 lost: MPUSS OFF

I was told by rmk that same as imx6q, omap44xx will retain L2 content across suspen/resume cycle. Then what does "L2 lost" mean here? Or what rmk meant is the case cpu_state == 2?

Quoted 68 lines · Shawn Guo

Hi Santosh,

Quoted 21 lines · Santosh Shilimkar

This patch adds the CPU0 and CPU1 off mode support. CPUX close switch retention (CSWR) is not supported by hardware design.

The CPUx OFF mode isn't supported on OMAP4430 ES1.0

CPUx sleep code is common for hotplug, suspend and CPUilde.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

---
 arch/arm/mach-omap2/Makefile                    |    3 +-
 arch/arm/mach-omap2/include/mach/omap-secure.h  |    8 +
 arch/arm/mach-omap2/include/mach/omap4-common.h |   25 +++
 arch/arm/mach-omap2/omap-mpuss-lowpower.c       |  249 +++++++++++++++++++++++
 arch/arm/mach-omap2/omap-smp.c                  |    6 +
 arch/arm/mach-omap2/omap4-sar-layout.h          |    9 +
 arch/arm/mach-omap2/pm44xx.c                    |    6 +
 arch/arm/mach-omap2/sleep44xx.S                 |  213 +++++++++++++++++++
 8 files changed, 518 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap-mpuss-lowpower.c

[...]

Quoted 41 lines

arch/arm/mach-omap2/sleep44xx.S +31 -0

index 049f426..230ab8c 100644
@@ -11,8 +11,221 @@
 
 #include<linux/linkage.h>
 #include<asm/system.h>
+#include<asm/smp_scu.h>
+#include<asm/memory.h>
+#include<asm/hardware/cache-l2x0.h>
 
+#include<plat/omap44xx.h>
 #include<mach/omap4-common.h>
+#include<mach/omap-secure.h>
+
+#include "omap4-sar-layout.h"
+
+#ifdef CONFIG_SMP
+
+.macro	DO_SMC
+	dsb
+	smc	#0
+	dsb
+.endm
+
+ppa_zero_params:
+	.word		0x0
+
+/*
+ * =============================
+ * == CPU suspend finisher ==
+ * =============================
+ *
+ * void omap4_finish_suspend(unsigned long cpu_state)
+ *
+ * This function code saves the CPU context and performs the CPU
+ * power down sequence. Calling WFI effectively changes the CPU
+ * power domains states to the desired target power state.
+ *
+ * @cpu_state : contains context save state (r0)

Quoted 9 lines

arch/arm/mach-omap2/sleep44xx.S line 50

Quoted 4 lines

arch/arm/mach-omap2/sleep44xx.S +4 -0

+ *	0 - No context lost
+ * 	1 - CPUx L1 and logic lost: MPUSS CSWR
+ * 	2 - CPUx L1 and logic lost + GIC lost: MPUSS OSWR
+ *	3 - CPUx L1 and logic lost + GIC + L2 lost: MPUSS OFF

I was told by rmk that same as imx6q, omap44xx will retain L2 content across suspen/resume cycle. Then what does "L2 lost" mean here? Or what rmk meant is the case cpu_state == 2?

Yes.

The last case is entire SOC OFF. We call Device OFF in OMAP. All voltages will scale to 0 V. This isn't supported by this series.

Quoted 78 lines · Santosh

Quoted 68 lines · Shawn Guo

Hi Santosh,

Quoted 21 lines · Santosh Shilimkar

This patch adds the CPU0 and CPU1 off mode support. CPUX close switch retention (CSWR) is not supported by hardware design.

The CPUx OFF mode isn't supported on OMAP4430 ES1.0

CPUx sleep code is common for hotplug, suspend and CPUilde.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

---

?arch/arm/mach-omap2/Makefile ? ? ? ? ? ? ? ? ? ?| ? ?3 +-
?arch/arm/mach-omap2/include/mach/omap-secure.h ?| ? ?8 +
?arch/arm/mach-omap2/include/mach/omap4-common.h | ? 25 +++
?arch/arm/mach-omap2/omap-mpuss-lowpower.c ? ? ? | ?249 +++++++++++++++++++++++
?arch/arm/mach-omap2/omap-smp.c ? ? ? ? ? ? ? ? ?| ? ?6 +
?arch/arm/mach-omap2/omap4-sar-layout.h ? ? ? ? ?| ? ?9 +
?arch/arm/mach-omap2/pm44xx.c ? ? ? ? ? ? ? ? ? ?| ? ?6 +
?arch/arm/mach-omap2/sleep44xx.S ? ? ? ? ? ? ? ? | ?213 +++++++++++++++++++

?8 files changed, 518 insertions(+), 1 deletions(-) ?create mode 100644 arch/arm/mach-omap2/omap-mpuss-lowpower.c

[...]

Quoted 41 lines

arch/arm/mach-omap2/sleep44xx.S +0 -0

index 049f426..230ab8c 100644
@@ -11,8 +11,221 @@
 

?#include<linux/linkage.h>
?#include<asm/system.h>

arch/arm/mach-omap2/sleep44xx.S +4 -0

+#include<asm/smp_scu.h>
+#include<asm/memory.h>
+#include<asm/hardware/cache-l2x0.h>
 
+#include<plat/omap44xx.h>

?#include<mach/omap4-common.h>

arch/arm/mach-omap2/sleep44xx.S +27 -0

+#include<mach/omap-secure.h>
+
+#include "omap4-sar-layout.h"
+
+#ifdef CONFIG_SMP
+
+.macro ? ? DO_SMC
+ ? dsb
+ ? smc ? ? #0
+ ? dsb
+.endm
+
+ppa_zero_params:
+ ? .word ? ? ? ? ? 0x0
+
+/*
+ * =============================
+ * == CPU suspend finisher ==
+ * =============================
+ *
+ * void omap4_finish_suspend(unsigned long cpu_state)
+ *
+ * This function code saves the CPU context and performs the CPU
+ * power down sequence. Calling WFI effectively changes the CPU
+ * power domains states to the desired target power state.
+ *
+ * @cpu_state : contains context save state (r0)

Quoted 9 lines

Quoted 4 lines

+ * 0 - No context lost
+ * ? ? ? ? 1 - CPUx L1 and logic lost: MPUSS CSWR
+ * ? ? ? ? 2 - CPUx L1 and logic lost + GIC lost: MPUSS OSWR
+ * 3 - CPUx L1 and logic lost + GIC + L2 lost: MPUSS OFF

I was told by rmk that same as imx6q, omap44xx will retain L2 content across suspen/resume cycle. ?Then what does "L2 lost" mean here? ?Or what rmk meant is the case cpu_state == 2?

Quoted 6 lines

Yes.

The last case is entire SOC OFF. We call Device OFF in OMAP. All voltages will scale to 0 V. This isn't supported by this series.

Then the second question would be what the following patch in this series is for.

? ?[PATCH 20/25] OMAP4: PM: Add L2X0 cache lowpower support

I could have read the patch incorrectly, but it seems l2x_clean_inv will also be called for "MPUSS OSWR" in which case L2 is retained? Shouldn't L2 for this case have been handled by rmk's patch (ARM: pm:
add L2 cache cleaning for suspend)?

Quoted 90 lines · Shawn Guo

Quoted 78 lines · Santosh

Quoted 68 lines · Shawn Guo

Hi Santosh,

Quoted 21 lines · Santosh Shilimkar

This patch adds the CPU0 and CPU1 off mode support. CPUX close switch retention (CSWR) is not supported by hardware design.

The CPUx OFF mode isn't supported on OMAP4430 ES1.0

CPUx sleep code is common for hotplug, suspend and CPUilde.

Signed-off-by Santosh Shilimkar<[email protected]>

Cc Kevin Hilman<[email protected]>

---
 arch/arm/mach-omap2/Makefile                    |    3 +- arch/arm/mach-omap2/include/mach/omap-secure.h  |    8 + arch/arm/mach-omap2/include/mach/omap4-common.h |   25 +++ arch/arm/mach-omap2/omap-mpuss-lowpower.c       |  249 +++++++++++++++++++++++ arch/arm/mach-omap2/omap-smp.c                  |    6 + arch/arm/mach-omap2/omap4-sar-layout.h          |    9 + arch/arm/mach-omap2/pm44xx.c                    |    6 + arch/arm/mach-omap2/sleep44xx.S                 |  213 +++++++++++++++++++

 8 files changed, 518 insertions(+), 1 deletions(-)

 create mode 100644 arch/arm/mach-omap2/omap-mpuss-lowpower.c

[...]

Quoted 41 lines

arch/arm/mach-omap2/sleep44xx.S +31 -0

index 049f426..230ab8c 100644
@@ -11,8 +11,221 @@
 
  #include<linux/linkage.h>
  #include<asm/system.h>
+#include<asm/smp_scu.h>
+#include<asm/memory.h>
+#include<asm/hardware/cache-l2x0.h>
 
+#include<plat/omap44xx.h>
  #include<mach/omap4-common.h>
+#include<mach/omap-secure.h>
+
+#include "omap4-sar-layout.h"
+
+#ifdef CONFIG_SMP
+
+.macro     DO_SMC
+   dsb
+   smc     #0
+   dsb
+.endm
+
+ppa_zero_params:
+   .word           0x0
+
+/*
+ * =============================
+ * == CPU suspend finisher ==
+ * =============================
+ *
+ * void omap4_finish_suspend(unsigned long cpu_state)
+ *
+ * This function code saves the CPU context and performs the CPU
+ * power down sequence. Calling WFI effectively changes the CPU
+ * power domains states to the desired target power state.
+ *
+ * @cpu_state : contains context save state (r0)

Quoted 9 lines

arch/arm/mach-omap2/sleep44xx.S line 50

Quoted 4 lines

arch/arm/mach-omap2/sleep44xx.S +4 -0

+ * 0 - No context lost
+ *         1 - CPUx L1 and logic lost: MPUSS CSWR
+ *         2 - CPUx L1 and logic lost + GIC lost: MPUSS OSWR
+ * 3 - CPUx L1 and logic lost + GIC + L2 lost: MPUSS OFF

I was told by rmk that same as imx6q, omap44xx will retain L2 content across suspen/resume cycle.  Then what does "L2 lost" mean here?  Or what rmk meant is the case cpu_state == 2?

Quoted 6 lines

Yes.

The last case is entire SOC OFF. We call Device OFF in OMAP. All voltages will scale to 0 V. This isn't supported by this series.

Then the second question would be what the following patch in this series is for.

   [PATCH 20/25] OMAP4: PM: Add L2X0 cache lowpower support

Quoted 5 lines

I could have read the patch incorrectly, but it seems l2x_clean_inv will also be called for "MPUSS OSWR" in which case L2 is retained? Shouldn't L2 for this case have been handled by rmk's patch (ARM: pm:
add L2 cache cleaning for suspend)?

Yes you did read it wrongly :)
It won't be called for OSWR

/*
* =============================
* == CPU suspend finisher ==
* =============================

*
* void omap4_finish_suspend(unsigned long cpu_state) *
* This function code saves the CPU context and performs the CPU
* power down sequence. Calling WFI effectively changes the CPU
* power domains states to the desired target power state. *
* @cpu_state : contains context save state (r0)

* 0 - No context lost
* 1 - CPUx L1 and logic lost: MPUSS CSWR
* 2 - CPUx L1 and logic lost + GIC lost: MPUSS OSWR
* 3 - CPUx L1 and logic lost + GIC + L2 lost: MPUSS OFF
* @return: This function never returns for CPU OFF and DORMANT power states.
* Post WFI, CPU transitions to DORMANT or OFF power state and on wake-up
* from this follows a full CPU reset path via ROM code to CPU restore code.
* It returns to the caller for CPU INACTIVE and ON power states or in case
* CPU failed to transition to targeted OFF/DORMANT state.
*/
ENTRY(omap4_finish_suspend)

	stmfd	sp!, {lr}
	cmp	r0, #0x0
	beq	do_WFI				@ No lowpower state, jump to WFI

	/*
	 * Flush all data from the L1 data cache before disabling
	 * SCTLR.C bit.
	 */
	bl	omap4_get_sar_ram_base

	ldr	r9, [r0, #OMAP_TYPE_OFFSET]
	cmp	r9, #0x1			@ Check for HS device
	bne	skip_secure_l1_clean
	mov	r0, #SCU_PM_NORMAL
	mov	r1, #0xFF			@ clean seucre L1
	stmfd   r13!, {r4-r12, r14}
	ldr	r12, =OMAP4_MON_SCU_PWR_INDEX
	DO_SMC
	ldmfd   r13!, {r4-r12, r14}

skip_secure_l1_clean:

	/*
	 * 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 data 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

	/*
	 * Switch the CPU from Symmetric Multiprocessing (SMP) mode
	 * to AsymmetricMultiprocessing (AMP) mode by programming
	 * the SCU power status to DORMANT or OFF mode.
	 * This enables the CPU to be taken out of coherency by
	 * preventing the CPU from receiving cache, TLB, or BTB
	 * maintenance operations broadcast by other CPUs in the cluster.
	 */
	bl	omap4_get_sar_ram_base
	mov	r8, r0
	ldr	r9, [r8, #OMAP_TYPE_OFFSET]
	cmp	r9, #0x1			@ Check for HS device
	bne	scu_gp_set
	mrc	p15, 0, r0, c0, c0, 5		@ Read MPIDR
	ands	r0, r0, #0x0f
	ldreq	r0, [r8, #SCU_OFFSET0]
	ldrne	r0, [r8, #SCU_OFFSET1]
	mov	r1, #0x00
	stmfd   r13!, {r4-r12, r14}
	ldr	r12, =OMAP4_MON_SCU_PWR_INDEX
	DO_SMC
	ldmfd   r13!, {r4-r12, r14}
	b	skip_scu_gp_set
scu_gp_set:
	mrc	p15, 0, r0, c0, c0, 5		@ Read MPIDR
	ands	r0, r0, #0x0f
	ldreq	r1, [r8, #SCU_OFFSET0]
	ldrne	r1, [r8, #SCU_OFFSET1]
	bl	omap4_get_scu_base
	bl	scu_power_mode
skip_scu_gp_set:
	mrc	p15, 0, r0, c1, c1, 2		@ Read NSACR data
	tst	r0, #(1 << 18)
	mrcne	p15, 0, r0, c1, c0, 1
	bicne	r0, r0, #(1 << 6)		@ Disable SMP bit
	mcrne	p15, 0, r0, c1, c0, 1
	isb
	dsb

#ifdef CONFIG_CACHE_L2X0
/*
* 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.
* OMAP4 hardware doesn't support exclusive monitor which can
* overcome exclusive access issue. Because of this, CPU can
* lead to deadlock.
*/
l2x_clean_inv:

	bl	omap4_get_sar_ram_base
	mov	r8, r0
	mrc	p15, 0, r5, c0, c0, 5		@ Read MPIDR
	ands	r5, r5, #0x0f
	ldreq	r0, [r8, #L2X0_SAVE_OFFSET0]
	ldrne	r0, [r8, #L2X0_SAVE_OFFSET1]
	cmp	r0, #3
	bne	do_WFI

#ifdef CONFIG_PL310_ERRATA_727915
	mov	r0, #0x03
	mov	r12, #OMAP4_MON_L2X0_DBG_CTRL_INDEX
	DO_SMC
#endif
	bl	omap4_get_l2cache_base
	mov	r2, r0
	ldr	r0, =0xffff
	str	r0, [r2, #L2X0_CLEAN_INV_WAY]
wait:
	ldr	r0, [r2, #L2X0_CLEAN_INV_WAY]
	ldr	r1, =0xffff
	ands	r0, r0, r1
	bne	wait
#ifdef CONFIG_PL310_ERRATA_727915
	mov	r0, #0x00
	mov	r12, #OMAP4_MON_L2X0_DBG_CTRL_INDEX
	DO_SMC
#endif
l2x_sync:
	bl	omap4_get_l2cache_base
	mov	r2, r0
	mov	r0, #0x0
	str	r0, [r2, #L2X0_CACHE_SYNC]
sync:
	ldr	r0, [r2, #L2X0_CACHE_SYNC]
	ands	r0, r0, #0x1
	bne	sync

#endif

do_WFI:

	bl	omap_do_wfi

	/*
	 * CPU is here when it failed to enter OFF/DORMANT or
	 * no low power state was attempted.
	 */
	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

	/*
	 * Ensure the CPU power state is set to NORMAL in
	 * SCU power state so that CPU is back in coherency.
	 * In non-coherent mode CPU can lock-up and lead to
	 * system deadlock.
	 */
	mrc	p15, 0, r0, c1, c0, 1
	tst	r0, #(1 << 6)			@ Check SMP bit enabled?
	orreq	r0, r0, #(1 << 6)
	mcreq	p15, 0, r0, c1, c0, 1
	isb
	bl	omap4_get_sar_ram_base
	mov	r8, r0
	ldr	r9, [r8, #OMAP_TYPE_OFFSET]
	cmp	r9, #0x1			@ Check for HS device
	bne	scu_gp_clear
	mov	r0, #SCU_PM_NORMAL
	mov	r1, #0x00
	stmfd   r13!, {r4-r12, r14}
	ldr	r12, =OMAP4_MON_SCU_PWR_INDEX
	DO_SMC
	ldmfd   r13!, {r4-r12, r14}
	b	skip_scu_gp_clear
scu_gp_clear:
	bl	omap4_get_scu_base
	mov	r1, #SCU_PM_NORMAL
	bl	scu_power_mode
skip_scu_gp_clear:
	isb
	dsb
	ldmfd	sp!, {pc}

ENDPROC(omap4_finish_suspend)

Quoted 4 lines · Santosh

Quoted 3 lines · Shawn Guo

Quoted 2 lines · Santosh

Quoted 1 line · Shawn Guo

Hi Santosh,

Quoted 12 lines

#ifdef CONFIG_CACHE_L2X0
/*
* 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.
* OMAP4 hardware doesn't support exclusive monitor which can
* overcome exclusive access issue. Because of this, CPU can
* lead to deadlock.
*/
l2x_clean_inv:

Quoted 8 lines

	bl	omap4_get_sar_ram_base
	mov	r8, r0
	mrc	p15, 0, r5, c0, c0, 5		@ Read MPIDR
	ands	r5, r5, #0x0f
	ldreq	r0, [r8, #L2X0_SAVE_OFFSET0]
	ldrne	r0, [r8, #L2X0_SAVE_OFFSET1]
	cmp	r0, #3
	bne	do_WFI

It looks like you are bypassing L2 clean and invalidate for cases "1" and "2" here. But I really do not understand how you get r0 back here.

Quoted 5 lines · Shawn Guo

Quoted 4 lines · Santosh

Quoted 3 lines · Shawn Guo

Quoted 2 lines · Santosh

Quoted 1 line · Shawn Guo

Hi Santosh,

[...]

Quoted 21 lines

Quoted 20 lines

#ifdef CONFIG_CACHE_L2X0
/*
* 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.
* OMAP4 hardware doesn't support exclusive monitor which can
* overcome exclusive access issue. Because of this, CPU can
* lead to deadlock.
*/
l2x_clean_inv:

	bl	omap4_get_sar_ram_base
	mov	r8, r0
	mrc	p15, 0, r5, c0, c0, 5		@ Read MPIDR
	ands	r5, r5, #0x0f
	ldreq	r0, [r8, #L2X0_SAVE_OFFSET0]
	ldrne	r0, [r8, #L2X0_SAVE_OFFSET1]
	cmp	r0, #3
	bne	do_WFI

Quoted 4 lines

It looks like you are bypassing L2 clean and invalidate for cases "1" and "2" here. But I really do not understand how you get r0 back here.

The value which is passed in R0 is also stored in scratch patch memory and retrieved using L2X0_SAVE_OFFSET0.
Simple :)

Quoted 34 lines · Santosh

Quoted 5 lines · Shawn Guo

Quoted 4 lines · Santosh

Quoted 3 lines · On Fri, Sep 9, 2011 at 7:43 PM, Shawn Guo<[email protected]> ?wrote:

Quoted 2 lines · Santosh

Quoted 1 line · Shawn Guo

Hi Santosh,

[...]

Quoted 25 lines

Quoted 12 lines

#ifdef CONFIG_CACHE_L2X0
? ? /*
? ? ?* 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. ? ? ?* OMAP4 hardware doesn't support exclusive monitor which can ? ? ?* overcome exclusive access issue. Because of this, CPU can ? ? ?* lead to deadlock.
? ? ?*/
l2x_clean_inv:

Quoted 8 lines

? ? bl ? ? ?omap4_get_sar_ram_base
? ? mov ? ? r8, r0
? ? mrc ? ? p15, 0, r5, c0, c0, 5 ? ? ? ? ? @ Read MPIDR ? ? ands ? ?r5, r5, #0x0f
? ? ldreq ? r0, [r8, #L2X0_SAVE_OFFSET0]
? ? ldrne ? r0, [r8, #L2X0_SAVE_OFFSET1]
? ? cmp ? ? r0, #3
? ? bne ? ? do_WFI

It looks like you are bypassing L2 clean and invalidate for cases "1" and "2" here. ?But I really do not understand how you get r0 back here.

Quoted 3 lines

The value which is passed in R0 is also stored in scratch patch memory and retrieved using L2X0_SAVE_OFFSET0.
Simple :)

Sounds like some in-code documentation needs to be added to make this a bit more understandable.

Quoted 5 lines · Shawn Guo

Quoted 4 lines · Santosh

Quoted 3 lines · Shawn Guo

Quoted 2 lines · Santosh

Quoted 1 line · Shawn Guo

Hi Santosh,

[...]

Quoted 21 lines

Quoted 20 lines

#ifdef CONFIG_CACHE_L2X0
/*
* 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.
* OMAP4 hardware doesn't support exclusive monitor which can
* overcome exclusive access issue. Because of this, CPU can
* lead to deadlock.
*/
l2x_clean_inv:

	bl	omap4_get_sar_ram_base
	mov	r8, r0
	mrc	p15, 0, r5, c0, c0, 5		@ Read MPIDR
	ands	r5, r5, #0x0f
	ldreq	r0, [r8, #L2X0_SAVE_OFFSET0]
	ldrne	r0, [r8, #L2X0_SAVE_OFFSET1]
	cmp	r0, #3
	bne	do_WFI

Quoted 4 lines

It looks like you are bypassing L2 clean and invalidate for cases "1" and "2" here. But I really do not understand how you get r0 back here.

The value which is passed in R0 is also stored in scratch patch memory and retrieved using L2X0_SAVE_OFFSET0.
Simple :)

Quoted 34 lines · Santosh

Quoted 5 lines · Shawn Guo

Quoted 4 lines · Santosh

Quoted 3 lines · On Fri, Sep 9, 2011 at 7:43 PM, Shawn Guo<[email protected]> ?wrote:

Quoted 2 lines · Santosh

Quoted 1 line · Shawn Guo

Hi Santosh,

[...]

Quoted 25 lines

Quoted 12 lines

#ifdef CONFIG_CACHE_L2X0
? ?/*
? ? * 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. ? ? * OMAP4 hardware doesn't support exclusive monitor which can ? ? * overcome exclusive access issue. Because of this, CPU can ? ? * lead to deadlock.
? ? */
l2x_clean_inv:

Quoted 8 lines

? ?bl ? ? ?omap4_get_sar_ram_base
? ?mov ? ? r8, r0
? ?mrc ? ? p15, 0, r5, c0, c0, 5 ? ? ? ? ? @ Read MPIDR ? ?ands ? ?r5, r5, #0x0f
? ?ldreq ? r0, [r8, #L2X0_SAVE_OFFSET0]
? ?ldrne ? r0, [r8, #L2X0_SAVE_OFFSET1]
? ?cmp ? ? r0, #3
? ?bne ? ? do_WFI

It looks like you are bypassing L2 clean and invalidate for cases "1" and "2" here. ?But I really do not understand how you get r0 back here.

Quoted 4 lines

The value which is passed in R0 is also stored in scratch patch memory and retrieved using L2X0_SAVE_OFFSET0.
Simple :)

Aha, no wonder that I read it wrongly. ?The parameter passing in r0 is here simply for confusing people.

Also, IMO, lable "l2x_clean_inv" should be put after the "bne do_WFI". Otherwise, my original statement (it seems l2x_clean_inv will be called for case "2") stands correct :)