All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore
@ 2010-11-29  6:20 Jeff Rickman
  2010-11-29  8:05 ` [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer Juerg Haefliger
                   ` (30 more replies)
  0 siblings, 31 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-11-29  6:20 UTC (permalink / raw)
  To: lm-sensors

Providing some feedback now that SCH5127 support is "mainline" in
LM_Sensors; no "force_id" option required.

I have an Acer easyStore H340 that has a SCH5127 monitoring chip inside.

[root@anas-01 ~]# uname -a
Linux anas-01.my.home 2.6.35.6-48.fc14.x86_64 #1 SMP Fri Oct 22 15:36:08
UTC 2010 x86_64 x86_64 x86_64 GNU/Linux

[root@anas-01 ~]# sensors -v
sensors version 3.2.0 with libsensors version 3.2.0

LM_Sensors identifies the SCH5127 chip as follows:

[root@anas-01 ~]# sensors-detect
Stopping lm_sensors:                                       [  OK  ]
# sensors-detect revision 5861 (2010-09-21 17:21:05 +0200)
# System: Acer Aspire easyStore H340
# Board: Acer WG945GCM

This program will help you determine which kernel modules you need
to load to use lm_sensors most effectively. It is generally safe
and recommended to accept the default answers to all questions,
unless you know what you're doing.

Some south bridges, CPUs or memory controllers contain embedded sensors.
Do you want to scan for them? This is totally safe. (YES/no):
Silicon Integrated Systems SIS5595...                       No
VIA VT82C686 Integrated Sensors...                          No
VIA VT8231 Integrated Sensors...                            No
AMD K8 thermal sensors...                                   No
AMD Family 10h thermal sensors...                           No
AMD Family 11h thermal sensors...                           No
Intel Core family thermal sensor...                         No
Intel Atom thermal sensor...                                Success!
    (driver `coretemp')
Intel AMB FB-DIMM thermal sensor...                         No
VIA C7 thermal sensor...                                    No
VIA Nano thermal sensor...                                  No

Some Super I/O chips contain embedded sensors. We have to write to
standard I/O ports to probe them. This is usually safe.
Do you want to scan for Super I/O sensors? (YES/no):
Probing for Super-I/O at 0x2e/0x2f
Trying family `National Semiconductor'...                   No
Trying family `SMSC'...                                     Yes
Found `SMSC SCH5127 Super IO'                               Success!
    (address 0x800, driver `dme1737')
Probing for Super-I/O at 0x4e/0x4f
Trying family `National Semiconductor'...                   No
Trying family `SMSC'...                                     No
Trying family `VIA/Winbond/Nuvoton/Fintek'...               No
Trying family `ITE'...                                      No

Some systems (mainly servers) implement IPMI, a set of common interfaces
through which system health data may be retrieved, amongst other things.
We first try to get the information from SMBIOS. If we don't find it
there, we have to read from arbitrary I/O ports to probe for such
interfaces. This is normally safe. Do you want to scan for IPMI
interfaces? (YES/no):
Probing for `IPMI BMC KCS' at 0xca0...                      No
Probing for `IPMI BMC SMIC' at 0xca8...                     No

Some hardware monitoring chips are accessible through the ISA I/O ports.
We have to write to arbitrary I/O ports to probe them. This is usually
safe though. Yes, you do have ISA I/O ports even if you do not have any
ISA slots! Do you want to scan the ISA I/O ports? (yes/NO):

Lastly, we can probe the I2C/SMBus adapters for connected hardware
monitoring devices. This is the most risky part, and while it works
reasonably well on most systems, it has been reported to cause trouble
on some systems.
Do you want to probe the I2C/SMBus adapters now? (YES/no):
Using driver `i2c-i801' for device 0000:00:1f.3: Intel 82801G ICH7
Module i2c-dev loaded successfully.

Next adapter: SMBus I801 adapter at 18a0 (i2c-0)
Do you want to scan it? (yes/NO/selectively):

Now follows a summary of the probes I have just done.
Just press ENTER to continue:

Driver `coretemp':
  * Chip `Intel Atom thermal sensor' (confidence: 9)

Driver `dme1737':
  * ISA bus, address 0x800
    Chip `SMSC SCH5127 Super IO' (confidence: 9)

Do you want to overwrite /etc/sysconfig/lm_sensors? (YES/no): yes

Now follows a summary of the probes I have just done.
Just press ENTER to continue:

Driver `coretemp':
  * Chip `Intel Atom thermal sensor' (confidence: 9)

Driver `dme1737':
  * ISA bus, address 0x800
    Chip `SMSC SCH5127 Super IO' (confidence: 9)

Do you want to overwrite /etc/sysconfig/lm_sensors? (YES/no):
Starting lm_sensors: loading module coretemp dme1737       [  OK  ]
Unloading i2c-dev... OK

[root@anas-01 ~]# service lm_sensors status
coretemp-isa-0000
Adapter: ISA adapter
Core 0:      +21.0°C  (crit = +90.0°C)

sch5127-isa-0870
Adapter: ISA adapter
V+1.5:       +1.42 V  (min =  +1.35 V, max =  +1.65 V)
5VTR:        +4.78 V  (min =  +4.50 V, max =  +5.48 V)
VBAT:        +3.32 V  (min =  +0.00 V, max =  +4.38 V)
V+5:         +5.09 V  (min =  +0.00 V, max =  +6.72 V)
Vccp:        +1.48 V  (min =  +1.35 V, max =  +1.49 V)
VCC:         +3.33 V  (min =  +0.00 V, max =  +4.38 V)
VTR:         +3.24 V  (min =  +0.00 V, max =  +4.38 V)
Side Fan:   2250 RPM  (min = 600 RPM)
MCH Fan:    5137 RPM  (min = 4500 RPM)
CPU Temp:    +47.6°C  (low  = +20.0°C, high = +60.0°C)
SYS Temp:    +35.1°C  (low  = +20.0°C, high = +60.0°C)

Here is my "/etc/sensors.d/local.conf" file:

[root@anas-01 ~]# cat /etc/sensors.d/local.conf
# libsensors configuration file
# -----------------------------

chip "sch5127-isa-0870"

    label in0 "V+1.5"
    label in1 "5VTR"
    label in2 "VBAT"
    label in3 "V+5"
    label in4 "Vccp"
    label in5 "VCC"
    label in6 "VTR"

# All inputs are listed here in order as displayed in BIOS.
# Values seen by "sensors" closely match values seen in BIOS.

    label fan1 "Side Fan"
    label fan2 "MCH Fan"
    ignore fan3

    label temp1 "CPU Temp"
    ignore temp2
#    label temp2 "SIO Temp"
    label temp3 "SYS Temp"

    compute in0 (@ * 0.8), (@ / 0.8)
    compute in1 (@ * 4), (@ / 4)
    compute in3 (@ * 4.5), (@ / 4.5)

    set in0_min 1.5 * 0.90
    set in0_max 1.5 * 1.10
    set in1_min 5.0 * 0.90
    set in1_max 5.0 * 1.10
    set in4_min 1.50 * 0.90
    set in4_max 1.50 * 1.10

    set fan1_min 600 <- per fan vendor
    set fan2_min 4500
    set temp1_min 20
    set temp1_max 60
    set temp2_min 20
    set temp3_min 20
    set temp3_max 60

Fan1 is a CoolerMaster Excalibur R4-EXBB-20PK-R0 120x120x25 mm fan. It has
a speed range of 600 ~ 2000 RPM +- 10 percent per vendor. It does get loud
at full speed but it also moves air: 26.4 ~ 85.6 CFM +- 10 percent per
vendor.

The system has 2 fan headers, both are 4-pin. I have only tested "fan1"
(Sensors label: Side Fan) ("pwmconfig" detected: hwmon1/device/pwm1) using
"pwmconfig":

[root@anas-01 ~]# pwmconfig
# pwmconfig revision 5857 (2010-08-22)
This program will search your sensors for pulse width modulation (pwm)
controls, and test each one to see if it controls a fan on
your motherboard. Note that many motherboards do not have pwm
circuitry installed, even if your sensor chip supports pwm.

We will attempt to briefly stop each fan using the pwm controls.
The program will attempt to restore each fan to full speed
after testing. However, it is ** very important ** that you
physically verify that the fans have been to full speed
after the program has completed.

Found the following devices:
   hwmon0/device is coretemp
   hwmon1/device is sch5127

Found the following PWM controls:
   hwmon1/device/pwm1 <- local label "Side Fan"
   hwmon1/device/pwm2 <- local label "MCH Fan"
   hwmon1/device/pwm3 <- no header on motherboard

Giving the fans some time to reach full speed...
Found the following fan sensors:
   hwmon1/device/fan1_input     current speed: 2253 RPM
   hwmon1/device/fan2_input     current speed: 5147 RPM
   hwmon1/device/fan3_input     current speed: 0 ... skipping!

Warning!!! This program will stop your fans, one at a time,
for approximately 5 seconds each!!!
This may cause your processor temperature to rise!!!
If you do not want to do this hit control-C now!!!
Hit return to continue:

Testing pwm control hwmon1/device/pwm1 ...
  hwmon1/device/fan1_input ... speed was 2253 now 699
    It appears that fan hwmon1/device/fan1_input
    is controlled by pwm hwmon1/device/pwm1
Would you like to generate a detailed correlation (y)? y
    PWM 255 FAN 2254
    PWM 240 FAN 2250
    PWM 225 FAN 2250
    PWM 210 FAN 2251
    PWM 195 FAN 2156
    PWM 180 FAN 2032
    PWM 165 FAN 1910
    PWM 150 FAN 1774
    PWM 135 FAN 1626
    PWM 120 FAN 1461
    PWM 105 FAN 1286
    PWM 90 FAN 1091
    PWM 75 FAN 907
    PWM 60 FAN 796
    PWM 45 FAN 730
    PWM 30 FAN 701
    PWM 28 FAN 698
    PWM 26 FAN 698
    PWM 24 FAN 700
    PWM 22 FAN 698
    PWM 20 FAN 700
    PWM 18 FAN 700
    PWM 16 FAN 700
    PWM 14 FAN 701
    PWM 12 FAN 701
    PWM 10 FAN 701
    PWM 8 FAN 700
    PWM 6 FAN 701
    PWM 4 FAN 700
    PWM 2 FAN 701
    PWM 0 FAN 701

  hwmon1/device/fan2_input ... speed was 5147 now 5172
    no correlation

Testing pwm control hwmon1/device/pwm2 ...
  hwmon1/device/fan1_input ... speed was 2253 now 2259
    no correlation
  hwmon1/device/fan2_input ... speed was 5147 now 5147
    no correlation

No correlations were detected.
There is either no fan connected to the output of hwmon1/device/pwm2,
or the connected fan has no rpm-signal connected to one of
the tested fan sensors. (Note: not all motherboards have
the pwm outputs connected to the fan connectors,
check out the hardware database on http://www.almico.com/forumindex.php)

Did you see/hear a fan stopping during the above test (n)? y

Testing pwm control hwmon1/device/pwm3 ...
  hwmon1/device/fan1_input ... speed was 2253 now 2253
    no correlation
  hwmon1/device/fan2_input ... speed was 5147 now 5152
    no correlation

No correlations were detected.
There is either no fan connected to the output of hwmon1/device/pwm3,
or the connected fan has no rpm-signal connected to one of
the tested fan sensors. (Note: not all motherboards have
the pwm outputs connected to the fan connectors,
check out the hardware database on http://www.almico.com/forumindex.php)

Did you see/hear a fan stopping during the above test (n)? n

Testing is complete.
Please verify that all fans have returned to their normal speed.

The fancontrol script can automatically respond to temperature changes
of your system by changing fanspeeds.
Do you want to set up its configuration file now (y)?
What should be the path to your fancontrol config file (/etc/fancontrol)?
Loading configuration from /etc/fancontrol ...

Select fan output to configure, or other action:
1) hwmon1/device/pwm2  3) Change INTERVAL     5) Save and quit
2) hwmon1/device/pwm1  4) Just quit           6) Show configuration
select (1-n): 2

Devices:
hwmon0/device is coretemp
hwmon1/device is sch5127

Current temperature readings are as follows:
hwmon0/device/temp1_input       22
hwmon1/device/temp1_input       48
hwmon1/device/temp2_input       125
hwmon1/device/temp3_input       36

Select a temperature sensor as source for hwmon1/device/pwm1:
1) hwmon0/device/temp1_input
2) hwmon1/device/temp1_input
3) hwmon1/device/temp2_input
4) hwmon1/device/temp3_input
5) None (Do not affect this PWM output)
select (1-n): 2

Enter the low temperature (degree C)
below which the fan should spin at minimum speed (20): 30

Enter the high temperature (degree C)
over which the fan should spin at maximum speed (60):

Enter the PWM value (0-255) to use when the temperature
is over the high temperature limit (255):


Select fan output to configure, or other action:
1) hwmon1/device/pwm2  3) Change INTERVAL     5) Save and quit
2) hwmon1/device/pwm1  4) Just quit           6) Show configuration
select (1-n): 6

Common Settings:
INTERVAL\x10

Settings of hwmon1/device/pwm2:
  Depends on
  Controls
  MINTEMP  MAXTEMP  MINSTART  MINSTOP
