OMAP3: CPUidle driver

PATCH In review Rajendra Nayak 2 participants 7 messages Oct 8, 2008 – Oct 19, 2008

Subsystems (3)

Arm (32-bit) Arm sub-architectures OMAP2+ support

lore

Share & tools

lore

Rajendra Nayak

author

2008-10-08 12:00 UTC #0 lore raw

cover OMAP3: CPUidle driver

Hi,

This patch set implements Cpuidle driver for OMAP3 platforms. It is validated on the 3430SDP (using the omap_3430sdp_min_defconfig) and applies on the pm-20081006 branch.

The following C states are defined and used

 *      C0 . System executing code (Not an idle state)
 *      C1 . MPU WFI + Core active
 *      C2 . MPU CSWR + Core active
 *      C3 . MPU OFF + Core active
 *      C4 . MPU CSWR + Core CSWR
 *      C5 . MPU OFF + Core CSWR
 *      C6 . MPU OFF + Core OFF

CPUidle support is not enabled by default in any of the defconfigs and hence has to enabled seperately if you plan to test these patches.

CPUIdle ---> [*] CPU idle PM support

regards,
Rajendra

Kevin Hilman

2008-10-09 13:27 UTC #1 lore raw

Quoted 3 lines · Rajendra Nayak

This patch set implements Cpuidle driver for OMAP3 platforms. It is validated on the 3430SDP (using the omap_3430sdp_min_defconfig) and applies on the pm-20081006 branch.

Excellent. Thank you. I'll pull into pm-20081006 today.

One question: You have C6 to hit chip-off, but it is never enabled. Has this been tested? Enabling it allows me to hit chip OFF, but it never comes back from OFF-mode. I will push some patches to make this work with /sys/power/enable_off_mode.

Some minor problems that I will fixup when rebasing to the next pm branch:

- You removed C0, but all the comments still refer to it
- /sys/power/enable_off_mode doesn't affect off-mode states in CPUidle

patch 1:
- I still don't like the #ifdefs for the init sequence.

patch 2:
- checkpatch warning

Kevin

Quoted 17 lines

The following C states are defined and used
 *      C0 . System executing code (Not an idle state)
 *      C1 . MPU WFI + Core active
 *      C2 . MPU CSWR + Core active
 *      C3 . MPU OFF + Core active
 *      C4 . MPU CSWR + Core CSWR
 *      C5 . MPU OFF + Core CSWR
 *      C6 . MPU OFF + Core OFF
CPUidle support is not enabled by default in any of the defconfigs and hence has to enabled seperately if you plan to test these patches.
CPUIdle ---> [*] CPU idle PM support
regards,
Rajendra

Kevin Hilman

2008-10-09 14:44 UTC #2 lore raw

Quoted 6 lines · Kevin Hilman

Quoted 3 lines · Rajendra Nayak
This patch set implements Cpuidle driver for OMAP3 platforms. It is validated on the 3430SDP (using the omap_3430sdp_min_defconfig) and applies on the pm-20081006 branch.

Quoted 6 lines

Excellent. Thank you. I'll pull into pm-20081006 today.
One question: You have C6 to hit chip-off, but it is never enabled. Has this been tested? Enabling it allows me to hit chip OFF, but it never comes back from OFF-mode. I will push some patches to make this work with /sys/power/enable_off_mode.

I got this to work on custom HW, but not on 3430SDP. Can you confirm whether or not you can use C6 on 3430SDP? Off-in-idle without CPUidle seems to work, but not with CPUidle + C6.

Kevin

Quoted 32 lines

Some minor problems that I will fixup when rebasing to the next pm branch:
- You removed C0, but all the comments still refer to it
- /sys/power/enable_off_mode doesn't affect off-mode states in CPUidle
patch 1:
- I still don't like the #ifdefs for the init sequence.
patch 2:
- checkpatch warning
Kevin
Quoted 17 lines
The following C states are defined and used
 *      C0 . System executing code (Not an idle state)
 *      C1 . MPU WFI + Core active
 *      C2 . MPU CSWR + Core active
 *      C3 . MPU OFF + Core active
 *      C4 . MPU CSWR + Core CSWR
 *      C5 . MPU OFF + Core CSWR
 *      C6 . MPU OFF + Core OFF
