* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:27 ` [linux-pm] " Santosh Shilimkar
@ 2011-06-09 16:40 ` Russell King - ARM Linux
2011-06-09 16:40 ` [linux-pm] " Russell King - ARM Linux
` (2 subsequent siblings)
3 siblings, 0 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-09 16:40 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Frank Hofmann, linux-pm, tuxonice-devel, linux-arm-kernel
On Thu, Jun 09, 2011 at 09:57:06PM +0530, Santosh Shilimkar wrote:
> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>
>> That's because OMAP was doing changes to their sleep code while I was
>> consolidating the sleep code, and although I asked several times that
>> the OMAP folk should participate in this effort, but evidentally I was
>> unsuccessful in achieving anything in that direction.
>
> Agreed but the situation at that point was the code was not at
> all in convertible position. Looking at your below comment,
> it's still not :)
Well, I had a look before posting this reply, and ran away from it.
I've gone back to it several times since, and got a similar reaction.
I seem to remember that it looked _more_ convertable when I looked at
it when doing the generic suspend/resume support - I could see a nice
simple way to pull out the saving and just leave the PLL resume stuff
in SRAM.
I'm now convinced that if I try to convert it use the generic support,
it will end up being a horrible broken mess.
^ permalink raw reply [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:27 ` [linux-pm] " Santosh Shilimkar
2011-06-09 16:40 ` Russell King - ARM Linux
@ 2011-06-09 16:40 ` Russell King - ARM Linux
2011-06-09 16:53 ` Santosh Shilimkar
2011-06-09 16:53 ` Santosh Shilimkar
2011-06-09 16:44 ` [linux-pm] " Frank Hofmann
2011-06-09 16:44 ` Frank Hofmann
3 siblings, 2 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-09 16:40 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 09, 2011 at 09:57:06PM +0530, Santosh Shilimkar wrote:
> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>
>> That's because OMAP was doing changes to their sleep code while I was
>> consolidating the sleep code, and although I asked several times that
>> the OMAP folk should participate in this effort, but evidentally I was
>> unsuccessful in achieving anything in that direction.
>
> Agreed but the situation at that point was the code was not at
> all in convertible position. Looking at your below comment,
> it's still not :)
Well, I had a look before posting this reply, and ran away from it.
I've gone back to it several times since, and got a similar reaction.
I seem to remember that it looked _more_ convertable when I looked at
it when doing the generic suspend/resume support - I could see a nice
simple way to pull out the saving and just leave the PLL resume stuff
in SRAM.
I'm now convinced that if I try to convert it use the generic support,
it will end up being a horrible broken mess.
^ permalink raw reply [flat|nested] 49+ messages in thread
* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:40 ` [linux-pm] " Russell King - ARM Linux
@ 2011-06-09 16:53 ` Santosh Shilimkar
2011-06-09 17:12 ` Russell King - ARM Linux
2011-06-09 17:12 ` [linux-pm] " Russell King - ARM Linux
2011-06-09 16:53 ` Santosh Shilimkar
1 sibling, 2 replies; 49+ messages in thread
From: Santosh Shilimkar @ 2011-06-09 16:53 UTC (permalink / raw)
To: linux-arm-kernel
On 6/9/2011 10:10 PM, Russell King - ARM Linux wrote:
> On Thu, Jun 09, 2011 at 09:57:06PM +0530, Santosh Shilimkar wrote:
>> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>>
>>> That's because OMAP was doing changes to their sleep code while I was
>>> consolidating the sleep code, and although I asked several times that
>>> the OMAP folk should participate in this effort, but evidentally I was
>>> unsuccessful in achieving anything in that direction.
>>
>> Agreed but the situation at that point was the code was not at
>> all in convertible position. Looking at your below comment,
>> it's still not :)
>
> Well, I had a look before posting this reply, and ran away from it.
> I've gone back to it several times since, and got a similar reaction.
>
> I seem to remember that it looked _more_ convertable when I looked at
> it when doing the generic suspend/resume support - I could see a nice
> simple way to pull out the saving and just leave the PLL resume stuff
> in SRAM.
>
> I'm now convinced that if I try to convert it use the generic support,
> it will end up being a horrible broken mess.
I must admit that I had same impression when I started looking at it.
Few provisions are necessary for OMAP which I can think of are:
1. WFI loop should be made a seperate function so that it can pushed
on SRAM which is must for OMAP3.
2. A callback before WFI to implement the Errata WA's
3. Avoid direct write to AUXCTRL in generic suspend code.
4. Before MMU is enabled in resume a callback to restore
secure register, setup auxctrl etc.
With above addressed, mostly we should be able to
get it working. But for sure it will mess up the
simple suspend hooks as they are today.
btw, for OMAP4 as well I looked at this suspend hooks
and most the requirement above apply except 4)
Additionally the L2 cache handling isn't part of
these common suspend hooks.
Regards
Santosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:53 ` Santosh Shilimkar
@ 2011-06-09 17:12 ` Russell King - ARM Linux
2011-06-09 17:12 ` [linux-pm] " Russell King - ARM Linux
1 sibling, 0 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-09 17:12 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Frank Hofmann, linux-pm, tuxonice-devel, linux-arm-kernel
On Thu, Jun 09, 2011 at 10:23:07PM +0530, Santosh Shilimkar wrote:
> On 6/9/2011 10:10 PM, Russell King - ARM Linux wrote:
>> On Thu, Jun 09, 2011 at 09:57:06PM +0530, Santosh Shilimkar wrote:
>>> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>>>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>>>
>>>> That's because OMAP was doing changes to their sleep code while I was
>>>> consolidating the sleep code, and although I asked several times that
>>>> the OMAP folk should participate in this effort, but evidentally I was
>>>> unsuccessful in achieving anything in that direction.
>>>
>>> Agreed but the situation at that point was the code was not at
>>> all in convertible position. Looking at your below comment,
>>> it's still not :)
>>
>> Well, I had a look before posting this reply, and ran away from it.
>> I've gone back to it several times since, and got a similar reaction.
>>
>> I seem to remember that it looked _more_ convertable when I looked at
>> it when doing the generic suspend/resume support - I could see a nice
>> simple way to pull out the saving and just leave the PLL resume stuff
>> in SRAM.
>>
>> I'm now convinced that if I try to convert it use the generic support,
>> it will end up being a horrible broken mess.
>
> I must admit that I had same impression when I started looking at it.
>
> Few provisions are necessary for OMAP which I can think of are:
> 1. WFI loop should be made a seperate function so that it can pushed
> on SRAM which is must for OMAP3.
If you look at the generic cpu suspend, it sits in the suspend path.
Once cpu_suspend() has been called, it will return as normal and you
can then do whatever you require to place the system into suspend -
including calling out to a function in WFI.
> 2. A callback before WFI to implement the Errata WA's
"WA's" ?
> 3. Avoid direct write to AUXCTRL in generic suspend code.
This is the only problematical one that I can see. We need to restore
this on systems running in secure mode. What we could do is rather than
writing to the register, read it first and compare its value with what
was saved to see whether we need to write it.
Then, if platforms run in non-secure mode, they are responsible for
restoring that register back to its pre-suspend value before their
assembly calls cpu_resume().
> 4. Before MMU is enabled in resume a callback to restore
> secure register, setup auxctrl etc.
You can do this before your assembly calls cpu_resume().
> Additionally the L2 cache handling isn't part of
> these common suspend hooks.
L2 cache handling can't fit into the generic code - it doesn't really
belong there either. It needs to be in the parent or hooked into the
syscore_ops stuff as I've said previously.
So:
ENTRY(my_soc_suspend)
stmfd sp!, {r4 - r12, lr}
ldr r3, =resume
bl cpu_suspend
/*
* Insert whatever code is required here for suspend
* eg, save secure mode, then jump to sram to call WFI function
*/
resume:
ldmfd sp!, {r4 - r12, pc}
ENDPROC(my_soc_suspend)
ENTRY(my_soc_resume)
/*
* Insert whatever other code is required to be run before resume
* eg, WFI function returns to this symbol after DDR becomes
* accessible. restore secure mode state
*/
b cpu_resume
ENDPROC(my_soc_resume)
What makes it far more complicated in the OMAP case is all that "is l1 state
lost? is l2 state lost?" stuff.
^ permalink raw reply [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:53 ` Santosh Shilimkar
2011-06-09 17:12 ` Russell King - ARM Linux
@ 2011-06-09 17:12 ` Russell King - ARM Linux
2011-06-09 17:21 ` Santosh Shilimkar
` (5 more replies)
1 sibling, 6 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-09 17:12 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 09, 2011 at 10:23:07PM +0530, Santosh Shilimkar wrote:
> On 6/9/2011 10:10 PM, Russell King - ARM Linux wrote:
>> On Thu, Jun 09, 2011 at 09:57:06PM +0530, Santosh Shilimkar wrote:
>>> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>>>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>>>
>>>> That's because OMAP was doing changes to their sleep code while I was
>>>> consolidating the sleep code, and although I asked several times that
>>>> the OMAP folk should participate in this effort, but evidentally I was
>>>> unsuccessful in achieving anything in that direction.
>>>
>>> Agreed but the situation at that point was the code was not at
>>> all in convertible position. Looking at your below comment,
>>> it's still not :)
>>
>> Well, I had a look before posting this reply, and ran away from it.
>> I've gone back to it several times since, and got a similar reaction.
>>
>> I seem to remember that it looked _more_ convertable when I looked at
>> it when doing the generic suspend/resume support - I could see a nice
>> simple way to pull out the saving and just leave the PLL resume stuff
>> in SRAM.
>>
>> I'm now convinced that if I try to convert it use the generic support,
>> it will end up being a horrible broken mess.
>
> I must admit that I had same impression when I started looking at it.
>
> Few provisions are necessary for OMAP which I can think of are:
> 1. WFI loop should be made a seperate function so that it can pushed
> on SRAM which is must for OMAP3.
If you look at the generic cpu suspend, it sits in the suspend path.
Once cpu_suspend() has been called, it will return as normal and you
can then do whatever you require to place the system into suspend -
including calling out to a function in WFI.
> 2. A callback before WFI to implement the Errata WA's
"WA's" ?
> 3. Avoid direct write to AUXCTRL in generic suspend code.
This is the only problematical one that I can see. We need to restore
this on systems running in secure mode. What we could do is rather than
writing to the register, read it first and compare its value with what
was saved to see whether we need to write it.
Then, if platforms run in non-secure mode, they are responsible for
restoring that register back to its pre-suspend value before their
assembly calls cpu_resume().
> 4. Before MMU is enabled in resume a callback to restore
> secure register, setup auxctrl etc.
You can do this before your assembly calls cpu_resume().
> Additionally the L2 cache handling isn't part of
> these common suspend hooks.
L2 cache handling can't fit into the generic code - it doesn't really
belong there either. It needs to be in the parent or hooked into the
syscore_ops stuff as I've said previously.
So:
ENTRY(my_soc_suspend)
stmfd sp!, {r4 - r12, lr}
ldr r3, =resume
bl cpu_suspend
/*
* Insert whatever code is required here for suspend
* eg, save secure mode, then jump to sram to call WFI function
*/
resume:
ldmfd sp!, {r4 - r12, pc}
ENDPROC(my_soc_suspend)
ENTRY(my_soc_resume)
/*
* Insert whatever other code is required to be run before resume
* eg, WFI function returns to this symbol after DDR becomes
* accessible. restore secure mode state
*/
b cpu_resume
ENDPROC(my_soc_resume)
What makes it far more complicated in the OMAP case is all that "is l1 state
lost? is l2 state lost?" stuff.
^ permalink raw reply [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 17:12 ` [linux-pm] " Russell King - ARM Linux
@ 2011-06-09 17:21 ` Santosh Shilimkar
2011-06-09 17:21 ` Santosh Shilimkar
` (4 subsequent siblings)
5 siblings, 0 replies; 49+ messages in thread
From: Santosh Shilimkar @ 2011-06-09 17:21 UTC (permalink / raw)
To: linux-arm-kernel
On 6/9/2011 10:42 PM, Russell King - ARM Linux wrote:
> On Thu, Jun 09, 2011 at 10:23:07PM +0530, Santosh Shilimkar wrote:
>> On 6/9/2011 10:10 PM, Russell King - ARM Linux wrote:
>>> On Thu, Jun 09, 2011 at 09:57:06PM +0530, Santosh Shilimkar wrote:
>>>> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>>>>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>>>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>>>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>>>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>>>>
>>>>> That's because OMAP was doing changes to their sleep code while I was
>>>>> consolidating the sleep code, and although I asked several times that
>>>>> the OMAP folk should participate in this effort, but evidentally I was
>>>>> unsuccessful in achieving anything in that direction.
>>>>
>>>> Agreed but the situation at that point was the code was not at
>>>> all in convertible position. Looking at your below comment,
>>>> it's still not :)
>>>
>>> Well, I had a look before posting this reply, and ran away from it.
>>> I've gone back to it several times since, and got a similar reaction.
>>>
>>> I seem to remember that it looked _more_ convertable when I looked at
>>> it when doing the generic suspend/resume support - I could see a nice
>>> simple way to pull out the saving and just leave the PLL resume stuff
>>> in SRAM.
>>>
>>> I'm now convinced that if I try to convert it use the generic support,
>>> it will end up being a horrible broken mess.
>>
>> I must admit that I had same impression when I started looking at it.
>>
>> Few provisions are necessary for OMAP which I can think of are:
>> 1. WFI loop should be made a seperate function so that it can pushed
>> on SRAM which is must for OMAP3.
>
> If you look at the generic cpu suspend, it sits in the suspend path.
> Once cpu_suspend() has been called, it will return as normal and you
> can then do whatever you require to place the system into suspend -
> including calling out to a function in WFI.
>
>> 2. A callback before WFI to implement the Errata WA's
>
> "WA's" ?
>
Software Work-Arounds for issues around WFI or special
cases.
>> 3. Avoid direct write to AUXCTRL in generic suspend code.
>
> This is the only problematical one that I can see. We need to restore
> this on systems running in secure mode. What we could do is rather than
> writing to the register, read it first and compare its value with what
> was saved to see whether we need to write it.
>
> Then, if platforms run in non-secure mode, they are responsible for
> restoring that register back to its pre-suspend value before their
> assembly calls cpu_resume().
>
>> 4. Before MMU is enabled in resume a callback to restore
>> secure register, setup auxctrl etc.
>
> You can do this before your assembly calls cpu_resume().
>
>> Additionally the L2 cache handling isn't part of
>> these common suspend hooks.
>
> L2 cache handling can't fit into the generic code - it doesn't really
> belong there either. It needs to be in the parent or hooked into the
> syscore_ops stuff as I've said previously.
>
Ok. I missed these points in last discussion.
> So:
>
> ENTRY(my_soc_suspend)
> stmfd sp!, {r4 - r12, lr}
> ldr r3, =resume
> bl cpu_suspend
> /*
> * Insert whatever code is required here for suspend
> * eg, save secure mode, then jump to sram to call WFI function
> */
> resume:
> ldmfd sp!, {r4 - r12, pc}
> ENDPROC(my_soc_suspend)
>
> ENTRY(my_soc_resume)
> /*
> * Insert whatever other code is required to be run before resume
> * eg, WFI function returns to this symbol after DDR becomes
> * accessible. restore secure mode state
> */
> b cpu_resume
> ENDPROC(my_soc_resume)
>
> What makes it far more complicated in the OMAP case is all that "is l1 state
> lost? is l2 state lost?" stuff.
Exactly. And that's where the most complexity comes in. Also on OMAP
we use same sleep code for CPUidle and suspend and in idle, there
are many low power states possible with variation of L1, l2, CPU logic,
L2 controller logic, interrupt controller logic etc.
Thanks for bringing up these points. Now I better understand
as well why I struggled to get anything running relaibly
on OMAP4 with few hours attempt with suspend hooks.
Regards
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 17:12 ` [linux-pm] " Russell King - ARM Linux
2011-06-09 17:21 ` Santosh Shilimkar
@ 2011-06-09 17:21 ` Santosh Shilimkar
2011-06-09 17:53 ` [linux-pm] " Russell King - ARM Linux
` (3 subsequent siblings)
5 siblings, 0 replies; 49+ messages in thread
From: Santosh Shilimkar @ 2011-06-09 17:21 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Frank Hofmann, linux-pm, tuxonice-devel, linux-arm-kernel
On 6/9/2011 10:42 PM, Russell King - ARM Linux wrote:
> On Thu, Jun 09, 2011 at 10:23:07PM +0530, Santosh Shilimkar wrote:
>> On 6/9/2011 10:10 PM, Russell King - ARM Linux wrote:
>>> On Thu, Jun 09, 2011 at 09:57:06PM +0530, Santosh Shilimkar wrote:
>>>> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>>>>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>>>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>>>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>>>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>>>>
>>>>> That's because OMAP was doing changes to their sleep code while I was
>>>>> consolidating the sleep code, and although I asked several times that
>>>>> the OMAP folk should participate in this effort, but evidentally I was
>>>>> unsuccessful in achieving anything in that direction.
>>>>
>>>> Agreed but the situation at that point was the code was not at
>>>> all in convertible position. Looking at your below comment,
>>>> it's still not :)
>>>
>>> Well, I had a look before posting this reply, and ran away from it.
>>> I've gone back to it several times since, and got a similar reaction.
>>>
>>> I seem to remember that it looked _more_ convertable when I looked at
>>> it when doing the generic suspend/resume support - I could see a nice
>>> simple way to pull out the saving and just leave the PLL resume stuff
>>> in SRAM.
>>>
>>> I'm now convinced that if I try to convert it use the generic support,
>>> it will end up being a horrible broken mess.
>>
>> I must admit that I had same impression when I started looking at it.
>>
>> Few provisions are necessary for OMAP which I can think of are:
>> 1. WFI loop should be made a seperate function so that it can pushed
>> on SRAM which is must for OMAP3.
>
> If you look at the generic cpu suspend, it sits in the suspend path.
> Once cpu_suspend() has been called, it will return as normal and you
> can then do whatever you require to place the system into suspend -
> including calling out to a function in WFI.
>
>> 2. A callback before WFI to implement the Errata WA's
>
> "WA's" ?
>
Software Work-Arounds for issues around WFI or special
cases.
>> 3. Avoid direct write to AUXCTRL in generic suspend code.
>
> This is the only problematical one that I can see. We need to restore
> this on systems running in secure mode. What we could do is rather than
> writing to the register, read it first and compare its value with what
> was saved to see whether we need to write it.
>
> Then, if platforms run in non-secure mode, they are responsible for
> restoring that register back to its pre-suspend value before their
> assembly calls cpu_resume().
>
>> 4. Before MMU is enabled in resume a callback to restore
>> secure register, setup auxctrl etc.
>
> You can do this before your assembly calls cpu_resume().
>
>> Additionally the L2 cache handling isn't part of
>> these common suspend hooks.
>
> L2 cache handling can't fit into the generic code - it doesn't really
> belong there either. It needs to be in the parent or hooked into the
> syscore_ops stuff as I've said previously.
>
Ok. I missed these points in last discussion.
> So:
>
> ENTRY(my_soc_suspend)
> stmfd sp!, {r4 - r12, lr}
> ldr r3, =resume
> bl cpu_suspend
> /*
> * Insert whatever code is required here for suspend
> * eg, save secure mode, then jump to sram to call WFI function
> */
> resume:
> ldmfd sp!, {r4 - r12, pc}
> ENDPROC(my_soc_suspend)
>
> ENTRY(my_soc_resume)
> /*
> * Insert whatever other code is required to be run before resume
> * eg, WFI function returns to this symbol after DDR becomes
> * accessible. restore secure mode state
> */
> b cpu_resume
> ENDPROC(my_soc_resume)
>
> What makes it far more complicated in the OMAP case is all that "is l1 state
> lost? is l2 state lost?" stuff.
Exactly. And that's where the most complexity comes in. Also on OMAP
we use same sleep code for CPUidle and suspend and in idle, there
are many low power states possible with variation of L1, l2, CPU logic,
L2 controller logic, interrupt controller logic etc.
Thanks for bringing up these points. Now I better understand
as well why I struggled to get anything running relaibly
on OMAP4 with few hours attempt with suspend hooks.
Regards
^ permalink raw reply [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 17:12 ` [linux-pm] " Russell King - ARM Linux
2011-06-09 17:21 ` Santosh Shilimkar
2011-06-09 17:21 ` Santosh Shilimkar
@ 2011-06-09 17:53 ` Russell King - ARM Linux
2011-06-21 10:11 ` Russell King - ARM Linux
2011-06-21 10:11 ` Russell King - ARM Linux
2011-06-09 17:53 ` Russell King - ARM Linux
` (2 subsequent siblings)
5 siblings, 2 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-09 17:53 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 09, 2011 at 06:12:55PM +0100, Russell King - ARM Linux wrote:
> > 3. Avoid direct write to AUXCTRL in generic suspend code.
>
> This is the only problematical one that I can see. We need to restore
> this on systems running in secure mode. What we could do is rather than
> writing to the register, read it first and compare its value with what
> was saved to see whether we need to write it.
>
> Then, if platforms run in non-secure mode, they are responsible for
> restoring that register back to its pre-suspend value before their
> assembly calls cpu_resume().
And here's a patch which does that:
8<-----------
From: Russell King <rmk+kernel@arm.linux.org.uk>
ARM: Avoid writing to auxctrl register unless it needs to be updated
As the auxiliary control register is not writable in non-secure mode
such as on OMAP, we must avoid writing the register when resuming in
non-secure mode. Avoid this by moving the responsibility to the
SoC code in this case to ensure that the auxiliary control register
is restored before cpu_resume() is called.
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
--
arch/arm/mm/proc-v7.S | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 3c38678..fa1e6d5 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -237,7 +237,9 @@ ENTRY(cpu_v7_do_resume)
mcr p15, 0, r7, c2, c0, 0 @ TTB 0
mcr p15, 0, r8, c2, c0, 1 @ TTB 1
mcr p15, 0, ip, c2, c0, 2 @ TTB control register
- mcr p15, 0, r10, c1, c0, 1 @ Auxiliary control register
+ mrc p15, 0, r4, c1, c0, 1 @ Read auxiliary control register
+ teq r4, r10
+ mcrne p15, 0, r10, c1, c0, 1 @ Auxiliary control register
mcr p15, 0, r11, c1, c0, 2 @ Co-processor access control
ldr r4, =PRRR @ PRRR
ldr r5, =NMRR @ NMRR
^ permalink raw reply related [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 17:53 ` [linux-pm] " Russell King - ARM Linux
@ 2011-06-21 10:11 ` Russell King - ARM Linux
2011-06-21 10:11 ` Russell King - ARM Linux
1 sibling, 0 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-21 10:11 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 09, 2011 at 06:53:13PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 09, 2011 at 06:12:55PM +0100, Russell King - ARM Linux wrote:
> > > 3. Avoid direct write to AUXCTRL in generic suspend code.
> >
> > This is the only problematical one that I can see. We need to restore
> > this on systems running in secure mode. What we could do is rather than
> > writing to the register, read it first and compare its value with what
> > was saved to see whether we need to write it.
> >
> > Then, if platforms run in non-secure mode, they are responsible for
> > restoring that register back to its pre-suspend value before their
> > assembly calls cpu_resume().
>
> And here's a patch which does that:
Ping.
> 8<-----------
> From: Russell King <rmk+kernel@arm.linux.org.uk>
> ARM: Avoid writing to auxctrl register unless it needs to be updated
>
> As the auxiliary control register is not writable in non-secure mode
> such as on OMAP, we must avoid writing the register when resuming in
> non-secure mode. Avoid this by moving the responsibility to the
> SoC code in this case to ensure that the auxiliary control register
> is restored before cpu_resume() is called.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> --
> arch/arm/mm/proc-v7.S | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index 3c38678..fa1e6d5 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -237,7 +237,9 @@ ENTRY(cpu_v7_do_resume)
> mcr p15, 0, r7, c2, c0, 0 @ TTB 0
> mcr p15, 0, r8, c2, c0, 1 @ TTB 1
> mcr p15, 0, ip, c2, c0, 2 @ TTB control register
> - mcr p15, 0, r10, c1, c0, 1 @ Auxiliary control register
> + mrc p15, 0, r4, c1, c0, 1 @ Read auxiliary control register
> + teq r4, r10
> + mcrne p15, 0, r10, c1, c0, 1 @ Auxiliary control register
> mcr p15, 0, r11, c1, c0, 2 @ Co-processor access control
> ldr r4, =PRRR @ PRRR
> ldr r5, =NMRR @ NMRR
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 17:53 ` [linux-pm] " Russell King - ARM Linux
2011-06-21 10:11 ` Russell King - ARM Linux
@ 2011-06-21 10:11 ` Russell King - ARM Linux
1 sibling, 0 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-21 10:11 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Frank Hofmann, linux-pm, tuxonice-devel, linux-arm-kernel
On Thu, Jun 09, 2011 at 06:53:13PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 09, 2011 at 06:12:55PM +0100, Russell King - ARM Linux wrote:
> > > 3. Avoid direct write to AUXCTRL in generic suspend code.
> >
> > This is the only problematical one that I can see. We need to restore
> > this on systems running in secure mode. What we could do is rather than
> > writing to the register, read it first and compare its value with what
> > was saved to see whether we need to write it.
> >
> > Then, if platforms run in non-secure mode, they are responsible for
> > restoring that register back to its pre-suspend value before their
> > assembly calls cpu_resume().
>
> And here's a patch which does that:
Ping.
> 8<-----------
> From: Russell King <rmk+kernel@arm.linux.org.uk>
> ARM: Avoid writing to auxctrl register unless it needs to be updated
>
> As the auxiliary control register is not writable in non-secure mode
> such as on OMAP, we must avoid writing the register when resuming in
> non-secure mode. Avoid this by moving the responsibility to the
> SoC code in this case to ensure that the auxiliary control register
> is restored before cpu_resume() is called.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> --
> arch/arm/mm/proc-v7.S | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index 3c38678..fa1e6d5 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -237,7 +237,9 @@ ENTRY(cpu_v7_do_resume)
> mcr p15, 0, r7, c2, c0, 0 @ TTB 0
> mcr p15, 0, r8, c2, c0, 1 @ TTB 1
> mcr p15, 0, ip, c2, c0, 2 @ TTB control register
> - mcr p15, 0, r10, c1, c0, 1 @ Auxiliary control register
> + mrc p15, 0, r4, c1, c0, 1 @ Read auxiliary control register
> + teq r4, r10
> + mcrne p15, 0, r10, c1, c0, 1 @ Auxiliary control register
> mcr p15, 0, r11, c1, c0, 2 @ Co-processor access control
> ldr r4, =PRRR @ PRRR
> ldr r5, =NMRR @ NMRR
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 17:12 ` [linux-pm] " Russell King - ARM Linux
` (2 preceding siblings ...)
2011-06-09 17:53 ` [linux-pm] " Russell King - ARM Linux
@ 2011-06-09 17:53 ` Russell King - ARM Linux
2011-06-10 12:22 ` [linux-pm] " Frank Hofmann
2011-06-10 12:22 ` Frank Hofmann
5 siblings, 0 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-09 17:53 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Frank Hofmann, linux-pm, tuxonice-devel, linux-arm-kernel
On Thu, Jun 09, 2011 at 06:12:55PM +0100, Russell King - ARM Linux wrote:
> > 3. Avoid direct write to AUXCTRL in generic suspend code.
>
> This is the only problematical one that I can see. We need to restore
> this on systems running in secure mode. What we could do is rather than
> writing to the register, read it first and compare its value with what
> was saved to see whether we need to write it.
>
> Then, if platforms run in non-secure mode, they are responsible for
> restoring that register back to its pre-suspend value before their
> assembly calls cpu_resume().
And here's a patch which does that:
8<-----------
From: Russell King <rmk+kernel@arm.linux.org.uk>
ARM: Avoid writing to auxctrl register unless it needs to be updated
As the auxiliary control register is not writable in non-secure mode
such as on OMAP, we must avoid writing the register when resuming in
non-secure mode. Avoid this by moving the responsibility to the
SoC code in this case to ensure that the auxiliary control register
is restored before cpu_resume() is called.
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
--
arch/arm/mm/proc-v7.S | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 3c38678..fa1e6d5 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -237,7 +237,9 @@ ENTRY(cpu_v7_do_resume)
mcr p15, 0, r7, c2, c0, 0 @ TTB 0
mcr p15, 0, r8, c2, c0, 1 @ TTB 1
mcr p15, 0, ip, c2, c0, 2 @ TTB control register
- mcr p15, 0, r10, c1, c0, 1 @ Auxiliary control register
+ mrc p15, 0, r4, c1, c0, 1 @ Read auxiliary control register
+ teq r4, r10
+ mcrne p15, 0, r10, c1, c0, 1 @ Auxiliary control register
mcr p15, 0, r11, c1, c0, 2 @ Co-processor access control
ldr r4, =PRRR @ PRRR
ldr r5, =NMRR @ NMRR
^ permalink raw reply related [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 17:12 ` [linux-pm] " Russell King - ARM Linux
` (3 preceding siblings ...)
2011-06-09 17:53 ` Russell King - ARM Linux
@ 2011-06-10 12:22 ` Frank Hofmann
2011-06-10 13:43 ` Russell King - ARM Linux
2011-06-10 13:43 ` Russell King - ARM Linux
2011-06-10 12:22 ` Frank Hofmann
5 siblings, 2 replies; 49+ messages in thread
From: Frank Hofmann @ 2011-06-10 12:22 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 9 Jun 2011, Russell King - ARM Linux wrote:
> On Thu, Jun 09, 2011 at 10:23:07PM +0530, Santosh Shilimkar wrote:
[ ... ]
>> 3. Avoid direct write to AUXCTRL in generic suspend code.
>
> This is the only problematical one that I can see. We need to restore
> this on systems running in secure mode. What we could do is rather than
> writing to the register, read it first and compare its value with what
> was saved to see whether we need to write it.
>
> Then, if platforms run in non-secure mode, they are responsible for
> restoring that register back to its pre-suspend value before their
> assembly calls cpu_resume().
While this is ok from the point of view of having cpu_suspend / resume
being service functions for the platform-specific idle/off-mode code, it
also illustrates the difficulty this creates for the hibernation code.
If it's not possible to call cpu_suspend / cpu_resume (or something like
it - not tied to names ...) as a full-featured generic interface, then
creating a true snapshot capability becomes problematic.
>
>> 4. Before MMU is enabled in resume a callback to restore
>> secure register, setup auxctrl etc.
>
> You can do this before your assembly calls cpu_resume().
Only that it's known at the point of call to cpu_suspend/resume.
Again, I admit to being biased regarding the usecase here ...
See below.
>
>> Additionally the L2 cache handling isn't part of
>> these common suspend hooks.
>
> L2 cache handling can't fit into the generic code - it doesn't really
> belong there either. It needs to be in the parent or hooked into the
> syscore_ops stuff as I've said previously.
For the "take things down" side, dode like machine_restart() already is
able to flush/inval/disable all these things on the way down, without any
SoC-specific knowledge.
OMAP suspend/resume has, just recently, gone half-way there (use the
provided flush/inval instead of the home-grown table walker code),
Agree with that. Cache flushing / disabling / invalidation has interfaces
(the outer_*, l2* and cache ops) already, and e.g. the OMAP code has
recently started to use some of those (kernel_flush instead of the
home-grown inval loop). Looks like that part is on its way.
On the resume side, there's actually a problem here - the generic way of
enabling a cache is through initcalls - which don't happen on resume, so
if the system comes out of a "low enough" state there's some work to do
here - which generic cpu_resume() does not do.
>
> So:
>
> ENTRY(my_soc_suspend)
> stmfd sp!, {r4 - r12, lr}
> ldr r3, =resume
> bl cpu_suspend
> /*
> * Insert whatever code is required here for suspend
> * eg, save secure mode, then jump to sram to call WFI function
> */
> resume:
> ldmfd sp!, {r4 - r12, pc}
> ENDPROC(my_soc_suspend)
>
> ENTRY(my_soc_resume)
> /*
> * Insert whatever other code is required to be run before resume
> * eg, WFI function returns to this symbol after DDR becomes
> * accessible. restore secure mode state
> */
> b cpu_resume
> ENDPROC(my_soc_resume)
That alone doesn't accommodate the following situations:
a) there might be pre-suspend / post-resume activities necessary, i.e.
the assumption that any SoC-specific "go down" activity can be done
after cpu_suspend() and any SoC-specific "bring up" activity before
cpu_resume() might not be sufficient.
Case in point: Reenabling L2 caches after resume.
b) my bias - snapshotting state (for hibernation).
Delegating this to a SoC-specific method risks creating code like
the OMAP stuff - where state saving, power management, off mode
and whatnot is all interwoven and interdependent.
It also creates the problem that _generic_ (platform-independent)
hibernation code becomes impossible to do ...
A clean thing are separate steps:
<mach preparation for suspend state snapshot>
<generic state snapshot>
<mach state snapshot>
...
<wfi> /* or not ... */
...
<mach prep for resume / basic initialization>
<generic resume>
<mach resume>
So why not have those hooks _inside_ cpu_suspend / cpu_resume, i.e. like:
.data
ENTRY(cpu_suspend)
mov r9, lr
#ifdef CONFIG_ARCH_NEEDS_SPAGHETTI_SUSPEND
mov lr, pc
ldr pc, mach_pre_suspend_hook
#endif
...
#ifdef CONFIG_ARCH_NEEDS_SPAGHETTI_SUSPEND
mov lr, r9
ldr r4, mach_post_suspend_hook
b r4
#else
mov pc, r9
END(cpu_suspend)
#ifdef CONFIG_ARCH_NEEDS_SPAGHETTI_SUSPEND
mach_pre_suspend_hook:
.long 0
mach_post_suspend_hook:
.long 0
#endif
and let the SoC initialization set them if it so desires ?
This would allow to use them for snapshotting the state as well.
Key point, again, from the hibernation bias, is really to have that
stuff _separate_ from power-down / wfi / whatever-to-enter-lowpower-mode.
As many of these activities as possible should be dealt with by sysdev /
syscore, agreed; but unfortunately there might be certain things,
especially around secure state, that are too closely tied in to delegate
it to those ?
>
> What makes it far more complicated in the OMAP case is all that "is l1 state
> lost? is l2 state lost?" stuff.
>
It looks like it's structured this way:
omap_cpu_suspend()
{
switch (state) {
case 3:
case 1:
/* save context */
case 2:
/* clean caches */
case 0:
wfi();
}
}
omap_cpu_resume() /* from OFF, case 2 / 0 never happens */
{
if (state == 3)
/* disable / inval L2 */
/* restore context */
/* reenable L2 */
}
But the code doesn't perfectly match the comments in it.
FrankH.
^ permalink raw reply [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-10 12:22 ` [linux-pm] " Frank Hofmann
@ 2011-06-10 13:43 ` Russell King - ARM Linux
2011-06-10 13:47 ` Frank Hofmann
2011-06-10 13:47 ` Frank Hofmann
2011-06-10 13:43 ` Russell King - ARM Linux
1 sibling, 2 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-10 13:43 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jun 10, 2011 at 01:22:24PM +0100, Frank Hofmann wrote:
> On Thu, 9 Jun 2011, Russell King - ARM Linux wrote:
>
>> On Thu, Jun 09, 2011 at 10:23:07PM +0530, Santosh Shilimkar wrote:
> [ ... ]
>>> 3. Avoid direct write to AUXCTRL in generic suspend code.
>>
>> This is the only problematical one that I can see. We need to restore
>> this on systems running in secure mode. What we could do is rather than
>> writing to the register, read it first and compare its value with what
>> was saved to see whether we need to write it.
>>
>> Then, if platforms run in non-secure mode, they are responsible for
>> restoring that register back to its pre-suspend value before their
>> assembly calls cpu_resume().
>
> While this is ok from the point of view of having cpu_suspend / resume
> being service functions for the platform-specific idle/off-mode code, it
> also illustrates the difficulty this creates for the hibernation code.
>
> If it's not possible to call cpu_suspend / cpu_resume (or something like
> it - not tied to names ...) as a full-featured generic interface, then
> creating a true snapshot capability becomes problematic.
It is not intended to be a full-featured interface. It is designed to
be an interface to handle the CPU specific part of the suspend/resume
only.
And I use the term CPU as strictly defined not as most people lazily do
to define the entire SoC. I mean the core processor itself.
A generic interface can not handle the issues of secure mode when there
is no defined secure mode API. It can't handle the L2 cache crap because
that's outside the scope of the CPU and is platform dependent. It can't
handle devices because that's again outside the scope of the CPU and is
SoC dependent.
The only thing it can do - and should do - is deal with the CPU specific
part. That's why it has a cpu_ prefix.
>>> 4. Before MMU is enabled in resume a callback to restore
>>> secure register, setup auxctrl etc.
>>
>> You can do this before your assembly calls cpu_resume().
>
> Only that it's known at the point of call to cpu_suspend/resume.
>
> Again, I admit to being biased regarding the usecase here ...
How can generic CPU code know about all the platform idiotic farces that
people pull? No, what you're asking for is total madness.
>>> Additionally the L2 cache handling isn't part of
>>> these common suspend hooks.
>>
>> L2 cache handling can't fit into the generic code - it doesn't really
>> belong there either. It needs to be in the parent or hooked into the
>> syscore_ops stuff as I've said previously.
>
> For the "take things down" side, dode like machine_restart() already is
> able to flush/inval/disable all these things on the way down, without any
> SoC-specific knowledge.
>
> OMAP suspend/resume has, just recently, gone half-way there (use the
> provided flush/inval instead of the home-grown table walker code),
>
> Agree with that. Cache flushing / disabling / invalidation has interfaces
> (the outer_*, l2* and cache ops) already, and e.g. the OMAP code has
> recently started to use some of those (kernel_flush instead of the
> home-grown inval loop). Looks like that part is on its way.
>
> On the resume side, there's actually a problem here - the generic way of
> enabling a cache is through initcalls - which don't happen on resume, so
> if the system comes out of a "low enough" state there's some work to do
> here - which generic cpu_resume() does not do.
And can't do.
>>
>> So:
>>
>> ENTRY(my_soc_suspend)
>> stmfd sp!, {r4 - r12, lr}
>> ldr r3, =resume
>> bl cpu_suspend
>> /*
>> * Insert whatever code is required here for suspend
>> * eg, save secure mode, then jump to sram to call WFI function
>> */
>> resume:
>> ldmfd sp!, {r4 - r12, pc}
>> ENDPROC(my_soc_suspend)
>>
>> ENTRY(my_soc_resume)
>> /*
>> * Insert whatever other code is required to be run before resume
>> * eg, WFI function returns to this symbol after DDR becomes
>> * accessible. restore secure mode state
>> */
>> b cpu_resume
>> ENDPROC(my_soc_resume)
>
> That alone doesn't accommodate the following situations:
>
> a) there might be pre-suspend / post-resume activities necessary, i.e.
> the assumption that any SoC-specific "go down" activity can be done
> after cpu_suspend() and any SoC-specific "bring up" activity before
> cpu_resume() might not be sufficient.
> Case in point: Reenabling L2 caches after resume.
Look, platform code calls cpu_suspend() as part of whatever is required
to do the suspend work. It deals with the core CPU crap, nothing more.
Platforms have to take care to deal with whatever shite they have before
the CPU core crap is handled, and then do whatever shite they need to
do after the CPU core crap has been handled.
You can't get away from that. You can't go stuffing L2 shite into the
middle of this.
> b) my bias - snapshotting state (for hibernation).
> Delegating this to a SoC-specific method risks creating code like
> the OMAP stuff - where state saving, power management, off mode
> and whatnot is all interwoven and interdependent.
> It also creates the problem that _generic_ (platform-independent)
> hibernation code becomes impossible to do ...
>
> A clean thing are separate steps:
>
> <mach preparation for suspend state snapshot>
> <generic state snapshot>
> <mach state snapshot>
> ...
> <wfi> /* or not ... */
> ...
> <mach prep for resume / basic initialization>
> <generic resume>
> <mach resume>
>
> So why not have those hooks _inside_ cpu_suspend / cpu_resume, i.e. like:
What's the point when platform code has *ALREADY* to call these functions?
Is it really too sodding difficult for platforms to do:
my_suspend_hook()
{
mach_pre_suspend_hook();
cpu_suspend();
mach_post_suspend_hook();
}
?
<not read the rest of the message, this is getting idiotic>
^ permalink raw reply [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-10 13:43 ` Russell King - ARM Linux
@ 2011-06-10 13:47 ` Frank Hofmann
2011-06-10 14:02 ` Russell King - ARM Linux
2011-06-10 14:02 ` [linux-pm] " Russell King - ARM Linux
2011-06-10 13:47 ` Frank Hofmann
1 sibling, 2 replies; 49+ messages in thread
From: Frank Hofmann @ 2011-06-10 13:47 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 10 Jun 2011, Russell King - ARM Linux wrote:
[ ... ]
> What's the point when platform code has *ALREADY* to call these functions?
>
> Is it really too sodding difficult for platforms to do:
>
> my_suspend_hook()
> {
> mach_pre_suspend_hook();
> cpu_suspend();
> mach_post_suspend_hook();
> }
>
> ?
>
> <not read the rest of the message, this is getting idiotic>
>
Sorry for being unclear.
Yes, they already have to do all this. And they should.
Except for one thing:
They all, IN ADDITION, do ALSO:
wfi(); /* or whatever else to power down */
For Hibernation, _THAT_ needs to be out of the codepath.
So that one can snapshot without powering down.
FrankH.
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-10 13:47 ` Frank Hofmann
@ 2011-06-10 14:02 ` Russell King - ARM Linux
2011-06-10 14:02 ` [linux-pm] " Russell King - ARM Linux
1 sibling, 0 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-10 14:02 UTC (permalink / raw)
To: Frank Hofmann; +Cc: linux-pm, tuxonice-devel, linux-arm-kernel
On Fri, Jun 10, 2011 at 02:47:30PM +0100, Frank Hofmann wrote:
> On Fri, 10 Jun 2011, Russell King - ARM Linux wrote:
>
> [ ... ]
>> What's the point when platform code has *ALREADY* to call these functions?
>>
>> Is it really too sodding difficult for platforms to do:
>>
>> my_suspend_hook()
>> {
>> mach_pre_suspend_hook();
>> cpu_suspend();
>> mach_post_suspend_hook();
>> }
>>
>> ?
>>
>> <not read the rest of the message, this is getting idiotic>
>>
>
> Sorry for being unclear.
>
> Yes, they already have to do all this. And they should.
>
>
> Except for one thing:
>
> They all, IN ADDITION, do ALSO:
>
> wfi(); /* or whatever else to power down */
>
>
> For Hibernation, _THAT_ needs to be out of the codepath.
>
>
> So that one can snapshot without powering down.
I think there's a fundamental problem here - what's required for S2RAM
is not what's required for hibernate. After cpu_suspend() has done
its job, you are in a _very_ specific environment designed for the last
stages of S2RAM _only_ and not hibernate.
In order to use cpu_suspend() for hibernate, it requires a completely
different path entirely, and there's no getting away from that.
You can see that when you analyze the differences between S2RAM and
hibernate, when you realize that the final part of the S2RAM process
(which happens after cpu_suspend() returns) on many SoCs is dealing
with putting SDRAM into self-refresh mode before writing some kind of
power mode register to tell the power supply to kill power to most
of the platform. That is all _very_ SoC specific.
Also realize that the code which executes after cpu_suspend() returns
is _not_ running in the same context as the code which called
cpu_suspend() - cpu_suspend() has modified the stack pointer to store
the CPU specific state and that is not the same stack pointer as was
the case before cpu_suspend() was called.
You don't want to run any of that code when you're dealing with hibernate,
so expecting to be able to reuse these S2RAM paths is not realistic.
What we could do is provide a cpu_hibernate() function which has saner
semantics for saving the CPU specific state for hibernate.
^ permalink raw reply [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-10 13:47 ` Frank Hofmann
2011-06-10 14:02 ` Russell King - ARM Linux
@ 2011-06-10 14:02 ` Russell King - ARM Linux
2011-06-10 14:54 ` Frank Hofmann
2011-06-10 14:54 ` [linux-pm] " Frank Hofmann
1 sibling, 2 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-10 14:02 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jun 10, 2011 at 02:47:30PM +0100, Frank Hofmann wrote:
> On Fri, 10 Jun 2011, Russell King - ARM Linux wrote:
>
> [ ... ]
>> What's the point when platform code has *ALREADY* to call these functions?
>>
>> Is it really too sodding difficult for platforms to do:
>>
>> my_suspend_hook()
>> {
>> mach_pre_suspend_hook();
>> cpu_suspend();
>> mach_post_suspend_hook();
>> }
>>
>> ?
>>
>> <not read the rest of the message, this is getting idiotic>
>>
>
> Sorry for being unclear.
>
> Yes, they already have to do all this. And they should.
>
>
> Except for one thing:
>
> They all, IN ADDITION, do ALSO:
>
> wfi(); /* or whatever else to power down */
>
>
> For Hibernation, _THAT_ needs to be out of the codepath.
>
>
> So that one can snapshot without powering down.
I think there's a fundamental problem here - what's required for S2RAM
is not what's required for hibernate. After cpu_suspend() has done
its job, you are in a _very_ specific environment designed for the last
stages of S2RAM _only_ and not hibernate.
In order to use cpu_suspend() for hibernate, it requires a completely
different path entirely, and there's no getting away from that.
You can see that when you analyze the differences between S2RAM and
hibernate, when you realize that the final part of the S2RAM process
(which happens after cpu_suspend() returns) on many SoCs is dealing
with putting SDRAM into self-refresh mode before writing some kind of
power mode register to tell the power supply to kill power to most
of the platform. That is all _very_ SoC specific.
Also realize that the code which executes after cpu_suspend() returns
is _not_ running in the same context as the code which called
cpu_suspend() - cpu_suspend() has modified the stack pointer to store
the CPU specific state and that is not the same stack pointer as was
the case before cpu_suspend() was called.
You don't want to run any of that code when you're dealing with hibernate,
so expecting to be able to reuse these S2RAM paths is not realistic.
What we could do is provide a cpu_hibernate() function which has saner
semantics for saving the CPU specific state for hibernate.
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-10 14:02 ` [linux-pm] " Russell King - ARM Linux
@ 2011-06-10 14:54 ` Frank Hofmann
2011-06-10 14:54 ` [linux-pm] " Frank Hofmann
1 sibling, 0 replies; 49+ messages in thread
From: Frank Hofmann @ 2011-06-10 14:54 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: tuxonice-devel, Frank Hofmann, linux-pm, linux-arm-kernel
On Fri, 10 Jun 2011, Russell King - ARM Linux wrote:
>> [ ... ]
> I think there's a fundamental problem here - what's required for S2RAM
> is not what's required for hibernate. After cpu_suspend() has done
> its job, you are in a _very_ specific environment designed for the last
> stages of S2RAM _only_ and not hibernate.
>
> In order to use cpu_suspend() for hibernate, it requires a completely
> different path entirely, and there's no getting away from that.
>
> You can see that when you analyze the differences between S2RAM and
> hibernate, when you realize that the final part of the S2RAM process
> (which happens after cpu_suspend() returns) on many SoCs is dealing
> with putting SDRAM into self-refresh mode before writing some kind of
> power mode register to tell the power supply to kill power to most
> of the platform. That is all _very_ SoC specific.
Yes, that's what I'm trying to say - the _final_ stage, for s2ram, sends
the SoC to low-power.
Up until there, we do the same for hibernation, don't we ? Where exactly
is it different ?
>
> Also realize that the code which executes after cpu_suspend() returns
> is _not_ running in the same context as the code which called
> cpu_suspend() - cpu_suspend() has modified the stack pointer to store
> the CPU specific state and that is not the same stack pointer as was
> the case before cpu_suspend() was called.
Yes, the function isn't "well behaved" from the ABI point of view because
it doesn't preserve registers (including the stack), but that can be
accommodated by the caller.
The current s2ram callers have to accommodate that as well. Which is
ultimately easy for them - since poweroff doesn't care.
The only reason why hibernation / swsusp_arch_suspend() is different there
is because the activity _after_ cpu_suspend() is extensive and _can fail_
(saving the image); on that failure, one would prefer to see an error
message and continue instead of panicing the system. So the stack change
you mention needs to be addressed, swsusp_arch_suspend() must be a
well-behaved function from the ABI point of view.
Normally, if all goes _right_, swsusp_save() does not return either. It
ends powering the system off. If one were willing to die without message
on failure to save the snapshot to disk, and would be willing to block
cpu_suspend during writing the snapshot (to guarantee sleep_save_sp isn't
changing) one wouldn't need to care about the stack and could simply:
ENTRY(swsusp_arch_suspend)
mrs r1, cpsr
mrs r2, spsr
stmfd sp! {r1-r12,lr}
bl __swsusp_arch_get_vpoffset
mov r1, r0
adr r3, .Lresume_post_mmu
bl cpu_suspend
bl swsusp_save
0:
b 0b @ should never reach this
ENDPROC(swsusp_arch_suspend)
Resume is quite trivial either way:
ENTRY(swsusp_arch_resume)
setmode PSR_I_BIT | PSR_F_BIT | SVC_MODE, r2
ldr sp, =(__swsusp_resume_stk + PAGE_SIZE / 2)
/*
* replays image, and ends in cpu_reset(cpu_resume)
*/
b __swsusp_arch_restore_image
.Lresume_post_mmu:
ldmfd sp!, {r1-r12}
msr cpsr, r1
msr spsr, r2
bl cpu_init @ reinitialize other modes
ldmfd sp!, {lr}
b __swsusp_arch_resume_finish @ cleanup
ENDPROC(swsusp_arch_resume)
>
> You don't want to run any of that code when you're dealing with hibernate,
> so expecting to be able to reuse these S2RAM paths is not realistic.
Hmm, well ... in the end, hibernation does:
<snapshot state>
<some long operation that writes the image out>
<poweroff>
while s2ram does:
<snapshot state>
<some quick operation setting low power modes>
<poweroff>
>
> What we could do is provide a cpu_hibernate() function which has saner
> semantics for saving the CPU specific state for hibernate.
Yes, that's exactly what I'm hoping for. From my point of view, this
would, though, end up in:
cpu_soc_suspend:
cpu_hibernate_snapshot_state();
/* S2RAM codepath to send soc to low power */
cpu_soc_resume:
/* S2RAM codepath for waking up soc essentials */
cpu_hibernate_restore_state();
At least I can't come up with a really good reason why the state
snapshotting operation would have to be different between s2ram and
s2disk.
FrankH.
^ permalink raw reply [flat|nested] 49+ messages in thread* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-10 14:02 ` [linux-pm] " Russell King - ARM Linux
2011-06-10 14:54 ` Frank Hofmann
@ 2011-06-10 14:54 ` Frank Hofmann
1 sibling, 0 replies; 49+ messages in thread
From: Frank Hofmann @ 2011-06-10 14:54 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 10 Jun 2011, Russell King - ARM Linux wrote:
>> [ ... ]
> I think there's a fundamental problem here - what's required for S2RAM
> is not what's required for hibernate. After cpu_suspend() has done
> its job, you are in a _very_ specific environment designed for the last
> stages of S2RAM _only_ and not hibernate.
>
> In order to use cpu_suspend() for hibernate, it requires a completely
> different path entirely, and there's no getting away from that.
>
> You can see that when you analyze the differences between S2RAM and
> hibernate, when you realize that the final part of the S2RAM process
> (which happens after cpu_suspend() returns) on many SoCs is dealing
> with putting SDRAM into self-refresh mode before writing some kind of
> power mode register to tell the power supply to kill power to most
> of the platform. That is all _very_ SoC specific.
Yes, that's what I'm trying to say - the _final_ stage, for s2ram, sends
the SoC to low-power.
Up until there, we do the same for hibernation, don't we ? Where exactly
is it different ?
>
> Also realize that the code which executes after cpu_suspend() returns
> is _not_ running in the same context as the code which called
> cpu_suspend() - cpu_suspend() has modified the stack pointer to store
> the CPU specific state and that is not the same stack pointer as was
> the case before cpu_suspend() was called.
Yes, the function isn't "well behaved" from the ABI point of view because
it doesn't preserve registers (including the stack), but that can be
accommodated by the caller.
The current s2ram callers have to accommodate that as well. Which is
ultimately easy for them - since poweroff doesn't care.
The only reason why hibernation / swsusp_arch_suspend() is different there
is because the activity _after_ cpu_suspend() is extensive and _can fail_
(saving the image); on that failure, one would prefer to see an error
message and continue instead of panicing the system. So the stack change
you mention needs to be addressed, swsusp_arch_suspend() must be a
well-behaved function from the ABI point of view.
Normally, if all goes _right_, swsusp_save() does not return either. It
ends powering the system off. If one were willing to die without message
on failure to save the snapshot to disk, and would be willing to block
cpu_suspend during writing the snapshot (to guarantee sleep_save_sp isn't
changing) one wouldn't need to care about the stack and could simply:
ENTRY(swsusp_arch_suspend)
mrs r1, cpsr
mrs r2, spsr
stmfd sp! {r1-r12,lr}
bl __swsusp_arch_get_vpoffset
mov r1, r0
adr r3, .Lresume_post_mmu
bl cpu_suspend
bl swsusp_save
0:
b 0b @ should never reach this
ENDPROC(swsusp_arch_suspend)
Resume is quite trivial either way:
ENTRY(swsusp_arch_resume)
setmode PSR_I_BIT | PSR_F_BIT | SVC_MODE, r2
ldr sp, =(__swsusp_resume_stk + PAGE_SIZE / 2)
/*
* replays image, and ends in cpu_reset(cpu_resume)
*/
b __swsusp_arch_restore_image
.Lresume_post_mmu:
ldmfd sp!, {r1-r12}
msr cpsr, r1
msr spsr, r2
bl cpu_init @ reinitialize other modes
ldmfd sp!, {lr}
b __swsusp_arch_resume_finish @ cleanup
ENDPROC(swsusp_arch_resume)
>
> You don't want to run any of that code when you're dealing with hibernate,
> so expecting to be able to reuse these S2RAM paths is not realistic.
Hmm, well ... in the end, hibernation does:
<snapshot state>
<some long operation that writes the image out>
<poweroff>
while s2ram does:
<snapshot state>
<some quick operation setting low power modes>
<poweroff>
>
> What we could do is provide a cpu_hibernate() function which has saner
> semantics for saving the CPU specific state for hibernate.
Yes, that's exactly what I'm hoping for. From my point of view, this
would, though, end up in:
cpu_soc_suspend:
cpu_hibernate_snapshot_state();
/* S2RAM codepath to send soc to low power */
cpu_soc_resume:
/* S2RAM codepath for waking up soc essentials */
cpu_hibernate_restore_state();
At least I can't come up with a really good reason why the state
snapshotting operation would have to be different between s2ram and
s2disk.
FrankH.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-10 13:43 ` Russell King - ARM Linux
2011-06-10 13:47 ` Frank Hofmann
@ 2011-06-10 13:47 ` Frank Hofmann
1 sibling, 0 replies; 49+ messages in thread
From: Frank Hofmann @ 2011-06-10 13:47 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: tuxonice-devel, Frank Hofmann, linux-pm, linux-arm-kernel
On Fri, 10 Jun 2011, Russell King - ARM Linux wrote:
[ ... ]
> What's the point when platform code has *ALREADY* to call these functions?
>
> Is it really too sodding difficult for platforms to do:
>
> my_suspend_hook()
> {
> mach_pre_suspend_hook();
> cpu_suspend();
> mach_post_suspend_hook();
> }
>
> ?
>
> <not read the rest of the message, this is getting idiotic>
>
Sorry for being unclear.
Yes, they already have to do all this. And they should.
Except for one thing:
They all, IN ADDITION, do ALSO:
wfi(); /* or whatever else to power down */
For Hibernation, _THAT_ needs to be out of the codepath.
So that one can snapshot without powering down.
FrankH.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-10 12:22 ` [linux-pm] " Frank Hofmann
2011-06-10 13:43 ` Russell King - ARM Linux
@ 2011-06-10 13:43 ` Russell King - ARM Linux
1 sibling, 0 replies; 49+ messages in thread
From: Russell King - ARM Linux @ 2011-06-10 13:43 UTC (permalink / raw)
To: Frank Hofmann; +Cc: linux-pm, tuxonice-devel, linux-arm-kernel
On Fri, Jun 10, 2011 at 01:22:24PM +0100, Frank Hofmann wrote:
> On Thu, 9 Jun 2011, Russell King - ARM Linux wrote:
>
>> On Thu, Jun 09, 2011 at 10:23:07PM +0530, Santosh Shilimkar wrote:
> [ ... ]
>>> 3. Avoid direct write to AUXCTRL in generic suspend code.
>>
>> This is the only problematical one that I can see. We need to restore
>> this on systems running in secure mode. What we could do is rather than
>> writing to the register, read it first and compare its value with what
>> was saved to see whether we need to write it.
>>
>> Then, if platforms run in non-secure mode, they are responsible for
>> restoring that register back to its pre-suspend value before their
>> assembly calls cpu_resume().
>
> While this is ok from the point of view of having cpu_suspend / resume
> being service functions for the platform-specific idle/off-mode code, it
> also illustrates the difficulty this creates for the hibernation code.
>
> If it's not possible to call cpu_suspend / cpu_resume (or something like
> it - not tied to names ...) as a full-featured generic interface, then
> creating a true snapshot capability becomes problematic.
It is not intended to be a full-featured interface. It is designed to
be an interface to handle the CPU specific part of the suspend/resume
only.
And I use the term CPU as strictly defined not as most people lazily do
to define the entire SoC. I mean the core processor itself.
A generic interface can not handle the issues of secure mode when there
is no defined secure mode API. It can't handle the L2 cache crap because
that's outside the scope of the CPU and is platform dependent. It can't
handle devices because that's again outside the scope of the CPU and is
SoC dependent.
The only thing it can do - and should do - is deal with the CPU specific
part. That's why it has a cpu_ prefix.
>>> 4. Before MMU is enabled in resume a callback to restore
>>> secure register, setup auxctrl etc.
>>
>> You can do this before your assembly calls cpu_resume().
>
> Only that it's known at the point of call to cpu_suspend/resume.
>
> Again, I admit to being biased regarding the usecase here ...
How can generic CPU code know about all the platform idiotic farces that
people pull? No, what you're asking for is total madness.
>>> Additionally the L2 cache handling isn't part of
>>> these common suspend hooks.
>>
>> L2 cache handling can't fit into the generic code - it doesn't really
>> belong there either. It needs to be in the parent or hooked into the
>> syscore_ops stuff as I've said previously.
>
> For the "take things down" side, dode like machine_restart() already is
> able to flush/inval/disable all these things on the way down, without any
> SoC-specific knowledge.
>
> OMAP suspend/resume has, just recently, gone half-way there (use the
> provided flush/inval instead of the home-grown table walker code),
>
> Agree with that. Cache flushing / disabling / invalidation has interfaces
> (the outer_*, l2* and cache ops) already, and e.g. the OMAP code has
> recently started to use some of those (kernel_flush instead of the
> home-grown inval loop). Looks like that part is on its way.
>
> On the resume side, there's actually a problem here - the generic way of
> enabling a cache is through initcalls - which don't happen on resume, so
> if the system comes out of a "low enough" state there's some work to do
> here - which generic cpu_resume() does not do.
And can't do.
>>
>> So:
>>
>> ENTRY(my_soc_suspend)
>> stmfd sp!, {r4 - r12, lr}
>> ldr r3, =resume
>> bl cpu_suspend
>> /*
>> * Insert whatever code is required here for suspend
>> * eg, save secure mode, then jump to sram to call WFI function
>> */
>> resume:
>> ldmfd sp!, {r4 - r12, pc}
>> ENDPROC(my_soc_suspend)
>>
>> ENTRY(my_soc_resume)
>> /*
>> * Insert whatever other code is required to be run before resume
>> * eg, WFI function returns to this symbol after DDR becomes
>> * accessible. restore secure mode state
>> */
>> b cpu_resume
>> ENDPROC(my_soc_resume)
>
> That alone doesn't accommodate the following situations:
>
> a) there might be pre-suspend / post-resume activities necessary, i.e.
> the assumption that any SoC-specific "go down" activity can be done
> after cpu_suspend() and any SoC-specific "bring up" activity before
> cpu_resume() might not be sufficient.
> Case in point: Reenabling L2 caches after resume.
Look, platform code calls cpu_suspend() as part of whatever is required
to do the suspend work. It deals with the core CPU crap, nothing more.
Platforms have to take care to deal with whatever shite they have before
the CPU core crap is handled, and then do whatever shite they need to
do after the CPU core crap has been handled.
You can't get away from that. You can't go stuffing L2 shite into the
middle of this.
> b) my bias - snapshotting state (for hibernation).
> Delegating this to a SoC-specific method risks creating code like
> the OMAP stuff - where state saving, power management, off mode
> and whatnot is all interwoven and interdependent.
> It also creates the problem that _generic_ (platform-independent)
> hibernation code becomes impossible to do ...
>
> A clean thing are separate steps:
>
> <mach preparation for suspend state snapshot>
> <generic state snapshot>
> <mach state snapshot>
> ...
> <wfi> /* or not ... */
> ...
> <mach prep for resume / basic initialization>
> <generic resume>
> <mach resume>
>
> So why not have those hooks _inside_ cpu_suspend / cpu_resume, i.e. like:
What's the point when platform code has *ALREADY* to call these functions?
Is it really too sodding difficult for platforms to do:
my_suspend_hook()
{
mach_pre_suspend_hook();
cpu_suspend();
mach_post_suspend_hook();
}
?
<not read the rest of the message, this is getting idiotic>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 17:12 ` [linux-pm] " Russell King - ARM Linux
` (4 preceding siblings ...)
2011-06-10 12:22 ` [linux-pm] " Frank Hofmann
@ 2011-06-10 12:22 ` Frank Hofmann
5 siblings, 0 replies; 49+ messages in thread
From: Frank Hofmann @ 2011-06-10 12:22 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: tuxonice-devel, Frank Hofmann, linux-pm, linux-arm-kernel
On Thu, 9 Jun 2011, Russell King - ARM Linux wrote:
> On Thu, Jun 09, 2011 at 10:23:07PM +0530, Santosh Shilimkar wrote:
[ ... ]
>> 3. Avoid direct write to AUXCTRL in generic suspend code.
>
> This is the only problematical one that I can see. We need to restore
> this on systems running in secure mode. What we could do is rather than
> writing to the register, read it first and compare its value with what
> was saved to see whether we need to write it.
>
> Then, if platforms run in non-secure mode, they are responsible for
> restoring that register back to its pre-suspend value before their
> assembly calls cpu_resume().
While this is ok from the point of view of having cpu_suspend / resume
being service functions for the platform-specific idle/off-mode code, it
also illustrates the difficulty this creates for the hibernation code.
If it's not possible to call cpu_suspend / cpu_resume (or something like
it - not tied to names ...) as a full-featured generic interface, then
creating a true snapshot capability becomes problematic.
>
>> 4. Before MMU is enabled in resume a callback to restore
>> secure register, setup auxctrl etc.
>
> You can do this before your assembly calls cpu_resume().
Only that it's known at the point of call to cpu_suspend/resume.
Again, I admit to being biased regarding the usecase here ...
See below.
>
>> Additionally the L2 cache handling isn't part of
>> these common suspend hooks.
>
> L2 cache handling can't fit into the generic code - it doesn't really
> belong there either. It needs to be in the parent or hooked into the
> syscore_ops stuff as I've said previously.
For the "take things down" side, dode like machine_restart() already is
able to flush/inval/disable all these things on the way down, without any
SoC-specific knowledge.
OMAP suspend/resume has, just recently, gone half-way there (use the
provided flush/inval instead of the home-grown table walker code),
Agree with that. Cache flushing / disabling / invalidation has interfaces
(the outer_*, l2* and cache ops) already, and e.g. the OMAP code has
recently started to use some of those (kernel_flush instead of the
home-grown inval loop). Looks like that part is on its way.
On the resume side, there's actually a problem here - the generic way of
enabling a cache is through initcalls - which don't happen on resume, so
if the system comes out of a "low enough" state there's some work to do
here - which generic cpu_resume() does not do.
>
> So:
>
> ENTRY(my_soc_suspend)
> stmfd sp!, {r4 - r12, lr}
> ldr r3, =resume
> bl cpu_suspend
> /*
> * Insert whatever code is required here for suspend
> * eg, save secure mode, then jump to sram to call WFI function
> */
> resume:
> ldmfd sp!, {r4 - r12, pc}
> ENDPROC(my_soc_suspend)
>
> ENTRY(my_soc_resume)
> /*
> * Insert whatever other code is required to be run before resume
> * eg, WFI function returns to this symbol after DDR becomes
> * accessible. restore secure mode state
> */
> b cpu_resume
> ENDPROC(my_soc_resume)
That alone doesn't accommodate the following situations:
a) there might be pre-suspend / post-resume activities necessary, i.e.
the assumption that any SoC-specific "go down" activity can be done
after cpu_suspend() and any SoC-specific "bring up" activity before
cpu_resume() might not be sufficient.
Case in point: Reenabling L2 caches after resume.
b) my bias - snapshotting state (for hibernation).
Delegating this to a SoC-specific method risks creating code like
the OMAP stuff - where state saving, power management, off mode
and whatnot is all interwoven and interdependent.
It also creates the problem that _generic_ (platform-independent)
hibernation code becomes impossible to do ...
A clean thing are separate steps:
<mach preparation for suspend state snapshot>
<generic state snapshot>
<mach state snapshot>
...
<wfi> /* or not ... */
...
<mach prep for resume / basic initialization>
<generic resume>
<mach resume>
So why not have those hooks _inside_ cpu_suspend / cpu_resume, i.e. like:
.data
ENTRY(cpu_suspend)
mov r9, lr
#ifdef CONFIG_ARCH_NEEDS_SPAGHETTI_SUSPEND
mov lr, pc
ldr pc, mach_pre_suspend_hook
#endif
...
#ifdef CONFIG_ARCH_NEEDS_SPAGHETTI_SUSPEND
mov lr, r9
ldr r4, mach_post_suspend_hook
b r4
#else
mov pc, r9
END(cpu_suspend)
#ifdef CONFIG_ARCH_NEEDS_SPAGHETTI_SUSPEND
mach_pre_suspend_hook:
.long 0
mach_post_suspend_hook:
.long 0
#endif
and let the SoC initialization set them if it so desires ?
This would allow to use them for snapshotting the state as well.
Key point, again, from the hibernation bias, is really to have that
stuff _separate_ from power-down / wfi / whatever-to-enter-lowpower-mode.
As many of these activities as possible should be dealt with by sysdev /
syscore, agreed; but unfortunately there might be certain things,
especially around secure state, that are too closely tied in to delegate
it to those ?
>
> What makes it far more complicated in the OMAP case is all that "is l1 state
> lost? is l2 state lost?" stuff.
>
It looks like it's structured this way:
omap_cpu_suspend()
{
switch (state) {
case 3:
case 1:
/* save context */
case 2:
/* clean caches */
case 0:
wfi();
}
}
omap_cpu_resume() /* from OFF, case 2 / 0 never happens */
{
if (state == 3)
/* disable / inval L2 */
/* restore context */
/* reenable L2 */
}
But the code doesn't perfectly match the comments in it.
FrankH.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:40 ` [linux-pm] " Russell King - ARM Linux
2011-06-09 16:53 ` Santosh Shilimkar
@ 2011-06-09 16:53 ` Santosh Shilimkar
1 sibling, 0 replies; 49+ messages in thread
From: Santosh Shilimkar @ 2011-06-09 16:53 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Frank Hofmann, linux-pm, tuxonice-devel, linux-arm-kernel
On 6/9/2011 10:10 PM, Russell King - ARM Linux wrote:
> On Thu, Jun 09, 2011 at 09:57:06PM +0530, Santosh Shilimkar wrote:
>> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>>
>>> That's because OMAP was doing changes to their sleep code while I was
>>> consolidating the sleep code, and although I asked several times that
>>> the OMAP folk should participate in this effort, but evidentally I was
>>> unsuccessful in achieving anything in that direction.
>>
>> Agreed but the situation at that point was the code was not at
>> all in convertible position. Looking at your below comment,
>> it's still not :)
>
> Well, I had a look before posting this reply, and ran away from it.
> I've gone back to it several times since, and got a similar reaction.
>
> I seem to remember that it looked _more_ convertable when I looked at
> it when doing the generic suspend/resume support - I could see a nice
> simple way to pull out the saving and just leave the PLL resume stuff
> in SRAM.
>
> I'm now convinced that if I try to convert it use the generic support,
> it will end up being a horrible broken mess.
I must admit that I had same impression when I started looking at it.
Few provisions are necessary for OMAP which I can think of are:
1. WFI loop should be made a seperate function so that it can pushed
on SRAM which is must for OMAP3.
2. A callback before WFI to implement the Errata WA's
3. Avoid direct write to AUXCTRL in generic suspend code.
4. Before MMU is enabled in resume a callback to restore
secure register, setup auxctrl etc.
With above addressed, mostly we should be able to
get it working. But for sure it will mess up the
simple suspend hooks as they are today.
btw, for OMAP4 as well I looked at this suspend hooks
and most the requirement above apply except 4)
Additionally the L2 cache handling isn't part of
these common suspend hooks.
Regards
Santosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:27 ` [linux-pm] " Santosh Shilimkar
2011-06-09 16:40 ` Russell King - ARM Linux
2011-06-09 16:40 ` [linux-pm] " Russell King - ARM Linux
@ 2011-06-09 16:44 ` Frank Hofmann
2011-06-09 16:56 ` Santosh Shilimkar
2011-06-09 16:56 ` [linux-pm] " Santosh Shilimkar
2011-06-09 16:44 ` Frank Hofmann
3 siblings, 2 replies; 49+ messages in thread
From: Frank Hofmann @ 2011-06-09 16:44 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 9 Jun 2011, Santosh Shilimkar wrote:
> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>
>> That's because OMAP was doing changes to their sleep code while I was
>> consolidating the sleep code, and although I asked several times that
>> the OMAP folk should participate in this effort, but evidentally I was
>> unsuccessful in achieving anything in that direction.
>>
> Agreed but the situation at that point was the code was not at
> all in convertible position. Looking at your below comment,
> it's still not :)
>
>> And of course since then it's been forgotten about, and I've given up
>> on that particular aspect. I've also come to the conclusion that OMAP
>> is sufficiently weird (requiring soo much to execute from SRAM) that
>> its hopeless to persue.
>>
> We did discuss this Russell and requested your help here. I guess
> you have already looked at OMAP code from generic suspend
> hooks point of view and the SRAM execution, Errata's seems to
> make you feel it's not going to work.
> Is that what you mean here ?
>
> Regards
> Santosh
>
Sorry for interjecting ... you're right there's a lot special about OMAP.
What I've been talking about is a rather small(ish) bit. Maybe the diff
illustrates what I mean - use cpu_suspend/resume for the parts of off-mode
save/restore that are non-OMAP-specific.
Like this (not tested, just for illustration what I mean):
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index 63f1066..913279b 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -216,63 +216,14 @@ save_context_wfi:
beq clean_caches
l1_logic_lost:
- mov r4, sp @ Store sp
+ mrs r4, cpsr @ Store cpsr
mrs r5, spsr @ Store spsr
- mov r6, lr @ Store lr
- stmia r8!, {r4-r6}
-
- mrc p15, 0, r4, c1, c0, 2 @ Coprocessor access control register
- mrc p15, 0, r5, c2, c0, 0 @ TTBR0
- mrc p15, 0, r6, c2, c0, 1 @ TTBR1
- mrc p15, 0, r7, c2, c0, 2 @ TTBCR
- stmia r8!, {r4-r7}
-
- mrc p15, 0, r4, c3, c0, 0 @ Domain access Control Register
- mrc p15, 0, r5, c10, c2, 0 @ PRRR
- mrc p15, 0, r6, c10, c2, 1 @ NMRR
- stmia r8!,{r4-r6}
-
- mrc p15, 0, r4, c13, c0, 1 @ Context ID
- mrc p15, 0, r5, c13, c0, 2 @ User r/w thread and process ID
- mrc p15, 0, r6, c12, c0, 0 @ Secure or NS vector base address
- mrs r7, cpsr @ Store current cpsr
- stmia r8!, {r4-r7}
-
- mrc p15, 0, r4, c1, c0, 0 @ save control register
- stmia r8!, {r4}
-
-clean_caches:
- /*
- * jump out to kernel flush routine
- * - reuse that code is better
- * - it executes in a cached space so is faster than refetch per-block
- * - should be faster and will change with kernel
- * - 'might' have to copy address, load and jump to it
- * Flush all data from the L1 data cache before disabling
- * SCTLR.C bit.
- */
- ldr r1, kernel_flush
- mov lr, pc
- bx r1
-
- /*
- * 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
+ stmia sp!, {r4-r5}
+ mov r1, #(PHYS_OFFSET - PAGE_OFFSET)
+ ldr r3, =restore_mmu_on
+ bl cpu_suspend
/*
- * 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.
- */
- ldr r1, kernel_flush
- blx r1
- /*
* The kernel doesn't interwork: v7_flush_dcache_all in particluar will
* always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
* This sequence switches back to ARM. Note that .align may insert a
@@ -463,115 +414,15 @@ l2_inv_gp:
ldr r0, [r3,#12]
mov r12, #0x2
smc #0 @ Call SMI monitor (smieq)
-logic_l1_restore:
- ldr r1, l2dis_3630
- cmp r1, #0x1 @ Test if L2 re-enable needed on 3630
- bne skipl2reen
- mrc p15, 0, r1, c1, c0, 1
- orr r1, r1, #2 @ re-enable L2 cache
- mcr p15, 0, r1, c1, c0, 1
-skipl2reen:
- mov r1, #0
- /*
- * Invalidate all instruction caches to PoU
- * and flush branch target cache
- */
- mcr p15, 0, r1, c7, c5, 0
-
- ldr r4, scratchpad_base
- ldr r3, [r4,#0xBC]
- adds r3, r3, #16
-
- ldmia r3!, {r4-r6}
- mov sp, r4 @ Restore sp
- msr spsr_cxsf, r5 @ Restore spsr
- mov lr, r6 @ Restore lr
-
- ldmia r3!, {r4-r7}
- mcr p15, 0, r4, c1, c0, 2 @ Coprocessor access Control Register
- mcr p15, 0, r5, c2, c0, 0 @ TTBR0
- mcr p15, 0, r6, c2, c0, 1 @ TTBR1
- mcr p15, 0, r7, c2, c0, 2 @ TTBCR
-
- ldmia r3!,{r4-r6}
- mcr p15, 0, r4, c3, c0, 0 @ Domain access Control Register
- mcr p15, 0, r5, c10, c2, 0 @ PRRR
- mcr p15, 0, r6, c10, c2, 1 @ NMRR
-
-
- ldmia r3!,{r4-r7}
- mcr p15, 0, r4, c13, c0, 1 @ Context ID
- mcr p15, 0, r5, c13, c0, 2 @ User r/w thread and process ID
- mrc p15, 0, r6, c12, c0, 0 @ Secure or NS vector base address
- msr cpsr, r7 @ store cpsr
-
- /* Enabling MMU here */
- mrc p15, 0, r7, c2, c0, 2 @ Read TTBRControl
- /* Extract N (0:2) bits and decide whether to use TTBR0 or TTBR1 */
- and r7, #0x7
- cmp r7, #0x0
- beq usettbr0
-ttbr_error:
- /*
- * More work needs to be done to support N[0:2] value other than 0
- * So looping here so that the error can be detected
- */
- b ttbr_error
-usettbr0:
- mrc p15, 0, r2, c2, c0, 0
- ldr r5, ttbrbit_mask
- and r2, r5
- mov r4, pc
- ldr r5, table_index_mask
- and r4, r5 @ r4 = 31 to 20 bits of pc
- /* Extract the value to be written to table entry */
- ldr r1, table_entry
- /* r1 has the value to be written to table entry*/
- add r1, r1, r4
- /* Getting the address of table entry to modify */
- lsr r4, #18
- /* r2 has the location which needs to be modified */
- add r2, r4
- /* Storing previous entry of location being modified */
- ldr r5, scratchpad_base
- ldr r4, [r2]
- str r4, [r5, #0xC0]
- /* Modify the table entry */
- str r1, [r2]
- /*
- * Storing address of entry being modified
- * - will be restored after enabling MMU
- */
- ldr r5, scratchpad_base
- str r2, [r5, #0xC4]
-
- mov r0, #0
- mcr p15, 0, r0, c7, c5, 4 @ Flush prefetch buffer
- mcr p15, 0, r0, c7, c5, 6 @ Invalidate branch predictor array
- mcr p15, 0, r0, c8, c5, 0 @ Invalidate instruction TLB
- mcr p15, 0, r0, c8, c6, 0 @ Invalidate data TLB
- /*
- * Restore control register. This enables the MMU.
- * The caches and prediction are not enabled here, they
- * will be enabled after restoring the MMU table entry.
- */
- ldmia r3!, {r4}
- /* Store previous value of control register in scratchpad */
- str r4, [r5, #0xC8]
- ldr r2, cache_pred_disable_mask
- and r4, r2
- mcr p15, 0, r4, c1, c0, 0
- dsb
- isb
- ldr r0, =restoremmu_on
- bx r0
-
/*
* ==============================
* == Exit point from OFF mode ==
* ==============================
*/
restoremmu_on:
+ ldmfd sp!, {r0,r1}
+ msr cpsr, r0
+ msr spsr, r1
ldmfd sp!, {r0-r12, pc} @ restore regs and return
FrankH.
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:44 ` [linux-pm] " Frank Hofmann
@ 2011-06-09 16:56 ` Santosh Shilimkar
2011-06-09 16:56 ` [linux-pm] " Santosh Shilimkar
1 sibling, 0 replies; 49+ messages in thread
From: Santosh Shilimkar @ 2011-06-09 16:56 UTC (permalink / raw)
To: frank.hofmann
Cc: linux-pm, Russell King - ARM Linux, tuxonice-devel,
linux-arm-kernel
On 6/9/2011 10:14 PM, Frank Hofmann wrote:
>
>
> On Thu, 9 Jun 2011, Santosh Shilimkar wrote:
>
>> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>>> Btw, when testing this I found that generic cpu_suspend seems to be
>>>> just
>>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>>
>>> That's because OMAP was doing changes to their sleep code while I was
>>> consolidating the sleep code, and although I asked several times that
>>> the OMAP folk should participate in this effort, but evidentally I was
>>> unsuccessful in achieving anything in that direction.
>>>
>> Agreed but the situation at that point was the code was not at
>> all in convertible position. Looking at your below comment,
>> it's still not :)
>>
>>> And of course since then it's been forgotten about, and I've given up
>>> on that particular aspect. I've also come to the conclusion that OMAP
>>> is sufficiently weird (requiring soo much to execute from SRAM) that
>>> its hopeless to persue.
>>>
>> We did discuss this Russell and requested your help here. I guess
>> you have already looked at OMAP code from generic suspend
>> hooks point of view and the SRAM execution, Errata's seems to
>> make you feel it's not going to work.
>> Is that what you mean here ?
>>
>> Regards
>> Santosh
>>
>
> Sorry for interjecting ... you're right there's a lot special about
> OMAP. What I've been talking about is a rather small(ish) bit. Maybe the
> diff illustrates what I mean - use cpu_suspend/resume for the parts of
> off-mode save/restore that are non-OMAP-specific.
>
> Like this (not tested, just for illustration what I mean):
>
Mostly it won't work.
Just replied to your questions. I think you can get the
answer on why this change won't work in it's current form.
Regards
Santosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:44 ` [linux-pm] " Frank Hofmann
2011-06-09 16:56 ` Santosh Shilimkar
@ 2011-06-09 16:56 ` Santosh Shilimkar
1 sibling, 0 replies; 49+ messages in thread
From: Santosh Shilimkar @ 2011-06-09 16:56 UTC (permalink / raw)
To: linux-arm-kernel
On 6/9/2011 10:14 PM, Frank Hofmann wrote:
>
>
> On Thu, 9 Jun 2011, Santosh Shilimkar wrote:
>
>> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>>> Btw, when testing this I found that generic cpu_suspend seems to be
>>>> just
>>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>>
>>> That's because OMAP was doing changes to their sleep code while I was
>>> consolidating the sleep code, and although I asked several times that
>>> the OMAP folk should participate in this effort, but evidentally I was
>>> unsuccessful in achieving anything in that direction.
>>>
>> Agreed but the situation at that point was the code was not at
>> all in convertible position. Looking at your below comment,
>> it's still not :)
>>
>>> And of course since then it's been forgotten about, and I've given up
>>> on that particular aspect. I've also come to the conclusion that OMAP
>>> is sufficiently weird (requiring soo much to execute from SRAM) that
>>> its hopeless to persue.
>>>
>> We did discuss this Russell and requested your help here. I guess
>> you have already looked at OMAP code from generic suspend
>> hooks point of view and the SRAM execution, Errata's seems to
>> make you feel it's not going to work.
>> Is that what you mean here ?
>>
>> Regards
>> Santosh
>>
>
> Sorry for interjecting ... you're right there's a lot special about
> OMAP. What I've been talking about is a rather small(ish) bit. Maybe the
> diff illustrates what I mean - use cpu_suspend/resume for the parts of
> off-mode save/restore that are non-OMAP-specific.
>
> Like this (not tested, just for illustration what I mean):
>
Mostly it won't work.
Just replied to your questions. I think you can get the
answer on why this change won't work in it's current form.
Regards
Santosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC PATCH v4] ARM hibernation/suspend-to-disk support
2011-06-09 16:27 ` [linux-pm] " Santosh Shilimkar
` (2 preceding siblings ...)
2011-06-09 16:44 ` [linux-pm] " Frank Hofmann
@ 2011-06-09 16:44 ` Frank Hofmann
3 siblings, 0 replies; 49+ messages in thread
From: Frank Hofmann @ 2011-06-09 16:44 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Russell King - ARM Linux, tuxonice-devel, Frank Hofmann, linux-pm,
linux-arm-kernel
On Thu, 9 Jun 2011, Santosh Shilimkar wrote:
> On 6/9/2011 9:10 PM, Russell King - ARM Linux wrote:
>> On Thu, Jun 09, 2011 at 04:30:08PM +0100, Frank Hofmann wrote:
>>> Btw, when testing this I found that generic cpu_suspend seems to be just
>>> fine for OMAP3; the OMAP platforms though do not at this time use the
>>> generic cpu_suspend/resume for sleep, is it planned to change that ?
>>
>> That's because OMAP was doing changes to their sleep code while I was
>> consolidating the sleep code, and although I asked several times that
>> the OMAP folk should participate in this effort, but evidentally I was
>> unsuccessful in achieving anything in that direction.
>>
> Agreed but the situation at that point was the code was not at
> all in convertible position. Looking at your below comment,
> it's still not :)
>
>> And of course since then it's been forgotten about, and I've given up
>> on that particular aspect. I've also come to the conclusion that OMAP
>> is sufficiently weird (requiring soo much to execute from SRAM) that
>> its hopeless to persue.
>>
> We did discuss this Russell and requested your help here. I guess
> you have already looked at OMAP code from generic suspend
> hooks point of view and the SRAM execution, Errata's seems to
> make you feel it's not going to work.
> Is that what you mean here ?
>
> Regards
> Santosh
>
Sorry for interjecting ... you're right there's a lot special about OMAP.
What I've been talking about is a rather small(ish) bit. Maybe the diff
illustrates what I mean - use cpu_suspend/resume for the parts of off-mode
save/restore that are non-OMAP-specific.
Like this (not tested, just for illustration what I mean):
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index 63f1066..913279b 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -216,63 +216,14 @@ save_context_wfi:
beq clean_caches
l1_logic_lost:
- mov r4, sp @ Store sp
+ mrs r4, cpsr @ Store cpsr
mrs r5, spsr @ Store spsr
- mov r6, lr @ Store lr
- stmia r8!, {r4-r6}
-
- mrc p15, 0, r4, c1, c0, 2 @ Coprocessor access control register
- mrc p15, 0, r5, c2, c0, 0 @ TTBR0
- mrc p15, 0, r6, c2, c0, 1 @ TTBR1
- mrc p15, 0, r7, c2, c0, 2 @ TTBCR
- stmia r8!, {r4-r7}
-
- mrc p15, 0, r4, c3, c0, 0 @ Domain access Control Register
- mrc p15, 0, r5, c10, c2, 0 @ PRRR
- mrc p15, 0, r6, c10, c2, 1 @ NMRR
- stmia r8!,{r4-r6}
-
- mrc p15, 0, r4, c13, c0, 1 @ Context ID
- mrc p15, 0, r5, c13, c0, 2 @ User r/w thread and process ID
- mrc p15, 0, r6, c12, c0, 0 @ Secure or NS vector base address
- mrs r7, cpsr @ Store current cpsr
- stmia r8!, {r4-r7}
-
- mrc p15, 0, r4, c1, c0, 0 @ save control register
- stmia r8!, {r4}
-
-clean_caches:
- /*
- * jump out to kernel flush routine
- * - reuse that code is better
- * - it executes in a cached space so is faster than refetch per-block
- * - should be faster and will change with kernel
- * - 'might' have to copy address, load and jump to it
- * Flush all data from the L1 data cache before disabling
- * SCTLR.C bit.
- */
- ldr r1, kernel_flush
- mov lr, pc
- bx r1
-
- /*
- * 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
+ stmia sp!, {r4-r5}
+ mov r1, #(PHYS_OFFSET - PAGE_OFFSET)
+ ldr r3, =restore_mmu_on
+ bl cpu_suspend
/*
- * 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.
- */
- ldr r1, kernel_flush
- blx r1
- /*
* The kernel doesn't interwork: v7_flush_dcache_all in particluar will
* always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
* This sequence switches back to ARM. Note that .align may insert a
@@ -463,115 +414,15 @@ l2_inv_gp:
ldr r0, [r3,#12]
mov r12, #0x2
smc #0 @ Call SMI monitor (smieq)
-logic_l1_restore:
- ldr r1, l2dis_3630
- cmp r1, #0x1 @ Test if L2 re-enable needed on 3630
- bne skipl2reen
- mrc p15, 0, r1, c1, c0, 1
- orr r1, r1, #2 @ re-enable L2 cache
- mcr p15, 0, r1, c1, c0, 1
-skipl2reen:
- mov r1, #0
- /*
- * Invalidate all instruction caches to PoU
- * and flush branch target cache
- */
- mcr p15, 0, r1, c7, c5, 0
-
- ldr r4, scratchpad_base
- ldr r3, [r4,#0xBC]
- adds r3, r3, #16
-
- ldmia r3!, {r4-r6}
- mov sp, r4 @ Restore sp
- msr spsr_cxsf, r5 @ Restore spsr
- mov lr, r6 @ Restore lr
-
- ldmia r3!, {r4-r7}
- mcr p15, 0, r4, c1, c0, 2 @ Coprocessor access Control Register
- mcr p15, 0, r5, c2, c0, 0 @ TTBR0
- mcr p15, 0, r6, c2, c0, 1 @ TTBR1
- mcr p15, 0, r7, c2, c0, 2 @ TTBCR
-
- ldmia r3!,{r4-r6}
- mcr p15, 0, r4, c3, c0, 0 @ Domain access Control Register
- mcr p15, 0, r5, c10, c2, 0 @ PRRR
- mcr p15, 0, r6, c10, c2, 1 @ NMRR
-
-
- ldmia r3!,{r4-r7}
- mcr p15, 0, r4, c13, c0, 1 @ Context ID
- mcr p15, 0, r5, c13, c0, 2 @ User r/w thread and process ID
- mrc p15, 0, r6, c12, c0, 0 @ Secure or NS vector base address
- msr cpsr, r7 @ store cpsr
-
- /* Enabling MMU here */
- mrc p15, 0, r7, c2, c0, 2 @ Read TTBRControl
- /* Extract N (0:2) bits and decide whether to use TTBR0 or TTBR1 */
- and r7, #0x7
- cmp r7, #0x0
- beq usettbr0
-ttbr_error:
- /*
- * More work needs to be done to support N[0:2] value other than 0
- * So looping here so that the error can be detected
- */
- b ttbr_error
-usettbr0:
- mrc p15, 0, r2, c2, c0, 0
- ldr r5, ttbrbit_mask
- and r2, r5
- mov r4, pc
- ldr r5, table_index_mask
- and r4, r5 @ r4 = 31 to 20 bits of pc
- /* Extract the value to be written to table entry */
- ldr r1, table_entry
- /* r1 has the value to be written to table entry*/
- add r1, r1, r4
- /* Getting the address of table entry to modify */
- lsr r4, #18
- /* r2 has the location which needs to be modified */
- add r2, r4
- /* Storing previous entry of location being modified */
- ldr r5, scratchpad_base
- ldr r4, [r2]
- str r4, [r5, #0xC0]
- /* Modify the table entry */
- str r1, [r2]
- /*
- * Storing address of entry being modified
- * - will be restored after enabling MMU
- */
- ldr r5, scratchpad_base
- str r2, [r5, #0xC4]
-
- mov r0, #0
- mcr p15, 0, r0, c7, c5, 4 @ Flush prefetch buffer
- mcr p15, 0, r0, c7, c5, 6 @ Invalidate branch predictor array
- mcr p15, 0, r0, c8, c5, 0 @ Invalidate instruction TLB
- mcr p15, 0, r0, c8, c6, 0 @ Invalidate data TLB
- /*
- * Restore control register. This enables the MMU.
- * The caches and prediction are not enabled here, they
- * will be enabled after restoring the MMU table entry.
- */
- ldmia r3!, {r4}
- /* Store previous value of control register in scratchpad */
- str r4, [r5, #0xC8]
- ldr r2, cache_pred_disable_mask
- and r4, r2
- mcr p15, 0, r4, c1, c0, 0
- dsb
- isb
- ldr r0, =restoremmu_on
- bx r0
-
/*
* ==============================
* == Exit point from OFF mode ==
* ==============================
*/
restoremmu_on:
+ ldmfd sp!, {r0,r1}
+ msr cpsr, r0
+ msr spsr, r1
ldmfd sp!, {r0-r12, pc} @ restore regs and return
FrankH.
^ permalink raw reply related [flat|nested] 49+ messages in thread