Skip to content

RFC OMAP4 cpuidle cleanup

Daniel Lezcano linux-arm-kernel linux-omap 1 version New

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 RFC Latest0 of 7 patches 0 repliesNew

Patches in v1 No applies recorded yet

None of the 7 patches the cover letter announces are in this archive.

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [RFC][PATCH 0/7] OMAP4 cpuidle cleanup
Quoted 22 lines · Jean Pihet
Quoted 20 lines · Santosh Shilimkar

Daniel,

Quoted 17 lines · Daniel Lezcano

This patchset is a proposition to improve a bit the code. The changes are code cleanup and does not change the behavior of the driver itself.

A couple a things call my intention. Why the cpuidle device is set for cpu0 only and why the WFI is not used ?

Daniel Lezcano (7):
ARM: OMAP4: cpuidle - Remove unused valid field
ARM: OMAP4: cpuidle - Declare the states with the driver declaration
ARM: OMAP4: cpuidle - Remove the cpuidle_params_table table
ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration
ARM: OMAP4: cpuidle - Initialize omap4_idle_data at compile time
ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot
time

Quoted 6 lines
Quoted 2 lines

The series looks fine to me in general. This clean-up is applicable for OMAP3 cpuidle code as well.

Great!
However OMAP3 has a few specific things that cannot be removed as easily:
- the 'valid' flag is used because only certain combinations of power domains states are possible,

When I look the board-rx51 code I see:

static struct cpuidle_params rx51_cpuidle_params[] = {
	/* C1 */
	{110 + 162, 5 , 1},
	/* C2 */
	{106 + 180, 309, 1},
	/* C3 */
	{107 + 410, 46057, 0},
	/* C4 */
	{121 + 3374, 46057, 0},
	/* C5 */
	{855 + 1146, 46057, 1},
	/* C6 */
	{7580 + 4134, 484329, 0},
	/* C7 */
	{7505 + 15274, 484329, 1},
};

If C3, C4, C6 are not valid, so AFAICS never used in the cpuidle code why the values are 'exit_latency' and 'target_residency' specified ? I mean why don't we have { 0, 0, 0 } ? Is it just for information ?

I understand the purpose of this code but it looks a bit tricky and hard to factor out. Is it acceptable to create a new cpuidle driver for rx51 then we factor out the code between omap3, omap4 and rx51 when all the code consistent ?

Quoted 14 lines

- the latency settings can be overriden by the board code, so the cpuidle_params struct is needed.

Quoted 8 lines

I want Jean to look at this series because some of his earlier clean up has introduced those custom functions which are getting removed in this series.

Regards
santosh

Thanks,
Jean

Signature · 5 lines
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Read the cover letter in its thread