Re: [ibm-acpi-devel] Fan level 7 after resume wit 2.6.28-rc3
From: Tino Keitel
Date: Thu Nov 06 2008 - 03:23:30 EST
On Thu, Nov 06, 2008 at 01:35:44 +0100, Tino Keitel wrote:
[...]
> I tried to write a correct patch, but I got lost in all that
> fan_control_desired_level, fan_control_initial_status and
> tp_features.fan_ctrl_status_undef stuff.
>
> My brain says that one would just read the current fan settings from
> the EC at initialization. Then, after resume, this setting is restored
> unconditionally, or at least if it differs from current_level. The
> attached patch works for me. However, I don't have all the knowledge
> about older models and their specific behaviour.
>
> Correction: I just tested a bit further, and it doesn't work. If I set
> fan level to 3, suspend, resume, set fan level to auto, and
> resume/suspend again, fan level is restored to 3. This is because
> fan_control_desired_level isn't updated by fan_update_desired_level()
> if it is set back to auto, but kept at the old value. So, my impression
> is that all the values and states should be cleaned up a bit and
> simplified. In the current state, there are a lot of strage checks and
> quirks that have side effects when other parts are changed.
The whole fan level stuff looks a bit complicated to me. Especially the
fan_control_desired_level handling is somethat strange because it
considers values as invalid that are written by fan_set_level() before.
It ignores the TP_EC_FAN_FULLSPEED and TP_EC_FAN_AUTO values. Now, in
fan_resume(), this value is checked against TP_EC_FAN_FULLSPEED which
makes no sense to me.
The attached patch tries to simplify this a bit. It sets
fan_control_desired_level to the current EC value in fan_init(), and
also tries to keep fan_control_desired_level and the real EC value in
sync. I also removed the checks against 7 and TP_EC_FAN_FULLSPEED in
fan_resume() as they made no sense to me. This works as intended on my
X61s after rmmod/insmod and suspend/resume.
Regards,
Tino
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index 4db1cf9..7f095c4 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -5329,13 +5329,8 @@ TPACPI_HANDLE(sfan, ec, "SFAN", /* 570 */
*/
static void fan_update_desired_level(u8 status)
{
- if ((status &
- (TP_EC_FAN_AUTO | TP_EC_FAN_FULLSPEED)) == 0) {
- if (status > 7)
- fan_control_desired_level = 7;
- else
- fan_control_desired_level = status;
- }
+ fan_control_desired_level = status &
+ (TP_EC_FAN_AUTO | TP_EC_FAN_FULLSPEED | 7);
}
static int fan_get_status(u8 *status)
@@ -5886,6 +5881,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
if (likely(acpi_ec_read(fan_status_offset,
&fan_control_initial_status))) {
fan_status_access_mode = TPACPI_FAN_RD_TPEC;
+ fan_update_desired_level(fan_control_initial_status);
/* In some ThinkPads, neither the EC nor the ACPI
* DSDT initialize the fan status, and it ends up
@@ -6025,9 +6021,8 @@ static void fan_resume(void)
break;
case TPACPI_FAN_WR_ACPI_FANS:
case TPACPI_FAN_WR_TPEC:
- do_set = ((saved_fan_level & TP_EC_FAN_FULLSPEED) ||
- (saved_fan_level == 7 &&
- !(current_level & TP_EC_FAN_FULLSPEED)));
+ do_set = (current_level != saved_fan_level) &&
+ !(current_level & TP_EC_FAN_FULLSPEED);
break;
default:
return;