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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Jul 13, 2012 at 02:23:01PM +0200, Daniel Lezcano wrote:
> 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.

Like in milliwatts?
> 
> 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.

Go for it. Looking at the Xen hypervisor code it just assigns the value
to its own structure and does nothing to it.

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