[lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Guenter Roeck]

Assume all three are compatible and have the same functionality.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
This patch depends on the other pending changes for the it87 driver in my
hwmon-next tree.

 Documentation/hwmon/it87 |   24 +++++++----
 drivers/hwmon/it87.c     |   93 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 100 insertions(+), 17 deletions(-)

diff --git a/Documentation/hwmon/it87 b/Documentation/hwmon/it87
index 23b7def..b0c1dcd 100644
--- a/Documentation/hwmon/it87
+++ b/Documentation/hwmon/it87
@@ -30,6 +30,10 @@ Supported chips:
     Prefix: 'it8728'
     Addresses scanned: from Super I/O config space (8 I/O ports)
     Datasheet: Not publicly available
+  * IT8781F/IT8782F/IT8783E/F
+    Prefix: 'it8783'
+    Addresses scanned: from Super I/O config space (8 I/O ports)
+    Datasheet: Not publicly available
   * SiS950   [clone of IT8705F]
     Prefix: 'it87'
     Addresses scanned: from Super I/O config space (8 I/O ports)
@@ -75,7 +79,8 @@ Description
 -----------
 
 This driver implements support for the IT8705F, IT8712F, IT8716F,
-IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E and SiS950 chips.
+IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E, IT8781F, IT8782F,
+IT8783E/F, and SiS950 chips.
 
 These chips are 'Super I/O chips', supporting floppy disks, infrared ports,
 joysticks and other miscellaneous stuff. For hardware monitoring, they
@@ -99,11 +104,11 @@ The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E and later IT8712F revisions
 have support for 2 additional fans. The additional fans are supported by the
 driver.
 
-The IT8716F, IT8718F, IT8720F and IT8721F/IT8758E, and late IT8712F and
-IT8705F also have optional 16-bit tachometer counters for fans 1 to 3. This
-is better (no more fan clock divider mess) but not compatible with the older
-chips and revisions. The 16-bit tachometer mode is enabled by the driver when
-one of the above chips is detected.
+The IT8716F, IT8718F, IT8720F and IT8721F/IT8758E, IT8781F, IT8782F, IT8783E/F,
+and late IT8712F and IT8705F also have optional 16-bit tachometer counters for
+fans 1 to 3. This is better (no more fan clock divider mess) but not compatible
+with the older chips and revisions. The 16-bit tachometer mode is enabled by the
+driver when one of the above chips is detected.
 
 The IT8726F is just bit enhanced IT8716F with additional hardware
 for AMD power sequencing. Therefore the chip will appear as IT8716F
@@ -131,9 +136,10 @@ inputs can measure voltages between 0 and 4.08 volts, with a resolution of
 0.016 volt (except IT8721F/IT8758E and IT8728F: 0.012 volt.) The battery
 voltage in8 does not have limit registers.
 
-On the IT8721F/IT8758E, some voltage inputs are internal and scaled inside
-the chip (in7, in8 and optionally in3). The driver handles this transparently
-so user-space doesn't have to care.
+On the IT8721F/IT8758E/IT8781F/IT8782F/IT8783E/F, some voltage inputs are
+internal and scaled inside the chip (in7 (optional for IT8781/2/3), in8 and
+optionally in3). The driver handles this transparently so user-space doesn't
+have to care.
 
 The VID lines (IT8712F/IT8716F/IT8718F/IT8720F) encode the core voltage value:
 the voltage level your processor should work with. This is hardcoded by
diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index 0b204e4..afaf19b 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -19,6 +19,9 @@
  *            IT8726F  Super I/O chip w/LPC interface
  *            IT8728F  Super I/O chip w/LPC interface
  *            IT8758E  Super I/O chip w/LPC interface
+ *            IT8781F  Super I/O chip w/LPC interface
+ *            IT8782F  Super I/O chip w/LPC interface
+ *            IT8783E/F Super I/O chip w/LPC interface
  *            Sis950   A clone of the IT8705F
  *
  *  Copyright (C) 2001 Chris Gauthron
@@ -59,7 +62,7 @@
 
 #define DRVNAME "it87"
 
-enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728 };
+enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8783 };
 
 static unsigned short force_id;
 module_param(force_id, ushort, 0);
@@ -137,13 +140,19 @@ static inline void superio_exit(void)
 #define IT8721F_DEVID 0x8721
 #define IT8726F_DEVID 0x8726
 #define IT8728F_DEVID 0x8728
+#define IT8781F_DEVID 0x8781
+#define IT8782F_DEVID 0x8782
+#define IT8783E_DEVID 0x8783
 #define IT87_ACT_REG  0x30
 #define IT87_BASE_REG 0x60
 
 /* Logical device 7 registers (IT8712F and later) */
+#define IT87_SIO_GPIO1_REG	0x25
 #define IT87_SIO_GPIO3_REG	0x27
 #define IT87_SIO_GPIO5_REG	0x29
+#define IT87_SIO_PINX1_REG	0x2a	/* Pin selection */
 #define IT87_SIO_PINX2_REG	0x2c	/* Pin selection */
+#define IT87_SIO_SPI_REG	0xef	/* SPI function pin select */
 #define IT87_SIO_VID_REG	0xfc	/* VID value */
 #define IT87_SIO_BEEP_PIN_REG	0xf6	/* Beep pin mapping */
 
@@ -313,8 +322,12 @@ static u8 in_to_reg(const struct it87_data *data, int nr, long val)
 			lsb = 24;
 		else
 			lsb = 12;
-	} else
-		lsb = 16;
+	} else {
+		if (data->in_scaled & (1 << nr))
+			lsb = 32;
+		else
+			lsb = 16;
+	}
 
 	val = DIV_ROUND_CLOSEST(val, lsb);
 	return SENSORS_LIMIT(val, 0, 255);
@@ -327,8 +340,12 @@ static int in_from_reg(const struct it87_data *data, int nr, int val)
 			return val * 24;
 		else
 			return val * 12;
-	} else
-		return val * 16;
+	} else {
+		if (data->in_scaled & (1 << nr))
+			return val * 32;
+		else
+			return val * 16;
+	}
 }
 
 static inline u8 FAN_TO_REG(long rpm, int div)
@@ -407,7 +424,8 @@ static inline int has_16bit_fans(const struct it87_data *data)
 	    || data->type = it8718
 	    || data->type = it8720
 	    || data->type = it8721
-	    || data->type = it8728;
+	    || data->type = it8728
+	    || data->type = it8783;
 }
 
 static inline int has_old_autopwm(const struct it87_data *data)
@@ -1651,6 +1669,11 @@ static int __init it87_find(unsigned short *address,
 	case IT8728F_DEVID:
 		sio_data->type = it8728;
 		break;
+	case IT8781F_DEVID:
+	case IT8782F_DEVID:
+	case IT8783E_DEVID:
+		sio_data->type = it8783;
+		break;
 	case 0xffff:	/* No device at all */
 		goto exit;
 	default:
@@ -1686,6 +1709,54 @@ static int __init it87_find(unsigned short *address,
 		/* The IT8705F has a different LD number for GPIO */
 		superio_select(5);
 		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
+	} else if (sio_data->type = it8783) {
+		int reg25, reg27, reg2A, reg2C, regEF;
+
+		sio_data->skip_vid = 1;	/* No VID */
+
+		superio_select(GPIO);
+
+		reg25 = superio_inb(IT87_SIO_GPIO1_REG);
+		reg27 = superio_inb(IT87_SIO_GPIO3_REG);
+		reg2A = superio_inb(IT87_SIO_PINX1_REG);
+		reg2C = superio_inb(IT87_SIO_PINX2_REG);
+		regEF = superio_inb(IT87_SIO_SPI_REG);
+
+		/* Check if fan3 is there or not */
+		if ((reg27 & (1 << 0)) || !(reg2C & (1 << 2)))
+			sio_data->skip_fan |= (1 << 2);
+		if ((reg25 & (1 << 4))
+		    || (!(reg2A & (1 << 1)) && (regEF & (1 << 0))))
+			sio_data->skip_pwm |= (1 << 2);
+
+		/* Check if fan2 is there or not */
+		if (reg27 & (1 << 7))
+			sio_data->skip_fan |= (1 << 1);
+		if (reg27 & (1 << 3))
+			sio_data->skip_pwm |= (1 << 1);
+
+		/* VIN5 */
+		if ((reg27 & (1 << 0)) || (reg2C & (1 << 2)))
+			; /* No VIN5 */
+
+		/* VIN6 */
+		if ((reg27 & (1 << 1)) || (reg2C & (1 << 2)))
+			; /* No VIN6 */
+
+		/*
+		 * VIN7
+		 * Does not depend on bit 2 of Reg2C, contrary to datasheet.
+		 */
+		if (reg27 & (1 << 2))
+			; /* No VIN7 */
+
+		if (reg2C & (1 << 0))
+			sio_data->internal |= (1 << 0);
+		if (reg2C & (1 << 1))
+			sio_data->internal |= (1 << 1);
+
+		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
+
 	} else {
 		int reg;
 
@@ -1823,6 +1894,7 @@ static int __devinit it87_probe(struct platform_device *pdev)
 		"it8720",
 		"it8721",
 		"it8728",
+		"it8783",
 	};
 
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
@@ -1867,6 +1939,11 @@ static int __devinit it87_probe(struct platform_device *pdev)
 			data->in_scaled |= (1 << 7);	/* in7 is VSB */
 		if (sio_data->internal & (1 << 2))
 			data->in_scaled |= (1 << 8);	/* in8 is Vbat */
+	} else if (sio_data->type = it8783) {
+		if (sio_data->internal & (1 << 0))
+			data->in_scaled |= (1 << 3);	/* in3 is VCC5V */
+		if (sio_data->internal & (1 << 1))
+			data->in_scaled |= (1 << 7);	/* in7 is VCCH5V */
 	}
 
 	/* Initialize the IT87 chip */
@@ -2143,8 +2220,8 @@ static void __devinit it87_init_device(struct platform_device *pdev)
 			it87_write_value(data, IT87_REG_FAN_16BIT,
 					 tmp | 0x07);
 		}
