| Message ID | 1342127026-1526-4-git-send-email-daniel.lezcano@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Thursday, July 12, 2012, Daniel Lezcano wrote: > Remove the power field as it is not used. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Looks good to me. Thanks, Rafael > --- > 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]; > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Right, the field is assigned but not used AFAICT. dst_cx->power = cx->power; Did you see it used in another place ? Thanks -- Daniel > I could emulate some value and stick it in there.. but I am > more curious - what is the intent of this value? > > >> 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
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
On Friday, July 13, 2012, 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. > > 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. Please do. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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];
Remove the power field as it is not used. 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(-)