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

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 3 Jun 2009 23:24:01 +0200 Peter Feuerer <peter@piie.net> wrote:

> Acerhdf is a driver for Acer Aspire One netbooks. It allows to access
> the temperature sensor and to control the fan.
> 
> Signed-off-by: Peter Feuerer <peter@piie.net>
> Reviewed-by: Borislav Petkov <petkovbb@gmail.com>
> Tested-by: Borislav Petkov <petkovbb@gmail.com>
> 
>
> ...
>
> +/*
> + * if you want the module to be started in kernel mode,
> + * define the following:
> + */
> +#undef START_IN_KERNEL_MODE

What does this mean?

afacit this functionality is already there at runtime via the
module/boot parameter, so we don't need the compile-time knob?

> +#define VERSION "0.5.5"

That must be a record version number for an initial submission :)

> +/*
> + * According to the Atom N270 datasheet,
> + * (http://download.intel.com/design/processor/datashts/320032.pdf) the
> + * CPU's optimal operating limits denoted in junction temperature as
> + * measured by the on-die thermal monitor are within 0 <= Tj <= 90. So,
> + * assume 89__C is critical temperature.
> + */
> +#define ACERHDF_TEMP_CRIT 89
> +#define ACERHDF_FAN_AUTO 1
> +#define ACERHDF_FAN_OFF 0
> +
> +/*
> + * No matter what value the user puts into the fanon variable, turn on the fan
> + * at 80 degree Celsius to prevent hardware damage
> + */
> +#define ACERHDF_MAX_FANON 80
> +
> +/*
> + * Maximal interval between to temperature checks is 20 seconds, as the die
> + * can get hot really fast under heavy load
> + */
> +#define ACERHDF_MAX_INTERVAL 20
> +
> +#define ACERHDF_ERROR -0xffff

Gad.  I had to write a C program to check that.  0xffff0001.  I wonder
if the compiler treats this as a signed or unsigned thing.

Can this be simplified?

> +
> +
> +#ifdef START_IN_KERNEL_MODE
> +static int kernelmode = 1;
> +#else
> +static int kernelmode;
> +#endif
> +
> +static unsigned int interval = 10;
> +static unsigned int fanon = 63;
> +static unsigned int fanoff = 58;
> +static unsigned int verbose;
> +static unsigned int fanstate = ACERHDF_FAN_AUTO;
> +static int disable_kernelmode;
> +static int bios_version = -1;
> +static char force_bios[16];
> +static unsigned int prev_interval;
> +struct thermal_zone_device *acerhdf_thz_dev;
> +struct thermal_cooling_device *acerhdf_cool_dev;
> +struct platform_device *acerhdf_device;
> +
> +module_param(kernelmode, uint, 0);
> +MODULE_PARM_DESC(kernelmode, "Kernel mode on / off");

What's kernel mode?  This should be explained in the code somewhere so
I'm the last to ask.

>
> ...
>
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Peter Feuerer");
> +MODULE_DESCRIPTION("Aspire One temperature and fan driver");
> +MODULE_ALIAS("dmi:*:*Acer*:*:");
> +MODULE_ALIAS("dmi:*:*Gateway*:*:");
> +MODULE_ALIAS("dmi:*:*Packard Bell*:*:");

It's a nice-looking driver.