-		/* IT8705F only supports three fans. */
-		if (data->type != it87) {
+		/* IT8705F and IT8783E/F only support three fans. */
+		if (data->type != it87 && data->type != it8783) {
 			if (tmp & (1 << 4))
 				data->has_fan |= (1 << 3); /* fan4 enabled */
 			if (tmp & (1 << 5))
-- 
1.7.5.4


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Bjoern Gerhart]

2012/3/8 Guenter Roeck <linux@roeck-us.net>:
> Assume all three are compatible and have the same functionality.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Bjoern Gerhart <oss@extracloud.de>
> ---
> This patch depends on the other pending changes for the it87 driver in my
> hwmon-next tree.
>
>  Documentation/hwmon/it87 |   24 +++++++----
>  drivers/hwmon/it87.c     |   93 ++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 100 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/hwmon/it87 b/Documentation/hwmon/it87
> index 23b7def..b0c1dcd 100644
> --- a/Documentation/hwmon/it87
> +++ b/Documentation/hwmon/it87
> @@ -30,6 +30,10 @@ Supported chips:
>     Prefix: 'it8728'
>     Addresses scanned: from Super I/O config space (8 I/O ports)
>     Datasheet: Not publicly available
> +  * IT8781F/IT8782F/IT8783E/F
> +    Prefix: 'it8783'
> +    Addresses scanned: from Super I/O config space (8 I/O ports)
> +    Datasheet: Not publicly available
>   * SiS950   [clone of IT8705F]
>     Prefix: 'it87'
>     Addresses scanned: from Super I/O config space (8 I/O ports)
> @@ -75,7 +79,8 @@ Description
>  -----------
>
>  This driver implements support for the IT8705F, IT8712F, IT8716F,
> -IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E and SiS950 chips.
> +IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E, IT8781F, IT8782F,
> +IT8783E/F, and SiS950 chips.
>
>  These chips are 'Super I/O chips', supporting floppy disks, infrared ports,
>  joysticks and other miscellaneous stuff. For hardware monitoring, they
> @@ -99,11 +104,11 @@ The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E and later IT8712F revisions
>  have support for 2 additional fans. The additional fans are supported by the
>  driver.
>
> -The IT8716F, IT8718F, IT8720F and IT8721F/IT8758E, and late IT8712F and
> -IT8705F also have optional 16-bit tachometer counters for fans 1 to 3. This
> -is better (no more fan clock divider mess) but not compatible with the older
> -chips and revisions. The 16-bit tachometer mode is enabled by the driver when
> -one of the above chips is detected.
> +The IT8716F, IT8718F, IT8720F and IT8721F/IT8758E, IT8781F, IT8782F, IT8783E/F,
> +and late IT8712F and IT8705F also have optional 16-bit tachometer counters for
> +fans 1 to 3. This is better (no more fan clock divider mess) but not compatible
> +with the older chips and revisions. The 16-bit tachometer mode is enabled by the
> +driver when one of the above chips is detected.
>
>  The IT8726F is just bit enhanced IT8716F with additional hardware
>  for AMD power sequencing. Therefore the chip will appear as IT8716F
> @@ -131,9 +136,10 @@ inputs can measure voltages between 0 and 4.08 volts, with a resolution of
>  0.016 volt (except IT8721F/IT8758E and IT8728F: 0.012 volt.) The battery
>  voltage in8 does not have limit registers.
>
> -On the IT8721F/IT8758E, some voltage inputs are internal and scaled inside
> -the chip (in7, in8 and optionally in3). The driver handles this transparently
> -so user-space doesn't have to care.
> +On the IT8721F/IT8758E/IT8781F/IT8782F/IT8783E/F, some voltage inputs are
> +internal and scaled inside the chip (in7 (optional for IT8781/2/3), in8 and
> +optionally in3). The driver handles this transparently so user-space doesn't
> +have to care.
>
>  The VID lines (IT8712F/IT8716F/IT8718F/IT8720F) encode the core voltage value:
>  the voltage level your processor should work with. This is hardcoded by
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 0b204e4..afaf19b 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -19,6 +19,9 @@
>  *            IT8726F  Super I/O chip w/LPC interface
>  *            IT8728F  Super I/O chip w/LPC interface
>  *            IT8758E  Super I/O chip w/LPC interface
> + *            IT8781F  Super I/O chip w/LPC interface
> + *            IT8782F  Super I/O chip w/LPC interface
> + *            IT8783E/F Super I/O chip w/LPC interface
>  *            Sis950   A clone of the IT8705F
>  *
>  *  Copyright (C) 2001 Chris Gauthron
> @@ -59,7 +62,7 @@
>
>  #define DRVNAME "it87"
>
> -enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728 };
> +enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8783 };
>
>  static unsigned short force_id;
>  module_param(force_id, ushort, 0);
> @@ -137,13 +140,19 @@ static inline void superio_exit(void)
>  #define IT8721F_DEVID 0x8721
>  #define IT8726F_DEVID 0x8726
>  #define IT8728F_DEVID 0x8728
> +#define IT8781F_DEVID 0x8781
> +#define IT8782F_DEVID 0x8782
> +#define IT8783E_DEVID 0x8783
>  #define IT87_ACT_REG  0x30
>  #define IT87_BASE_REG 0x60
>
>  /* Logical device 7 registers (IT8712F and later) */
> +#define IT87_SIO_GPIO1_REG     0x25
>  #define IT87_SIO_GPIO3_REG     0x27
>  #define IT87_SIO_GPIO5_REG     0x29
> +#define IT87_SIO_PINX1_REG     0x2a    /* Pin selection */
>  #define IT87_SIO_PINX2_REG     0x2c    /* Pin selection */
> +#define IT87_SIO_SPI_REG       0xef    /* SPI function pin select */
>  #define IT87_SIO_VID_REG       0xfc    /* VID value */
>  #define IT87_SIO_BEEP_PIN_REG  0xf6    /* Beep pin mapping */
>
> @@ -313,8 +322,12 @@ static u8 in_to_reg(const struct it87_data *data, int nr, long val)
>                        lsb = 24;
>                else
>                        lsb = 12;
> -       } else
> -               lsb = 16;
> +       } else {
> +               if (data->in_scaled & (1 << nr))
> +                       lsb = 32;
> +               else
> +                       lsb = 16;
> +       }
>
>        val = DIV_ROUND_CLOSEST(val, lsb);
>        return SENSORS_LIMIT(val, 0, 255);
> @@ -327,8 +340,12 @@ static int in_from_reg(const struct it87_data *data, int nr, int val)
>                        return val * 24;
>                else
>                        return val * 12;
> -       } else
> -               return val * 16;
> +       } else {
> +               if (data->in_scaled & (1 << nr))
> +                       return val * 32;
> +               else
> +                       return val * 16;
> +       }
>  }
>
>  static inline u8 FAN_TO_REG(long rpm, int div)
> @@ -407,7 +424,8 @@ static inline int has_16bit_fans(const struct it87_data *data)
>            || data->type == it8718
>            || data->type == it8720
>            || data->type == it8721
> -           || data->type == it8728;
> +           || data->type == it8728
> +           || data->type == it8783;
>  }
>
>  static inline int has_old_autopwm(const struct it87_data *data)
> @@ -1651,6 +1669,11 @@ static int __init it87_find(unsigned short *address,
>        case IT8728F_DEVID:
>                sio_data->type = it8728;
>                break;
> +       case IT8781F_DEVID:
> +       case IT8782F_DEVID:
> +       case IT8783E_DEVID:
> +               sio_data->type = it8783;
> +               break;
>        case 0xffff:    /* No device at all */
>                goto exit;
>        default:
> @@ -1686,6 +1709,54 @@ static int __init it87_find(unsigned short *address,
>                /* The IT8705F has a different LD number for GPIO */
>                superio_select(5);
>                sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> +       } else if (sio_data->type == it8783) {
> +               int reg25, reg27, reg2A, reg2C, regEF;
> +
> +               sio_data->skip_vid = 1; /* No VID */
> +
> +               superio_select(GPIO);
> +
> +               reg25 = superio_inb(IT87_SIO_GPIO1_REG);
> +               reg27 = superio_inb(IT87_SIO_GPIO3_REG);
> +               reg2A = superio_inb(IT87_SIO_PINX1_REG);
> +               reg2C = superio_inb(IT87_SIO_PINX2_REG);
> +               regEF = superio_inb(IT87_SIO_SPI_REG);
> +
> +               /* Check if fan3 is there or not */
> +               if ((reg27 & (1 << 0)) || !(reg2C & (1 << 2)))
> +                       sio_data->skip_fan |= (1 << 2);
> +               if ((reg25 & (1 << 4))
> +                   || (!(reg2A & (1 << 1)) && (regEF & (1 << 0))))
> +                       sio_data->skip_pwm |= (1 << 2);
> +
> +               /* Check if fan2 is there or not */
> +               if (reg27 & (1 << 7))
> +                       sio_data->skip_fan |= (1 << 1);
> +               if (reg27 & (1 << 3))
> +                       sio_data->skip_pwm |= (1 << 1);
> +
> +               /* VIN5 */
> +               if ((reg27 & (1 << 0)) || (reg2C & (1 << 2)))
> +                       ; /* No VIN5 */
> +
> +               /* VIN6 */
> +               if ((reg27 & (1 << 1)) || (reg2C & (1 << 2)))
> +                       ; /* No VIN6 */
> +
> +               /*
> +                * VIN7
> +                * Does not depend on bit 2 of Reg2C, contrary to datasheet.
> +                */
> +               if (reg27 & (1 << 2))
> +                       ; /* No VIN7 */
> +
> +               if (reg2C & (1 << 0))
> +                       sio_data->internal |= (1 << 0);
> +               if (reg2C & (1 << 1))
> +                       sio_data->internal |= (1 << 1);
> +
> +               sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> +
>        } else {
>                int reg;
>
> @@ -1823,6 +1894,7 @@ static int __devinit it87_probe(struct platform_device *pdev)
>                "it8720",
>                "it8721",
>                "it8728",
> +               "it8783",
>        };
>
>        res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> @@ -1867,6 +1939,11 @@ static int __devinit it87_probe(struct platform_device *pdev)
>                        data->in_scaled |= (1 << 7);    /* in7 is VSB */
>                if (sio_data->internal & (1 << 2))
>                        data->in_scaled |= (1 << 8);    /* in8 is Vbat */
> +       } else if (sio_data->type == it8783) {
> +               if (sio_data->internal & (1 << 0))
> +                       data->in_scaled |= (1 << 3);    /* in3 is VCC5V */
> +               if (sio_data->internal & (1 << 1))
> +                       data->in_scaled |= (1 << 7);    /* in7 is VCCH5V */
>        }
>
>        /* Initialize the IT87 chip */
> @@ -2143,8 +2220,8 @@ static void __devinit it87_init_device(struct platform_device *pdev)
>                        it87_write_value(data, IT87_REG_FAN_16BIT,
>                                         tmp | 0x07);
>                }
> -               /* IT8705F only supports three fans. */
> -               if (data->type != it87) {
> +               /* IT8705F and IT8783E/F only support three fans. */
> +               if (data->type != it87 && data->type != it8783) {
>                        if (tmp & (1 << 4))
>                                data->has_fan |= (1 << 3); /* fan4 enabled */
>                        if (tmp & (1 << 5))
> --
> 1.7.5.4
>
>
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Guenter Roeck]

On Wed, 2012-03-07 at 23:25 -0500, Guenter Roeck wrote:
> Assume all three are compatible and have the same functionality.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---

So .. I got Tested-by feedback (thanks), but unfortunately no review
feedback. The 3.3 kernel is just around the corner, meaning we might
miss the 3.4 commit window, since I won't let this patch go in without
Reviewed-by: or Acked-by:. 

Jean, I know you are terribly busy, but is there any chance for you to
have a look ?

Thanks,
Guenter

> This patch depends on the other pending changes for the it87 driver in my
> hwmon-next tree.
> 
>  Documentation/hwmon/it87 |   24 +++++++----
>  drivers/hwmon/it87.c     |   93 ++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 100 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/hwmon/it87 b/Documentation/hwmon/it87
> index 23b7def..b0c1dcd 100644
> --- a/Documentation/hwmon/it87
> +++ b/Documentation/hwmon/it87
> @@ -30,6 +30,10 @@ Supported chips:
>      Prefix: 'it8728'
>      Addresses scanned: from Super I/O config space (8 I/O ports)
>      Datasheet: Not publicly available
> +  * IT8781F/IT8782F/IT8783E/F
> +    Prefix: 'it8783'
> +    Addresses scanned: from Super I/O config space (8 I/O ports)
> +    Datasheet: Not publicly available
>    * SiS950   [clone of IT8705F]
>      Prefix: 'it87'
>      Addresses scanned: from Super I/O config space (8 I/O ports)
> @@ -75,7 +79,8 @@ Description
>  -----------
>  
>  This driver implements support for the IT8705F, IT8712F, IT8716F,
> -IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E and SiS950 chips.
> +IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E, IT8781F, IT8782F,
> +IT8783E/F, and SiS950 chips.
>  
>  These chips are 'Super I/O chips', supporting floppy disks, infrared ports,
>  joysticks and other miscellaneous stuff. For hardware monitoring, they
> @@ -99,11 +104,11 @@ The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E and later IT8712F revisions
>  have support for 2 additional fans. The additional fans are supported by the
>  driver.
>  
> -The IT8716F, IT8718F, IT8720F and IT8721F/IT8758E, and late IT8712F and
> -IT8705F also have optional 16-bit tachometer counters for fans 1 to 3. This
> -is better (no more fan clock divider mess) but not compatible with the older
> -chips and revisions. The 16-bit tachometer mode is enabled by the driver when
> -one of the above chips is detected.
> +The IT8716F, IT8718F, IT8720F and IT8721F/IT8758E, IT8781F, IT8782F, IT8783E/F,
> +and late IT8712F and IT8705F also have optional 16-bit tachometer counters for
> +fans 1 to 3. This is better (no more fan clock divider mess) but not compatible
> +with the older chips and revisions. The 16-bit tachometer mode is enabled by the
> +driver when one of the above chips is detected.
>  
>  The IT8726F is just bit enhanced IT8716F with additional hardware
>  for AMD power sequencing. Therefore the chip will appear as IT8716F
> @@ -131,9 +136,10 @@ inputs can measure voltages between 0 and 4.08 volts, with a resolution of
>  0.016 volt (except IT8721F/IT8758E and IT8728F: 0.012 volt.) The battery
>  voltage in8 does not have limit registers.
>  
> -On the IT8721F/IT8758E, some voltage inputs are internal and scaled inside
> -the chip (in7, in8 and optionally in3). The driver handles this transparently
> -so user-space doesn't have to care.
> +On the IT8721F/IT8758E/IT8781F/IT8782F/IT8783E/F, some voltage inputs are
> +internal and scaled inside the chip (in7 (optional for IT8781/2/3), in8 and
> +optionally in3). The driver handles this transparently so user-space doesn't
> +have to care.
>  
>  The VID lines (IT8712F/IT8716F/IT8718F/IT8720F) encode the core voltage value:
>  the voltage level your processor should work with. This is hardcoded by
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 0b204e4..afaf19b 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -19,6 +19,9 @@
>   *            IT8726F  Super I/O chip w/LPC interface
>   *            IT8728F  Super I/O chip w/LPC interface
>   *            IT8758E  Super I/O chip w/LPC interface
> + *            IT8781F  Super I/O chip w/LPC interface
> + *            IT8782F  Super I/O chip w/LPC interface
> + *            IT8783E/F Super I/O chip w/LPC interface
>   *            Sis950   A clone of the IT8705F
>   *
>   *  Copyright (C) 2001 Chris Gauthron
> @@ -59,7 +62,7 @@
>  
>  #define DRVNAME "it87"
>  
> -enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728 };
> +enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8783 };
>  
>  static unsigned short force_id;
>  module_param(force_id, ushort, 0);
> @@ -137,13 +140,19 @@ static inline void superio_exit(void)
>  #define IT8721F_DEVID 0x8721
>  #define IT8726F_DEVID 0x8726
>  #define IT8728F_DEVID 0x8728
> +#define IT8781F_DEVID 0x8781
> +#define IT8782F_DEVID 0x8782
> +#define IT8783E_DEVID 0x8783
>  #define IT87_ACT_REG  0x30
>  #define IT87_BASE_REG 0x60
>  
>  /* Logical device 7 registers (IT8712F and later) */
> +#define IT87_SIO_GPIO1_REG	0x25
>  #define IT87_SIO_GPIO3_REG	0x27
>  #define IT87_SIO_GPIO5_REG	0x29
> +#define IT87_SIO_PINX1_REG	0x2a	/* Pin selection */
>  #define IT87_SIO_PINX2_REG	0x2c	/* Pin selection */
> +#define IT87_SIO_SPI_REG	0xef	/* SPI function pin select */
>  #define IT87_SIO_VID_REG	0xfc	/* VID value */
>  #define IT87_SIO_BEEP_PIN_REG	0xf6	/* Beep pin mapping */
>  
> @@ -313,8 +322,12 @@ static u8 in_to_reg(const struct it87_data *data, int nr, long val)
>  			lsb = 24;
>  		else
>  			lsb = 12;
> -	} else
> -		lsb = 16;
> +	} else {
> +		if (data->in_scaled & (1 << nr))
> +			lsb = 32;
> +		else
> +			lsb = 16;
> +	}
>  
>  	val = DIV_ROUND_CLOSEST(val, lsb);
>  	return SENSORS_LIMIT(val, 0, 255);
> @@ -327,8 +340,12 @@ static int in_from_reg(const struct it87_data *data, int nr, int val)
>  			return val * 24;
>  		else
>  			return val * 12;
> -	} else
> -		return val * 16;
> +	} else {
> +		if (data->in_scaled & (1 << nr))
> +			return val * 32;
> +		else
> +			return val * 16;
> +	}
>  }
>  
>  static inline u8 FAN_TO_REG(long rpm, int div)
> @@ -407,7 +424,8 @@ static inline int has_16bit_fans(const struct it87_data *data)
>  	    || data->type = it8718
>  	    || data->type = it8720
>  	    || data->type = it8721
> -	    || data->type = it8728;
> +	    || data->type = it8728
> +	    || data->type = it8783;
>  }
>  
>  static inline int has_old_autopwm(const struct it87_data *data)
> @@ -1651,6 +1669,11 @@ static int __init it87_find(unsigned short *address,
>  	case IT8728F_DEVID:
>  		sio_data->type = it8728;
>  		break;
> +	case IT8781F_DEVID:
> +	case IT8782F_DEVID:
> +	case IT8783E_DEVID:
> +		sio_data->type = it8783;
> +		break;
>  	case 0xffff:	/* No device at all */
>  		goto exit;
>  	default:
> @@ -1686,6 +1709,54 @@ static int __init it87_find(unsigned short *address,
>  		/* The IT8705F has a different LD number for GPIO */
>  		superio_select(5);
>  		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> +	} else if (sio_data->type = it8783) {
> +		int reg25, reg27, reg2A, reg2C, regEF;
> +
> +		sio_data->skip_vid = 1;	/* No VID */
> +
> +		superio_select(GPIO);
> +
> +		reg25 = superio_inb(IT87_SIO_GPIO1_REG);
> +		reg27 = superio_inb(IT87_SIO_GPIO3_REG);
> +		reg2A = superio_inb(IT87_SIO_PINX1_REG);
> +		reg2C = superio_inb(IT87_SIO_PINX2_REG);
> +		regEF = superio_inb(IT87_SIO_SPI_REG);
> +
> +		/* Check if fan3 is there or not */
> +		if ((reg27 & (1 << 0)) || !(reg2C & (1 << 2)))
> +			sio_data->skip_fan |= (1 << 2);
> +		if ((reg25 & (1 << 4))
> +		    || (!(reg2A & (1 << 1)) && (regEF & (1 << 0))))
> +			sio_data->skip_pwm |= (1 << 2);
> +
> +		/* Check if fan2 is there or not */
> +		if (reg27 & (1 << 7))
> +			sio_data->skip_fan |= (1 << 1);
> +		if (reg27 & (1 << 3))
> +			sio_data->skip_pwm |= (1 << 1);
> +
> +		/* VIN5 */
> +		if ((reg27 & (1 << 0)) || (reg2C & (1 << 2)))
> +			; /* No VIN5 */
> +
> +		/* VIN6 */
> +		if ((reg27 & (1 << 1)) || (reg2C & (1 << 2)))
> +			; /* No VIN6 */
> +
> +		/*
> +		 * VIN7
> +		 * Does not depend on bit 2 of Reg2C, contrary to datasheet.
> +		 */
> +		if (reg27 & (1 << 2))
> +			; /* No VIN7 */
> +
> +		if (reg2C & (1 << 0))
> +			sio_data->internal |= (1 << 0);
> +		if (reg2C & (1 << 1))
> +			sio_data->internal |= (1 << 1);
> +
> +		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> +
>  	} else {
>  		int reg;
>  
> @@ -1823,6 +1894,7 @@ static int __devinit it87_probe(struct platform_device *pdev)
>  		"it8720",
>  		"it8721",
>  		"it8728",
> +		"it8783",
>  	};
>  
>  	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> @@ -1867,6 +1939,11 @@ static int __devinit it87_probe(struct platform_device *pdev)
>  			data->in_scaled |= (1 << 7);	/* in7 is VSB */
>  		if (sio_data->internal & (1 << 2))
>  			data->in_scaled |= (1 << 8);	/* in8 is Vbat */
> +	} else if (sio_data->type = it8783) {
> +		if (sio_data->internal & (1 << 0))
> +			data->in_scaled |= (1 << 3);	/* in3 is VCC5V */
> +		if (sio_data->internal & (1 << 1))
> +			data->in_scaled |= (1 << 7);	/* in7 is VCCH5V */
>  	}
>  
>  	/* Initialize the IT87 chip */
> @@ -2143,8 +2220,8 @@ static void __devinit it87_init_device(struct platform_device *pdev)
>  			it87_write_value(data, IT87_REG_FAN_16BIT,
>  					 tmp | 0x07);
>  		}
> -		/* IT8705F only supports three fans. */
> -		if (data->type != it87) {
> +		/* IT8705F and IT8783E/F only support three fans. */
> +		if (data->type != it87 && data->type != it8783) {
>  			if (tmp & (1 << 4))
>  				data->has_fan |= (1 << 3); /* fan4 enabled */
>  			if (tmp & (1 << 5))



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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Jean Delvare]

On Thu, 15 Mar 2012 14:13:27 -0700, Guenter Roeck wrote:
> On Wed, 2012-03-07 at 23:25 -0500, Guenter Roeck wrote:
> > Assume all three are compatible and have the same functionality.
> > 
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> 
> So .. I got Tested-by feedback (thanks), but unfortunately no review
> feedback. The 3.3 kernel is just around the corner, meaning we might
> miss the 3.4 commit window, since I won't let this patch go in without
> Reviewed-by: or Acked-by:. 
> 
> Jean, I know you are terribly busy, but is there any chance for you to
> have a look ?

That's on my to-do list, but not before next Monday I'm afraid :(

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Jean Delvare]

On Tue, 13 Mar 2012 19:43:15 +0100, Bjoern Gerhart wrote:
> 2012/3/8 Guenter Roeck <linux@roeck-us.net>:
> > Assume all three are compatible and have the same functionality.
> >
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Tested-by: Bjoern Gerhart <oss@extracloud.de>

Care to share the output of "sensors" with us?

Thanks,
-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Jean Delvare]

Hi Guenter,

On Wed,  7 Mar 2012 20:25:39 -0800, Guenter Roeck wrote:
> Assume all three are compatible and have the same functionality.

A dangerous assumption, if you ask me. The pin selection part in
particular is tricky and I wouldn't be surprised if it differs between
these chips. I don't think anyone asked for IT8781F or IT8782F support
yet, so we could play it safe and ignore these chips for now?

> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> This patch depends on the other pending changes for the it87 driver in my
> hwmon-next tree.

Review:

