Fix fan speeds on ADM1026

Fix fan speeds on ADM1026

[Philip Pokorny]

Jerome Hsiao @ Arima pointed out to me that the fan speeds from the 
ADM1026 driver were not consistent with other drivers.

Seems that most (all?) drivers assume two pulses per rev from the fans. 
  I expected that users would need to use a "compute" directive to 
modify the fan speed for the number of pulses-per-rev of their fans. 
(All the fans I use are 4 ppr so I'm always having to "compute" a 
correct value.)

But it's not consistent with the other drives...

So here is a patch to fix the ADM1026 so it reports fan speeds 
consistent with the other drivers...

This was done against CVS...

:v)


-- 
Philip Pokorny, Director of Engineering
Tel: 415-358-2635   Fax: 415-358-2646   Toll Free: 888-PENGUIN
PENGUIN COMPUTING, INC.
www.penguincomputing.com
-------------- next part --------------
Index: lm_sensors2/kernel/chips/adm1026.c
=================================RCS file: /home/cvs/lm_sensors2/kernel/chips/adm1026.c,v
retrieving revision 1.1
diff -u -r1.1 adm1026.c
--- lm_sensors2/kernel/chips/adm1026.c	8 Jun 2003 21:33:38 -0000	1.1
+++ lm_sensors2/kernel/chips/adm1026.c	12 Jun 2003 03:13:03 -0000
@@ -29,6 +29,10 @@
                  Fix silly bug in fan_div logic
                  Fix fan_min handling so that 0xff is 0 is 0xff
     2003-05-25   Fix more silly typos...
+    2003-06-11   Change FAN_xx_REG macros to use different scaling
+                 Most (all?) drivers assume two pulses per rev fans
+                 and the old scaling was producing double the RPM's
+                 Thanks to Jerome Hsiao @ Arima for pointing this out.
 */
 
 #include <linux/version.h>
@@ -244,10 +248,11 @@
 #endif
 
 /* FAN speed is measured using 22.5kHz clock and counts for 2 pulses
- *      22500 * 60 * 2 = 2700000
+ *   and we assume a 2 pulse-per-rev fan tach signal
+ *      22500 kHz * 60 (sec/min) * 2 (pulse) / 2 (pulse/rev) = 1350000
  */
-#define FAN_TO_REG(val,div)  ((val)<=0 ? 0xff : SENSORS_LIMIT(2700000/((val)*(div)),1,254))
-#define FAN_FROM_REG(val,div) ((val)=0?-1:(val)=0xff ? 0 : 2700000/((val)*(div)))
+#define FAN_TO_REG(val,div)  ((val)<=0 ? 0xff : SENSORS_LIMIT(1350000/((val)*(div)),1,254))
+#define FAN_FROM_REG(val,div) ((val)=0?-1:(val)=0xff ? 0 : 1350000/((val)*(div)))
 #define DIV_FROM_REG(val) (1<<(val))
 #define DIV_TO_REG(val) ((val)>=8 ? 3 : (val)>=4 ? 2 : (val)>=2 ? 1 : 0)
 
@@ -1117,7 +1122,7 @@
 		for (i = 0 ; i <= 7 ; ++i) {
 			data->fan_min[i]  			    adm1026_read_value(client, ADM1026_REG_FAN_MIN(i));
-			data->fan_div[i] = 1 << (value & 0x03);
+			data->fan_div[i] = DIV_FROM_REG(value & 0x03);
 			value >>= 2 ;
 		}
 

Fix fan speeds on ADM1026

[Jean Delvare]

> Jerome Hsiao @ Arima pointed out to me that the fan speeds from the 
> ADM1026 driver were not consistent with other drivers.
> 
> Seems that most (all?) drivers assume two pulses per rev from the
> fans. 
>   I expected that users would need to use a "compute" directive to 
> modify the fan speed for the number of pulses-per-rev of their fans. 
> (All the fans I use are 4 ppr so I'm always having to "compute" a 
> correct value.)
> 
> But it's not consistent with the other drives...
> 
> So here is a patch to fix the ADM1026 so it reports fan speeds 
> consistent with the other drivers...
> 
> This was done against CVS...

Well, that's what fanX_div is meant for. As far as I know, the number of
pulse/rotation depends mostly on the fan itself, not on the monitoring
chipset. That's why we let the users change the fanX_div value through
the config file.

Maybe there's actually something to be fixed in the adm1026 driver, but
maybe this is just a matter of setting the right fanX_div for unregular
fans.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

