Re: [PATCH] Request driver inclusion - acer aspire one fan control

From: Peter Feuerer
Date: Thu Jun 18 2009 - 09:39:36 EST


Borislav Petkov writes:


On Thu, Jun 18, 2009 at 1:49 PM, Peter Feuerer<peter@xxxxxxxx> wrote:
Actually I think pre_suspend_kernelmode is needed, so it won't be

and it is needed, because...?

It's needed because we do now a clean revert to bios mode before we
And after resume we have to switch to kernelmode again, if the driver was
kernelmode before suspend. So we need to keep track of in what state the
driver was before suspending. That's what's this variable is for.

You've got that state in the 'kernelmode' variable. See full comment:

We are talking about patch 0.5.9 and not 0.5.8, are we?

have a look at at line 543:
+ Â Â Â /* remember previous setting */
+ Â Â Â pre_suspend_kernelmode = kernelmode;
+ Â Â Â if (kernelmode) {
+ Â Â Â Â Â Â Â acerhdf_revert_to_bios_mode();
+ Â Â Â Â Â Â Â if (acerhdf_thz_dev)
+ Â Â Â Â Â Â Â Â Â Â Â thermal_zone_device_update(acerhdf_thz_dev);
+ Â Â Â }

ok, this starts to look quite a bit overengineered for no reason. First,
acerhdf_revert_to_bios_mode() sets the fan to auto. Then, you've added
a thermal_zone_device_update() call in there which does set the fan to
auto indirectly _again_. And we end up with _three_ variables which
represent only _one_ state. Here's what it should do:

You are partly right, setting the fan to auto in "acerhdf_revert_to_bios_mode" can be removed, as this is done by the thermal layer when calling "acerhdf_set_cur_state" with disable_kernelmode=1.
But besides that I think it is a clean way. This way we _completely_ disable the thermal polling before going suspend and ensure the thermal layer doesn't handle the fan anymore. When we resume we let the thermal layer take over the fan again. In my opinion this is much cleaner than just switching the fan to auto without keeping the polling of the thermal layer in mind.
The big problem with the polling is, that you don't know when the next thermal polling shot arrives. Is it after acerhdf_suspend was called, or after system-resume but before acerhdf_resume was called, or after acerhdf_resume was calledâ You can't know! That's why in my opinion completely disabling our kernelmode is the only clean solution.


P.S. I built the official 2.6.30 release (because current git is freezing all the time), patched it with acerhdf 0.5.9 and was testing suspend/resume about 40 times now. Was always hitting the suspend button while working ;). I wasn't able to reproduce the "unexpected fanstate" issue, <=0.5.8 had.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at