Settings of hwmon1/device/pwm1:
  Depends on hwmon1/device/temp1_input
  Controls hwmon1/device/fan1_input
  MINTEMP0
  MAXTEMP`
  MINSTART\x150
  MINSTOP=0


Select fan output to configure, or other action:
1) hwmon1/device/pwm2  3) Change INTERVAL     5) Save and quit
2) hwmon1/device/pwm1  4) Just quit           6) Show configuration
select (1-n): 5

Saving configuration to /etc/fancontrol...
Configuration saved


Experiences so far:

(1) "fancontrol" script does work, but I would like it to run as a
"background" process or even as a daemon. I'm not a programmer so I don't
know if this is even possible or how to do it.

(2) Fan speed on "Fan1" does go up and down with system temperatures.




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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
@ 2010-11-29  8:05 ` Juerg Haefliger
  2010-11-29  9:41 ` Jeff Rickman
                   ` (29 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Juerg Haefliger @ 2010-11-29  8:05 UTC (permalink / raw)
  To: lm-sensors

Hi Jeff,

Thanks for the detailed feedback.


> Experiences so far:
>
> (1) "fancontrol" script does work, but I would like it to run as a
> "background" process or even as a daemon. I'm not a programmer so I don't
> know if this is even possible or how to do it.
>
> (2) Fan speed on "Fan1" does go up and down with system temperatures.

I don't understand. Does fan1 change as a result of your running the
fancontrol script? The SCH5127 has a built in fan controller so you
shouldn't need to run any external scripts. The chip should
automatically take care of adjusting the fan speed based on the temp.
Can you please send the output of the following command:

grep "" /sys/class/hwmon/hwmon*/*

Thanks
...Juerg

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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
  2010-11-29  8:05 ` [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer Juerg Haefliger
@ 2010-11-29  9:41 ` Jeff Rickman
  2010-11-29  9:48 ` Jean Delvare
                   ` (28 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-11-29  9:41 UTC (permalink / raw)
  To: lm-sensors

Hi Juerg,

> Hi Jeff,
>
> Thanks for the detailed feedback.
>
>
>> Experiences so far:
>>
>> (1) "fancontrol" script does work, but I would like it to run as a
>> "background" process or even as a daemon. I'm not a programmer so I
>> don't
>> know if this is even possible or how to do it.
>>
>> (2) Fan speed on "Fan1" does go up and down with system temperatures.
>
> I don't understand. Does fan1 change as a result of your running the
> fancontrol script? The SCH5127 has a built in fan controller so you
> shouldn't need to run any external scripts. The chip should
> automatically take care of adjusting the fan speed based on the temp.
> Can you please send the output of the following command:
>
> grep "" /sys/class/hwmon/hwmon*/*
>
> Thanks
> ...Juerg
>

No output from your command in my SSH session, but there is symbolically
linked 'content' in that file structure.

[root@anas-01 ~]# tree -af /sys/class/hwmon
/sys/class/hwmon
 /sys/class/hwmon/hwmon0 -> ../../devices/platform/coretemp.0/hwmon/hwmon0
 /sys/class/hwmon/hwmon1 -> ../../devices/platform/dme1737.2160/hwmon/hwmon1

These directories have all sorts of files in both of them.

[root@anas-01 ~]# ls /sys/devices/platform/coretemp.0
driver  hwmon  modalias  name  power  subsystem  temp1_crit 
temp1_crit_alarm  temp1_input  temp1_label  uevent
[root@anas-01 ~]# ls /sys/devices/platform/dme1737.2160
driver      fan3_alarm  in1_alarm  in3_input  in5_max    pwm1             
       pwm2_auto_point1_pwm     pwm3_enable     temp2_alarm  temp3_min    
            zone2_auto_point3_temp
fan1_alarm  fan3_input  in1_input  in3_max    in5_min   
pwm1_auto_channels_zone  pwm2_auto_point2_pwm     pwm3_freq      
temp2_fault  uevent
fan1_input  fan3_min    in1_max    in3_min    in6_alarm 
pwm1_auto_point1_pwm     pwm2_enable              pwm3_ramp_rate 
temp2_input  zone1_auto_channels_temp
fan1_min    fan3_type   in1_min    in4_alarm  in6_input 
pwm1_auto_point2_pwm     pwm2_freq                subsystem      
temp2_max    zone1_auto_point1_temp
fan1_type   hwmon       in2_alarm  in4_input  in6_max    pwm1_enable      
       pwm2_ramp_rate           temp1_alarm     temp2_min   
zone1_auto_point2_temp
fan2_alarm  in0_alarm   in2_input  in4_max    in6_min    pwm1_freq        
       pwm3                     temp1_fault     temp3_alarm 
zone1_auto_point3_temp
fan2_input  in0_input   in2_max    in4_min    modalias   pwm1_ramp_rate   
       pwm3_auto_channels_zone  temp1_input     temp3_fault 
zone2_auto_channels_temp
fan2_min    in0_max     in2_min    in5_alarm  name       pwm2             
       pwm3_auto_point1_pwm     temp1_max       temp3_input 
zone2_auto_point1_temp
fan2_type   in0_min     in3_alarm  in5_input  power     
pwm2_auto_channels_zone  pwm3_auto_point2_pwm     temp1_min      
temp3_max    zone2_auto_point2_temp

Hopefully the formatting doesn't look ugly. Only PWM1 (Fan1...4-pin) and
PWM2 (Fan2...4-pin) connect to motherboard hardware. PWM3 does not have a
motherboard connector.

I did not know that SCH5127 had an internal fan controller, but I
suspected something going on when I changed the system fan from a 3-pin
format to a 4-pin format by reconnecting the 'blue' PWM wire on the
header. With the PWM wire added back I saw my Fan1 speed drop to ~750 rpm;
full speed is ~1900 rpm. I run a tool called "systemgraph" (google
it...nice application and "open source" too) to display a webpage of
various values it collects via Perl scripts and deposits into RRD tables
for a nice "history" view.

In the 4-pin format and no other adjustments, sensors reported the Fan1
speed around 750 rpm and HDD temperatures are ~40 C. With the 3-pin
format, Fan1 speed is ~1900 rpm and HDD temps are ~34 C. So in 4-wire Fan
mode I tinkered with "pwmconfig" and "fancontrol" for a while to
understand how the "/etc/fancontrol" config file works. I finally settled
on this:

[root@anas-01 ~]# cat /etc/fancontrol
# Configuration file generated by pwmconfig, changes will be lost
INTERVAL\x10
DEVPATH=hwmon1Þvices/platform/dme1737.2160
DEVNAME=hwmon1=sch5127
FCTEMPS= hwmon1/device/pwm1=hwmon1/device/temp1_input
FCFANS= hwmon1/device/pwm1=hwmon1/device/fan1_input
MINTEMP= hwmon1/device/pwm10
MAXTEMP= hwmon1/device/pwm12
MINSTART= hwmon1/device/pwm1%5
MINSTOP= hwmon1/device/pwm1%4
MINPWM= hwmon1/device/pwm1%4

Then I edited "/etc/rc.local" to add this line:

/usr/sbin/fancontrol &

Fancontrol now runs in the background all the time unless I "kill it". I
don't have a 4-pin fan with a "loose" PWM wire inside the chassis. I can
tape and secure the PWM wire, but then I have to disconnect stuff and open
the case when I wish to experiment. I can restore full 4-pin operation by
removing the line added to "/etc/rc.local" and then echoing the desired
value to "pwm1_enable" or rebooting the system. Some might call my method
the "lazy sysadmin" approach to system thermal control. Works for me!

This setup does a nice job of keeping the 4-pin fan running at it's
maximum speed of ~1900 rpm and HDD temperatures stay below 40 Celsius even
under load.




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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
  2010-11-29  8:05 ` [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer Juerg Haefliger
  2010-11-29  9:41 ` Jeff Rickman
@ 2010-11-29  9:48 ` Jean Delvare
  2010-11-29 10:23 ` Jean Delvare
                   ` (27 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-11-29  9:48 UTC (permalink / raw)
  To: lm-sensors

On Mon, 29 Nov 2010 03:41:35 -0600 (CST), Jeff Rickman wrote:
> Hi Juerg,
> > I don't understand. Does fan1 change as a result of your running the
> > fancontrol script? The SCH5127 has a built in fan controller so you
> > shouldn't need to run any external scripts. The chip should
> > automatically take care of adjusting the fan speed based on the temp.
> > Can you please send the output of the following command:
> >
> > grep "" /sys/class/hwmon/hwmon*/*
> 
> No output from your command in my SSH session, but there is symbolically
> linked 'content' in that file structure.

Juerg certainly meant instead:

grep "" /sys/class/hwmon/hwmon*/device/*

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (2 preceding siblings ...)
  2010-11-29  9:48 ` Jean Delvare
@ 2010-11-29 10:23 ` Jean Delvare
  2010-11-29 15:51 ` Jeff Rickman
                   ` (26 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-11-29 10:23 UTC (permalink / raw)
  To: lm-sensors

Hi Jeff,

On Mon, 29 Nov 2010 00:20:44 -0600 (CST), Jeff Rickman wrote:
> Providing some feedback now that SCH5127 support is "mainline" in
> LM_Sensors; no "force_id" option required.
> (...)
> [root@anas-01 ~]# service lm_sensors status
> coretemp-isa-0000
> Adapter: ISA adapter
> Core 0:      +21.0°C  (crit = +90.0°C)
> 
> sch5127-isa-0870
> Adapter: ISA adapter
> V+1.5:       +1.42 V  (min =  +1.35 V, max =  +1.65 V)
> 5VTR:        +4.78 V  (min =  +4.50 V, max =  +5.48 V)
> VBAT:        +3.32 V  (min =  +0.00 V, max =  +4.38 V)
> V+5:         +5.09 V  (min =  +0.00 V, max =  +6.72 V)
> Vccp:        +1.48 V  (min =  +1.35 V, max =  +1.49 V)
> VCC:         +3.33 V  (min =  +0.00 V, max =  +4.38 V)
> VTR:         +3.24 V  (min =  +0.00 V, max =  +4.38 V)
> Side Fan:   2250 RPM  (min = 600 RPM)
> MCH Fan:    5137 RPM  (min = 4500 RPM)
> CPU Temp:    +47.6°C  (low  = +20.0°C, high = +60.0°C)
> SYS Temp:    +35.1°C  (low  = +20.0°C, high = +60.0°C)
> 
> Here is my "/etc/sensors.d/local.conf" file:
> 
> [root@anas-01 ~]# cat /etc/sensors.d/local.conf
> # libsensors configuration file
> # -----------------------------
> 
> chip "sch5127-isa-0870"
> 
>     label in0 "V+1.5"
>     label in1 "5VTR"
>     label in2 "VBAT"
>     label in3 "V+5"
>     label in4 "Vccp"
>     label in5 "VCC"
>     label in6 "VTR"
> 
> # All inputs are listed here in order as displayed in BIOS.

This is often the case, but not always. So this should only be used as
a last resort decision factor.

> # Values seen by "sensors" closely match values seen in BIOS.
> 
>     label fan1 "Side Fan"
>     label fan2 "MCH Fan"
>     ignore fan3
> 
>     label temp1 "CPU Temp"
>     ignore temp2
> #    label temp2 "SIO Temp"

This is an internal sensor, it should always be present and correct, so
why would you ignore it?

>     label temp3 "SYS Temp"
> 
>     compute in0 (@ * 0.8), (@ / 0.8)

This is extremely unlikely. While it is frequent to scale down voltage
inputs so that the results fits in the ADC range, there is no point in
scaling a voltage up. Especially not by a factor so close to 1: the
error incurred by the operation would far outweigh the resolution
improvement at the ADC level. Not to mention that scaling up needs an
amplifier so it's not a cheap operation - no PC vendor would do this.

So, I seriously doubt that in0 is +1.5V.

In fact, the SCH5127 already has internal scaling resistors on most
voltage inputs, so if the voltage lines are wired correctly, only
voltages over +3.3V should need external scaling. These voltages (+5V
and 5VTR in your case) should be wired to in3 and in4 per chip design,
to limit the number of inputs which require scaling. VBAT, VCC and VTR
should be on in2, in5 and in6, which matches your findings. Which means
that +1.5V and Vccp should be on in0 and in1 (or vice-versa.)

Now it is of course possible that Acer did things differently, either
for pin proximity reasons, or just because they are bad ;)

Assuming that your CPU does frequency and voltage scaling based on
load, you should try to put some load on the CPU and check which
voltage input raises. This would be Vcore (Vccp) and should require no
scaling. If you can figure that one out, it might help sort out the
rest.

>     compute in1 (@ * 4), (@ / 4)
>     compute in3 (@ * 4.5), (@ / 4.5)
> 
>     set in0_min 1.5 * 0.90
>     set in0_max 1.5 * 1.10
>     set in1_min 5.0 * 0.90
>     set in1_max 5.0 * 1.10
>     set in4_min 1.50 * 0.90
>     set in4_max 1.50 * 1.10
> 
>     set fan1_min 600 <- per fan vendor

You're missing a # before your comment.

>     set fan2_min 4500
>     set temp1_min 20
>     set temp1_max 60
>     set temp2_min 20
>     set temp3_min 20
>     set temp3_max 60

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (3 preceding siblings ...)
  2010-11-29 10:23 ` Jean Delvare
@ 2010-11-29 15:51 ` Jeff Rickman
  2010-11-29 16:00 ` Jean Delvare
                   ` (25 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-11-29 15:51 UTC (permalink / raw)
  To: lm-sensors

Hi Jean,

> Hi Jeff,
>
> On Mon, 29 Nov 2010 00:20:44 -0600 (CST), Jeff Rickman wrote:
>> Providing some feedback now that SCH5127 support is "mainline" in
>> LM_Sensors; no "force_id" option required.
>> (...)
>> [root@anas-01 ~]# service lm_sensors status
>> coretemp-isa-0000
>> Adapter: ISA adapter
>> Core 0:      +21.0°C  (crit = +90.0°C)
>>
>> sch5127-isa-0870
>> Adapter: ISA adapter
>> V+1.5:       +1.42 V  (min =  +1.35 V, max =  +1.65 V)
>> 5VTR:        +4.78 V  (min =  +4.50 V, max =  +5.48 V)
>> VBAT:        +3.32 V  (min =  +0.00 V, max =  +4.38 V)
>> V+5:         +5.09 V  (min =  +0.00 V, max =  +6.72 V)
>> Vccp:        +1.48 V  (min =  +1.35 V, max =  +1.49 V)
>> VCC:         +3.33 V  (min =  +0.00 V, max =  +4.38 V)
>> VTR:         +3.24 V  (min =  +0.00 V, max =  +4.38 V)
>> Side Fan:   2250 RPM  (min = 600 RPM)
>> MCH Fan:    5137 RPM  (min = 4500 RPM)
>> CPU Temp:    +47.6°C  (low  = +20.0°C, high = +60.0°C)
>> SYS Temp:    +35.1°C  (low  = +20.0°C, high = +60.0°C)
>>
>> Here is my "/etc/sensors.d/local.conf" file:
>>
>> [root@anas-01 ~]# cat /etc/sensors.d/local.conf
>> # libsensors configuration file
>> # -----------------------------
>>
>> chip "sch5127-isa-0870"
>>
>>     label in0 "V+1.5"
>>     label in1 "5VTR"
>>     label in2 "VBAT"
>>     label in3 "V+5"
>>     label in4 "Vccp"
>>     label in5 "VCC"
>>     label in6 "VTR"
>>
>> # All inputs are listed here in order as displayed in BIOS.
>
> This is often the case, but not always. So this should only be used as
> a last resort decision factor.

I did attempt to match by voltage values between "sensors" and BIOS.

>> # Values seen by "sensors" closely match values seen in BIOS.
>>
>>     label fan1 "Side Fan"
>>     label fan2 "MCH Fan"
>>     ignore fan3
>>
>>     label temp1 "CPU Temp"
>>     ignore temp2
>> #    label temp2 "SIO Temp"
>
> This is an internal sensor, it should always be present and correct, so
> why would you ignore it?
>

The value is high...very high. Does a flucuating value between 120 and 128
Celsius make sense?

>>     label temp3 "SYS Temp"
>>
>>     compute in0 (@ * 0.8), (@ / 0.8)

Removing this compute line shows a fairly stable in0 value of 1.78

> This is extremely unlikely. While it is frequent to scale down voltage
> inputs so that the results fits in the ADC range, there is no point in
> scaling a voltage up. Especially not by a factor so close to 1: the
> error incurred by the operation would far outweigh the resolution
> improvement at the ADC level. Not to mention that scaling up needs an
> amplifier so it's not a cheap operation - no PC vendor would do this.
>
> So, I seriously doubt that in0 is +1.5V.
>
> In fact, the SCH5127 already has internal scaling resistors on most
> voltage inputs, so if the voltage lines are wired correctly, only
> voltages over +3.3V should need external scaling. These voltages (+5V
> and 5VTR in your case) should be wired to in3 and in4 per chip design,
> to limit the number of inputs which require scaling. VBAT, VCC and VTR
> should be on in2, in5 and in6, which matches your findings. Which means
> that +1.5V and Vccp should be on in0 and in1 (or vice-versa.)

I will dig out this chassis and install a video card in it so I can access
the BIOS values. I can get the system to boot very fast (<=1m boot to
login prompt) so checking "sensors" values against BIOS values is
reasonable.

> Now it is of course possible that Acer did things differently, either
> for pin proximity reasons, or just because they are bad ;)

They can be bad so anything is possible ;)

> Assuming that your CPU does frequency and voltage scaling based on
> load, you should try to put some load on the CPU and check which
> voltage input raises. This would be Vcore (Vccp) and should require no
> scaling. If you can figure that one out, it might help sort out the
> rest.

I will need to find some type of CPU stress program. Even moving 250,000
files (about 70+GB) between hard drives inside the chassis using Rsync
only placed <5% load on the CPU.

>>     compute in1 (@ * 4), (@ / 4)
>>     compute in3 (@ * 4.5), (@ / 4.5)
>>
>>     set in0_min 1.5 * 0.90
>>     set in0_max 1.5 * 1.10
>>     set in1_min 5.0 * 0.90
>>     set in1_max 5.0 * 1.10
>>     set in4_min 1.50 * 0.90
>>     set in4_max 1.50 * 1.10
>>
>>     set fan1_min 600 <- per fan vendor
>
> You're missing a # before your comment.

Comment added during "cut & paste" into email....

>>     set fan2_min 4500
>>     set temp1_min 20
>>     set temp1_max 60
>>     set temp2_min 20
>>     set temp3_min 20
>>     set temp3_max 60
>
> --
> 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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (4 preceding siblings ...)
  2010-11-29 15:51 ` Jeff Rickman
@ 2010-11-29 16:00 ` Jean Delvare
  2010-12-03 21:16 ` Jeff Rickman
                   ` (24 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-11-29 16:00 UTC (permalink / raw)
  To: lm-sensors

Hi Jeff,

On Mon, 29 Nov 2010 09:51:08 -0600 (CST), Jeff Rickman wrote:
> > On Mon, 29 Nov 2010 00:20:44 -0600 (CST), Jeff Rickman wrote:
> >> #    label temp2 "SIO Temp"
> >
> > This is an internal sensor, it should always be present and correct, so
> > why would you ignore it?
> 
> The value is high...very high. Does a flucuating value between 120 and 128
> Celsius make sense?

Indeed not. Juerg, you have the datasheet (I think?), I don't, can you
please check if temp2 is still internal on the SCH5127?

> >>     label temp3 "SYS Temp"
> >>
> >>     compute in0 (@ * 0.8), (@ / 0.8)
> 
> Removing this compute line shows a fairly stable in0 value of 1.78

Which I admit isn't very appealing. Make me wonder if the internal
scaling factors in the driver are correct. Again, Juerg, I have to
defer to you.

> > (...)
> > Assuming that your CPU does frequency and voltage scaling based on
> > load, you should try to put some load on the CPU and check which
> > voltage input raises. This would be Vcore (Vccp) and should require no
> > scaling. If you can figure that one out, it might help sort out the
> > rest.
> 
> I will need to find some type of CPU stress program. Even moving 250,000
> files (about 70+GB) between hard drives inside the chassis using Rsync
> only placed <5% load on the CPU.

I use "md5sum /dev/zero" for this.

-- 
Jean Delvare
http://khali.linux-fr.org/wishlist.html

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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (5 preceding siblings ...)
  2010-11-29 16:00 ` Jean Delvare
@ 2010-12-03 21:16 ` Jeff Rickman
  2010-12-03 23:27 ` Jean Delvare
                   ` (23 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-12-03 21:16 UTC (permalink / raw)
  To: lm-sensors

Hi Jean,

> Hi Jeff,
>
> On Mon, 29 Nov 2010 09:51:08 -0600 (CST), Jeff Rickman wrote:
>> > On Mon, 29 Nov 2010 00:20:44 -0600 (CST), Jeff Rickman wrote:
>> >> #    label temp2 "SIO Temp"
>> >
>> > This is an internal sensor, it should always be present and correct,
>> so
>> > why would you ignore it?
>>
>> The value is high...very high. Does a flucuating value between 120 and
>> 128
>> Celsius make sense?
>
> Indeed not. Juerg, you have the datasheet (I think?), I don't, can you
> please check if temp2 is still internal on the SCH5127?
>
>> >>     label temp3 "SYS Temp"
>> >>
>> >>     compute in0 (@ * 0.8), (@ / 0.8)
>>
>> Removing this compute line shows a fairly stable in0 value of 1.78
>
> Which I admit isn't very appealing. Make me wonder if the internal
> scaling factors in the driver are correct. Again, Juerg, I have to
> defer to you.

I looked at the BIOS report for a sensor it calls "V+1.5". It is the very
first value reported (Top of the list) in the Hardware Monitoring BIOS
screen. The value shown is right on "1.5 volts" at system startup.

>> > (...)
>> > Assuming that your CPU does frequency and voltage scaling based on
>> > load, you should try to put some load on the CPU and check which
>> > voltage input raises. This would be Vcore (Vccp) and should require no
>> > scaling. If you can figure that one out, it might help sort out the
>> > rest.
>>
>> I will need to find some type of CPU stress program. Even moving 250,000
>> files (about 70+GB) between hard drives inside the chassis using Rsync
>> only placed <5% load on the CPU.
>
> I use "md5sum /dev/zero" for this.

Thank you for the info. I ran this test for about 15 minutes. The system
voltages changed slightly...no more than 5 hundredths up or down: that
looks like a very stable power supply. System temperatures did go up: it
pushed CPU temp up about 5 degrees Celsius, from ~42 to ~47, but never
reached 50 degrees Celsius.

Systemgraph shows CPU MHz ramped up to full speed: this system can use the
"userspace" governor with ~200 MHz steps starting at ~200 MHz and topping
out at ~1600 MHz.

>
> --
> Jean Delvare
> http://khali.linux-fr.org/wishlist.html
>



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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (6 preceding siblings ...)
  2010-12-03 21:16 ` Jeff Rickman
@ 2010-12-03 23:27 ` Jean Delvare
  2010-12-09 11:18 ` Juerg Haefliger
                   ` (22 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-03 23:27 UTC (permalink / raw)
  To: lm-sensors

Hi Jeff,

On Fri, 3 Dec 2010 15:16:26 -0600 (CST), Jeff Rickman wrote:
> > Which I admit isn't very appealing. Make me wonder if the internal
> > scaling factors in the driver are correct. Again, Juerg, I have to
> > defer to you.
> 
> I looked at the BIOS report for a sensor it calls "V+1.5". It is the very
> first value reported (Top of the list) in the Hardware Monitoring BIOS
> screen. The value shown is right on "1.5 volts" at system startup.

Strange. This suggests that in0's nominal value is +2.0V and not +2.5V
as the driver assumes. But without the datasheet I can't confirm this. I
don't even remember if we ever had a successful report for SCH5127
support in general and voltages in particular. Juerg?

Note that I wouldn't necessarily trust the order of the voltage labels
in your BIOS to match the DME1737 inputs. There are so many strange
things about your voltages...

> > (...)
> > I use "md5sum /dev/zero" for this.
> 
> Thank you for the info. I ran this test for about 15 minutes. The system
> voltages changed slightly...no more than 5 hundredths up or down: that
> looks like a very stable power supply.

I didn't expect all voltages to change more than this, but I would have
expected one to do and that would have been Vcore. I thought that all
recent CPUs doing frequency scaling were also doing voltage scaling,
but maybe I was wrong on this. But maybe the Intel Atom are different
in this respect, I don't know. Which Atom model is this exactly?

One thing I just noticed about the voltages is that in4 is almost
saturated. It reaches 1.48V while the max is 1.49V (as a matter of
fact, you tried to set the max limit to 1.65V in your configuration
file but it got clamped to 1.49V.) This could mean that you have
voltage seriously off... but assuming your system is running OK, it
doesn't make sense to wire a voltage sensor that way, unless you only
want to detect low voltage on that line, and not high voltage.

The only voltage input for which this makes sense IMHO is Vbat. But if
in4 is Vbat then what is in2? I always thought that it was a little
high for Vbat at +3.32V, but OTOH you already have 2 +3.3V lines as in5
and in6, and it doesn't really make sense to monitor +3.3V three times.
OTOH, one of them could be +5V with ad-hoc scaling, so in1 or in3 would
be Vcore. Which would make some sense as they have values around +1.1V
before scaling, which seems OK for an Atom CPU.

You may be able to find out which voltage is Vbat by booting the system
without a battery inserted - assuming the battery is removable AND this
particular system accepts booting without it.

If you want to sort things out, another option is to stay in the BIOS
setup screen for a longer time and write down _all_ values which are
displayed for _all_ voltages lines over time. The more different values
you collect, the better. It can only work if the BIOS displays 3
decimal places. By analyzing the difference between values, we can
deduce the candidate scaling factors and thus the candidate pin
mappings. This is a little more complex with this chip because of the
internal scaling factors, but this should still be doable. If you write
down all the values and post them on this list, I'll take a look.

Of course there is also still the possibility that Acer messed it all
up so some my assumption are wrong.

> System temperatures did go up: it
> pushed CPU temp up about 5 degrees Celsius, from ~42 to ~47, but never
> reached 50 degrees Celsius.

At least this confirms that you got the temp labels right.

> Systemgraph shows CPU MHz ramped up to full speed: this system can use the
> "userspace" governor with ~200 MHz steps starting at ~200 MHz and topping
> out at ~1600 MHz.

What drives the user-space governor on your system? And what is the
minimum value at which it sets the CPU frequency? Maybe you need to use
the lowest frequency to see a voltage drop. Finding which input is
Vcore would really help a lot.

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (7 preceding siblings ...)
  2010-12-03 23:27 ` Jean Delvare
@ 2010-12-09 11:18 ` Juerg Haefliger
  2010-12-09 11:31 ` Juerg Haefliger
                   ` (21 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Juerg Haefliger @ 2010-12-09 11:18 UTC (permalink / raw)
  To: lm-sensors

Hi Jean and Jeff,

>> >> #    label temp2 "SIO Temp"
>> >
>> > This is an internal sensor, it should always be present and correct, so
>> > why would you ignore it?
>>
>> The value is high...very high. Does a flucuating value between 120 and 128
>> Celsius make sense?
>
> Indeed not. Juerg, you have the datasheet (I think?), I don't, can you
> please check if temp2 is still internal on the SCH5127?

temp2 is still the internal temp diode. Jeff, do you actually get a
value of +128? That would indicate a sensor error. Can you check if
temp2_fault reports a value of 1? It's not a sticky value so you might
have to poll it multiple times to catch it. The datasheet however is
not completely clear if the error condition only applies to the remote
sensors which would make more sense.

...juerg

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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (8 preceding siblings ...)
  2010-12-09 11:18 ` Juerg Haefliger
@ 2010-12-09 11:31 ` Juerg Haefliger
  2010-12-09 12:57 ` Jean Delvare
                   ` (20 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Juerg Haefliger @ 2010-12-09 11:31 UTC (permalink / raw)
  To: lm-sensors

Hi Jean and Jeff,


> On Mon, 29 Nov 2010 00:20:44 -0600 (CST), Jeff Rickman wrote:
>> Providing some feedback now that SCH5127 support is "mainline" in
>> LM_Sensors; no "force_id" option required.
>> (...)
>> [root@anas-01 ~]# service lm_sensors status
>> coretemp-isa-0000
>> Adapter: ISA adapter
>> Core 0:      +21.0°C  (crit = +90.0°C)
>>
>> sch5127-isa-0870
>> Adapter: ISA adapter
>> V+1.5:       +1.42 V  (min =  +1.35 V, max =  +1.65 V)
>> 5VTR:        +4.78 V  (min =  +4.50 V, max =  +5.48 V)
>> VBAT:        +3.32 V  (min =  +0.00 V, max =  +4.38 V)
>> V+5:         +5.09 V  (min =  +0.00 V, max =  +6.72 V)
>> Vccp:        +1.48 V  (min =  +1.35 V, max =  +1.49 V)
>> VCC:         +3.33 V  (min =  +0.00 V, max =  +4.38 V)
>> VTR:         +3.24 V  (min =  +0.00 V, max =  +4.38 V)
>> Side Fan:   2250 RPM  (min = 600 RPM)
>> MCH Fan:    5137 RPM  (min = 4500 RPM)
>> CPU Temp:    +47.6°C  (low  = +20.0°C, high = +60.0°C)
>> SYS Temp:    +35.1°C  (low  = +20.0°C, high = +60.0°C)
>>
>> Here is my "/etc/sensors.d/local.conf" file:
>>
>> [root@anas-01 ~]# cat /etc/sensors.d/local.conf
>> # libsensors configuration file
>> # -----------------------------
>>
>> chip "sch5127-isa-0870"
>>
>>     label in0 "V+1.5"
>>     label in1 "5VTR"
>>     label in2 "VBAT"
>>     label in3 "V+5"
>>     label in4 "Vccp"
>>     label in5 "VCC"
>>     label in6 "VTR"

According to the datasheet (and I just double-checked the scaling
factors in the driver and they are correct), the voltages are as
follows:

in0: 2.5V   (2.5V nominal, 3.320V max)
in1: Vccp   (2.25V nominal, 2.988V max)
in2: VCC   (3.3V nominal, 4.383V max)
in3: V2_IN   (1.125V nominal, 1.5V max, should be wired to scaled down 5V)
in4: V1_IN   (1.125V nominal, 1.5V max, should be wired to scaled down 12V)
in5: VTR   (3.3V nominal, 4.383V max)
in6: VBat   (3.0V nominal, 4.383V max)

If the chip is hooked up according to the datasheet then the only
voltages that require scaling are in3 & 4.

...Juerg

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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (9 preceding siblings ...)
  2010-12-09 11:31 ` Juerg Haefliger
@ 2010-12-09 12:57 ` Jean Delvare
  2010-12-09 13:46 ` Juerg Haefliger
                   ` (19 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-09 12:57 UTC (permalink / raw)
  To: lm-sensors

On Thu, 9 Dec 2010 12:31:28 +0100, Juerg Haefliger wrote:
> Hi Jean and Jeff,
> 
> 
> > On Mon, 29 Nov 2010 00:20:44 -0600 (CST), Jeff Rickman wrote:
> >> Providing some feedback now that SCH5127 support is "mainline" in
> >> LM_Sensors; no "force_id" option required.
> >> (...)
> >> [root@anas-01 ~]# service lm_sensors status
> >> coretemp-isa-0000
> >> Adapter: ISA adapter
> >> Core 0:      +21.0°C  (crit = +90.0°C)
> >>
> >> sch5127-isa-0870
> >> Adapter: ISA adapter
> >> V+1.5:       +1.42 V  (min =  +1.35 V, max =  +1.65 V)
> >> 5VTR:        +4.78 V  (min =  +4.50 V, max =  +5.48 V)
> >> VBAT:        +3.32 V  (min =  +0.00 V, max =  +4.38 V)
> >> V+5:         +5.09 V  (min =  +0.00 V, max =  +6.72 V)
> >> Vccp:        +1.48 V  (min =  +1.35 V, max =  +1.49 V)
> >> VCC:         +3.33 V  (min =  +0.00 V, max =  +4.38 V)
> >> VTR:         +3.24 V  (min =  +0.00 V, max =  +4.38 V)
> >> Side Fan:   2250 RPM  (min = 600 RPM)
> >> MCH Fan:    5137 RPM  (min = 4500 RPM)
> >> CPU Temp:    +47.6°C  (low  = +20.0°C, high = +60.0°C)
> >> SYS Temp:    +35.1°C  (low  = +20.0°C, high = +60.0°C)
> >>
> >> Here is my "/etc/sensors.d/local.conf" file:
> >>
> >> [root@anas-01 ~]# cat /etc/sensors.d/local.conf
> >> # libsensors configuration file
> >> # -----------------------------
> >>
> >> chip "sch5127-isa-0870"
> >>
> >>     label in0 "V+1.5"
> >>     label in1 "5VTR"
> >>     label in2 "VBAT"
> >>     label in3 "V+5"
> >>     label in4 "Vccp"
> >>     label in5 "VCC"
> >>     label in6 "VTR"
> 
> According to the datasheet (and I just double-checked the scaling
> factors in the driver and they are correct), the voltages are as
> follows:
> 
> in0: 2.5V   (2.5V nominal, 3.320V max)
> in1: Vccp   (2.25V nominal, 2.988V max)
> in2: VCC   (3.3V nominal, 4.383V max)
> in3: V2_IN   (1.125V nominal, 1.5V max, should be wired to scaled down 5V)
> in4: V1_IN   (1.125V nominal, 1.5V max, should be wired to scaled down 12V)
> in5: VTR   (3.3V nominal, 4.383V max)
> in6: VBat   (3.0V nominal, 4.383V max)
> 
> If the chip is hooked up according to the datasheet then the only
> voltages that require scaling are in3 & 4.

OK, thanks for checking. Your conclusion matches mine.

Do you happen to know if any of these are internal (so that the
mapping/label doesn't depend on the system)? This would help. in2 and
in6 would be candidates, maybe in5 as well. Do you have any clue what
"VTR" stands for?

For internal voltages we can put the labels in the default
sensors3.conf file, to help the 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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (10 preceding siblings ...)
  2010-12-09 12:57 ` Jean Delvare
@ 2010-12-09 13:46 ` Juerg Haefliger
  2010-12-09 16:09 ` Jean Delvare
                   ` (18 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Juerg Haefliger @ 2010-12-09 13:46 UTC (permalink / raw)
  To: lm-sensors

> On Thu, 9 Dec 2010 12:31:28 +0100, Juerg Haefliger wrote:
>> Hi Jean and Jeff,
>>
>>
>> > On Mon, 29 Nov 2010 00:20:44 -0600 (CST), Jeff Rickman wrote:
>> >> Providing some feedback now that SCH5127 support is "mainline" in
>> >> LM_Sensors; no "force_id" option required.
>> >> (...)
>> >> [root@anas-01 ~]# service lm_sensors status
>> >> coretemp-isa-0000
>> >> Adapter: ISA adapter
>> >> Core 0:      +21.0°C  (crit = +90.0°C)
>> >>
>> >> sch5127-isa-0870
>> >> Adapter: ISA adapter
>> >> V+1.5:       +1.42 V  (min =  +1.35 V, max =  +1.65 V)
>> >> 5VTR:        +4.78 V  (min =  +4.50 V, max =  +5.48 V)
>> >> VBAT:        +3.32 V  (min =  +0.00 V, max =  +4.38 V)
>> >> V+5:         +5.09 V  (min =  +0.00 V, max =  +6.72 V)
>> >> Vccp:        +1.48 V  (min =  +1.35 V, max =  +1.49 V)
>> >> VCC:         +3.33 V  (min =  +0.00 V, max =  +4.38 V)
>> >> VTR:         +3.24 V  (min =  +0.00 V, max =  +4.38 V)
>> >> Side Fan:   2250 RPM  (min = 600 RPM)
>> >> MCH Fan:    5137 RPM  (min = 4500 RPM)
>> >> CPU Temp:    +47.6°C  (low  = +20.0°C, high = +60.0°C)
>> >> SYS Temp:    +35.1°C  (low  = +20.0°C, high = +60.0°C)
>> >>
>> >> Here is my "/etc/sensors.d/local.conf" file:
>> >>
>> >> [root@anas-01 ~]# cat /etc/sensors.d/local.conf
>> >> # libsensors configuration file
>> >> # -----------------------------
>> >>
>> >> chip "sch5127-isa-0870"
>> >>
>> >>     label in0 "V+1.5"
>> >>     label in1 "5VTR"
>> >>     label in2 "VBAT"
>> >>     label in3 "V+5"
>> >>     label in4 "Vccp"
>> >>     label in5 "VCC"
>> >>     label in6 "VTR"
>>
>> According to the datasheet (and I just double-checked the scaling
>> factors in the driver and they are correct), the voltages are as
>> follows:
>>
>> in0: 2.5V   (2.5V nominal, 3.320V max)
>> in1: Vccp   (2.25V nominal, 2.988V max)
>> in2: VCC   (3.3V nominal, 4.383V max)
>> in3: V2_IN   (1.125V nominal, 1.5V max, should be wired to scaled down 5V)
>> in4: V1_IN   (1.125V nominal, 1.5V max, should be wired to scaled down 12V)
>> in5: VTR   (3.3V nominal, 4.383V max)
>> in6: VBat   (3.0V nominal, 4.383V max)
>>
>> If the chip is hooked up according to the datasheet then the only
>> voltages that require scaling are in3 & 4.
>
> OK, thanks for checking. Your conclusion matches mine.
>
> Do you happen to know if any of these are internal (so that the
> mapping/label doesn't depend on the system)? This would help. in2 and
> in6 would be candidates, maybe in5 as well.

From the datasheet it looks like in2, in5 and in6 need to be wired as
specified otherwise the chip will not work properly under certain
conditions. VTR is the standby voltage and if standby mode is not
required, could be connected to VCC externally.


> Do you have any clue what
> "VTR" stands for?

VTR is the ATX standby voltage also called trickle voltage (courtesy
of google :-)


> For internal voltages we can put the labels in the default
> sensors3.conf file, to help the users.

Ah. I just realized that the SCH5127 has another voltage input that
the other dme1737 clones don't have but which isn't exposed through
the driver. Register 0x1f, nominal 1.5V, max 2.0V.


...Juerg


> --
> 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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (11 preceding siblings ...)
  2010-12-09 13:46 ` Juerg Haefliger
@ 2010-12-09 16:09 ` Jean Delvare
  2010-12-10  9:43 ` Juerg Haefliger
                   ` (17 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-09 16:09 UTC (permalink / raw)
  To: lm-sensors

On Thu, 9 Dec 2010 14:46:39 +0100, Juerg Haefliger wrote:
> > Do you happen to know if any of these are internal (so that the
> > mapping/label doesn't depend on the system)? This would help. in2 and
> > in6 would be candidates, maybe in5 as well.
> 
> From the datasheet it looks like in2, in5 and in6 need to be wired as
> specified otherwise the chip will not work properly under certain
> conditions. VTR is the standby voltage and if standby mode is not
> required, could be connected to VCC externally.

I propose the following default configuration file section for the
SCH5217:

chip "sch5127-*"

   label in2  "+3.3V"
   label in5  "3VSB"
   label in6  "Vbat"

> > Do you have any clue what
> > "VTR" stands for?
> 
> VTR is the ATX standby voltage also called trickle voltage (courtesy
> of google :-)

Strange, http://en.wikipedia.org/wiki/ATX doesn't mention either VTR or
trickle voltage. It does mention "+5 V standby" though (purple wire.)

Anyway, I think we can assume this is what other chips name "3VSB".
Which is strange though because apparently ATX has a +5V stand-by but
no +3.3V standby. So I guess that 3VSB/VTR is actually 5VSB with a
scaling factor, either internal or external.

> > For internal voltages we can put the labels in the default
> > sensors3.conf file, to help the users.
> 
> Ah. I just realized that the SCH5127 has another voltage input that
> the other dme1737 clones don't have but which isn't exposed through
> the driver. Register 0x1f, nominal 1.5V, max 2.0V.

Ah ah! Very interesting. It could be where Jeff's mysterious +1.5V is
connected! Could you write a patch adding support?

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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (12 preceding siblings ...)
  2010-12-09 16:09 ` Jean Delvare
@ 2010-12-10  9:43 ` Juerg Haefliger
  2010-12-10 14:27 ` Jean Delvare
                   ` (16 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Juerg Haefliger @ 2010-12-10  9:43 UTC (permalink / raw)
  To: lm-sensors

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

Hi Jeff,

>> Ah. I just realized that the SCH5127 has another voltage input that
>> the other dme1737 clones don't have but which isn't exposed through
>> the driver. Register 0x1f, nominal 1.5V, max 2.0V.
>
> Ah ah! Very interesting. It could be where Jeff's mysterious +1.5V is
> connected! Could you write a patch adding support?

Attached is a modified driver with support for Vtrip monitoring (1.5V,
in7). Could you give it a try and let me know how it goes? The new
driver should expose the following new sysfs files: in7_input,
in7_min, in7_max, in7_alarm. No user-level scaling is required. The
nominal voltage is 1.5V.

Thanks
...Juerg

[-- Attachment #2: dme1737.c --]
[-- Type: text/x-csrc, Size: 80102 bytes --]

/*
 * dme1737.c - Driver for the SMSC DME1737, Asus A8000, SMSC SCH311x, SCH5027,
 *             and SCH5127 Super-I/O chips integrated hardware monitoring
 *             features.
 * Copyright (c) 2007, 2008, 2009, 2010 Juerg Haefliger <juergh@gmail.com>
 *
 * This driver is an I2C/ISA hybrid, meaning that it uses the I2C bus to access
 * the chip registers if a DME1737, A8000, or SCH5027 is found and the ISA bus
 * if a SCH311x or SCH5127 chip is found. Both types of chips have very
 * similar hardware monitoring capabilities but differ in the way they can be
 * accessed.
 *
 * 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
 * the Free Software Foundation; either version 2 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the Free Software
 * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 */

#include <linux/module.h>
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/jiffies.h>
#include <linux/i2c.h>
#include <linux/platform_device.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
#include <linux/hwmon-vid.h>
#include <linux/err.h>
#include <linux/mutex.h>
#include <linux/acpi.h>
#include <linux/io.h>

/* ISA device, if found */
static struct platform_device *pdev;

/* Module load parameters */
static int force_start;
module_param(force_start, bool, 0);
MODULE_PARM_DESC(force_start, "Force the chip to start monitoring inputs");

static unsigned short force_id;
module_param(force_id, ushort, 0);
MODULE_PARM_DESC(force_id, "Override the detected device ID");

static int probe_all_addr;
module_param(probe_all_addr, bool, 0);
MODULE_PARM_DESC(probe_all_addr, "Include probing of non-standard LPC "
		 "addresses");

/* Addresses to scan */
static const unsigned short normal_i2c[] = {0x2c, 0x2d, 0x2e, I2C_CLIENT_END};

enum chips { dme1737, sch5027, sch311x, sch5127 };

/* ---------------------------------------------------------------------
 * Registers
 *
 * The sensors are defined as follows:
 *
 * Voltages                          Temperatures
 * --------                          ------------
 * in0   +5VTR (+5V stdby)           temp1   Remote diode 1
 * in1   Vccp  (proc core)           temp2   Internal temp
 * in2   VCC   (internal +3.3V)      temp3   Remote diode 2
 * in3   +5V
 * in4   +12V
 * in5   VTR   (+3.3V stby)
 * in6   Vbat
 * in7   Vtrip (sch5127 only)
 *
 * --------------------------------------------------------------------- */

/* Voltages (in) numbered 0-7 (ix) */
#define	DME1737_REG_IN(ix)		((ix) < 5 ? 0x20 + (ix) : \
					 (ix) < 7 ? 0x94 + (ix) : \
						    0x1f)
#define	DME1737_REG_IN_MIN(ix)		((ix) < 5 ? 0x44 + (ix) * 2 : \
					 (ix) < 7 ? 0x91 + (ix) * 2 : \
						    0x9f)
#define	DME1737_REG_IN_MAX(ix)		((ix) < 5 ? 0x45 + (ix) * 2 : \
					 (ix) < 7 ? 0x92 + (ix) * 2 : \
						    0xa0)

/* Temperatures (temp) numbered 0-2 (ix) */
#define DME1737_REG_TEMP(ix)		(0x25 + (ix))
#define DME1737_REG_TEMP_MIN(ix)	(0x4e + (ix) * 2)
#define DME1737_REG_TEMP_MAX(ix)	(0x4f + (ix) * 2)
#define DME1737_REG_TEMP_OFFSET(ix)	((ix) == 0 ? 0x1f \
						   : 0x1c + (ix))

/* Voltage and temperature LSBs
 * The LSBs (4 bits each) are stored in 5 registers with the following layouts:
 *    IN_TEMP_LSB(0) = [in5, in6]
 *    IN_TEMP_LSB(1) = [temp3, temp1]
 *    IN_TEMP_LSB(2) = [in4, temp2]
 *    IN_TEMP_LSB(3) = [in3, in0]
 *    IN_TEMP_LSB(4) = [in2, in1]
 *    IN_TEMP_LSB(5) = [res, in7] */
#define DME1737_REG_IN_TEMP_LSB(ix)	(0x84 + (ix))
static const u8 DME1737_REG_IN_LSB[] = {3, 4, 4, 3, 2, 0, 0, 5};
static const u8 DME1737_REG_IN_LSB_SHL[] = {4, 4, 0, 0, 0, 0, 4, 4};
static const u8 DME1737_REG_TEMP_LSB[] = {1, 2, 1};
static const u8 DME1737_REG_TEMP_LSB_SHL[] = {4, 4, 0};

/* Fans numbered 0-5 (ix) */
#define DME1737_REG_FAN(ix)		((ix) < 4 ? 0x28 + (ix) * 2 \
						  : 0xa1 + (ix) * 2)
#define DME1737_REG_FAN_MIN(ix)		((ix) < 4 ? 0x54 + (ix) * 2 \
						  : 0xa5 + (ix) * 2)
#define DME1737_REG_FAN_OPT(ix)		((ix) < 4 ? 0x90 + (ix) \
						  : 0xb2 + (ix))
#define DME1737_REG_FAN_MAX(ix)		(0xb4 + (ix)) /* only for fan[4-5] */

/* PWMs numbered 0-2, 4-5 (ix) */
#define DME1737_REG_PWM(ix)		((ix) < 3 ? 0x30 + (ix) \
						  : 0xa1 + (ix))
#define DME1737_REG_PWM_CONFIG(ix)	(0x5c + (ix)) /* only for pwm[0-2] */
#define DME1737_REG_PWM_MIN(ix)		(0x64 + (ix)) /* only for pwm[0-2] */
#define DME1737_REG_PWM_FREQ(ix)	((ix) < 3 ? 0x5f + (ix) \
						  : 0xa3 + (ix))
/* The layout of the ramp rate registers is different from the other pwm
 * registers. The bits for the 3 PWMs are stored in 2 registers:
 *    PWM_RR(0) = [OFF3, OFF2,  OFF1,  RES,   RR1E, RR1-2, RR1-1, RR1-0]
 *    PWM_RR(1) = [RR2E, RR2-2, RR2-1, RR2-0, RR3E, RR3-2, RR3-1, RR3-0] */
#define DME1737_REG_PWM_RR(ix)		(0x62 + (ix)) /* only for pwm[0-2] */

/* Thermal zones 0-2 */
#define DME1737_REG_ZONE_LOW(ix)	(0x67 + (ix))
#define DME1737_REG_ZONE_ABS(ix)	(0x6a + (ix))
/* The layout of the hysteresis registers is different from the other zone
 * registers. The bits for the 3 zones are stored in 2 registers:
 *    ZONE_HYST(0) = [H1-3,  H1-2,  H1-1, H1-0, H2-3, H2-2, H2-1, H2-0]
 *    ZONE_HYST(1) = [H3-3,  H3-2,  H3-1, H3-0, RES,  RES,  RES,  RES] */
#define DME1737_REG_ZONE_HYST(ix)	(0x6d + (ix))

/* Alarm registers and bit mapping
 * The 3 8-bit alarm registers will be concatenated to a single 32-bit
 * alarm value [0, ALARM3, ALARM2, ALARM1]. */
#define DME1737_REG_ALARM1		0x41
#define DME1737_REG_ALARM2		0x42
#define DME1737_REG_ALARM3		0x83
static const u8 DME1737_BIT_ALARM_IN[] = {0, 1, 2, 3, 8, 16, 17, 18};
static const u8 DME1737_BIT_ALARM_TEMP[] = {4, 5, 6};
static const u8 DME1737_BIT_ALARM_FAN[] = {10, 11, 12, 13, 22, 23};

/* Miscellaneous registers */
#define DME1737_REG_DEVICE		0x3d
#define DME1737_REG_COMPANY		0x3e
#define DME1737_REG_VERSTEP		0x3f
#define DME1737_REG_CONFIG		0x40
#define DME1737_REG_CONFIG2		0x7f
#define DME1737_REG_VID			0x43
#define DME1737_REG_TACH_PWM		0x81

/* ---------------------------------------------------------------------
 * Misc defines
 * --------------------------------------------------------------------- */

/* Chip identification */
#define DME1737_COMPANY_SMSC	0x5c
#define DME1737_VERSTEP		0x88
#define DME1737_VERSTEP_MASK	0xf8
#define SCH311X_DEVICE		0x8c
#define SCH5027_VERSTEP		0x69
#define SCH5127_DEVICE		0x8e

/* Device ID values (global configuration register index 0x20) */
#define DME1737_ID_1	0x77
#define DME1737_ID_2	0x78
#define SCH3112_ID	0x7c
#define SCH3114_ID	0x7d
#define SCH3116_ID	0x7f
#define SCH5027_ID	0x89
#define SCH5127_ID	0x86

/* Length of ISA address segment */
#define DME1737_EXTENT	2

/* chip-dependent features */
#define HAS_TEMP_OFFSET		(1 << 0)		/* bit 0 */
#define HAS_VID			(1 << 1)		/* bit 1 */
#define HAS_ZONE3		(1 << 2)		/* bit 2 */
#define HAS_ZONE_HYST		(1 << 3)		/* bit 3 */
#define HAS_PWM_MIN		(1 << 4)		/* bit 4 */
#define HAS_FAN(ix)		(1 << ((ix) + 5))	/* bits 5-10 */
#define HAS_PWM(ix)		(1 << ((ix) + 11))	/* bits 11-16 */
#define HAS_IN7			(1 << 17)		/* bit 17 */

/* ---------------------------------------------------------------------
 * Data structures and manipulation thereof
 * --------------------------------------------------------------------- */

struct dme1737_data {
	struct i2c_client *client;	/* for I2C devices only */
	struct device *hwmon_dev;
	const char *name;
	unsigned int addr;		/* for ISA devices only */

	struct mutex update_lock;
	int valid;			/* !=0 if following fields are valid */
	unsigned long last_update;	/* in jiffies */
	unsigned long last_vbat;	/* in jiffies */
	enum chips type;
	const int *in_nominal;		/* pointer to IN_NOMINAL array */

	u8 vid;
	u8 pwm_rr_en;
	u32 has_features;

	/* Register values */
	u16 in[7];
	u8  in_min[7];
	u8  in_max[7];
	s16 temp[3];
	s8  temp_min[3];
	s8  temp_max[3];
	s8  temp_offset[3];
	u8  config;
	u8  config2;
	u8  vrm;
	u16 fan[6];
	u16 fan_min[6];
	u8  fan_max[2];
	u8  fan_opt[6];
	u8  pwm[6];
	u8  pwm_min[3];
	u8  pwm_config[3];
	u8  pwm_acz[3];
	u8  pwm_freq[6];
	u8  pwm_rr[2];
	u8  zone_low[3];
	u8  zone_abs[3];
	u8  zone_hyst[2];
	u32 alarms;
};

/* Nominal voltage values */
static const int IN_NOMINAL_DME1737[] = {5000, 2250, 3300, 5000, 12000, 3300,
					 3300};
static const int IN_NOMINAL_SCH311x[] = {2500, 1500, 3300, 5000, 12000, 3300,
					 3300};
static const int IN_NOMINAL_SCH5027[] = {5000, 2250, 3300, 1125, 1125, 3300,
					 3300};
static const int IN_NOMINAL_SCH5127[] = {2500, 2250, 3300, 1125, 1125, 3300,
					 3300, 1500};
#define IN_NOMINAL(type)	((type) == sch311x ? IN_NOMINAL_SCH311x : \
				 (type) == sch5027 ? IN_NOMINAL_SCH5027 : \
				 (type) == sch5127 ? IN_NOMINAL_SCH5127 : \
				 IN_NOMINAL_DME1737)

/* Voltage input
 * Voltage inputs have 16 bits resolution, limit values have 8 bits
 * resolution. */
static inline int IN_FROM_REG(int reg, int nominal, int res)
{
	return (reg * nominal + (3 << (res - 3))) / (3 << (res - 2));
}

static inline int IN_TO_REG(int val, int nominal)
{
	return SENSORS_LIMIT((val * 192 + nominal / 2) / nominal, 0, 255);
}

/* Temperature input
 * The register values represent temperatures in 2's complement notation from
 * -127 degrees C to +127 degrees C. Temp inputs have 16 bits resolution, limit
 * values have 8 bits resolution. */
static inline int TEMP_FROM_REG(int reg, int res)
{
	return (reg * 1000) >> (res - 8);
}

static inline int TEMP_TO_REG(int val)
{
	return SENSORS_LIMIT((val < 0 ? val - 500 : val + 500) / 1000,
			     -128, 127);
}

/* Temperature range */
static const int TEMP_RANGE[] = {2000, 2500, 3333, 4000, 5000, 6666, 8000,
				 10000, 13333, 16000, 20000, 26666, 32000,
				 40000, 53333, 80000};

static inline int TEMP_RANGE_FROM_REG(int reg)
{
	return TEMP_RANGE[(reg >> 4) & 0x0f];
}

static int TEMP_RANGE_TO_REG(int val, int reg)
{
	int i;

	for (i = 15; i > 0; i--) {
		if (val > (TEMP_RANGE[i] + TEMP_RANGE[i - 1] + 1) / 2) {
			break;
		}
	}

	return (reg & 0x0f) | (i << 4);
}

/* Temperature hysteresis
 * Register layout:
 *    reg[0] = [H1-3, H1-2, H1-1, H1-0, H2-3, H2-2, H2-1, H2-0]
 *    reg[1] = [H3-3, H3-2, H3-1, H3-0, xxxx, xxxx, xxxx, xxxx] */
static inline int TEMP_HYST_FROM_REG(int reg, int ix)
{
	return (((ix == 1) ? reg : reg >> 4) & 0x0f) * 1000;
}

static inline int TEMP_HYST_TO_REG(int val, int ix, int reg)
{
	int hyst = SENSORS_LIMIT((val + 500) / 1000, 0, 15);

	return (ix == 1) ? (reg & 0xf0) | hyst : (reg & 0x0f) | (hyst << 4);
}

/* Fan input RPM */
static inline int FAN_FROM_REG(int reg, int tpc)
{
	if (tpc) {
		return tpc * reg;
	} else {
		return (reg == 0 || reg == 0xffff) ? 0 : 90000 * 60 / reg;
	}
}

static inline int FAN_TO_REG(int val, int tpc)
{
	if (tpc) {
		return SENSORS_LIMIT(val / tpc, 0, 0xffff);
	} else {
		return (val <= 0) ? 0xffff :
			SENSORS_LIMIT(90000 * 60 / val, 0, 0xfffe);
	}
}

/* Fan TPC (tach pulse count)
 * Converts a register value to a TPC multiplier or returns 0 if the tachometer
 * is configured in legacy (non-tpc) mode */
static inline int FAN_TPC_FROM_REG(int reg)
{
	return (reg & 0x20) ? 0 : 60 >> (reg & 0x03);
}

/* Fan type
 * The type of a fan is expressed in number of pulses-per-revolution that it
 * emits */
static inline int FAN_TYPE_FROM_REG(int reg)
{
	int edge = (reg >> 1) & 0x03;

	return (edge > 0) ? 1 << (edge - 1) : 0;
}

static inline int FAN_TYPE_TO_REG(int val, int reg)
{
	int edge = (val == 4) ? 3 : val;

	return (reg & 0xf9) | (edge << 1);
}

/* Fan max RPM */
static const int FAN_MAX[] = {0x54, 0x38, 0x2a, 0x21, 0x1c, 0x18, 0x15, 0x12,
			      0x11, 0x0f, 0x0e};

static int FAN_MAX_FROM_REG(int reg)
{
	int i;

	for (i = 10; i > 0; i--) {
		if (reg == FAN_MAX[i]) {
			break;
		}
	}

	return 1000 + i * 500;
}

static int FAN_MAX_TO_REG(int val)
{
	int i;

	for (i = 10; i > 0; i--) {
		if (val > (1000 + (i - 1) * 500)) {
			break;
		}
	}

	return FAN_MAX[i];
}

/* PWM enable
 * Register to enable mapping:
 * 000:  2  fan on zone 1 auto
 * 001:  2  fan on zone 2 auto
 * 010:  2  fan on zone 3 auto
 * 011:  0  fan full on
 * 100: -1  fan disabled
 * 101:  2  fan on hottest of zones 2,3 auto
 * 110:  2  fan on hottest of zones 1,2,3 auto
 * 111:  1  fan in manual mode */
static inline int PWM_EN_FROM_REG(int reg)
{
	static const int en[] = {2, 2, 2, 0, -1, 2, 2, 1};

	return en[(reg >> 5) & 0x07];
}

static inline int PWM_EN_TO_REG(int val, int reg)
{
	int en = (val == 1) ? 7 : 3;

	return (reg & 0x1f) | ((en & 0x07) << 5);
}

/* PWM auto channels zone
 * Register to auto channels zone mapping (ACZ is a bitfield with bit x
 * corresponding to zone x+1):
 * 000: 001  fan on zone 1 auto
 * 001: 010  fan on zone 2 auto
 * 010: 100  fan on zone 3 auto
 * 011: 000  fan full on
 * 100: 000  fan disabled
 * 101: 110  fan on hottest of zones 2,3 auto
 * 110: 111  fan on hottest of zones 1,2,3 auto
 * 111: 000  fan in manual mode */
static inline int PWM_ACZ_FROM_REG(int reg)
{
	static const int acz[] = {1, 2, 4, 0, 0, 6, 7, 0};

	return acz[(reg >> 5) & 0x07];
}

static inline int PWM_ACZ_TO_REG(int val, int reg)
{
	int acz = (val == 4) ? 2 : val - 1;

	return (reg & 0x1f) | ((acz & 0x07) << 5);
}

/* PWM frequency */
static const int PWM_FREQ[] = {11, 15, 22, 29, 35, 44, 59, 88,
			       15000, 20000, 30000, 25000, 0, 0, 0, 0};

static inline int PWM_FREQ_FROM_REG(int reg)
{
	return PWM_FREQ[reg & 0x0f];
}

static int PWM_FREQ_TO_REG(int val, int reg)
{
	int i;

	/* the first two cases are special - stupid chip design! */
	if (val > 27500) {
		i = 10;
	} else if (val > 22500) {
		i = 11;
	} else {
		for (i = 9; i > 0; i--) {
			if (val > (PWM_FREQ[i] + PWM_FREQ[i - 1] + 1) / 2) {
				break;
			}
		}
	}

	return (reg & 0xf0) | i;
}

/* PWM ramp rate
 * Register layout:
 *    reg[0] = [OFF3,  OFF2,  OFF1,  RES,   RR1-E, RR1-2, RR1-1, RR1-0]
 *    reg[1] = [RR2-E, RR2-2, RR2-1, RR2-0, RR3-E, RR3-2, RR3-1, RR3-0] */
static const u8 PWM_RR[] = {206, 104, 69, 41, 26, 18, 10, 5};

static inline int PWM_RR_FROM_REG(int reg, int ix)
{
	int rr = (ix == 1) ? reg >> 4 : reg;

	return (rr & 0x08) ? PWM_RR[rr & 0x07] : 0;
}

static int PWM_RR_TO_REG(int val, int ix, int reg)
{
	int i;

	for (i = 0; i < 7; i++) {
		if (val > (PWM_RR[i] + PWM_RR[i + 1] + 1) / 2) {
			break;
		}
	}

	return (ix == 1) ? (reg & 0x8f) | (i << 4) : (reg & 0xf8) | i;
}

/* PWM ramp rate enable */
static inline int PWM_RR_EN_FROM_REG(int reg, int ix)
{
	return PWM_RR_FROM_REG(reg, ix) ? 1 : 0;
}

static inline int PWM_RR_EN_TO_REG(int val, int ix, int reg)
{
	int en = (ix == 1) ? 0x80 : 0x08;

	return val ? reg | en : reg & ~en;
}

/* PWM min/off
 * The PWM min/off bits are part of the PMW ramp rate register 0 (see above for
 * the register layout). */
static inline int PWM_OFF_FROM_REG(int reg, int ix)
{
	return (reg >> (ix + 5)) & 0x01;
}

static inline int PWM_OFF_TO_REG(int val, int ix, int reg)
{
	return (reg & ~(1 << (ix + 5))) | ((val & 0x01) << (ix + 5));
}

/* ---------------------------------------------------------------------
 * Device I/O access
 *
 * ISA access is performed through an index/data register pair and needs to
 * be protected by a mutex during runtime (not required for initialization).
 * We use data->update_lock for this and need to ensure that we acquire it
 * before calling dme1737_read or dme1737_write.
 * --------------------------------------------------------------------- */

static u8 dme1737_read(const struct dme1737_data *data, u8 reg)
{
	struct i2c_client *client = data->client;
	s32 val;

	if (client) { /* I2C device */
		val = i2c_smbus_read_byte_data(client, reg);

		if (val < 0) {
			dev_warn(&client->dev, "Read from register "
				 "0x%02x failed! Please report to the driver "
				 "maintainer.\n", reg);
		}
	} else { /* ISA device */
		outb(reg, data->addr);
		val = inb(data->addr + 1);
	}

	return val;
}

static s32 dme1737_write(const struct dme1737_data *data, u8 reg, u8 val)
{
	struct i2c_client *client = data->client;
	s32 res = 0;

	if (client) { /* I2C device */
		res = i2c_smbus_write_byte_data(client, reg, val);

		if (res < 0) {
			dev_warn(&client->dev, "Write to register "
				 "0x%02x failed! Please report to the driver "
				 "maintainer.\n", reg);
		}
	} else { /* ISA device */
		outb(reg, data->addr);
		outb(val, data->addr + 1);
	}

	return res;
}

static struct dme1737_data *dme1737_update_device(struct device *dev)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	int ix;
	u8 lsb[6];

	mutex_lock(&data->update_lock);

	/* Enable a Vbat monitoring cycle every 10 mins */
	if (time_after(jiffies, data->last_vbat + 600 * HZ) || !data->valid) {
		dme1737_write(data, DME1737_REG_CONFIG, dme1737_read(data,
						DME1737_REG_CONFIG) | 0x10);
		data->last_vbat = jiffies;
	}

	/* Sample register contents every 1 sec */
	if (time_after(jiffies, data->last_update + HZ) || !data->valid) {
		if (data->has_features & HAS_VID) {
			data->vid = dme1737_read(data, DME1737_REG_VID) &
				0x3f;
		}

		/* In (voltage) registers */
		for (ix = 0; ix < ARRAY_SIZE(data->in); ix++) {
			/* Voltage inputs are stored as 16 bit values even
			 * though they have only 12 bits resolution. This is
			 * to make it consistent with the temp inputs. */
			if (ix != 7 || (data->has_features & HAS_IN7)) {
				data->in[ix] = dme1737_read(data,
					DME1737_REG_IN(ix)) << 8;
				data->in_min[ix] = dme1737_read(data,
					DME1737_REG_IN_MIN(ix));
				data->in_max[ix] = dme1737_read(data,
					DME1737_REG_IN_MAX(ix));
			}
		}

		/* Temp registers */
		for (ix = 0; ix < ARRAY_SIZE(data->temp); ix++) {
			/* Temp inputs are stored as 16 bit values even
			 * though they have only 12 bits resolution. This is
			 * to take advantage of implicit conversions between
			 * register values (2's complement) and temp values
			 * (signed decimal). */
			data->temp[ix] = dme1737_read(data,
					DME1737_REG_TEMP(ix)) << 8;
			data->temp_min[ix] = dme1737_read(data,
					DME1737_REG_TEMP_MIN(ix));
			data->temp_max[ix] = dme1737_read(data,
					DME1737_REG_TEMP_MAX(ix));
			if (data->has_features & HAS_TEMP_OFFSET) {
				data->temp_offset[ix] = dme1737_read(data,
						DME1737_REG_TEMP_OFFSET(ix));
			}
		}

		/* In and temp LSB registers
		 * The LSBs are latched when the MSBs are read, so the order in
		 * which the registers are read (MSB first, then LSB) is
		 * important! */
		for (ix = 0; ix < ARRAY_SIZE(lsb); ix++) {
			if (ix != 5 || (data->has_features & HAS_IN7)) {
				lsb[ix] = dme1737_read(data,
					DME1737_REG_IN_TEMP_LSB(ix));
			}
		}
		for (ix = 0; ix < ARRAY_SIZE(data->in); ix++) {
			if (ix != 7 || (data->has_features & HAS_IN7)) {
				data->in[ix] |= (lsb[DME1737_REG_IN_LSB[ix]] <<
					DME1737_REG_IN_LSB_SHL[ix]) & 0xf0;
			}
		}
		for (ix = 0; ix < ARRAY_SIZE(data->temp); ix++) {
			data->temp[ix] |= (lsb[DME1737_REG_TEMP_LSB[ix]] <<
					DME1737_REG_TEMP_LSB_SHL[ix]) & 0xf0;
		}

		/* Fan registers */
		for (ix = 0; ix < ARRAY_SIZE(data->fan); ix++) {
			/* Skip reading registers if optional fans are not
			 * present */
			if (!(data->has_features & HAS_FAN(ix))) {
				continue;
			}
			data->fan[ix] = dme1737_read(data,
					DME1737_REG_FAN(ix));
			data->fan[ix] |= dme1737_read(data,
					DME1737_REG_FAN(ix) + 1) << 8;
			data->fan_min[ix] = dme1737_read(data,
					DME1737_REG_FAN_MIN(ix));
			data->fan_min[ix] |= dme1737_read(data,
					DME1737_REG_FAN_MIN(ix) + 1) << 8;
			data->fan_opt[ix] = dme1737_read(data,
					DME1737_REG_FAN_OPT(ix));
			/* fan_max exists only for fan[5-6] */
			if (ix > 3) {
				data->fan_max[ix - 4] = dme1737_read(data,
					DME1737_REG_FAN_MAX(ix));
			}
		}

		/* PWM registers */
		for (ix = 0; ix < ARRAY_SIZE(data->pwm); ix++) {
			/* Skip reading registers if optional PWMs are not
			 * present */
			if (!(data->has_features & HAS_PWM(ix))) {
				continue;
			}
			data->pwm[ix] = dme1737_read(data,
					DME1737_REG_PWM(ix));
			data->pwm_freq[ix] = dme1737_read(data,
					DME1737_REG_PWM_FREQ(ix));
			/* pwm_config and pwm_min exist only for pwm[1-3] */
			if (ix < 3) {
				data->pwm_config[ix] = dme1737_read(data,
						DME1737_REG_PWM_CONFIG(ix));
				data->pwm_min[ix] = dme1737_read(data,
						DME1737_REG_PWM_MIN(ix));
			}
		}
		for (ix = 0; ix < ARRAY_SIZE(data->pwm_rr); ix++) {
			data->pwm_rr[ix] = dme1737_read(data,
						DME1737_REG_PWM_RR(ix));
		}

		/* Thermal zone registers */
		for (ix = 0; ix < ARRAY_SIZE(data->zone_low); ix++) {
			/* Skip reading registers if zone3 is not present */
			if ((ix == 2) && !(data->has_features & HAS_ZONE3)) {
				continue;
			}
			/* sch5127 zone2 registers are special */
			if ((ix == 1) && (data->type == sch5127)) {
				data->zone_low[1] = dme1737_read(data,
						DME1737_REG_ZONE_LOW(2));
				data->zone_abs[1] = dme1737_read(data,
						DME1737_REG_ZONE_ABS(2));
			} else {
				data->zone_low[ix] = dme1737_read(data,
						DME1737_REG_ZONE_LOW(ix));
				data->zone_abs[ix] = dme1737_read(data,
						DME1737_REG_ZONE_ABS(ix));
			}
		}
		if (data->has_features & HAS_ZONE_HYST) {
			for (ix = 0; ix < ARRAY_SIZE(data->zone_hyst); ix++) {
				data->zone_hyst[ix] = dme1737_read(data,
						DME1737_REG_ZONE_HYST(ix));
			}
		}

		/* Alarm registers */
		data->alarms = dme1737_read(data,
						DME1737_REG_ALARM1);
		/* Bit 7 tells us if the other alarm registers are non-zero and
		 * therefore also need to be read */
		if (data->alarms & 0x80) {
			data->alarms |= dme1737_read(data,
						DME1737_REG_ALARM2) << 8;
			data->alarms |= dme1737_read(data,
						DME1737_REG_ALARM3) << 16;
		}

		/* The ISA chips require explicit clearing of alarm bits.
		 * Don't worry, an alarm will come back if the condition
		 * that causes it still exists */
		if (!data->client) {
			if (data->alarms & 0xff0000) {
				dme1737_write(data, DME1737_REG_ALARM3,
					      0xff);
			}
			if (data->alarms & 0xff00) {
				dme1737_write(data, DME1737_REG_ALARM2,
					      0xff);
			}
			if (data->alarms & 0xff) {
				dme1737_write(data, DME1737_REG_ALARM1,
					      0xff);
			}
		}

		data->last_update = jiffies;
		data->valid = 1;
	}

	mutex_unlock(&data->update_lock);

	return data;
}

/* ---------------------------------------------------------------------
 * Voltage sysfs attributes
 * ix = [0-6]
 * --------------------------------------------------------------------- */

#define SYS_IN_INPUT	0
#define SYS_IN_MIN	1
#define SYS_IN_MAX	2
#define SYS_IN_ALARM	3

static ssize_t show_in(struct device *dev, struct device_attribute *attr,
		       char *buf)
{
	struct dme1737_data *data = dme1737_update_device(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	int res;

	switch (fn) {
	case SYS_IN_INPUT:
		res = IN_FROM_REG(data->in[ix], data->in_nominal[ix], 16);
		break;
	case SYS_IN_MIN:
		res = IN_FROM_REG(data->in_min[ix], data->in_nominal[ix], 8);
		break;
	case SYS_IN_MAX:
		res = IN_FROM_REG(data->in_max[ix], data->in_nominal[ix], 8);
		break;
	case SYS_IN_ALARM:
		res = (data->alarms >> DME1737_BIT_ALARM_IN[ix]) & 0x01;
		break;
	default:
		res = 0;
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}

	return sprintf(buf, "%d\n", res);
}

static ssize_t set_in(struct device *dev, struct device_attribute *attr,
		      const char *buf, size_t count)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	long val = simple_strtol(buf, NULL, 10);

	mutex_lock(&data->update_lock);
	switch (fn) {
	case SYS_IN_MIN:
		data->in_min[ix] = IN_TO_REG(val, data->in_nominal[ix]);
		dme1737_write(data, DME1737_REG_IN_MIN(ix),
			      data->in_min[ix]);
		break;
	case SYS_IN_MAX:
		data->in_max[ix] = IN_TO_REG(val, data->in_nominal[ix]);
		dme1737_write(data, DME1737_REG_IN_MAX(ix),
			      data->in_max[ix]);
		break;
	default:
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}
	mutex_unlock(&data->update_lock);

	return count;
}

/* ---------------------------------------------------------------------
 * Temperature sysfs attributes
 * ix = [0-2]
 * --------------------------------------------------------------------- */

#define SYS_TEMP_INPUT			0
#define SYS_TEMP_MIN			1
#define SYS_TEMP_MAX			2
#define SYS_TEMP_OFFSET			3
#define SYS_TEMP_ALARM			4
#define SYS_TEMP_FAULT			5

static ssize_t show_temp(struct device *dev, struct device_attribute *attr,
			 char *buf)
{
	struct dme1737_data *data = dme1737_update_device(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	int res;

	switch (fn) {
	case SYS_TEMP_INPUT:
		res = TEMP_FROM_REG(data->temp[ix], 16);
		break;
	case SYS_TEMP_MIN:
		res = TEMP_FROM_REG(data->temp_min[ix], 8);
		break;
	case SYS_TEMP_MAX:
		res = TEMP_FROM_REG(data->temp_max[ix], 8);
		break;
	case SYS_TEMP_OFFSET:
		res = TEMP_FROM_REG(data->temp_offset[ix], 8);
		break;
	case SYS_TEMP_ALARM:
		res = (data->alarms >> DME1737_BIT_ALARM_TEMP[ix]) & 0x01;
		break;
	case SYS_TEMP_FAULT:
		res = (((u16)data->temp[ix] & 0xff00) == 0x8000);
		break;
	default:
		res = 0;
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}

	return sprintf(buf, "%d\n", res);
}

static ssize_t set_temp(struct device *dev, struct device_attribute *attr,
			const char *buf, size_t count)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	long val = simple_strtol(buf, NULL, 10);

	mutex_lock(&data->update_lock);
	switch (fn) {
	case SYS_TEMP_MIN:
		data->temp_min[ix] = TEMP_TO_REG(val);
		dme1737_write(data, DME1737_REG_TEMP_MIN(ix),
			      data->temp_min[ix]);
		break;
	case SYS_TEMP_MAX:
		data->temp_max[ix] = TEMP_TO_REG(val);
		dme1737_write(data, DME1737_REG_TEMP_MAX(ix),
			      data->temp_max[ix]);
		break;
	case SYS_TEMP_OFFSET:
		data->temp_offset[ix] = TEMP_TO_REG(val);
		dme1737_write(data, DME1737_REG_TEMP_OFFSET(ix),
			      data->temp_offset[ix]);
		break;
	default:
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}
	mutex_unlock(&data->update_lock);

	return count;
}

/* ---------------------------------------------------------------------
 * Zone sysfs attributes
 * ix = [0-2]
 * --------------------------------------------------------------------- */

#define SYS_ZONE_AUTO_CHANNELS_TEMP	0
#define SYS_ZONE_AUTO_POINT1_TEMP_HYST	1
#define SYS_ZONE_AUTO_POINT1_TEMP	2
#define SYS_ZONE_AUTO_POINT2_TEMP	3
#define SYS_ZONE_AUTO_POINT3_TEMP	4

static ssize_t show_zone(struct device *dev, struct device_attribute *attr,
			 char *buf)
{
	struct dme1737_data *data = dme1737_update_device(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	int res;

	switch (fn) {
	case SYS_ZONE_AUTO_CHANNELS_TEMP:
		/* check config2 for non-standard temp-to-zone mapping */
		if ((ix == 1) && (data->config2 & 0x02)) {
			res = 4;
		} else {
			res = 1 << ix;
		}
		break;
	case SYS_ZONE_AUTO_POINT1_TEMP_HYST:
		res = TEMP_FROM_REG(data->zone_low[ix], 8) -
		      TEMP_HYST_FROM_REG(data->zone_hyst[ix == 2], ix);
		break;
	case SYS_ZONE_AUTO_POINT1_TEMP:
		res = TEMP_FROM_REG(data->zone_low[ix], 8);
		break;
	case SYS_ZONE_AUTO_POINT2_TEMP:
		/* pwm_freq holds the temp range bits in the upper nibble */
		res = TEMP_FROM_REG(data->zone_low[ix], 8) +
		      TEMP_RANGE_FROM_REG(data->pwm_freq[ix]);
		break;
	case SYS_ZONE_AUTO_POINT3_TEMP:
		res = TEMP_FROM_REG(data->zone_abs[ix], 8);
		break;
	default:
		res = 0;
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}

	return sprintf(buf, "%d\n", res);
}

static ssize_t set_zone(struct device *dev, struct device_attribute *attr,
			const char *buf, size_t count)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	long val = simple_strtol(buf, NULL, 10);

	mutex_lock(&data->update_lock);
	switch (fn) {
	case SYS_ZONE_AUTO_POINT1_TEMP_HYST:
		/* Refresh the cache */
		data->zone_low[ix] = dme1737_read(data,
						  DME1737_REG_ZONE_LOW(ix));
		/* Modify the temp hyst value */
		data->zone_hyst[ix == 2] = TEMP_HYST_TO_REG(
					TEMP_FROM_REG(data->zone_low[ix], 8) -
					val, ix, dme1737_read(data,
					DME1737_REG_ZONE_HYST(ix == 2)));
		dme1737_write(data, DME1737_REG_ZONE_HYST(ix == 2),
			      data->zone_hyst[ix == 2]);
		break;
	case SYS_ZONE_AUTO_POINT1_TEMP:
		data->zone_low[ix] = TEMP_TO_REG(val);
		dme1737_write(data, DME1737_REG_ZONE_LOW(ix),
			      data->zone_low[ix]);
		break;
	case SYS_ZONE_AUTO_POINT2_TEMP:
		/* Refresh the cache */
		data->zone_low[ix] = dme1737_read(data,
						  DME1737_REG_ZONE_LOW(ix));
		/* Modify the temp range value (which is stored in the upper
		 * nibble of the pwm_freq register) */
		data->pwm_freq[ix] = TEMP_RANGE_TO_REG(val -
					TEMP_FROM_REG(data->zone_low[ix], 8),
					dme1737_read(data,
					DME1737_REG_PWM_FREQ(ix)));
		dme1737_write(data, DME1737_REG_PWM_FREQ(ix),
			      data->pwm_freq[ix]);
		break;
	case SYS_ZONE_AUTO_POINT3_TEMP:
		data->zone_abs[ix] = TEMP_TO_REG(val);
		dme1737_write(data, DME1737_REG_ZONE_ABS(ix),
			      data->zone_abs[ix]);
		break;
	default:
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}
	mutex_unlock(&data->update_lock);

	return count;
}

/* ---------------------------------------------------------------------
 * Fan sysfs attributes
 * ix = [0-5]
 * --------------------------------------------------------------------- */

#define SYS_FAN_INPUT	0
#define SYS_FAN_MIN	1
#define SYS_FAN_MAX	2
#define SYS_FAN_ALARM	3
#define SYS_FAN_TYPE	4

static ssize_t show_fan(struct device *dev, struct device_attribute *attr,
			char *buf)
{
	struct dme1737_data *data = dme1737_update_device(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	int res;

	switch (fn) {
	case SYS_FAN_INPUT:
		res = FAN_FROM_REG(data->fan[ix],
				   ix < 4 ? 0 :
				   FAN_TPC_FROM_REG(data->fan_opt[ix]));
		break;
	case SYS_FAN_MIN:
		res = FAN_FROM_REG(data->fan_min[ix],
				   ix < 4 ? 0 :
				   FAN_TPC_FROM_REG(data->fan_opt[ix]));
		break;
	case SYS_FAN_MAX:
		/* only valid for fan[5-6] */
		res = FAN_MAX_FROM_REG(data->fan_max[ix - 4]);
		break;
	case SYS_FAN_ALARM:
		res = (data->alarms >> DME1737_BIT_ALARM_FAN[ix]) & 0x01;
		break;
	case SYS_FAN_TYPE:
		/* only valid for fan[1-4] */
		res = FAN_TYPE_FROM_REG(data->fan_opt[ix]);
		break;
	default:
		res = 0;
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}

	return sprintf(buf, "%d\n", res);
}

static ssize_t set_fan(struct device *dev, struct device_attribute *attr,
		       const char *buf, size_t count)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	long val = simple_strtol(buf, NULL, 10);

	mutex_lock(&data->update_lock);
	switch (fn) {
	case SYS_FAN_MIN:
		if (ix < 4) {
			data->fan_min[ix] = FAN_TO_REG(val, 0);
		} else {
			/* Refresh the cache */
			data->fan_opt[ix] = dme1737_read(data,
						DME1737_REG_FAN_OPT(ix));
			/* Modify the fan min value */
			data->fan_min[ix] = FAN_TO_REG(val,
					FAN_TPC_FROM_REG(data->fan_opt[ix]));
		}
		dme1737_write(data, DME1737_REG_FAN_MIN(ix),
			      data->fan_min[ix] & 0xff);
		dme1737_write(data, DME1737_REG_FAN_MIN(ix) + 1,
			      data->fan_min[ix] >> 8);
		break;
	case SYS_FAN_MAX:
		/* Only valid for fan[5-6] */
		data->fan_max[ix - 4] = FAN_MAX_TO_REG(val);
		dme1737_write(data, DME1737_REG_FAN_MAX(ix),
			      data->fan_max[ix - 4]);
		break;
	case SYS_FAN_TYPE:
		/* Only valid for fan[1-4] */
		if (!(val == 1 || val == 2 || val == 4)) {
			count = -EINVAL;
			dev_warn(dev, "Fan type value %ld not "
				 "supported. Choose one of 1, 2, or 4.\n",
				 val);
			goto exit;
		}
		data->fan_opt[ix] = FAN_TYPE_TO_REG(val, dme1737_read(data,
					DME1737_REG_FAN_OPT(ix)));
		dme1737_write(data, DME1737_REG_FAN_OPT(ix),
			      data->fan_opt[ix]);
		break;
	default:
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}
exit:
	mutex_unlock(&data->update_lock);

	return count;
}

/* ---------------------------------------------------------------------
 * PWM sysfs attributes
 * ix = [0-4]
 * --------------------------------------------------------------------- */

#define SYS_PWM				0
#define SYS_PWM_FREQ			1
#define SYS_PWM_ENABLE			2
#define SYS_PWM_RAMP_RATE		3
#define SYS_PWM_AUTO_CHANNELS_ZONE	4
#define SYS_PWM_AUTO_PWM_MIN		5
#define SYS_PWM_AUTO_POINT1_PWM		6
#define SYS_PWM_AUTO_POINT2_PWM		7

static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
			char *buf)
{
	struct dme1737_data *data = dme1737_update_device(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	int res;

	switch (fn) {
	case SYS_PWM:
		if (PWM_EN_FROM_REG(data->pwm_config[ix]) == 0) {
			res = 255;
		} else {
			res = data->pwm[ix];
		}
		break;
	case SYS_PWM_FREQ:
		res = PWM_FREQ_FROM_REG(data->pwm_freq[ix]);
		break;
	case SYS_PWM_ENABLE:
		if (ix >= 3) {
			res = 1; /* pwm[5-6] hard-wired to manual mode */
		} else {
			res = PWM_EN_FROM_REG(data->pwm_config[ix]);
		}
		break;
	case SYS_PWM_RAMP_RATE:
		/* Only valid for pwm[1-3] */
		res = PWM_RR_FROM_REG(data->pwm_rr[ix > 0], ix);
		break;
	case SYS_PWM_AUTO_CHANNELS_ZONE:
		/* Only valid for pwm[1-3] */
		if (PWM_EN_FROM_REG(data->pwm_config[ix]) == 2) {
			res = PWM_ACZ_FROM_REG(data->pwm_config[ix]);
		} else {
			res = data->pwm_acz[ix];
		}
		break;
	case SYS_PWM_AUTO_PWM_MIN:
		/* Only valid for pwm[1-3] */
		if (PWM_OFF_FROM_REG(data->pwm_rr[0], ix)) {
			res = data->pwm_min[ix];
		} else {
			res = 0;
		}
		break;
	case SYS_PWM_AUTO_POINT1_PWM:
		/* Only valid for pwm[1-3] */
		res = data->pwm_min[ix];
		break;
	case SYS_PWM_AUTO_POINT2_PWM:
		/* Only valid for pwm[1-3] */
		res = 255; /* hard-wired */
		break;
	default:
		res = 0;
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}

	return sprintf(buf, "%d\n", res);
}

static struct attribute *dme1737_pwm_chmod_attr[];
static void dme1737_chmod_file(struct device*, struct attribute*, mode_t);

static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
		       const char *buf, size_t count)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	struct sensor_device_attribute_2
		*sensor_attr_2 = to_sensor_dev_attr_2(attr);
	int ix = sensor_attr_2->index;
	int fn = sensor_attr_2->nr;
	long val = simple_strtol(buf, NULL, 10);

	mutex_lock(&data->update_lock);
	switch (fn) {
	case SYS_PWM:
		data->pwm[ix] = SENSORS_LIMIT(val, 0, 255);
		dme1737_write(data, DME1737_REG_PWM(ix), data->pwm[ix]);
		break;
	case SYS_PWM_FREQ:
		data->pwm_freq[ix] = PWM_FREQ_TO_REG(val, dme1737_read(data,
						DME1737_REG_PWM_FREQ(ix)));
		dme1737_write(data, DME1737_REG_PWM_FREQ(ix),
			      data->pwm_freq[ix]);
		break;
	case SYS_PWM_ENABLE:
		/* Only valid for pwm[1-3] */
		if (val < 0 || val > 2) {
			count = -EINVAL;
			dev_warn(dev, "PWM enable %ld not "
				 "supported. Choose one of 0, 1, or 2.\n",
				 val);
			goto exit;
		}
		/* Refresh the cache */
		data->pwm_config[ix] = dme1737_read(data,
						DME1737_REG_PWM_CONFIG(ix));
		if (val == PWM_EN_FROM_REG(data->pwm_config[ix])) {
			/* Bail out if no change */
			goto exit;
		}
		/* Do some housekeeping if we are currently in auto mode */
		if (PWM_EN_FROM_REG(data->pwm_config[ix]) == 2) {
			/* Save the current zone channel assignment */
			data->pwm_acz[ix] = PWM_ACZ_FROM_REG(
							data->pwm_config[ix]);
			/* Save the current ramp rate state and disable it */
			data->pwm_rr[ix > 0] = dme1737_read(data,
						DME1737_REG_PWM_RR(ix > 0));
			data->pwm_rr_en &= ~(1 << ix);
			if (PWM_RR_EN_FROM_REG(data->pwm_rr[ix > 0], ix)) {
				data->pwm_rr_en |= (1 << ix);
				data->pwm_rr[ix > 0] = PWM_RR_EN_TO_REG(0, ix,
							data->pwm_rr[ix > 0]);
				dme1737_write(data,
					      DME1737_REG_PWM_RR(ix > 0),
					      data->pwm_rr[ix > 0]);
			}
		}
		/* Set the new PWM mode */
		switch (val) {
		case 0:
			/* Change permissions of pwm[ix] to read-only */
			dme1737_chmod_file(dev, dme1737_pwm_chmod_attr[ix],
					   S_IRUGO);
			/* Turn fan fully on */
			data->pwm_config[ix] = PWM_EN_TO_REG(0,
							data->pwm_config[ix]);
			dme1737_write(data, DME1737_REG_PWM_CONFIG(ix),
				      data->pwm_config[ix]);
			break;
		case 1:
			/* Turn on manual mode */
			data->pwm_config[ix] = PWM_EN_TO_REG(1,
							data->pwm_config[ix]);
			dme1737_write(data, DME1737_REG_PWM_CONFIG(ix),
				      data->pwm_config[ix]);
			/* Change permissions of pwm[ix] to read-writeable */
			dme1737_chmod_file(dev, dme1737_pwm_chmod_attr[ix],
					   S_IRUGO | S_IWUSR);
			break;
		case 2:
			/* Change permissions of pwm[ix] to read-only */
			dme1737_chmod_file(dev, dme1737_pwm_chmod_attr[ix],
					   S_IRUGO);
			/* Turn on auto mode using the saved zone channel
			 * assignment */
			data->pwm_config[ix] = PWM_ACZ_TO_REG(
							data->pwm_acz[ix],
							data->pwm_config[ix]);
			dme1737_write(data, DME1737_REG_PWM_CONFIG(ix),
				      data->pwm_config[ix]);
			/* Enable PWM ramp rate if previously enabled */
			if (data->pwm_rr_en & (1 << ix)) {
				data->pwm_rr[ix > 0] = PWM_RR_EN_TO_REG(1, ix,
						dme1737_read(data,
						DME1737_REG_PWM_RR(ix > 0)));
				dme1737_write(data,
					      DME1737_REG_PWM_RR(ix > 0),
					      data->pwm_rr[ix > 0]);
			}
			break;
		}
		break;
	case SYS_PWM_RAMP_RATE:
		/* Only valid for pwm[1-3] */
		/* Refresh the cache */
		data->pwm_config[ix] = dme1737_read(data,
						DME1737_REG_PWM_CONFIG(ix));
		data->pwm_rr[ix > 0] = dme1737_read(data,
						DME1737_REG_PWM_RR(ix > 0));
		/* Set the ramp rate value */
		if (val > 0) {
			data->pwm_rr[ix > 0] = PWM_RR_TO_REG(val, ix,
							data->pwm_rr[ix > 0]);
		}
		/* Enable/disable the feature only if the associated PWM
		 * output is in automatic mode. */
		if (PWM_EN_FROM_REG(data->pwm_config[ix]) == 2) {
			data->pwm_rr[ix > 0] = PWM_RR_EN_TO_REG(val > 0, ix,
							data->pwm_rr[ix > 0]);
		}
		dme1737_write(data, DME1737_REG_PWM_RR(ix > 0),
			      data->pwm_rr[ix > 0]);
		break;
	case SYS_PWM_AUTO_CHANNELS_ZONE:
		/* Only valid for pwm[1-3] */
		if (!(val == 1 || val == 2 || val == 4 ||
		      val == 6 || val == 7)) {
			count = -EINVAL;
			dev_warn(dev, "PWM auto channels zone %ld "
				 "not supported. Choose one of 1, 2, 4, 6, "
				 "or 7.\n", val);
			goto exit;
		}
		/* Refresh the cache */
		data->pwm_config[ix] = dme1737_read(data,
						DME1737_REG_PWM_CONFIG(ix));
		if (PWM_EN_FROM_REG(data->pwm_config[ix]) == 2) {
			/* PWM is already in auto mode so update the temp
			 * channel assignment */
			data->pwm_config[ix] = PWM_ACZ_TO_REG(val,
						data->pwm_config[ix]);
			dme1737_write(data, DME1737_REG_PWM_CONFIG(ix),
				      data->pwm_config[ix]);
		} else {
			/* PWM is not in auto mode so we save the temp
			 * channel assignment for later use */
			data->pwm_acz[ix] = val;
		}
		break;
	case SYS_PWM_AUTO_PWM_MIN:
		/* Only valid for pwm[1-3] */
		/* Refresh the cache */
		data->pwm_min[ix] = dme1737_read(data,
						DME1737_REG_PWM_MIN(ix));
		/* There are only 2 values supported for the auto_pwm_min
		 * value: 0 or auto_point1_pwm. So if the temperature drops
		 * below the auto_point1_temp_hyst value, the fan either turns
		 * off or runs at auto_point1_pwm duty-cycle. */
		if (val > ((data->pwm_min[ix] + 1) / 2)) {
			data->pwm_rr[0] = PWM_OFF_TO_REG(1, ix,
						dme1737_read(data,
						DME1737_REG_PWM_RR(0)));
		} else {
			data->pwm_rr[0] = PWM_OFF_TO_REG(0, ix,
						dme1737_read(data,
						DME1737_REG_PWM_RR(0)));
		}
		dme1737_write(data, DME1737_REG_PWM_RR(0),
			      data->pwm_rr[0]);
		break;
	case SYS_PWM_AUTO_POINT1_PWM:
		/* Only valid for pwm[1-3] */
		data->pwm_min[ix] = SENSORS_LIMIT(val, 0, 255);
		dme1737_write(data, DME1737_REG_PWM_MIN(ix),
			      data->pwm_min[ix]);
		break;
	default:
		dev_dbg(dev, "Unknown function %d.\n", fn);
	}
exit:
	mutex_unlock(&data->update_lock);

	return count;
}

/* ---------------------------------------------------------------------
 * Miscellaneous sysfs attributes
 * --------------------------------------------------------------------- */

static ssize_t show_vrm(struct device *dev, struct device_attribute *attr,
			char *buf)
{
	struct i2c_client *client = to_i2c_client(dev);
	struct dme1737_data *data = i2c_get_clientdata(client);

	return sprintf(buf, "%d\n", data->vrm);
}

static ssize_t set_vrm(struct device *dev, struct device_attribute *attr,
		       const char *buf, size_t count)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	long val = simple_strtol(buf, NULL, 10);

	data->vrm = val;
	return count;
}

static ssize_t show_vid(struct device *dev, struct device_attribute *attr,
			char *buf)
{
	struct dme1737_data *data = dme1737_update_device(dev);

	return sprintf(buf, "%d\n", vid_from_reg(data->vid, data->vrm));
}

static ssize_t show_name(struct device *dev, struct device_attribute *attr,
			 char *buf)
{
	struct dme1737_data *data = dev_get_drvdata(dev);

	return sprintf(buf, "%s\n", data->name);
}

/* ---------------------------------------------------------------------
 * Sysfs device attribute defines and structs
 * --------------------------------------------------------------------- */

/* Voltages 0-7 */

#define SENSOR_DEVICE_ATTR_IN(ix) \
static SENSOR_DEVICE_ATTR_2(in##ix##_input, S_IRUGO, \
	show_in, NULL, SYS_IN_INPUT, ix); \
static SENSOR_DEVICE_ATTR_2(in##ix##_min, S_IRUGO | S_IWUSR, \
	show_in, set_in, SYS_IN_MIN, ix); \
static SENSOR_DEVICE_ATTR_2(in##ix##_max, S_IRUGO | S_IWUSR, \
	show_in, set_in, SYS_IN_MAX, ix); \
static SENSOR_DEVICE_ATTR_2(in##ix##_alarm, S_IRUGO, \
	show_in, NULL, SYS_IN_ALARM, ix)

SENSOR_DEVICE_ATTR_IN(0);
SENSOR_DEVICE_ATTR_IN(1);
SENSOR_DEVICE_ATTR_IN(2);
SENSOR_DEVICE_ATTR_IN(3);
SENSOR_DEVICE_ATTR_IN(4);
SENSOR_DEVICE_ATTR_IN(5);
SENSOR_DEVICE_ATTR_IN(6);
SENSOR_DEVICE_ATTR_IN(7);

/* Temperatures 1-3 */

#define SENSOR_DEVICE_ATTR_TEMP(ix) \
static SENSOR_DEVICE_ATTR_2(temp##ix##_input, S_IRUGO, \
	show_temp, NULL, SYS_TEMP_INPUT, ix-1); \
static SENSOR_DEVICE_ATTR_2(temp##ix##_min, S_IRUGO | S_IWUSR, \
	show_temp, set_temp, SYS_TEMP_MIN, ix-1); \
static SENSOR_DEVICE_ATTR_2(temp##ix##_max, S_IRUGO | S_IWUSR, \
	show_temp, set_temp, SYS_TEMP_MAX, ix-1); \
static SENSOR_DEVICE_ATTR_2(temp##ix##_offset, S_IRUGO, \
	show_temp, set_temp, SYS_TEMP_OFFSET, ix-1); \
static SENSOR_DEVICE_ATTR_2(temp##ix##_alarm, S_IRUGO, \
	show_temp, NULL, SYS_TEMP_ALARM, ix-1); \
static SENSOR_DEVICE_ATTR_2(temp##ix##_fault, S_IRUGO, \
	show_temp, NULL, SYS_TEMP_FAULT, ix-1)

SENSOR_DEVICE_ATTR_TEMP(1);
SENSOR_DEVICE_ATTR_TEMP(2);
SENSOR_DEVICE_ATTR_TEMP(3);

/* Zones 1-3 */

#define SENSOR_DEVICE_ATTR_ZONE(ix) \
static SENSOR_DEVICE_ATTR_2(zone##ix##_auto_channels_temp, S_IRUGO, \
	show_zone, NULL, SYS_ZONE_AUTO_CHANNELS_TEMP, ix-1); \
static SENSOR_DEVICE_ATTR_2(zone##ix##_auto_point1_temp_hyst, S_IRUGO, \
	show_zone, set_zone, SYS_ZONE_AUTO_POINT1_TEMP_HYST, ix-1); \
static SENSOR_DEVICE_ATTR_2(zone##ix##_auto_point1_temp, S_IRUGO, \
	show_zone, set_zone, SYS_ZONE_AUTO_POINT1_TEMP, ix-1); \
static SENSOR_DEVICE_ATTR_2(zone##ix##_auto_point2_temp, S_IRUGO, \
	show_zone, set_zone, SYS_ZONE_AUTO_POINT2_TEMP, ix-1); \
static SENSOR_DEVICE_ATTR_2(zone##ix##_auto_point3_temp, S_IRUGO, \
	show_zone, set_zone, SYS_ZONE_AUTO_POINT3_TEMP, ix-1)

SENSOR_DEVICE_ATTR_ZONE(1);
SENSOR_DEVICE_ATTR_ZONE(2);
SENSOR_DEVICE_ATTR_ZONE(3);

/* Fans 1-4 */

#define SENSOR_DEVICE_ATTR_FAN_1TO4(ix) \
static SENSOR_DEVICE_ATTR_2(fan##ix##_input, S_IRUGO, \
	show_fan, NULL, SYS_FAN_INPUT, ix-1); \
static SENSOR_DEVICE_ATTR_2(fan##ix##_min, S_IRUGO | S_IWUSR, \
	show_fan, set_fan, SYS_FAN_MIN, ix-1); \
static SENSOR_DEVICE_ATTR_2(fan##ix##_alarm, S_IRUGO, \
	show_fan, NULL, SYS_FAN_ALARM, ix-1); \
static SENSOR_DEVICE_ATTR_2(fan##ix##_type, S_IRUGO | S_IWUSR, \
	show_fan, set_fan, SYS_FAN_TYPE, ix-1)

SENSOR_DEVICE_ATTR_FAN_1TO4(1);
SENSOR_DEVICE_ATTR_FAN_1TO4(2);
SENSOR_DEVICE_ATTR_FAN_1TO4(3);
SENSOR_DEVICE_ATTR_FAN_1TO4(4);

/* Fans 5-6 */

#define SENSOR_DEVICE_ATTR_FAN_5TO6(ix) \
static SENSOR_DEVICE_ATTR_2(fan##ix##_input, S_IRUGO, \
	show_fan, NULL, SYS_FAN_INPUT, ix-1); \
static SENSOR_DEVICE_ATTR_2(fan##ix##_min, S_IRUGO | S_IWUSR, \
	show_fan, set_fan, SYS_FAN_MIN, ix-1); \
static SENSOR_DEVICE_ATTR_2(fan##ix##_alarm, S_IRUGO, \
	show_fan, NULL, SYS_FAN_ALARM, ix-1); \
static SENSOR_DEVICE_ATTR_2(fan##ix##_max, S_IRUGO | S_IWUSR, \
	show_fan, set_fan, SYS_FAN_MAX, ix-1)

SENSOR_DEVICE_ATTR_FAN_5TO6(5);
SENSOR_DEVICE_ATTR_FAN_5TO6(6);

/* PWMs 1-3 */

#define SENSOR_DEVICE_ATTR_PWM_1TO3(ix) \
static SENSOR_DEVICE_ATTR_2(pwm##ix, S_IRUGO, \
	show_pwm, set_pwm, SYS_PWM, ix-1); \
static SENSOR_DEVICE_ATTR_2(pwm##ix##_freq, S_IRUGO, \
	show_pwm, set_pwm, SYS_PWM_FREQ, ix-1); \
static SENSOR_DEVICE_ATTR_2(pwm##ix##_enable, S_IRUGO, \
	show_pwm, set_pwm, SYS_PWM_ENABLE, ix-1); \
static SENSOR_DEVICE_ATTR_2(pwm##ix##_ramp_rate, S_IRUGO, \
	show_pwm, set_pwm, SYS_PWM_RAMP_RATE, ix-1); \
static SENSOR_DEVICE_ATTR_2(pwm##ix##_auto_channels_zone, S_IRUGO, \
	show_pwm, set_pwm, SYS_PWM_AUTO_CHANNELS_ZONE, ix-1); \
static SENSOR_DEVICE_ATTR_2(pwm##ix##_auto_pwm_min, S_IRUGO, \
	show_pwm, set_pwm, SYS_PWM_AUTO_PWM_MIN, ix-1); \
static SENSOR_DEVICE_ATTR_2(pwm##ix##_auto_point1_pwm, S_IRUGO, \
	show_pwm, set_pwm, SYS_PWM_AUTO_POINT1_PWM, ix-1); \
static SENSOR_DEVICE_ATTR_2(pwm##ix##_auto_point2_pwm, S_IRUGO, \
	show_pwm, NULL, SYS_PWM_AUTO_POINT2_PWM, ix-1)

SENSOR_DEVICE_ATTR_PWM_1TO3(1);
SENSOR_DEVICE_ATTR_PWM_1TO3(2);
SENSOR_DEVICE_ATTR_PWM_1TO3(3);

/* PWMs 5-6 */

#define SENSOR_DEVICE_ATTR_PWM_5TO6(ix) \
static SENSOR_DEVICE_ATTR_2(pwm##ix, S_IRUGO, \
	show_pwm, set_pwm, SYS_PWM, ix-1); \
static SENSOR_DEVICE_ATTR_2(pwm##ix##_freq, S_IRUGO, \
	show_pwm, set_pwm, SYS_PWM_FREQ, ix-1); \
static SENSOR_DEVICE_ATTR_2(pwm##ix##_enable, S_IRUGO, \
	show_pwm, NULL, SYS_PWM_ENABLE, ix-1)

SENSOR_DEVICE_ATTR_PWM_5TO6(5);
SENSOR_DEVICE_ATTR_PWM_5TO6(6);

/* Misc */

static DEVICE_ATTR(vrm, S_IRUGO | S_IWUSR, show_vrm, set_vrm);
static DEVICE_ATTR(cpu0_vid, S_IRUGO, show_vid, NULL);
static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);   /* for ISA devices */

/* This struct holds all the attributes that are always present and need to be
 * created unconditionally. The attributes that need modification of their
 * permissions are created read-only and write permissions are added or removed
 * on the fly when required */
static struct attribute *dme1737_attr[] ={
	/* Voltages */
	&sensor_dev_attr_in0_input.dev_attr.attr,
	&sensor_dev_attr_in0_min.dev_attr.attr,
	&sensor_dev_attr_in0_max.dev_attr.attr,
	&sensor_dev_attr_in0_alarm.dev_attr.attr,
	&sensor_dev_attr_in1_input.dev_attr.attr,
	&sensor_dev_attr_in1_min.dev_attr.attr,
	&sensor_dev_attr_in1_max.dev_attr.attr,
	&sensor_dev_attr_in1_alarm.dev_attr.attr,
	&sensor_dev_attr_in2_input.dev_attr.attr,
	&sensor_dev_attr_in2_min.dev_attr.attr,
	&sensor_dev_attr_in2_max.dev_attr.attr,
	&sensor_dev_attr_in2_alarm.dev_attr.attr,
	&sensor_dev_attr_in3_input.dev_attr.attr,
	&sensor_dev_attr_in3_min.dev_attr.attr,
	&sensor_dev_attr_in3_max.dev_attr.attr,
	&sensor_dev_attr_in3_alarm.dev_attr.attr,
	&sensor_dev_attr_in4_input.dev_attr.attr,
	&sensor_dev_attr_in4_min.dev_attr.attr,
	&sensor_dev_attr_in4_max.dev_attr.attr,
	&sensor_dev_attr_in4_alarm.dev_attr.attr,
	&sensor_dev_attr_in5_input.dev_attr.attr,
	&sensor_dev_attr_in5_min.dev_attr.attr,
	&sensor_dev_attr_in5_max.dev_attr.attr,
	&sensor_dev_attr_in5_alarm.dev_attr.attr,
	&sensor_dev_attr_in6_input.dev_attr.attr,
	&sensor_dev_attr_in6_min.dev_attr.attr,
	&sensor_dev_attr_in6_max.dev_attr.attr,
	&sensor_dev_attr_in6_alarm.dev_attr.attr,
	/* Temperatures */
	&sensor_dev_attr_temp1_input.dev_attr.attr,
	&sensor_dev_attr_temp1_min.dev_attr.attr,
	&sensor_dev_attr_temp1_max.dev_attr.attr,
	&sensor_dev_attr_temp1_alarm.dev_attr.attr,
	&sensor_dev_attr_temp1_fault.dev_attr.attr,
	&sensor_dev_attr_temp2_input.dev_attr.attr,
	&sensor_dev_attr_temp2_min.dev_attr.attr,
	&sensor_dev_attr_temp2_max.dev_attr.attr,
	&sensor_dev_attr_temp2_alarm.dev_attr.attr,
	&sensor_dev_attr_temp2_fault.dev_attr.attr,
	&sensor_dev_attr_temp3_input.dev_attr.attr,
	&sensor_dev_attr_temp3_min.dev_attr.attr,
	&sensor_dev_attr_temp3_max.dev_attr.attr,
	&sensor_dev_attr_temp3_alarm.dev_attr.attr,
	&sensor_dev_attr_temp3_fault.dev_attr.attr,
	/* Zones */
	&sensor_dev_attr_zone1_auto_point1_temp.dev_attr.attr,
	&sensor_dev_attr_zone1_auto_point2_temp.dev_attr.attr,
	&sensor_dev_attr_zone1_auto_point3_temp.dev_attr.attr,
	&sensor_dev_attr_zone1_auto_channels_temp.dev_attr.attr,
	&sensor_dev_attr_zone2_auto_point1_temp.dev_attr.attr,
	&sensor_dev_attr_zone2_auto_point2_temp.dev_attr.attr,
	&sensor_dev_attr_zone2_auto_point3_temp.dev_attr.attr,
	&sensor_dev_attr_zone2_auto_channels_temp.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_group = {
	.attrs = dme1737_attr,
};

/* The following struct holds temp offset attributes, which are not available
 * in all chips. The following chips support them:
 * DME1737, SCH311x */
static struct attribute *dme1737_temp_offset_attr[] = {
	&sensor_dev_attr_temp1_offset.dev_attr.attr,
	&sensor_dev_attr_temp2_offset.dev_attr.attr,
	&sensor_dev_attr_temp3_offset.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_temp_offset_group = {
	.attrs = dme1737_temp_offset_attr,
};

/* The following struct holds VID related attributes, which are not available
 * in all chips. The following chips support them:
 * DME1737 */
static struct attribute *dme1737_vid_attr[] = {
	&dev_attr_vrm.attr,
	&dev_attr_cpu0_vid.attr,
	NULL
};

static const struct attribute_group dme1737_vid_group = {
	.attrs = dme1737_vid_attr,
};

/* The following struct holds temp zone 3 related attributes, which are not
 * available in all chips. The following chips support them:
 * DME1737, SCH311x, SCH5027 */
static struct attribute *dme1737_zone3_attr[] = {
	&sensor_dev_attr_zone3_auto_point1_temp.dev_attr.attr,
	&sensor_dev_attr_zone3_auto_point2_temp.dev_attr.attr,
	&sensor_dev_attr_zone3_auto_point3_temp.dev_attr.attr,
	&sensor_dev_attr_zone3_auto_channels_temp.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_zone3_group = {
	.attrs = dme1737_zone3_attr,
};

/* The following struct holds temp zone hysteresis related attributes, which
 * are not available in all chips. The following chips support them:
 * DME1737, SCH311x */
static struct attribute *dme1737_zone_hyst_attr[] = {
	&sensor_dev_attr_zone1_auto_point1_temp_hyst.dev_attr.attr,
	&sensor_dev_attr_zone2_auto_point1_temp_hyst.dev_attr.attr,
	&sensor_dev_attr_zone3_auto_point1_temp_hyst.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_zone_hyst_group = {
	.attrs = dme1737_zone_hyst_attr,
};

/* The following struct holds voltage in7 related attributes, which
 * are not available in all chips. The following chips support them:
 * SCH5127 */
static struct attribute *dme1737_in7_attr[] = {
	&sensor_dev_attr_in7_input.dev_attr.attr,
	&sensor_dev_attr_in7_min.dev_attr.attr,
	&sensor_dev_attr_in7_max.dev_attr.attr,
	&sensor_dev_attr_in7_alarm.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_in7_group = {
	.attrs = dme1737_in7_attr,
};

/* The following structs hold the PWM attributes, some of which are optional.
 * Their creation depends on the chip configuration which is determined during
 * module load. */
static struct attribute *dme1737_pwm1_attr[] = {
	&sensor_dev_attr_pwm1.dev_attr.attr,
	&sensor_dev_attr_pwm1_freq.dev_attr.attr,
	&sensor_dev_attr_pwm1_enable.dev_attr.attr,
	&sensor_dev_attr_pwm1_ramp_rate.dev_attr.attr,
	&sensor_dev_attr_pwm1_auto_channels_zone.dev_attr.attr,
	&sensor_dev_attr_pwm1_auto_point1_pwm.dev_attr.attr,
	&sensor_dev_attr_pwm1_auto_point2_pwm.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_pwm2_attr[] = {
	&sensor_dev_attr_pwm2.dev_attr.attr,
	&sensor_dev_attr_pwm2_freq.dev_attr.attr,
	&sensor_dev_attr_pwm2_enable.dev_attr.attr,
	&sensor_dev_attr_pwm2_ramp_rate.dev_attr.attr,
	&sensor_dev_attr_pwm2_auto_channels_zone.dev_attr.attr,
	&sensor_dev_attr_pwm2_auto_point1_pwm.dev_attr.attr,
	&sensor_dev_attr_pwm2_auto_point2_pwm.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_pwm3_attr[] = {
	&sensor_dev_attr_pwm3.dev_attr.attr,
	&sensor_dev_attr_pwm3_freq.dev_attr.attr,
	&sensor_dev_attr_pwm3_enable.dev_attr.attr,
	&sensor_dev_attr_pwm3_ramp_rate.dev_attr.attr,
	&sensor_dev_attr_pwm3_auto_channels_zone.dev_attr.attr,
	&sensor_dev_attr_pwm3_auto_point1_pwm.dev_attr.attr,
	&sensor_dev_attr_pwm3_auto_point2_pwm.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_pwm5_attr[] = {
	&sensor_dev_attr_pwm5.dev_attr.attr,
	&sensor_dev_attr_pwm5_freq.dev_attr.attr,
	&sensor_dev_attr_pwm5_enable.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_pwm6_attr[] = {
	&sensor_dev_attr_pwm6.dev_attr.attr,
	&sensor_dev_attr_pwm6_freq.dev_attr.attr,
	&sensor_dev_attr_pwm6_enable.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_pwm_group[] = {
	{ .attrs = dme1737_pwm1_attr },
	{ .attrs = dme1737_pwm2_attr },
	{ .attrs = dme1737_pwm3_attr },
	{ .attrs = NULL },
	{ .attrs = dme1737_pwm5_attr },
	{ .attrs = dme1737_pwm6_attr },
};

/* The following struct holds auto PWM min attributes, which are not available
 * in all chips. Their creation depends on the chip type which is determined
 * during module load. */
static struct attribute *dme1737_auto_pwm_min_attr[] = {
	&sensor_dev_attr_pwm1_auto_pwm_min.dev_attr.attr,
	&sensor_dev_attr_pwm2_auto_pwm_min.dev_attr.attr,
	&sensor_dev_attr_pwm3_auto_pwm_min.dev_attr.attr,
};

/* The following structs hold the fan attributes, some of which are optional.
 * Their creation depends on the chip configuration which is determined during
 * module load. */
static struct attribute *dme1737_fan1_attr[] = {
	&sensor_dev_attr_fan1_input.dev_attr.attr,
	&sensor_dev_attr_fan1_min.dev_attr.attr,
	&sensor_dev_attr_fan1_alarm.dev_attr.attr,
	&sensor_dev_attr_fan1_type.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_fan2_attr[] = {
	&sensor_dev_attr_fan2_input.dev_attr.attr,
	&sensor_dev_attr_fan2_min.dev_attr.attr,
	&sensor_dev_attr_fan2_alarm.dev_attr.attr,
	&sensor_dev_attr_fan2_type.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_fan3_attr[] = {
	&sensor_dev_attr_fan3_input.dev_attr.attr,
	&sensor_dev_attr_fan3_min.dev_attr.attr,
	&sensor_dev_attr_fan3_alarm.dev_attr.attr,
	&sensor_dev_attr_fan3_type.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_fan4_attr[] = {
	&sensor_dev_attr_fan4_input.dev_attr.attr,
	&sensor_dev_attr_fan4_min.dev_attr.attr,
	&sensor_dev_attr_fan4_alarm.dev_attr.attr,
	&sensor_dev_attr_fan4_type.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_fan5_attr[] = {
	&sensor_dev_attr_fan5_input.dev_attr.attr,
	&sensor_dev_attr_fan5_min.dev_attr.attr,
	&sensor_dev_attr_fan5_alarm.dev_attr.attr,
	&sensor_dev_attr_fan5_max.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_fan6_attr[] = {
	&sensor_dev_attr_fan6_input.dev_attr.attr,
	&sensor_dev_attr_fan6_min.dev_attr.attr,
	&sensor_dev_attr_fan6_alarm.dev_attr.attr,
	&sensor_dev_attr_fan6_max.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_fan_group[] = {
	{ .attrs = dme1737_fan1_attr },
	{ .attrs = dme1737_fan2_attr },
	{ .attrs = dme1737_fan3_attr },
	{ .attrs = dme1737_fan4_attr },
	{ .attrs = dme1737_fan5_attr },
	{ .attrs = dme1737_fan6_attr },
};

/* The permissions of the following zone attributes are changed to read-
 * writeable if the chip is *not* locked. Otherwise they stay read-only. */
static struct attribute *dme1737_zone_chmod_attr[] = {
	&sensor_dev_attr_zone1_auto_point1_temp.dev_attr.attr,
	&sensor_dev_attr_zone1_auto_point2_temp.dev_attr.attr,
	&sensor_dev_attr_zone1_auto_point3_temp.dev_attr.attr,
	&sensor_dev_attr_zone2_auto_point1_temp.dev_attr.attr,
	&sensor_dev_attr_zone2_auto_point2_temp.dev_attr.attr,
	&sensor_dev_attr_zone2_auto_point3_temp.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_zone_chmod_group = {
	.attrs = dme1737_zone_chmod_attr,
};


/* The permissions of the following zone 3 attributes are changed to read-
 * writeable if the chip is *not* locked. Otherwise they stay read-only. */
static struct attribute *dme1737_zone3_chmod_attr[] = {
	&sensor_dev_attr_zone3_auto_point1_temp.dev_attr.attr,
	&sensor_dev_attr_zone3_auto_point2_temp.dev_attr.attr,
	&sensor_dev_attr_zone3_auto_point3_temp.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_zone3_chmod_group = {
	.attrs = dme1737_zone3_chmod_attr,
};

/* The permissions of the following PWM attributes are changed to read-
 * writeable if the chip is *not* locked and the respective PWM is available.
 * Otherwise they stay read-only. */
static struct attribute *dme1737_pwm1_chmod_attr[] = {
	&sensor_dev_attr_pwm1_freq.dev_attr.attr,
	&sensor_dev_attr_pwm1_enable.dev_attr.attr,
	&sensor_dev_attr_pwm1_ramp_rate.dev_attr.attr,
	&sensor_dev_attr_pwm1_auto_channels_zone.dev_attr.attr,
	&sensor_dev_attr_pwm1_auto_point1_pwm.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_pwm2_chmod_attr[] = {
	&sensor_dev_attr_pwm2_freq.dev_attr.attr,
	&sensor_dev_attr_pwm2_enable.dev_attr.attr,
	&sensor_dev_attr_pwm2_ramp_rate.dev_attr.attr,
	&sensor_dev_attr_pwm2_auto_channels_zone.dev_attr.attr,
	&sensor_dev_attr_pwm2_auto_point1_pwm.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_pwm3_chmod_attr[] = {
	&sensor_dev_attr_pwm3_freq.dev_attr.attr,
	&sensor_dev_attr_pwm3_enable.dev_attr.attr,
	&sensor_dev_attr_pwm3_ramp_rate.dev_attr.attr,
	&sensor_dev_attr_pwm3_auto_channels_zone.dev_attr.attr,
	&sensor_dev_attr_pwm3_auto_point1_pwm.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_pwm5_chmod_attr[] = {
	&sensor_dev_attr_pwm5.dev_attr.attr,
	&sensor_dev_attr_pwm5_freq.dev_attr.attr,
	NULL
};
static struct attribute *dme1737_pwm6_chmod_attr[] = {
	&sensor_dev_attr_pwm6.dev_attr.attr,
	&sensor_dev_attr_pwm6_freq.dev_attr.attr,
	NULL
};

static const struct attribute_group dme1737_pwm_chmod_group[] = {
	{ .attrs = dme1737_pwm1_chmod_attr },
	{ .attrs = dme1737_pwm2_chmod_attr },
	{ .attrs = dme1737_pwm3_chmod_attr },
	{ .attrs = NULL },
	{ .attrs = dme1737_pwm5_chmod_attr },
	{ .attrs = dme1737_pwm6_chmod_attr },
};

/* Pwm[1-3] are read-writeable if the associated pwm is in manual mode and the
 * chip is not locked. Otherwise they are read-only. */
static struct attribute *dme1737_pwm_chmod_attr[] = {
	&sensor_dev_attr_pwm1.dev_attr.attr,
	&sensor_dev_attr_pwm2.dev_attr.attr,
	&sensor_dev_attr_pwm3.dev_attr.attr,
};

/* ---------------------------------------------------------------------
 * Super-IO functions
 * --------------------------------------------------------------------- */

static inline void dme1737_sio_enter(int sio_cip)
{
	outb(0x55, sio_cip);
}

static inline void dme1737_sio_exit(int sio_cip)
{
	outb(0xaa, sio_cip);
}

static inline int dme1737_sio_inb(int sio_cip, int reg)
{
	outb(reg, sio_cip);
	return inb(sio_cip + 1);
}

static inline void dme1737_sio_outb(int sio_cip, int reg, int val)
{
	outb(reg, sio_cip);
	outb(val, sio_cip + 1);
}

/* ---------------------------------------------------------------------
 * Device initialization
 * --------------------------------------------------------------------- */

static int dme1737_i2c_get_features(int, struct dme1737_data*);

static void dme1737_chmod_file(struct device *dev,
			       struct attribute *attr, mode_t mode)
{
	if (sysfs_chmod_file(&dev->kobj, attr, mode)) {
		dev_warn(dev, "Failed to change permissions of %s.\n",
			 attr->name);
	}
}

static void dme1737_chmod_group(struct device *dev,
				const struct attribute_group *group,
				mode_t mode)
{
	struct attribute **attr;

	for (attr = group->attrs; *attr; attr++) {
		dme1737_chmod_file(dev, *attr, mode);
	}
}

static void dme1737_remove_files(struct device *dev)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	int ix;

	for (ix = 0; ix < ARRAY_SIZE(dme1737_fan_group); ix++) {
		if (data->has_features & HAS_FAN(ix)) {
			sysfs_remove_group(&dev->kobj,
					   &dme1737_fan_group[ix]);
		}
	}

	for (ix = 0; ix < ARRAY_SIZE(dme1737_pwm_group); ix++) {
		if (data->has_features & HAS_PWM(ix)) {
			sysfs_remove_group(&dev->kobj,
					   &dme1737_pwm_group[ix]);
			if ((data->has_features & HAS_PWM_MIN) && ix < 3) {
				sysfs_remove_file(&dev->kobj,
						dme1737_auto_pwm_min_attr[ix]);
			}
		}
	}

	if (data->has_features & HAS_TEMP_OFFSET) {
		sysfs_remove_group(&dev->kobj, &dme1737_temp_offset_group);
	}
	if (data->has_features & HAS_VID) {
		sysfs_remove_group(&dev->kobj, &dme1737_vid_group);
	}
	if (data->has_features & HAS_ZONE3) {
		sysfs_remove_group(&dev->kobj, &dme1737_zone3_group);
	}
	if (data->has_features & HAS_ZONE_HYST) {
		sysfs_remove_group(&dev->kobj, &dme1737_zone_hyst_group);
	}
	if (data->has_features & HAS_IN7) {
		sysfs_remove_group(&dev->kobj, &dme1737_in7_group);
	}
	sysfs_remove_group(&dev->kobj, &dme1737_group);

	if (!data->client) {
		sysfs_remove_file(&dev->kobj, &dev_attr_name.attr);
	}
}

static int dme1737_create_files(struct device *dev)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	int err, ix;

	/* Create a name attribute for ISA devices */
	if (!data->client &&
	    (err = sysfs_create_file(&dev->kobj, &dev_attr_name.attr))) {
		goto exit;
	}

	/* Create standard sysfs attributes */
	if ((err = sysfs_create_group(&dev->kobj, &dme1737_group))) {
		goto exit_remove;
	}

	/* Create chip-dependent sysfs attributes */
	if ((data->has_features & HAS_TEMP_OFFSET) &&
	    (err = sysfs_create_group(&dev->kobj,
				      &dme1737_temp_offset_group))) {
		goto exit_remove;
	}
	if ((data->has_features & HAS_VID) &&
	    (err = sysfs_create_group(&dev->kobj,
				      &dme1737_vid_group))) {
		goto exit_remove;
	}
	if ((data->has_features & HAS_ZONE3) &&
	    (err = sysfs_create_group(&dev->kobj,
				      &dme1737_zone3_group))) {
		goto exit_remove;
	}
	if ((data->has_features & HAS_ZONE_HYST) &&
	    (err = sysfs_create_group(&dev->kobj,
				      &dme1737_zone_hyst_group))) {
		goto exit_remove;
	}
	if ((data->has_features & HAS_IN7) &&
	    (err = sysfs_create_group(&dev->kobj,
				      &dme1737_in7_group))) {
		goto exit_remove;
	}

	/* Create fan sysfs attributes */
	for (ix = 0; ix < ARRAY_SIZE(dme1737_fan_group); ix++) {
		if (data->has_features & HAS_FAN(ix)) {
			if ((err = sysfs_create_group(&dev->kobj,
						&dme1737_fan_group[ix]))) {
				goto exit_remove;
			}
		}
	}

	/* Create PWM sysfs attributes */
	for (ix = 0; ix < ARRAY_SIZE(dme1737_pwm_group); ix++) {
		if (data->has_features & HAS_PWM(ix)) {
			if ((err = sysfs_create_group(&dev->kobj,
						&dme1737_pwm_group[ix]))) {
				goto exit_remove;
			}
			if ((data->has_features & HAS_PWM_MIN) && ix < 3 &&
			    (err = sysfs_create_file(&dev->kobj,
					dme1737_auto_pwm_min_attr[ix]))) {
				goto exit_remove;
			}
		}
	}

	/* Inform if the device is locked. Otherwise change the permissions of
	 * selected attributes from read-only to read-writeable. */
	if (data->config & 0x02) {
		dev_info(dev, "Device is locked. Some attributes "
			 "will be read-only.\n");
	} else {
		/* Change permissions of zone sysfs attributes */
		dme1737_chmod_group(dev, &dme1737_zone_chmod_group,
				    S_IRUGO | S_IWUSR);

		/* Change permissions of chip-dependent sysfs attributes */
		if (data->has_features & HAS_TEMP_OFFSET) {
			dme1737_chmod_group(dev, &dme1737_temp_offset_group,
					    S_IRUGO | S_IWUSR);
		}
		if (data->has_features & HAS_ZONE3) {
			dme1737_chmod_group(dev, &dme1737_zone3_chmod_group,
					    S_IRUGO | S_IWUSR);
		}
		if (data->has_features & HAS_ZONE_HYST) {
			dme1737_chmod_group(dev, &dme1737_zone_hyst_group,
					    S_IRUGO | S_IWUSR);
		}

		/* Change permissions of PWM sysfs attributes */
		for (ix = 0; ix < ARRAY_SIZE(dme1737_pwm_chmod_group); ix++) {
			if (data->has_features & HAS_PWM(ix)) {
				dme1737_chmod_group(dev,
						&dme1737_pwm_chmod_group[ix],
						S_IRUGO | S_IWUSR);
				if ((data->has_features & HAS_PWM_MIN) &&
				    ix < 3) {
					dme1737_chmod_file(dev,
						dme1737_auto_pwm_min_attr[ix],
						S_IRUGO | S_IWUSR);
				}
			}
		}

		/* Change permissions of pwm[1-3] if in manual mode */
		for (ix = 0; ix < 3; ix++) {
			if ((data->has_features & HAS_PWM(ix)) &&
			    (PWM_EN_FROM_REG(data->pwm_config[ix]) == 1)) {
				dme1737_chmod_file(dev,
						dme1737_pwm_chmod_attr[ix],
						S_IRUGO | S_IWUSR);
			}
		}
	}

	return 0;

exit_remove:
	dme1737_remove_files(dev);
exit:
	return err;
}

static int dme1737_init_device(struct device *dev)
{
	struct dme1737_data *data = dev_get_drvdata(dev);
	struct i2c_client *client = data->client;
	int ix;
	u8 reg;

	/* Point to the right nominal voltages array */
	data->in_nominal = IN_NOMINAL(data->type);

	data->config = dme1737_read(data, DME1737_REG_CONFIG);
	/* Inform if part is not monitoring/started */
	if (!(data->config & 0x01)) {
		if (!force_start) {
			dev_err(dev, "Device is not monitoring. "
				"Use the force_start load parameter to "
				"override.\n");
			return -EFAULT;
		}

		/* Force monitoring */
		data->config |= 0x01;
		dme1737_write(data, DME1737_REG_CONFIG, data->config);
	}
	/* Inform if part is not ready */
	if (!(data->config & 0x04)) {
		dev_err(dev, "Device is not ready.\n");
		return -EFAULT;
	}

	/* Determine which optional fan and pwm features are enabled (only
	 * valid for I2C devices) */
	if (client) {   /* I2C chip */
		data->config2 = dme1737_read(data, DME1737_REG_CONFIG2);
		/* Check if optional fan3 input is enabled */
		if (data->config2 & 0x04) {
			data->has_features |= HAS_FAN(2);
		}

		/* Fan4 and pwm3 are only available if the client's I2C address
		 * is the default 0x2e. Otherwise the I/Os associated with
		 * these functions are used for addr enable/select. */
		if (client->addr == 0x2e) {
			data->has_features |= HAS_FAN(3) | HAS_PWM(2);
		}

		/* Determine which of the optional fan[5-6] and pwm[5-6]
		 * features are enabled. For this, we need to query the runtime
		 * registers through the Super-IO LPC interface. Try both
		 * config ports 0x2e and 0x4e. */
		if (dme1737_i2c_get_features(0x2e, data) &&
		    dme1737_i2c_get_features(0x4e, data)) {
			dev_warn(dev, "Failed to query Super-IO for optional "
				 "features.\n");
		}
	}

	/* Fan[1-2] and pwm[1-2] are present in all chips */
	data->has_features |= HAS_FAN(0) | HAS_FAN(1) | HAS_PWM(0) | HAS_PWM(1);

	/* Chip-dependent features */
	switch (data->type) {
	case dme1737:
		data->has_features |= HAS_TEMP_OFFSET | HAS_VID | HAS_ZONE3 |
			HAS_ZONE_HYST | HAS_PWM_MIN;
		break;
	case sch311x:
		data->has_features |= HAS_TEMP_OFFSET | HAS_ZONE3 |
			HAS_ZONE_HYST | HAS_PWM_MIN | HAS_FAN(2) | HAS_PWM(2);
		break;
	case sch5027:
		data->has_features |= HAS_ZONE3;
		break;
	case sch5127:
		data->has_features |= HAS_FAN(2) | HAS_PWM(2) | HAS_IN7;
		break;
	default:
		break;
	}

	dev_info(dev, "Optional features: pwm3=%s, pwm5=%s, pwm6=%s, "
		 "fan3=%s, fan4=%s, fan5=%s, fan6=%s.\n",
		 (data->has_features & HAS_PWM(2)) ? "yes" : "no",
		 (data->has_features & HAS_PWM(4)) ? "yes" : "no",
		 (data->has_features & HAS_PWM(5)) ? "yes" : "no",
		 (data->has_features & HAS_FAN(2)) ? "yes" : "no",
		 (data->has_features & HAS_FAN(3)) ? "yes" : "no",
		 (data->has_features & HAS_FAN(4)) ? "yes" : "no",
		 (data->has_features & HAS_FAN(5)) ? "yes" : "no");

	reg = dme1737_read(data, DME1737_REG_TACH_PWM);
	/* Inform if fan-to-pwm mapping differs from the default */
	if (client && reg != 0xa4) {   /* I2C chip */
		dev_warn(dev, "Non-standard fan to pwm mapping: "
			 "fan1->pwm%d, fan2->pwm%d, fan3->pwm%d, "
			 "fan4->pwm%d. Please report to the driver "
			 "maintainer.\n",
			 (reg & 0x03) + 1, ((reg >> 2) & 0x03) + 1,
			 ((reg >> 4) & 0x03) + 1, ((reg >> 6) & 0x03) + 1);
	} else if (!client && reg != 0x24) {   /* ISA chip */
		dev_warn(dev, "Non-standard fan to pwm mapping: "
			 "fan1->pwm%d, fan2->pwm%d, fan3->pwm%d. "
			 "Please report to the driver maintainer.\n",
			 (reg & 0x03) + 1, ((reg >> 2) & 0x03) + 1,
			 ((reg >> 4) & 0x03) + 1);
	}

	/* Switch pwm[1-3] to manual mode if they are currently disabled and
	 * set the duty-cycles to 0% (which is identical to the PWMs being
	 * disabled). */
	if (!(data->config & 0x02)) {
		for (ix = 0; ix < 3; ix++) {
			data->pwm_config[ix] = dme1737_read(data,
						DME1737_REG_PWM_CONFIG(ix));
			if ((data->has_features & HAS_PWM(ix)) &&
			    (PWM_EN_FROM_REG(data->pwm_config[ix]) == -1)) {
				dev_info(dev, "Switching pwm%d to "
					 "manual mode.\n", ix + 1);
				data->pwm_config[ix] = PWM_EN_TO_REG(1,
							data->pwm_config[ix]);
				dme1737_write(data, DME1737_REG_PWM(ix), 0);
				dme1737_write(data,
					      DME1737_REG_PWM_CONFIG(ix),
					      data->pwm_config[ix]);
			}
		}
	}

	/* Initialize the default PWM auto channels zone (acz) assignments */
	data->pwm_acz[0] = 1;	/* pwm1 -> zone1 */
	data->pwm_acz[1] = 2;	/* pwm2 -> zone2 */
	data->pwm_acz[2] = 4;	/* pwm3 -> zone3 */

	/* Set VRM */
	if (data->has_features & HAS_VID) {
		data->vrm = vid_which_vrm();
	}

	return 0;
}

/* ---------------------------------------------------------------------
 * I2C device detection and registration
 * --------------------------------------------------------------------- */

static struct i2c_driver dme1737_i2c_driver;

static int dme1737_i2c_get_features(int sio_cip, struct dme1737_data *data)
{
	int err = 0, reg;
	u16 addr;

	dme1737_sio_enter(sio_cip);

	/* Check device ID
	 * We currently know about two kinds of DME1737 and SCH5027. */
	reg = force_id ? force_id : dme1737_sio_inb(sio_cip, 0x20);
	if (!(reg == DME1737_ID_1 || reg == DME1737_ID_2 ||
	      reg == SCH5027_ID)) {
		err = -ENODEV;
		goto exit;
	}

	/* Select logical device A (runtime registers) */
	dme1737_sio_outb(sio_cip, 0x07, 0x0a);

	/* Get the base address of the runtime registers */
	if (!(addr = (dme1737_sio_inb(sio_cip, 0x60) << 8) |
		      dme1737_sio_inb(sio_cip, 0x61))) {
		err = -ENODEV;
		goto exit;
	}

	/* Read the runtime registers to determine which optional features
	 * are enabled and available. Bits [3:2] of registers 0x43-0x46 are set
	 * to '10' if the respective feature is enabled. */
	if ((inb(addr + 0x43) & 0x0c) == 0x08) { /* fan6 */
		data->has_features |= HAS_FAN(5);
	}
	if ((inb(addr + 0x44) & 0x0c) == 0x08) { /* pwm6 */
		data->has_features |= HAS_PWM(5);
	}
	if ((inb(addr + 0x45) & 0x0c) == 0x08) { /* fan5 */
		data->has_features |= HAS_FAN(4);
	}
	if ((inb(addr + 0x46) & 0x0c) == 0x08) { /* pwm5 */
		data->has_features |= HAS_PWM(4);
	}

exit:
	dme1737_sio_exit(sio_cip);

	return err;
}

/* Return 0 if detection is successful, -ENODEV otherwise */
static int dme1737_i2c_detect(struct i2c_client *client,
			      struct i2c_board_info *info)
{
	struct i2c_adapter *adapter = client->adapter;
	struct device *dev = &adapter->dev;
	u8 company, verstep = 0;
	const char *name;

	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
		return -ENODEV;
	}

	company = i2c_smbus_read_byte_data(client, DME1737_REG_COMPANY);
	verstep = i2c_smbus_read_byte_data(client, DME1737_REG_VERSTEP);

	if (company == DME1737_COMPANY_SMSC &&
	    verstep == SCH5027_VERSTEP) {
		name = "sch5027";
	} else if (company == DME1737_COMPANY_SMSC &&
		   (verstep & DME1737_VERSTEP_MASK) == DME1737_VERSTEP) {
		name = "dme1737";
	} else {
		return -ENODEV;
	}

	dev_info(dev, "Found a %s chip at 0x%02x (rev 0x%02x).\n",
		 verstep == SCH5027_VERSTEP ? "SCH5027" : "DME1737",
		 client->addr, verstep);
	strlcpy(info->type, name, I2C_NAME_SIZE);

	return 0;
}

static int dme1737_i2c_probe(struct i2c_client *client,
			     const struct i2c_device_id *id)
{
	struct dme1737_data *data;
	struct device *dev = &client->dev;
	int err;

	data = kzalloc(sizeof(struct dme1737_data), GFP_KERNEL);
	if (!data) {
		err = -ENOMEM;
		goto exit;
	}

	i2c_set_clientdata(client, data);
	data->type = id->driver_data;
	data->client = client;
	data->name = client->name;
	mutex_init(&data->update_lock);

	/* Initialize the DME1737 chip */
	if ((err = dme1737_init_device(dev))) {
		dev_err(dev, "Failed to initialize device.\n");
		goto exit_kfree;
	}

	/* Create sysfs files */
	if ((err = dme1737_create_files(dev))) {
		dev_err(dev, "Failed to create sysfs files.\n");
		goto exit_kfree;
	}

	/* Register device */
	data->hwmon_dev = hwmon_device_register(dev);
	if (IS_ERR(data->hwmon_dev)) {
		dev_err(dev, "Failed to register device.\n");
		err = PTR_ERR(data->hwmon_dev);
		goto exit_remove;
	}

	return 0;

exit_remove:
	dme1737_remove_files(dev);
exit_kfree:
	kfree(data);
exit:
	return err;
}

static int dme1737_i2c_remove(struct i2c_client *client)
{
	struct dme1737_data *data = i2c_get_clientdata(client);

	hwmon_device_unregister(data->hwmon_dev);
	dme1737_remove_files(&client->dev);

	kfree(data);
	return 0;
}

static const struct i2c_device_id dme1737_id[] = {
	{ "dme1737", dme1737 },
	{ "sch5027", sch5027 },
	{ }
};
MODULE_DEVICE_TABLE(i2c, dme1737_id);

static struct i2c_driver dme1737_i2c_driver = {
	.class = I2C_CLASS_HWMON,
	.driver = {
		.name = "dme1737",
	},
	.probe = dme1737_i2c_probe,
	.remove = dme1737_i2c_remove,
	.id_table = dme1737_id,
	.detect = dme1737_i2c_detect,
	.address_list = normal_i2c,
};

/* ---------------------------------------------------------------------
 * ISA device detection and registration
 * --------------------------------------------------------------------- */

static int __init dme1737_isa_detect(int sio_cip, unsigned short *addr)
{
	int err = 0, reg;
	unsigned short base_addr;

	dme1737_sio_enter(sio_cip);

	/* Check device ID
	 * We currently know about SCH3112, SCH3114, SCH3116, and SCH5127 */
	reg = force_id ? force_id : dme1737_sio_inb(sio_cip, 0x20);
	if (!(reg == SCH3112_ID || reg == SCH3114_ID || reg == SCH3116_ID ||
	      reg == SCH5127_ID)) {
		err = -ENODEV;
		goto exit;
	}

	/* Select logical device A (runtime registers) */
	dme1737_sio_outb(sio_cip, 0x07, 0x0a);

	/* Get the base address of the runtime registers */
	if (!(base_addr = (dme1737_sio_inb(sio_cip, 0x60) << 8) |
			   dme1737_sio_inb(sio_cip, 0x61))) {
		printk(KERN_ERR "dme1737: Base address not set.\n");
		err = -ENODEV;
		goto exit;
	}

	/* Access to the hwmon registers is through an index/data register
	 * pair located at offset 0x70/0x71. */
	*addr = base_addr + 0x70;

exit:
	dme1737_sio_exit(sio_cip);
	return err;
}

static int __init dme1737_isa_device_add(unsigned short addr)
{
	struct resource res = {
		.start	= addr,
		.end	= addr + DME1737_EXTENT - 1,
		.name	= "dme1737",
		.flags	= IORESOURCE_IO,
	};
	int err;

	err = acpi_check_resource_conflict(&res);
	if (err)
		goto exit;

	if (!(pdev = platform_device_alloc("dme1737", addr))) {
		printk(KERN_ERR "dme1737: Failed to allocate device.\n");
		err = -ENOMEM;
		goto exit;
	}

	if ((err = platform_device_add_resources(pdev, &res, 1))) {
		printk(KERN_ERR "dme1737: Failed to add device resource "
		       "(err = %d).\n", err);
		goto exit_device_put;
	}

	if ((err = platform_device_add(pdev))) {
		printk(KERN_ERR "dme1737: Failed to add device (err = %d).\n",
		       err);
		goto exit_device_put;
	}

	return 0;

exit_device_put:
	platform_device_put(pdev);
	pdev = NULL;
exit:
	return err;
}

static int __devinit dme1737_isa_probe(struct platform_device *pdev)
{
	u8 company, device;
	struct resource *res;
	struct dme1737_data *data;
	struct device *dev = &pdev->dev;
	int err;

	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
	if (!request_region(res->start, DME1737_EXTENT, "dme1737")) {
		dev_err(dev, "Failed to request region 0x%04x-0x%04x.\n",
			(unsigned short)res->start,
			(unsigned short)res->start + DME1737_EXTENT - 1);
                err = -EBUSY;
                goto exit;
        }

	if (!(data = kzalloc(sizeof(struct dme1737_data), GFP_KERNEL))) {
		err = -ENOMEM;
		goto exit_release_region;
	}

	data->addr = res->start;
	platform_set_drvdata(pdev, data);

	/* Skip chip detection if module is loaded with force_id parameter */
	switch (force_id) {
	case SCH3112_ID:
	case SCH3114_ID:
	case SCH3116_ID:
		data->type = sch311x;
		break;
	case SCH5127_ID:
		data->type = sch5127;
		break;
	default:
		company = dme1737_read(data, DME1737_REG_COMPANY);
		device = dme1737_read(data, DME1737_REG_DEVICE);

		if ((company == DME1737_COMPANY_SMSC) &&
		    (device == SCH311X_DEVICE)) {
			data->type = sch311x;
		} else if ((company == DME1737_COMPANY_SMSC) &&
			   (device == SCH5127_DEVICE)) {
			data->type = sch5127;
		} else {
			err = -ENODEV;
			goto exit_kfree;
		}
	}

	if (data->type == sch5127) {
		data->name = "sch5127";
	} else {
		data->name = "sch311x";
	}

	/* Initialize the mutex */
	mutex_init(&data->update_lock);

	dev_info(dev, "Found a %s chip at 0x%04x\n",
		 data->type == sch5127 ? "SCH5127" : "SCH311x", data->addr);

	/* Initialize the chip */
	if ((err = dme1737_init_device(dev))) {
		dev_err(dev, "Failed to initialize device.\n");
		goto exit_kfree;
	}

	/* Create sysfs files */
	if ((err = dme1737_create_files(dev))) {
		dev_err(dev, "Failed to create sysfs files.\n");
		goto exit_kfree;
	}

	/* Register device */
	data->hwmon_dev = hwmon_device_register(dev);
	if (IS_ERR(data->hwmon_dev)) {
		dev_err(dev, "Failed to register device.\n");
		err = PTR_ERR(data->hwmon_dev);
		goto exit_remove_files;
	}

	return 0;

exit_remove_files:
	dme1737_remove_files(dev);
exit_kfree:
	platform_set_drvdata(pdev, NULL);
	kfree(data);
exit_release_region:
	release_region(res->start, DME1737_EXTENT);
exit:
	return err;
}

static int __devexit dme1737_isa_remove(struct platform_device *pdev)
{
	struct dme1737_data *data = platform_get_drvdata(pdev);

	hwmon_device_unregister(data->hwmon_dev);
	dme1737_remove_files(&pdev->dev);
	release_region(data->addr, DME1737_EXTENT);
	platform_set_drvdata(pdev, NULL);
	kfree(data);

	return 0;
}

static struct platform_driver dme1737_isa_driver = {
	.driver = {
		.owner = THIS_MODULE,
		.name = "dme1737",
	},
	.probe = dme1737_isa_probe,
	.remove = __devexit_p(dme1737_isa_remove),
};

/* ---------------------------------------------------------------------
 * Module initialization and cleanup
 * --------------------------------------------------------------------- */

static int __init dme1737_init(void)
{
	int err;
	unsigned short addr;

	if ((err = i2c_add_driver(&dme1737_i2c_driver))) {
		goto exit;
	}

	if (dme1737_isa_detect(0x2e, &addr) &&
	    dme1737_isa_detect(0x4e, &addr) &&
	    (!probe_all_addr ||
	     (dme1737_isa_detect(0x162e, &addr) &&
	      dme1737_isa_detect(0x164e, &addr)))) {
		/* Return 0 if we didn't find an ISA device */
		return 0;
	}

	if ((err = platform_driver_register(&dme1737_isa_driver))) {
		goto exit_del_i2c_driver;
	}

	/* Sets global pdev as a side effect */
	if ((err = dme1737_isa_device_add(addr))) {
		goto exit_del_isa_driver;
	}

	return 0;

exit_del_isa_driver:
	platform_driver_unregister(&dme1737_isa_driver);
exit_del_i2c_driver:
	i2c_del_driver(&dme1737_i2c_driver);
exit:
	return err;
}

static void __exit dme1737_exit(void)
{
	if (pdev) {
		platform_device_unregister(pdev);
		platform_driver_unregister(&dme1737_isa_driver);
	}

	i2c_del_driver(&dme1737_i2c_driver);
}

MODULE_AUTHOR("Juerg Haefliger <juergh@gmail.com>");
MODULE_DESCRIPTION("DME1737 sensors");
MODULE_LICENSE("GPL");

module_init(dme1737_init);
module_exit(dme1737_exit);

[-- 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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (13 preceding siblings ...)
  2010-12-10  9:43 ` Juerg Haefliger
@ 2010-12-10 14:27 ` Jean Delvare
  2010-12-11  7:02 ` Jeff Rickman
                   ` (15 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-10 14:27 UTC (permalink / raw)
  To: lm-sensors

Hi Juerg,

On Fri, 10 Dec 2010 10:43:57 +0100, Juerg Haefliger wrote:
> Attached is a modified driver with support for Vtrip monitoring (1.5V,
> in7). Could you give it a try and let me know how it goes? The new
> driver should expose the following new sysfs files: in7_input,
> in7_min, in7_max, in7_alarm. No user-level scaling is required. The
> nominal voltage is 1.5V.

The changes look overall good to me, but I have a few comments if you
are interested:

> --- dme1737.c	2010-08-02 00:11:14.000000000 +0200
> +++ /home/khali/dme1737.c	2010-12-10 14:39:04.000000000 +0100
> @@ -75,16 +75,20 @@
>   * in4   +12V
>   * in5   VTR   (+3.3V stby)
>   * in6   Vbat
> + * in7   Vtrip (sch5127 only)
>   *
>   * --------------------------------------------------------------------- */
>  
> -/* Voltages (in) numbered 0-6 (ix) */
> -#define	DME1737_REG_IN(ix)		((ix) < 5 ? 0x20 + (ix) \
> -						  : 0x94 + (ix))
> -#define	DME1737_REG_IN_MIN(ix)		((ix) < 5 ? 0x44 + (ix) * 2 \
> -						  : 0x91 + (ix) * 2)
> -#define	DME1737_REG_IN_MAX(ix)		((ix) < 5 ? 0x45 + (ix) * 2 \
> -						  : 0x92 + (ix) * 2)
> +/* Voltages (in) numbered 0-7 (ix) */
> +#define	DME1737_REG_IN(ix)		((ix) < 5 ? 0x20 + (ix) : \
> +					 (ix) < 7 ? 0x94 + (ix) : \
> +						    0x1f)
> +#define	DME1737_REG_IN_MIN(ix)		((ix) < 5 ? 0x44 + (ix) * 2 : \
> +					 (ix) < 7 ? 0x91 + (ix) * 2 : \
> +						    0x9f)
> +#define	DME1737_REG_IN_MAX(ix)		((ix) < 5 ? 0x45 + (ix) * 2 : \
> +					 (ix) < 7 ? 0x92 + (ix) * 2 : \
> +						    0xa0)

Maybe it is time to give up macros and go with const arrays? The macros
are never called with a constant parameters so the compiler can't
optimize the calls.

>  
>  /* Temperatures (temp) numbered 0-2 (ix) */
>  #define DME1737_REG_TEMP(ix)		(0x25 + (ix))
> @@ -99,10 +103,11 @@
>   *    IN_TEMP_LSB(1) = [temp3, temp1]
>   *    IN_TEMP_LSB(2) = [in4, temp2]
>   *    IN_TEMP_LSB(3) = [in3, in0]
> - *    IN_TEMP_LSB(4) = [in2, in1] */
> + *    IN_TEMP_LSB(4) = [in2, in1]
> + *    IN_TEMP_LSB(5) = [res, in7] */
>  #define DME1737_REG_IN_TEMP_LSB(ix)	(0x84 + (ix))
> -static const u8 DME1737_REG_IN_LSB[] = {3, 4, 4, 3, 2, 0, 0};
> -static const u8 DME1737_REG_IN_LSB_SHL[] = {4, 4, 0, 0, 0, 0, 4};
> +static const u8 DME1737_REG_IN_LSB[] = {3, 4, 4, 3, 2, 0, 0, 5};
> +static const u8 DME1737_REG_IN_LSB_SHL[] = {4, 4, 0, 0, 0, 0, 4, 4};
>  static const u8 DME1737_REG_TEMP_LSB[] = {1, 2, 1};
>  static const u8 DME1737_REG_TEMP_LSB_SHL[] = {4, 4, 0};
>  
> @@ -143,7 +148,7 @@
>  #define DME1737_REG_ALARM1		0x41
>  #define DME1737_REG_ALARM2		0x42
>  #define DME1737_REG_ALARM3		0x83
> -static const u8 DME1737_BIT_ALARM_IN[] = {0, 1, 2, 3, 8, 16, 17};
> +static const u8 DME1737_BIT_ALARM_IN[] = {0, 1, 2, 3, 8, 16, 17, 18};
>  static const u8 DME1737_BIT_ALARM_TEMP[] = {4, 5, 6};
>  static const u8 DME1737_BIT_ALARM_FAN[] = {10, 11, 12, 13, 22, 23};
>  
> @@ -188,6 +193,7 @@
>  #define HAS_PWM_MIN		(1 << 4)		/* bit 4 */
>  #define HAS_FAN(ix)		(1 << ((ix) + 5))	/* bits 5-10 */
>  #define HAS_PWM(ix)		(1 << ((ix) + 11))	/* bits 11-16 */
> +#define HAS_IN7			(1 << 17)		/* bit 17 */
>  
>  /* ---------------------------------------------------------------------
>   * Data structures and manipulation thereof
> @@ -245,7 +251,7 @@
>  static const int IN_NOMINAL_SCH5027[] = {5000, 2250, 3300, 1125, 1125, 3300,
>  					 3300};
>  static const int IN_NOMINAL_SCH5127[] = {2500, 2250, 3300, 1125, 1125, 3300,
> -					 3300};
> +					 3300, 1500};
>  #define IN_NOMINAL(type)	((type) = sch311x ? IN_NOMINAL_SCH311x : \
>  				 (type) = sch5027 ? IN_NOMINAL_SCH5027 : \
>  				 (type) = sch5127 ? IN_NOMINAL_SCH5127 : \
> @@ -578,7 +584,7 @@
>  {
>  	struct dme1737_data *data = dev_get_drvdata(dev);
>  	int ix;
> -	u8 lsb[5];
> +	u8 lsb[6];
>  
>  	mutex_lock(&data->update_lock);
>  
> @@ -601,12 +607,14 @@
>  			/* Voltage inputs are stored as 16 bit values even
>  			 * though they have only 12 bits resolution. This is
>  			 * to make it consistent with the temp inputs. */
> -			data->in[ix] = dme1737_read(data,
> +			if (ix != 7 || (data->has_features & HAS_IN7)) {

It is less intrusive to do the opposite test:

			if (ix = 7 && !(data->has_features & HAS_IN7))
				continue;

This avoids reindenting the whole block.

> +				data->in[ix] = dme1737_read(data,
>  					DME1737_REG_IN(ix)) << 8;
> -			data->in_min[ix] = dme1737_read(data,
> +				data->in_min[ix] = dme1737_read(data,
>  					DME1737_REG_IN_MIN(ix));
> -			data->in_max[ix] = dme1737_read(data,
> +				data->in_max[ix] = dme1737_read(data,
>  					DME1737_REG_IN_MAX(ix));
> +			}
>  		}
>  
>  		/* Temp registers */
> @@ -633,12 +641,16 @@
>  		 * which the registers are read (MSB first, then LSB) is
>  		 * important! */
>  		for (ix = 0; ix < ARRAY_SIZE(lsb); ix++) {
> -			lsb[ix] = dme1737_read(data,
> +			if (ix != 5 || (data->has_features & HAS_IN7)) {
> +				lsb[ix] = dme1737_read(data,
>  					DME1737_REG_IN_TEMP_LSB(ix));
> +			}
>  		}
>  		for (ix = 0; ix < ARRAY_SIZE(data->in); ix++) {
> -			data->in[ix] |= (lsb[DME1737_REG_IN_LSB[ix]] <<
> +			if (ix != 7 || (data->has_features & HAS_IN7)) {
> +				data->in[ix] |= (lsb[DME1737_REG_IN_LSB[ix]] <<
>  					DME1737_REG_IN_LSB_SHL[ix]) & 0xf0;
> +			}
>  		}
>  		for (ix = 0; ix < ARRAY_SIZE(data->temp); ix++) {
>  			data->temp[ix] |= (lsb[DME1737_REG_TEMP_LSB[ix]] <<
> @@ -760,7 +772,7 @@
>  
>  /* ---------------------------------------------------------------------
>   * Voltage sysfs attributes
> - * ix = [0-5]
> + * ix = [0-6]

Shouldn't this be [0-7]?

>   * --------------------------------------------------------------------- */
>  
>  #define SYS_IN_INPUT	0
> @@ -1437,7 +1449,7 @@
>   * Sysfs device attribute defines and structs
>   * --------------------------------------------------------------------- */
>  
> -/* Voltages 0-6 */
> +/* Voltages 0-7 */
>  
>  #define SENSOR_DEVICE_ATTR_IN(ix) \
>  static SENSOR_DEVICE_ATTR_2(in##ix##_input, S_IRUGO, \
> @@ -1456,6 +1468,7 @@
>  SENSOR_DEVICE_ATTR_IN(4);
>  SENSOR_DEVICE_ATTR_IN(5);
>  SENSOR_DEVICE_ATTR_IN(6);
> +SENSOR_DEVICE_ATTR_IN(7);
>  
>  /* Temperatures 1-3 */
>  
> @@ -1678,8 +1691,7 @@
>  	.attrs = dme1737_zone3_attr,
>  };
>  
> -
> -/* The following struct holds temp zone hysteresis  related attributes, which
> +/* The following struct holds temp zone hysteresis related attributes, which

Cleanups are preferred as separate patches (makes backporting easier.)

>   * are not available in all chips. The following chips support them:
>   * DME1737, SCH311x */
>  static struct attribute *dme1737_zone_hyst_attr[] = {
> @@ -1693,6 +1705,21 @@
>  	.attrs = dme1737_zone_hyst_attr,
>  };
>  
> +/* The following struct holds voltage in7 related attributes, which
> + * are not available in all chips. The following chips support them:
> + * SCH5127 */
> +static struct attribute *dme1737_in7_attr[] = {
> +	&sensor_dev_attr_in7_input.dev_attr.attr,
> +	&sensor_dev_attr_in7_min.dev_attr.attr,
> +	&sensor_dev_attr_in7_max.dev_attr.attr,
> +	&sensor_dev_attr_in7_alarm.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group dme1737_in7_group = {
> +	.attrs = dme1737_in7_attr,
> +};
> +
>  /* The following structs hold the PWM attributes, some of which are optional.
>   * Their creation depends on the chip configuration which is determined during
>   * module load. */
> @@ -1984,6 +2011,9 @@
>  	if (data->has_features & HAS_ZONE_HYST) {
>  		sysfs_remove_group(&dev->kobj, &dme1737_zone_hyst_group);
>  	}
> +	if (data->has_features & HAS_IN7) {
> +		sysfs_remove_group(&dev->kobj, &dme1737_in7_group);
> +	}
>  	sysfs_remove_group(&dev->kobj, &dme1737_group);
>  
>  	if (!data->client) {
> @@ -2028,6 +2058,11 @@
>  				      &dme1737_zone_hyst_group))) {
>  		goto exit_remove;
>  	}
> +	if ((data->has_features & HAS_IN7) &&
> +	    (err = sysfs_create_group(&dev->kobj,
> +				      &dme1737_in7_group))) {
> +		goto exit_remove;
> +	}

checkpatch.pl will complain. It wants:

	if (data->has_features & HAS_IN7) {
		err = sysfs_create_group(&dev->kobj, &dme1737_in7_group);
		if (err)
			goto exit_remove;
	}

>  
>  	/* Create fan sysfs attributes */
>  	for (ix = 0; ix < ARRAY_SIZE(dme1737_fan_group); ix++) {
> @@ -2186,7 +2221,7 @@
>  		data->has_features |= HAS_ZONE3;
>  		break;
>  	case sch5127:
> -		data->has_features |= HAS_FAN(2) | HAS_PWM(2);
> +		data->has_features |= HAS_FAN(2) | HAS_PWM(2) | HAS_IN7;
>  		break;
>  	default:
>  		break;
> 

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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (14 preceding siblings ...)
  2010-12-10 14:27 ` Jean Delvare
@ 2010-12-11  7:02 ` Jeff Rickman
  2010-12-11 10:16 ` Jeff Rickman
                   ` (14 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-12-11  7:02 UTC (permalink / raw)
  To: lm-sensors

Hi Juerg,

> Hi Jeff,
>
>>> Ah. I just realized that the SCH5127 has another voltage input that
>>> the other dme1737 clones don't have but which isn't exposed through
>>> the driver. Register 0x1f, nominal 1.5V, max 2.0V.
>>
>> Ah ah! Very interesting. It could be where Jeff's mysterious +1.5V is
>> connected! Could you write a patch adding support?
>
> Attached is a modified driver with support for Vtrip monitoring (1.5V,
> in7). Could you give it a try and let me know how it goes? The new
> driver should expose the following new sysfs files: in7_input,
> in7_min, in7_max, in7_alarm. No user-level scaling is required. The
> nominal voltage is 1.5V.
>
> Thanks
> ...Juerg
>

I successfully compiled the module. Then I removed the old module with
"modprobe -r dme1737". Next I copied the new "dme1737.ko" module into the
proper location in "/lib/modules/`uname -r`/kernel/drivers/hwmon" and set
the proper permissions with "chmod"; FC14 uses permissions of '744'. Now I
installed the kernel module with "modprobe -v dme1737" and the Console
printed the following:

insmod /lib/modules/2.6.35.9-64.fc14.x86_64/kernel/drivers/hwmon/hwmon-vid.ko
insmod /lib/modules/2.6.35.9-64.fc14.x86_64/kernel/drivers/hwmon/dme1737.ko

So far so good.....

Now I ran "sensors" without any changes to my "/etc/sensors.d/local.conf"
file.

[root@anas-01 ~]# sensors
sch5127-isa-0870
Adapter: ISA adapter
in0:         +1.78 V  (min =  +0.00 V, max =  +3.32 V)
Vccp:        +1.19 V  (min =  +1.08 V, max =  +1.32 V)
VCC:         +3.31 V  (min =  +2.97 V, max =  +3.63 V)
in3:         +1.14 V  (min =  +0.00 V, max =  +1.49 V)
in4:         +1.48 V  (min =  +0.00 V, max =  +1.49 V)
VTR:         +3.33 V  (min =  +2.97 V, max =  +3.63 V)
VBAT:        +3.24 V  (min =  +2.70 V, max =  +3.30 V)
in7:         +0.72 V  (min =  +1.99 V, max =  +0.25 V)
Side Fan:   1884 RPM  (min =  700 RPM)
MCH Fan:    5027 RPM  (min = 4500 RPM)
CPU Temp:    +40.1°C  (low  = +20.0°C, high = +60.0°C)
SYS Temp:    +32.9°C  (low  = +20.0°C, high = +60.0°C)

For the record I have "blacklisted" the 'coretemp' module on this machine
as it reports strange temp values (anything from ~9C to ~15C and those are
too cold) for an Intel Atom 230 CPU (per "/proc/cpuinfo": CPU family 6,
model 28, stepping 2) compared to the "CPU Temp" value reported by
'sensors'.

"in7" is now reported but it will require a "compute (@ * 2), (@ / 2)"
line in my "/etc/sensors.d/local.conf" to provide a reasonable value of
1.44: 0.72 * 2 = 1.44, which is close enough to the BIOS reported +1.5V
value.

Both "in3" and "in4" are "unknowns" to me. I know they will be external
voltage inputs to the SCH5127, but it's a guess as to what is being
measured. I located a "service guide" for the Acer H340, but it is useless
in this regard. The same judgement applies to "in0". the remaining inputs
are marked per the kernel documentation notes written by Juerg.

Checking the BIOS screen for this machine shows the "V+1.5" reading out a
steady ~1.5V.

Also, a comment for Jean. I pulled the CMOS battery and booted the
machine...before making this kernel module change. I did not see any
change in the BIOS reported value for "VBAT" and "sensors" did not see any
reported change; BIOS reported +3.20V and "sensors" reported +3.24V.
Perhaps Acer makes that measurement ahead of the CMOS battery on the "+"
voltage line? I wonder why. Or perhaps I don't understand what "VBAT" is
supposed to measure.

Finally, for good measure I ran "depmod -a" and rebooted the machine. The
'updated' dme1737.ko module is correctly loaded on reboot.

Dec 11 00:59:00 anas-01 kernel: [   24.137243] dme1737 dme1737.2160: Found
a SCH5127 chip at 0x0870
Dec 11 00:59:00 anas-01 kernel: [   24.167411] dme1737 dme1737.2160:
Optional features: pwm3=yes, pwm5=no, pwm6=no, fan3=yes, fan4=no, fan5=no,
fan6=no.




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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (15 preceding siblings ...)
  2010-12-11  7:02 ` Jeff Rickman
@ 2010-12-11 10:16 ` Jeff Rickman
  2010-12-12 18:03 ` Jean Delvare
                   ` (13 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-12-11 10:16 UTC (permalink / raw)
  To: lm-sensors

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

Hi Juerg,

> Hi Jeff,
>
>>> Ah. I just realized that the SCH5127 has another voltage input that
>>> the other dme1737 clones don't have but which isn't exposed through
>>> the driver. Register 0x1f, nominal 1.5V, max 2.0V.
>>
>> Ah ah! Very interesting. It could be where Jeff's mysterious +1.5V is
>> connected! Could you write a patch adding support?
>
> Attached is a modified driver with support for Vtrip monitoring (1.5V,
> in7). Could you give it a try and let me know how it goes? The new
> driver should expose the following new sysfs files: in7_input,
> in7_min, in7_max, in7_alarm. No user-level scaling is required. The
> nominal voltage is 1.5V.
>
> Thanks
> ...Juerg
>

See attached text file for output of "/sys/devices/platform/dme1737.2160".
It confirms what you coded. I should have sent this along earlier.

For the record:
CPU: Intel Atom 230 Stepping 2 (Family 6, Model 28, Stepping 2)
RAM: 2GB
OS: Fedora Core 14  2.6.35.9-64.fc14.x86_64

Please look at the "min" and "max" values for "in7". They look reversed.

[root@anas-01 ~]# sensors
sch5127-isa-0870
Adapter: ISA adapter
in0:         +1.78 V  (min =  +0.00 V, max =  +3.32 V)
Vccp:        +1.19 V  (min =  +1.08 V, max =  +1.32 V)
VCC:         +3.31 V  (min =  +2.97 V, max =  +3.63 V)
in3:         +1.13 V  (min =  +0.00 V, max =  +1.49 V)
in4:         +1.48 V  (min =  +0.00 V, max =  +1.49 V)
VTR:         +3.33 V  (min =  +2.97 V, max =  +3.63 V)
VBAT:        +3.24 V  (min =  +2.70 V, max =  +3.30 V)
in7:         +0.72 V  (min =  +1.99 V, max =  +1.75 V)
Side Fan:   1863 RPM  (min =  700 RPM)
MCH Fan:    5023 RPM  (min = 4500 RPM)
CPU Temp:    +39.9°C  (low  = +20.0°C, high = +60.0°C)
SYS Temp:    +32.4°C  (low  = +20.0°C, high = +60.0°C)

I do not have any settings in my "/etc/sensors.d/local.conf" file for
"in7" that would cause this. My "/etc/sensors3.conf" file is "as
delivered" by LM_Sensors:

[root@anas-01 ~]# sensors -v
sensors version 3.2.0 with libsensors version 3.2.0

[root@anas-01 ~]# yum list installed | grep -i "sensors"
lm_sensors.x86_64 3.2.0-1.fc14   
@anaconda-InstallationRepo-201010211827.x86_64
lm_sensors-libs.x86_64


[-- Attachment #2: updated_dme1737_sysfs.txt --]
[-- Type: text/plain, Size: 2077 bytes --]

[root@anas-01 ~]# ls /sys/devices/platform/dme1737.2160    
driver      fan3_alarm  in1_alarm  in3_input  in5_max    in7_min                  pwm1_freq                pwm3                     temp1_fault  temp3_alarm               zone1_auto_point3_temp
fan1_alarm  fan3_input  in1_input  in3_max    in5_min    modalias                 pwm1_ramp_rate           pwm3_auto_channels_zone  temp1_input  temp3_fault               zone2_auto_channels_temp
fan1_input  fan3_min    in1_max    in3_min    in6_alarm  name                     pwm2                     pwm3_auto_point1_pwm     temp1_max    temp3_input               zone2_auto_point1_temp
fan1_min    fan3_type   in1_min    in4_alarm  in6_input  power                    pwm2_auto_channels_zone  pwm3_auto_point2_pwm     temp1_min    temp3_max                 zone2_auto_point2_temp
fan1_type   hwmon       in2_alarm  in4_input  in6_max    pwm1                     pwm2_auto_point1_pwm     pwm3_enable              temp2_alarm  temp3_min                 zone2_auto_point3_temp
fan2_alarm  in0_alarm   in2_input  in4_max    in6_min    pwm1_auto_channels_zone  pwm2_auto_point2_pwm     pwm3_freq                temp2_fault  uevent
fan2_input  in0_input   in2_max    in4_min    in7_alarm  pwm1_auto_point1_pwm     pwm2_enable              pwm3_ramp_rate           temp2_input  zone1_auto_channels_temp
fan2_min    in0_max     in2_min    in5_alarm  in7_input  pwm1_auto_point2_pwm     pwm2_freq                subsystem                temp2_max    zone1_auto_point1_temp
fan2_type   in0_min     in3_alarm  in5_input  in7_max    pwm1_enable              pwm2_ramp_rate           temp1_alarm              temp2_min    zone1_auto_point2_temp
[root@anas-01 ~]# ls /sys/devices/platform/dme1737.2160/in7_min
/sys/devices/platform/dme1737.2160/in7_min
[root@anas-01 ~]# cat /sys/devices/platform/dme1737.2160/in7_min
1992
[root@anas-01 ~]# cat /sys/devices/platform/dme1737.2160/in7_max
1750
[root@anas-01 ~]# cat /sys/devices/platform/dme1737.2160/in7_input
719
[root@anas-01 ~]# cat /sys/devices/platform/dme1737.2160/in7_alarm
0

[-- 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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (16 preceding siblings ...)
  2010-12-11 10:16 ` Jeff Rickman
@ 2010-12-12 18:03 ` Jean Delvare
  2010-12-12 20:57 ` Jean Delvare
                   ` (12 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-12 18:03 UTC (permalink / raw)
  To: lm-sensors

Hi again Juerg,

On Fri, 10 Dec 2010 15:27:02 +0100, Jean Delvare wrote:
> On Fri, 10 Dec 2010 10:43:57 +0100, Juerg Haefliger wrote:
> > --- dme1737.c	2010-08-02 00:11:14.000000000 +0200
> > +++ /home/khali/dme1737.c	2010-12-10 14:39:04.000000000 +0100
> (...)
> > -/* Voltages (in) numbered 0-6 (ix) */
> > -#define	DME1737_REG_IN(ix)		((ix) < 5 ? 0x20 + (ix) \
> > -						  : 0x94 + (ix))
> > -#define	DME1737_REG_IN_MIN(ix)		((ix) < 5 ? 0x44 + (ix) * 2 \
> > -						  : 0x91 + (ix) * 2)
> > -#define	DME1737_REG_IN_MAX(ix)		((ix) < 5 ? 0x45 + (ix) * 2 \
> > -						  : 0x92 + (ix) * 2)
> > +/* Voltages (in) numbered 0-7 (ix) */
> > +#define	DME1737_REG_IN(ix)		((ix) < 5 ? 0x20 + (ix) : \
> > +					 (ix) < 7 ? 0x94 + (ix) : \
> > +						    0x1f)
> > +#define	DME1737_REG_IN_MIN(ix)		((ix) < 5 ? 0x44 + (ix) * 2 : \
> > +					 (ix) < 7 ? 0x91 + (ix) * 2 : \
> > +						    0x9f)
> > +#define	DME1737_REG_IN_MAX(ix)		((ix) < 5 ? 0x45 + (ix) * 2 : \
> > +					 (ix) < 7 ? 0x92 + (ix) * 2 : \
> > +						    0xa0)

I just noticed that 0x91 + 7 * 2 = 0x9f and 0x92 + 7 * 2 = 0xa0. So
the changes to DME1737_REG_IN_MIN and DME1737_REG_IN_MAX are no-ops.

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (17 preceding siblings ...)
  2010-12-12 18:03 ` Jean Delvare
@ 2010-12-12 20:57 ` Jean Delvare
  2010-12-12 21:05 ` Jean Delvare
                   ` (11 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-12 20:57 UTC (permalink / raw)
  To: lm-sensors

Hi Jeff,

On Sat, 11 Dec 2010 01:02:02 -0600 (CST), Jeff Rickman wrote:
> > Attached is a modified driver with support for Vtrip monitoring (1.5V,
> > in7). Could you give it a try and let me know how it goes? The new
> > driver should expose the following new sysfs files: in7_input,
> > in7_min, in7_max, in7_alarm. No user-level scaling is required. The
> > nominal voltage is 1.5V.
> 
> I successfully compiled the module. Then I removed the old module with
> "modprobe -r dme1737". Next I copied the new "dme1737.ko" module into the
> proper location in "/lib/modules/`uname -r`/kernel/drivers/hwmon" and set
> the proper permissions with "chmod"; FC14 uses permissions of '744'. Now I
> installed the kernel module with "modprobe -v dme1737" and the Console
> printed the following:
> 
> insmod /lib/modules/2.6.35.9-64.fc14.x86_64/kernel/drivers/hwmon/hwmon-vid.ko
> insmod /lib/modules/2.6.35.9-64.fc14.x86_64/kernel/drivers/hwmon/dme1737.ko
> 
> So far so good.....
> 
> Now I ran "sensors" without any changes to my "/etc/sensors.d/local.conf"
> file.
> 
> [root@anas-01 ~]# sensors
> sch5127-isa-0870
> Adapter: ISA adapter
> in0:         +1.78 V  (min =  +0.00 V, max =  +3.32 V)
> Vccp:        +1.19 V  (min =  +1.08 V, max =  +1.32 V)
> VCC:         +3.31 V  (min =  +2.97 V, max =  +3.63 V)
> in3:         +1.14 V  (min =  +0.00 V, max =  +1.49 V)
> in4:         +1.48 V  (min =  +0.00 V, max =  +1.49 V)
> VTR:         +3.33 V  (min =  +2.97 V, max =  +3.63 V)
> VBAT:        +3.24 V  (min =  +2.70 V, max =  +3.30 V)
> in7:         +0.72 V  (min =  +1.99 V, max =  +0.25 V)
> Side Fan:   1884 RPM  (min =  700 RPM)
> MCH Fan:    5027 RPM  (min = 4500 RPM)
> CPU Temp:    +40.1°C  (low  = +20.0°C, high = +60.0°C)
> SYS Temp:    +32.9°C  (low  = +20.0°C, high = +60.0°C)
> 
> For the record I have "blacklisted" the 'coretemp' module on this machine
> as it reports strange temp values (anything from ~9C to ~15C and those are
> too cold) for an Intel Atom 230 CPU (per "/proc/cpuinfo": CPU family 6,
> model 28, stepping 2) compared to the "CPU Temp" value reported by
> 'sensors'.
> 
> "in7" is now reported but it will require a "compute (@ * 2), (@ / 2)"
> line in my "/etc/sensors.d/local.conf" to provide a reasonable value of
> 1.44: 0.72 * 2 = 1.44, which is close enough to the BIOS reported +1.5V
> value.

Close enough? I wouldn't say that. Which value does the BIOS report
exactly?

BTW, you never clearly reported all exact voltage values reported by the
BIOS. This would help a lot.

If you stay in the BIOS long enough to spot more than one value for
each voltage, this can help deduce scaling factors.

> Both "in3" and "in4" are "unknowns" to me. I know they will be external
> voltage inputs to the SCH5127, but it's a guess as to what is being
> measured. I located a "service guide" for the Acer H340, but it is useless
> in this regard. The same judgement applies to "in0". the remaining inputs
> are marked per the kernel documentation notes written by Juerg.

I believe that in4 isn't used. It's too close to the ADC's high limit
to be useful. And the dme1737 driver now shows has one more voltage
value than your BIOS does, right?

> Checking the BIOS screen for this machine shows the "V+1.5" reading out a
> steady ~1.5V.

1.5 V is quite different from 1.44 V, so either it's not in7, or the
scaling factor of 2 isn't correct.

Anyway, it beats me why the manufacturer would scale down a voltage
which it could monitor directly. Very odd.

> Also, a comment for Jean. I pulled the CMOS battery and booted the
> machine...before making this kernel module change. I did not see any
> change in the BIOS reported value for "VBAT" and "sensors" did not see any
> reported change; BIOS reported +3.20V and "sensors" reported +3.24V.
> Perhaps Acer makes that measurement ahead of the CMOS battery on the "+"
> voltage line? I wonder why. Or perhaps I don't understand what "VBAT" is
> supposed to measure.

Vbat is measuring the voltage at the Vbat pin of the chip, which is
supposed to be connected to the battery.

Now it is possible that the board designer decided to also connect
+3.3V or 3VSB there, so that the battery is only used when other power
sources aren't available, to extend the battery's life. This makes
sense, but makes monitoring of Vbat useless, of course. This would
explain the relatively high Vbat value. CR2032 batteries are rated at
3.0V, not 3.3V.

> Finally, for good measure I ran "depmod -a" and rebooted the machine. The
> 'updated' dme1737.ko module is correctly loaded on reboot.
> 
> Dec 11 00:59:00 anas-01 kernel: [   24.137243] dme1737 dme1737.2160: Found
> a SCH5127 chip at 0x0870

Strange. The address is different from the one sensors-detect reported
(0x800.)

> Dec 11 00:59:00 anas-01 kernel: [   24.167411] dme1737 dme1737.2160:
> Optional features: pwm3=yes, pwm5=no, pwm6=no, fan3=yes, fan4=no, fan5=no,
> fan6=no.

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (18 preceding siblings ...)
  2010-12-12 20:57 ` Jean Delvare
@ 2010-12-12 21:05 ` Jean Delvare
  2010-12-12 21:22 ` Jean Delvare
                   ` (10 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-12 21:05 UTC (permalink / raw)
  To: lm-sensors

On Sat, 11 Dec 2010 04:16:07 -0600 (CST), Jeff Rickman wrote:
> Please look at the "min" and "max" values for "in7". They look reversed.
> 
> [root@anas-01 ~]# sensors
> sch5127-isa-0870
> Adapter: ISA adapter
> in0:         +1.78 V  (min =  +0.00 V, max =  +3.32 V)
> Vccp:        +1.19 V  (min =  +1.08 V, max =  +1.32 V)
> VCC:         +3.31 V  (min =  +2.97 V, max =  +3.63 V)
> in3:         +1.13 V  (min =  +0.00 V, max =  +1.49 V)
> in4:         +1.48 V  (min =  +0.00 V, max =  +1.49 V)
> VTR:         +3.33 V  (min =  +2.97 V, max =  +3.63 V)
> VBAT:        +3.24 V  (min =  +2.70 V, max =  +3.30 V)
> in7:         +0.72 V  (min =  +1.99 V, max =  +1.75 V)
> Side Fan:   1863 RPM  (min =  700 RPM)
> MCH Fan:    5023 RPM  (min = 4500 RPM)
> CPU Temp:    +39.9°C  (low  = +20.0°C, high = +60.0°C)
> SYS Temp:    +32.4°C  (low  = +20.0°C, high = +60.0°C)

The in7 limit values are probably simply not initialized by the BIOS.
At least the in7_max value differs from your previous post.

I admit the in7_min limit is surprising, as it didn't change and
corresponds to the max value (register value 0xff), which would make
sense for a max limit but not for a min limit. But even more surprising
is that the alarm flag doesn't show even though the reading is below
the min limit. There's something fishy here. You should check if you
can change the limits and if you are able to get the alarm flag to
trigger.

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (19 preceding siblings ...)
  2010-12-12 21:05 ` Jean Delvare
@ 2010-12-12 21:22 ` Jean Delvare
  2010-12-13  4:02 ` Jeff Rickman
                   ` (9 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-12 21:22 UTC (permalink / raw)
  To: lm-sensors

On Sun, 12 Dec 2010 21:57:34 +0100, Jean Delvare wrote:
> Strange. The address is different from the one sensors-detect reported
> (0x800.)

Oh well, after reading the code it's all OK, there's an offset of 0x70
to the relevant register, so the difference is expected.

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (20 preceding siblings ...)
  2010-12-12 21:22 ` Jean Delvare
@ 2010-12-13  4:02 ` Jeff Rickman
  2010-12-13  4:18 ` Jeff Rickman
                   ` (8 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-12-13  4:02 UTC (permalink / raw)
  To: lm-sensors

Hi Jean,

> Hi Jeff,
>
> On Sat, 11 Dec 2010 01:02:02 -0600 (CST), Jeff Rickman wrote:
>> > Attached is a modified driver with support for Vtrip monitoring (1.5V,
>> > in7). Could you give it a try and let me know how it goes? The new
>> > driver should expose the following new sysfs files: in7_input,
>> > in7_min, in7_max, in7_alarm. No user-level scaling is required. The
>> > nominal voltage is 1.5V.
>>
>> I successfully compiled the module. Then I removed the old module with
>> "modprobe -r dme1737". Next I copied the new "dme1737.ko" module into
>> the
>> proper location in "/lib/modules/`uname -r`/kernel/drivers/hwmon" and
>> set
>> the proper permissions with "chmod"; FC14 uses permissions of '744'. Now
>> I
>> installed the kernel module with "modprobe -v dme1737" and the Console
>> printed the following:
>>
>> insmod
>> /lib/modules/2.6.35.9-64.fc14.x86_64/kernel/drivers/hwmon/hwmon-vid.ko
>> insmod
>> /lib/modules/2.6.35.9-64.fc14.x86_64/kernel/drivers/hwmon/dme1737.ko
>>
>> So far so good.....
>>
>> Now I ran "sensors" without any changes to my
>> "/etc/sensors.d/local.conf"
>> file.
>>
>> [root@anas-01 ~]# sensors
>> sch5127-isa-0870
>> Adapter: ISA adapter
>> in0:         +1.78 V  (min =  +0.00 V, max =  +3.32 V)
>> Vccp:        +1.19 V  (min =  +1.08 V, max =  +1.32 V)
>> VCC:         +3.31 V  (min =  +2.97 V, max =  +3.63 V)
>> in3:         +1.14 V  (min =  +0.00 V, max =  +1.49 V)
>> in4:         +1.48 V  (min =  +0.00 V, max =  +1.49 V)
>> VTR:         +3.33 V  (min =  +2.97 V, max =  +3.63 V)
>> VBAT:        +3.24 V  (min =  +2.70 V, max =  +3.30 V)
>> in7:         +0.72 V  (min =  +1.99 V, max =  +0.25 V)
>> Side Fan:   1884 RPM  (min =  700 RPM)
>> MCH Fan:    5027 RPM  (min = 4500 RPM)
>> CPU Temp:    +40.1°C  (low  = +20.0°C, high = +60.0°C)
>> SYS Temp:    +32.9°C  (low  = +20.0°C, high = +60.0°C)
>>
>> For the record I have "blacklisted" the 'coretemp' module on this
>> machine
>> as it reports strange temp values (anything from ~9C to ~15C and those
>> are
>> too cold) for an Intel Atom 230 CPU (per "/proc/cpuinfo": CPU family 6,
>> model 28, stepping 2) compared to the "CPU Temp" value reported by
>> 'sensors'.
>>
>> "in7" is now reported but it will require a "compute (@ * 2), (@ / 2)"
>> line in my "/etc/sensors.d/local.conf" to provide a reasonable value of
>> 1.44: 0.72 * 2 = 1.44, which is close enough to the BIOS reported +1.5V
>> value.
>
> Close enough? I wouldn't say that. Which value does the BIOS report
> exactly?

During 1 test run I kept the system "in the BIOS" on the "Hardware
Monitor" screen for almost an hour. The voltages never changed. I suspect
the vendor isn't reporting any measured voltages here; could be "static"
voltage values. As for the "V+1.5" value in the BIOS, it never changed
from "1.50". Never. Only the System Fan Speed (LM_Sensors 'Fan1'), CPU
Temp, and SYS Temp values ever change on this screen.

>
> BTW, you never clearly reported all exact voltage values reported by the
> BIOS. This would help a lot.

As displayed "in order" from the "Hardware Monitor" screen:

V+1.5 = 1.50 V
5VTR  = 4.90 V
VBAT  = 3.20 V
V+5   = 5 V
Vccp  = 1.20 V
VCC   = 3.30 V
VTR   = 3.30 V

>
> If you stay in the BIOS long enough to spot more than one value for
> each voltage, this can help deduce scaling factors.

As I write this email now, none of the voltages ever change, not even by
1/100 (0.01) of a Volt while sitting in the BIOS for almost an hour. I
hardly believe any power supply or system power rails are that stable,
based on experiences with other PC-style computers.

>
>> Both "in3" and "in4" are "unknowns" to me. I know they will be external
>> voltage inputs to the SCH5127, but it's a guess as to what is being
>> measured. I located a "service guide" for the Acer H340, but it is
>> useless
>> in this regard. The same judgement applies to "in0". the remaining
>> inputs
>> are marked per the kernel documentation notes written by Juerg.
>
> I believe that in4 isn't used. It's too close to the ADC's high limit
> to be useful. And the dme1737 driver now shows has one more voltage
> value than your BIOS does, right?
>
>> Checking the BIOS screen for this machine shows the "V+1.5" reading out
>> a
>> steady ~1.5V.
>
> 1.5 V is quite different from 1.44 V, so either it's not in7, or the
> scaling factor of 2 isn't correct.

1.44V is within +- 10 percent of 1.5V, but I agree that improvement is
needed; changing scaling values in LM_Sensors is a "math problem".

>
> Anyway, it beats me why the manufacturer would scale down a voltage
> which it could monitor directly. Very odd.

This entire Acer system is "very odd".

Why would a vendor set the system fan to ~760 rpm when HDD temperatures at
that level almost reach 50C and before any serious system load is applied?
50C is a dangerous threshold for most HDD. Thus I use "fancontrol" to
override that automatic setting and push fan speed to "full" to bring HDD
temps down to <@C. If there is a better method than "fancontrol" that I
can call from "rc.local" (like "echo 'value' > 'someplace in system'"), I
would appreciate it since there is no BIOS control for fan speed at all:
it's based on a "Phoenix TrustedCore" BIOS / Intel Shelton Reference BIOS.

>
>> Also, a comment for Jean. I pulled the CMOS battery and booted the
>> machine...before making this kernel module change. I did not see any
>> change in the BIOS reported value for "VBAT" and "sensors" did not see
>> any
>> reported change; BIOS reported +3.20V and "sensors" reported +3.24V.
>> Perhaps Acer makes that measurement ahead of the CMOS battery on the "+"
>> voltage line? I wonder why. Or perhaps I don't understand what "VBAT" is
>> supposed to measure.
>
> Vbat is measuring the voltage at the Vbat pin of the chip, which is
> supposed to be connected to the battery.
>
> Now it is possible that the board designer decided to also connect
> +3.3V or 3VSB there, so that the battery is only used when other power
> sources aren't available, to extend the battery's life. This makes
> sense, but makes monitoring of Vbat useless, of course. This would
> explain the relatively high Vbat value. CR2032 batteries are rated at
> 3.0V, not 3.3V.
>
>> Finally, for good measure I ran "depmod -a" and rebooted the machine.
>> The
>> 'updated' dme1737.ko module is correctly loaded on reboot.
>>
>> Dec 11 00:59:00 anas-01 kernel: [   24.137243] dme1737 dme1737.2160:
>> Found
>> a SCH5127 chip at 0x0870
>
> Strange. The address is different from the one sensors-detect reported
> (0x800.)
>
>> Dec 11 00:59:00 anas-01 kernel: [   24.167411] dme1737 dme1737.2160:
>> Optional features: pwm3=yes, pwm5=no, pwm6=no, fan3=yes, fan4=no,
>> fan5=no,
>> fan6=no.
>
> --
> 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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (21 preceding siblings ...)
  2010-12-13  4:02 ` Jeff Rickman
@ 2010-12-13  4:18 ` Jeff Rickman
  2010-12-15 10:04 ` Juerg Haefliger
                   ` (7 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-12-13  4:18 UTC (permalink / raw)
  To: lm-sensors

Hi Jean,

> On Sat, 11 Dec 2010 04:16:07 -0600 (CST), Jeff Rickman wrote:
>> Please look at the "min" and "max" values for "in7". They look reversed.
>>
>> [root@anas-01 ~]# sensors
>> sch5127-isa-0870
>> Adapter: ISA adapter
>> in0:         +1.78 V  (min =  +0.00 V, max =  +3.32 V)
>> Vccp:        +1.19 V  (min =  +1.08 V, max =  +1.32 V)
>> VCC:         +3.31 V  (min =  +2.97 V, max =  +3.63 V)
>> in3:         +1.13 V  (min =  +0.00 V, max =  +1.49 V)
>> in4:         +1.48 V  (min =  +0.00 V, max =  +1.49 V)
>> VTR:         +3.33 V  (min =  +2.97 V, max =  +3.63 V)
>> VBAT:        +3.24 V  (min =  +2.70 V, max =  +3.30 V)
>> in7:         +0.72 V  (min =  +1.99 V, max =  +1.75 V)
>> Side Fan:   1863 RPM  (min =  700 RPM)
>> MCH Fan:    5023 RPM  (min = 4500 RPM)
>> CPU Temp:    +39.9°C  (low  = +20.0°C, high = +60.0°C)
>> SYS Temp:    +32.4°C  (low  = +20.0°C, high = +60.0°C)
>
> The in7 limit values are probably simply not initialized by the BIOS.
> At least the in7_max value differs from your previous post.
>
> I admit the in7_min limit is surprising, as it didn't change and
> corresponds to the max value (register value 0xff), which would make
> sense for a max limit but not for a min limit. But even more surprising
> is that the alarm flag doesn't show even though the reading is below
> the min limit. There's something fishy here. You should check if you
> can change the limits and if you are able to get the alarm flag to
> trigger.
>
> --
> Jean Delvare
>

I rechecked my "/etc/sensors.d/local.conf" file. There are no labels,
compute lines, or set lines applied to "in7" for this chip. After a system
reboot, then issuing a "sensors -s" and waiting a while, no ALARM flag is
seen for "in7". My "sensors3.conf" file does not have any changes or even
references to "in7" in the "dme1737", "sch311x-", and "sch5027-*"
sections. Just trying to be thorough in checking.... Successive readings
of "sensors" shows the "max" value for "in7" changing from 1.50V to 0.25V;
the "min" value never changes from 1.99V.

Now I add "set" lines for "in7":
  set in7_min 1.5 * 0.90
  set in7_max 1.5 * 1.10

After a "sensors -s", "sensors" very briefly reports "min" and "max"
values set to 1.35 and 1.65 respectively. Then I repeat "sensors" a few
more times to see these values change back to 1.99 and anything from 0.0
to 1.0 respectively. Very odd indeed.




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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (22 preceding siblings ...)
  2010-12-13  4:18 ` Jeff Rickman
@ 2010-12-15 10:04 ` Juerg Haefliger
  2010-12-16  9:22 ` Jean Delvare
                   ` (6 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Juerg Haefliger @ 2010-12-15 10:04 UTC (permalink / raw)
  To: lm-sensors

> Hi Juerg,

Hi Jean,

Thanks for the review. It wasn't really ready for your consumption but
thanks anyways :-)


> On Fri, 10 Dec 2010 10:43:57 +0100, Juerg Haefliger wrote:
>> Attached is a modified driver with support for Vtrip monitoring (1.5V,
>> in7). Could you give it a try and let me know how it goes? The new
>> driver should expose the following new sysfs files: in7_input,
>> in7_min, in7_max, in7_alarm. No user-level scaling is required. The
>> nominal voltage is 1.5V.
>
> The changes look overall good to me, but I have a few comments if you
> are interested:
>
>> --- dme1737.c 2010-08-02 00:11:14.000000000 +0200
>> +++ /home/khali/dme1737.c     2010-12-10 14:39:04.000000000 +0100
>> @@ -75,16 +75,20 @@
>>   * in4   +12V
>>   * in5   VTR   (+3.3V stby)
>>   * in6   Vbat
>> + * in7   Vtrip (sch5127 only)
>>   *
>>   * --------------------------------------------------------------------- */
>>
>> -/* Voltages (in) numbered 0-6 (ix) */
>> -#define      DME1737_REG_IN(ix)              ((ix) < 5 ? 0x20 + (ix) \
>> -                                               : 0x94 + (ix))
>> -#define      DME1737_REG_IN_MIN(ix)          ((ix) < 5 ? 0x44 + (ix) * 2 \
>> -                                               : 0x91 + (ix) * 2)
>> -#define      DME1737_REG_IN_MAX(ix)          ((ix) < 5 ? 0x45 + (ix) * 2 \
>> -                                               : 0x92 + (ix) * 2)
>> +/* Voltages (in) numbered 0-7 (ix) */
>> +#define      DME1737_REG_IN(ix)              ((ix) < 5 ? 0x20 + (ix) : \
>> +                                      (ix) < 7 ? 0x94 + (ix) : \
>> +                                                 0x1f)
>> +#define      DME1737_REG_IN_MIN(ix)          ((ix) < 5 ? 0x44 + (ix) * 2 : \
>> +                                      (ix) < 7 ? 0x91 + (ix) * 2 : \
>> +                                                 0x9f)
>> +#define      DME1737_REG_IN_MAX(ix)          ((ix) < 5 ? 0x45 + (ix) * 2 : \
>> +                                      (ix) < 7 ? 0x92 + (ix) * 2 : \
>> +                                                 0xa0)
>
> Maybe it is time to give up macros and go with const arrays? The macros
> are never called with a constant parameters so the compiler can't
> optimize the calls.

I thought about that too. I keep it for now for the REG_IN for
consistency reasons (since MIN and MAX don't need any modification as
you pointed out in a subsequent email. duh!).


>>
>>  /* Temperatures (temp) numbered 0-2 (ix) */
>>  #define DME1737_REG_TEMP(ix)         (0x25 + (ix))
>> @@ -99,10 +103,11 @@
>>   *    IN_TEMP_LSB(1) = [temp3, temp1]
>>   *    IN_TEMP_LSB(2) = [in4, temp2]
>>   *    IN_TEMP_LSB(3) = [in3, in0]
>> - *    IN_TEMP_LSB(4) = [in2, in1] */
>> + *    IN_TEMP_LSB(4) = [in2, in1]
>> + *    IN_TEMP_LSB(5) = [res, in7] */
>>  #define DME1737_REG_IN_TEMP_LSB(ix)  (0x84 + (ix))
>> -static const u8 DME1737_REG_IN_LSB[] = {3, 4, 4, 3, 2, 0, 0};
>> -static const u8 DME1737_REG_IN_LSB_SHL[] = {4, 4, 0, 0, 0, 0, 4};
>> +static const u8 DME1737_REG_IN_LSB[] = {3, 4, 4, 3, 2, 0, 0, 5};
>> +static const u8 DME1737_REG_IN_LSB_SHL[] = {4, 4, 0, 0, 0, 0, 4, 4};
>>  static const u8 DME1737_REG_TEMP_LSB[] = {1, 2, 1};
>>  static const u8 DME1737_REG_TEMP_LSB_SHL[] = {4, 4, 0};
>>
>> @@ -143,7 +148,7 @@
>>  #define DME1737_REG_ALARM1           0x41
>>  #define DME1737_REG_ALARM2           0x42
>>  #define DME1737_REG_ALARM3           0x83
>> -static const u8 DME1737_BIT_ALARM_IN[] = {0, 1, 2, 3, 8, 16, 17};
>> +static const u8 DME1737_BIT_ALARM_IN[] = {0, 1, 2, 3, 8, 16, 17, 18};
>>  static const u8 DME1737_BIT_ALARM_TEMP[] = {4, 5, 6};
>>  static const u8 DME1737_BIT_ALARM_FAN[] = {10, 11, 12, 13, 22, 23};
>>
>> @@ -188,6 +193,7 @@
>>  #define HAS_PWM_MIN          (1 << 4)                /* bit 4 */
>>  #define HAS_FAN(ix)          (1 << ((ix) + 5))       /* bits 5-10 */
>>  #define HAS_PWM(ix)          (1 << ((ix) + 11))      /* bits 11-16 */
>> +#define HAS_IN7                      (1 << 17)               /* bit 17 */
>>
>>  /* ---------------------------------------------------------------------
>>   * Data structures and manipulation thereof
>> @@ -245,7 +251,7 @@
>>  static const int IN_NOMINAL_SCH5027[] = {5000, 2250, 3300, 1125, 1125, 3300,
>>                                        3300};
>>  static const int IN_NOMINAL_SCH5127[] = {2500, 2250, 3300, 1125, 1125, 3300,
>> -                                      3300};
>> +                                      3300, 1500};
>>  #define IN_NOMINAL(type)     ((type) = sch311x ? IN_NOMINAL_SCH311x : \
>>                                (type) = sch5027 ? IN_NOMINAL_SCH5027 : \
>>                                (type) = sch5127 ? IN_NOMINAL_SCH5127 : \
>> @@ -578,7 +584,7 @@
>>  {
>>       struct dme1737_data *data = dev_get_drvdata(dev);
>>       int ix;
>> -     u8 lsb[5];
>> +     u8 lsb[6];
>>
>>       mutex_lock(&data->update_lock);
>>
>> @@ -601,12 +607,14 @@
>>                       /* Voltage inputs are stored as 16 bit values even
>>                        * though they have only 12 bits resolution. This is
>>                        * to make it consistent with the temp inputs. */
>> -                     data->in[ix] = dme1737_read(data,
>> +                     if (ix != 7 || (data->has_features & HAS_IN7)) {
>
> It is less intrusive to do the opposite test:
>
>                        if (ix = 7 && !(data->has_features & HAS_IN7))
>                                continue;

Agreed.


> This avoids reindenting the whole block.
>
>> +                             data->in[ix] = dme1737_read(data,
>>                                       DME1737_REG_IN(ix)) << 8;
>> -                     data->in_min[ix] = dme1737_read(data,
>> +                             data->in_min[ix] = dme1737_read(data,
>>                                       DME1737_REG_IN_MIN(ix));
>> -                     data->in_max[ix] = dme1737_read(data,
>> +                             data->in_max[ix] = dme1737_read(data,
>>                                       DME1737_REG_IN_MAX(ix));
>> +                     }
>>               }
>>
>>               /* Temp registers */
>> @@ -633,12 +641,16 @@
>>                * which the registers are read (MSB first, then LSB) is
>>                * important! */
>>               for (ix = 0; ix < ARRAY_SIZE(lsb); ix++) {
>> -                     lsb[ix] = dme1737_read(data,
>> +                     if (ix != 5 || (data->has_features & HAS_IN7)) {
>> +                             lsb[ix] = dme1737_read(data,
>>                                       DME1737_REG_IN_TEMP_LSB(ix));
>> +                     }
>>               }
>>               for (ix = 0; ix < ARRAY_SIZE(data->in); ix++) {
>> -                     data->in[ix] |= (lsb[DME1737_REG_IN_LSB[ix]] <<
>> +                     if (ix != 7 || (data->has_features & HAS_IN7)) {
>> +                             data->in[ix] |= (lsb[DME1737_REG_IN_LSB[ix]] <<
>>                                       DME1737_REG_IN_LSB_SHL[ix]) & 0xf0;
>> +                     }
>>               }
>>               for (ix = 0; ix < ARRAY_SIZE(data->temp); ix++) {
>>                       data->temp[ix] |= (lsb[DME1737_REG_TEMP_LSB[ix]] <<
>> @@ -760,7 +772,7 @@
>>
>>  /* ---------------------------------------------------------------------
>>   * Voltage sysfs attributes
>> - * ix = [0-5]
>> + * ix = [0-6]
>
> Shouldn't this be [0-7]?


Yes. And increasing the actual in[] array in the 'data' structure
might be a good thing too to prevent array overruns :-) Duh! No wonder
Jeff's min/max values behave weird.



>>   * --------------------------------------------------------------------- */
>>
>>  #define SYS_IN_INPUT 0
>> @@ -1437,7 +1449,7 @@
>>   * Sysfs device attribute defines and structs
>>   * --------------------------------------------------------------------- */
>>
>> -/* Voltages 0-6 */
>> +/* Voltages 0-7 */
>>
>>  #define SENSOR_DEVICE_ATTR_IN(ix) \
>>  static SENSOR_DEVICE_ATTR_2(in##ix##_input, S_IRUGO, \
>> @@ -1456,6 +1468,7 @@
>>  SENSOR_DEVICE_ATTR_IN(4);
>>  SENSOR_DEVICE_ATTR_IN(5);
>>  SENSOR_DEVICE_ATTR_IN(6);
>> +SENSOR_DEVICE_ATTR_IN(7);
>>
>>  /* Temperatures 1-3 */
>>
>> @@ -1678,8 +1691,7 @@
>>       .attrs = dme1737_zone3_attr,
>>  };
>>
>> -
>> -/* The following struct holds temp zone hysteresis  related attributes, which
>> +/* The following struct holds temp zone hysteresis related attributes, which
>
> Cleanups are preferred as separate patches (makes backporting easier.)

OK. Will send a separate patch.


>>   * are not available in all chips. The following chips support them:
>>   * DME1737, SCH311x */
>>  static struct attribute *dme1737_zone_hyst_attr[] = {
>> @@ -1693,6 +1705,21 @@
>>       .attrs = dme1737_zone_hyst_attr,
>>  };
>>
>> +/* The following struct holds voltage in7 related attributes, which
>> + * are not available in all chips. The following chips support them:
>> + * SCH5127 */
>> +static struct attribute *dme1737_in7_attr[] = {
>> +     &sensor_dev_attr_in7_input.dev_attr.attr,
>> +     &sensor_dev_attr_in7_min.dev_attr.attr,
>> +     &sensor_dev_attr_in7_max.dev_attr.attr,
>> +     &sensor_dev_attr_in7_alarm.dev_attr.attr,
>> +     NULL
>> +};
>> +
>> +static const struct attribute_group dme1737_in7_group = {
>> +     .attrs = dme1737_in7_attr,
>> +};
>> +
>>  /* The following structs hold the PWM attributes, some of which are optional.
>>   * Their creation depends on the chip configuration which is determined during
>>   * module load. */
>> @@ -1984,6 +2011,9 @@
>>       if (data->has_features & HAS_ZONE_HYST) {
>>               sysfs_remove_group(&dev->kobj, &dme1737_zone_hyst_group);
>>       }
>> +     if (data->has_features & HAS_IN7) {
>> +             sysfs_remove_group(&dev->kobj, &dme1737_in7_group);
>> +     }
>>       sysfs_remove_group(&dev->kobj, &dme1737_group);
>>
>>       if (!data->client) {
>> @@ -2028,6 +2058,11 @@
>>                                     &dme1737_zone_hyst_group))) {
>>               goto exit_remove;
>>       }
>> +     if ((data->has_features & HAS_IN7) &&
>> +         (err = sysfs_create_group(&dev->kobj,
>> +                                   &dme1737_in7_group))) {
>> +             goto exit_remove;
>> +     }
>
> checkpatch.pl will complain. It wants:
>
>        if (data->has_features & HAS_IN7) {
>                err = sysfs_create_group(&dev->kobj, &dme1737_in7_group);
>                if (err)
>                        goto exit_remove;
>        }

OK. Assignments in if statements are being used all over the place in
this driver. I guess I should clean those up, shouldn't I?


>>
>>       /* Create fan sysfs attributes */
>>       for (ix = 0; ix < ARRAY_SIZE(dme1737_fan_group); ix++) {
>> @@ -2186,7 +2221,7 @@
>>               data->has_features |= HAS_ZONE3;
>>               break;
>>       case sch5127:
>> -             data->has_features |= HAS_FAN(2) | HAS_PWM(2);
>> +             data->has_features |= HAS_FAN(2) | HAS_PWM(2) | HAS_IN7;
>>               break;
>>       default:
>>               break;
>>
>
> Thanks,
> --
> Jean Delvare


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

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

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (23 preceding siblings ...)
  2010-12-15 10:04 ` Juerg Haefliger
@ 2010-12-16  9:22 ` Jean Delvare
  2010-12-16  9:46 ` Jean Delvare
                   ` (5 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-16  9:22 UTC (permalink / raw)
  To: lm-sensors

On Sun, 12 Dec 2010 22:02:13 -0600 (CST), Jeff Rickman wrote:
> > Close enough? I wouldn't say that. Which value does the BIOS report
> > exactly?
> 
> During 1 test run I kept the system "in the BIOS" on the "Hardware
> Monitor" screen for almost an hour. The voltages never changed. I suspect
> the vendor isn't reporting any measured voltages here; could be "static"
> voltage values. As for the "V+1.5" value in the BIOS, it never changed
> from "1.50". Never. Only the System Fan Speed (LM_Sensors 'Fan1'), CPU
> Temp, and SYS Temp values ever change on this screen.
> 
> > BTW, you never clearly reported all exact voltage values reported by the
> > BIOS. This would help a lot.
> 
> As displayed "in order" from the "Hardware Monitor" screen:
> 
> V+1.5 = 1.50 V
> 5VTR  = 4.90 V
> VBAT  = 3.20 V
> V+5   = 5 V
> Vccp  = 1.20 V
> VCC   = 3.30 V
> VTR   = 3.30 V

Thanks.

> > If you stay in the BIOS long enough to spot more than one value for
> > each voltage, this can help deduce scaling factors.
> 
> As I write this email now, none of the voltages ever change, not even by
> 1/100 (0.01) of a Volt while sitting in the BIOS for almost an hour. I
> hardly believe any power supply or system power rails are that stable,
> based on experiences with other PC-style computers.

I would tend to agree. It could be that the BIOS reads the voltages
once when you enter the screen and doesn't update them after that. But
most probably they are rounding the value to 0.1 V: don't you find it
highly suspicious that the second decimal place is 0 for all readings?
With a 0.1 V resolution, it isn't all that surprising that the values
never change (and if they did, we couldn't get any useful information
out of that anyway.)

> > (...)
> > 1.5 V is quite different from 1.44 V, so either it's not in7, or the
> > scaling factor of 2 isn't correct.
> 
> 1.44V is within +- 10 percent of 1.5V, but I agree that improvement is
> needed; changing scaling values in LM_Sensors is a "math problem".

Well, now that you have pointed out how suspicious the BIOS values
look, I agree that there is no point in trying to match them exactly in
"sensors".

> > Anyway, it beats me why the manufacturer would scale down a voltage
> > which it could monitor directly. Very odd.
> 
> This entire Acer system is "very odd".
> 
> Why would a vendor set the system fan to ~760 rpm when HDD temperatures at
> that level almost reach 50C and before any serious system load is applied?
> 50C is a dangerous threshold for most HDD.

My HDD is currently running at 31°C and has never exceeded this
temperature :) But this is a low-power model and mounted in a Zalman
heatpipe enclosure.

> Thus I use "fancontrol" to
> override that automatic setting and push fan speed to "full" to bring HDD
> temps down to <@C. If there is a better method than "fancontrol" that I
> can call from "rc.local" (like "echo 'value' > 'someplace in system'"), I
> would appreciate it since there is no BIOS control for fan speed at all:
> it's based on a "Phoenix TrustedCore" BIOS / Intel Shelton Reference BIOS.

The SCH5127 has an automatic fan speed control mode which you should be
able to setup by writing the proper values to sysfs. Should be
equivalent to "fancontrol" but more reactive and less CPU-intensive.

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (24 preceding siblings ...)
  2010-12-16  9:22 ` Jean Delvare
@ 2010-12-16  9:46 ` Jean Delvare
  2010-12-16  9:51 ` Jean Delvare
                   ` (4 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-16  9:46 UTC (permalink / raw)
  To: lm-sensors

On Sun, 12 Dec 2010 22:18:44 -0600 (CST), Jeff Rickman wrote:
> I rechecked my "/etc/sensors.d/local.conf" file. There are no labels,
> compute lines, or set lines applied to "in7" for this chip. After a system
> reboot, then issuing a "sensors -s" and waiting a while, no ALARM flag is
> seen for "in7". My "sensors3.conf" file does not have any changes or even
> references to "in7" in the "dme1737", "sch311x-", and "sch5027-*"
> sections. Just trying to be thorough in checking.... Successive readings
> of "sensors" shows the "max" value for "in7" changing from 1.50V to 0.25V;
> the "min" value never changes from 1.99V.
> 
> Now I add "set" lines for "in7":
>   set in7_min 1.5 * 0.90
>   set in7_max 1.5 * 1.10
> 
> After a "sensors -s", "sensors" very briefly reports "min" and "max"
> values set to 1.35 and 1.65 respectively. Then I repeat "sensors" a few
> more times to see these values change back to 1.99 and anything from 0.0
> to 1.0 respectively. Very odd indeed.

Hmm. So the limit registers aren't behaving as they should. This
suggests that Juerg's patch is not correct. Juerg, can you please
double-check what the datasheet says for registers 0x9f and 0xa0? Could
these be read-only in certain conditions, maybe? And are they really
in7 limits to start with? The value of 0xa0 changing on its own doesn't
make it look so.

Anyway, the value of 0.72 V doesn't make much sense if the line is
supposed to monitor +1.5, as it could do so without scaling. Seeing how
odd the whole system seems to be, I would no longer be surprised if
they really connected +1.5V to in4...

And I take not to avoid Acer hardware. Jeff, I am afraid that we won't
be able to go much farther with this. The only way to get proper
voltage handling would be to ask Acer for the pin wiring of the voltage
monitoring inputs together with the resistor values they used for
scaling. Without that we can only try and guess things, and get them
wrong.

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (25 preceding siblings ...)
  2010-12-16  9:46 ` Jean Delvare
@ 2010-12-16  9:51 ` Jean Delvare
  2010-12-17 18:52 ` Jeff Rickman
                   ` (3 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-16  9:51 UTC (permalink / raw)
  To: lm-sensors

On Wed, 15 Dec 2010 11:04:15 +0100, Juerg Haefliger wrote:
> > On Fri, 10 Dec 2010 10:43:57 +0100, Juerg Haefliger wrote:
> >> --- dme1737.c 2010-08-02 00:11:14.000000000 +0200
> >> +++ /home/khali/dme1737.c     2010-12-10 14:39:04.000000000 +0100
> >> @@ -75,16 +75,20 @@
> >>   * in4   +12V
> >>   * in5   VTR   (+3.3V stby)
> >>   * in6   Vbat
> >> + * in7   Vtrip (sch5127 only)
> >>   *
> >>   * --------------------------------------------------------------------- */
> >>
> >> -/* Voltages (in) numbered 0-6 (ix) */
> >> -#define      DME1737_REG_IN(ix)              ((ix) < 5 ? 0x20 + (ix) \
> >> -                                               : 0x94 + (ix))
> >> -#define      DME1737_REG_IN_MIN(ix)          ((ix) < 5 ? 0x44 + (ix) * 2 \
> >> -                                               : 0x91 + (ix) * 2)
> >> -#define      DME1737_REG_IN_MAX(ix)          ((ix) < 5 ? 0x45 + (ix) * 2 \
> >> -                                               : 0x92 + (ix) * 2)
> >> +/* Voltages (in) numbered 0-7 (ix) */
> >> +#define      DME1737_REG_IN(ix)              ((ix) < 5 ? 0x20 + (ix) : \
> >> +                                      (ix) < 7 ? 0x94 + (ix) : \
> >> +                                                 0x1f)
> >> +#define      DME1737_REG_IN_MIN(ix)          ((ix) < 5 ? 0x44 + (ix) * 2 : \
> >> +                                      (ix) < 7 ? 0x91 + (ix) * 2 : \
> >> +                                                 0x9f)
> >> +#define      DME1737_REG_IN_MAX(ix)          ((ix) < 5 ? 0x45 + (ix) * 2 : \
> >> +                                      (ix) < 7 ? 0x92 + (ix) * 2 : \
> >> +                                                 0xa0)
> >
> > Maybe it is time to give up macros and go with const arrays? The macros
> > are never called with a constant parameters so the compiler can't
> > optimize the calls.
> 
> I thought about that too. I keep it for now for the REG_IN for
> consistency reasons (since MIN and MAX don't need any modification as
> you pointed out in a subsequent email. duh!).

Agreed.

> > (...)
> > Shouldn't this be [0-7]?
> 
> Yes. And increasing the actual in[] array in the 'data' structure
> might be a good thing too to prevent array overruns :-) Duh! No wonder
> Jeff's min/max values behave weird.

D'oh, how did I manage to miss that in my review :(

Jeff, please search for "struct dme1737_data" in the source code, and
change:

	u16 in[7];
	u8  in_min[7];
	u8  in_max[7];

to

	u16 in[8];
	u8  in_min[8];
	u8  in_max[8];

then rebuild the driver and try again!

> > (...)
> > checkpatch.pl will complain. It wants:
> >
> >        if (data->has_features & HAS_IN7) {
> >                err = sysfs_create_group(&dev->kobj, &dme1737_in7_group);
> >                if (err)
> >                        goto exit_remove;
> >        }
> 
> OK. Assignments in if statements are being used all over the place in
> this driver. I guess I should clean those up, shouldn't I?

If you have time, yes. OTOH there are some cases where it actually
makes the code a lot more complex so I am not always following
checkpatch.pl on this myself. So you might as well keep your code as is
if it is consistent throughout the driver.

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (26 preceding siblings ...)
  2010-12-16  9:51 ` Jean Delvare
@ 2010-12-17 18:52 ` Jeff Rickman
  2010-12-17 19:04 ` Jeff Rickman
                   ` (2 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-12-17 18:52 UTC (permalink / raw)
  To: lm-sensors

Hi Jean,

> On Sun, 12 Dec 2010 22:18:44 -0600 (CST), Jeff Rickman wrote:
>> I rechecked my "/etc/sensors.d/local.conf" file. There are no labels,
>> compute lines, or set lines applied to "in7" for this chip. After a
>> system
>> reboot, then issuing a "sensors -s" and waiting a while, no ALARM flag
>> is
>> seen for "in7". My "sensors3.conf" file does not have any changes or
>> even
>> references to "in7" in the "dme1737", "sch311x-", and "sch5027-*"
>> sections. Just trying to be thorough in checking.... Successive readings
>> of "sensors" shows the "max" value for "in7" changing from 1.50V to
>> 0.25V;
>> the "min" value never changes from 1.99V.
>>
>> Now I add "set" lines for "in7":
>>   set in7_min 1.5 * 0.90
>>   set in7_max 1.5 * 1.10
>>
>> After a "sensors -s", "sensors" very briefly reports "min" and "max"
>> values set to 1.35 and 1.65 respectively. Then I repeat "sensors" a few
>> more times to see these values change back to 1.99 and anything from 0.0
>> to 1.0 respectively. Very odd indeed.
>
> Hmm. So the limit registers aren't behaving as they should. This
> suggests that Juerg's patch is not correct. Juerg, can you please
> double-check what the datasheet says for registers 0x9f and 0xa0? Could
> these be read-only in certain conditions, maybe? And are they really
> in7 limits to start with? The value of 0xa0 changing on its own doesn't
> make it look so.
>
> Anyway, the value of 0.72 V doesn't make much sense if the line is
> supposed to monitor +1.5, as it could do so without scaling. Seeing how
> odd the whole system seems to be, I would no longer be surprised if
> they really connected +1.5V to in4...
>
> And I take not to avoid Acer hardware. Jeff, I am afraid that we won't
> be able to go much farther with this. The only way to get proper
> voltage handling would be to ask Acer for the pin wiring of the voltage
> monitoring inputs together with the resistor values they used for
> scaling. Without that we can only try and guess things, and get them
> wrong.
>

I truly appreciate the effort and support from everyone involved.

The expected voltages on the expected pins seem reasonable. The temps and
fans readings seem correct compared to the BIOS. Even the PWM fan function
works! I would call that a respectable achievement.

I would say the effort to get SCH5127 code working in LM_Sensors was very
good, especially since Juerg did not have access to this specific hardware
platform for testing. Should someone else encounter the SCH5127 chip in
the future, this effort will benefit them.

> --
> 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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (27 preceding siblings ...)
  2010-12-17 18:52 ` Jeff Rickman
@ 2010-12-17 19:04 ` Jeff Rickman
  2010-12-17 19:42 ` Jean Delvare
  2010-12-17 19:46 ` Jean Delvare
  30 siblings, 0 replies; 32+ messages in thread
From: Jeff Rickman @ 2010-12-17 19:04 UTC (permalink / raw)
  To: lm-sensors

Hi Jean,

> On Wed, 15 Dec 2010 11:04:15 +0100, Juerg Haefliger wrote:
>> > On Fri, 10 Dec 2010 10:43:57 +0100, Juerg Haefliger wrote:
>> >> --- dme1737.c 2010-08-02 00:11:14.000000000 +0200
>> >> +++ /home/khali/dme1737.c     2010-12-10 14:39:04.000000000 +0100
>> >> @@ -75,16 +75,20 @@
>> >>   * in4   +12V
>> >>   * in5   VTR   (+3.3V stby)
>> >>   * in6   Vbat
>> >> + * in7   Vtrip (sch5127 only)
>> >>   *
>> >>   *
>> ---------------------------------------------------------------------
>> */
>> >>
>> >> -/* Voltages (in) numbered 0-6 (ix) */
>> >> -#define      DME1737_REG_IN(ix)              ((ix) < 5 ? 0x20 + (ix)
>> \
>> >> -                                               : 0x94 + (ix))
>> >> -#define      DME1737_REG_IN_MIN(ix)          ((ix) < 5 ? 0x44 + (ix)
>> * 2 \
>> >> -                                               : 0x91 + (ix) * 2)
>> >> -#define      DME1737_REG_IN_MAX(ix)          ((ix) < 5 ? 0x45 + (ix)
>> * 2 \
>> >> -                                               : 0x92 + (ix) * 2)
>> >> +/* Voltages (in) numbered 0-7 (ix) */
>> >> +#define      DME1737_REG_IN(ix)              ((ix) < 5 ? 0x20 + (ix)
>> : \
>> >> +                                      (ix) < 7 ? 0x94 + (ix) : \
>> >> +                                                 0x1f)
>> >> +#define      DME1737_REG_IN_MIN(ix)          ((ix) < 5 ? 0x44 + (ix)
>> * 2 : \
>> >> +                                      (ix) < 7 ? 0x91 + (ix) * 2 : \
>> >> +                                                 0x9f)
>> >> +#define      DME1737_REG_IN_MAX(ix)          ((ix) < 5 ? 0x45 + (ix)
>> * 2 : \
>> >> +                                      (ix) < 7 ? 0x92 + (ix) * 2 : \
>> >> +                                                 0xa0)
>> >
>> > Maybe it is time to give up macros and go with const arrays? The
>> macros
>> > are never called with a constant parameters so the compiler can't
>> > optimize the calls.
>>
>> I thought about that too. I keep it for now for the REG_IN for
>> consistency reasons (since MIN and MAX don't need any modification as
>> you pointed out in a subsequent email. duh!).
>
> Agreed.
>
>> > (...)
>> > Shouldn't this be [0-7]?
>>
>> Yes. And increasing the actual in[] array in the 'data' structure
>> might be a good thing too to prevent array overruns :-) Duh! No wonder
>> Jeff's min/max values behave weird.
>
> D'oh, how did I manage to miss that in my review :(
>
> Jeff, please search for "struct dme1737_data" in the source code, and
> change:
>
> 	u16 in[7];
> 	u8  in_min[7];
> 	u8  in_max[7];
>
> to
>
> 	u16 in[8];
> 	u8  in_min[8];
> 	u8  in_max[8];
>
> then rebuild the driver and try again!
>

Changes made. Recompile successful. Old module removed. Module installs
without error. Output as follows:

[root@anas-01 sensors]# sensors
sch5127-isa-0870
Adapter: ISA adapter
in0:         +1.78 V  (min =  +0.00 V, max =  +3.32 V)
Vccp:        +1.19 V  (min =  +1.08 V, max =  +1.32 V)
VCC:         +3.31 V  (min =  +2.97 V, max =  +3.63 V)
in3:         +1.13 V  (min =  +0.00 V, max =  +1.49 V)
in4:         +1.48 V  (min =  +0.00 V, max =  +1.49 V)
VTR:         +3.33 V  (min =  +2.97 V, max =  +3.63 V)
VBAT:        +3.25 V  (min =  +2.70 V, max =  +3.30 V)
in7:         +1.43 V  (min =  +1.35 V, max =  +1.65 V)
Side Fan:   1905 RPM  (min =  700 RPM)
MCH Fan:    5032 RPM  (min = 4500 RPM)
CPU Temp:    +41.8°C  (low  = +20.0°C, high = +60.0°C)
SYS Temp:    +32.9°C  (low  = +20.0°C, high = +60.0°C)

1.43 volts isn't 1.5 volts, but within +- 10 percent.

>> > (...)
>> > checkpatch.pl will complain. It wants:
>> >
>> >        if (data->has_features & HAS_IN7) {
>> >                err = sysfs_create_group(&dev->kobj,
>> &dme1737_in7_group);
>> >                if (err)
>> >                        goto exit_remove;
>> >        }
>>
>> OK. Assignments in if statements are being used all over the place in
>> this driver. I guess I should clean those up, shouldn't I?
>
> If you have time, yes. OTOH there are some cases where it actually
> makes the code a lot more complex so I am not always following
> checkpatch.pl on this myself. So you might as well keep your code as is
> if it is consistent throughout the driver.
>
> --
> 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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (28 preceding siblings ...)
  2010-12-17 19:04 ` Jeff Rickman
@ 2010-12-17 19:42 ` Jean Delvare
  2010-12-17 19:46 ` Jean Delvare
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-17 19:42 UTC (permalink / raw)
  To: lm-sensors

On Fri, 17 Dec 2010 12:43:35 -0600 (CST), Jeff Rickman wrote:
> > The SCH5127 has an automatic fan speed control mode which you should be
> > able to setup by writing the proper values to sysfs. Should be
> > equivalent to "fancontrol" but more reactive and less CPU-intensive.
> 
> I studied the sysfs entries carefully while using fancontrol
> and not using fancontrol. I decided to do the following:
> 
> echo 1 > /sys/devices/platform/dme1737.2160/pwm1_enable
> echo 255 > /sys/devices/platform/dme1737.2160/pwm1
> 
> Works fine.

This puts the fan to full speed (100% duty cycle.) This is equivalent
to, but presumably less efficient than:

$ echo 0 > /sys/devices/platform/dme1737.2160/pwm1_enable

If the noise is bearable then you're done indeed. But what I actually
had in mind was setting to automatic mode:

$ echo 2 > /sys/devices/platform/dme1737.2160/pwm1_enable

And then fine-tuning the pwm1_auto_channels_zone,
zone*_auto_channels_temp and zone*_auto_point*_temp values. This would
let you tune the fan speeds from medium at idle to full speed under load
(best noise/temperature trade-off.) If you try that, you'll have to
read the documentation in Documentation/hwmon/dme1737.

-- 
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] 32+ messages in thread

