Bug 11520
| Summary: | eeepc_laptop hwmon missing name | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | Florian Heimgaertner (florian-bug-kernel) |
| Component: | Hardware Monitoring | Assignee: | Corentin Chary (corentin.chary) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | jdelvare |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.26 | Subsystem: | |
| Regression: | --- | Bisected commit-id: | |
| Attachments: |
eeepc-laptop: add the name attr
Add the right name attr Add the right name attr Fix the hwmon interface |
||
|
Description
Florian Heimgaertner
2008-09-08 04:58:04 UTC
Corentin, please provide a patch for this bug. There was a patch on the ml, but it was broken. I'll resend a new one soon. Created attachment 17968 [details]
eeepc-laptop: add the name attr
Here is a patch, sory for the time it took me to send 3 lignes of code.
"fan" is way too generic as a hwmon device name. You'll have to come up with something better than that. libsensors uses the name attribute to differentiate between hardware monitoring device types. "eeepc" would do, I guess. Note that dashes are prohibited. Florian, any chance you can test Corentin's patch and report? In case other things need to be fixed in the eeepc-laptop driver. Created attachment 17969 [details]
Add the right name attr
Here is a new patch ...
Looks good, thank you. Assuming that I can add your Signed-off-by to this patch, I'll push it upstream. Hmmm, you didn't even try building the code? drivers/misc/eeepc-laptop.c:533:57: error: macro "SENSOR_DEVICE_ATTR" requires 5 arguments, but only 4 given drivers/misc/eeepc-laptop.c:533: warning: type defaults to 'int' in declaration of 'SENSOR_DEVICE_ATTR' drivers/misc/eeepc-laptop.c:539: error: 'sensor_dev_attr_name' undeclared here (not in a function) Please provide a patch that builds properly, with your Signed-off-by. Created attachment 17972 [details]
Add the right name attr
Here is the right patch ... again.
Sorry for the last, I tried to build asus-laptop instead -_-.
Hop this is all ok now. Anyway, it build.
(In reply to comment #8) > Created an attachment (id=17972) [details] > Add the right name attr > > Here is the right patch ... again. Ok, tested with 2.6.26.4. lm-sensors now shows the fan speed. pwmconfig/fancontrol still don't work, they apparently don't like "virtual" sensors... pwmconfig and fancontrol do not care about virtual vs. real sensors. All they care about is that the driver respects the standard sysfs interface for hwmon drivers. Which apparently the eeepc-laptop doesn't... apparently it uses values 0-100 for PWM duty cycles, where the standard interface says 0-255 should be used. So scaling is missing in the driver. Note that fancontrol requires a temperature value so that it can compute the target fan speed. Apparently the eeepc-laptop module doesn't provide a temperature value, so unless there is another thermal sensor driver on the machine, it is expected that pwmconfig and fancontrol won't work. pwmconfig is looking at /sys/class/hwmon/hwmon*/device, which doesn't exist for eeepc-laptop. btw: according to a comment in pwmconfig fan[1-9]_pwm is deprecated, so fan1_pwm should be renamed to pwm1 (In reply to comment #11) > pwmconfig is looking at /sys/class/hwmon/hwmon*/device, which doesn't exist > for eeepc-laptop. Ah, you're correct. This problem has been addressed in libsensors some times ago already, but not in pwmconfig. I've just fixed that: http://www.lm-sensors.org/browser/lm-sensors/branches/lm-sensors-3.0.0/prog/pwm/pwmconfig?rev=5343&format=raw Can you please give it a try? > btw: according to a comment in pwmconfig fan[1-9]_pwm is deprecated, so > fan1_pwm should be renamed to pwm1 You're totally right. Corentin, can you please attach a patch fixing both the scaling and the pwm file name? Thanks. No problem, there will be a patch monday. I don't know why there is no "device" file. Should'nt hwmon_device_register(dev) do that ? hwmon_device_register(dev) should indeed create the device link. Maybe you should check the value returned by acpi_get_physical_device(ehotk->device->handle). It doesn't really matter though, as the attributes we are interested in are those of the hwmon device. I'm currently reinstalling my eeepc as my external hd died. I'll do some test, then send a patch. Created attachment 18144 [details] Fix the hwmon interface Here is another patch. There is still no "device" link, but it's not needed anymore. Now lm-sensors support "virtual" sensors. http://www.lm-sensors.org/ticket/2309 http://www.lm-sensors.org/changeset/5176 Patch looks good, thanks. I've added it to my hwmon tree, and will be sending it to Linus shortly. Florian, can you please test the patch and confirm that it works fine for you too? Fix is in kernel 2.6.27: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=04dcd84bc79d9f756bf5b9fc16c7df3344823ca8 Corentin, you can close this bug. |