Re: [PATCH 4/5] acpi : remove power from acpi_processor_cx structure

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 07/13/2012 02:17 AM, Konrad Rzeszutek Wilk wrote:
> On Thu, Jul 12, 2012 at 11:03:45PM +0200, Daniel Lezcano wrote:
>> Remove the power field as it is not used.
>>
> It looks to be used in drivers/xen/xen-acpi-processor.c.
> 
> I could emulate some value and stick it in there.. but I am
> more curious - what is the intent of this value?

At the first glance, this value is the power consumption of the
specified state. I am not sure all acpi returns a correct value.

I can imagine the power should be copied to the cpuidle_state structure
to the power field where it is used by the governor to choose the better
C-state. As it is not specified, cpuidle will assume the C-State N
consumes less than the C-State N-1.

If we want to add the power consumption we should also set the
'power_specified' flag for the driver, but that could change the
behavior of the cpuidle driver.

Anyway, IMO, this field is useless for this structure and should be
specified later, if that makes sense, directly in the cpuidle_state
structure like the other drivers do.

If nobody complains, I will remove the field also from Xen and resend
this patch.

Thanks
  -- Daniel

>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/acpi/processor_idle.c |    2 --
>>  include/acpi/processor.h      |    1 -
>>  2 files changed, 0 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index d044588..99ba58f 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -485,8 +485,6 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr)
>>  		if (obj->type != ACPI_TYPE_INTEGER)
>>  			continue;
>>  
>> -		cx.power = obj->integer.value;
>> -
>>  		current_count++;
>>  		memcpy(&(pr->power.states[current_count]), &cx, sizeof(cx));
>>  
>> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
>> index 0957457..87bb9d7 100644
>> --- a/include/acpi/processor.h
>> +++ b/include/acpi/processor.h
>> @@ -58,7 +58,6 @@ struct acpi_processor_cx {
>>  	u32 address;
>>  	u8 entry_method;
>>  	u32 latency;
>> -	u32 power;
>>  	u64 time;
>>  	u8 bm_sts_skip;
>>  	char desc[ACPI_CX_DESC_LEN];
>> -- 
>> 1.7.5.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
 <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

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html