From: Len Brown <lenb@kernel.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
Lists Linaro-dev <linaro-dev@lists.linaro.org>
Subject: Re: [PATCH 1/4] intel_idle: stop using driver_data for static flags
Date: Sat, 02 Feb 2013 15:18:22 -0500 [thread overview]
Message-ID: <510D748E.4010605@kernel.org> (raw)
In-Reply-To: <510CDFF1.80408@linaro.org>
>> I think that a driver's private flag definitions
>> should remain local to the driver. It makes no sense
>> to pollute the name space of other drivers for stuff
>> that doesn't mean anything to them. MWAIT is pretty
>> specific to x86 -- and re-naming it to something 'generic'
>> isn't going to make the code easier to read.
>
> Ok, let me rephrase it because I think how it was presented was not clear.
>
> As we want to use the half of the state flags for private purpose, I
> suggested to add the encoding/decoding function in the shared header file.
>
> The mwait eax flags are not encoded and the CPUIDLE_FLAG_TLB_FLUSHED is
> encoded.
>
> I suggested to unify both and to use an encoding function from the
> shared header file.
>
> #define CPUIDLE_PRIVATE_FLAGS(_flags_) \
> ((_flags_) << 16) & CPUIDLE_DRIVER_FLAGS_MASK
>
> For example:
>
> #define FLAG_TLB_FLUSHED CPUIDLE_PRIVATE_FLAGS(0x1)
> #define FLAG_MWAIT_C1 CPUIDLE_PRIVATE_FLAGS(0x0)
> #define FLAG_MWAIT_C2 CPUIDLE_PRIVATE_FLAGS(0x10)
> #define FLAG_MWAIT_C3 CPUIDLE_PRIVATE_FLAGS(0x20)
> #define FLAG_MWAIT_C4 CPUIDLE_PRIVATE_FLAGS(0x30)
> #define FLAG_MWAIT_C5 CPUIDLE_PRIVATE_FLAGS(0x40)
> #define FLAG_MWAIT_C6 CPUIDLE_PRIVATE_FLAGS(0x52)
>
> And then:
>
> ...
>
> .flags = FLAG_TLB_FLUSHED | FLAG_MWAIT_C2 | CPUIDLE_FLAG_TIME_VALID
I like your syntax better than what I used -- thanks for suggesting it.
I can't use it as-is, however, because I really do need to be able to
support unique parameters on each entry (eg. different flags for C3
on different processors). When some of the logical and functional
changes in flux settle out I'll come back to re-visit the syntax.
thanks,
-Len Brown, Intel Open Source Technology Center
prev parent reply other threads:[~2013-02-02 20:18 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-01 4:11 intel_idle and turbostat patches supporting Haswell Len Brown
2013-02-01 4:11 ` [PATCH 1/4] intel_idle: stop using driver_data for static flags Len Brown
2013-02-01 4:11 ` [PATCH 2/4] tools/power turbostat: support HSW Len Brown
2013-02-01 4:11 ` [PATCH 3/4] tools/power turbostat: decode MSR_IA32_POWER_CTL Len Brown
2013-02-01 4:11 ` [PATCH 4/4] intel_idle: initial HSW support Len Brown
2013-02-01 8:44 ` [PATCH 1/4] intel_idle: stop using driver_data for static flags Daniel Lezcano
2013-02-01 18:40 ` Len Brown
2013-02-01 22:16 ` Daniel Lezcano
2013-02-02 2:16 ` Len Brown
[not found] ` <510C7708.4080707-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-02-02 9:44 ` Daniel Lezcano
2013-02-02 20:18 ` Len Brown [this message]
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=510D748E.4010605@kernel.org \
--to=lenb@kernel.org \
--cc=daniel.lezcano@linaro.org \
--cc=linaro-dev@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).