device tree support for adm1278

[shenki]

Following from the discussion in https://github.com/openbmc/openbmc/pull/212#issuecomment-199149334, we need extra features not present in the upstream driver:

• device tree probing https://github.com/shenki/linux/commit/5fbf374af9fc88bc15a21cde9055f2503c43ab50
• support sense resistor https://github.com/shenki/linux/commit/dd13a2c59d1221f8dd7ca3eb61b587a652c32d8f
• add to Barreleye device tree https://github.com/shenki/linux/commit/f24385a1c35f94da042c0b75046efce983df0f38

@adamliyi can you please review these changes for me, and if you are happy with them, I will add them to our kernel tree

[shenki]

[   13.450000] adm1275 4-0010: using r_sense from dt 500
[   13.520000] adm1275 5-0010: using r_sense from dt 500
[   13.580000] adm1275 6-0010: using r_sense from dt 500

# sensors 
adm1278-i2c-4-10
Adapter: Aspeed i2c-4
vin:         +12.07 V  (min =  +0.00 V, max = +20.89 V)
                       (highest = +12.11 V)
pin:           0.00 W  (highest = -184.00 uW, max = -117.00 uW)
iout1:        +0.00 A  (max =  +0.05 A, highest =  +0.00 A)

adm1278-i2c-5-10
Adapter: Aspeed i2c-5
vin:         +12.07 V  (min =  +0.00 V, max = +20.89 V)
                       (highest = +12.13 V)
pin:           4.00 uW (highest = 341.00 uW, max = -117.00 uW)
iout1:        +0.00 A  (max =  +0.05 A, highest =  +0.00 A)

adm1278-i2c-6-10
Adapter: Aspeed i2c-6
vin:         +12.07 V  (min =  +0.00 V, max = +20.89 V)
                       (highest = +12.07 V)
pin:         -520.00 uW (highest = -697.00 uW, max = -117.00 uW)
iout1:        +0.00 A  (max =  +0.05 A, highest =  +0.00 A)

[adamliyi]

@shenki . I agree this is a better solution. The code looks OK with me. I will do more test on HW (to check the sensors we want are there) today.

[shenki]

@adamliyi I added the three to the device tree that you mentioned in your patches. The output is pasted in the comment above. Are the numbers correct?

[mdmillerii]

The user space framework already supports a scale factor, so that patch could be delayed. Instancing from the device tree is required.

I think a generic scaling framework for all hwmon sensors should be proposed instead of a one off feature for one driver.

[adamliyi]

Hi Joel,

With @mdmillerii 's comments, and I read again the document you mentioned:

(https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface):

    Actual voltage depends on the scaling resistors on the motherboard, as
    recommended in the chip datasheet. This varies by chip and by motherboard.
    Because of this variation, values are generally NOT scaled by the chip driver,
    and must be done by the application. However, some drivers (notably lm87 and
    via686a) do scale, because of internal resistors built into a chip. These
    drivers will output the actual voltage. Rule of thumb: drivers should report
    the voltage values at the "pins" of the chip.

So it looks we using a "scale factor" for the sense resistor in user space make more sense? In obmc's case, we can set the "scale factor" in skeleton.

BTW, the sensor resistor value on Barreleye is 250 microohms.

[shenki]

I have sent the device tree change upstream: http://article.gmane.org/gmane.linux.kernel.hwmon/24

I disagree with upstream; in the case of the device tree we have a well suited mechanism to do scaling in the kernel. However, until we convince them of this, I will back out the change to do the scaling in the device tree and we will scale from userspace.

@adamliyi can you please send a patch to perform the scaling in userspace?

[shenki]

The change is backed out as of https://github.com/openbmc/linux/commit/c2f2e9f1fbeead3f670eb4aa9f142bbbcd20267c and https://github.com/openbmc/linux/releases/tag/openbmc-20160329-2

[adamliyi]

@shenki , Hi Joel, I just found there might be some issue with the upstream adm1278 hwmon driver. Some sensors are missing, e.g, "in2_input". This stands for "Vout" register in adm1278. Customer will need this sensor.

Here is the sensors currently available:

root@barreleye:~# cat /sys/class/hwmon/hwmon3/
curr1_highest         in1_label             power1_alarm
curr1_input           in1_max               power1_input
curr1_label           in1_max_alarm         power1_input_highest
curr1_max             in1_min               power1_label
curr1_max_alarm       in1_min_alarm         power1_max
curr1_reset_history   in1_reset_history     power1_reset_history
device/               name                  subsystem/
in1_highest           of_node/              uevent
in1_input             power/                
root@barreleye:~# cat /sys/class/hwmon/hwmon3/in1_input 
11990
root@barreleye:~# cat /sys/class/hwmon/hwmon3/in1_label  
vin

We will need

/sys/class/hwmon/hwmon3/in2_input

Kernel version:

root@barreleye:~# uname -a
Linux barreleye 4.4.6-openbmc-20160329-2 #1 Wed Apr 20 23:06:07 CST 2016 armv5tejl GNU/Linux
root@barreleye:~# cat /proc/version  
Linux version 4.4.6-openbmc-20160329-2 (adam@ubuntu) (gcc version 4.9.3 (GCC) ) #1 Wed Apr 20 23:06:07 CST 2016

I might need to add another issue to track this problem.

[shenki]

@adamliyi Could you add support to the existing upstream driver for these extra readings?

[adamliyi]

OK. I will dig into it.

[adamliyi]

Sensor resistor value added as a scale value in skeleton. Please see: https://github.com/openbmc/skeleton/pull/57.