Another it87 patch.

Another it87 patch.

Post by Zephaniah E. Hul » Fri, 09 May 2003 10:30:16



This is against my last.

While the old code most definitely did /something/ to the register for
setting the fan div, the 'what' is a more interesting question.

To be honest I could not figure out what it was trying to do, because
the way it was inserting values disagreed with not only the data sheet,
but how it parsed the very same register.

This corrects the issue, and allows one to properly control the divisor
on all 3 fans, including the (much more limited) 3rd fan.

--

           92ED 94E4 B1E6 3624 226D  5727 4453 008B E65A 7801
            CCs of replies from mailing lists are requested.

<Upholder> Seen on the back of a T-Shirt: "I am a bomb technician.  If you see
           me running, try to keep up."

--- old/drivers/i2c/chips/it87.c        2003-05-08 04:16:39.000000000 -0400

                                205-(val)*5)
 #define ALARMS_FROM_REG(val) (val)

-#define DIV_TO_REG(val) ((val)==8?3:(val)==4?2:(val)==1?0:1)
+static int log2(int val)
+{
+    int answer = 0;
+    while ((val >>= 1))
+       answer++;
+    return answer;
+}
+#define DIV_TO_REG(val) log2(val)
 #define DIV_FROM_REG(val) (1 << (val))


        struct i2c_client *client = to_i2c_client(dev);
        struct it87_data *data = i2c_get_clientdata(client);
        int val = simple_strtol(buf, NULL, 10);
-       int old = it87_read_value(client, IT87_REG_FAN_DIV);
-       data->fan_div[nr] = DIV_TO_REG(val);
-       old = (old & 0x0f) | (data->fan_div[1] << 6) | (data->fan_div[0] << 4);
-       it87_write_value(client, IT87_REG_FAN_DIV, old);
+       u8 old = it87_read_value(client, IT87_REG_FAN_DIV);
+
+       switch (nr) {
+           case 0:
+           case 1:
+               data->fan_div[nr] = DIV_TO_REG(val);
+               break;
+           case 2:
+               if (val < 8)
+                   data->fan_div[nr] = 1;
+               else
+                   data->fan_div[nr] = 3;
+       }
+       val = old & 0x100;
+       val |= (data->fan_div[0] & 0x07);
+       val |= (data->fan_div[1] & 0x07) << 3;
+       if (data->fan_div[2] == 3)
+           val |= 0x1 << 6;
+       it87_write_value(client, IT87_REG_FAN_DIV, val);
        return count;
 }

                i = it87_read_value(client, IT87_REG_FAN_DIV);
                data->fan_div[0] = i & 0x07;
                data->fan_div[1] = (i >> 3) & 0x07;
-               data->fan_div[2] = 1;
+               data->fan_div[2] = (i & 0x40) ? 3 : 1;

                data->alarms =
                        it87_read_value(client, IT87_REG_ALARM1) |

  application_pgp-signature_part
< 1K Download
 
 
 

Another it87 patch.

Post by Zephaniah E. Hul » Fri, 09 May 2003 17:00:07


On Thu, May 08, 2003 at 04:25:24AM -0400, Zephaniah E. Hull wrote:
> This is against my last.

And this is against that one.

Ok, after writing up something in the way of a perl script to make some
sense of the data for voltages, and finding that there is no sense to
make, I took a longer look at things.

The it87 driver in 2.5.x is doing some, down right /odd/ math on the
numbers for the in_input* readings, and the 2.4.x driver is doing
something quite different.

And while it might be possible to get sane numbers out of the 2.5.x
driver, people /expect/ to get the numbers that they were getting from
2.4.x.

So this patch puts things back to the simpler calculations done by the
2.4.x lm-sensors drivers, and my script confirms that the numbers come
out right.

(If anyone would like the script, let me know and I'll put it up
somewhere, however it is a messy kludge at the moment.)

--
        1024D/E65A7801 Zephaniah E. Hull <w...@babylon.d2dc.net>
           92ED 94E4 B1E6 3624 226D  5727 4453 008B E65A 7801
            CCs of replies from mailing lists are requested.

"Sir," barked one of those useless aristocratic generals to William
Howard Russell, the great Times war correspondent, "I do not like what
you write." "Then, sir," retorted Russell, "I suggest you do not do what
I write about."

--- linux-2.5.65/drivers/i2c/chips/it87.c       2003-05-08 10:38:23.000000000 -0400
+++ linux-2.5.69-mm1/drivers/i2c/chips/it87.c   2003-05-08 09:59:49.000000000 -0400
@@ -99,46 +99,8 @@

 #define IT87_REG_CHIPID        0x58