Fix fan speeds on ADM1026

[Philip Pokorny]

Jean Delvare wrote:
>>Jerome Hsiao @ Arima pointed out to me that the fan speeds from the 
>>ADM1026 driver were not consistent with other drivers.
>>
>>Seems that most (all?) drivers assume two pulses per rev from the
>>fans. 
>>  I expected that users would need to use a "compute" directive to 
>>modify the fan speed for the number of pulses-per-rev of their fans. 
>>(All the fans I use are 4 ppr so I'm always having to "compute" a 
>>correct value.)
>>
>>But it's not consistent with the other drives...
>>
>>So here is a patch to fix the ADM1026 so it reports fan speeds 
>>consistent with the other drivers...
>>
>>This was done against CVS...
> 
> 
> Well, that's what fanX_div is meant for. As far as I know, the number of
> pulse/rotation depends mostly on the fan itself, not on the monitoring
> chipset. That's why we let the users change the fanX_div value through
> the config file.
> 
> Maybe there's actually something to be fixed in the adm1026 driver, but
> maybe this is just a matter of setting the right fanX_div for unregular
> fans.

No, fan_div doesn't work that way in practice.

Most (all?) drivers include fan_div in the conversion from counts to 
RPM.  That means that when you change fan_div the RPM reading stays the 
same.

In practice, changing fan_div affects the minimum detectable fan speed 
and the precision of the measured fan speed.

With a 22.5kHz clock, and assuming 2 pulse/rev fans, you get the following:

                   RPM
                Min    Typ    Significant Bits in Count
     Fan Div    254    153     3000   4400   10k 4ppr
     -------- ------ ------   ------ ------  ---------
        1      2657   4411     7.81   7.26     5.08
        2      1329   2206     6.81   6.26     4.08
        4       664   1103     5.81   5.26     3.08
        8       332    551     4.81   4.26     2.08

At a fan_div setting of 8, the 10k, 4ppr fan is only gating 4 counts on 
average giving fan readings of:  14063 (3), 10547 (4), 8437 (5) 
depending on how many counts you got that sample.  Clearly, you want to 
set the fan_div as low as possible and still detect your fans.

:v)

-- 
Philip Pokorny, Director of Engineering
Tel: 415-358-2635   Fax: 415-358-2646   Toll Free: 888-PENGUIN
PENGUIN COMPUTING, INC.
www.penguincomputing.com

Fix fan speeds on ADM1026

[Mark Studebaker]

doc/fan-divisors also has a good explanation

Philip Pokorny wrote:
> 
> Jean Delvare wrote:
> >>Jerome Hsiao @ Arima pointed out to me that the fan speeds from the
> >>ADM1026 driver were not consistent with other drivers.
> >>
> >>Seems that most (all?) drivers assume two pulses per rev from the
> >>fans.
> >>  I expected that users would need to use a "compute" directive to
> >>modify the fan speed for the number of pulses-per-rev of their fans.
> >>(All the fans I use are 4 ppr so I'm always having to "compute" a
> >>correct value.)
> >>
> >>But it's not consistent with the other drives...
> >>
> >>So here is a patch to fix the ADM1026 so it reports fan speeds
> >>consistent with the other drivers...
> >>
> >>This was done against CVS...
> >
> >
> > Well, that's what fanX_div is meant for. As far as I know, the number of
> > pulse/rotation depends mostly on the fan itself, not on the monitoring
> > chipset. That's why we let the users change the fanX_div value through
> > the config file.
> >
> > Maybe there's actually something to be fixed in the adm1026 driver, but
> > maybe this is just a matter of setting the right fanX_div for unregular
> > fans.
> 
> No, fan_div doesn't work that way in practice.
> 
> Most (all?) drivers include fan_div in the conversion from counts to
> RPM.  That means that when you change fan_div the RPM reading stays the
> same.
> 
> In practice, changing fan_div affects the minimum detectable fan speed
> and the precision of the measured fan speed.
> 
> With a 22.5kHz clock, and assuming 2 pulse/rev fans, you get the following:
> 
>                    RPM
>                 Min    Typ    Significant Bits in Count
>      Fan Div    254    153     3000   4400   10k 4ppr
>      -------- ------ ------   ------ ------  ---------
>         1      2657   4411     7.81   7.26     5.08
>         2      1329   2206     6.81   6.26     4.08
>         4       664   1103     5.81   5.26     3.08
>         8       332    551     4.81   4.26     2.08
> 
> At a fan_div setting of 8, the 10k, 4ppr fan is only gating 4 counts on
> average giving fan readings of:  14063 (3), 10547 (4), 8437 (5)
> depending on how many counts you got that sample.  Clearly, you want to
> set the fan_div as low as possible and still detect your fans.
> 
> :v)
> 
> --
> Philip Pokorny, Director of Engineering
> Tel: 415-358-2635   Fax: 415-358-2646   Toll Free: 888-PENGUIN
> PENGUIN COMPUTING, INC.
> www.penguincomputing.com

