lkml.org 
[lkml]   [2013]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/4] intel_idle: stop using driver_data for static flags
On 02/01/2013 03:44 AM, Daniel Lezcano wrote:
> On 02/01/2013 05:11 AM, Len Brown wrote:
>> From: Len Brown <len.brown@intel.com>
>>
>> The commit, 4202735e8ab6ecfb0381631a0d0b58fefe0bd4e2
>> (cpuidle: Split cpuidle_state structure and move per-cpu statistics fields)
>> observed that the MWAIT flags for Cn on every processor to date were the
>> same, and created get_driver_data() to supply them.
>>
>> Unfortunately, that assumption is false, going forward.
>> So here we restore the MWAIT flags to the cpuidle_state table.
>> However, instead restoring the old "driver_data" field,
>> we put the flags into the existing "flags" field,
>> where they probalby should have lived all along.
>
> Removing the driver_data is a good thing but I am not sure it is the
> case by moving the MWAIT flags to the cpuidle_state's flags field. We
> are mixing arch specific flags with a generic code.
>
> This is prone to errors because new flags could appear for the cpuidle
> core code and could collide with the arch specific flags.
>
> Wouldn't make sense to add a private field in the struct cpuidle_state
> structure to let the driver/arch specific to store whatever is needed ?
>
> struct cpuidle_state {
>
> ...
> unsigned long private;
> ...
>
> }

The top half of the flags are reserved for the driver,
as noted by the definition of CPUIDLE_DRIVER_FLAGS_MASK
with the generic flag definitions:

#define CPUIDLE_FLAG_TIME_VALID (0x01) /* is residency time measurable? */
#define CPUIDLE_FLAG_COUPLED (0x02) /* state applies to multiple cpus */

#define CPUIDLE_DRIVER_FLAGS_MASK (0xFFFF0000)

intel_idle already uses a driver-specific flag:

#define CPUIDLE_FLAG_TLB_FLUSHED 0x10000

This patch just uses 4 more bits along with that one.

Sure, if we run out of space, we can add an additional field.
But I don't see an immediate need for it.

thanks,
Len Brown
Intel Open Source Technology Center



\
 
 \ /
  Last update: 2013-02-01 20:22    [W:0.062 / U:1.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site