>  Documentation/hwmon/it87 |   24 +++++++----
>  drivers/hwmon/it87.c     |   93 ++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 100 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/hwmon/it87 b/Documentation/hwmon/it87
> index 23b7def..b0c1dcd 100644
> --- a/Documentation/hwmon/it87
> +++ b/Documentation/hwmon/it87
> @@ -30,6 +30,10 @@ Supported chips:
>      Prefix: 'it8728'
>      Addresses scanned: from Super I/O config space (8 I/O ports)
>      Datasheet: Not publicly available
> +  * IT8781F/IT8782F/IT8783E/F
> +    Prefix: 'it8783'
> +    Addresses scanned: from Super I/O config space (8 I/O ports)
> +    Datasheet: Not publicly available
>    * SiS950   [clone of IT8705F]
>      Prefix: 'it87'
>      Addresses scanned: from Super I/O config space (8 I/O ports)
> @@ -75,7 +79,8 @@ Description
>  -----------
>  
>  This driver implements support for the IT8705F, IT8712F, IT8716F,
> -IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E and SiS950 chips.
> +IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E, IT8781F, IT8782F,
> +IT8783E/F, and SiS950 chips.
>  
>  These chips are 'Super I/O chips', supporting floppy disks, infrared ports,
>  joysticks and other miscellaneous stuff. For hardware monitoring, they
> @@ -99,11 +104,11 @@ The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E and later IT8712F revisions
>  have support for 2 additional fans. The additional fans are supported by the
>  driver.
>  
> -The IT8716F, IT8718F, IT8720F and IT8721F/IT8758E, and late IT8712F and
> -IT8705F also have optional 16-bit tachometer counters for fans 1 to 3. This
> -is better (no more fan clock divider mess) but not compatible with the older
> -chips and revisions. The 16-bit tachometer mode is enabled by the driver when
> -one of the above chips is detected.
> +The IT8716F, IT8718F, IT8720F and IT8721F/IT8758E, IT8781F, IT8782F, IT8783E/F,
> +and late IT8712F and IT8705F also have optional 16-bit tachometer counters for
> +fans 1 to 3. This is better (no more fan clock divider mess) but not compatible
> +with the older chips and revisions. The 16-bit tachometer mode is enabled by the
> +driver when one of the above chips is detected.
>  
>  The IT8726F is just bit enhanced IT8716F with additional hardware
>  for AMD power sequencing. Therefore the chip will appear as IT8716F
> @@ -131,9 +136,10 @@ inputs can measure voltages between 0 and 4.08 volts, with a resolution of
>  0.016 volt (except IT8721F/IT8758E and IT8728F: 0.012 volt.) The battery
>  voltage in8 does not have limit registers.
>  
> -On the IT8721F/IT8758E, some voltage inputs are internal and scaled inside
> -the chip (in7, in8 and optionally in3). The driver handles this transparently
> -so user-space doesn't have to care.
> +On the IT8721F/IT8758E/IT8781F/IT8782F/IT8783E/F, some voltage inputs are

So far we used "/" to separate chip names which have the same device ID
so they can't be differentiated by the driver. For really different
chips we use a comma instead.

> +internal and scaled inside the chip (in7 (optional for IT8781/2/3), in8 and

I prefer when device names are spelled completely (except maybe the
suffix) rather than shortened that way. This allows the users to grep
quickly for their device name and find all occurrences right away.

> +optionally in3). The driver handles this transparently so user-space doesn't
> +have to care.
>  
>  The VID lines (IT8712F/IT8716F/IT8718F/IT8720F) encode the core voltage value:
>  the voltage level your processor should work with. This is hardcoded by
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 0b204e4..afaf19b 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -19,6 +19,9 @@
>   *            IT8726F  Super I/O chip w/LPC interface
>   *            IT8728F  Super I/O chip w/LPC interface
>   *            IT8758E  Super I/O chip w/LPC interface
> + *            IT8781F  Super I/O chip w/LPC interface
> + *            IT8782F  Super I/O chip w/LPC interface
> + *            IT8783E/F Super I/O chip w/LPC interface
>   *            Sis950   A clone of the IT8705F
>   *
>   *  Copyright (C) 2001 Chris Gauthron
> @@ -59,7 +62,7 @@
>  
>  #define DRVNAME "it87"
>  
> -enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728 };
> +enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8783 };
>  
>  static unsigned short force_id;
>  module_param(force_id, ushort, 0);
> @@ -137,13 +140,19 @@ static inline void superio_exit(void)
>  #define IT8721F_DEVID 0x8721
>  #define IT8726F_DEVID 0x8726
>  #define IT8728F_DEVID 0x8728
> +#define IT8781F_DEVID 0x8781
> +#define IT8782F_DEVID 0x8782
> +#define IT8783E_DEVID 0x8783
>  #define IT87_ACT_REG  0x30
>  #define IT87_BASE_REG 0x60
>  
>  /* Logical device 7 registers (IT8712F and later) */
> +#define IT87_SIO_GPIO1_REG	0x25
>  #define IT87_SIO_GPIO3_REG	0x27
>  #define IT87_SIO_GPIO5_REG	0x29
> +#define IT87_SIO_PINX1_REG	0x2a	/* Pin selection */
>  #define IT87_SIO_PINX2_REG	0x2c	/* Pin selection */
> +#define IT87_SIO_SPI_REG	0xef	/* SPI function pin select */
>  #define IT87_SIO_VID_REG	0xfc	/* VID value */
>  #define IT87_SIO_BEEP_PIN_REG	0xf6	/* Beep pin mapping */
>  
> @@ -313,8 +322,12 @@ static u8 in_to_reg(const struct it87_data *data, int nr, long val)
>  			lsb = 24;
>  		else
>  			lsb = 12;
> -	} else
> -		lsb = 16;
> +	} else {
> +		if (data->in_scaled & (1 << nr))
> +			lsb = 32;
> +		else
> +			lsb = 16;
> +	}
>  
>  	val = DIV_ROUND_CLOSEST(val, lsb);
>  	return SENSORS_LIMIT(val, 0, 255);
> @@ -327,8 +340,12 @@ static int in_from_reg(const struct it87_data *data, int nr, int val)
>  			return val * 24;
>  		else
>  			return val * 12;
> -	} else
> -		return val * 16;
> +	} else {
> +		if (data->in_scaled & (1 << nr))
> +			return val * 32;
> +		else
> +			return val * 16;
> +	}
>  }

I think both functions can then be rewritten more efficiently. I'll do
some testing and send a patch later.

>  
>  static inline u8 FAN_TO_REG(long rpm, int div)
> @@ -407,7 +424,8 @@ static inline int has_16bit_fans(const struct it87_data *data)
>  	    || data->type = it8718
>  	    || data->type = it8720
>  	    || data->type = it8721
> -	    || data->type = it8728;
> +	    || data->type = it8728
> +	    || data->type = it8783;
>  }

As the list grows longer, it might make sense to cache the result of
this function into struct it87_data itself. Right now it's called twice
in it87_update_device() amongst other. Separate patch, obviously...

>  
>  static inline int has_old_autopwm(const struct it87_data *data)
> @@ -1651,6 +1669,11 @@ static int __init it87_find(unsigned short *address,
>  	case IT8728F_DEVID:
>  		sio_data->type = it8728;
>  		break;
> +	case IT8781F_DEVID:
> +	case IT8782F_DEVID:
> +	case IT8783E_DEVID:
> +		sio_data->type = it8783;

I'm curious why you decided to go with a single chip type for all 3
devices. Having separate prefixes would seem to have some value, from
a supportability perspective is nothing else.

> +		break;
>  	case 0xffff:	/* No device at all */
>  		goto exit;
>  	default:
> @@ -1686,6 +1709,54 @@ static int __init it87_find(unsigned short *address,
>  		/* The IT8705F has a different LD number for GPIO */
>  		superio_select(5);
>  		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> +	} else if (sio_data->type = it8783) {
> +		int reg25, reg27, reg2A, reg2C, regEF;
> +
> +		sio_data->skip_vid = 1;	/* No VID */
> +
> +		superio_select(GPIO);
> +
> +		reg25 = superio_inb(IT87_SIO_GPIO1_REG);
> +		reg27 = superio_inb(IT87_SIO_GPIO3_REG);
> +		reg2A = superio_inb(IT87_SIO_PINX1_REG);
> +		reg2C = superio_inb(IT87_SIO_PINX2_REG);
> +		regEF = superio_inb(IT87_SIO_SPI_REG);
> +
> +		/* Check if fan3 is there or not */
> +		if ((reg27 & (1 << 0)) || !(reg2C & (1 << 2)))
> +			sio_data->skip_fan |= (1 << 2);

I'm a bit skeptical how a single pin can be FAN_TAC3 and SIN6 at the
same time...

> +		if ((reg25 & (1 << 4))
> +		    || (!(reg2A & (1 << 1)) && (regEF & (1 << 0))))
> +			sio_data->skip_pwm |= (1 << 2);
> +
> +		/* Check if fan2 is there or not */
> +		if (reg27 & (1 << 7))
> +			sio_data->skip_fan |= (1 << 1);
> +		if (reg27 & (1 << 3))
> +			sio_data->skip_pwm |= (1 << 1);
> +
> +		/* VIN5 */
> +		if ((reg27 & (1 << 0)) || (reg2C & (1 << 2)))
> +			; /* No VIN5 */

Obviously we can't leave things that way. I presume we want to add a
skip_in field to struct it87_sio_data and a has_in field to struct
it87_data, same we have for fans? I'm fine having this as a separate
patch, but both should go upstream together.

> +
> +		/* VIN6 */
> +		if ((reg27 & (1 << 1)) || (reg2C & (1 << 2)))
> +			; /* No VIN6 */
> +
> +		/*
> +		 * VIN7
> +		 * Does not depend on bit 2 of Reg2C, contrary to datasheet.

Datasheet is pretty confusing for this configuration bit. If the pin
can be used for a 3rd function ("SO") there must be a extra bit to
check somewhere... Hopefully it won't matter much as I expect vin7 to
be internal on most systems.

> +		 */
> +		if (reg27 & (1 << 2))
> +			; /* No VIN7 */

Unless vin7 is internal, in which case the test above doesn't matter.

> +
> +		if (reg2C & (1 << 0))
> +			sio_data->internal |= (1 << 0);
> +		if (reg2C & (1 << 1))
> +			sio_data->internal |= (1 << 1);
> +
> +		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> +
>  	} else {
>  		int reg;
>  
> @@ -1823,6 +1894,7 @@ static int __devinit it87_probe(struct platform_device *pdev)
>  		"it8720",
>  		"it8721",
>  		"it8728",
> +		"it8783",
>  	};
>  
>  	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> @@ -1867,6 +1939,11 @@ static int __devinit it87_probe(struct platform_device *pdev)
>  			data->in_scaled |= (1 << 7);	/* in7 is VSB */
>  		if (sio_data->internal & (1 << 2))
>  			data->in_scaled |= (1 << 8);	/* in8 is Vbat */
> +	} else if (sio_data->type = it8783) {
> +		if (sio_data->internal & (1 << 0))
> +			data->in_scaled |= (1 << 3);	/* in3 is VCC5V */
> +		if (sio_data->internal & (1 << 1))
> +			data->in_scaled |= (1 << 7);	/* in7 is VCCH5V */

The "Features" page says that Vbat is measured internally. If this
true, then it is necessarily always scaled too. This should let you
merge both blocks.

>  	}
>  
>  	/* Initialize the IT87 chip */
> @@ -2143,8 +2220,8 @@ static void __devinit it87_init_device(struct platform_device *pdev)
>  			it87_write_value(data, IT87_REG_FAN_16BIT,
>  					 tmp | 0x07);
>  		}
> -		/* IT8705F only supports three fans. */
> -		if (data->type != it87) {
> +		/* IT8705F and IT8783E/F only support three fans. */
> +		if (data->type != it87 && data->type != it8783) {
>  			if (tmp & (1 << 4))
>  				data->has_fan |= (1 << 3); /* fan4 enabled */
>  			if (tmp & (1 << 5))

The rest looks reasonable, although to be completely sure, a throughout
reading of the datasheet would be needed, for which I unfortunately do
not have the time.

Good work!

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Björn Gerhart]

Hi Jean,


Am 23.03.2012 um 22:02 schrieb Jean Delvare:

> On Tue, 13 Mar 2012 19:43:15 +0100, Bjoern Gerhart wrote:
>> 2012/3/8 Guenter Roeck <linux@roeck-us.net>:
>>> Assume all three are compatible and have the same functionality.
>>> 
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> Tested-by: Bjoern Gerhart <oss@extracloud.de>
> 
> Care to share the output of "sensors" with us?
> 
Sure. To be more precise, I tested with an IT8783 chip configured and wired by the hardware team at the company I'm working for on a proprietary mainboard. With the sensors.conf created therefore, the sensors output looks like this:

[root@localhost ~]# LANG=C sensors
it8783-isa-0a10
Adapter: ISA adapter
CPU Core:    +1.15 V  (min =  +0.70 V, max =  +1.70 V)
+1.50V:      +1.50 V  (min =  +1.10 V, max =  +1.90 V)
+3.30V:      +3.38 V  (min =  +2.70 V, max =  +4.00 V)
+12.0V:     +12.16 V  (min = +10.69 V, max = +13.70 V)
-12.0V:     -11.95 V  (min = -13.68 V, max = -10.30 V)
+1.10V:      +1.12 V  (min =  +0.90 V, max =  +1.22 V)
VBat:        +3.09 V
Fan1:       1662 RPM  (min =  300 RPM)
PSU:        1245 RPM  (min =  500 RPM)
Board T.:    +30.0 C  (low  =  +0.0 C, high = +55.0 C)  sensor = thermal diode
CPU T.:      +47.0 C  (low  =  +0.0 C, high = +63.0 C)  sensor = thermal diode

coretemp-isa-0000
Adapter: ISA adapter
Core 0:      +40.0 C  (high = +86.0 C, crit = +100.0 C)
Core 1:      +43.0 C  (high = +86.0 C, crit = +100.0 C)


These are about the same values as the "Hardware Monitor" section at the BIOS setup shows also. So all values look reasonable.


In general, for future Tested-bys: does it make sense to append a kind of test report (like the above one) directly after the Tested-by statement, or should it be located in a separate mail?

Björn


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Guenter Roeck]

Hi Jean,

On Fri, Mar 23, 2012 at 05:17:40PM -0400, Jean Delvare wrote:
> Hi Guenter,
> 
> On Wed,  7 Mar 2012 20:25:39 -0800, Guenter Roeck wrote:
> > Assume all three are compatible and have the same functionality.
> 
> A dangerous assumption, if you ask me. The pin selection part in
> particular is tricky and I wouldn't be surprised if it differs between
> these chips. I don't think anyone asked for IT8781F or IT8782F support
> yet, so we could play it safe and ignore these chips for now?
> 

Agreed, especially since I stumbled over a datasheet for IT8782F, and it turns out
to be more closely related to IT8721F than to IT8783F.

I'll drop IT8781F and keep the other two.

[ ... ]

> > -On the IT8721F/IT8758E, some voltage inputs are internal and scaled inside
> > -the chip (in7, in8 and optionally in3). The driver handles this transparently
> > -so user-space doesn't have to care.
> > +On the IT8721F/IT8758E/IT8781F/IT8782F/IT8783E/F, some voltage inputs are
> 
> So far we used "/" to separate chip names which have the same device ID
> so they can't be differentiated by the driver. For really different
> chips we use a comma instead.
> 
> > +internal and scaled inside the chip (in7 (optional for IT8781/2/3), in8 and
> 
> I prefer when device names are spelled completely (except maybe the
> suffix) rather than shortened that way. This allows the users to grep
> quickly for their device name and find all occurrences right away.
> 
ok.

> >               else
> >                       return val * 12;
> > -     } else
> > -             return val * 16;
> > +     } else {
> > +             if (data->in_scaled & (1 << nr))
> > +                     return val * 32;
> > +             else
> > +                     return val * 16;
> > +     }
> >  }
> 
> I think both functions can then be rewritten more efficiently. I'll do
> some testing and send a patch later.
> 
I know, I was a bit lazy ;).

How about the following ?

static int adc_lsb(const struct it87_data *data)
{
	int lsb = has_12mv_adc(data) ? 12 : 16;
	if (data->in_scaled & (1 << nr))
		lsb <<= 1;
	return lsb;
}

and:
	val = DIV_ROUND_CLOSEST(val, adc_lsb(data));

and:
	return val * adc_lsb(data);

> >
> >  static inline u8 FAN_TO_REG(long rpm, int div)
> > @@ -407,7 +424,8 @@ static inline int has_16bit_fans(const struct it87_data *data)
> >           || data->type = it8718
> >           || data->type = it8720
> >           || data->type = it8721
> > -         || data->type = it8728;
> > +         || data->type = it8728
> > +         || data->type = it8783;
> >  }
> 
> As the list grows longer, it might make sense to cache the result of
> this function into struct it87_data itself. Right now it's called twice
> in it87_update_device() amongst other. Separate patch, obviously...
> 
Agreed.

