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

thanks again for all you helpful comments. A new patch will follow shortly.

Borislav Petkov writes:

> Ok, minor nitpicks below but it starting to shape up quite ok. You could
> send it for inclusion upstream.

How exactly do I send the patch for inclusion?

>> +/* acer ec functions */
>> +static int acerhdf_get_temp(void)
>> +{
>> + u8 temp;
>> + /* read temperature */
>> + if (!ec_read(bios_settings[bios_version].tempreg, &temp)) {
>> + if (verbose)
>
> you need to check the error status here before printing the temperature
> since it might be invalid if the ec_read has failed:
>
> u8 temp;
> int err;
>
> err = ec_read(bios_settings[bios_version].tempreg, &temp);
>
> if (err)
> return ACERHDF_ERROR;
>
> if (verbose)
> acerhdf_notice("temp %d\n", temp);
>
> return temp;
> }
>

The printf was already omitted when ec_read fails the way I wrote it, wasn't
it? - Only if ec_read returns 0, the printf is launched and the temperature
is returned.

>> + acerhdf_notice("temp %d\n", temp);
>> + return temp;
>> + }
>> + return ACERHDF_ERROR;
>> +}
>> +
>> +
>> + if (verbose)
>> + acerhdf_error("read state: %d expected state: %d\n",
>> + old_state, fanstate);
>> +
>> + acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
>> + disable_kernelmode = 1;
>> + }
>> +
>> + if (state == 0) {
>> + /* turn fan off only if below fanoff temperature */
>> + if ((old_state == ACERHDF_FAN_AUTO) &&
>> + (acerhdf_get_temp() < fanoff))
>
> it might be cool to tell the user why you're not turning off the fan.
>
> if (verbose)
> acerhdf_notice("Not turning off fan due to current temp "
> "exceeding fanoff value\n");
>

Hm.. I think it should be clear that the fan is turned off, as soon as the
temperature is below the fanoff temperature. In my opinion printing this
message would be a case for a "verbose==2" verbose mode :)

kind regards,
--peter


\
 
 \ /
  Last update: 2009-06-01 16:23    [W:0.242 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site