Fix fan speeds on ADM1026

[Mark D. Studebaker]

I have the via datasheets and other info,
the scale factor for the via chips is correct.

Philip Pokorny wrote:
> Doh...  I guess I should RTFM next time...
> 
> A quick grep surprised me.  Almost all of the chips use a 22.5kHz clock for
> fan sensing.  I didn't expect that.  The exceptions are:
> 
> 1. The lm85 which has a 16-bit fan counter and 90kHz clock
> 2. The SMSC47m1 which has some funky preload thing going on
> 3. The VIA VT1211 and VT8231 which use 1310720 instead of 1350000
> 
> I wonder if the VIA scale factor (2^17 * 10) is a mistake.  Does anyone 
> have a
> datasheet for those chips?
> 
> :v)
> 
> Mark Studebaker wrote:
> 
>  > doc/fan-divisors also has a good explanation
>  >
>  > Philip Pokorny wrote:
>  >
>  >>Jean Delvare wrote:
>  >>
>  >>>>Jerome Hsiao @ Arima pointed out to me that the fan speeds from the
>  >>>>ADM1026 driver were not consistent with other drivers.
>  >>>>
>  >>>>Seems that most (all?) drivers assume two pulses per rev from the
>  >>>>fans.
>  >>>> I expected that users would need to use a "compute" directive to
>  >>>>modify the fan speed for the number of pulses-per-rev of their fans.
>  >>>>(All the fans I use are 4 ppr so I'm always having to "compute" a
>  >>>>correct value.)
>  >>>>
>  >>>>But it's not consistent with the other drives...
>  >>>>
>  >>>>So here is a patch to fix the ADM1026 so it reports fan speeds
>  >>>>consistent with the other drivers...
>  >>>>
>  >>>>This was done against CVS...
>  >>>>
>  >>>
>  >>>Well, that's what fanX_div is meant for. As far as I know, the 
> number of
>  >>>pulse/rotation depends mostly on the fan itself, not on the monitoring
>  >>>chipset. That's why we let the users change the fanX_div value through
>  >>>the config file.
>  >>>
>  >>>Maybe there's actually something to be fixed in the adm1026 driver, but
>  >>>maybe this is just a matter of setting the right fanX_div for unregular
>  >>>fans.
>  >>>
>  >>No, fan_div doesn't work that way in practice.
>  >>
>  >>Most (all?) drivers include fan_div in the conversion from counts to
>  >>RPM.  That means that when you change fan_div the RPM reading stays the
>  >>same.
>  >>
>  >>In practice, changing fan_div affects the minimum detectable fan speed
>  >>and the precision of the measured fan speed.
>  >>
>  >>With a 22.5kHz clock, and assuming 2 pulse/rev fans, you get the 
> following:
>  >>
>  >>                   RPM
>  >>                Min    Typ    Significant Bits in Count
>  >>     Fan Div    254    153     3000   4400   10k 4ppr
>  >>     -------- ------ ------   ------ ------  ---------
>  >>        1      2657   4411     7.81   7.26     5.08
>  >>        2      1329   2206     6.81   6.26     4.08
>  >>        4       664   1103     5.81   5.26     3.08
>  >>        8       332    551     4.81   4.26     2.08
>  >>
>  >>At a fan_div setting of 8, the 10k, 4ppr fan is only gating 4 counts on
>  >>average giving fan readings of:  14063 (3), 10547 (4), 8437 (5)
>  >>depending on how many counts you got that sample.  Clearly, you want to
>  >>set the fan_div as low as possible and still detect your fans.
>  >>
>  >>:v)
>  >>
>  >>--
>  >>Philip Pokorny, Director of Engineering
>  >>Tel: 415-358-2635   Fax: 415-358-2646   Toll Free: 888-PENGUIN
>  >>PENGUIN COMPUTING, INC.
>  >>www.penguincomputing.com
>  >>
>  >
> 
> 
>