> >
> >  static inline int has_old_autopwm(const struct it87_data *data)
> > @@ -1651,6 +1669,11 @@ static int __init it87_find(unsigned short *address,
> >       case IT8728F_DEVID:
> >               sio_data->type = it8728;
> >               break;
> > +     case IT8781F_DEVID:
> > +     case IT8782F_DEVID:
> > +     case IT8783E_DEVID:
> > +             sio_data->type = it8783;
> 
> I'm curious why you decided to go with a single chip type for all 3
> devices. Having separate prefixes would seem to have some value, from
> a supportability perspective is nothing else.
> 
No special reason, and it was wrong anyway since the IT8781F does not have 6 uarts
per its shortened data sheet and thus presumably does not multiplex the respective pins.
And IT8782F is all different.

I'll split it8782 and it8783 and drop IT8781F support until we get a daatasheet.

> > @@ -1686,6 +1709,54 @@ static int __init it87_find(unsigned short *address,
> >               /* The IT8705F has a different LD number for GPIO */
> >               superio_select(5);
> >               sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> > +     } else if (sio_data->type = it8783) {
> > +             int reg25, reg27, reg2A, reg2C, regEF;
> > +
> > +             sio_data->skip_vid = 1; /* No VID */
> > +
> > +             superio_select(GPIO);
> > +
> > +             reg25 = superio_inb(IT87_SIO_GPIO1_REG);
> > +             reg27 = superio_inb(IT87_SIO_GPIO3_REG);
> > +             reg2A = superio_inb(IT87_SIO_PINX1_REG);
> > +             reg2C = superio_inb(IT87_SIO_PINX2_REG);
> > +             regEF = superio_inb(IT87_SIO_SPI_REG);
> > +
> > +             /* Check if fan3 is there or not */
> > +             if ((reg27 & (1 << 0)) || !(reg2C & (1 << 2)))
> > +                     sio_data->skip_fan |= (1 << 2);
> 
> I'm a bit skeptical how a single pin can be FAN_TAC3 and SIN6 at the
> same time...
> 

Issue here is that the pin is also multiplexed to VIN5 (see below), and it is
kind of difficult to determine how GP30, VIN5, FAN_TAC3, and SIN6 are selected
with only two configuration bits (one of which selects GP30). Maybe there is
another configuration bit somewhere, but I did not find it.

> > +             if ((reg25 & (1 << 4))
> > +                 || (!(reg2A & (1 << 1)) && (regEF & (1 << 0))))
> > +                     sio_data->skip_pwm |= (1 << 2);
> > +
> > +             /* Check if fan2 is there or not */
> > +             if (reg27 & (1 << 7))
> > +                     sio_data->skip_fan |= (1 << 1);
> > +             if (reg27 & (1 << 3))
> > +                     sio_data->skip_pwm |= (1 << 1);
> > +
> > +             /* VIN5 */
> > +             if ((reg27 & (1 << 0)) || (reg2C & (1 << 2)))
> > +                     ; /* No VIN5 */
> 
> Obviously we can't leave things that way. I presume we want to add a
> skip_in field to struct it87_sio_data and a has_in field to struct
> it87_data, same we have for fans? I'm fine having this as a separate
> patch, but both should go upstream together.
> 
Something like that. I'll think about it. I prefer a separate patch since
it will require quite some reshuffling of code (which is why I did not do it
in the first place). 

> > +
> > +             /* VIN6 */
> > +             if ((reg27 & (1 << 1)) || (reg2C & (1 << 2)))
> > +                     ; /* No VIN6 */
> > +
> > +             /*
> > +              * VIN7
> > +              * Does not depend on bit 2 of Reg2C, contrary to datasheet.
> 
> Datasheet is pretty confusing for this configuration bit. If the pin
> can be used for a 3rd function ("SO") there must be a extra bit to
> check somewhere... Hopefully it won't matter much as I expect vin7 to
> be internal on most systems.
> 
Same as with VIN5 ...

> > +              */
> > +             if (reg27 & (1 << 2))
> > +                     ; /* No VIN7 */
> 
> Unless vin7 is internal, in which case the test above doesn't matter.
> 
I'll add a note.

> > +
> > +             if (reg2C & (1 << 0))
> > +                     sio_data->internal |= (1 << 0);
> > +             if (reg2C & (1 << 1))
> > +                     sio_data->internal |= (1 << 1);
> > +
> > +             sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> > +
> >       } else {
> >               int reg;
> >
> > @@ -1823,6 +1894,7 @@ static int __devinit it87_probe(struct platform_device *pdev)
> >               "it8720",
> >               "it8721",
> >               "it8728",
> > +             "it8783",
> >       };
> >
> >       res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> > @@ -1867,6 +1939,11 @@ static int __devinit it87_probe(struct platform_device *pdev)
> >                       data->in_scaled |= (1 << 7);    /* in7 is VSB */
> >               if (sio_data->internal & (1 << 2))
> >                       data->in_scaled |= (1 << 8);    /* in8 is Vbat */
> > +     } else if (sio_data->type = it8783) {
> > +             if (sio_data->internal & (1 << 0))
> > +                     data->in_scaled |= (1 << 3);    /* in3 is VCC5V */
> > +             if (sio_data->internal & (1 << 1))
> > +                     data->in_scaled |= (1 << 7);    /* in7 is VCCH5V */
> 
> The "Features" page says that Vbat is measured internally. If this
> true, then it is necessarily always scaled too. This should let you
> merge both blocks.
> 
The IT8783F has a 16mv ADV which measures up to 4V, so scaling VBAT is not necessary.
I think the sensors output confirms that it is not scaled, unless I am missing something.

Thanks,
Guenter


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Jean Delvare]

SGFsbG8gQmrDtnJuLAoKT24gU2F0LCAyNCBNYXIgMjAxMiAwMDoyNDo1OCArMDEwMCwgQmrDtnJu
IEdlcmhhcnQgd3JvdGU6Cj4gQW0gMjMuMDMuMjAxMiB1bSAyMjowMiBzY2hyaWViIEplYW4gRGVs
dmFyZToKPiAKPiA+IE9uIFR1ZSwgMTMgTWFyIDIwMTIgMTk6NDM6MTUgKzAxMDAsIEJqb2VybiBH
ZXJoYXJ0IHdyb3RlOgo+ID4+IDIwMTIvMy84IEd1ZW50ZXIgUm9lY2sgPGxpbnV4QHJvZWNrLXVz
Lm5ldD46Cj4gPj4+IEFzc3VtZSBhbGwgdGhyZWUgYXJlIGNvbXBhdGlibGUgYW5kIGhhdmUgdGhl
IHNhbWUgZnVuY3Rpb25hbGl0eS4KPiA+Pj4gCj4gPj4+IFNpZ25lZC1vZmYtYnk6IEd1ZW50ZXIg
Um9lY2sgPGxpbnV4QHJvZWNrLXVzLm5ldD4KPiA+PiBUZXN0ZWQtYnk6IEJqb2VybiBHZXJoYXJ0
IDxvc3NAZXh0cmFjbG91ZC5kZT4KPiA+IAo+ID4gQ2FyZSB0byBzaGFyZSB0aGUgb3V0cHV0IG9m
ICJzZW5zb3JzIiB3aXRoIHVzPwo+ID4gCj4gU3VyZS4gVG8gYmUgbW9yZSBwcmVjaXNlLCBJIHRl
c3RlZCB3aXRoIGFuIElUODc4MyBjaGlwIGNvbmZpZ3VyZWQgYW5kIHdpcmVkIGJ5IHRoZSBoYXJk
d2FyZSB0ZWFtIGF0IHRoZSBjb21wYW55IEknbSB3b3JraW5nIGZvciBvbiBhIHByb3ByaWV0YXJ5
IG1haW5ib2FyZC4gV2l0aCB0aGUgc2Vuc29ycy5jb25mIGNyZWF0ZWQgdGhlcmVmb3JlLCB0aGUg
c2Vuc29ycyBvdXRwdXQgbG9va3MgbGlrZSB0aGlzOgo+IAo+IFtyb290QGxvY2FsaG9zdCB+XSMg
TEFORz1DIHNlbnNvcnMKPiBpdDg3ODMtaXNhLTBhMTAKPiBBZGFwdGVyOiBJU0EgYWRhcHRlcgo+
IENQVSBDb3JlOiAgICArMS4xNSBWICAobWluID0gICswLjcwIFYsIG1heCA9ICArMS43MCBWKQo+
ICsxLjUwVjogICAgICArMS41MCBWICAobWluID0gICsxLjEwIFYsIG1heCA9ICArMS45MCBWKQo+
ICszLjMwVjogICAgICArMy4zOCBWICAobWluID0gICsyLjcwIFYsIG1heCA9ICArNC4wMCBWKQo+
ICsxMi4wVjogICAgICsxMi4xNiBWICAobWluID0gKzEwLjY5IFYsIG1heCA9ICsxMy43MCBWKQo+
IC0xMi4wVjogICAgIC0xMS45NSBWICAobWluID0gLTEzLjY4IFYsIG1heCA9IC0xMC4zMCBWKQo+
ICsxLjEwVjogICAgICArMS4xMiBWICAobWluID0gICswLjkwIFYsIG1heCA9ICArMS4yMiBWKQo+
IFZCYXQ6ICAgICAgICArMy4wOSBWCj4gRmFuMTogICAgICAgMTY2MiBSUE0gIChtaW4gPSAgMzAw
IFJQTSkKPiBQU1U6ICAgICAgICAxMjQ1IFJQTSAgKG1pbiA9ICA1MDAgUlBNKQo+IEJvYXJkIFQu
OiAgICArMzAuMCBDICAobG93ICA9ICArMC4wIEMsIGhpZ2ggPSArNTUuMCBDKSAgc2Vuc29yID0g
dGhlcm1hbCBkaW9kZQo+IENQVSBULjogICAgICArNDcuMCBDICAobG93ICA9ICArMC4wIEMsIGhp
Z2ggPSArNjMuMCBDKSAgc2Vuc29yID0gdGhlcm1hbCBkaW9kZQo+IAo+IGNvcmV0ZW1wLWlzYS0w
MDAwCj4gQWRhcHRlcjogSVNBIGFkYXB0ZXIKPiBDb3JlIDA6ICAgICAgKzQwLjAgQyAgKGhpZ2gg
PSArODYuMCBDLCBjcml0ID0gKzEwMC4wIEMpCj4gQ29yZSAxOiAgICAgICs0My4wIEMgIChoaWdo
ID0gKzg2LjAgQywgY3JpdCA9ICsxMDAuMCBDKQo+IAo+IAo+IFRoZXNlIGFyZSBhYm91dCB0aGUg
c2FtZSB2YWx1ZXMgYXMgdGhlICJIYXJkd2FyZSBNb25pdG9yIiBzZWN0aW9uIGF0IHRoZSBCSU9T
IHNldHVwIHNob3dzIGFsc28uIFNvIGFsbCB2YWx1ZXMgbG9vayByZWFzb25hYmxlLgoKVGhhbmtz
LiBDYW4gd2Ugc2VlIHRoZSBjb25maWd1cmF0aW9uIGZpbGUgYXNzb2NpYXRlZCB3aXRoIHRoaXM/
IFdoYXQgSQphbSBwYXJ0aWN1bGFybHkgaW50ZXJlc3RlZCBpbiBhcmUgaW4zLCBpbjcgYW5kIGlu
OCAoVmJhdCkgYXMgdGhleSBhcmUKdHJlYXRlZCBzcGVjaWFsbHkgYnkgdGhlIGNoaXAgYW5kIHRo
ZSBkcml2ZXIuCgo+IEluIGdlbmVyYWwsIGZvciBmdXR1cmUgVGVzdGVkLWJ5czogZG9lcyBpdCBt
YWtlIHNlbnNlIHRvIGFwcGVuZCBhIGtpbmQgb2YgdGVzdCByZXBvcnQgKGxpa2UgdGhlIGFib3Zl
IG9uZSkgZGlyZWN0bHkgYWZ0ZXIgdGhlIFRlc3RlZC1ieSBzdGF0ZW1lbnQsIG9yIHNob3VsZCBp
dCBiZSBsb2NhdGVkIGluIGEgc2VwYXJhdGUgbWFpbD8KClllcywgdGVzdCByZXN1bHRzIGFyZSBh
bHdheXMgd2VsY29tZSwgYW5kIGluY2x1ZGluZyB0aGVtIGluIHRoZSBzYW1lCmUtbWFpbCBpcyBm
aW5lLgoKLS0gCkplYW4gRGVsdmFyZQoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX18KbG0tc2Vuc29ycyBtYWlsaW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1zZW5z
b3JzLm9yZwpodHRwOi8vbGlzdHMubG0tc2Vuc29ycy5vcmcvbWFpbG1hbi9saXN0aW5mby9sbS1z
ZW5zb3Jz

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Björn Gerhart]

Salut Jean,

Am 24.03.2012 um 08:05 schrieb Jean Delvare:
> Hallo Björn,
> 
> On Sat, 24 Mar 2012 00:24:58 +0100, Björn Gerhart wrote:
>> Am 23.03.2012 um 22:02 schrieb Jean Delvare:
>> 
>>> On Tue, 13 Mar 2012 19:43:15 +0100, Bjoern Gerhart wrote:
>>>> 2012/3/8 Guenter Roeck <linux@roeck-us.net>:
>>>>> Assume all three are compatible and have the same functionality.
>>>>> 
>>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>> Tested-by: Bjoern Gerhart <oss@extracloud.de>
>>> 
>>> Care to share the output of "sensors" with us?
>>> 
>> Sure. To be more precise, I tested with an IT8783 chip configured and wired by the hardware team at the company I'm working for on a proprietary mainboard. With the sensors.conf created therefore, the sensors output looks like this:
>> 
>> [root@localhost ~]# LANG=C sensors
>> it8783-isa-0a10
>> Adapter: ISA adapter
>> CPU Core:    +1.15 V  (min =  +0.70 V, max =  +1.70 V)
>> +1.50V:      +1.50 V  (min =  +1.10 V, max =  +1.90 V)
>> +3.30V:      +3.38 V  (min =  +2.70 V, max =  +4.00 V)
>> +12.0V:     +12.16 V  (min = +10.69 V, max = +13.70 V)
>> -12.0V:     -11.95 V  (min = -13.68 V, max = -10.30 V)
>> +1.10V:      +1.12 V  (min =  +0.90 V, max =  +1.22 V)
>> VBat:        +3.09 V
>> Fan1:       1662 RPM  (min =  300 RPM)
>> PSU:        1245 RPM  (min =  500 RPM)
>> Board T.:    +30.0 C  (low  =  +0.0 C, high = +55.0 C)  sensor = thermal diode
>> CPU T.:      +47.0 C  (low  =  +0.0 C, high = +63.0 C)  sensor = thermal diode
>> 
>> coretemp-isa-0000
>> Adapter: ISA adapter
>> Core 0:      +40.0 C  (high = +86.0 C, crit = +100.0 C)
>> Core 1:      +43.0 C  (high = +86.0 C, crit = +100.0 C)
>> 
>> 
>> These are about the same values as the "Hardware Monitor" section at the BIOS setup shows also. So all values look reasonable.
> 
> Thanks. Can we see the configuration file associated with this? What I
> am particularly interested in are in3, in7 and in8 (Vbat) as they are
> treated specially by the chip and the driver.
> 
Ok. in3 gets ignored, maybe not wired on that design. in7 and in8 don't get computed.

chip "it8783-isa-0a10"

    #=== voltages
    label in0 "CPU Core"
    compute in0  @, @
    set in0_min  0.7
    set in0_max  1.7
    
    label in1 "+1.50V"
    compute in1 @,@
    set in1_min  1.1
    set in1_max  1.9

    label in2 "+3.30V"
    compute in2  @, @
    set in2_min  2.7
    set in2_max  4.0

    label in4 "+12.0V"
    compute in4 ((30/10) +1)*@  , @/((30/10) +1)
    set in4_min  10.7
    set in4_max  13.7

    label in6 "-12.0V"
    compute in6 (1+232/56)*@ - 4.096*232/56, (@ + 4.096*232/56)/(1+232/56)
    set in6_min  -13.7
    set in6_max  -10.3
    
    label in7 "+1.10V"
    set in7_min  0.89
    set in7_max  1.21
    
    label in8 "VBat"

    ignore  in3
    ignore  in5
    ignore  vid

    #=== temps
    label temp2 "Board T."
    set temp2_min   0
    set temp2_max   55
    
    label temp3 "CPU T."
    compute temp3 100+@,-1*(100-@)
    set temp3_min   0
    set temp3_max   63

    ignore temp1
    ignore temp4

    #=== fans
    label fan1 "Fan1"
    set fan1_min  300

    label fan3 "PSU"
    set fan3_min  500

    ignore fan2
    ignore fan4

>> In general, for future Tested-bys: does it make sense to append a kind of test report (like the above one) directly after the Tested-by statement, or should it be located in a separate mail?
> 
> Yes, test results are always welcome, and including them in the same
> e-mail is fine.
> 
ok, perfect  ;-)

Björn


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Jean Delvare]