CPUidle support is not enabled by default in any of the defconfigs and hence has to enabled seperately if you plan to test these patches.
CPUIdle ---> [*] CPU idle PM support
regards,
Rajendra

Rajendra Nayak

author

2008-10-10 03:47 UTC #3 lore raw

Quoted 21 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Kevin Hilman
Sent: Thursday, October 09, 2008 8:15 PM
To: Rajendra Nayak
Cc: [email protected]
Subject: Re: [PATCH 00/05] OMAP3: CPUidle driver
Quoted 11 lines · Kevin Hilman
Quoted 3 lines · Rajendra Nayak
This patch set implements Cpuidle driver for OMAP3 platforms. It is validated on the 3430SDP (using the omap_3430sdp_min_defconfig) and applies on the pm-20081006 branch.
Excellent. Thank you. I'll pull into pm-20081006 today.
One question: You have C6 to hit chip-off, but it is never enabled. Has this been tested? Enabling it allows me to hit chip OFF, but it never comes back from OFF-mode. I will push some patches

Quoted 6 lines

to make this
Quoted 1 line
work with /sys/power/enable_off_mode.
I got this to work on custom HW, but not on 3430SDP. Can you confirm whether or not you can use C6 on 3430SDP? Off-in-idle without CPUidle seems to work, but not with CPUidle + C6.

Ok.. I'll try and fix this. I seem to have missed enabling this after all the comment fixes that I did. Maybe I broke something.

Quoted 43 lines

Kevin
Quoted 7 lines
Some minor problems that I will fixup when rebasing to the next pm branch:
- You removed C0, but all the comments still refer to it
- /sys/power/enable_off_mode doesn't affect off-mode states in CPUidle
Quoted 19 lines
patch 1:
- I still don't like the #ifdefs for the init sequence.
patch 2:
- checkpatch warning
Kevin
Quoted 11 lines
The following C states are defined and used
 *      C0 . System executing code (Not an idle state)
 *      C1 . MPU WFI + Core active
 *      C2 . MPU CSWR + Core active
 *      C3 . MPU OFF + Core active
 *      C4 . MPU CSWR + Core CSWR
 *      C5 . MPU OFF + Core CSWR
 *      C6 . MPU OFF + Core OFF
CPUidle support is not enabled by default in any of the
defconfigs and hence has to enabled seperately
Quoted 6 lines
Quoted 6 lines
if you plan to test these patches.
CPUIdle ---> [*] CPU idle PM support
regards,
Rajendra
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Rajendra Nayak

author

2008-10-14 10:41 UTC #4 lore raw

In reply to Kevin Hilman

Quoted 22 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Kevin Hilman
Sent: Thursday, October 09, 2008 8:15 PM
To: Rajendra Nayak
Cc: [email protected]
Subject: Re: [PATCH 00/05] OMAP3: CPUidle driver
Quoted 11 lines · Kevin Hilman
Quoted 3 lines · Rajendra Nayak
This patch set implements Cpuidle driver for OMAP3 platforms. It is validated on the 3430SDP (using the omap_3430sdp_min_defconfig) and applies on the pm-20081006 branch.
Excellent. Thank you. I'll pull into pm-20081006 today.
One question: You have C6 to hit chip-off, but it is never enabled. Has this been tested? Enabling it allows me to hit chip OFF, but it never comes back from OFF-mode. I will push some patches

Quoted 8 lines

to make this
Quoted 1 line
work with /sys/power/enable_off_mode.
I got this to work on custom HW, but not on 3430SDP. Can you confirm whether or not you can use C6 on 3430SDP? Off-in-idle without CPUidle seems to work, but not with CPUidle + C6.

Hi Kevin,

With just this patch below I could see that I was able to hit sysoff even with CPUidle.

Doing this I could see the SYSOFF LED toggle..

# echo -n 1 > /sys/power/clocks_off_while_idle
# echo -n 1 > /sys/power/sleep_while_idle
# echo -n 1 > /sys/power/voltage_off_while_idle
# echo -n 1 > /sys/power/enable_off_mode

