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

From: Borislav Petkov
Date: Thu Jun 18 2009 - 08:45:54 EST


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
>>>>> dropped.
>>>> and it is needed, because...?
>>> It's needed because we do now a clean revert to bios mode before we
>>> suspend.
>>> And after resume we have to switch to kernelmode again, if the driver was
>>> in
>>> 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:

- set fan to auto

- the thermal layer figures out what to do based on the 'kernelmode' and
current temperature.

That's it, everything else is too much.

I'll have a deeper look during the weekend.

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