[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