login
Search
Hi, I updated the acerhdf module, it uses now the polling functionality of the thermal api. kind regards, --peter 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> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 284ebac..d1bf882 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -34,6 +34,25 @@ config ACER_WMI If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M here. +config ACERHDF + tristate "Acer Aspire One temperature and fan driver" + depends on THERMAL + depends on THERMAL_HWMON + ---help--- + This is a driver for Acer Aspire One netbooks. It allows to access + the temperature sensor and to control the fan. + + The driver is started in "user" mode where the Bios takes care about + controlling the fan, unless a userspace program controls it. + To let the kernelmodule handle the fan, do: + echo kernel > /sys/class/thermal/thermal_zone0/mode + + For more information about this driver see + <http://piie.net/files/acerhdf_README.txt> + + If you have an Acer Aspire One netbook, say Y or M + here. + config ASUS_LAPTOP tristate "Asus Laptop Extras (EXPERIMENTAL)" depends on ACPI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index e40c7bd..581b44f 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_COMPAL_LAPTOP) += compal-laptop.o obj-$(CONFIG_DELL_LAPTOP) += dell-laptop.o obj-$(CONFIG_DELL_WMI) += dell-wmi.o obj-$(CONFIG_ACER_WMI) += acer-wmi.o +obj-$(CONFIG_ACERHDF) += acerhdf.o obj-$(CONFIG_HP_WMI) += hp-wmi.o obj-$(CONFIG_TC1100_WMI) += tc1100-wmi.o obj-$(CONFIG_SONY_LAPTOP) += sony-laptop.o diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c new file mode 100644 index 0000000..63dc485 --- /dev/null +++ ...
Sorry, forgot to modify the Maintainers. Here is the new patch with Maintainers entry. The patch is compiled and tested against current git/torvalds/linux-2.6.git checkout. What do you think? Do you have any questions? kind regards, --peter 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> diff --git a/MAINTAINERS b/MAINTAINERS index ef03abe..0fc8f06 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -222,6 +222,13 @@ L: linux-acenic@sunsite.dk S: Maintained F: drivers/net/acenic* +ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER +P: Peter Feuerer +M: peter@piie.net +W: http://piie.net/?section=acerhdf +S: Maintained +F: drivers/platform/x86/acerhdf.c + ACER WMI LAPTOP EXTRAS P: Carlos Corbacho M: carlos@strangeworlds.co.uk diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 284ebac..d1bf882 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -34,6 +34,25 @@ config ACER_WMI If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M here. +config ACERHDF + tristate "Acer Aspire One temperature and fan driver" + depends on THERMAL + depends on THERMAL_HWMON + ---help--- + This is a driver for Acer Aspire One netbooks. It allows to access + the temperature sensor and to control the fan. + + The driver is started in "user" mode where the Bios takes care about + controlling the fan, unless a userspace program controls it. + To let the kernelmodule handle the fan, do: + echo kernel > /sys/class/thermal/thermal_zone0/mode + + For more information about this driver see + <http://piie.net/files/acerhdf_README.txt> + + If you have an Acer Aspire One netbook, say Y or M + here. + config ASUS_LAPTOP tristate "Asus Laptop Extras (EXPERIMENTAL)" depends on ACPI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index ...
On Sat, Apr 25, 2009 at 10:42:51AM +0200, Peter Feuerer wrote: This could be clearer. In user mode the fan will be controlled by the Perhaps only do this if verbose mode is enabled? 5 lines of output for This is the default behaviour, right? So that's another 5 lines by default. I don't think it's really necessary :) I don't have an Aspire One to hand so can't test this, but otherwise it looks pretty good. -- Matthew Garrett | mjg59@srcf.ucam.org --
Hi Matthey, thanks for your email. A modified patch will follow soon. I'll leave it with the define and add the kernelmode variable to the parameters. This way it's easy for people who want to compile the module with kernelmode enabled and it's also easy for those who simply want to load I need this hack for the new "polling" way of the thermal api. I can't disable polling from outside. I can change the polling delay, but the next polling shot is already assigned. So when I switch on the fan (to BIOS mode), this assigned last shot will just switch it off again (if the temperature is low). And then neither the BIOS nor the kernel module care about the fan anymore :-( The variable just prevents the last shot from I added these lines as I got lot's of emails when I set the user mode to default. People were complaining that the module wouldn't work, but they just didn't know how to switch to kernel mode and were too lazy to read the code or the documentation. So I think this information should stay there. I can try to cut it to 2 lines. Or do you have another idea to manage this? best regards, --peter --
Hi,
I did some testing on my Aspire One machine here and it looks pretty
nice: while compiling a kernel watched the fan going on when the
temperature reaches 67-68 (I don't think this is Celsius though, no?)
and then turning itself off when temp goes below 60.
See below for comments on the code.
This allows for the user to potentially melt his CPU by entering a too high
value. You should check that in the acerhdf_init() against the max allowed
according to spec, I gather it is 67?
#define MAX_FANON_TEMP 67
if (fanon > MAX_FANON_TEMP)
fanon = MAX_FANON_TEMP;
let's put a prefix to all those function names, otherwise their names
are all too generic:
get_temp -> acerhdf_get_temp
change_fanstate -> acerhdf_change_fanstate
you can wrap all those "if (verbose)"-checks in a macro making the code more
readable:
#define acerhdf_printk(fmt, args...) \
if (unlikely(verbose)) \
printk(KERN_NOTICE, fmt, ## args);
and then in the code you do:
too unreadable.
how about:
u8 cmd = (state) ? bios_settings[bios_version].cmd_auto
: bios_settings[bios_version].cmd_off;
those printk's shouldn't depend on verbose since this is important info
IMHO and I want to know that I've changed modes successfully and that my
you need error handling here:
if (*state < 0) {
*state = 0xffff;
return 1;
}
return 0;
or some other invalid value similar to how it's done in get_temp()
let's have defines for those fan states
#define ACERHDF_FAN_OFF 0
#define ACERHDF_FAN_AUTO 1
and then do
maybe I'm missing something but shouldn't this be enabled by default and
only when the user wants to have acerfand or some other uspace tool do
the controlling, only then turn it off. I'd rather trust this is done
in the kernel instead of some flaky uspace thread which could maybe
--
Regards/Gruss,
Boris.
--
Hi Boris, thank you very much for your email. It is Celsius, the specification of the chipset and the atom core say that the chips are allowed to get 99 degree celsuis hot. So I think 70 degree I will add a maximum temperature, I guess something about 80 degree Celsuis. But anyways, the user can still melt his/her cpu by switching to I think it makes sense to leave it this way, because we don't know, what Hm... I like those "if (verbose)" lines, as you can see directly, when the I think they should only be printed out in verbose mode, as it would flood the log file otherwise. Besides that the people who use this module can Matthew suggested to start the module in usermode, where the BIOS takes care about the fan as long as there is no userspace tool or the user want best regards, --peter --
Hi Peter, Their N270 datasheet (http://download.intel.com/design/processor/datashts/320032.pdf) talks about max Tj (junction temperature) being 90°C for a TDP of 2.5W and if you operate below that limit "functionality and long-term reliability can be expected." :) There's also an internal termtrip sensor which is designed to go off at That's true, the user can do all sorts of damages to the machine but I think it is sensible to catch honest mistakes like mistyping the temperature and then unwillingly killing your hardware. And yes, you shouldn't use the max allowed temp Tj=90 according to the spec which could turn out to be bad choice due to imprecise measurements/latent reaction of software. Instead, a nice safety gap of yes, however, let's do it only when they change it instead of predicting the future :) But when the BIOS "takes care" of the fan, it boils down to the last being always on, no? I'd rather have it when the fan is controlled by the kernel module and gets turned on only when its trip temperature is exceeded. Thanks. -- Regards/Gruss, Boris. --
I have small note: So yes cpu can withstand 90 C, but will rest of the motherboard? (Most other cpus are rated at 60C) Regards, Maxim Levitsky --
That's why the module is started in "user mode" where the bios controls the fan. - The Bios just starts the fan as soon as the temperature is higher than 40 degree celsius. The user must explicitely start "kernel mode" to make the kernel controlling the fan. Maybe we should add a warning somewhere, that it could harm the hardware? regards, --peter --
Then its fine. (I probably won't turn fan off though). I have a aspire one, so I install this driver. one more note thought, you might want to rename it to something else, or maybe merge with acer-wmi? If I understand correctly, this driver also provides temperature reading, which is in my opinion very nice. Thus, this seems more a 'acer aspire extras' Or, maybe even better, maybe you can name it a 'EC extra driver', since today many notebooks are controlled via EC, and thus you or other contributors might add more features to this driver. How about a EC register dump for example in /sys, so users could look at EC registers comfortably But of course, I think this driver should first be merged. Another small note, is would be very nice to have a locking mechanism for EC reads/writes, so there would be no conflicts between accesses. For example my main notebook, acer 5720, has semi broken fan control (they broke it in newer bioses, but I have to use them, because fan won't work at all in 64-bit OS I use (linux of course). There is a way to control the fan, by writing fake temperatures to EC and doing that very often, since I think SMI writes there too. But when I do so, ofnen other EC related things fail, like brightness control for example. I think this is due to the fact that acer_ec does direct I/O. so I would like to see a interface to read/write EC properly. Best regards, and thanks, Maxim Levitsky --
Hi, here's a new patch, compiled and tested with current linus git state, what's your opinion about it? cheers, --peter 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> diff --git a/MAINTAINERS b/MAINTAINERS index ef03abe..0fc8f06 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -222,6 +222,13 @@ L: linux-acenic@sunsite.dk S: Maintained F: drivers/net/acenic* +ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER +P: Peter Feuerer +M: peter@piie.net +W: http://piie.net/?section=acerhdf +S: Maintained +F: drivers/platform/x86/acerhdf.c + ACER WMI LAPTOP EXTRAS P: Carlos Corbacho M: carlos@strangeworlds.co.uk diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 284ebac..c4fce42 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -34,6 +34,24 @@ config ACER_WMI If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M here. +config ACERHDF + tristate "Acer Aspire One temperature and fan driver" + depends on THERMAL && THERMAL_HWMON + ---help--- + This is a driver for Acer Aspire One netbooks. It allows to access + the temperature sensor and to control the fan. + + The driver is started in "user-mode" where the Bios takes care about + controlling the fan, unless a userspace program controls it. + To let the module handle the fan, do: + echo kernel > /sys/class/thermal/thermal_zone0/mode + + For more information about this driver see + <http://piie.net/files/acerhdf_README.txt> + + If you have an Acer Aspire One netbook, say Y or M + here. + config ASUS_LAPTOP tristate "Asus Laptop Extras (EXPERIMENTAL)" depends on ACPI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index e40c7bd..641b8bf 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_COMPAL_LAPTOP) += ...
Hi, this has definitely improved since the last one, thanks. See below for comments. Which spec exactly, URL? The Atom N270 datasheet (http://download.intel.com/design/processor/datashts/320032.pdf) says the CPU's optimal operating limits wrt to junction temperature as measured by the on-die thermal monitor is 0 <= Tj <= 90. Is this the temperature you mean? How do you come up with the value of 89°C no, this way is ugly and causes confusion. Define your own macros instead: #define acerhdf_printk(loglevel, fmt, args) \ printk(loglevel "acerhdf: " fmt, args); #define acerhdf_notice(fmt, args...) \ acerhdf_printk(KERN_NOTICE, fmt, ## args); #define acerhdf_err(fmt, args...) \ acerhdf_printk(KERN_ERR, fmt, ## args); and then do acerhdf_notice("interval changed to: %d\n", interval); but you still assume that the ON is 1 and OFF is 0 and are not using the constants you've defined above. How about the following: if (state == ACERHDF_FAN_AUTO) cmd = bios_settings[bios_version].cmd_auto; else cmd = bios_settings[bios_version].cmd_off; ec_write(bios_settings[bios_version].fanreg, cmd); fanstate = state; how about a max interval also, just in case, similar to the MAX_FANON thingy as before, those shouldn't be verbose - they're important enough to how about prefixes to all those functions too? This way it is confusing and the function names are too generic, possibly leading to conflicts why are we changing the fan state if kernelmode is off? If I'm not you should be checking old_state == ACERHDF_ERROR You're assuming here again that state AUTO is 1 (true value) and state OFF os 0 (false value) which makes the code hard to read and understand. The thermal_sys.c code is also broken because it is using naked values too. How about: if (old_state == ACERHDF_FAN_AUTO) { /* * turn fan off only if below fanoff temperature */ if ((state == ACERHDF_FAN_OFF) && (acerhdf_get_temp() < ...
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: 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 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 --
Hi, new patch attached. Compiled and tested with latest linus git. Any further suggestions? kind regards, --peter 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> diff --git a/MAINTAINERS b/MAINTAINERS index c547f4a..262d721 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -222,6 +222,13 @@ L: linux-acenic@sunsite.dk S: Maintained F: drivers/net/acenic* +ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER +P: Peter Feuerer +M: peter@piie.net +W: http://piie.net/?section=acerhdf +S: Maintained +F: drivers/platform/x86/acerhdf.c + ACER WMI LAPTOP EXTRAS P: Carlos Corbacho M: carlos@strangeworlds.co.uk diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 284ebac..c4fce42 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -34,6 +34,24 @@ config ACER_WMI If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M here. +config ACERHDF + tristate "Acer Aspire One temperature and fan driver" + depends on THERMAL && THERMAL_HWMON + ---help--- + This is a driver for Acer Aspire One netbooks. It allows to access + the temperature sensor and to control the fan. + + The driver is started in "user" mode where the Bios takes care about + controlling the fan, unless a userspace program controls it. + To let the module handle the fan, do: + echo kernel > /sys/class/thermal/thermal_zone0/mode + + For more information about this driver see + <http://piie.net/files/acerhdf_README.txt> + + If you have an Acer Aspire One netbook, say Y or M + here. + config ASUS_LAPTOP tristate "Asus Laptop Extras (EXPERIMENTAL)" depends on ACPI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index e40c7bd..641b8bf 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_COMPAL_LAPTOP) += ...
Hi, On Thu, May 07, 2009 at 12:17:02AM +0200, Peter Feuerer wrote: [..] 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. I think I've corrected that one already but it somehow got dropped: BIOS is usually written in all caps and not "Bios." Also, no need to write "user" mode where you actually mean: When the driver is loaded, the BIOS is in control of the fan. To actually activate the kernel module's control over it, do: Please formulate that more precisely: 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, you can remove those /* START_IN_KERNEL_MODE */ comments because the remove those /****... */ splitters please, they're screaming :). A obviously, no need for the comment since the function name says exactly align those ...
Hello, The idea of not starting the module in kernel mode was from Matthew. And he stated that it could harm the hardware when software controls the fan instead of the BIOS. It may also be possible, that the warranty gets invalid when you do that. Not sure about how acer would handle a defect which could be caused by overheating and when they detect that software was controlling The main reason to do this in kernel was the availabilty of atomic ec- read and write functions. But I agree with you that either kernel or BIOS should control the fan and not a userspace tool. I added the user mode just because it wasn't really much more code than just an implementation of the After reading and thinking about all this a while, I agree with you. In the This may really be updated if they release newer AO version. But the time Good idea. kind regards, --peter --
Hi,
We actually don't have any reliable source for the temperature envelope
of the surrounding hardware, right? Quick search didn't reveal anything
here. I only came across a bunch of freeware tools which do fan control
of the aspire ones but all the temperature trip points for the fan in
those were ranging from 50 - 63° which leads to think that those all are
kinda "common sense"-settings the authors came up with without any hard
data from the manufacturer.
For example, mine has a Seagate Momentum ATA-8 ST9160310AS hdd and its
spec[¹] says:
"2.10.1 Ambient temperature
Ambient temperature is defined as the temperature of the environment
immediately surrounding the drive. Actual drive case temperature should
not exceed 65°C (149°F) within the operating ambient conditions."
From what I could measure here empirically, the fan starts in low RPM
mode at around 37°C and and gets cranked up to max when the temp
reaches ~55°C. This envelope in the BIOS code is taking surrounding
devices into consideration, I guess and am wondering whether the 67°C
setting in your driver is still within safe limits, hmmm?
It seems that the BIOS setting is lower/safer so, yes, you're right, we
don't want to void any warranties, so the BIOS control _should_ actually
be the default since we are not at all sure whether moving the envelope
up the temperature scale won't hurt the hardware.
/me would love to see some reliable info on that from the manufacturer...
[..]
Thanks for your work.
¹http://www.seagate.com/staticfiles/support/disc/manuals/notebook/momentus/5400.4/SATA/100468842a.pdf
--
Regards/Gruss,
Boris.
--
Hi, sorry for the delay, was quite busy the last days. New patch attached. I hope I didn't forget anything you suggested. As usual patched against and tested with current linus git. How does it work with those "Signed-off-by" and "Tested-by" lines, should I add you to the list of Signed-off people? Or is this done by the person who merges the patch? kind regards and many thanks for your help, --peter Signed-off-by: Peter Feuerer <peter@piie.net> diff --git a/MAINTAINERS b/MAINTAINERS index 2b349ba..ded6b0c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -222,6 +222,13 @@ L: linux-acenic@sunsite.dk S: Maintained F: drivers/net/acenic* +ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER +P: Peter Feuerer +M: peter@piie.net +W: http://piie.net/?section=acerhdf +S: Maintained +F: drivers/platform/x86/acerhdf.c + ACER WMI LAPTOP EXTRAS P: Carlos Corbacho M: carlos@strangeworlds.co.uk diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 284ebac..48b7362 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -34,6 +34,23 @@ config ACER_WMI If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M here. +config ACERHDF + tristate "Acer Aspire One temperature and fan driver" + depends on THERMAL && THERMAL_HWMON + ---help--- + This is a driver for Acer Aspire One netbooks. It allows to access + the temperature sensor and to control the fan. + + After loading this driver the BIOS controls still the fan. + To let the kernel handle the fan, do: + echo -n enabled > /sys/class/thermal/thermal_zone0/mode + + For more information about this driver see + <http://piie.net/files/acerhdf_README.txt> + + If you have an Acer Aspire One netbook, say Y or M + here. + config ASUS_LAPTOP tristate "Asus Laptop Extras (EXPERIMENTAL)" depends on ACPI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index e40c7bd..641b8bf 100644 --- ...
Could you please not create new prefixes for printk? I think: #define pr_fmt(fmt) "acerhdf:" fmt and use of pr_err and pr_notice would be better. cheers, Joe --
Hi Joe, I could do that. Just curious, is there any deeper reason? Or does it just feel wrong in some matter? best regards, --peter --
No deeper reason than there's an existing source convention that does what you need. Might as well use it. cheers, Joe --
Hi, There's an excellent manual explaining all that at length: <Documentation/development-process/> and especially <Documentation/development-process/5.Posting> and <Documentation/SubmittingPatches, Section 14> which answers the particular question you have. I could try to rephrase that here but Ok, minor nitpicks below but it starting to shape up quite ok. You could send it for inclusion upstream. Reviewed-by: Borislav Petkov <petkovbb@gmail.com> the BIOS is still in control of the fan * define the following: */ #undef START_IN_KERNEL_MODE let's make all those module parameters unsigned just in case because you can write negative values to them and acerhdf_check_param() won't catch invalid values: diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c index 99ae2e6..d4fc5d3 100644 --- a/drivers/platform/x86/acerhdf.c +++ b/drivers/platform/x86/acerhdf.c @@ -83,32 +83,32 @@ #ifdef START_IN_KERNEL_MODE -static int kernelmode = 1; +static unsigned int kernelmode = 1; #else -static int kernelmode; +static unsigned int kernelmode; #endif -static int interval = 10; -static int fanon = 65; -static int fanoff = 60; -static int verbose; -static int fanstate = ACERHDF_FAN_AUTO; +static unsigned int interval = 10; +static unsigned int fanon = 65; +static unsigned int fanoff = 60; +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 int prev_interval; +static unsigned int prev_interval; struct thermal_zone_device *acerhdf_thz_dev; struct thermal_cooling_device *acerhdf_cool_dev; -module_param(kernelmode, int, 0); +module_param(kernelmode, uint, 0); MODULE_PARM_DESC(kernelmode, "Kernel mode on / off"); -module_param(interval, int, 0600); +module_param(interval, uint, 0600); MODULE_PARM_DESC(interval, "Polling interval of temperature check"); -module_param(fanon, int, ...
Hi everybody, new acerhdf patch, as usual patched against and tested with recent linus git tree. I lowered the default trip points to fanon=63 and fanoff=58 as it's more secure and still high enaugh to keep the netbook quiet while idling. kind regards, --peter 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> diff --git a/MAINTAINERS b/MAINTAINERS index 41c6605..bd7617e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -222,6 +222,13 @@ L: linux-acenic@sunsite.dk S: Maintained F: drivers/net/acenic* +ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER +P: Peter Feuerer +M: peter@piie.net +W: http://piie.net/?section=acerhdf +S: Maintained +F: drivers/platform/x86/acerhdf.c + ACER WMI LAPTOP EXTRAS P: Carlos Corbacho M: carlos@strangeworlds.co.uk diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 284ebac..fe14dfd 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -34,6 +34,23 @@ config ACER_WMI If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M here. +config ACERHDF + tristate "Acer Aspire One temperature and fan driver" + depends on THERMAL && THERMAL_HWMON + ---help--- + This is a driver for Acer Aspire One netbooks. It allows to access + the temperature sensor and to control the fan. + + After loading this driver the BIOS is still in control of the fan. + To let the kernel handle the fan, do: + echo -n enabled > /sys/class/thermal/thermal_zone0/mode + + For more information about this driver see + <http://piie.net/files/acerhdf_README.txt> + + If you have an Acer Aspire One netbook, say Y or M + here. + config ASUS_LAPTOP tristate "Asus Laptop Extras (EXPERIMENTAL)" depends on ACPI diff --git a/drivers/platform/x86/Makefile ...
Hi Boris, This is the way Joe suggested, how to use printk's: You define the pr_fmt(fmt) before any includes and you can just do 'pr_notice("Fan control off, to enable:\n");' to printk a notice which looks like this: "acerhdf: Fan control off...". Same with pr_err(...) which prints an error. kind regards, --peter --
Hi,
Ah, this thing. Ok.
...
+static int acerhdf_unbind(struct thermal_zone_device *thermal,
+ struct thermal_cooling_device *cdev)
+{
+ if (cdev != acerhdf_cool_dev)
+ return 0;
+
+ if (thermal_zone_unbind_cooling_device(thermal, 0, cdev)) {
+ pr_err("acerhdf: error unbinding cooling dev\n");
you've missed one here.
+ return -EINVAL;
+ }
+ return 0;
+}
...
--
Regards/Gruss,
Boris.
--
Hi Boris, thanks again for all you helpful comments. A new patch will follow shortly. 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 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 --
Hi,
just rediff it against latest git and send an email to Len Brown (i
assume, from looking at git log drivers/thermal/ output) requesting for
driver inclusion.
Len?
If you hurry and do it this week it might be possible to get it in .31
My reasoning was that because this is called from sysfs and the user
sees nothing happening even if he'd turned off the fan by calling
.set_cur_state that it might be useful to tell him why.
--
Regards/Gruss,
Boris.
--
Hi, Ok, I'll try to directly address len with subject "Request driver inclusion acer aspire one fan control" and send a diff against latest git, right? (CC'ing lkml of course). Just wondering if it'll work, as Len has always But the user isn't allowed to change the fan state from userspace anymore. If you try to change the fan state from userspace you'll get the "changing fan state is not allowed" message. best regards, --peter --
Hi, By the way, the system log is being polluted with that message after a suspend/resume cycle: [99027.020952] acerhdf: failed reading fan state, disabling kernelmode. [99027.520172] ACPI: EC: missing confirmations, switch off interrupt mode. [99047.672142] acerhdf: changing fan state is not allowed [99057.696151] acerhdf: changing fan state is not allowed [99067.720125] acerhdf: changing fan state is not allowed [99077.744164] acerhdf: changing fan state is not allowed [99087.796127] acerhdf: changing fan state is not allowed [99097.820147] acerhdf: changing fan state is not allowed [99107.844136] acerhdf: changing fan state is not allowed [99117.908153] acerhdf: changing fan state is not allowed [99127.932155] acerhdf: changing fan state is not allowed [99137.123893] [drm:i915_get_vblank_counter] *ERROR* trying to get vblank count for disabled pipe 0 [99137.956075] acerhdf: changing fan state is not allowed [99147.980158] acerhdf: changing fan state is not allowed [99158.004180] acerhdf: changing fan state is not allowed [99168.028148] acerhdf: changing fan state is not allowed [99170.207885] [drm:i915_get_vblank_counter] *ERROR* trying to get vblank count for disabled pipe 0 [99178.052149] acerhdf: changing fan state is not allowed [99188.076149] acerhdf: changing fan state is not allowed [99198.100148] acerhdf: changing fan state is not allowed [99208.124150] acerhdf: changing fan state is not allowed [99210.558715] acerhdf: kernelmode ON [99210.581161] acerhdf: changing fan state is not allowed because the suspend/resume cycle is causing the EC error message above. In that case, you shouldn't probably switch off kernel mode but unregister the driver completely until the EC error is fixed (if ever)... Hmm... -- Regards/Gruss, Boris --
Hi, So you think it makes sense to allow it, too? For me it doesn't really matter whether user is allowed to change the fan or not. I think controlling the fan belongs into kernel, but if the user wants to have his own userspace tool I'm fine with this too. So I'll enable userspace control of the fan There's a bug in the algorithm which disables the kernelmode in case of unexpected register value. → It doesn't stop polling. And I will re-add suspend / resume code to get rid of the resume problem. Thanks! best regards, --peter --
Hi Boris, I fixed the resume / suspend issue and re-enabled userspace control. Can you please also have a look if this time the patch isn't line wrapped for you? - I saved the patch from my last submit and applied it, it's working. I also downloaded the diff from http://lkml.org/lkml/2009/6/3/117 and it's working too. regards, peter --- 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> diff --git a/MAINTAINERS b/MAINTAINERS index cf4abdd..e9457fe 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -222,6 +222,13 @@ L: linux-acenic@sunsite.dk S: Maintained F: drivers/net/acenic* +ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER +P: Peter Feuerer +M: peter@piie.net +W: http://piie.net/?section=acerhdf +S: Maintained +F: drivers/platform/x86/acerhdf.c + ACER WMI LAPTOP EXTRAS P: Carlos Corbacho M: carlos@strangeworlds.co.uk diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 284ebac..fe14dfd 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -34,6 +34,23 @@ config ACER_WMI If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M here. +config ACERHDF + tristate "Acer Aspire One temperature and fan driver" + depends on THERMAL && THERMAL_HWMON + ---help--- + This is a driver for Acer Aspire One netbooks. It allows to access + the temperature sensor and to control the fan. + + After loading this driver the BIOS is still in control of the fan. + To let the kernel handle the fan, do: + echo -n enabled > /sys/class/thermal/thermal_zone0/mode + + For more information about this driver see + <http://piie.net/files/acerhdf_README.txt> + + If you have an Acer Aspire One netbook, say Y or M + here. + config ASUS_LAPTOP tristate "Asus Laptop Extras ...
Hi, Unfortunately it is still line-wrapped. Your mailer is sending format=flowed mails: Mime-Version: 1.0 Content-Type: text/plain; format=flowed; charset="UTF-8" Content-Disposition: inline which can cause unexpected line breaks. The diff at http://lkml.org/lkml/diff/2009/6/3/211/1 applies cleanly, though. -- Regards/Gruss, Boris --
Kernels crash, too, just like userspace does. It would still make sense to allow userspace to increase fan speed. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html --
It seems like the fan in the aspire one's is used for cooling the surrounding devices too and while the thermal envelope of the CPU is much wider, the peripherals are much more susceptible to temperatures outside of their allowed operating range. That's why currently the driver lets the BIOS control the fan since its settings are most Well, if the kernel is dead, userspace has already died too. Besides, the module can still be toggled on/off from sysfs. Actually, empirically measured, there seem to be three states of the fan: off, on and on-max where you can hear it rotating at max RPM. The kernel module can handle those completely if you know the respective ACPI EC commands and there's no need for userspace daemon, IMHO. -- Regards/Gruss, Boris --
Yep, I don't disagree. But I strongly suspect that if you force the fan off and overheat the machine, it will shut down in hardware before It would be still nice to let the userspace lower the trip points for maximum flexibility. No need for userspace _daemon_. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html --
Yep, however we won't come that far with the current driver since it relinquishes control of the fan to the BIOS after a critical temp of 89° is reached. I'm still quite unpersuaded about that "magical" number since it is 1 degree below the high interval boundary of the juncture temperature of the Atom CPU. I would like to have a more reliable source for the allowed envelope based not only on the CPU but on the whole I'm sure Peter wouldn't mind adding some other sysfs entries in future versions of the driver controlling exactly that. -- Regards/Gruss, Boris. --
Hi, I think we are fine the way it is now. The driver allows to read the temperature by default and people who know what they are doing can easily You can already change the trip points via the /sys/module/acerhdf/parameters/* files on the fly. cheers, --peter --
There's no other thermal device_mode state than ENABLED/DISABLED. No printing here either. You could #define pr_fmt(fmt) "acerhd: " fmt instead of prefixing all the printks with "acerhd:" --
Hi Joe, Jip, will do something like this. kind regards, --peter --
