lkml.org 
[lkml]   [2009]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] Acer Aspire One Fan Control
    Date
    Hi Boris,

    thank you very much! A modified patch with your suggestions will follow
    as soon as I tested it. Just some things I would like to discuss:

    Borislav Petkov writes:
    >> +/* change current fan state - is overwritten when running in kernel mode */
    >> +static int set_cur_state(struct thermal_cooling_device *cdev,
    >> + unsigned long state)
    >> +{
    >> + int old_state;
    >> +
    >> + /* let the thermal layer disable kernelmode. This ensures that
    >> + * the thermal layer doesn't switch off the fan again */
    >> + if (disable_kernelmode) {
    >> + acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
    >> + disable_kernelmode = 0;
    >> + kernelmode = 0;
    >> + return 0;
    >> + }
    >> +
    >> + if (!kernelmode) {
    >> + acerhdf_change_fanstate(state);
    >> + return 0;
    >> + }
    >
    > why are we changing the fan state if kernelmode is off? If I'm not
    > mistaken, the BIOS should be controlling the fan here.

    The user can control the fan in userspace (by e.g. echoing 0 to the
    /sys/class/thermal.../cur_state file) then this function is called. This is
    useful if somebody wants to program an userspace tool to handle the
    temperature and the fan.

    >> + old_state = acerhdf_get_fanstate();
    >> +
    >> + /* if reading the fan's state returns unexpected value, there's a
    >> + * problem with the ec register. -> let the BIOS take control of
    >> + * the fan to prevent hardware damage */
    >> + if (old_state != fanstate) {
    >
    > you should be checking
    >
    > old_state == ACERHDF_ERROR
    >
    > here instead.

    Comparing to fanstate is better. It implys the ACERHDF_ERROR check, as
    fanstate can only be ACERHDF_FAN_AUTO or ACERHDF_FAN_OFF. And on the other
    Hand there will be thrown an error too, if the ec register contains an
    unexpected value. So it is more failsafe this way.

    What do you think will be the next steps to get the kernel into mainline?
    Matthew said I should CC Len Brown, as he is responsible to include
    the module. But Len didn't write anything yet :-(

    kind regards,
    --peter


    \
     
     \ /
      Last update: 2009-05-06 21:43    [W:3.789 / U:0.152 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site