[phosphor-pid-control] scaling issue

[Hank Liou (劉晉翰)]

Hi Patrick,

>-----Original Message-----
>From: Patrick Venture [mailto:venture at google.com]
>Sent: Wednesday, August 21, 2019 10:32 PM
>To: Hank Liou (劉晉翰) <Hank.Liou at quantatw.com>; James Feist
><james.feist at linux.intel.com>
>Cc: openbmc at lists.ozlabs.org
>Subject: Re: [phosphor-pid-control] scaling issue
>
>On Wed, Aug 21, 2019 at 1:11 AM Hank Liou (劉晉翰)
><Hank.Liou at quantatw.com> wrote:
>>
>> Hi All,
>>
>>
>> After commit [1], I found my temp sensor reading would be re-scaled by
>multiplying 1 over 255, making temperature into unfamiliar unit. Also the fan
>rpm reading would lie in [0,1] interval, letting the fan input to be 0 (since the
>input value of fan is from an integer array [2]). Are these normal behaviors?
>Or do I miss something?
>
>Are you using dbus configuration or json?  If json, can you attach your config.
>Since you're saying it was working and now isn't, I'm assuming there's
>something about the config being treated differently with the code changes in
>an unexpected way.

I found pid control will first read min and max from dbus and then (before commit [1]) revise them if 

info->min != conf::inheritValueFromDbus (in dbus/dbuspassive.cpp)

After value initialization, the min and max would be the ones in json file (Json file [3]). However, after commit [1] the min and max values of config would not be fed into the fan control process. The scaling issue is originated from commit [4] with the aim to treat fan rpm as percentage. It seems that commit [1] unexpectedly affects temp sensors in the sense that the temp is the integer reading not percentage. Before commit [1], it would not re-scale the temp reading since my min and max are 0 [6].



There is another issue with commit [1]. Now the fan rpm would be something like

1500 / 20000 = 0.075

where rpm max 20000 is from dbus. However the fan input function would transfer double into int, which is 0 for 0.075 (see [5]). Hence the fan input is 0 not percentage. Is there something wrong?

[1] https://github.com/openbmc/phosphor-pid-control/commit/fc2e803f5d9256944e18c7c878a441606b1f121c
[2] https://github.com/openbmc/phosphor-pid-control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/pid/fancontroller.cpp#L86
[3]
       {
            "name": "fan1",
            "type": "fan",
            "readPath": "/sys/class/hwmon/hwmon1/fan1_input",
            "writePath": "/sys/class/hwmon/hwmon1/pwm1",
            "min": 0,
            "max": 255
        },
        {
            "name": "temp1",
            "type": "temp",
            "readPath": "/xyz/openbmc_project/sensors/temperature/temp1",
            "writePath": "",
            "min": 0,
            "max": 0
        }
[4] https://github.com/openbmc/phosphor-pid-control/commit/75eb769d351434547899186f73ff70ae00d7934a
[5] https://github.com/openbmc/phosphor-pid-control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/pid/fancontroller.cpp#L64
[6] https://github.com/openbmc/phosphor-pid-control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/dbus/dbuspassive.cpp#L158

>
>>
>>
>> [1]
>> https://github.com/openbmc/phosphor-pid-
>control/commit/fc2e803f5d92569
>> 44e18c7c878a441606b1f121c
>>
>> [2]
>> https://github.com/openbmc/phosphor-pid-
>control/blob/a7ec8350d17b70153
>> cebe666d3fbe88bddd02a1a/pid/fancontroller.cpp#L86
>>
>>
>> Thanks,
>>
>>
>> Hank Liou
>>
>> Quanta Computer Inc.
>>
>>

Sincerely,
Hank