On Tue, Nov 12, 2013 at 02:48:03PM +0100, Rudolf Marek wrote:
> Hi all,
>
Hi Rudolf,
> I'm attaching the preliminary changes to support the IT8603E. The
> chip is very similar to IT8728, but it has extra internal voltage
> sensors located at 0x2f. I solved that with new "in9" and even with
> a label, but I consider my implementation not elegant. If you have
> any idea how to make it better please let me know or fix it. The
> 16-bit tachometer enable regs are used for something else! Therefore
> not touch them with this chip (this is also reserved in IT8728, so
> one may need to change the driver). There are also bits used for the
> other stuff which overlap old functionality. The ON/OFF mode is
> gone. We cannot touch the regs anymore. It will turn on something
> called "virtual temperature" and this is what David seen, that
> temperature was stuck. I took the E/F version print change from
> David patch. Rest seems ok.
>
> Just in case:
>
> Signed-off-by: Rudolf Marek <r.marek@xxxxxxxxxxxx>
>
> The sensors config:
>
> chip "it8603-*"
> label temp1 "CPU Temp"
> label temp2 "M/B Temp"
>
> label in0 "Vcore"
> label in1 "in1"
> label in2 "+12V"
> label in3 "+5V"
> label fan1 "CPU Fan"
> label fan2 "CHA Fan"
> label fan3 "PWR Fan"
>
> compute in2 @ * (12/2), @ / (12/2)
> compute in3 @ * (25/10), @ / (25/10)
>
>
> k10temp-pci-00c3
> Adapter: PCI adapter
> temp1: +0.0°C (high = +70.0°C)
> (crit = +70.0°C, hyst = +69.0°C)
>
> it8603-isa-0290
> Adapter: ISA adapter
> Vcore: +0.98 V (min = +2.87 V, max = +0.28 V) ALARM
> in1: +1.64 V (min = +0.24 V, max = +0.38 V) ALARM
> +12V: +12.17 V (min = +0.29 V, max = +9.00 V) ALARM
> +5V: +5.13 V (min = +5.04 V, max = +1.17 V) ALARM
> in4: +1.20 V (min = +0.06 V, max = +1.85 V)
> 3VSB: +3.34 V (min = +1.75 V, max = +2.54 V) ALARM
> Vbat: +3.24 V
> AVCC3: +3.38 V
> CPU Fan: 1744 RPM (min = 200 RPM)
> CHA Fan: 0 RPM (min = 600 RPM) ALARM
> CPU Temp: +31.0°C (low = +71.0°C, high = +109.0°C) sensor = thermistor
> M/B Temp: +34.0°C (low = -72.0°C, high = -72.0°C) ALARM sensor = thermistor
> temp3: -128.0°C (low = +0.0°C, high = +8.0°C) sensor = thermistor
> intrusion0: OK
>
>
> I already commit the sensors-detect change.
>
> Thanks
> Rudolf
>
> Index: linux-3.12/Documentation/hwmon/it87
It might make sense to update Kconfig as well.
> ===================================================================
> --- linux-3.12.orig/Documentation/hwmon/it87 2013-11-04 00:41:51.000000000 +0100
> +++ linux-3.12/Documentation/hwmon/it87 2013-11-12 14:09:30.940290706 +0100
> @@ -2,6 +2,10 @@
> ==================
>
> Supported chips:
> + * IT8603E
> + Prefix: 'it8603'
> + Addresses scanned: from Super I/O config space (8 I/O ports)
> + Datasheet: Not publicly available
> * IT8705F
> Prefix: 'it87'
> Addresses scanned: from Super I/O config space (8 I/O ports)
> @@ -90,7 +94,7 @@
> Description
> -----------
>
> -This driver implements support for the IT8705F, IT8712F, IT8716F,
> +This driver implements support for the IT8603, IT8705F, IT8712F, IT8716F,
IT8603 -> IT8603E
> IT8718F, IT8720F, IT8721F, IT8726F, IT8728F, IT8758E, IT8771E, IT8772E,
> IT8782F, IT8783E/F, and SiS950 chips.
>
> @@ -129,6 +133,10 @@
> The IT8728F, IT8771E, and IT8772E are considered compatible with the IT8721F,
> until a datasheet becomes available (hopefully.)
>
> +The IT8603E is a custom design, hardware monitoring part is similar to
> +IT8728F. There is 16-bit only fan mode only, the full speed mode of the fan
... It only supports 16-bit fan mode ... ?
> +is not supported (value 0 of pwmX_enable).
> +
> Temperatures are measured in degrees Celsius. An alarm is triggered once
> when the Overtemperature Shutdown limit is crossed.
>
> @@ -145,10 +153,10 @@
> 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 IT8721F/IT8758E and IT8728F: 0.012 volt.) The battery
> +0.016 volt (except IT8721F/IT8758E/IT8603E and IT8728F: 0.012 volt.) The battery
> voltage in8 does not have limit registers.
>
> -On the IT8721F/IT8758E, IT8782F, and IT8783E/F, some voltage inputs are
> +On the IT8721F/IT8758E/IT8603E, IT8782F, and IT8783E/F, some voltage inputs are
> internal and scaled inside the chip (in7 (optional for IT8782F and IT8783E/F),
> in8 and optionally in3). The driver handles this transparently so user-space
> doesn't have to care.
> Index: linux-3.12/drivers/hwmon/it87.c
> ===================================================================
> --- linux-3.12.orig/drivers/hwmon/it87.c 2013-11-04 00:41:51.000000000 +0100
> +++ linux-3.12/drivers/hwmon/it87.c 2013-11-12 14:21:31.555231353 +0100
> @@ -23,6 +23,7 @@
> * IT8772E Super I/O chip w/LPC interface
> * IT8782F Super I/O chip w/LPC interface
> * IT8783E/F Super I/O chip w/LPC interface
> + * IT8603E Super I/O chip w/LPC interface
Wonder if the chip listings should all be in order (ie IT8603E comes first).
> * Sis950 A clone of the IT8705F
> *
> * Copyright (C) 2001 Chris Gauthron
> @@ -64,7 +65,7 @@
> #define DRVNAME "it87"
>
> enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8771,
> - it8772, it8782, it8783 };
> + it8772, it8782, it8783, it8603 };
>
> static unsigned short force_id;
> module_param(force_id, ushort, 0);
> @@ -146,6 +147,7 @@
> #define IT8772E_DEVID 0x8772
> #define IT8782F_DEVID 0x8782
> #define IT8783E_DEVID 0x8783
> +#define IT8306E_DEVID 0x8603
> #define IT87_ACT_REG 0x30
> #define IT87_BASE_REG 0x60
>
> @@ -315,6 +317,12 @@
> | FEAT_TEMP_OLD_PECI,
> .old_peci_mask = 0x4,
> },
> + [it8603] = {
> + .name = "it8603",
> + .features = FEAT_NEWER_AUTOPWM | FEAT_12MV_ADC | FEAT_16BIT_FANS
> + | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI,
> + .peci_mask = 0x07,
> + },
> };
>
> #define has_16bit_fans(data) ((data)->features & FEAT_16BIT_FANS)
> @@ -334,7 +342,7 @@
> u8 revision;
> u8 vid_value;
> u8 beep_pin;
> - u8 internal; /* Internal sensors can be labeled */
> + u16 internal; /* Internal sensors can be labeled */
> /* Features skipped based on config or DMI */
> u16 skip_in;
> u8 skip_vid;
> @@ -361,7 +369,7 @@
> unsigned long last_updated; /* In jiffies */
>
> u16 in_scaled; /* Internal voltage sensors are scaled */
> - u8 in[9][3]; /* [nr][0]=in, [1]=min, [2]=max */
> + u8 in[10][3]; /* [nr][0]=in, [1]=min, [2]=max */
> u8 has_fan; /* Bitfield, fans enabled */
> u16 fan[5][2]; /* Register values, [nr][0]=fan, [1]=min */
> u8 has_temp; /* Bitfield, temp sensors enabled */
> @@ -578,6 +586,8 @@
> 7, 2);
>
> static SENSOR_DEVICE_ATTR_2(in8_input, S_IRUGO, show_in, NULL, 8, 0);
> +static SENSOR_DEVICE_ATTR_2(in9_input, S_IRUGO, show_in, NULL, 9, 0);
> +
>
> /* 3 temperatures */
> static ssize_t show_temp(struct device *dev, struct device_attribute *attr,
> @@ -734,7 +744,7 @@
> {
> int ctrl = data->fan_main_ctrl & (1 << nr);
>
> - if (ctrl == 0) /* Full speed */
> + if ((ctrl == 0) && (data->type != it8603)) /* Full speed */
I personally dislike those unnecessary extra ( ). It always confuses me.
Especially since it is not done elsewhere in the driver and thus inconsistent.
> return 0;
> if (data->pwm_ctrl[nr] & 0x80) /* Automatic mode */
> return 2;
> @@ -929,6 +939,10 @@
> return -EINVAL;
> }
>
> + /* IT8603E does not have on/off mode */
> + if ((val == 0) && (data->type == it8603))
> + return -EINVAL;
> +
> mutex_lock(&data->update_lock);
>
> if (val == 0) {
> @@ -948,10 +962,13 @@
> else /* Automatic mode */
> data->pwm_ctrl[nr] = 0x80 | data->pwm_temp_map[nr];
> it87_write_value(data, IT87_REG_PWM(nr), data->pwm_ctrl[nr]);
> - /* set SmartGuardian mode */
> - data->fan_main_ctrl |= (1 << nr);
> - it87_write_value(data, IT87_REG_FAN_MAIN_CTRL,
> - data->fan_main_ctrl);
> +
> + if (data->type != it8603) {
> + /* set SmartGuardian mode */
> + data->fan_main_ctrl |= (1 << nr);
> + it87_write_value(data, IT87_REG_FAN_MAIN_CTRL,
> + data->fan_main_ctrl);
> + }
> }
>
> mutex_unlock(&data->update_lock);
> @@ -1406,15 +1423,25 @@
> "3VSB",
> "Vbat",
> };
> + static const char * const labels_it8603[] = {
> + "AVCC3",
> + "3VSB",
> + "Vbat",
> + };
> struct it87_data *data = dev_get_drvdata(dev);
> int nr = to_sensor_dev_attr(attr)->index;
>
> + if (data->type == it8603)
> + return sprintf(buf, "%s\n", labels_it8603[nr]);
> +
> return sprintf(buf, "%s\n", has_12mv_adc(data) ? labels_it8721[nr]
> : labels[nr]);
> }
> static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, show_label, NULL, 0);
> static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, show_label, NULL, 1);
> static SENSOR_DEVICE_ATTR(in8_label, S_IRUGO, show_label, NULL, 2);
> +/* special AVCC3 IT8306 in9 */
> +static SENSOR_DEVICE_ATTR(in9_label, S_IRUGO, show_label, NULL, 0);
>
> static ssize_t show_name(struct device *dev, struct device_attribute
> *devattr, char *buf)
> @@ -1424,7 +1451,7 @@
> }
> static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
>
> -static struct attribute *it87_attributes_in[9][5] = {
> +static struct attribute *it87_attributes_in[10][5] = {
> {
> &sensor_dev_attr_in0_input.dev_attr.attr,
> &sensor_dev_attr_in0_min.dev_attr.attr,
> @@ -1476,9 +1503,12 @@
> }, {
> &sensor_dev_attr_in8_input.dev_attr.attr,
> NULL
> +}, {
> + &sensor_dev_attr_in9_input.dev_attr.attr,
> + NULL
> } };
>
> -static const struct attribute_group it87_group_in[9] = {
> +static const struct attribute_group it87_group_in[10] = {
> { .attrs = it87_attributes_in[0] },
> { .attrs = it87_attributes_in[1] },
> { .attrs = it87_attributes_in[2] },
> @@ -1488,6 +1518,7 @@
> { .attrs = it87_attributes_in[6] },
> { .attrs = it87_attributes_in[7] },
> { .attrs = it87_attributes_in[8] },
> + { .attrs = it87_attributes_in[9] },
> };
>
> static struct attribute *it87_attributes_temp[3][6] = {
> @@ -1685,6 +1716,7 @@
> &sensor_dev_attr_in3_label.dev_attr.attr,
> &sensor_dev_attr_in7_label.dev_attr.attr,
> &sensor_dev_attr_in8_label.dev_attr.attr,
> + &sensor_dev_attr_in9_label.dev_attr.attr,
> NULL
> };
>
> @@ -1742,6 +1774,9 @@
> case IT8783E_DEVID:
> sio_data->type = it8783;
> break;
> + case IT8306E_DEVID:
> + sio_data->type = it8603;
> + break;
> case 0xffff: /* No device at all */
> goto exit;
> default:
> @@ -1763,12 +1798,15 @@
>
> err = 0;
> sio_data->revision = superio_inb(DEVREV) & 0x0f;
> - pr_info("Found IT%04xF chip at 0x%x, revision %d\n",
> - chip_type, *address, sio_data->revision);
> + pr_info("Found IT%04x%c chip at 0x%x, revision %d\n", chip_type,
> + (chip_type == 0x8603) ? 'E' : 'F', *address,
> + sio_data->revision);
>
> /* in8 (Vbat) is always internal */
> sio_data->internal = (1 << 2);
>
> + sio_data->skip_in |= (1 << 9); /* No VIN9 */
> +
> /* Read GPIO config and VID value from LDN 7 (GPIO) */
> if (sio_data->type == it87) {
> /* The IT8705F doesn't have VID pins at all */
> @@ -1844,7 +1882,42 @@
> sio_data->internal |= (1 << 1);
>
> sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> + } if (sio_data->type == it8603) {
> + int reg27, reg29;
> +
> + sio_data->skip_vid = 1; /* No VID */
> + superio_select(GPIO);
>
> + reg27 = superio_inb(IT87_SIO_GPIO3_REG);
> +
> + /* Check if fan3 is there or not */
> + if (reg27 & (1 << 6))
> + sio_data->skip_pwm |= (1 << 2);
> + if (reg27 & (1 << 7))
> + sio_data->skip_fan |= (1 << 2);
> +
> + /* Check if fan2 is there or not */
> + reg29 = superio_inb(IT87_SIO_GPIO5_REG);
> + if (reg29 & (1 << 1))
> + sio_data->skip_pwm |= (1 << 1);
> + if (reg29 & (1 << 2))
> + sio_data->skip_fan |= (1 << 1);
> +
> + sio_data->skip_in |= (1 << 5); /* No VIN5 */
> + sio_data->skip_in |= (1 << 6); /* No VIN6 */
> +
> + /* no fan4 */
> + sio_data->skip_pwm |= (1 << 3);
> + sio_data->skip_fan |= (1 << 3);
> +
> + /* AVCC3 needs a special handling, map 0x2f to in9 */
> + sio_data->internal |= (1 << 3); /* in9 has label */
> + sio_data->skip_in &= ~(1 << 9); /* VIN9 mapped to 0x2f */
> +
> + sio_data->internal |= (1 << 1); /* in7 is VSB */
> + sio_data->internal |= (1 << 2); /* in8 is VBAT */
> +
> + sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> } else {
> int reg;
> bool uart6;
> @@ -1854,7 +1927,7 @@
> reg = superio_inb(IT87_SIO_GPIO3_REG);
> if (sio_data->type == it8721 || sio_data->type == it8728 ||
> sio_data->type == it8771 || sio_data->type == it8772 ||
> - sio_data->type == it8782) {
> + sio_data->type == it8782 || sio_data->type == it8603) {
> /*
> * IT8721F/IT8758E, and IT8782F don't have VID pins
> * at all, not sure about the IT8728F and compatibles.
> @@ -2072,6 +2145,10 @@
> /* Check PWM configuration */
> enable_pwm_interface = it87_check_pwm(dev);
>
> +
> + if (data->type == it8603)
> + data->in_scaled |= (1 << 9); /* in9 is AVCC3 */
> +
> /* Starting with IT8721F, we handle scaling of internal voltages */
> if (has_12mv_adc(data)) {
> if (sio_data->internal & (1 << 0))
> @@ -2102,7 +2179,7 @@
> if (err)
> return err;
>
> - for (i = 0; i < 9; i++) {
> + for (i = 0; i < 10; i++) {
> if (sio_data->skip_in & (1 << i))
> continue;
> err = sysfs_create_group(&dev->kobj, &it87_group_in[i]);
> @@ -2202,7 +2279,7 @@
> }
>
> /* Export labels for internal sensors */
> - for (i = 0; i < 3; i++) {
> + for (i = 0; i < 4; i++) {
> if (!(sio_data->internal & (1 << i)))
> continue;
> err = sysfs_create_file(&dev->kobj,
> @@ -2383,8 +2460,8 @@
> }
> data->has_fan = (data->fan_main_ctrl >> 4) & 0x07;
>
> - /* Set tachometers to 16-bit mode if needed */
> - if (has_16bit_fans(data)) {
> + /* Set tachometers to 16-bit mode if needed, IT8603E/IT8728? has it by default */
> + if ((has_16bit_fans(data)) && (data->type != it8603)) {
> tmp = it87_read_value(data, IT87_REG_FAN_16BIT);
> if (~tmp & 0x07 & data->has_fan) {
> dev_dbg(&pdev->dev,
> @@ -2462,6 +2539,10 @@
> data->in[i][2] =
> it87_read_value(data, IT87_REG_VIN_MAX(i));
> }
> +
> + if (data->type == it8603)
> + data->in[9][0] = it87_read_value(data, 0x2f);
> +
> /* in8 (battery) has no limit registers */
> data->in[8][0] = it87_read_value(data, IT87_REG_VIN(8));
>
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@xxxxxxxxxxxxxx
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
