[lm-sensors] New sysfs attribute for fan control: fan pulses per

[lm-sensors] New sysfs attribute for fan control: fan pulses per

[Guenter Roeck]

Hi all,

while working on NCT6776F support, I realized that there is a fan control
attribute which is not currently supported by the syfs ABI: The number of
fan pulses per revolution. On the NCT6776F, this can be configured in Bank 6,
registers 0x44 to 0x46. Possible values are 0..3 for 4, 1, 2, or 3 pulses 
per revolution.

I encountered the same parameter when working on PMBus devices. At the time,
I thought this was a variant of a fan divisor (ie a divisor of 1, 2, 3, or 4),
but apparently it is different and independent of the fan divisor.

Question is if it would make sense to merge support for this attribute into fan[1-*]_div,
or if we should define a new attribute specifically for fan pulses per revolution.

Merging it into fan[1-*]_div would mean we would have to permit new values for it,
and we would need a more complex description. Changing the value could be ambiguous
if there is ever a chip which supports both fan divisor and pulse/revolution
configuration registers (NCT6776F and PMBus devices don't, so we would be safe there).

A new attribute (fanX_pulses ? fanX_ppr ?) would be more straightforward,
but ... it would be a new attribute.

Any thoughts ?

Thanks,
Guenter
 

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] New sysfs attribute for fan control: fan pulses

[Phillip Susi]

On 2/22/2011 10:54 PM, Guenter Roeck wrote:
> Hi all,
> 
> while working on NCT6776F support, I realized that there is a fan control
> attribute which is not currently supported by the syfs ABI: The number of
> fan pulses per revolution. On the NCT6776F, this can be configured in Bank 6,
> registers 0x44 to 0x46. Possible values are 0..3 for 4, 1, 2, or 3 pulses 
> per revolution.
> 
> I encountered the same parameter when working on PMBus devices. At the time,
> I thought this was a variant of a fan divisor (ie a divisor of 1, 2, 3, or 4),
> but apparently it is different and independent of the fan divisor.

How is it any different than the fan divisor?

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] New sysfs attribute for fan control: fan pulses

[Guenter Roeck]

On Wed, 2011-02-23 at 11:29 -0500, Phillip Susi wrote:
> On 2/22/2011 10:54 PM, Guenter Roeck wrote:
> > Hi all,
> > 
> > while working on NCT6776F support, I realized that there is a fan control
> > attribute which is not currently supported by the syfs ABI: The number of
> > fan pulses per revolution. On the NCT6776F, this can be configured in Bank 6,
> > registers 0x44 to 0x46. Possible values are 0..3 for 4, 1, 2, or 3 pulses 
> > per revolution.
> > 
> > I encountered the same parameter when working on PMBus devices. At the time,
> > I thought this was a variant of a fan divisor (ie a divisor of 1, 2, 3, or 4),
> > but apparently it is different and independent of the fan divisor.
> 
> How is it any different than the fan divisor?

The divisor affects count granularity, since the fan speed reported by
the chip has to be multiplied with the divisor value to compute the
actual fan speed.
	speed = <reported value> * <divisor>
My understanding is that it is mainly used to increase the supported
speed range if there is a limited number of bits in the speed count
register.

Pulses per revolution tells the chip how many pulses per fan revolution
to expect for an accurate fan speed reading. The reported fan speed is
corrected by the chip based on this parameter, and the driver does not
have to do any fan speed calculations.
	speed = <reported value>

One could of course play around by always configuring the chip to a
fixed value for pulses/revolution and then use SW to calculate the real
speed. But it seems to me that would only add code and thus risk, and
not provide any real benefits (other maybe to be able to claim that
pulses/revolution is some kind of disguised fan divisor).

Thanks,
Guenter



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] New sysfs attribute for fan control: fan pulses

[Jean Delvare]

Hi Guenter,

On Tue, 22 Feb 2011 19:54:14 -0800, Guenter Roeck wrote:
> while working on NCT6776F support, I realized that there is a fan control
> attribute which is not currently supported by the syfs ABI: The number of
> fan pulses per revolution. On the NCT6776F, this can be configured in Bank 6,
> registers 0x44 to 0x46. Possible values are 0..3 for 4, 1, 2, or 3 pulses 
> per revolution.
> 
> I encountered the same parameter when working on PMBus devices. At the time,
> I thought this was a variant of a fan divisor (ie a divisor of 1, 2, 3, or 4),
> but apparently it is different and independent of the fan divisor.
> 
> Question is if it would make sense to merge support for this attribute into fan[1-*]_div,
> or if we should define a new attribute specifically for fan pulses per revolution.

I remember discussing this years ago. It's so old that I don't remember
the details and couldn't find the discussion in the archive. I seem to
recall that I proposed to introduce a new attribute for this, and was
replied (don't remember by whom) that it was just a variant of fan
clock divisors and it didn't need special handling.

I suspect this might be true for some devices. It all depends on how
the pulse-per-revolution register works. It could be informative only
(to let the BIOS or pin wrapping set it) and the driver must take the
value into account. Or it could be handled by the device itself, in
which case the device may or may not adjust the clock frequency to
stick to a range of measurements (in other words, changing the
pulse-per-revolution value may influence the divisor or not.)

I suspect that the lack of proper support for this feature is the
result of several factors: few devices support it, few users need it
(the vast majority of consumer fans emit 2 PPR) and handling the
correction is easily done in user-space with a compute statement in the
libsensors config file. It's even documented in our FAQ.

> Merging it into fan[1-*]_div would mean we would have to permit new values for it,
> and we would need a more complex description. Changing the value could be ambiguous
> if there is ever a chip which supports both fan divisor and pulse/revolution
> configuration registers (NCT6776F and PMBus devices don't, so we would be safe there).

FWIW, some of the LM85-compatible devices (ADM1027 at least) have a
pulse-per-revolution register at 0x7b. The Linux 2.6 driver lacks
support for it, but the Linux 2.4 driver included it:
http://www.lm-sensors.org/browser/lm-sensors/branches/lm-sensors-2.10/kernel/chips/lm85.c

Interestingly, this family of chips is one of the oldest with more than
8 bits for fan speed values, and thus doesn't have a fan clock divisor
mechanism. So apparently this setting is mutually exclusive with fan
div on all devices we know.

> A new attribute (fanX_pulses ? fanX_ppr ?) would be more straightforward,
> but ... it would be a new attribute.

There's nothing wrong with new attributes :)

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors