Re: [PATCH 5/5] Export the temperatures via hwmon
From: Guenter Roeck
Date: Fri Aug 08 2014 - 12:30:21 EST
On 08/08/2014 07:54 AM, Goffredo Baroncelli wrote:
On 08/07/2014 11:19 PM, Matt Helsley wrote:
On Thu, Aug 07, 2014 at 07:50:00PM +0200, Goffredo Baroncelli wrote:
On 08/07/2014 09:36 AM, Guenter Roeck wrote:
On 08/06/2014 11:52 PM, Jean Delvare wrote:
Hi Guenter,
On Wed, 06 Aug 2014 23:20:32 -0700, Guenter Roeck wrote:
Patch 4/5 is "Return the fan speed via sysfs: /sys/devices/temperature/fan_level".
So you are saying that returning the fan speed with a non-hwmon attribute works,
but returning it with a hwmon attribute doesn't ? Not really sure if I understand
your logic. Either fan_level doesn't return the fan speed (or an abstraction of it),
or something in your line of argument is inconsistent.
fan_level is a fan speed _control_ value, like pwm1. It is not a fan
speed monitoring value.
Ah, ok. The patch description doesn't seem to match, though.
And why not export it as pwm1, if that is what it is ?
Guenter
the exported fan_level value is a coefficient near proportional to the speed [*];
so it is not the speed nor the pwm.
I tried to read the pwm/speed value, but when I did it, every 5/6 seconds the
fan seemed to stop for 1s, then the speed raised.... So I stopped the test.
These patches (the first two) solved a real issue: with the last kernels this
driver doesn't work at all, and the fan go to maximum speed (very loud !)
The other three are an improvement.
When (if) these patches will be accepted I want to write another solution,
but definitely not now. And even if it would work for me, it is very likely
that will be accepted because nobody is able to test it on all hardware.
BR
G.Baroncelli
[*] It is a bit more complicated: basically the adm1030 controls the fan speed
on the basis of an external temperature sensor (the "case" sensor).
Higher is this temperature higher is the fan speed.
The pwm of the fan is computed on the basis of the following value:
- min_temp
- range_temp -> max_temp=min_temp+range_temp
- min_pwm_value
and of course
- "case" sensor
The fan_level is an index computed on the basis of the "cpu" temperature (
which is different from the "case" temperature referred above). This
temperature is used to update the min_temp above.
So the fan speed id computed on the basis of two external sensor:
- cpu sensor
- case sensor.
To complicate further the things, the range_temp is set to an undocumented
value: this parameter is set via a register (0x25, bit 2:0), which accepted
values between 0x00 and 0x04. However it is set to 0x07
See http://lxr.free-electrons.com/source/drivers/macintosh/therm_windtunnel.c#L153
and the adm1030 datasheet.
I am reluctant to change this thing because this driver is for an obsolete
platform (apple stopped the powermac in 2004/2005), and bad or good I have to
suppose that it works. Moreover I am not in the position to test the changes
outside my hardware (1 powermac DP@1Ghz mdd)
This seems like a valuable explanation of the meaning of the fan_level
values. When I read the code I was wondering what the magic number "11" was
doing for instance and this helps clear it up a little. It's a nit perhaps
but some of this information might be useful in the commit message for that
patch, the code, or both.
I will take in account your suggestion. In case of another revision, I will improve the comment.
I just noticed - hwmon_device_register_with_groups() returns ERR_PTR
on errors, not NULL, so checking for a NULL return value won't help
much if there is an error.
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/