From: khilman@ti•com (Kevin Hilman)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC][PATCH 0/7] OMAP4 cpuidle cleanup
Date: Wed, 21 Mar 2012 14:54:23 -0700 [thread overview]
Message-ID: <873991d3hs.fsf@ti.com> (raw)
In-Reply-To: <4F6A04F3.6050306@linaro.org> (Daniel Lezcano's message of "Wed, 21 Mar 2012 17:42:27 +0100")
Daniel Lezcano <daniel.lezcano@linaro•org> writes:
> On 03/21/2012 02:43 PM, Jean Pihet wrote:
>> On Wed, Mar 21, 2012 at 11:07 AM, Santosh Shilimkar
>> <santosh.shilimkar@ti•com> wrote:
>>> Daniel,
>>>
>>> On Wednesday 21 March 2012 02:57 PM, Daniel Lezcano wrote:
>>>> 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
>>>>
>>> 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 ?
Yes, because getting those numbers were based on some board-specific
measurements, and we don't want those values to be lost. Also, some
usage patterns might want to (re)enable those states.
> 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 ?
I don't like that idea. All the code is the same, the only thing we
need is the ability to pass in board-specific latency numbers for the
C-states.
These latency numbers are obviously quite significant when it comes to
the final power consumption, and these values often depend on
board-specific settings. Until there are generic frameworks for
defining all the latencies involved, passing these values in from board
files is absolutly needed.
Kevin
next prev parent reply other threads:[~2012-03-21 21:54 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-21 9:27 [RFC][PATCH 0/7] OMAP4 cpuidle cleanup Daniel Lezcano
2012-03-21 9:27 ` [RFC][PATCH 1/7] ARM: OMAP4: cpuidle - Remove unused valid field Daniel Lezcano
2012-03-21 9:41 ` Shilimkar, Santosh
2012-03-21 9:46 ` Daniel Lezcano
2012-03-21 10:03 ` Santosh Shilimkar
2012-03-21 13:28 ` Jean Pihet
2012-03-21 9:27 ` [RFC][PATCH 2/7] ARM: OMAP4: cpuidle - Declare the states with the driver declaration Daniel Lezcano
2012-03-21 9:50 ` Santosh Shilimkar
2012-03-21 13:31 ` Jean Pihet
2012-03-21 14:12 ` Daniel Lezcano
2012-03-21 9:27 ` [RFC][PATCH 3/7] ARM: OMAP4: cpuidle - Remove the cpuidle_params_table table Daniel Lezcano
2012-03-21 9:27 ` [RFC][PATCH 4/7] ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration Daniel Lezcano
2012-03-21 9:51 ` Santosh Shilimkar
2012-03-21 9:27 ` [RFC][PATCH 5/7] ARM: OMAP4: cpuidle - Initialize omap4_idle_data at compile time Daniel Lezcano
2012-03-21 9:27 ` [RFC][PATCH 6/7] ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly Daniel Lezcano
2012-03-21 9:27 ` [RFC][PATCH 7/7] ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot time Daniel Lezcano
2012-03-21 9:36 ` [RFC][PATCH 0/7] OMAP4 cpuidle cleanup Shilimkar, Santosh
2012-03-21 9:51 ` Daniel Lezcano
2012-03-21 9:56 ` Santosh Shilimkar
2012-03-21 10:43 ` Daniel Lezcano
2012-03-21 10:49 ` Shilimkar, Santosh
2012-03-21 10:59 ` Daniel Lezcano
2012-03-21 10:07 ` Santosh Shilimkar
2012-03-21 10:49 ` Daniel Lezcano
2012-03-21 13:19 ` Jean Pihet
2012-03-21 14:13 ` Daniel Lezcano
2012-03-21 14:23 ` Shilimkar, Santosh
2012-03-21 13:43 ` Jean Pihet
2012-03-21 14:19 ` Daniel Lezcano
2012-03-21 16:42 ` Daniel Lezcano
2012-03-21 21:54 ` Kevin Hilman [this message]
2012-03-21 22:20 ` Daniel Lezcano
2012-03-22 18:36 ` Kevin Hilman
2012-03-22 21:45 ` Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=873991d3hs.fsf@ti.com \
--to=khilman@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox