On Wednesday, November 20, 2013 02:54:32 PM Al Stone wrote: > On 11/17/2013 03:20 PM, Rafael J. Wysocki wrote: > > On Saturday, November 09, 2013 06:36:19 PM al.stone@xxxxxxxxxx wrote: > >> From: Al Stone <ahs3@xxxxxxxxxx> > >> > >> Signed-off-by: Al Stone <al.stone@xxxxxxxxxx> > >> --- > >> drivers/acpi/processor_throttling.c | 8 ++++++++ > >> 1 file changed, 8 insertions(+) > >> > >> diff --git a/drivers/acpi/processor_throttling.c b/drivers/acpi/processor_throttling.c > >> index e7dd2c1..200738e 100644 > >> --- a/drivers/acpi/processor_throttling.c > >> +++ b/drivers/acpi/processor_throttling.c > >> @@ -942,6 +942,10 @@ static int acpi_processor_get_fadt_info(struct acpi_processor *pr) > >> return -EINVAL; > >> } > >> > >> + /* > >> + * NB: in HW reduced mode, duty_width is always zero > >> + * so this count may not be what is wanted. > >> + */ > >> pr->throttling.state_count = 1 << acpi_gbl_FADT.duty_width; > >> > >> /* > >> @@ -991,6 +995,10 @@ static int acpi_processor_set_throttling_fadt(struct acpi_processor *pr, > >> /* Used to clear all duty_value bits */ > >> duty_mask = pr->throttling.state_count - 1; > >> > >> + /* > >> + * NB: in HW reduced mode, duty_offset is always zero > >> + * so this mask may not be what is wanted. > >> + */ > >> duty_mask <<= acpi_gbl_FADT.duty_offset; > >> duty_mask = ~duty_mask; > >> } > > > > I'm not sure how these comments help to be honest. It looks like > > pr->throttling.state_count should be 0 in HW reduced mode, shouldn't it? > > > > It should. The comments clarified things for me but perhaps > they should just note that these values are always zero in > reduced HW mode. The other option would be to not add any > comments, of course. Hopefully someone working with reduced > HW mode would be aware of these changes to the FADT values. > > I can go either way; what's the preference? I would just avoid making changes until we figure out what to do ultimately here. That depends on what pr->throttling.state_count is used for and that part should be hardened against pr->throttling.state_count == 0. Having done that, we can simply set pr->throttling.state_count = 0 in the HW reduced mode without adding any comments at all. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
