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