Re: Avoid critical temp shutdowns on specific ThinkPad T4x(p) and R50p

[Thomas Renninger <trenn-l3A5Bk7waGM@public.gmane.org>]

On Monday 11 August 2008 13:30:05 Andi Kleen wrote:
> Do we have a final patch for this yet? I assume it needs at least more
> entries in the DMI list as Henrique pointed out?

On longterm I like to try to fix this in a more generic way:

Use Matthew's basic idea/patch of introducing a passive trip point 5 degree 
below the critical trip point and set a polling frequency.
And also (this is what covers the ThinkPads) lower the trip point if the 
passive trip point is too close to the critical one (and also set a polling 
frequency).

I expect 5 degree below critical tp is not enough, IMO it's better to use e.g. 
8-10 degree (you never want to work that close at the critical temperture) 
and do not use that high polling freq instead.

However, the patch from Matthew, Zhang pointed me to, is rather broken.
Beside a typo that prevents compilation, pointer types are mixed up (char vs 
unsigned long):
---------------
include/linux/thermal.h
struct thermal_zone_device_ops {
..
	int (*get_temp) (struct thermal_zone_device *, char *);
..
}
static void thermal_update(struct work_struct *work)
{
...
	unsigned long temp;
		tz->ops->get_temp(tz, &temp);
...
}
---------------
So Matthew's patch more looks like a suggestion/proof of concept patch.
Another bug I saw by looking over this one:
+			tz->ops->get_crit_temp(tz, &crit_temp);
+			tz->force_passive_temp = crit_temp-5000;
It must get checked whether a critical trip point exists at all.

This might also have to do with the fact that his first attempts have been 
rejected...

Anyway, I like to pick this up and improve as suggested above, what makes me 
wonder about the thermal_sys.c driver are the char* based device_ops to get 
temperatures.
All (with exception to the critical temp, which is even more weird) 
temperature handling functions return char*. This looks convenient for sysfs 
processing, but it's use should be rather rare (especially if more code gets 
added)?
Instead of char->long temperature conversions every time one gets a temp from 
the thermal_sys driver, shouldn't these functions better return long and you 
have one long->char conversion for sysfs?

If Matthew already has something newer it would be great if we could somehow 
coordinate work. Otherwise I will start over on this one. Probably beginning 
with a cleanup patch to use long for get_temp and get_trip_temp functions 
instead of using char* in struct thermal_zone_device_ops {...}.
Zhang: Would that be ok or have I overseen something?


Is this a bug in current drivers/thermal/Kconfig code:
config THERMAL_HWMON
        bool "Hardware monitoring support"
        depends on HWMON=y || HWMON=THERMAL
Should the last line be:
        depends on HWMON
?

      Thomas

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/