All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] ITE it8603e
@ 2012-11-28 20:41 David Hubbard
  2012-11-29  5:05 ` Guenter Roeck
                   ` (27 more replies)
  0 siblings, 28 replies; 29+ messages in thread
From: David Hubbard @ 2012-11-28 20:41 UTC (permalink / raw)
  To: lm-sensors

Hi Jean,

Rudolf and I are collaborating on a new Asus board for coreboot.org,
the F2A85-M, which has a new ITE chip, the it8603e. sensors-detect
says:

Probing for Super-I/O at 0x2e/0x2f
Trying family `National Semiconductor/ITE'...               No
Trying family `SMSC'...                                     No
Trying family `VIA/Winbond/Nuvoton/Fintek'...               No
Trying family `ITE'...                                      Yes
Found unknown chip with ID 0x8603
    (logical device 4 has address 0x290, could be sensors)

I created a quick patch for sensors-detect to recognize the chip. I'd
like to start poking at it to see if the it87 driver can extract any
useful information (likely). Rudolf has requested the datasheet from
ITE, but in the likely event nothing happens, we thought it would be
worth asking you - you know people in the industry. I would be happy
to sign an NDA if that will help.

Thanks,
David

--- sensors-detect    2012-11-28 21:15:16.601391282 -0700
+++ sensors-detect    2012-11-28 21:34:24.392508598 -0700
@@ -2133,6 +2133,12 @@
         devid => 0x8772,
         logdev => 0x04,
         features => FEAT_IN | FEAT_FAN | FEAT_TEMP,
+    }, {
+        name => "ITE IT8603E Super IO Sensors",
+        driver => "to-be-written",    # it87
+        devid => 0x8603,
+        logdev => 0x04,
+        features => FEAT_IN | FEAT_FAN | FEAT_TEMP,
     }
 );

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
@ 2012-11-29  5:05 ` Guenter Roeck
  2012-11-29 10:09 ` David Hubbard
                   ` (26 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2012-11-29  5:05 UTC (permalink / raw)
  To: lm-sensors

On Wed, Nov 28, 2012 at 01:41:03PM -0700, David Hubbard wrote:
> Hi Jean,
> 
> Rudolf and I are collaborating on a new Asus board for coreboot.org,
> the F2A85-M, which has a new ITE chip, the it8603e. sensors-detect
> says:
> 
Hi David,

good luck with that ... I don't find any information about the chip except for
Rudolf's e-mail on the coreboot mailing list. I suspect it may be one of those
special vendor specific ITE chips like the IT877x, for which no documentation is
available at all (though someone in the coreboot project seems to have access
to the datasheet for IT8772F).

Guenter

> Probing for Super-I/O at 0x2e/0x2f
> Trying family `National Semiconductor/ITE'...               No
> Trying family `SMSC'...                                     No
> Trying family `VIA/Winbond/Nuvoton/Fintek'...               No
> Trying family `ITE'...                                      Yes
> Found unknown chip with ID 0x8603
>     (logical device 4 has address 0x290, could be sensors)
> 
> I created a quick patch for sensors-detect to recognize the chip. I'd
> like to start poking at it to see if the it87 driver can extract any
> useful information (likely). Rudolf has requested the datasheet from
> ITE, but in the likely event nothing happens, we thought it would be
> worth asking you - you know people in the industry. I would be happy
> to sign an NDA if that will help.
> 
> Thanks,
> David
> 
> --- sensors-detect    2012-11-28 21:15:16.601391282 -0700
> +++ sensors-detect    2012-11-28 21:34:24.392508598 -0700
> @@ -2133,6 +2133,12 @@
>          devid => 0x8772,
>          logdev => 0x04,
>          features => FEAT_IN | FEAT_FAN | FEAT_TEMP,
> +    }, {
> +        name => "ITE IT8603E Super IO Sensors",
> +        driver => "to-be-written",    # it87
> +        devid => 0x8603,
> +        logdev => 0x04,
> +        features => FEAT_IN | FEAT_FAN | FEAT_TEMP,
>      }
>  );
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
  2012-11-29  5:05 ` Guenter Roeck
