the more I'm looking at the driver, the more I get annoyed by that
user/kernel mode operation split. Remind me again why the driver should
be loaded and not started automatically but the user should be required
to activate it explicitly?
That's not so optimal, I'd say. The kernel module should _replace_
the userspace program, not work alongside it, since the last is flaky
and unreliable, and this was the main reason the kernel module was
introduced in the first place - to control the fan from kernel space,
which is the more sane choice.
What is more, if the userspace program would fail, there's no way for
the module to get activated again and jump in instead of the userspace
program to the rescue. Which goes more to show that you don't need
userspace control _at_ _all_ and the only two agents controlling the fan
should be the BIOS or the kernel module.
So I think that the kernel module should take over fan control upon
load. This way you'll be able to get rid of all that needless complexity
around kernelmode/disable_kernelmode and have a simple clean design.
+
+ if (!force_bios[0]) {
+ /* check if product is a AO - Aspire One */
+ if (strncmp(product, "AO", 2)) {
I suppose this check will have to be updated if Acer release newer
Aspire One versions which product names are not AOXXXX anymore, right?
But who knows, they might fix the fan in their A1 versions :)
+ /* search BIOS version and BIOS vendor in BIOS settings table */
+ for (i = 0; bios_settings[i].version[0]; ++i) {
+ if (!strcmp(bios_settings[i].vendor, vendor) &&
+ !strcmp(bios_settings[i].version, version)) {
+ bios_version = i;
+ break;
+ }
+ }
+ if (bios_version == -1) {
+ acerhdf_error("cannot find BIOS version\n");
+ ret_val = -ENODEV;
+ goto EXIT;
+ }
you could split this function since logically you do basic checking of hardware
and below this line you register with the thermal core so the rest of that
function could go into another one, something like:
static int acerhdf_register(int mode) {
...
}
and then call it in the acerhdf_init() like that:
return acerhdf_register(kernelmode);