[phosphor-pid-control] scaling issue
James Feist
james.feist at linux.intel.com
Fri Aug 30 02:27:10 AEST 2019
On 8/29/19 9:21 AM, Patrick Venture wrote:
> On Thu, Aug 29, 2019 at 9:14 AM James Feist <james.feist at linux.intel.com> wrote:
>>
>> On 8/29/19 7:24 AM, Patrick Venture wrote:
>>> 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.
>>>
>>> So, iirc, the min/max in the json is only for sensors that write, and
>>> not read. Sorry, I'm ramping up on a new team and slower to catch up
>>> to emails.
>>>
>>> Yeah, the min/max in the json are for basically converting a
>>> percentage value to a PWM values -- that's its initial goal.
>>> Temperature sensors typically don't have minimum and maximum values in
>>> the code's use-cases. I added James to this because they use the
>>> daemon for other cases -- where perhaps that'll make sense.
>>
>> I believe the history of min/max being removed for passive sensors was
>> here:
>> https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-pid-control/+/23470
>>
>> The only other use case we have for min / max is for fan RPM so that we
>> can create a PID based on Tach %, so that the same PID can be reused on
>> multiple platforms with different sized fans.
>>
>> -James
>>
>>>
>>>>
>>>> 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
>
> Hank, do you need to use the min/max on the temperature sensors? Try
> your build with those entries deleted from the json file (they're
> optional).
>
> James, I think I should add a warning and ignore those fields in that
> case. What do you think? (I honestly thought they were already
> ignored for temperature sensors -- have to look at the code to
> verify).
I don't have much of an opinion here as I don't use the json
configuration, but more warnings never hurt.
>
More information about the openbmc
mailing list