@ 2012-11-29 10:09 ` David Hubbard
  2012-11-29 11:48 ` Jean Delvare
                   ` (25 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: David Hubbard @ 2012-11-29 10:09 UTC (permalink / raw)
  To: lm-sensors

Hi Jean, Guenter,

>> Rudolf and I are collaborating on a new Asus board for coreboot.org,
>> the F2A85-M, which has a new ITE chip, the it8603e. sensors-detect
>> says:
>>
> good luck with that ... I don't find any information about the chip except for
> Rudolf's e-mail on the coreboot mailing list. I suspect it may be one of those
> special vendor specific ITE chips like the IT877x, for which no documentation is
> available at all (though someone in the coreboot project seems to have access
> to the datasheet for IT8772F).
>
> Guenter

I'm sure I can figure it out. For example:

This adds experimental support for the it8603e chip found in the asus
f2a85-m motherboard.
Write only tested for pwmN and pwmN_enable.
Read tested, but the following appear broken:
  alarms
  fanN_alarm
  inN_alarm
  inN_max
  inN_min
  intrusionN_alarm
  pwmN_auto_channels_temp
  pwmN_freq

--- drivers/hwmon/it87.c    2012-11-28 21:50:32.218599922 -0700
+++ drivers/hwmon/it87.c    2012-11-29 10:55:31.576117770 -0700
@@ -21,10 +21,12 @@
  *            IT8758E  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
  *            Sis950   A clone of the IT8705F
  *
  *  Copyright (C) 2001 Chris Gauthron
  *  Copyright (C) 2005-2010 Jean Delvare <khali@linux-fr.org>
+ *  Copyright (C) 2012 David Hubbard <david.c.hubbard@gmail.com>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -62,7 +64,7 @@
 #define DRVNAME "it87"

 enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8782,
-         it8783 };
+         it8783, it8603 };

 static unsigned short force_id;
 module_param(force_id, ushort, 0);
@@ -142,6 +144,7 @@
 #define IT8728F_DEVID 0x8728
 #define IT8782F_DEVID 0x8782
 #define IT8783E_DEVID 0x8783
+#define IT8603E_DEVID 0x8603
 #define IT87_ACT_REG  0x30
 #define IT87_BASE_REG 0x60

@@ -303,7 +306,8 @@
      * on selected inputs.
      */
     return data->type = it8721
-        || data->type = it8728;
+        || data->type = it8728
+        || data->type = it8603;
 }

 static inline int has_newer_autopwm(const struct it87_data *data)
@@ -313,7 +317,8 @@
      * mapping and the manual duty cycle.
      */
     return data->type = it8721
-        || data->type = it8728;
+        || data->type = it8728
+        || data->type = it8603;
 }

 static int adc_lsb(const struct it87_data *data, int nr)
@@ -413,7 +418,8 @@
         || data->type = it8721
         || data->type = it8728
         || data->type = it8782
-        || data->type = it8783;
+        || data->type = it8783
+        || data->type = it8603;
 }

 static inline int has_old_autopwm(const struct it87_data *data)
@@ -1707,6 +1713,9 @@
     case IT8783E_DEVID:
         sio_data->type = it8783;
         break;
+    case IT8603E_DEVID:
+        sio_data->type = it8603;
+        break;
     case 0xffff:    /* No device at all */
         goto exit;
     default:
@@ -1728,8 +1737,8 @@

     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);
@@ -1818,10 +1827,12 @@

         reg = superio_inb(IT87_SIO_GPIO3_REG);
         if (sio_data->type = it8721 || sio_data->type = it8728 ||
-            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.
+             * at all, not sure about the IT8728F or IT8603E
+             * (IT8603E - "hwmon_vid: Unknown VRM version of your x86 CPU"
+             * and cpu0_vid reads as 0)
              */
             sio_data->skip_vid = 1;
         } else {
@@ -1986,6 +1997,7 @@
         "it8728",
         "it8782",
         "it8783",
+        "it8603",
     };

     res = platform_get_resource(pdev, IORESOURCE_IO, 0);

Regards,
David

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
  2012-11-29  5:05 ` Guenter Roeck
  2012-11-29 10:09 ` David Hubbard
@ 2012-11-29 11:48 ` Jean Delvare
  2012-11-29 18:00 ` David Hubbard
                   ` (24 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Jean Delvare @ 2012-11-29 11:48 UTC (permalink / raw)
  To: lm-sensors

On Thu, 29 Nov 2012 03:09:01 -0700, David Hubbard wrote:
> +             * at all, not sure about the IT8728F or IT8603E
> +             * (IT8603E - "hwmon_vid: Unknown VRM version of your x86 CPU"
> +             * and cpu0_vid reads as 0)

This has nothing to do with the IT8603E but only with your CPU. Which
kernel version are you running, and can we see (one entry in)
your /proc/cpuinfo?

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (2 preceding siblings ...)
  2012-11-29 11:48 ` Jean Delvare
@ 2012-11-29 18:00 ` David Hubbard
  2012-12-02 17:57 ` Jean Delvare
                   ` (23 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: David Hubbard @ 2012-11-29 18:00 UTC (permalink / raw)
  To: lm-sensors

Hi Jean,

On Thu, Nov 29, 2012 at 4:48 AM, Jean Delvare <khali@linux-fr.org> wrote:
> On Thu, 29 Nov 2012 03:09:01 -0700, David Hubbard wrote:
>> +             * at all, not sure about the IT8728F or IT8603E
>> +             * (IT8603E - "hwmon_vid: Unknown VRM version of your x86 CPU"
>> +             * and cpu0_vid reads as 0)
>
> This has nothing to do with the IT8603E but only with your CPU. Which
> kernel version are you running, and can we see (one entry in)
> your /proc/cpuinfo?

Yes. I am most interested in the datasheet, but I appreciate your help
to tweak the it87 driver as a fallback.

$ uname -a
Linux f2a85 3.6.6-gentoo #13 SMP Wed Nov 28 22:29:55 MST 2012 x86_64
AMD A10-5800K APU with Radeon(tm) HD Graphics AuthenticAMD GNU/Linux

$ cat /proc/cpuinfo
processor       : 0
vendor_id       : AuthenticAMD
cpu family      : 21
model           : 16
model name      : AMD A10-5800K APU with Radeon(tm) HD Graphics
stepping        : 1
microcode       : 0x6001116
cpu MHz         : 3800.000
cache size      : 2048 KB
physical id     : 0
siblings        : 4
core id         : 0
cpu cores       : 2
apicid          : 16
initial apicid  : 0
fpu             : yes
fpu_exception   : yes
cpuid level     : 13
wp              : yes
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge
mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext
fxsr_opt pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc
extd_apicid aperfmperf pni pclmulqdq monitor ssse3 fma cx16 sse4_1
sse4_2 popcnt aes xsave avx f16c lahf_lm cmp_legacy svm extapic
cr8_legacy abm sse4a misalignsse 3dnowprefetch osvw ibs xop skinit wdt
lwp fma4 tce nodeid_msr tbm topoext perfctr_core arat cpb hw_pstate
npt lbrv svm_lock nrip_save tsc_scale vmcb_clean flushbyasid
decodeassists pausefilflags       old bmi1
bogomips        : 7637.75
TLB size        : 1536 4K pages
clflush size  clflush size  clflush size  clflush size  clflush size
clflus, 48 bits virtual
power management: ts ttp tm 100mhzsteps hwpstate cpb eff_freq_ro


Thanks,
David

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (3 preceding siblings ...)
  2012-11-29 18:00 ` David Hubbard
@ 2012-12-02 17:57 ` Jean Delvare
  2012-12-02 18:26 ` Guenter Roeck
                   ` (22 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Jean Delvare @ 2012-12-02 17:57 UTC (permalink / raw)
  To: lm-sensors

Hi David,

On Wed, 28 Nov 2012 13:41:03 -0700, David Hubbard wrote:
> I created a quick patch for sensors-detect to recognize the chip. I'd
> like to start poking at it to see if the it87 driver can extract any
> useful information (likely). Rudolf has requested the datasheet from
> ITE, but in the likely event nothing happens, we thought it would be
> worth asking you - you know people in the industry. I would be happy
> to sign an NDA if that will help.

The information I was able to get from my contact at ITE:

* The IT8603E does supports hardware monitoring.

* The chip has a built-in 8-bit ADC, two thermal inputs with mode
  selection (thermistor / transistor), 5 external voltage inputs +
  3VSB, AVCC3 and VBAT measured internally. It features limits for all
  measured values (although probably not for VBAT) and fetching of
  external temperature values over SST/PECI/AMDSTI/PCH SM-Link.

* The IT8603E does support automatic fan speed control.

* The IT8603E is compatible to some degree with the IT87xxF family.

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (4 preceding siblings ...)
  2012-12-02 17:57 ` Jean Delvare
@ 2012-12-02 18:26 ` Guenter Roeck
  2012-12-02 18:52 ` Jean Delvare
                   ` (21 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2012-12-02 18:26 UTC (permalink / raw)
  To: lm-sensors

On Sun, Dec 02, 2012 at 06:57:28PM +0100, Jean Delvare wrote:
> Hi David,
> 
> On Wed, 28 Nov 2012 13:41:03 -0700, David Hubbard wrote:
> > I created a quick patch for sensors-detect to recognize the chip. I'd
> > like to start poking at it to see if the it87 driver can extract any
> > useful information (likely). Rudolf has requested the datasheet from
> > ITE, but in the likely event nothing happens, we thought it would be
> > worth asking you - you know people in the industry. I would be happy
> > to sign an NDA if that will help.
> 
> The information I was able to get from my contact at ITE:
> 
> * The IT8603E does supports hardware monitoring.
> 
> * The chip has a built-in 8-bit ADC, two thermal inputs with mode
>   selection (thermistor / transistor), 5 external voltage inputs +
>   3VSB, AVCC3 and VBAT measured internally. It features limits for all
>   measured values (although probably not for VBAT) and fetching of
>   external temperature values over SST/PECI/AMDSTI/PCH SM-Link.
> 
> * The IT8603E does support automatic fan speed control.
> 
> * The IT8603E is compatible to some degree with the IT87xxF family.
> 
Will that be good enough for us ? We know way more about IT8771E and IT8772E,
mostly thanks to the code in coreboot and ohm, yet so far declined to add
support for it because we don't have data sheets. Or is it time to add support
for those chips as well, assuming they are compatible to IT8728F ?

Guenter

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (5 preceding siblings ...)
  2012-12-02 18:26 ` Guenter Roeck
@ 2012-12-02 18:52 ` Jean Delvare
  2012-12-02 19:20 ` Guenter Roeck
                   ` (20 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Jean Delvare @ 2012-12-02 18:52 UTC (permalink / raw)
  To: lm-sensors

On Sun, 2 Dec 2012 10:26:51 -0800, Guenter Roeck wrote:
> On Sun, Dec 02, 2012 at 06:57:28PM +0100, Jean Delvare wrote:
> > The information I was able to get from my contact at ITE:
> > 
> > * The IT8603E does supports hardware monitoring.
> > 
> > * The chip has a built-in 8-bit ADC, two thermal inputs with mode
> >   selection (thermistor / transistor), 5 external voltage inputs +
> >   3VSB, AVCC3 and VBAT measured internally. It features limits for all
> >   measured values (although probably not for VBAT) and fetching of
> >   external temperature values over SST/PECI/AMDSTI/PCH SM-Link.
> > 
> > * The IT8603E does support automatic fan speed control.
> > 
> > * The IT8603E is compatible to some degree with the IT87xxF family.
>
> Will that be good enough for us ? We know way more about IT8771E and IT8772E,
> mostly thanks to the code in coreboot and ohm, yet so far declined to add
> support for it because we don't have data sheets. Or is it time to add support
> for those chips as well, assuming they are compatible to IT8728F ?

I have no objection if you feel confident.

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (6 preceding siblings ...)
  2012-12-02 18:52 ` Jean Delvare
@ 2012-12-02 19:20 ` Guenter Roeck
  2012-12-03  8:44 ` Jean Delvare
                   ` (19 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2012-12-02 19:20 UTC (permalink / raw)
  To: lm-sensors

On Sun, Dec 02, 2012 at 07:52:37PM +0100, Jean Delvare wrote:
> On Sun, 2 Dec 2012 10:26:51 -0800, Guenter Roeck wrote:
> > On Sun, Dec 02, 2012 at 06:57:28PM +0100, Jean Delvare wrote:
> > > The information I was able to get from my contact at ITE:
> > > 
> > > * The IT8603E does supports hardware monitoring.
> > > 
> > > * The chip has a built-in 8-bit ADC, two thermal inputs with mode
> > >   selection (thermistor / transistor), 5 external voltage inputs +
> > >   3VSB, AVCC3 and VBAT measured internally. It features limits for all
> > >   measured values (although probably not for VBAT) and fetching of
> > >   external temperature values over SST/PECI/AMDSTI/PCH SM-Link.
> > > 
> > > * The IT8603E does support automatic fan speed control.
> > > 
> > > * The IT8603E is compatible to some degree with the IT87xxF family.
> >
> > Will that be good enough for us ? We know way more about IT8771E and IT8772E,
> > mostly thanks to the code in coreboot and ohm, yet so far declined to add
> > support for it because we don't have data sheets. Or is it time to add support
> > for those chips as well, assuming they are compatible to IT8728F ?
> 
> I have no objection if you feel confident.
> 
As confident as we can get w/o datasheets. Only open question is pin
assignments, which I assumed to be compatible w/ IT8728F. I sent the patches a
minute ago.

Guenter

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (7 preceding siblings ...)
  2012-12-02 19:20 ` Guenter Roeck
@ 2012-12-03  8:44 ` Jean Delvare
  2012-12-03  8:56 ` Rudolf Marek
                   ` (18 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Jean Delvare @ 2012-12-03  8:44 UTC (permalink / raw)
  To: lm-sensors

On Sun, 2 Dec 2012 18:57:28 +0100, Jean Delvare wrote:
> On Wed, 28 Nov 2012 13:41:03 -0700, David Hubbard wrote:
> > I created a quick patch for sensors-detect to recognize the chip. I'd
> > like to start poking at it to see if the it87 driver can extract any
> > useful information (likely). Rudolf has requested the datasheet from
> > ITE, but in the likely event nothing happens, we thought it would be
> > worth asking you - you know people in the industry. I would be happy
> > to sign an NDA if that will help.
> 
> The information I was able to get from my contact at ITE:
> 
> * The IT8603E does supports hardware monitoring.
> 
> * The chip has a built-in 8-bit ADC, two thermal inputs with mode
>   selection (thermistor / transistor), 5 external voltage inputs +
>   3VSB, AVCC3 and VBAT measured internally. It features limits for all
>   measured values (although probably not for VBAT) and fetching of
>   external temperature values over SST/PECI/AMDSTI/PCH SM-Link.
> 
> * The IT8603E does support automatic fan speed control.
> 
> * The IT8603E is compatible to some degree with the IT87xxF family.

In addition: this is a custom part for a specific customer (= board
maker.) My contact says ITE can't disclose the datasheet for such parts
and it should be requested from the board designer.

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (8 preceding siblings ...)
  2012-12-03  8:44 ` Jean Delvare
@ 2012-12-03  8:56 ` Rudolf Marek
  2012-12-03 14:35 ` Guenter Roeck
                   ` (17 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Rudolf Marek @ 2012-12-03  8:56 UTC (permalink / raw)
  To: lm-sensors


> In addition: this is a custom part for a specific customer (= board
> maker.) My contact says ITE can't disclose the datasheet for such parts
> and it should be requested from the board designer.
>    

I'm working on that.

Thanks
Rudolf


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (9 preceding siblings ...)
  2012-12-03  8:56 ` Rudolf Marek
@ 2012-12-03 14:35 ` Guenter Roeck
  2012-12-12 14:45 ` Jean Delvare
                   ` (16 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2012-12-03 14:35 UTC (permalink / raw)
  To: lm-sensors

On Mon, Dec 03, 2012 at 09:44:30AM +0100, Jean Delvare wrote:
> On Sun, 2 Dec 2012 18:57:28 +0100, Jean Delvare wrote:
> > On Wed, 28 Nov 2012 13:41:03 -0700, David Hubbard wrote:
> > > I created a quick patch for sensors-detect to recognize the chip. I'd
> > > like to start poking at it to see if the it87 driver can extract any
> > > useful information (likely). Rudolf has requested the datasheet from
> > > ITE, but in the likely event nothing happens, we thought it would be
> > > worth asking you - you know people in the industry. I would be happy
> > > to sign an NDA if that will help.
> > 
> > The information I was able to get from my contact at ITE:
> > 
> > * The IT8603E does supports hardware monitoring.
> > 
> > * The chip has a built-in 8-bit ADC, two thermal inputs with mode
> >   selection (thermistor / transistor), 5 external voltage inputs +
> >   3VSB, AVCC3 and VBAT measured internally. It features limits for all
> >   measured values (although probably not for VBAT) and fetching of
> >   external temperature values over SST/PECI/AMDSTI/PCH SM-Link.
> > 
> > * The IT8603E does support automatic fan speed control.
> > 
> > * The IT8603E is compatible to some degree with the IT87xxF family.
> 
> In addition: this is a custom part for a specific customer (= board
> maker.) My contact says ITE can't disclose the datasheet for such parts
> and it should be requested from the board designer.
> 
Heard the same before, for IT8771E and ASUS. And ASUS doesn't provide it.

Guenter

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (10 preceding siblings ...)
  2012-12-03 14:35 ` Guenter Roeck
@ 2012-12-12 14:45 ` Jean Delvare
  2012-12-13  7:58 ` David Hubbard
                   ` (15 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Jean Delvare @ 2012-12-12 14:45 UTC (permalink / raw)
  To: lm-sensors

Hi David,

On Thu, 29 Nov 2012 11:00:21 -0700, David Hubbard wrote:
> Hi Jean,
> 
> On Thu, Nov 29, 2012 at 4:48 AM, Jean Delvare <khali@linux-fr.org> wrote:
> > On Thu, 29 Nov 2012 03:09:01 -0700, David Hubbard wrote:
> >> +             * at all, not sure about the IT8728F or IT8603E
> >> +             * (IT8603E - "hwmon_vid: Unknown VRM version of your x86 CPU"
> >> +             * and cpu0_vid reads as 0)
> >
> > This has nothing to do with the IT8603E but only with your CPU. Which
> > kernel version are you running, and can we see (one entry in)
> > your /proc/cpuinfo?
> 
> Yes. I am most interested in the datasheet, but I appreciate your help
> to tweak the it87 driver as a fallback.
> 
> $ uname -a
> Linux f2a85 3.6.6-gentoo #13 SMP Wed Nov 28 22:29:55 MST 2012 x86_64
> AMD A10-5800K APU with Radeon(tm) HD Graphics AuthenticAMD GNU/Linux
> 
> $ cat /proc/cpuinfo
> processor       : 0
> vendor_id       : AuthenticAMD
> cpu family      : 21
> model           : 16
> model name      : AMD A10-5800K APU with Radeon(tm) HD Graphics
> stepping        : 1

We do not support any VID decoding since family 10h CPUs for AMD:

	{X86_VENDOR_AMD, 0x10, 0x0, ANY, ANY, 25},	/* NPT family 10h */

Starting with family 11h CPUs, AMD has given up on parallel VID in
favor of serial VID which uses only 2 pins. I know that the IT8720F has
support for this, but I did not find any trace in other ITE datasheets.

The serial VID uses 7-bit codes which are incompatible with the
previous 6-bit parallel VID codes used by family 10h CPUs. BTW the
family 10h CPUs already supported serial VID, and I have no idea how to
support that properly as hwmon-vid has no idea whether the code came
from the serial or parallel VID interface.

The following should at least get rid of the warning in the kernel logs:

* * * * *

Since family 11h processors, AMD is exclusively using 7-bit VID codes
transmitted using a serial protocol over two pins (clock and data.)

Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
 drivers/hwmon/hwmon-vid.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

--- linux-3.7-rc8.orig/drivers/hwmon/hwmon-vid.c	2012-10-20 21:11:32.000000000 +0200
+++ linux-3.7-rc8/drivers/hwmon/hwmon-vid.c	2012-12-12 15:36:36.309882929 +0100
@@ -115,6 +115,12 @@ int vid_from_reg(int val, u8 vrm)
 		return (val < 32) ? 1550 - 25 * val
 			: 775 - (25 * (val - 31)) / 2;
 
+	case 26:		/* AMD family 10h to 15h, serial VID */
+		val &= 0x7f;
+		if (val >= 0x7c)
+			return 0;
+		return DIV_ROUND_CLOSEST(15500 - 125 * val, 10);
+
 	case 91:		/* VRM 9.1 */
 	case 90:		/* VRM 9.0 */
 		val &= 0x1f;
@@ -195,6 +201,10 @@ static struct vrm_model vrm_models[] = {
 	{X86_VENDOR_AMD, 0xF, 0x40, 0x7F, ANY, 24},	/* NPT family 0Fh */
 	{X86_VENDOR_AMD, 0xF, 0x80, ANY, ANY, 25},	/* future fam. 0Fh */
 	{X86_VENDOR_AMD, 0x10, 0x0, ANY, ANY, 25},	/* NPT family 10h */
+	{X86_VENDOR_AMD, 0x11, 0x0, ANY, ANY, 26},	/* family 11h */
+	{X86_VENDOR_AMD, 0x12, 0x0, ANY, ANY, 26},	/* family 12h */
+	{X86_VENDOR_AMD, 0x14, 0x0, ANY, ANY, 26},	/* family 14h */
+	{X86_VENDOR_AMD, 0x15, 0x0, ANY, ANY, 26},	/* family 15h */
 
 	{X86_VENDOR_INTEL, 0x6, 0x0, 0x6, ANY, 82},	/* Pentium Pro,
 							 * Pentium II, Xeon,

Also available as a standalone driver at:
  http://khali.linux-fr.org/devel/misc/hwmon-vid/

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (11 preceding siblings ...)
  2012-12-12 14:45 ` Jean Delvare
@ 2012-12-13  7:58 ` David Hubbard
  2012-12-13  8:00 ` David Hubbard
                   ` (14 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: David Hubbard @ 2012-12-13  7:58 UTC (permalink / raw)
  To: lm-sensors


[-- Attachment #1.1: Type: text/plain, Size: 6994 bytes --]

Hi Jean,

On Wed, Dec 12, 2012 at 7:45 AM, Jean Delvare <khali@linux-fr.org> wrote:

> > >> +             * at all, not sure about the IT8728F or IT8603E
> > >> +             * (IT8603E - "hwmon_vid: Unknown VRM version of your
> x86 CPU"
> > >> +             * and cpu0_vid reads as 0)
> > >
> > > This has nothing to do with the IT8603E but only with your CPU. Which
> > > kernel version are you running, and can we see (one entry in)
> > > your /proc/cpuinfo?
> > $ cat /proc/cpuinfo
> > processor       : 0
> > vendor_id       : AuthenticAMD
> > cpu family      : 21
> > model           : 16
> > model name      : AMD A10-5800K APU with Radeon(tm) HD Graphics
> > stepping        : 1
> We do not support any VID decoding since family 10h CPUs for AMD:
>
>         {X86_VENDOR_AMD, 0x10, 0x0, ANY, ANY, 25},      /* NPT family 10h
> */
>
> Starting with family 11h CPUs, AMD has given up on parallel VID in
> favor of serial VID which uses only 2 pins. I know that the IT8720F has
> support for this, but I did not find any trace in other ITE datasheets.
>
> The serial VID uses 7-bit codes which are incompatible with the
> previous 6-bit parallel VID codes used by family 10h CPUs. BTW the
> family 10h CPUs already supported serial VID, and I have no idea how to
> support that properly as hwmon-vid has no idea whether the code came
> from the serial or parallel VID interface.
>
> The following should at least get rid of the warning in the kernel logs:
>
> * * * * *
>
> Since family 11h processors, AMD is exclusively using 7-bit VID codes
> transmitted using a serial protocol over two pins (clock and data.)
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
>  drivers/hwmon/hwmon-vid.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> --- linux-3.7-rc8.orig/drivers/hwmon/hwmon-vid.c        2012-10-20
> 21:11:32.000000000 +0200
> +++ linux-3.7-rc8/drivers/hwmon/hwmon-vid.c     2012-12-12
> 15:36:36.309882929 +0100
> @@ -115,6 +115,12 @@ int vid_from_reg(int val, u8 vrm)
>                 return (val < 32) ? 1550 - 25 * val
>                         : 775 - (25 * (val - 31)) / 2;
>
> +       case 26:                /* AMD family 10h to 15h, serial VID */
> +               val &= 0x7f;
> +               if (val >= 0x7c)
> +                       return 0;
> +               return DIV_ROUND_CLOSEST(15500 - 125 * val, 10);
> +
>         case 91:                /* VRM 9.1 */
>         case 90:                /* VRM 9.0 */
>                 val &= 0x1f;
> @@ -195,6 +201,10 @@ static struct vrm_model vrm_models[] = {
>         {X86_VENDOR_AMD, 0xF, 0x40, 0x7F, ANY, 24},     /* NPT family 0Fh
> */
>         {X86_VENDOR_AMD, 0xF, 0x80, ANY, ANY, 25},      /* future fam. 0Fh
> */
>         {X86_VENDOR_AMD, 0x10, 0x0, ANY, ANY, 25},      /* NPT family 10h
> */
> +       {X86_VENDOR_AMD, 0x11, 0x0, ANY, ANY, 26},      /* family 11h */
> +       {X86_VENDOR_AMD, 0x12, 0x0, ANY, ANY, 26},      /* family 12h */
> +       {X86_VENDOR_AMD, 0x14, 0x0, ANY, ANY, 26},      /* family 14h */
> +       {X86_VENDOR_AMD, 0x15, 0x0, ANY, ANY, 26},      /* family 15h */
>
>         {X86_VENDOR_INTEL, 0x6, 0x0, 0x6, ANY, 82},     /* Pentium Pro,
>                                                          * Pentium II,
> Xeon,
>
> Also available as a standalone driver at:
>   http://khali.linux-fr.org/devel/misc/hwmon-vid/
>
>
After applying the patch you sent cpu0_vid reads as 1550. Thanks much, that
was very helpful.

Hopefully here is an updated patch ready to add IT8603E support in the it87
driver. There is one question maybe still worth asking, namely: temp3_input
- should this be disabled because the chip only has 2 analog temperature
inputs?

Is the SST/PECI/AMDSTI/PCH SM-Link read using temp3_input? Otherwise I
assume it is outside the scope of the it87 driver?

* * * * *

Add experimental support for the it8603e chip (Asus f2a85-m motherboard)
Write only tested for pwmN and pwmN_enable.
Read tested, but the following appear broken:
  alarms
  fanN_alarm
  inN_alarm
  inN_max
  inN_min
  intrusionN_alarm
  pwmN_auto_channels_temp
  pwmN_freq
  temp3_input (there is no 3rd analog temp input to the chip)

--- a/drivers/hwmon/it87.c    2012-12-10 02:51:13.680000001 -0700
+++ b/drivers/hwmon/it87.c    2012-12-13 08:42:54.195515406 -0700
@@ -21,10 +21,12 @@
  *            IT8758E  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
  *            Sis950   A clone of the IT8705F
  *
  *  Copyright (C) 2001 Chris Gauthron
  *  Copyright (C) 2005-2010 Jean Delvare <khali@linux-fr.org>
+ *  Copyright (C) 2012 David Hubbard <david.c.hubbard@gmail.com>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -62,7 +64,7 @@
 #define DRVNAME "it87"

 enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8782,
-         it8783 };
+         it8783, it8603 };

 static unsigned short force_id;
 module_param(force_id, ushort, 0);
@@ -142,6 +144,7 @@
 #define IT8728F_DEVID 0x8728
 #define IT8782F_DEVID 0x8782
 #define IT8783E_DEVID 0x8783
+#define IT8603E_DEVID 0x8603
 #define IT87_ACT_REG  0x30
 #define IT87_BASE_REG 0x60

@@ -303,7 +306,8 @@
      * on selected inputs.
      */
     return data->type == it8721
-        || data->type == it8728;
+        || data->type == it8728
+        || data->type == it8603;
 }

 static inline int has_newer_autopwm(const struct it87_data *data)
@@ -313,7 +317,8 @@
      * mapping and the manual duty cycle.
      */
     return data->type == it8721
-        || data->type == it8728;
+        || data->type == it8728
+        || data->type == it8603;
 }

 static int adc_lsb(const struct it87_data *data, int nr)
@@ -413,7 +418,8 @@
         || data->type == it8721
         || data->type == it8728
         || data->type == it8782
-        || data->type == it8783;
+        || data->type == it8783
+        || data->type == it8603;
 }

 static inline int has_old_autopwm(const struct it87_data *data)
@@ -1707,6 +1713,9 @@
     case IT8783E_DEVID:
         sio_data->type = it8783;
         break;
+    case IT8603E_DEVID:
+        sio_data->type = it8603;
+        break;
     case 0xffff:    /* No device at all */
         goto exit;
     default:
@@ -1728,8 +1737,8 @@

     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);
@@ -1986,6 +1995,7 @@
         "it8728",
         "it8782",
         "it8783",
+        "it8603",
     };

     res = platform_get_resource(pdev, IORESOURCE_IO, 0);

[-- Attachment #1.2: Type: text/html, Size: 8433 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (12 preceding siblings ...)
  2012-12-13  7:58 ` David Hubbard
@ 2012-12-13  8:00 ` David Hubbard
  2012-12-13 16:07 ` Guenter Roeck
                   ` (13 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: David Hubbard @ 2012-12-13  8:00 UTC (permalink / raw)
  To: lm-sensors


[-- Attachment #1.1: Type: text/plain, Size: 3328 bytes --]

I'd like to send the same patch again but add signed-off-by this time :) I
apologize.

* * * * *

Add experimental support for the it8603e chip (Asus f2a85-m motherboard)
Write only tested for pwmN and pwmN_enable.
Read tested, but the following appear broken:
  alarms
  fanN_alarm
  inN_alarm
  inN_max
  inN_min
  intrusionN_alarm
  pwmN_auto_channels_temp
  pwmN_freq
  temp3_input (there is no 3rd analog temp input to the chip)

Signed-off-by: David Hubbard <david.c.hubbard@gmail.com>

--- a/drivers/hwmon/it87.c    2012-12-10 02:51:13.680000001 -0700
+++ b/drivers/hwmon/it87.c    2012-12-13 08:42:54.195515406 -0700
@@ -21,10 +21,12 @@
  *            IT8758E  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
  *            Sis950   A clone of the IT8705F
  *
  *  Copyright (C) 2001 Chris Gauthron
  *  Copyright (C) 2005-2010 Jean Delvare <khali@linux-fr.org>
+ *  Copyright (C) 2012 David Hubbard <david.c.hubbard@gmail.com>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -62,7 +64,7 @@
 #define DRVNAME "it87"

 enum chips { it87, it8712, it8716, it8718, it8720, it8721, it8728, it8782,
-         it8783 };
+         it8783, it8603 };

 static unsigned short force_id;
 module_param(force_id, ushort, 0);
@@ -142,6 +144,7 @@
 #define IT8728F_DEVID 0x8728
 #define IT8782F_DEVID 0x8782
 #define IT8783E_DEVID 0x8783
+#define IT8603E_DEVID 0x8603
 #define IT87_ACT_REG  0x30
 #define IT87_BASE_REG 0x60

@@ -303,7 +306,8 @@
      * on selected inputs.
      */
     return data->type == it8721
-        || data->type == it8728;
+        || data->type == it8728
+        || data->type == it8603;
 }

 static inline int has_newer_autopwm(const struct it87_data *data)
@@ -313,7 +317,8 @@
      * mapping and the manual duty cycle.
      */
     return data->type == it8721
-        || data->type == it8728;
+        || data->type == it8728
+        || data->type == it8603;
 }

 static int adc_lsb(const struct it87_data *data, int nr)
@@ -413,7 +418,8 @@
         || data->type == it8721
         || data->type == it8728
         || data->type == it8782
-        || data->type == it8783;
+        || data->type == it8783
+        || data->type == it8603;
 }

 static inline int has_old_autopwm(const struct it87_data *data)
@@ -1707,6 +1713,9 @@
     case IT8783E_DEVID:
         sio_data->type = it8783;
         break;
+    case IT8603E_DEVID:
+        sio_data->type = it8603;
+        break;
     case 0xffff:    /* No device at all */
         goto exit;
     default:
@@ -1728,8 +1737,8 @@

     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);
@@ -1986,6 +1995,7 @@
         "it8728",
         "it8782",
         "it8783",
+        "it8603",
     };

     res = platform_get_resource(pdev, IORESOURCE_IO, 0);

[-- Attachment #1.2: Type: text/html, Size: 4015 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (13 preceding siblings ...)
  2012-12-13  8:00 ` David Hubbard
@ 2012-12-13 16:07 ` Guenter Roeck
  2012-12-13 21:27 ` David Hubbard
                   ` (12 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2012-12-13 16:07 UTC (permalink / raw)
  To: lm-sensors

On Thu, Dec 13, 2012 at 01:00:30AM -0700, David Hubbard wrote:
> I'd like to send the same patch again but add signed-off-by this time :) I
> apologize.
> 
> * * * * *
> 
> Add experimental support for the it8603e chip (Asus f2a85-m motherboard)
> Write only tested for pwmN and pwmN_enable.
> Read tested, but the following appear broken:
>   alarms
>   fanN_alarm
>   inN_alarm
>   inN_max
>   inN_min
>   intrusionN_alarm
>   pwmN_auto_channels_temp
>   pwmN_freq

I don't think it is a good idea to report attributes which "appear to be
broken". At least for the voltage limits and the alarms it should be relatively
easy to find out if they work/don't work. Whatever doesn't work should not be
there.

>   temp3_input (there is no 3rd analog temp input to the chip)
> 
Other IT87xx chips report the AMDSTI/PCH temperature as one of the inputs. This
is usually configurable.

Another question is to what extend we can depend on the logic in it87_find() to
detect enabled chip features. It might make more sense to create another if case
in that function to handle the IT8603 separately.

Can you improve superiotool to display the chip's registers, and/or create a
manual register dump ? Maybe that would help identifying some of the missing
pieces.

Guenter

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (14 preceding siblings ...)
  2012-12-13 16:07 ` Guenter Roeck
@ 2012-12-13 21:27 ` David Hubbard
  2012-12-13 22:13 ` Guenter Roeck
                   ` (11 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: David Hubbard @ 2012-12-13 21:27 UTC (permalink / raw)
  To: lm-sensors


[-- Attachment #1.1: Type: text/plain, Size: 1626 bytes --]

Hi Guenter,

On Thu, Dec 13, 2012 at 9:07 AM, Guenter Roeck <groeck-dsl@sbcglobal.net>wrote:

> On Thu, Dec 13, 2012 at 01:00:30AM -0700, David Hubbard wrote:
> > I'd like to send the same patch again but add signed-off-by this time :)
> I
> > apologize.
> >
> > * * * * *
> >
> > Add experimental support for the it8603e chip (Asus f2a85-m motherboard)
> > Write only tested for pwmN and pwmN_enable.
> > Read tested, but the following appear broken:
> >   alarms
> >   fanN_alarm
> >   inN_alarm
> >   inN_max
> >   inN_min
> >   intrusionN_alarm
> >   pwmN_auto_channels_temp
> >   pwmN_freq
>
> I don't think it is a good idea to report attributes which "appear to be
> broken". At least for the voltage limits and the alarms it should be
> relatively
> easy to find out if they work/don't work. Whatever doesn't work should not
> be
> there.
>

They don't work but I don't know why. Until we have a datasheet, I'd like
to leave them in the *experimental* driver and wait for a second opinion
from someone who actually has the chip.


>
> >   temp3_input (there is no 3rd analog temp input to the chip)
> >
> Other IT87xx chips report the AMDSTI/PCH temperature as one of the inputs.
> This
> is usually configurable.
>
>
Ok, fine with me.

Another question is to what extend we can depend on the logic in
> it87_find() to
> detect enabled chip features. It might make more sense to create another
> if case
> in that function to handle the IT8603 separately.
>

it87_find() seems to have identified the correct features of the IT8603. I
don't understand your concern with the way it is currently working.

Regards,
David

[-- Attachment #1.2: Type: text/html, Size: 2450 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (15 preceding siblings ...)
  2012-12-13 21:27 ` David Hubbard
@ 2012-12-13 22:13 ` Guenter Roeck
  2012-12-13 22:40 ` David Hubbard
                   ` (10 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2012-12-13 22:13 UTC (permalink / raw)
  To: lm-sensors

On Thu, Dec 13, 2012 at 02:27:57PM -0700, David Hubbard wrote:
> Hi Guenter,
> 
> On Thu, Dec 13, 2012 at 9:07 AM, Guenter Roeck <groeck-dsl@sbcglobal.net>wrote:
> 
> > On Thu, Dec 13, 2012 at 01:00:30AM -0700, David Hubbard wrote:
> > > I'd like to send the same patch again but add signed-off-by this time :)
> > I
> > > apologize.
> > >
> > > * * * * *
> > >
> > > Add experimental support for the it8603e chip (Asus f2a85-m motherboard)
> > > Write only tested for pwmN and pwmN_enable.
> > > Read tested, but the following appear broken:
> > >   alarms
> > >   fanN_alarm
> > >   inN_alarm
> > >   inN_max
> > >   inN_min
> > >   intrusionN_alarm
> > >   pwmN_auto_channels_temp
> > >   pwmN_freq
> >
> > I don't think it is a good idea to report attributes which "appear to be
> > broken". At least for the voltage limits and the alarms it should be
> > relatively
> > easy to find out if they work/don't work. Whatever doesn't work should not
> > be
> > there.
> >
> 
> They don't work but I don't know why. Until we have a datasheet, I'd like
> to leave them in the *experimental* driver and wait for a second opinion
> from someone who actually has the chip.
> 
> 
> >
> > >   temp3_input (there is no 3rd analog temp input to the chip)
> > >
> > Other IT87xx chips report the AMDSTI/PCH temperature as one of the inputs.
> > This
> > is usually configurable.
> >
> >
> Ok, fine with me.
> 
> Another question is to what extend we can depend on the logic in
> > it87_find() to
> > detect enabled chip features. It might make more sense to create another
> > if case
> > in that function to handle the IT8603 separately.
> >
> 
> it87_find() seems to have identified the correct features of the IT8603. I
> don't understand your concern with the way it is currently working.
> 
"they don't work ... " and "identified the correct features" seems to
somewhat contradict each other.

Guenter

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (16 preceding siblings ...)
  2012-12-13 22:13 ` Guenter Roeck
@ 2012-12-13 22:40 ` David Hubbard
  2012-12-16 10:15 ` Jean Delvare
                   ` (9 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: David Hubbard @ 2012-12-13 22:40 UTC (permalink / raw)
  To: lm-sensors


[-- Attachment #1.1: Type: text/plain, Size: 2709 bytes --]

Hi Guenter,

On Thu, Dec 13, 2012 at 3:13 PM, Guenter Roeck <linux@roeck-us.net> wrote:

> On Thu, Dec 13, 2012 at 02:27:57PM -0700, David Hubbard wrote:
> > Hi Guenter,
> >
> > On Thu, Dec 13, 2012 at 9:07 AM, Guenter Roeck <groeck-dsl@sbcglobal.net
> >wrote:
> >
> > > On Thu, Dec 13, 2012 at 01:00:30AM -0700, David Hubbard wrote:
> > > > I'd like to send the same patch again but add signed-off-by this
> time :)
> > > I
> > > > apologize.
> > > >
> > > > * * * * *
> > > >
> > > > Add experimental support for the it8603e chip (Asus f2a85-m
> motherboard)
> > > > Write only tested for pwmN and pwmN_enable.
> > > > Read tested, but the following appear broken:
> > > >   alarms
> > > >   fanN_alarm
> > > >   inN_alarm
> > > >   inN_max
> > > >   inN_min
> > > >   intrusionN_alarm
> > > >   pwmN_auto_channels_temp
> > > >   pwmN_freq
> > >
> > > I don't think it is a good idea to report attributes which "appear to
> be
> > > broken". At least for the voltage limits and the alarms it should be
> > > relatively
> > > easy to find out if they work/don't work. Whatever doesn't work should
> not
> > > be
> > > there.
> > >
> >
> > They don't work but I don't know why. Until we have a datasheet, I'd like
> > to leave them in the *experimental* driver and wait for a second opinion
> > from someone who actually has the chip.
> >
> >
> > >
> > > >   temp3_input (there is no 3rd analog temp input to the chip)
> > > >
> > > Other IT87xx chips report the AMDSTI/PCH temperature as one of the
> inputs.
> > > This
> > > is usually configurable.
> > >
> > >
> > Ok, fine with me.
> >
> > Another question is to what extend we can depend on the logic in
> > > it87_find() to
> > > detect enabled chip features. It might make more sense to create
> another
> > > if case
> > > in that function to handle the IT8603 separately.
> > >
> >
> > it87_find() seems to have identified the correct features of the IT8603.
> I
> > don't understand your concern with the way it is currently working.
> >
> "they don't work ... " and "identified the correct features" seems to
> somewhat contradict each other.
>
>
Oh, okay. That's easy. "They don't work," but "it87_find() seems to have
identified the correct features" -- the features exist, independently
confirmed by the OEM Asus BIOS. So it87_find() reports data consistent with
the Asus BIOS. I'd like to leave them in the experimental driver because I
think it87_find() is operating correctly.

All that needs to be done to get them to work is a datasheet or equivalent
technical description of the I/O interface. I don't think Asus is going to
provide one, but another motherboard with the same chip could be extremely
useful for A/B testing.

Thanks,
David

[-- Attachment #1.2: Type: text/html, Size: 3750 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (17 preceding siblings ...)
  2012-12-13 22:40 ` David Hubbard
@ 2012-12-16 10:15 ` Jean Delvare
  2012-12-16 16:54 ` Guenter Roeck
                   ` (8 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Jean Delvare @ 2012-12-16 10:15 UTC (permalink / raw)
  To: lm-sensors

Hi David,

On Thu, 13 Dec 2012 00:58:35 -0700, David Hubbard wrote:
> On Wed, Dec 12, 2012 at 7:45 AM, Jean Delvare <khali@linux-fr.org> wrote:
> > We do not support any VID decoding since family 10h CPUs for AMD:
> >
> >         {X86_VENDOR_AMD, 0x10, 0x0, ANY, ANY, 25},      /* NPT family 10h
> > */
> >
> > Starting with family 11h CPUs, AMD has given up on parallel VID in
> > favor of serial VID which uses only 2 pins. I know that the IT8720F has
> > support for this, but I did not find any trace in other ITE datasheets.
> >
> > The serial VID uses 7-bit codes which are incompatible with the
> > previous 6-bit parallel VID codes used by family 10h CPUs. BTW the
> > family 10h CPUs already supported serial VID, and I have no idea how to
> > support that properly as hwmon-vid has no idea whether the code came
> > from the serial or parallel VID interface.
> >
> > The following should at least get rid of the warning in the kernel logs:
> >
> > * * * * *
> >
> > Since family 11h processors, AMD is exclusively using 7-bit VID codes
> > transmitted using a serial protocol over two pins (clock and data.)
> >
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > ---
> >  drivers/hwmon/hwmon-vid.c |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > --- linux-3.7-rc8.orig/drivers/hwmon/hwmon-vid.c        2012-10-20
> > 21:11:32.000000000 +0200
> > +++ linux-3.7-rc8/drivers/hwmon/hwmon-vid.c     2012-12-12
> > 15:36:36.309882929 +0100
> > @@ -115,6 +115,12 @@ int vid_from_reg(int val, u8 vrm)
> >                 return (val < 32) ? 1550 - 25 * val
> >                         : 775 - (25 * (val - 31)) / 2;
> >
> > +       case 26:                /* AMD family 10h to 15h, serial VID */
> > +               val &= 0x7f;
> > +               if (val >= 0x7c)
> > +                       return 0;
> > +               return DIV_ROUND_CLOSEST(15500 - 125 * val, 10);
> > +
> >         case 91:                /* VRM 9.1 */
> >         case 90:                /* VRM 9.0 */
> >                 val &= 0x1f;
> > @@ -195,6 +201,10 @@ static struct vrm_model vrm_models[] = {
> >         {X86_VENDOR_AMD, 0xF, 0x40, 0x7F, ANY, 24},     /* NPT family 0Fh
> > */
> >         {X86_VENDOR_AMD, 0xF, 0x80, ANY, ANY, 25},      /* future fam. 0Fh
> > */
> >         {X86_VENDOR_AMD, 0x10, 0x0, ANY, ANY, 25},      /* NPT family 10h
> > */
> > +       {X86_VENDOR_AMD, 0x11, 0x0, ANY, ANY, 26},      /* family 11h */
> > +       {X86_VENDOR_AMD, 0x12, 0x0, ANY, ANY, 26},      /* family 12h */
> > +       {X86_VENDOR_AMD, 0x14, 0x0, ANY, ANY, 26},      /* family 14h */
> > +       {X86_VENDOR_AMD, 0x15, 0x0, ANY, ANY, 26},      /* family 15h */
> >
> >         {X86_VENDOR_INTEL, 0x6, 0x0, 0x6, ANY, 82},     /* Pentium Pro,
> >                                                          * Pentium II,
> > Xeon,
> >
> > Also available as a standalone driver at:
> >   http://khali.linux-fr.org/devel/misc/hwmon-vid/
> >
> >
> After applying the patch you sent cpu0_vid reads as 1550. Thanks much, that
> was very helpful.

1.550 V is the maximum value supported by the VID encoding on these
processors. This corresponds to value 00h. This means that either the
chip doesn't support VID, or not in this register, or it is supported
but not wired on the board in question.

Either way, thanks for testing, I'll push this upstream for other AMD
family 11h+ processor users.

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (18 preceding siblings ...)
  2012-12-16 10:15 ` Jean Delvare
@ 2012-12-16 16:54 ` Guenter Roeck
  2013-11-12 13:48 ` Rudolf Marek
                   ` (7 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2012-12-16 16:54 UTC (permalink / raw)
  To: lm-sensors

On Sun, Dec 16, 2012 at 11:15:06AM +0100, Jean Delvare wrote:
> Hi David,
> 
> On Thu, 13 Dec 2012 00:58:35 -0700, David Hubbard wrote:
> > On Wed, Dec 12, 2012 at 7:45 AM, Jean Delvare <khali@linux-fr.org> wrote:
> > > We do not support any VID decoding since family 10h CPUs for AMD:
> > >
> > >         {X86_VENDOR_AMD, 0x10, 0x0, ANY, ANY, 25},      /* NPT family 10h
> > > */
> > >
> > > Starting with family 11h CPUs, AMD has given up on parallel VID in
> > > favor of serial VID which uses only 2 pins. I know that the IT8720F has
> > > support for this, but I did not find any trace in other ITE datasheets.
> > >
> > > The serial VID uses 7-bit codes which are incompatible with the
> > > previous 6-bit parallel VID codes used by family 10h CPUs. BTW the
> > > family 10h CPUs already supported serial VID, and I have no idea how to
> > > support that properly as hwmon-vid has no idea whether the code came
> > > from the serial or parallel VID interface.
> > >
> > > The following should at least get rid of the warning in the kernel logs:
> > >
> > > * * * * *
> > >
> > > Since family 11h processors, AMD is exclusively using 7-bit VID codes
> > > transmitted using a serial protocol over two pins (clock and data.)
> > >
> > > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > > ---
> > >  drivers/hwmon/hwmon-vid.c |   10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > --- linux-3.7-rc8.orig/drivers/hwmon/hwmon-vid.c        2012-10-20
> > > 21:11:32.000000000 +0200
> > > +++ linux-3.7-rc8/drivers/hwmon/hwmon-vid.c     2012-12-12
> > > 15:36:36.309882929 +0100
> > > @@ -115,6 +115,12 @@ int vid_from_reg(int val, u8 vrm)
> > >                 return (val < 32) ? 1550 - 25 * val
> > >                         : 775 - (25 * (val - 31)) / 2;
> > >
> > > +       case 26:                /* AMD family 10h to 15h, serial VID */
> > > +               val &= 0x7f;
> > > +               if (val >= 0x7c)
> > > +                       return 0;
> > > +               return DIV_ROUND_CLOSEST(15500 - 125 * val, 10);
> > > +
> > >         case 91:                /* VRM 9.1 */
> > >         case 90:                /* VRM 9.0 */
> > >                 val &= 0x1f;
> > > @@ -195,6 +201,10 @@ static struct vrm_model vrm_models[] = {
> > >         {X86_VENDOR_AMD, 0xF, 0x40, 0x7F, ANY, 24},     /* NPT family 0Fh
> > > */
> > >         {X86_VENDOR_AMD, 0xF, 0x80, ANY, ANY, 25},      /* future fam. 0Fh
> > > */
> > >         {X86_VENDOR_AMD, 0x10, 0x0, ANY, ANY, 25},      /* NPT family 10h
> > > */
> > > +       {X86_VENDOR_AMD, 0x11, 0x0, ANY, ANY, 26},      /* family 11h */
> > > +       {X86_VENDOR_AMD, 0x12, 0x0, ANY, ANY, 26},      /* family 12h */
> > > +       {X86_VENDOR_AMD, 0x14, 0x0, ANY, ANY, 26},      /* family 14h */
> > > +       {X86_VENDOR_AMD, 0x15, 0x0, ANY, ANY, 26},      /* family 15h */
> > >
> > >         {X86_VENDOR_INTEL, 0x6, 0x0, 0x6, ANY, 82},     /* Pentium Pro,
> > >                                                          * Pentium II,
> > > Xeon,
> > >
> > > Also available as a standalone driver at:
> > >   http://khali.linux-fr.org/devel/misc/hwmon-vid/
> > >
> > >
> > After applying the patch you sent cpu0_vid reads as 1550. Thanks much, that
> > was very helpful.
> 
> 1.550 V is the maximum value supported by the VID encoding on these
> processors. This corresponds to value 00h. This means that either the
> chip doesn't support VID, or not in this register, or it is supported
> but not wired on the board in question.
> 
> Either way, thanks for testing, I'll push this upstream for other AMD
> family 11h+ processor users.
> 
Feed free to add

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Guenter

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (19 preceding siblings ...)
  2012-12-16 16:54 ` Guenter Roeck
@ 2013-11-12 13:48 ` Rudolf Marek
  2013-11-13  4:23 ` Guenter Roeck
                   ` (6 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Rudolf Marek @ 2013-11-12 13:48 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 2311 bytes --]

Hi all,

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@assembler.cz>

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


[-- Attachment #2: it8306.patch --]
[-- Type: text/x-diff, Size: 11274 bytes --]

Index: linux-3.12/Documentation/hwmon/it87
===================================================================
--- 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,
 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
+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
  *            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 */
 		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));
 

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (20 preceding siblings ...)
  2013-11-12 13:48 ` Rudolf Marek
@ 2013-11-13  4:23 ` Guenter Roeck
  2013-11-16 11:19 ` Rudolf Marek
                   ` (5 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2013-11-13  4:23 UTC (permalink / raw)
  To: lm-sensors

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@assembler.cz>
> 
> 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@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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (21 preceding siblings ...)
  2013-11-13  4:23 ` Guenter Roeck
@ 2013-11-16 11:19 ` Rudolf Marek
  2013-11-16 15:02 ` Guenter Roeck
                   ` (4 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Rudolf Marek @ 2013-11-16 11:19 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

Hi all,

I'm attaching fixed version for next round.

Signed-off-by: Rudolf Marek <r.marek@assembler.cz>

Just a side note. The newer ITE chips implement over/undervoltage protection 
mechanism. It makes 12V/5V pins fixed. Maybe we could detect that in the driver 
and if protection is enabled we can label/scale respective pins.

Thanks
Rudolf

>> Index: linux-3.12/Documentation/hwmon/it87
>
> It might make sense to update Kconfig as well.

Fixed


> IT8603 -> IT8603E

Fixed

> ... It only supports 16-bit fan mode ... ?

Fixed

>
>> +is not supported (value 0 of pwmX_enable).
>> +

>>    *            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).

Yes I put it first now.

>> +	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.

Sorry coding style habit I fixed that.

>> @@ -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.

Whoops I reverted this chunk, it is old code.

Thanks
Rudolf

[-- Attachment #2: it8306.patch --]
[-- Type: text/x-diff, Size: 10991 bytes --]

Index: linux-3.12/Documentation/hwmon/it87
===================================================================
--- linux-3.12.orig/Documentation/hwmon/it87	2013-11-04 00:41:51.000000000 +0100
+++ linux-3.12/Documentation/hwmon/it87	2013-11-16 10:08:54.636693747 +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 IT8603E, IT8705F, IT8712F, IT8716F,
 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. It only supports 16-bit fan mode, the full speed mode of the
+fan 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-16 10:21:24.480412040 +0100
@@ -10,7 +10,8 @@
  *  This driver supports only the Environment Controller in the IT8705F and
  *  similar parts.  The other devices are supported by different drivers.
  *
- *  Supports: IT8705F  Super I/O chip w/LPC interface
+ *  Supports: IT8603E  Super I/O chip w/LPC interface
+ *            IT8705F  Super I/O chip w/LPC interface
  *            IT8712F  Super I/O chip w/LPC interface
  *            IT8716F  Super I/O chip w/LPC interface
  *            IT8718F  Super I/O chip w/LPC interface
@@ -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 */
 		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,11 +1798,13 @@
 
 	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 by default */
 
 	/* Read GPIO config and VID value from LDN 7 (GPIO) */
 	if (sio_data->type == it87) {
@@ -1844,7 +1881,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;
@@ -2072,6 +2144,9 @@
 	/* 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 +2177,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 +2277,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 +2458,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 +2537,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));
 

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (22 preceding siblings ...)
  2013-11-16 11:19 ` Rudolf Marek
@ 2013-11-16 15:02 ` Guenter Roeck
  2013-11-19  7:50 ` Rudolf Marek
                   ` (3 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2013-11-16 15:02 UTC (permalink / raw)
  To: lm-sensors

On Sat, Nov 16, 2013 at 12:19:02PM +0100, Rudolf Marek wrote:
> Hi all,
> 
Hi Rudolf,

[ ... ]

> Index: linux-3.12/Documentation/hwmon/it87
> =================================> --- linux-3.12.orig/Documentation/hwmon/it87	2013-11-04 00:41:51.000000000 +0100
> +++ linux-3.12/Documentation/hwmon/it87	2013-11-16 10:08:54.636693747 +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 IT8603E, IT8705F, IT8712F, IT8716F,
>  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. It only supports 16-bit fan mode, the full speed mode of the
> +fan 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-16 10:21:24.480412040 +0100
> @@ -10,7 +10,8 @@
>   *  This driver supports only the Environment Controller in the IT8705F and
>   *  similar parts.  The other devices are supported by different drivers.
>   *
> - *  Supports: IT8705F  Super I/O chip w/LPC interface
> + *  Supports: IT8603E  Super I/O chip w/LPC interface
> + *            IT8705F  Super I/O chip w/LPC interface
>   *            IT8712F  Super I/O chip w/LPC interface
>   *            IT8716F  Super I/O chip w/LPC interface
>   *            IT8718F  Super I/O chip w/LPC interface
> @@ -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]ún, [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 */
>  		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))

Still extra ( ).

> +		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,11 +1798,13 @@
>  
>  	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,

Extra ( )

> +		sio_data->revision);
>  
>  	/* in8 (Vbat) is always internal */
>  	sio_data->internal = (1 << 2);
> +	sio_data->skip_in |= (1 << 9); /* No VIN9 by default */
>  
>  	/* Read GPIO config and VID value from LDN 7 (GPIO) */
>  	if (sio_data->type = it87) {
> @@ -1844,7 +1881,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;
> @@ -2072,6 +2144,9 @@
>  	/* 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 +2177,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 +2277,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 +2458,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)) {

Extra ( )

>  		tmp = it87_read_value(data, IT87_REG_FAN_16BIT);
>  		if (~tmp & 0x07 & data->has_fan) {
>  			dev_dbg(&pdev->dev,
> @@ -2462,6 +2537,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@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (23 preceding siblings ...)
  2013-11-16 15:02 ` Guenter Roeck
@ 2013-11-19  7:50 ` Rudolf Marek
  2013-11-19 16:29 ` Guenter Roeck
                   ` (2 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: Rudolf Marek @ 2013-11-19  7:50 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 151 bytes --]

Hi Guenter,

Thanks for the review! Here is a re-spin.

Add support for IT8603E.

Signed-off-by: Rudolf Marek <r.marek@assembler.cz>

Thanks
Rudolf




[-- Attachment #2: it8306.patch --]
[-- Type: text/x-diff, Size: 10983 bytes --]

Index: linux-3.12/Documentation/hwmon/it87
===================================================================
--- linux-3.12.orig/Documentation/hwmon/it87	2013-11-18 23:56:09.701100351 +0100
+++ linux-3.12/Documentation/hwmon/it87	2013-11-18 23:57:00.216465289 +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 IT8603E, IT8705F, IT8712F, IT8716F,
 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. It only supports 16-bit fan mode, the full speed mode of the
+fan 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-18 23:56:09.633101207 +0100
+++ linux-3.12/drivers/hwmon/it87.c	2013-11-18 23:59:25.998632558 +0100
@@ -10,7 +10,8 @@
  *  This driver supports only the Environment Controller in the IT8705F and
  *  similar parts.  The other devices are supported by different drivers.
  *
- *  Supports: IT8705F  Super I/O chip w/LPC interface
+ *  Supports: IT8603E  Super I/O chip w/LPC interface
+ *            IT8705F  Super I/O chip w/LPC interface
  *            IT8712F  Super I/O chip w/LPC interface
  *            IT8716F  Super I/O chip w/LPC interface
  *            IT8718F  Super I/O chip w/LPC interface
@@ -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 */
 		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,11 +1798,13 @@
 
 	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 by default */
 
 	/* Read GPIO config and VID value from LDN 7 (GPIO) */
 	if (sio_data->type == it87) {
@@ -1844,7 +1881,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;
@@ -2072,6 +2144,9 @@
 	/* 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 +2177,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 +2277,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 +2458,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 +2537,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));
 



[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (24 preceding siblings ...)
  2013-11-19  7:50 ` Rudolf Marek
@ 2013-11-19 16:29 ` Guenter Roeck
  2013-11-22 12:42 ` Jean Delvare
  2013-11-22 21:51 ` Rudolf Marek
  27 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2013-11-19 16:29 UTC (permalink / raw)
  To: lm-sensors

On Tue, Nov 19, 2013 at 08:50:03AM +0100, Rudolf Marek wrote:
> Hi Guenter,
> 
> Thanks for the review! Here is a re-spin.
> 
> Add support for IT8603E.
> 
> Signed-off-by: Rudolf Marek <r.marek@assembler.cz>
> 
Looks good to me.

Jean, I assume you'll take this one ? Or I can take it if you Ack it.

Thanks,
Guenter

> Thanks
> Rudolf
> 
> 
> 

> Index: linux-3.12/Documentation/hwmon/it87
> =================================> --- linux-3.12.orig/Documentation/hwmon/it87	2013-11-18 23:56:09.701100351 +0100
> +++ linux-3.12/Documentation/hwmon/it87	2013-11-18 23:57:00.216465289 +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 IT8603E, IT8705F, IT8712F, IT8716F,
>  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. It only supports 16-bit fan mode, the full speed mode of the
> +fan 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-18 23:56:09.633101207 +0100
> +++ linux-3.12/drivers/hwmon/it87.c	2013-11-18 23:59:25.998632558 +0100
> @@ -10,7 +10,8 @@
>   *  This driver supports only the Environment Controller in the IT8705F and
>   *  similar parts.  The other devices are supported by different drivers.
>   *
> - *  Supports: IT8705F  Super I/O chip w/LPC interface
> + *  Supports: IT8603E  Super I/O chip w/LPC interface
> + *            IT8705F  Super I/O chip w/LPC interface
>   *            IT8712F  Super I/O chip w/LPC interface
>   *            IT8716F  Super I/O chip w/LPC interface
>   *            IT8718F  Super I/O chip w/LPC interface
> @@ -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]ún, [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 */
>  		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,11 +1798,13 @@
>  
>  	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 by default */
>  
>  	/* Read GPIO config and VID value from LDN 7 (GPIO) */
>  	if (sio_data->type = it87) {
> @@ -1844,7 +1881,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;
> @@ -2072,6 +2144,9 @@
>  	/* 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 +2177,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 +2277,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 +2458,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 +2537,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@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (25 preceding siblings ...)
  2013-11-19 16:29 ` Guenter Roeck
@ 2013-11-22 12:42 ` Jean Delvare
  2013-11-22 21:51 ` Rudolf Marek
  27 siblings, 0 replies; 29+ messages in thread
From: Jean Delvare @ 2013-11-22 12:42 UTC (permalink / raw)
  To: lm-sensors

Hi Rudolf, hi Guenter,

Rudolf, thanks for the code. Guenter, thanks for the preliminary review.

On Tue, 19 Nov 2013 08:50:03 +0100, Rudolf Marek wrote:
> Add support for IT8603E.
> 
> Signed-off-by: Rudolf Marek <r.marek@assembler.cz>

My own comments:

> Index: linux-3.12/Documentation/hwmon/it87
> =================================> --- linux-3.12.orig/Documentation/hwmon/it87	2013-11-18 23:56:09.701100351 +0100
> +++ linux-3.12/Documentation/hwmon/it87	2013-11-18 23:57:00.216465289 +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 IT8603E, IT8705F, IT8712F, IT8716F,
>  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. It only supports 16-bit fan mode, the full speed mode of the
> +fan 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

"IT8721F/IT8758E" was because both chips are undistinguishable. For all
other chips we use a comma (or "and") as a list separator.

>  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

Same here.

>  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-18 23:56:09.633101207 +0100
> +++ linux-3.12/drivers/hwmon/it87.c	2013-11-18 23:59:25.998632558 +0100
> @@ -10,7 +10,8 @@
>   *  This driver supports only the Environment Controller in the IT8705F and
>   *  similar parts.  The other devices are supported by different drivers.
>   *
> - *  Supports: IT8705F  Super I/O chip w/LPC interface
> + *  Supports: IT8603E  Super I/O chip w/LPC interface
> + *            IT8705F  Super I/O chip w/LPC interface
>   *            IT8712F  Super I/O chip w/LPC interface
>   *            IT8716F  Super I/O chip w/LPC interface
>   *            IT8718F  Super I/O chip w/LPC interface
> @@ -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 */

I see no point in this change as you use bit 3 for in9.

>  	/* 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]ún, [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);
> +

Unneeded blank line addition.

>  
>  /* 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 */

Unneeded space between tabs and comment.

>  		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",
> +	};

I see no good reason to add a separate list for the IT8603E, the list
is the same as labels_it8721 above except for "+3.3V" which has been
renamed to "AVCC3" but really that's exactly the same. Let's reuse
labels_it8721 to optimize the driver size.

>  	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 */

Missing trailing "E".

> +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,11 +1798,13 @@
>  
>  	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);

The IT877x chips are also 'E' not 'F', but this can be fixed in a
subsequent patch at it isn't related to IT8603E support.

>  
>  	/* in8 (Vbat) is always internal */
>  	sio_data->internal = (1 << 2);
> +	sio_data->skip_in |= (1 << 9); /* No VIN9 by default */

Please stick to the comment position already in place. Furthermore you
could do it only if sio_data->type != it8603, that way you don't have
to undo it later.

>  
>  	/* Read GPIO config and VID value from LDN 7 (GPIO) */
>  	if (sio_data->type = it87) {
> @@ -1844,7 +1881,42 @@
>  			sio_data->internal |= (1 << 1);
>  
>  		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> +	} if (sio_data->type = it8603) {

This coding style is misleading. Either you have an "else", or you move
the "if" to the following line. I think you're missing the "else"
actually, and without it you introduce a big regression in the it87
driver.

Note that checkpatch DOES warn about this.

> +		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 */

VBAT is already handled in the common code above.

> +
> +		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
>  	} else {
>  		int reg;
>  		bool uart6;
> @@ -2072,6 +2144,9 @@
>  	/* Check PWM configuration */
>  	enable_pwm_interface = it87_check_pwm(dev);
>  
> +	if (data->type = it8603)
> +		data->in_scaled |= (1 << 9); /* in9 is AVCC3 */

I see no good reason for special-casing this, when it can be handled
genericly in the block below. Plus there was no point in setting
sio_data->internal |= (1 << 3) before if you were not going to check it
here.

> +
>  	/* Starting with IT8721F, we handle scaling of internal voltages */
>  	if (has_12mv_adc(data)) {
>  		if (sio_data->internal & (1 << 0))
> @@ -2102,7 +2177,7 @@
>  	if (err)
>  		return err;
>  
> -	for (i = 0; i < 9; i++) {
> +	for (i = 0; i < 10; i++) {

Same change is needed at removal time.

>  		if (sio_data->skip_in & (1 << i))
>  			continue;
>  		err = sysfs_create_group(&dev->kobj, &it87_group_in[i]);
> @@ -2202,7 +2277,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 +2458,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 */

Missing "F" in comment, and line too long.

> +	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 +2537,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));
>  
> 
> 

I've make all the suggested changes myself to avoid another round trip.
Also I've updated Kconfig as suggested by Guenter. The resulting patch
can be seen at:
http://khali.linux-fr.org/devel/linux-3/jdelvare-hwmon/hwmon-it87-01-add-it8603e-support.patch
Please let me know if I got anything wrong. Retesting would be
appreciated, just in case.

I have also made the support available as a standalone driver which
builds at least down to kernel 3.0:
http://khali.linux-fr.org/devel/lm-sensors/drivers/it87/

This might make testing easier.

Thanks,
-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [lm-sensors] ITE it8603e
  2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
                   ` (26 preceding siblings ...)
  2013-11-22 12:42 ` Jean Delvare
@ 2013-11-22 21:51 ` Rudolf Marek
  27 siblings, 0 replies; 29+ messages in thread
From: Rudolf Marek @ 2013-11-22 21:51 UTC (permalink / raw)
  To: lm-sensors

Hi Khali,
 > I see no good reason to add a separate list for the IT8603E, the list
> is the same as labels_it8721 above except for "+3.3V" which has been
> renamed to "AVCC3" but really that's exactly the same. Let's reuse
> labels_it8721 to optimize the driver size.

OK

>>   		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
>> +	} if (sio_data->type = it8603) {
>

> This coding style is misleading. Either you have an "else", or you move
> the "if" to the following line. I think you're missing the "else"
> actually, and without it you introduce a big regression in the it87
> driver.

Yes sorry about that. It was somehow overlooked during developement.

> I've make all the suggested changes myself to avoid another round trip.

Great thanks a lot!

> Also I've updated Kconfig as suggested by Guenter. The resulting patch

Yes I did that but forgot to include it in the patch.

> can be seen at:
> http://khali.linux-fr.org/devel/linux-3/jdelvare-hwmon/hwmon-it87-01-add-it8603e-support.patch
> Please let me know if I got anything wrong. Retesting would be
> appreciated, just in case.

I tested that and it is OK. Thank you again for review and help,

Rudolf


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2013-11-22 21:51 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-28 20:41 [lm-sensors] ITE it8603e David Hubbard
2012-11-29  5:05 ` Guenter Roeck
2012-11-29 10:09 ` David Hubbard
2012-11-29 11:48 ` Jean Delvare
2012-11-29 18:00 ` David Hubbard
2012-12-02 17:57 ` Jean Delvare
2012-12-02 18:26 ` Guenter Roeck
2012-12-02 18:52 ` Jean Delvare
2012-12-02 19:20 ` Guenter Roeck
2012-12-03  8:44 ` Jean Delvare
2012-12-03  8:56 ` Rudolf Marek
2012-12-03 14:35 ` Guenter Roeck
2012-12-12 14:45 ` Jean Delvare
2012-12-13  7:58 ` David Hubbard
2012-12-13  8:00 ` David Hubbard
2012-12-13 16:07 ` Guenter Roeck
2012-12-13 21:27 ` David Hubbard
2012-12-13 22:13 ` Guenter Roeck
2012-12-13 22:40 ` David Hubbard
2012-12-16 10:15 ` Jean Delvare
2012-12-16 16:54 ` Guenter Roeck
2013-11-12 13:48 ` Rudolf Marek
2013-11-13  4:23 ` Guenter Roeck
2013-11-16 11:19 ` Rudolf Marek
2013-11-16 15:02 ` Guenter Roeck
2013-11-19  7:50 ` Rudolf Marek
2013-11-19 16:29 ` Guenter Roeck
2013-11-22 12:42 ` Jean Delvare
2013-11-22 21:51 ` Rudolf Marek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.