-static inline u8 IN_TO_REG(long val, int inNum)
-{
-       /* to avoid floating point, we multiply everything by 100.
-        val is guaranteed to be positive, so we can achieve the effect of
-        rounding by (...*10+5)/10.  Note that the *10 is hidden in the
-        /250 (which should really be /2500).
-        At the end, we need to /100 because we *100 everything and we need
-        to /10 because of the rounding thing, so we /1000.   */
-       if (inNum <= 1)
-               return (u8)
-                   SENSORS_LIMIT(((val * 210240 - 13300) / 250 + 5) / 1000,
-                                 0, 255);
-       else if (inNum == 2)
-               return (u8)
-                   SENSORS_LIMIT(((val * 157370 - 13300) / 250 + 5) / 1000,
-                                 0, 255);
-       else if (inNum == 3)
-               return (u8)
-                   SENSORS_LIMIT(((val * 101080 - 13300) / 250 + 5) / 1000,
-                                 0, 255);
-       else
-               return (u8) SENSORS_LIMIT(((val * 41714 - 13300) / 250 + 5)
-                                         / 1000, 0, 255);
-}
-
-static inline long IN_FROM_REG(u8 val, int inNum)
-{
-       /* to avoid floating point, we multiply everything by 100.
-        val is guaranteed to be positive, so we can achieve the effect of
-        rounding by adding 0.5.  Or, to avoid fp math, we do (...*10+5)/10.
-        We need to scale with *100 anyway, so no need to /100 at the end. */
-       if (inNum <= 1)
-               return (long) (((250000 * val + 13300) / 210240 * 10 + 5) /10);
-       else if (inNum == 2)
-               return (long) (((250000 * val + 13300) / 157370 * 10 + 5) /10);
-       else if (inNum == 3)
-               return (long) (((250000 * val + 13300) / 101080 * 10 + 5) /10);
-       else
-               return (long) (((250000 * val + 13300) / 41714 * 10 + 5) /10);
-}
+#define IN_TO_REG(val)  (SENSORS_LIMIT((((val) * 10 + 8)/16),0,255))
+#define IN_FROM_REG(val) (((val) *  16) / 10)

 static inline u8 FAN_TO_REG(long rpm, int div)
 {
@@ -279,7 +241,7 @@
        struct i2c_client *client = to_i2c_client(dev);
        struct it87_data *data = i2c_get_clientdata(client);
        it87_update_client(client);
-       return sprintf(buf, "%ld\n", IN_FROM_REG(data->in[nr], nr)*10 );
+       return sprintf(buf, "%d\n", IN_FROM_REG(data->in[nr])*10 );
 }

 static ssize_t show_in_min(struct device *dev, char *buf, int nr)
@@ -287,7 +249,7 @@
        struct i2c_client *client = to_i2c_client(dev);
        struct it87_data *data = i2c_get_clientdata(client);
        it87_update_client(client);
-       return sprintf(buf, "%ld\n", IN_FROM_REG(data->in_min[nr], nr)*10 );
+       return sprintf(buf, "%d\n", IN_FROM_REG(data->in_min[nr])*10 );
 }

 static ssize_t show_in_max(struct device *dev, char *buf, int nr)
@@ -295,7 +257,7 @@
        struct i2c_client *client = to_i2c_client(dev);
        struct it87_data *data = i2c_get_clientdata(client);
        it87_update_client(client);