On Fri, 23 Mar 2012 21:18:32 -0700, Guenter Roeck wrote:
> On Fri, Mar 23, 2012 at 05:17:40PM -0400, Jean Delvare wrote:
> > On Wed,  7 Mar 2012 20:25:39 -0800, Guenter Roeck wrote:
> > >               else
> > >                       return val * 12;
> > > -     } else
> > > -             return val * 16;
> > > +     } else {
> > > +             if (data->in_scaled & (1 << nr))
> > > +                     return val * 32;
> > > +             else
> > > +                     return val * 16;
> > > +     }
> > >  }
> > 
> > I think both functions can then be rewritten more efficiently. I'll do
> > some testing and send a patch later.
>
> I know, I was a bit lazy ;).
> 
> How about the following ?
> 
> static int adc_lsb(const struct it87_data *data)
> {
> 	int lsb = has_12mv_adc(data) ? 12 : 16;
> 	if (data->in_scaled & (1 << nr))
> 		lsb <<= 1;
> 	return lsb;
> }
> 
> and:
> 	val = DIV_ROUND_CLOSEST(val, adc_lsb(data));
> 
> and:
> 	return val * adc_lsb(data);

Except that you have to pass nr around as a parameter, but otherwise
yes, that's what I had in mind.

> > > +             /* Check if fan3 is there or not */
> > > +             if ((reg27 & (1 << 0)) || !(reg2C & (1 << 2)))
> > > +                     sio_data->skip_fan |= (1 << 2);
> > 
> > I'm a bit skeptical how a single pin can be FAN_TAC3 and SIN6 at the
> > same time...
> 
> Issue here is that the pin is also multiplexed to VIN5 (see below), and it is
> kind of difficult to determine how GP30, VIN5, FAN_TAC3, and SIN6 are selected
> with only two configuration bits (one of which selects GP30). Maybe there is
> another configuration bit somewhere, but I did not find it.

Neither did I. I think the datasheet is incomplete.

> > > (...)
> > > @@ -1867,6 +1939,11 @@ static int __devinit it87_probe(struct platform_device *pdev)
> > >                       data->in_scaled |= (1 << 7);    /* in7 is VSB */
> > >               if (sio_data->internal & (1 << 2))
> > >                       data->in_scaled |= (1 << 8);    /* in8 is Vbat */
> > > +     } else if (sio_data->type = it8783) {
> > > +             if (sio_data->internal & (1 << 0))
> > > +                     data->in_scaled |= (1 << 3);    /* in3 is VCC5V */
> > > +             if (sio_data->internal & (1 << 1))
> > > +                     data->in_scaled |= (1 << 7);    /* in7 is VCCH5V */
> > 
> > The "Features" page says that Vbat is measured internally. If this
> > true, then it is necessarily always scaled too. This should let you
> > merge both blocks.
>
> The IT8783F has a 16mv ADV which measures up to 4V, so scaling VBAT is not necessary.
> I think the sensors output confirms that it is not scaled, unless I am missing something.

Oh, you're completely right, I had forgotten about the different LSB
value, sorry.

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Guenter Roeck]

Hi Jean,

On Sat, Mar 24, 2012 at 04:37:44AM -0400, Jean Delvare wrote:
> On Fri, 23 Mar 2012 21:18:32 -0700, Guenter Roeck wrote:
> > On Fri, Mar 23, 2012 at 05:17:40PM -0400, Jean Delvare wrote:
> > > On Wed,  7 Mar 2012 20:25:39 -0800, Guenter Roeck wrote:
> > > >               else
> > > >                       return val * 12;
> > > > -     } else
> > > > -             return val * 16;
> > > > +     } else {
> > > > +             if (data->in_scaled & (1 << nr))
> > > > +                     return val * 32;
> > > > +             else
> > > > +                     return val * 16;
> > > > +     }
> > > >  }
> > > 
> > > I think both functions can then be rewritten more efficiently. I'll do
> > > some testing and send a patch later.
> >
> > I know, I was a bit lazy ;).
> > 
> > How about the following ?
> > 
> > static int adc_lsb(const struct it87_data *data)
> > {
> > 	int lsb = has_12mv_adc(data) ? 12 : 16;
> > 	if (data->in_scaled & (1 << nr))
> > 		lsb <<= 1;
> > 	return lsb;
> > }
> > 
> > and:
> > 	val = DIV_ROUND_CLOSEST(val, adc_lsb(data));
> > 
> > and:
> > 	return val * adc_lsb(data);
> 
> Except that you have to pass nr around as a parameter, but otherwise
> yes, that's what I had in mind.
> 
Yes, I noticed that when I tried to compile it. Dumb compiler, should have
known what I meant and auto-fixed it ;).

Thanks,
Guenter

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

[lm-sensors] [PATCH] hwmon: (it87) Add support for IT8732F

[Justin Maggard]

Add support for the IT8732F.  This chip is pretty similar to IT8721F,
with the main difference being that the ADC LSB is 10.9 mV instead of
12 mV.

Signed-off-by: Justin Maggard <jmaggard@netgear.com>
---
 Documentation/hwmon/it87 | 35 +++++++++++++++++++------------
 drivers/hwmon/it87.c     | 54 +++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 66 insertions(+), 23 deletions(-)

diff --git a/Documentation/hwmon/it87 b/Documentation/hwmon/it87
index e872948..733296d 100644
--- a/Documentation/hwmon/it87
+++ b/Documentation/hwmon/it87
@@ -38,6 +38,10 @@ Supported chips:
     Prefix: 'it8728'
     Addresses scanned: from Super I/O config space (8 I/O ports)
     Datasheet: Not publicly available
+  * IT8732F
+    Prefix: 'it8732'
+    Addresses scanned: from Super I/O config space (8 I/O ports)
+    Datasheet: Not publicly available
   * IT8771E
     Prefix: 'it8771'
     Addresses scanned: from Super I/O config space (8 I/O ports)
@@ -111,9 +115,9 @@ Description
 -----------
 
 This driver implements support for the IT8603E, IT8620E, IT8623E, IT8705F,
-IT8712F, IT8716F, IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E,
-IT8771E, IT8772E, IT8781F, IT8782F, IT8783E/F, IT8786E, IT8790E, and SiS950
-chips.
+IT8712F, IT8716F, IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8732F,
+IT8758E, IT8771E, IT8772E, IT8781F, IT8782F, IT8783E/F, IT8786E, IT8790E, and
+SiS950 chips.
 
 These chips are 'Super I/O chips', supporting floppy disks, infrared ports,
 joysticks and other miscellaneous stuff. For hardware monitoring, they
@@ -137,10 +141,10 @@ The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E and later IT8712F revisions
 have support for 2 additional fans. The additional fans are supported by the
 driver.
 
-The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E, IT8781F, IT8782F, IT8783E/F,
-and late IT8712F and IT8705F also have optional 16-bit tachometer counters
-for fans 1 to 3. This is better (no more fan clock divider mess) but not
-compatible with the older chips and revisions. The 16-bit tachometer mode
+The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E, IT8732F, IT8781F, IT8782F,
+IT8783E/F, and late IT8712F and IT8705F also have optional 16-bit tachometer
+counters for fans 1 to 3. This is better (no more fan clock divider mess) but
+not compatible with the older chips and revisions. The 16-bit tachometer mode
 is enabled by the driver when one of the above chips is detected.
 
 The IT8726F is just bit enhanced IT8716F with additional hardware
@@ -159,6 +163,9 @@ IT8728F. It only supports 16-bit fan mode.
 
 The IT8790E supports up to 3 fans. 16-bit fan mode is always enabled.
 
+The IT8732F supports a closed-loop mode for fan control, but this is not
+currently implemented by the driver.
+
 Temperatures are measured in degrees Celsius. An alarm is triggered once
 when the Overtemperature Shutdown limit is crossed.
 
@@ -173,12 +180,14 @@ is done.
 Voltage sensors (also known as IN sensors) report their values in volts. An
 alarm is triggered if the voltage has crossed a programmable minimum or
 maximum limit. Note that minimum in this case always means 'closest to
-zero'; this is important for negative voltage measurements. All voltage
-inputs can measure voltages between 0 and 4.08 volts, with a resolution of
-0.016 volt (except IT8603E, IT8721F/IT8758E and IT8728F: 0.012 volt.) The
-battery voltage in8 does not have limit registers.
-
-On the IT8603E, IT8721F/IT8758E, IT8781F, IT8782F, and IT8783E/F, some
+zero'; this is important for negative voltage measurements. On most chips, all
+voltage inputs can measure voltages between 0 and 4.08 volts, with a resolution
+of 0.016 volt.  IT8603E, IT8721F/IT8758E and IT8728F can measure between 0 and
+3.06 volts, with a resolution of 0.012 volt.  IT8732F can measure between 0 and
+2.8 volts with a resolution of 0.0109 volt.  The battery voltage in8 does not
+have limit registers.
+
+On the IT8603E, IT8721F/IT8758E, IT8732F, IT8781F, IT8782F, and IT8783E/F, some
 voltage inputs are internal and scaled inside the chip:
 * in3 (optional)
 * in7 (optional for IT8781F, IT8782F, and IT8783E/F)
diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index d0ee556..4f0efe7 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -21,6 +21,7 @@
  *            IT8721F  Super I/O chip w/LPC interface
  *            IT8726F  Super I/O chip w/LPC interface
  *            IT8728F  Super I/O chip w/LPC interface
+ *            IT8732F  Super I/O chip w/LPC interface
  *            IT8758E  Super I/O chip w/LPC interface
  *            IT8771E  Super I/O chip w/LPC interface
  *            IT8772E  Super I/O chip w/LPC interface
@@ -69,8 +70,9 @@
 
 #define DRVNAME "it87"
 
-enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8771,
-	     it8772, it8781, it8782, it8783, it8786, it8790, it8603, it8620 };
+enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8732,
+	     it8771, it8772, it8781, it8782, it8783, it8786, it8790, it8603,
+	     it8620 };
 
 static unsigned short force_id;
 module_param(force_id, ushort, 0);
@@ -147,6 +149,7 @@ static inline void superio_exit(void)
 #define IT8720F_DEVID 0x8720
 #define IT8721F_DEVID 0x8721
 #define IT8726F_DEVID 0x8726
+#define IT8732F_DEVID 0x8732
 #define IT8728F_DEVID 0x8728
 #define IT8771E_DEVID 0x8771
 #define IT8772E_DEVID 0x8772
