Hi Guenter,
On Tue, 31 May 2011 07:15:20 -0700, Guenter Roeck wrote:
> Commit a321cedb12904114e2ba5041a3673ca24deb09c9 excludes CPU models 0xe, 0xf,
> and 0x16 from TjMax temperature adjustment, even though several of those CPUs
> are known to have TiMax other than 100 degrees C, and even though the code in
> adjust_tjmax() explicitly handles those CPUs and points to a Web document
> listing several of the affected CPU IDs.
Good catch. Tested on a Core Duo T2600 (model ID 0xe), your patch gets
rid of the "TjMax is assumed as 100 C!" warning message.
> Reinstate TjMax adjustment for CPUs with model ID 0xe, 0xf, and 0x16.
But then why not 0x1a as well? The documentation lists a number of
Nehalem-based processors with TjMax 90 or 105°C. It seems that
adjust_tjmax() would figure it out properly, so we should just call it?
What about 0x1e (Lynnfield)? It was already supported prior to
Carsten's changes. I don't know when MSR_IA32_TEMPERATURE_TARGET was
introduced, but again I see no reason why adjust_tjmax() wouldn't be
good for Lynnfield - at least as good as blindly assuming TjMax at
100°C.
BTW, I am curious why we read MSR_IA32_TEMPERATURE_TARGET on CPU models
which we know do _not_ have this MSR? This leads to a warning message
in the logs. It would seem more logical to call adjust_tjmax() directly
if MSR_IA32_TEMPERATURE_TARGET isn't supported.
> Signed-off-by: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx>
> Cc: Huaxu Wan <huaxu.wan@xxxxxxxxxxxxxxx>
> Cc: Carsten Emde <C.Emde@xxxxxxxxx>
> Cc: Valdis Kletnieks <valdis.kletnieks@xxxxxx>
> Cc: Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
> Cc: Yong Wang <yong.y.wang@xxxxxxxxxxxxxxx>
> Cc: Rudolf Marek <r.marek@xxxxxxxxxxxx>
> ---
> Reason for sending this as RFC is that I don't know if the original change to
> exclude the affected CPU IDs from adjustment was made on purpose or not. Please
> let me know if you recall the reasons for this exclusion.
If anybody knows, that would be Carsten, as he wrote the code. Me, as
the original commit description did NOT mention the change, AND we got
at least one complaint about the regression, I wouldn't hesitate to
apply your proposed fix. I even think it should go to stable@xxxxxxxxxxx
So, this version or one covering more models, is:
Acked-by: Jean Delvare <khali@xxxxxxxxxxxx>
>
> drivers/hwmon/coretemp.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> index de3d246..897a257 100644
> --- a/drivers/hwmon/coretemp.c
> +++ b/drivers/hwmon/coretemp.c
> @@ -308,12 +308,12 @@ static int get_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev)
> */
>
> switch (c->x86_model) {
> - case 0xe:
> - case 0xf:
> - case 0x16:
> case 0x1a:
> dev_warn(dev, "TjMax is assumed as 100 C!\n");
> return 100000;
> + case 0xe:
> + case 0xf:
> + case 0x16: /* Core 2 CPUs */
> case 0x17:
> case 0x1c: /* Atom CPUs */
> return adjust_tjmax(c, id, dev);
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