-       return sprintf(buf, "%ld\n", IN_FROM_REG(data->in_max[nr], nr)*10 );
+       return sprintf(buf, "%d\n", IN_FROM_REG(data->in_max[nr])*10 );
 }

 static ssize_t set_in_min(struct device *dev, const char *buf,
@@ -304,7 +266,7 @@
        struct i2c_client *client = to_i2c_client(dev);
        struct it87_data *data = i2c_get_clientdata(client);
        unsigned long val = simple_strtoul(buf, NULL, 10)/10;
-       data->in_min[nr] = IN_TO_REG(val,nr);
+       data->in_min[nr] = IN_TO_REG(val);
        it87_write_value(client, IT87_REG_VIN_MIN(nr),
                        data->in_min[nr]);
        return count;
@@ -315,7 +277,7 @@
        struct i2c_client *client = to_i2c_client(dev);
        struct it87_data *data = i2c_get_clientdata(client);
        unsigned long val = simple_strtoul(buf, NULL, 10)/10;
-       data->in_max[nr] = IN_TO_REG(val,nr);
+       data->in_max[nr] = IN_TO_REG(val);
        it87_write_value(client, IT87_REG_VIN_MAX(nr),
                        data->in_max[nr]);
        return count;
@@ -851,37 +813,37 @@
           This sets fan-divs to 2, among others */
        it87_write_value(client, IT87_REG_CONFIG, 0x80);
        it87_write_value(client, IT87_REG_VIN_MIN(0),
-                        IN_TO_REG(IT87_INIT_IN_MIN_0, 0));
+                        IN_TO_REG(IT87_INIT_IN_MIN_0));
        it87_write_value(client, IT87_REG_VIN_MAX(0),
-                        IN_TO_REG(IT87_INIT_IN_MAX_0, 0));
+                        IN_TO_REG(IT87_INIT_IN_MAX_0));
        it87_write_value(client, IT87_REG_VIN_MIN(1),
-                        IN_TO_REG(IT87_INIT_IN_MIN_1, 1));
+                        IN_TO_REG(IT87_INIT_IN_MIN_1));
        it87_write_value(client, IT87_REG_VIN_MAX(1),
-                        IN_TO_REG(IT87_INIT_IN_MAX_1, 1));
+                        IN_TO_REG(IT87_INIT_IN_MAX_1));
        it87_write_value(client, IT87_REG_VIN_MIN(2),
-                        IN_TO_REG(IT87_INIT_IN_MIN_2, 2));
+                        IN_TO_REG(IT87_INIT_IN_MIN_2));
        it87_write_value(client, IT87_REG_VIN_MAX(2),
-                        IN_TO_REG(IT87_INIT_IN_MAX_2, 2));
+                        IN_TO_REG(IT87_INIT_IN_MAX_2));
        it87_write_value(client, IT87_REG_VIN_MIN(3),
-                        IN_TO_REG(IT87_INIT_IN_MIN_3, 3));
+                        IN_TO_REG(IT87_INIT_IN_MIN_3));
        it87_write_value(client, IT87_REG_VIN_MAX(3),
-                        IN_TO_REG(IT87_INIT_IN_MAX_3, 3));
+                        IN_TO_REG(IT87_INIT_IN_MAX_3));
        it87_write_value(client, IT87_REG_VIN_MIN(4),
-                        IN_TO_REG(IT87_INIT_IN_MIN_4, 4));
+                        IN_TO_REG(IT87_INIT_IN_MIN_4));
        it87_write_value(client, IT87_REG_VIN_MAX(4),
-                        IN_TO_REG(IT87_INIT_IN_MAX_4, 4));
+                        IN_TO_REG(IT87_INIT_IN_MAX_4));
        it87_write_value(client, IT87_REG_VIN_MIN(5),
-                        IN_TO_REG(IT87_INIT_IN_MIN_5, 5));
+                        IN_TO_REG(IT87_INIT_IN_MIN_5));
        it87_write_value(client, IT87_REG_VIN_MAX(5),
-                        IN_TO_REG(IT87_INIT_IN_MAX_5, 5));
+                        IN_TO_REG(IT87_INIT_IN_MAX_5));
        it87_write_value(client, IT87_REG_VIN_MIN(6),
-                        IN_TO_REG(IT87_INIT_IN_MIN_6, 6));
+                        IN_TO_REG(IT87_INIT_IN_MIN_6));
        it87_write_value(client, IT87_REG_VIN_MAX(6),
-                        IN_TO_REG(IT87_INIT_IN_MAX_6, 6));
+                        IN_TO_REG(IT87_INIT_IN_MAX_6));
        it87_write_value(client, IT87_REG_VIN_MIN(7),
-                        IN_TO_REG(IT87_INIT_IN_MIN_7, 7));
+                        IN_TO_REG(IT87_INIT_IN_MIN_7));
        it87_write_value(client, IT87_REG_VIN_MAX(7),
-                        IN_TO_REG(IT87_INIT_IN_MAX_7, 7));
+                        IN_TO_REG(IT87_INIT_IN_MAX_7));
        /* Note: Battery voltage does not have limit registers */
        it87_write_value(client, IT87_REG_FAN_MIN(1),
                         FAN_TO_REG(IT87_INIT_FAN_MIN_1, 2));

  application_pgp-signature_part
< 1K Download

 
 
 

Another it87 patch.

Post by Greg K » Sun, 11 May 2003 00:00:16



> This is against my last.

Thanks, I've applied all 3 of these patches to my tree, and will send
them on.

One minor note though:

Quote:> +static int log2(int val)
> +{
> +    int answer = 0;
> +    while ((val >>= 1))
> +  answer++;
> +    return answer;
> +}

Can you start using the proper kernel coding style of 8 character tabs?
I've fixed up this in your different patches before applying them, but
in the future it would be nicer if I didn't have to.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/