@@ -265,6 +268,7 @@ struct it87_devices {
 #define FEAT_VID		(1 << 9)	/* Set if chip supports VID */
 #define FEAT_IN7_INTERNAL	(1 << 10)	/* Set if in7 is internal */
 #define FEAT_SIX_FANS		(1 << 11)	/* Supports six fans */
+#define FEAT_10_9MV_ADC		(1 << 12)
 
 static const struct it87_devices it87_devices[] = {
 	[it87] = {
@@ -315,6 +319,15 @@ static const struct it87_devices it87_devices[] = {
 		  | FEAT_IN7_INTERNAL,
 		.peci_mask = 0x07,
 	},
+	[it8732] = {
+		.name = "it8732",
+		.suffix = "F",
+		.features = FEAT_NEWER_AUTOPWM | FEAT_16BIT_FANS
+		  | FEAT_TEMP_OFFSET | FEAT_TEMP_OLD_PECI | FEAT_TEMP_PECI
+		  | FEAT_10_9MV_ADC | FEAT_IN7_INTERNAL,
+		.peci_mask = 0x07,
+		.old_peci_mask = 0x02,	/* Actually reports PCH */
+	},
 	[it8771] = {
 		.name = "it8771",
 		.suffix = "E",
@@ -391,6 +404,7 @@ static const struct it87_devices it87_devices[] = {
 
 #define has_16bit_fans(data)	((data)->features & FEAT_16BIT_FANS)
 #define has_12mv_adc(data)	((data)->features & FEAT_12MV_ADC)
+#define has_10_9mv_adc(data)	((data)->features & FEAT_10_9MV_ADC)
 #define has_newer_autopwm(data)	((data)->features & FEAT_NEWER_AUTOPWM)
 #define has_old_autopwm(data)	((data)->features & FEAT_OLD_AUTOPWM)
 #define has_temp_offset(data)	((data)->features & FEAT_TEMP_OFFSET)
@@ -473,23 +487,35 @@ struct it87_data {
 	s8 auto_temp[3][5];	/* [nr][0] is point1_temp_hyst */
 };
 
-static int adc_lsb(const struct it87_data *data, int nr)
+static void adc_lsb(const struct it87_data *data, int nr, int *num, int *denom)
 {
-	int lsb = has_12mv_adc(data) ? 12 : 16;
+	if (has_12mv_adc(data)) {
+		*num =  12;
+		*denom =  1;
+	} else if (has_10_9mv_adc(data)) {
+		*num =  109;
+		*denom =  10;
+	} else {
+		*num =  16;
+		*denom =  1;
+	}
 	if (data->in_scaled & (1 << nr))
-		lsb <<= 1;
-	return lsb;
+		*num <<= 1;
 }
 
 static u8 in_to_reg(const struct it87_data *data, int nr, long val)
 {
-	val = DIV_ROUND_CLOSEST(val, adc_lsb(data, nr));
+	int num, denom;
+	adc_lsb(data, nr, &num, &denom);
+	val = DIV_ROUND_CLOSEST(val * denom, num);
 	return clamp_val(val, 0, 255);
 }
 
 static int in_from_reg(const struct it87_data *data, int nr, int val)
 {
-	return val * adc_lsb(data, nr);
+	int num, denom;
+	adc_lsb(data, nr, &num, &denom);
+	return val * num / denom;
 }
 
 static inline u8 FAN_TO_REG(long rpm, int div)
@@ -1515,9 +1541,14 @@ static ssize_t show_label(struct device *dev, struct device_attribute *attr,
 	};
 	struct it87_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr(attr)->index;
+	const char *label;
 
-	return sprintf(buf, "%s\n", has_12mv_adc(data) ? labels_it8721[nr]
-						       : labels[nr]);
+	if (has_12mv_adc(data) || has_10_9mv_adc(data))
+		label = labels_it8721[nr];
+	else
+		label = labels[nr];
+
+	return sprintf(buf, "%s\n", label);
 }
 static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, show_label, NULL, 0);
 static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, show_label, NULL, 1);
@@ -1853,6 +1884,9 @@ static int __init it87_find(unsigned short *address,
 	case IT8728F_DEVID:
 		sio_data->type = it8728;
 		break;
+	case IT8732F_DEVID:
+		sio_data->type = it8732;
+		break;
 	case IT8771E_DEVID:
 		sio_data->type = it8771;
 		break;
-- 
2.5.0


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8732F

[Guenter Roeck]

Hi Justin,

On Tue, Aug 04, 2015 at 12:45:31PM -0700, Justin Maggard wrote:
> Add support for the IT8732F.  This chip is pretty similar to IT8721F,
> with the main difference being that the ADC LSB is 10.9 mV instead of
> 12 mV.
> 

I assume you have a datasheet. Since you say "main difference",
are you aware of any other differences ? Would it by any chance
be possible to share the datasheet with us ?

Further comments inline.

Thanks,
Guenter

> Signed-off-by: Justin Maggard <jmaggard@netgear.com>
> ---
>  Documentation/hwmon/it87 | 35 +++++++++++++++++++------------
>  drivers/hwmon/it87.c     | 54 +++++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 66 insertions(+), 23 deletions(-)
> 
> diff --git a/Documentation/hwmon/it87 b/Documentation/hwmon/it87
> index e872948..733296d 100644
> --- a/Documentation/hwmon/it87
> +++ b/Documentation/hwmon/it87
> @@ -38,6 +38,10 @@ Supported chips:
>      Prefix: 'it8728'
>      Addresses scanned: from Super I/O config space (8 I/O ports)
>      Datasheet: Not publicly available
> +  * IT8732F
> +    Prefix: 'it8732'
> +    Addresses scanned: from Super I/O config space (8 I/O ports)
> +    Datasheet: Not publicly available
>    * IT8771E
>      Prefix: 'it8771'
>      Addresses scanned: from Super I/O config space (8 I/O ports)
> @@ -111,9 +115,9 @@ Description
>  -----------
>  
>  This driver implements support for the IT8603E, IT8620E, IT8623E, IT8705F,
> -IT8712F, IT8716F, IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E,
> -IT8771E, IT8772E, IT8781F, IT8782F, IT8783E/F, IT8786E, IT8790E, and SiS950
> -chips.
> +IT8712F, IT8716F, IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8732F,
> +IT8758E, IT8771E, IT8772E, IT8781F, IT8782F, IT8783E/F, IT8786E, IT8790E, and
> +SiS950 chips.
>  
>  These chips are 'Super I/O chips', supporting floppy disks, infrared ports,
>  joysticks and other miscellaneous stuff. For hardware monitoring, they
> @@ -137,10 +141,10 @@ The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E and later IT8712F revisions
>  have support for 2 additional fans. The additional fans are supported by the
>  driver.
>  
> -The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E, IT8781F, IT8782F, IT8783E/F,
> -and late IT8712F and IT8705F also have optional 16-bit tachometer counters
> -for fans 1 to 3. This is better (no more fan clock divider mess) but not
> -compatible with the older chips and revisions. The 16-bit tachometer mode
> +The IT8716F, IT8718F, IT8720F, IT8721F/IT8758E, IT8732F, IT8781F, IT8782F,
> +IT8783E/F, and late IT8712F and IT8705F also have optional 16-bit tachometer
> +counters for fans 1 to 3. This is better (no more fan clock divider mess) but
> +not compatible with the older chips and revisions. The 16-bit tachometer mode
>  is enabled by the driver when one of the above chips is detected.
>  
>  The IT8726F is just bit enhanced IT8716F with additional hardware
> @@ -159,6 +163,9 @@ IT8728F. It only supports 16-bit fan mode.
>  
>  The IT8790E supports up to 3 fans. 16-bit fan mode is always enabled.
>  
> +The IT8732F supports a closed-loop mode for fan control, but this is not
> +currently implemented by the driver.
> +
>  Temperatures are measured in degrees Celsius. An alarm is triggered once
>  when the Overtemperature Shutdown limit is crossed.
>  
> @@ -173,12 +180,14 @@ is done.
>  Voltage sensors (also known as IN sensors) report their values in volts. An
>  alarm is triggered if the voltage has crossed a programmable minimum or
>  maximum limit. Note that minimum in this case always means 'closest to
> -zero'; this is important for negative voltage measurements. All voltage
> -inputs can measure voltages between 0 and 4.08 volts, with a resolution of
> -0.016 volt (except IT8603E, IT8721F/IT8758E and IT8728F: 0.012 volt.) The
> -battery voltage in8 does not have limit registers.
> -
> -On the IT8603E, IT8721F/IT8758E, IT8781F, IT8782F, and IT8783E/F, some
> +zero'; this is important for negative voltage measurements. On most chips, all
> +voltage inputs can measure voltages between 0 and 4.08 volts, with a resolution
> +of 0.016 volt.  IT8603E, IT8721F/IT8758E and IT8728F can measure between 0 and
> +3.06 volts, with a resolution of 0.012 volt.  IT8732F can measure between 0 and
> +2.8 volts with a resolution of 0.0109 volt.  The battery voltage in8 does not
> +have limit registers.
> +
> +On the IT8603E, IT8721F/IT8758E, IT8732F, IT8781F, IT8782F, and IT8783E/F, some
>  voltage inputs are internal and scaled inside the chip:
>  * in3 (optional)
>  * in7 (optional for IT8781F, IT8782F, and IT8783E/F)
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index d0ee556..4f0efe7 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -21,6 +21,7 @@
>   *            IT8721F  Super I/O chip w/LPC interface
>   *            IT8726F  Super I/O chip w/LPC interface
>   *            IT8728F  Super I/O chip w/LPC interface
> + *            IT8732F  Super I/O chip w/LPC interface
>   *            IT8758E  Super I/O chip w/LPC interface
>   *            IT8771E  Super I/O chip w/LPC interface
>   *            IT8772E  Super I/O chip w/LPC interface
> @@ -69,8 +70,9 @@
>  
>  #define DRVNAME "it87"
>  
> -enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8771,
> -	     it8772, it8781, it8782, it8783, it8786, it8790, it8603, it8620 };
> +enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8732,
> +	     it8771, it8772, it8781, it8782, it8783, it8786, it8790, it8603,
> +	     it8620 };
>  
>  static unsigned short force_id;
>  module_param(force_id, ushort, 0);
> @@ -147,6 +149,7 @@ static inline void superio_exit(void)
>  #define IT8720F_DEVID 0x8720
>  #define IT8721F_DEVID 0x8721
>  #define IT8726F_DEVID 0x8726
> +#define IT8732F_DEVID 0x8732
>  #define IT8728F_DEVID 0x8728
>  #define IT8771E_DEVID 0x8771
>  #define IT8772E_DEVID 0x8772
> @@ -265,6 +268,7 @@ struct it87_devices {
>  #define FEAT_VID		(1 << 9)	/* Set if chip supports VID */
>  #define FEAT_IN7_INTERNAL	(1 << 10)	/* Set if in7 is internal */
>  #define FEAT_SIX_FANS		(1 << 11)	/* Supports six fans */
> +#define FEAT_10_9MV_ADC		(1 << 12)
>  
>  static const struct it87_devices it87_devices[] = {
>  	[it87] = {
> @@ -315,6 +319,15 @@ static const struct it87_devices it87_devices[] = {
>  		  | FEAT_IN7_INTERNAL,
>  		.peci_mask = 0x07,
>  	},
> +	[it8732] = {
> +		.name = "it8732",
> +		.suffix = "F",
> +		.features = FEAT_NEWER_AUTOPWM | FEAT_16BIT_FANS
> +		  | FEAT_TEMP_OFFSET | FEAT_TEMP_OLD_PECI | FEAT_TEMP_PECI
> +		  | FEAT_10_9MV_ADC | FEAT_IN7_INTERNAL,
> +		.peci_mask = 0x07,
> +		.old_peci_mask = 0x02,	/* Actually reports PCH */
> +	},
>  	[it8771] = {
>  		.name = "it8771",
>  		.suffix = "E",
> @@ -391,6 +404,7 @@ static const struct it87_devices it87_devices[] = {
>  
>  #define has_16bit_fans(data)	((data)->features & FEAT_16BIT_FANS)
>  #define has_12mv_adc(data)	((data)->features & FEAT_12MV_ADC)
> +#define has_10_9mv_adc(data)	((data)->features & FEAT_10_9MV_ADC)
>  #define has_newer_autopwm(data)	((data)->features & FEAT_NEWER_AUTOPWM)
>  #define has_old_autopwm(data)	((data)->features & FEAT_OLD_AUTOPWM)
>  #define has_temp_offset(data)	((data)->features & FEAT_TEMP_OFFSET)
> @@ -473,23 +487,35 @@ struct it87_data {
>  	s8 auto_temp[3][5];	/* [nr][0] is point1_temp_hyst */
>  };
>  
> -static int adc_lsb(const struct it87_data *data, int nr)
> +static void adc_lsb(const struct it87_data *data, int nr, int *num, int *denom)
>  {
> -	int lsb = has_12mv_adc(data) ? 12 : 16;
> +	if (has_12mv_adc(data)) {
> +		*num =  12;
> +		*denom =  1;
> +	} else if (has_10_9mv_adc(data)) {
> +		*num =  109;
> +		*denom =  10;
> +	} else {
> +		*num =  16;
> +		*denom =  1;
> +	}
>  	if (data->in_scaled & (1 << nr))
> -		lsb <<= 1;
> -	return lsb;
> +		*num <<= 1;
>  }

I would suggest to scale all return values up by a factor of 10,
and always divide by 10 in the calling code.

>  
>  static u8 in_to_reg(const struct it87_data *data, int nr, long val)
>  {
> -	val = DIV_ROUND_CLOSEST(val, adc_lsb(data, nr));
> +	int num, denom;
> +	adc_lsb(data, nr, &num, &denom);
> +	val = DIV_ROUND_CLOSEST(val * denom, num);

.. making this 

	val = DIV_ROUND_CLOSEST(val * 10, adc_lsb(data, nr));

>  	return clamp_val(val, 0, 255);
>  }
>  
>  static int in_from_reg(const struct it87_data *data, int nr, int val)
>  {
> -	return val * adc_lsb(data, nr);
> +	int num, denom;

[ side note: use checkpatch ]

> +	adc_lsb(data, nr, &num, &denom);
> +	return val * num / denom;

similar
	return (val * adc_lsb(data, nr)) / 10;
or better
	return DIV_ROUND_CLOSEST(val * adc_lsb(data, nr), 10);

>  }
>  
>  static inline u8 FAN_TO_REG(long rpm, int div)
> @@ -1515,9 +1541,14 @@ static ssize_t show_label(struct device *dev, struct device_attribute *attr,
>  	};
>  	struct it87_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr(attr)->index;
> +	const char *label;
>  
> -	return sprintf(buf, "%s\n", has_12mv_adc(data) ? labels_it8721[nr]
> -						       : labels[nr]);
> +	if (has_12mv_adc(data) || has_10_9mv_adc(data))
> +		label = labels_it8721[nr];
> +	else
> +		label = labels[nr];
> +
> +	return sprintf(buf, "%s\n", label);
>  }
>  static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, show_label, NULL, 0);
>  static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, show_label, NULL, 1);
> @@ -1853,6 +1884,9 @@ static int __init it87_find(unsigned short *address,
>  	case IT8728F_DEVID:
>  		sio_data->type = it8728;
>  		break;
> +	case IT8732F_DEVID:
> +		sio_data->type = it8732;
> +		break;
>  	case IT8771E_DEVID:
>  		sio_data->type = it8771;
>  		break;
> -- 
> 2.5.0
> 
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8732F

[Jean Delvare]

On Tue,  4 Aug 2015 12:45:31 -0700, Justin Maggard wrote:
> Add support for the IT8732F.  This chip is pretty similar to IT8721F,
> with the main difference being that the ADC LSB is 10.9 mV instead of
> 12 mV.

10.9 mV. No kidding. Sigh.

Drunk people shouldn't be allowed to design chipsets really.

-- 
Jean Delvare
SUSE L3 Support

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8732F

[Guenter Roeck]

On 08/04/2015 02:03 PM, Jean Delvare wrote:
> On Tue,  4 Aug 2015 12:45:31 -0700, Justin Maggard wrote:
>> Add support for the IT8732F.  This chip is pretty similar to IT8721F,
>> with the main difference being that the ADC LSB is 10.9 mV instead of
>> 12 mV.
>
> 10.9 mV. No kidding. Sigh.
>
> Drunk people shouldn't be allowed to design chipsets really.
>

Well, we are lucky. It could be 10.931 mV, after all. Wait, maybe it is ;-).

Makes me wonder, though - I hope this is a real chip value and not just
the value computed as the result after some external resistor pre-scaling.

Guenter


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8732F

[Justin Maggard]

On Tue, Aug 4, 2015 at 1:08 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> Hi Justin,
>
> On Tue, Aug 04, 2015 at 12:45:31PM -0700, Justin Maggard wrote:
>> Add support for the IT8732F.  This chip is pretty similar to IT8721F,
>> with the main difference being that the ADC LSB is 10.9 mV instead of
>> 12 mV.
>>
>
> I assume you have a datasheet. Since you say "main difference",
> are you aware of any other differences ? Would it by any chance
> be possible to share the datasheet with us ?
>

Yes, I do have a datasheet.  I'm investigating our confidentiality
obligations now, but I'm unsure what that outcome will be.  I don't
have datasheets for any other chips, so I can't really do a proper
comparison.  But the driver code for IT8721F seems to line up well
with the IT8732F datasheet.

IT8732F has up to 4 fan tachometer inputs, and up to 4 PWM outputs.
The "SmartGuardian" piece appears identical.  As I mentioned in the
doc change, it also has a fan tachometer closed-loop mode that seems
new (at least I didn't see any reference to it in the driver).  That
gets enabled by setting IT87_REG_PWM to automatic mode, and also
setting bit 2.  Temp sensors appear identical, with the obvious
exception of the 10.9 mV ADS LSB.

> Further comments inline.
>

OK, I'll whip up a v2 patch shortly to address your comments.

-Justin

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8732F

[Guenter Roeck]

Hi Justin,

On 08/04/2015 02:29 PM, Justin Maggard wrote:
> On Tue, Aug 4, 2015 at 1:08 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> Hi Justin,
>>
>> On Tue, Aug 04, 2015 at 12:45:31PM -0700, Justin Maggard wrote:
>>> Add support for the IT8732F.  This chip is pretty similar to IT8721F,
>>> with the main difference being that the ADC LSB is 10.9 mV instead of
>>> 12 mV.
>>>
>>
>> I assume you have a datasheet. Since you say "main difference",
>> are you aware of any other differences ? Would it by any chance
>> be possible to share the datasheet with us ?
>>
>
> Yes, I do have a datasheet.  I'm investigating our confidentiality
> obligations now, but I'm unsure what that outcome will be.  I don't

Ok.

> have datasheets for any other chips, so I can't really do a proper
> comparison.  But the driver code for IT8721F seems to line up well
> with the IT8732F datasheet.
>
> IT8732F has up to 4 fan tachometer inputs, and up to 4 PWM outputs.
> The "SmartGuardian" piece appears identical.  As I mentioned in the
> doc change, it also has a fan tachometer closed-loop mode that seems
> new (at least I didn't see any reference to it in the driver).  That

IT8772F supports it as well. Datasheet isn't published, but you might be
able to find leaked versions on the web. Search for IT8772_F_V0.4_031612.

Guenter


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Björn Gerhart]

Hi Guenter,

Am 28.03.2012 um 01:28 schrieb Guenter Roeck:
> On Tue, Mar 27, 2012 at 03:21:35PM -0400, Björn Gerhart wrote:
>> Hi Jean,
>> 
>> Am 26.03.2012 um 19:01 schrieb Jean Delvare:
>>> [..]
>>>> If actually it is the job of the module to do such calculations, I would offer to create an it87 patch for IT8783 fixing this - analog to an existing module with a similar behavior (if you know one).
>>> 
>>> You assume that the PECI value always refers to the CPU. I do not think
>>> this is true, other parts (e.g. north bridge) could report their
>>> temperature through PECI as well.
>>> 
>>> And if it is the CPU temperature, we already have the coretemp driver
>>> providing the same information, so there's little point in implementing
>>> the same in it87. The only benefit of handling PECI sensors in it87 and
>>> the like would be so that they can be used as sources for automatic fan
>>> speed control, but if the chip itself has no idea about the absolute
>>> value of the temperature then I doubt it can use it for automatic fan
>>> speed control.
>>> 
>> Yes, with the coretemp driver even both core temperatures are displayed correctly. So I agree, it's nonsense to implement our special design into the generic driver  ;-)
> 
> Depends. As Jean mentioned, it would make sense if the temperature can be used for
> automatic fan control.
> 
That's right, for instance the BIOS on our mainboard controls the fan RPM through that PECI value.

> The PECI specification says (I qoute Wikipedia) "PECI reports a negative value expressing
> the difference between the current temperature and the thermal throttle point (at which
> the CPU reduces speed or shuts down to prevent damage due to overheating) instead of the
> absolute temperature". So the negative value and slope is to be expected.
> 
> Question is if there is a way to tell the chip the how to calculate the real temperature
> from the raw value reported by PECI. The Nuvoton chips have a set of registers for that
> purpose.
> 
> Since the raw PECI value is not very useful, it might well be that it is possible
> to program the ITE chip to adust its readings. No idea how to do that, though.
> If your HW folks have a contact at ITE, maybe you could ask them to try to find out.
> Or maybe there is some BIOS code available from ITE and/or from BIOS vendors.
> 
My HW colleagues don't have deeper knowledge about PECI. I'll try to get the ITE contact address to ask ITE directly..
As far as I know, our BIOS simply uses the intel CPU register to calculate the real temperature.

> Alternatively, it might be possible to program fan control to take the negative slope
> into account; at least Wikipedia suggests that as possibility (to have the fan run faster
> as the reported temperature approaches zero from below). No idea how to do that either,
> though.
> 
> There is also register 0x0a, bits 4-6. Is that set to PECI on your board ?
> 
Yes, in our design on Index Register 0x0A, bits 6-4 are set to 110, which also means that PECI is enabled.


However, on our mainboard we have PECI mode enabled but at the same time we can use UART6. There is a trick to disable the parallel port and therefore enable COM6 on the parallel port's pins:
When PECI mode is enabled, UART6 is not necessarily disabled. It depends on the wiring. When JP4 (pin 1) is set to 0, the parallel port on pins 100..106 gets disabled. Therefore, on these pins UART6 can be used instead (see chapter "Power On Strapping Options").

Björn


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Guenter Roeck]

On Tue, Mar 27, 2012 at 03:21:35PM -0400, Björn Gerhart wrote:
> Hi Jean,
> 
> Am 26.03.2012 um 19:01 schrieb Jean Delvare:
> > [..]
> >> If actually it is the job of the module to do such calculations, I would offer to create an it87 patch for IT8783 fixing this - analog to an existing module with a similar behavior (if you know one).
> > 
> > You assume that the PECI value always refers to the CPU. I do not think
> > this is true, other parts (e.g. north bridge) could report their
> > temperature through PECI as well.
> > 
> > And if it is the CPU temperature, we already have the coretemp driver
> > providing the same information, so there's little point in implementing
> > the same in it87. The only benefit of handling PECI sensors in it87 and
> > the like would be so that they can be used as sources for automatic fan
> > speed control, but if the chip itself has no idea about the absolute
> > value of the temperature then I doubt it can use it for automatic fan
> > speed control.
> > 
> Yes, with the coretemp driver even both core temperatures are displayed correctly. So I agree, it's nonsense to implement our special design into the generic driver  ;-)

Depends. As Jean mentioned, it would make sense if the temperature can be used for
automatic fan control.

The PECI specification says (I qoute Wikipedia) "PECI reports a negative value expressing
the difference between the current temperature and the thermal throttle point (at which
the CPU reduces speed or shuts down to prevent damage due to overheating) instead of the
absolute temperature". So the negative value and slope is to be expected.

Question is if there is a way to tell the chip the how to calculate the real temperature
from the raw value reported by PECI. The Nuvoton chips have a set of registers for that
purpose.

Since the raw PECI value is not very useful, it might well be that it is possible
to program the ITE chip to adust its readings. No idea how to do that, though.
If your HW folks have a contact at ITE, maybe you could ask them to try to find out.
Or maybe there is some BIOS code available from ITE and/or from BIOS vendors.

Alternatively, it might be possible to program fan control to take the negative slope
into account; at least Wikipedia suggests that as possibility (to have the fan run faster
as the reported temperature approaches zero from below). No idea how to do that either,
though.

There is also register 0x0a, bits 4-6. Is that set to PECI on your board ?

Either case, I think it would make sense for us to report the sensor type as PECI
if so configured. Yet another patch ...

Thanks,
Guenter

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Björn Gerhart]

Hi Jean,

Am 26.03.2012 um 19:01 schrieb Jean Delvare:
> [..]
>> If actually it is the job of the module to do such calculations, I would offer to create an it87 patch for IT8783 fixing this - analog to an existing module with a similar behavior (if you know one).
> 
> You assume that the PECI value always refers to the CPU. I do not think
> this is true, other parts (e.g. north bridge) could report their
> temperature through PECI as well.
> 
> And if it is the CPU temperature, we already have the coretemp driver
> providing the same information, so there's little point in implementing
> the same in it87. The only benefit of handling PECI sensors in it87 and
> the like would be so that they can be used as sources for automatic fan
> speed control, but if the chip itself has no idea about the absolute
> value of the temperature then I doubt it can use it for automatic fan
> speed control.
> 
Yes, with the coretemp driver even both core temperatures are displayed correctly. So I agree, it's nonsense to implement our special design into the generic driver  ;-)

Björn


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Björn Gerhart]

HI Guenter,

Am 26.03.2012 um 21:05 schrieb Guenter Roeck:
> On Mon, 2012-03-26 at 12:08 -0400, Björn Gerhart wrote:
>> Hi Guenter,
>> 
>> Am 24.03.2012 um 19:07 schrieb Guenter Roeck:
>>> On Sat, Mar 24, 2012 at 12:23:13PM -0400, Guenter Roeck wrote:
>>>> Hi Bjoern,
>>>> 
>>>> At 01:11 AM 3/24/2012, Björn Gerhart wrote:
>>>> [ ... ]
>>>> 
>>>>>   label temp3 "CPU T."
>>>>>   compute temp3 100+@,-1*(100-@)
>>>> 
>>>> Is it possible that this is due to a bad sensor type configuration ?
>>>> 
>>>> Thermal diodes have a "current mode" and a 
>>>> "voltage mode", and if misconfigured (or 
>>>> miswired) the reported temperature difference is 
>>>> just about in that range. Difference in wiring, 
>>>> if I understand it correctly, is that the + pin 
>>>> is pulled high with a resistor in voltage mode, 
>>>> which is not the case in current mode.
>>>> 
>> As I have learned this morning from our hardware developers, neither "current" nor "voltage" mode is used with temp3, but digital "PECI" mode. In this case, temp3's value describes the _difference_ between the actual temperature and the CPU's maximum temperature (where the CPU will begin to throttle).
>> So actually, I think the static sensors.conf is the wrong place to specify this value, because different CPU models have different maximum temperatures. Therefore, dealing with 100 in our own sensors.conf (as implemented by now) is just a very rough inexact value.
>> 
>> Afaik, only temp3@IT8783 can be used in PECI mode, and in general other ITE sensor chip models can be driven in PECI mode also.
>> 
>> So, how could the PECI mode be handled on the driver side?
>> Detecting if it8783 is in PECI mode:
>> - Index register 0x2C, bit 6: set to 1 (Enable PECI)
>> - temp3 value is negative until CPU is not throttling
>> 
> Possibly also register 0x55 (ADC Temperature Extra Channel Enable
> Register), bit 7.
> 
> Can you check with your HW folks ?
> 
Your're right, checked that point with my HW colleagues this morning. They say when bit7 is 1 (as it is in our design), then the chip is in PECI resp. SST mode for getting digital temperatures @temp3.

Björn


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Guenter Roeck]