And doing this stops the toggle..

# echo -n 0 > /sys/power/enable_off_mode

Was there any other issue you saw with CPUidle + C6?

Rajendra

---
 arch/arm/mach-omap2/cpuidle34xx.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

arch/arm/mach-omap2/cpuidle34xx.c +1 -1

Index: linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.c
===================================================================
@@ -191,7 +191,7 @@ void omap_init_power_states(void)
 				CPUIDLE_FLAG_CHECK_BM;
 
 	/* C6 . MPU OFF + Core OFF */
-	omap3_power_states[OMAP3_STATE_C6].valid = 0;
+	omap3_power_states[OMAP3_STATE_C6].valid = 1;
 	omap3_power_states[OMAP3_STATE_C6].type = OMAP3_STATE_C6;
 	omap3_power_states[OMAP3_STATE_C6].sleep_latency = 10000;
 	omap3_power_states[OMAP3_STATE_C6].wakeup_latency = 30000;

Quoted 41 lines

Quoted 8 lines
Some minor problems that I will fixup when rebasing to the next pm branch:
- You removed C0, but all the comments still refer to it
- /sys/power/enable_off_mode doesn't affect off-mode states in CPUidle
Quoted 19 lines
patch 1:
- I still don't like the #ifdefs for the init sequence.
patch 2:
- checkpatch warning
Kevin
Quoted 11 lines
The following C states are defined and used
 *      C0 . System executing code (Not an idle state)
 *      C1 . MPU WFI + Core active
 *      C2 . MPU CSWR + Core active
 *      C3 . MPU OFF + Core active
 *      C4 . MPU CSWR + Core CSWR
 *      C5 . MPU OFF + Core CSWR
 *      C6 . MPU OFF + Core OFF
CPUidle support is not enabled by default in any of the
defconfigs and hence has to enabled seperately
Quoted 6 lines
Quoted 6 lines
if you plan to test these patches.
CPUIdle ---> [*] CPU idle PM support
regards,
Rajendra
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Kevin Hilman

2008-10-19 09:26 UTC #5 lore raw

Quoted 38 lines · Rajendra Nayak

Quoted 16 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Kevin Hilman
Sent: Thursday, October 09, 2008 8:15 PM
To: Rajendra Nayak
Cc: [email protected]
Subject: Re: [PATCH 00/05] OMAP3: CPUidle driver
Quoted 7 lines · Kevin Hilman
Quoted 3 lines · Rajendra Nayak
This patch set implements Cpuidle driver for OMAP3 platforms. It is validated on the 3430SDP (using the omap_3430sdp_min_defconfig) and applies on the pm-20081006 branch.
Excellent. Thank you. I'll pull into pm-20081006 today.
Quoted 10 lines
Quoted 3 lines
One question: You have C6 to hit chip-off, but it is never enabled. Has this been tested? Enabling it allows me to hit chip OFF, but it never comes back from OFF-mode. I will push some patches
to make this
Quoted 1 line
work with /sys/power/enable_off_mode.
I got this to work on custom HW, but not on 3430SDP. Can you confirm whether or not you can use C6 on 3430SDP? Off-in-idle without CPUidle seems to work, but not with CPUidle + C6.
Hi Kevin,
With just this patch below I could see that I was able to hit sysoff even with CPUidle.
Doing this I could see the SYSOFF LED toggle..
# echo -n 1 > /sys/power/clocks_off_while_idle
# echo -n 1 > /sys/power/sleep_while_idle
# echo -n 1 > /sys/power/voltage_off_while_idle
# echo -n 1 > /sys/power/enable_off_mode

Quoted 5 lines

And doing this stops the toggle..
# echo -n 0 > /sys/power/enable_off_mode
Was there any other issue you saw with CPUidle + C6?

Yes, I was seeing hangs as soon as I enabled off-mode.

I'm currently on the road and when I get home (Wed.) I won't have access
to an SDP for a while, so I'll trust you on this one. Maybe I need an
updated u-boot on my SDP?

