lkml.org 
[lkml]   [2009]   [Jun]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] Request driver inclusion - acer aspire one fan control
From
Hi,

On Thu, Jun 18, 2009 at 1:49 PM, Peter Feuerer<peter@piie.net> 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:
>> http://marc.info/?l=linux-kernel&m=124482114200865
>
> We are talking about patch 0.5.9 and not 0.5.8, are we?
> http://patchwork.kernel.org/patch/30733/mbox/
>
> 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:

suspend:
- set fan to auto

resume:
- 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.

--
Regards/Gruss,
Boris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-06-18 14:49    [from the cache]
©2003-2011 Jasper Spaans