* Re: [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer
  2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
                   ` (29 preceding siblings ...)
  2010-12-17 19:42 ` Jean Delvare
@ 2010-12-17 19:46 ` Jean Delvare
  30 siblings, 0 replies; 32+ messages in thread
From: Jean Delvare @ 2010-12-17 19:46 UTC (permalink / raw)
  To: lm-sensors

On Fri, 17 Dec 2010 13:04:53 -0600 (CST), Jeff Rickman wrote:
> Changes made. Recompile successful. Old module removed. Module installs
> without error. Output as follows:
> 
> [root@anas-01 sensors]# sensors
> sch5127-isa-0870
> Adapter: ISA adapter
> in0:         +1.78 V  (min =  +0.00 V, max =  +3.32 V)
> Vccp:        +1.19 V  (min =  +1.08 V, max =  +1.32 V)
> VCC:         +3.31 V  (min =  +2.97 V, max =  +3.63 V)
> in3:         +1.13 V  (min =  +0.00 V, max =  +1.49 V)
> in4:         +1.48 V  (min =  +0.00 V, max =  +1.49 V)
> VTR:         +3.33 V  (min =  +2.97 V, max =  +3.63 V)
> VBAT:        +3.25 V  (min =  +2.70 V, max =  +3.30 V)
> in7:         +1.43 V  (min =  +1.35 V, max =  +1.65 V)
> Side Fan:   1905 RPM  (min =  700 RPM)
> MCH Fan:    5032 RPM  (min = 4500 RPM)
> CPU Temp:    +41.8°C  (low  = +20.0°C, high = +60.0°C)
> SYS Temp:    +32.9°C  (low  = +20.0°C, high = +60.0°C)
> 
> 1.43 volts isn't 1.5 volts, but within +- 10 percent.

Indeed, much better! Thanks for testing and reporting! Now, I can merge
Juerg's patch.

I think we can conclude that in7 is +1.5V on your system, and in0 and
in3 would be +5V and 5VSB (or vice-versa) with unknown scaling factors.

-- 
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] 32+ messages in thread

end of thread, other threads:[~2010-12-17 19:46 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-29  6:20 [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer easyStore Jeff Rickman
2010-11-29  8:05 ` [lm-sensors] Testing LM-Sensor Support of SCH5127 in Acer Juerg Haefliger
2010-11-29  9:41 ` Jeff Rickman
2010-11-29  9:48 ` Jean Delvare
2010-11-29 10:23 ` Jean Delvare
2010-11-29 15:51 ` Jeff Rickman
2010-11-29 16:00 ` Jean Delvare
2010-12-03 21:16 ` Jeff Rickman
2010-12-03 23:27 ` Jean Delvare
2010-12-09 11:18 ` Juerg Haefliger
2010-12-09 11:31 ` Juerg Haefliger
2010-12-09 12:57 ` Jean Delvare
2010-12-09 13:46 ` Juerg Haefliger
2010-12-09 16:09 ` Jean Delvare
2010-12-10  9:43 ` Juerg Haefliger
2010-12-10 14:27 ` Jean Delvare
2010-12-11  7:02 ` Jeff Rickman
2010-12-11 10:16 ` Jeff Rickman
2010-12-12 18:03 ` Jean Delvare
2010-12-12 20:57 ` Jean Delvare
2010-12-12 21:05 ` Jean Delvare
2010-12-12 21:22 ` Jean Delvare
2010-12-13  4:02 ` Jeff Rickman
2010-12-13  4:18 ` Jeff Rickman
2010-12-15 10:04 ` Juerg Haefliger
2010-12-16  9:22 ` Jean Delvare
2010-12-16  9:46 ` Jean Delvare
2010-12-16  9:51 ` Jean Delvare
2010-12-17 18:52 ` Jeff Rickman
2010-12-17 19:04 ` Jeff Rickman
2010-12-17 19:42 ` Jean Delvare
2010-12-17 19:46 ` Jean Delvare

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.