Kevin

Quoted 58 lines

 arch/arm/mach-omap2/cpuidle34xx.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
Index: linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.c
===================================================================
arch/arm/mach-omap2/cpuidle34xx.c +1 -1
@@ -191,7 +191,7 @@ void omap_init_power_states(void)
 				CPUIDLE_FLAG_CHECK_BM;
 
 	/* C6 . MPU OFF + Core OFF */
-	omap3_power_states[OMAP3_STATE_C6].valid = 0;
+	omap3_power_states[OMAP3_STATE_C6].valid = 1;
 	omap3_power_states[OMAP3_STATE_C6].type = OMAP3_STATE_C6;
 	omap3_power_states[OMAP3_STATE_C6].sleep_latency = 10000;
 	omap3_power_states[OMAP3_STATE_C6].wakeup_latency = 30000;
Quoted 41 lines
Quoted 9 lines
Some minor problems that I will fixup when rebasing to the next pm branch:
- You removed C0, but all the comments still refer to it
- /sys/power/enable_off_mode doesn't affect off-mode states in CPUidle
Quoted 19 lines
patch 1:
- I still don't like the #ifdefs for the init sequence.
patch 2:
- checkpatch warning
Kevin
Quoted 11 lines
The following C states are defined and used
 *      C0 . System executing code (Not an idle state)
 *      C1 . MPU WFI + Core active
 *      C2 . MPU CSWR + Core active
 *      C3 . MPU OFF + Core active
 *      C4 . MPU CSWR + Core CSWR
 *      C5 . MPU OFF + Core CSWR
 *      C6 . MPU OFF + Core OFF
CPUidle support is not enabled by default in any of the
defconfigs and hence has to enabled seperately
Quoted 6 lines
Quoted 6 lines
if you plan to test these patches.
CPUIdle ---> [*] CPU idle PM support
regards,
Rajendra
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Rajendra Nayak

author

2008-10-10 03:44 UTC #6 lore raw

In reply to Kevin Hilman

Quoted 13 lines

-----Original Message-----
From: Kevin Hilman [mailto:[email protected]]
Sent: Thursday, October 09, 2008 6:57 PM
To: Rajendra Nayak
Cc: [email protected]
Subject: Re: [PATCH 00/05] OMAP3: CPUidle driver
Quoted 3 lines · Rajendra Nayak
This patch set implements Cpuidle driver for OMAP3 platforms. It is validated on the 3430SDP (using the omap_3430sdp_min_defconfig) and applies on the pm-20081006 branch.

Quoted 6 lines

Excellent. Thank you. I'll pull into pm-20081006 today.
One question: You have C6 to hit chip-off, but it is never enabled. Has this been tested? Enabling it allows me to hit chip OFF, but it never comes back from OFF-mode. I will push some patches to make this work with /sys/power/enable_off_mode.

Yes, that was something I missed out in those patches. I will send in a patch to do that if you have'nt done it already.

Quoted 8 lines

Some minor problems that I will fixup when rebasing to the next pm branch:
- You removed C0, but all the comments still refer to it
- /sys/power/enable_off_mode doesn't affect off-mode states in CPUidle

Quoted 5 lines

patch 1:
- I still don't like the #ifdefs for the init sequence.
patch 2:
- checkpatch warning

That was for an extern in a C file (cpuidle34xx.c) and I was'nt sure which header was the right place for it (since now we don't have a cpuidle34xx.h). So I kind of left it there.

Quoted 23 lines

Kevin
Quoted 11 lines
The following C states are defined and used
 *      C0 . System executing code (Not an idle state)
 *      C1 . MPU WFI + Core active
 *      C2 . MPU CSWR + Core active
 *      C3 . MPU OFF + Core active
 *      C4 . MPU CSWR + Core CSWR
 *      C5 . MPU OFF + Core CSWR
 *      C6 . MPU OFF + Core OFF
CPUidle support is not enabled by default in any of the
defconfigs and hence has to enabled seperately
Quoted 6 lines
if you plan to test these patches.
CPUIdle ---> [*] CPU idle PM support
regards,
Rajendra