On 11/20/2013 05:14 PM, Rafael J. Wysocki wrote:
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'll remove this patch for now. I think what makes sense is a
separate patch to harden the use of pr->throttling.state_count
in processor_throttling.c and processor_thermal.c. I see a
couple of places where the values may not make sense when it
is zero (regardless of reduced HW mode) but I think I'd like
to treat that as a separate issue and think it through some
more.
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone@xxxxxxxxxx
-----------------------------------
--
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