T24gTW9uLCAyMDEyLTAzLTI2IGF0IDEyOjA4IC0wNDAwLCBCasO2cm4gR2VyaGFydCB3cm90ZToK
PiBIaSBHdWVudGVyLAo+IAo+IEFtIDI0LjAzLjIwMTIgdW0gMTk6MDcgc2NocmllYiBHdWVudGVy
IFJvZWNrOgo+ID4gT24gU2F0LCBNYXIgMjQsIDIwMTIgYXQgMTI6MjM6MTNQTSAtMDQwMCwgR3Vl
bnRlciBSb2VjayB3cm90ZToKPiA+PiBIaSBCam9lcm4sCj4gPj4gCj4gPj4gQXQgMDE6MTEgQU0g
My8yNC8yMDEyLCBCasO2cm4gR2VyaGFydCB3cm90ZToKPiA+PiBbIC4uLiBdCj4gPj4gCj4gPj4+
ICAgIGxhYmVsIHRlbXAzICJDUFUgVC4iCj4gPj4+ICAgIGNvbXB1dGUgdGVtcDMgMTAwK0AsLTEq
KDEwMC1AKQo+ID4+IAo+ID4+IElzIGl0IHBvc3NpYmxlIHRoYXQgdGhpcyBpcyBkdWUgdG8gYSBi
YWQgc2Vuc29yIHR5cGUgY29uZmlndXJhdGlvbiA/Cj4gPj4gCj4gPj4gVGhlcm1hbCBkaW9kZXMg
aGF2ZSBhICJjdXJyZW50IG1vZGUiIGFuZCBhIAo+ID4+ICJ2b2x0YWdlIG1vZGUiLCBhbmQgaWYg
bWlzY29uZmlndXJlZCAob3IgCj4gPj4gbWlzd2lyZWQpIHRoZSByZXBvcnRlZCB0ZW1wZXJhdHVy
ZSBkaWZmZXJlbmNlIGlzIAo+ID4+IGp1c3QgYWJvdXQgaW4gdGhhdCByYW5nZS4gRGlmZmVyZW5j
ZSBpbiB3aXJpbmcsIAo+ID4+IGlmIEkgdW5kZXJzdGFuZCBpdCBjb3JyZWN0bHksIGlzIHRoYXQg
dGhlICsgcGluIAo+ID4+IGlzIHB1bGxlZCBoaWdoIHdpdGggYSByZXNpc3RvciBpbiB2b2x0YWdl
IG1vZGUsIAo+ID4+IHdoaWNoIGlzIG5vdCB0aGUgY2FzZSBpbiBjdXJyZW50IG1vZGUuCj4gPj4g
Cj4gQXMgSSBoYXZlIGxlYXJuZWQgdGhpcyBtb3JuaW5nIGZyb20gb3VyIGhhcmR3YXJlIGRldmVs
b3BlcnMsIG5laXRoZXIgImN1cnJlbnQiIG5vciAidm9sdGFnZSIgbW9kZSBpcyB1c2VkIHdpdGgg
dGVtcDMsIGJ1dCBkaWdpdGFsICJQRUNJIiBtb2RlLiBJbiB0aGlzIGNhc2UsIHRlbXAzJ3MgdmFs
dWUgZGVzY3JpYmVzIHRoZSBfZGlmZmVyZW5jZV8gYmV0d2VlbiB0aGUgYWN0dWFsIHRlbXBlcmF0
dXJlIGFuZCB0aGUgQ1BVJ3MgbWF4aW11bSB0ZW1wZXJhdHVyZSAod2hlcmUgdGhlIENQVSB3aWxs
IGJlZ2luIHRvIHRocm90dGxlKS4KPiBTbyBhY3R1YWxseSwgSSB0aGluayB0aGUgc3RhdGljIHNl
bnNvcnMuY29uZiBpcyB0aGUgd3JvbmcgcGxhY2UgdG8gc3BlY2lmeSB0aGlzIHZhbHVlLCBiZWNh
dXNlIGRpZmZlcmVudCBDUFUgbW9kZWxzIGhhdmUgZGlmZmVyZW50IG1heGltdW0gdGVtcGVyYXR1
cmVzLiBUaGVyZWZvcmUsIGRlYWxpbmcgd2l0aCAxMDAgaW4gb3VyIG93biBzZW5zb3JzLmNvbmYg
KGFzIGltcGxlbWVudGVkIGJ5IG5vdykgaXMganVzdCBhIHZlcnkgcm91Z2ggaW5leGFjdCB2YWx1
ZS4KPiAKPiBBZmFpaywgb25seSB0ZW1wM0BJVDg3ODMgY2FuIGJlIHVzZWQgaW4gUEVDSSBtb2Rl
LCBhbmQgaW4gZ2VuZXJhbCBvdGhlciBJVEUgc2Vuc29yIGNoaXAgbW9kZWxzIGNhbiBiZSBkcml2
ZW4gaW4gUEVDSSBtb2RlIGFsc28uCj4gCj4gU28sIGhvdyBjb3VsZCB0aGUgUEVDSSBtb2RlIGJl
IGhhbmRsZWQgb24gdGhlIGRyaXZlciBzaWRlPwo+IERldGVjdGluZyBpZiBpdDg3ODMgaXMgaW4g
UEVDSSBtb2RlOgo+IC0gSW5kZXggcmVnaXN0ZXIgMHgyQywgYml0IDY6IHNldCB0byAxIChFbmFi
bGUgUEVDSSkKPiAtIHRlbXAzIHZhbHVlIGlzIG5lZ2F0aXZlIHVudGlsIENQVSBpcyBub3QgdGhy
b3R0bGluZwo+IApQb3NzaWJseSBhbHNvIHJlZ2lzdGVyIDB4NTUgKEFEQyBUZW1wZXJhdHVyZSBF
eHRyYSBDaGFubmVsIEVuYWJsZQpSZWdpc3RlciksIGJpdCA3LgoKQ2FuIHlvdSBjaGVjayB3aXRo
IHlvdXIgSFcgZm9sa3MgPwoKVGhhbmtzLApHdWVudGVyCgo+IFdoZXJlIHRvIGdldCBjdXJyZW50
IENQVSdzIG1heGltdW0gdGVtcGVyYXR1cmUgZnJvbToKPiAtIE1TUiAweDFBMiwgYml0MjMuLi5i
aXQgMTYKPiAKCgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X18KbG0tc2Vuc29ycyBtYWlsaW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1zZW5zb3JzLm9yZwpodHRw
Oi8vbGlzdHMubG0tc2Vuc29ycy5vcmcvbWFpbG1hbi9saXN0aW5mby9sbS1zZW5zb3Jz

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Guenter Roeck]

T24gTW9uLCAyMDEyLTAzLTI2IGF0IDEzOjAxIC0wNDAwLCBKZWFuIERlbHZhcmUgd3JvdGU6Cj4g
T24gTW9uLCAyNiBNYXIgMjAxMiAxODowODozNSArMDIwMCwgQmrDtnJuIEdlcmhhcnQgd3JvdGU6
Cj4gPiBIaSBHdWVudGVyLAo+ID4gCj4gPiBBbSAyNC4wMy4yMDEyIHVtIDE5OjA3IHNjaHJpZWIg
R3VlbnRlciBSb2VjazoKPiA+ID4gT24gU2F0LCBNYXIgMjQsIDIwMTIgYXQgMTI6MjM6MTNQTSAt
MDQwMCwgR3VlbnRlciBSb2VjayB3cm90ZToKPiA+ID4+IEhpIEJqb2VybiwKPiA+ID4+IAo+ID4g
Pj4gQXQgMDE6MTEgQU0gMy8yNC8yMDEyLCBCasO2cm4gR2VyaGFydCB3cm90ZToKPiA+ID4+IFsg
Li4uIF0KPiA+ID4+IAo+ID4gPj4+ICAgIGxhYmVsIHRlbXAzICJDUFUgVC4iCj4gPiA+Pj4gICAg
Y29tcHV0ZSB0ZW1wMyAxMDArQCwtMSooMTAwLUApCj4gPiA+PiAKPiA+ID4+IElzIGl0IHBvc3Np
YmxlIHRoYXQgdGhpcyBpcyBkdWUgdG8gYSBiYWQgc2Vuc29yIHR5cGUgY29uZmlndXJhdGlvbiA/
Cj4gPiA+PiAKPiA+ID4+IFRoZXJtYWwgZGlvZGVzIGhhdmUgYSAiY3VycmVudCBtb2RlIiBhbmQg
YSAKPiA+ID4+ICJ2b2x0YWdlIG1vZGUiLCBhbmQgaWYgbWlzY29uZmlndXJlZCAob3IgCj4gPiA+
PiBtaXN3aXJlZCkgdGhlIHJlcG9ydGVkIHRlbXBlcmF0dXJlIGRpZmZlcmVuY2UgaXMgCj4gPiA+
PiBqdXN0IGFib3V0IGluIHRoYXQgcmFuZ2UuIERpZmZlcmVuY2UgaW4gd2lyaW5nLCAKPiA+ID4+
IGlmIEkgdW5kZXJzdGFuZCBpdCBjb3JyZWN0bHksIGlzIHRoYXQgdGhlICsgcGluIAo+ID4gPj4g
aXMgcHVsbGVkIGhpZ2ggd2l0aCBhIHJlc2lzdG9yIGluIHZvbHRhZ2UgbW9kZSwgCj4gPiA+PiB3
aGljaCBpcyBub3QgdGhlIGNhc2UgaW4gY3VycmVudCBtb2RlLgo+ID4gPj4gCj4gPiBBcyBJIGhh
dmUgbGVhcm5lZCB0aGlzIG1vcm5pbmcgZnJvbSBvdXIgaGFyZHdhcmUgZGV2ZWxvcGVycywgbmVp
dGhlciAiY3VycmVudCIgbm9yICJ2b2x0YWdlIiBtb2RlIGlzIHVzZWQgd2l0aCB0ZW1wMywgYnV0
IGRpZ2l0YWwgIlBFQ0kiIG1vZGUuIEluIHRoaXMgY2FzZSwgdGVtcDMncyB2YWx1ZSBkZXNjcmli
ZXMgdGhlIF9kaWZmZXJlbmNlXyBiZXR3ZWVuIHRoZSBhY3R1YWwgdGVtcGVyYXR1cmUgYW5kIHRo
ZSBDUFUncyBtYXhpbXVtIHRlbXBlcmF0dXJlICh3aGVyZSB0aGUgQ1BVIHdpbGwgYmVnaW4gdG8g
dGhyb3R0bGUpLgo+ID4gU28gYWN0dWFsbHksIEkgdGhpbmsgdGhlIHN0YXRpYyBzZW5zb3JzLmNv
bmYgaXMgdGhlIHdyb25nIHBsYWNlIHRvIHNwZWNpZnkgdGhpcyB2YWx1ZSwgYmVjYXVzZSBkaWZm
ZXJlbnQgQ1BVIG1vZGVscyBoYXZlIGRpZmZlcmVudCBtYXhpbXVtIHRlbXBlcmF0dXJlcy4gVGhl
cmVmb3JlLCBkZWFsaW5nIHdpdGggMTAwIGluIG91ciBvd24gc2Vuc29ycy5jb25mIChhcyBpbXBs
ZW1lbnRlZCBieSBub3cpIGlzIGp1c3QgYSB2ZXJ5IHJvdWdoIGluZXhhY3QgdmFsdWUuCj4gPiAK
PiA+IEFmYWlrLCBvbmx5IHRlbXAzQElUODc4MyBjYW4gYmUgdXNlZCBpbiBQRUNJIG1vZGUsIGFu
ZCBpbiBnZW5lcmFsIG90aGVyIElURSBzZW5zb3IgY2hpcCBtb2RlbHMgY2FuIGJlIGRyaXZlbiBp
biBQRUNJIG1vZGUgYWxzby4KPiA+IAo+ID4gU28sIGhvdyBjb3VsZCB0aGUgUEVDSSBtb2RlIGJl
IGhhbmRsZWQgb24gdGhlIGRyaXZlciBzaWRlPwo+ID4gRGV0ZWN0aW5nIGlmIGl0ODc4MyBpcyBp
biBQRUNJIG1vZGU6Cj4gPiAtIEluZGV4IHJlZ2lzdGVyIDB4MkMsIGJpdCA2OiBzZXQgdG8gMSAo
RW5hYmxlIFBFQ0kpCj4gPiAtIHRlbXAzIHZhbHVlIGlzIG5lZ2F0aXZlIHVudGlsIENQVSBpcyBu
b3QgdGhyb3R0bGluZwo+ID4gCj4gPiBXaGVyZSB0byBnZXQgY3VycmVudCBDUFUncyBtYXhpbXVt
IHRlbXBlcmF0dXJlIGZyb206Cj4gPiAtIE1TUiAweDFBMiwgYml0MjMuLi5iaXQgMTYKPiAKPiBU
aGlzIG9ubHkgd29ya3MgZm9yIEludGVsIENQVXMsIHJpZ2h0Pwo+IAo+ID4gSWYgYWN0dWFsbHkg
aXQgaXMgdGhlIGpvYiBvZiB0aGUgbW9kdWxlIHRvIGRvIHN1Y2ggY2FsY3VsYXRpb25zLCBJIHdv
dWxkIG9mZmVyIHRvIGNyZWF0ZSBhbiBpdDg3IHBhdGNoIGZvciBJVDg3ODMgZml4aW5nIHRoaXMg
LSBhbmFsb2cgdG8gYW4gZXhpc3RpbmcgbW9kdWxlIHdpdGggYSBzaW1pbGFyIGJlaGF2aW9yIChp
ZiB5b3Uga25vdyBvbmUpLgo+IAo+IFlvdSBhc3N1bWUgdGhhdCB0aGUgUEVDSSB2YWx1ZSBhbHdh
eXMgcmVmZXJzIHRvIHRoZSBDUFUuIEkgZG8gbm90IHRoaW5rCj4gdGhpcyBpcyB0cnVlLCBvdGhl
ciBwYXJ0cyAoZS5nLiBub3J0aCBicmlkZ2UpIGNvdWxkIHJlcG9ydCB0aGVpcgo+IHRlbXBlcmF0
dXJlIHRocm91Z2ggUEVDSSBhcyB3ZWxsLgo+IApEZWZpbml0ZWx5LiBBY3R1YWxseSwgYXQgbGVh
c3Qgd2l0aCB0aGUgTnV2b3RvbiBjaGlwcywgSSBoYXZlIG5ldmVyIHNlZW4KdGhlIG9mZnNldCB0
eXBlIHRlbXBlcmF0dXJlcyByZXBvcnRlZCBieSBQRUNJLCBvbmx5IHRoZSAicmVhbCIgdmFsdWVz
LgoKV29yc2UsIEkgZG9uJ3QgZmluZCBhbnl0aGluZyBpbiBhbnkgSVRFIGRhdGFzaGVldCBzdWdl
c3RpbmcgdGhhdCB0ZW1wMwoob3IgYW55IG90aGVyIHRlbXBlcmF0dXJlIHNlbnNvcikgd291bGQg
cmVwb3J0IFBFQ0kgdGVtcGVyYXR1cmVzIGlmCmF2YWlsYWJsZSwgYW5kL29yIHRoYXQgaXQgd291
bGQgYmUgcG9zc2libGUgdG8gY29uZmlndXJlIGl0LiBPdGhlciBjaGlwcwpzdXBwb3J0IFBFQ0ks
IGJ1dCBkb24ndCBoYXZlIGEgYml0IHRvIGFjdHVhbGx5IGVuYWJsZSBpdCAoaWUgYml0IDYgb2YK
cmVnaXN0ZXIgMkMgaXMgcmVzZXJ2ZWQgb3IgdXNlZCBmb3Igc29tZXRoaW5nIGVsc2UpLiBTbyB1
bmxlc3MgdGhlcmUgaXMKYWRkaXRpb25hbCBpbmZvcm1hdGlvbiwgSSBkb24ndCB0aGluayB0aGVy
ZSBpcyBhbnl0aGluZyB3ZSBfY2FuXyBkbyBpbgp0aGUgZHJpdmVyLgoKV2hhdCBJIGRpZCBmaW5k
IGlzIHRoYXQgdGhlIHRlbXAzIGlucHV0IHBpbiBpcyB1c2VkIGZvciBSSTYgaWYgdWFydDYgaXMK
ZW5hYmxlZCAob24gY2hpcHMgc3VwcG9ydGluZyA2IHVhcnRzKS4gSSdsbCB0YWtlIGNhcmUgb2Yg
dGhhdCBpbiBhbm90aGVyCnBhdGNoLgoKR3VlbnRlcgoKCgpfX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fXwpsbS1zZW5zb3JzIG1haWxpbmcgbGlzdApsbS1zZW5z
b3JzQGxtLXNlbnNvcnMub3JnCmh0dHA6Ly9saXN0cy5sbS1zZW5zb3JzLm9yZy9tYWlsbWFuL2xp
c3RpbmZvL2xtLXNlbnNvcnM

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Jean Delvare]

