[phosphor-pid-control] scaling issue

[Patrick Venture]

PTAL - https://gerrit.openbmc-project.xyz/24827 - this is merged, and
the srcrev bump should propagate into openbmc/openbmc in a day or two.

On Wed, Aug 28, 2019 at 11:00 PM Hank Liou (劉晉翰) <Hank.Liou at quantatw.com> wrote:
>
> Hi Patrick,
>
> I think it's OK to parse the config min&max for temp sensors.
>
> Any suggestion?
>
> Thanks,
> Hank
>
> >-----Original Message-----
> >From: openbmc [mailto:openbmc-
> >bounces+hank.liou=quantatw.com at lists.ozlabs.org] On Behalf Of Hank Liou
> >(劉晉翰)
> >Sent: Friday, August 23, 2019 4:31 PM
> >To: Patrick Venture <venture at google.com>; James Feist
> ><james.feist at linux.intel.com>
> >Cc: openbmc at lists.ozlabs.org
> >Subject: RE: [phosphor-pid-control] scaling issue
> >
> >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/fancontrolle
> >r.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/fancontrolle
> >r.cpp#L64
> >[6] https://github.com/openbmc/phosphor-pid-
> >control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/dbus/dbuspassi
> >ve.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