T24gTW9uLCAyNiBNYXIgMjAxMiAxODowODozNSArMDIwMCwgQmrDtnJuIEdlcmhhcnQgd3JvdGU6
Cj4gSGkgR3VlbnRlciwKPiAKPiBBbSAyNC4wMy4yMDEyIHVtIDE5OjA3IHNjaHJpZWIgR3VlbnRl
ciBSb2VjazoKPiA+IE9uIFNhdCwgTWFyIDI0LCAyMDEyIGF0IDEyOjIzOjEzUE0gLTA0MDAsIEd1
ZW50ZXIgUm9lY2sgd3JvdGU6Cj4gPj4gSGkgQmpvZXJuLAo+ID4+IAo+ID4+IEF0IDAxOjExIEFN
IDMvMjQvMjAxMiwgQmrDtnJuIEdlcmhhcnQgd3JvdGU6Cj4gPj4gWyAuLi4gXQo+ID4+IAo+ID4+
PiAgICBsYWJlbCB0ZW1wMyAiQ1BVIFQuIgo+ID4+PiAgICBjb21wdXRlIHRlbXAzIDEwMCtALC0x
KigxMDAtQCkKPiA+PiAKPiA+PiBJcyBpdCBwb3NzaWJsZSB0aGF0IHRoaXMgaXMgZHVlIHRvIGEg
YmFkIHNlbnNvciB0eXBlIGNvbmZpZ3VyYXRpb24gPwo+ID4+IAo+ID4+IFRoZXJtYWwgZGlvZGVz
IGhhdmUgYSAiY3VycmVudCBtb2RlIiBhbmQgYSAKPiA+PiAidm9sdGFnZSBtb2RlIiwgYW5kIGlm
IG1pc2NvbmZpZ3VyZWQgKG9yIAo+ID4+IG1pc3dpcmVkKSB0aGUgcmVwb3J0ZWQgdGVtcGVyYXR1
cmUgZGlmZmVyZW5jZSBpcyAKPiA+PiBqdXN0IGFib3V0IGluIHRoYXQgcmFuZ2UuIERpZmZlcmVu
Y2UgaW4gd2lyaW5nLCAKPiA+PiBpZiBJIHVuZGVyc3RhbmQgaXQgY29ycmVjdGx5LCBpcyB0aGF0
IHRoZSArIHBpbiAKPiA+PiBpcyBwdWxsZWQgaGlnaCB3aXRoIGEgcmVzaXN0b3IgaW4gdm9sdGFn
ZSBtb2RlLCAKPiA+PiB3aGljaCBpcyBub3QgdGhlIGNhc2UgaW4gY3VycmVudCBtb2RlLgo+ID4+
IAo+IEFzIEkgaGF2ZSBsZWFybmVkIHRoaXMgbW9ybmluZyBmcm9tIG91ciBoYXJkd2FyZSBkZXZl
bG9wZXJzLCBuZWl0aGVyICJjdXJyZW50IiBub3IgInZvbHRhZ2UiIG1vZGUgaXMgdXNlZCB3aXRo
IHRlbXAzLCBidXQgZGlnaXRhbCAiUEVDSSIgbW9kZS4gSW4gdGhpcyBjYXNlLCB0ZW1wMydzIHZh
bHVlIGRlc2NyaWJlcyB0aGUgX2RpZmZlcmVuY2VfIGJldHdlZW4gdGhlIGFjdHVhbCB0ZW1wZXJh
dHVyZSBhbmQgdGhlIENQVSdzIG1heGltdW0gdGVtcGVyYXR1cmUgKHdoZXJlIHRoZSBDUFUgd2ls
bCBiZWdpbiB0byB0aHJvdHRsZSkuCj4gU28gYWN0dWFsbHksIEkgdGhpbmsgdGhlIHN0YXRpYyBz
ZW5zb3JzLmNvbmYgaXMgdGhlIHdyb25nIHBsYWNlIHRvIHNwZWNpZnkgdGhpcyB2YWx1ZSwgYmVj
YXVzZSBkaWZmZXJlbnQgQ1BVIG1vZGVscyBoYXZlIGRpZmZlcmVudCBtYXhpbXVtIHRlbXBlcmF0
dXJlcy4gVGhlcmVmb3JlLCBkZWFsaW5nIHdpdGggMTAwIGluIG91ciBvd24gc2Vuc29ycy5jb25m
IChhcyBpbXBsZW1lbnRlZCBieSBub3cpIGlzIGp1c3QgYSB2ZXJ5IHJvdWdoIGluZXhhY3QgdmFs
dWUuCj4gCj4gQWZhaWssIG9ubHkgdGVtcDNASVQ4NzgzIGNhbiBiZSB1c2VkIGluIFBFQ0kgbW9k
ZSwgYW5kIGluIGdlbmVyYWwgb3RoZXIgSVRFIHNlbnNvciBjaGlwIG1vZGVscyBjYW4gYmUgZHJp
dmVuIGluIFBFQ0kgbW9kZSBhbHNvLgo+IAo+IFNvLCBob3cgY291bGQgdGhlIFBFQ0kgbW9kZSBi
ZSBoYW5kbGVkIG9uIHRoZSBkcml2ZXIgc2lkZT8KPiBEZXRlY3RpbmcgaWYgaXQ4NzgzIGlzIGlu
IFBFQ0kgbW9kZToKPiAtIEluZGV4IHJlZ2lzdGVyIDB4MkMsIGJpdCA2OiBzZXQgdG8gMSAoRW5h
YmxlIFBFQ0kpCj4gLSB0ZW1wMyB2YWx1ZSBpcyBuZWdhdGl2ZSB1bnRpbCBDUFUgaXMgbm90IHRo
cm90dGxpbmcKPiAKPiBXaGVyZSB0byBnZXQgY3VycmVudCBDUFUncyBtYXhpbXVtIHRlbXBlcmF0
dXJlIGZyb206Cj4gLSBNU1IgMHgxQTIsIGJpdDIzLi4uYml0IDE2CgpUaGlzIG9ubHkgd29ya3Mg
Zm9yIEludGVsIENQVXMsIHJpZ2h0PwoKPiBJZiBhY3R1YWxseSBpdCBpcyB0aGUgam9iIG9mIHRo
ZSBtb2R1bGUgdG8gZG8gc3VjaCBjYWxjdWxhdGlvbnMsIEkgd291bGQgb2ZmZXIgdG8gY3JlYXRl
IGFuIGl0ODcgcGF0Y2ggZm9yIElUODc4MyBmaXhpbmcgdGhpcyAtIGFuYWxvZyB0byBhbiBleGlz
dGluZyBtb2R1bGUgd2l0aCBhIHNpbWlsYXIgYmVoYXZpb3IgKGlmIHlvdSBrbm93IG9uZSkuCgpZ
b3UgYXNzdW1lIHRoYXQgdGhlIFBFQ0kgdmFsdWUgYWx3YXlzIHJlZmVycyB0byB0aGUgQ1BVLiBJ
IGRvIG5vdCB0aGluawp0aGlzIGlzIHRydWUsIG90aGVyIHBhcnRzIChlLmcuIG5vcnRoIGJyaWRn
ZSkgY291bGQgcmVwb3J0IHRoZWlyCnRlbXBlcmF0dXJlIHRocm91Z2ggUEVDSSBhcyB3ZWxsLgoK
QW5kIGlmIGl0IGlzIHRoZSBDUFUgdGVtcGVyYXR1cmUsIHdlIGFscmVhZHkgaGF2ZSB0aGUgY29y
ZXRlbXAgZHJpdmVyCnByb3ZpZGluZyB0aGUgc2FtZSBpbmZvcm1hdGlvbiwgc28gdGhlcmUncyBs
aXR0bGUgcG9pbnQgaW4gaW1wbGVtZW50aW5nCnRoZSBzYW1lIGluIGl0ODcuIFRoZSBvbmx5IGJl
bmVmaXQgb2YgaGFuZGxpbmcgUEVDSSBzZW5zb3JzIGluIGl0ODcgYW5kCnRoZSBsaWtlIHdvdWxk
IGJlIHNvIHRoYXQgdGhleSBjYW4gYmUgdXNlZCBhcyBzb3VyY2VzIGZvciBhdXRvbWF0aWMgZmFu
CnNwZWVkIGNvbnRyb2wsIGJ1dCBpZiB0aGUgY2hpcCBpdHNlbGYgaGFzIG5vIGlkZWEgYWJvdXQg
dGhlIGFic29sdXRlCnZhbHVlIG9mIHRoZSB0ZW1wZXJhdHVyZSB0aGVuIEkgZG91YnQgaXQgY2Fu
IHVzZSBpdCBmb3IgYXV0b21hdGljIGZhbgpzcGVlZCBjb250cm9sLgoKLS0gCkplYW4gRGVsdmFy
ZQoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbG0tc2Vu
c29ycyBtYWlsaW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1zZW5zb3JzLm9yZwpodHRwOi8vbGlzdHMu
bG0tc2Vuc29ycy5vcmcvbWFpbG1hbi9saXN0aW5mby9sbS1zZW5zb3Jz

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Björn Gerhart]

Hi Guenter,

Am 24.03.2012 um 19:07 schrieb Guenter Roeck:
> On Sat, Mar 24, 2012 at 12:23:13PM -0400, Guenter Roeck wrote:
>> Hi Bjoern,
>> 
>> At 01:11 AM 3/24/2012, Björn Gerhart wrote:
>> [ ... ]
>> 
>>>    label temp3 "CPU T."
>>>    compute temp3 100+@,-1*(100-@)
>> 
>> Is it possible that this is due to a bad sensor type configuration ?
>> 
>> Thermal diodes have a "current mode" and a 
>> "voltage mode", and if misconfigured (or 
>> miswired) the reported temperature difference is 
>> just about in that range. Difference in wiring, 
>> if I understand it correctly, is that the + pin 
>> is pulled high with a resistor in voltage mode, 
>> which is not the case in current mode.
>> 
As I have learned this morning from our hardware developers, neither "current" nor "voltage" mode is used with temp3, but digital "PECI" mode. In this case, temp3's value describes the _difference_ between the actual temperature and the CPU's maximum temperature (where the CPU will begin to throttle).
So actually, I think the static sensors.conf is the wrong place to specify this value, because different CPU models have different maximum temperatures. Therefore, dealing with 100 in our own sensors.conf (as implemented by now) is just a very rough inexact value.

Afaik, only temp3@IT8783 can be used in PECI mode, and in general other ITE sensor chip models can be driven in PECI mode also.

So, how could the PECI mode be handled on the driver side?
Detecting if it8783 is in PECI mode:
- Index register 0x2C, bit 6: set to 1 (Enable PECI)
- temp3 value is negative until CPU is not throttling

Where to get current CPU's maximum temperature from:
- MSR 0x1A2, bit23...bit 16

If actually it is the job of the module to do such calculations, I would offer to create an it87 patch for IT8783 fixing this - analog to an existing module with a similar behavior (if you know one).


>> Problem with the wrong reading is that it affects 
>> the sensor's dynamic range. You don't have much 
>> dynamic range left if the reading is off by 100 degrees.
>> 
> Hi again,
> 
> you might want to check registers 0x56, 0x57, and 0x59.
> It might be better to adjust temperature values with those registers
> than with sensors.conf, to retain the sensor's dynamic range.
> 
Thanks for your attentive review.

Björn


> Thanks,
> Guenter


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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Guenter Roeck]

On Sat, Mar 24, 2012 at 12:23:13PM -0400, Guenter Roeck wrote:
> Hi Bjoern,
> 
> At 01:11 AM 3/24/2012, Björn Gerhart wrote:
> [ ... ]
> 
> >     label temp3 "CPU T."
> >     compute temp3 100+@,-1*(100-@)
> 
> Is it possible that this is due to a bad sensor type configuration ?
> 
> Thermal diodes have a "current mode" and a 
> "voltage mode", and if misconfigured (or 
> miswired) the reported temperature difference is 
> just about in that range. Difference in wiring, 
> if I understand it correctly, is that the + pin 
> is pulled high with a resistor in voltage mode, 
> which is not the case in current mode.
> 
> Problem with the wrong reading is that it affects 
> the sensor's dynamic range. You don't have much 
> dynamic range left if the reading is off by 100 degrees.
> 
Hi again,

you might want to check registers 0x56, 0x57, and 0x59.
It might be better to adjust temperature values with those registers
than with sensors.conf, to retain the sensor's dynamic range.

Thanks,
Guenter

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

Re: [lm-sensors] [PATCH] hwmon: (it87) Add support for IT8781F, IT8782F, IT8783E/F

[Guenter Roeck]

Hi Bjoern,

At 01:11 AM 3/24/2012, Björn Gerhart wrote:
[ ... ]

>     label temp3 "CPU T."
>     compute temp3 100+@,-1*(100-@)

Is it possible that this is due to a bad sensor type configuration ?

Thermal diodes have a "current mode" and a 
"voltage mode", and if misconfigured (or 
miswired) the reported temperature difference is 
just about in that range. Difference in wiring, 
if I understand it correctly, is that the + pin 
is pulled high with a resistor in voltage mode, 
which is not the case in current mode.

Problem with the wrong reading is that it affects 
the sensor's dynamic range. You don't have much 
dynamic range left if the reading is off by 100 degrees.

Thanks,
Guenter 


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