
<html>

<head>

<meta http-equiv="Content-Type" content="text/html; charset=big5">

<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>

</head>

<body dir="ltr">

<div id="divtagdefaultwrapper" style="font-size: 12pt; color: rgb(0, 0, 0); font-family: Calibri, Helvetica, sans-serif, EmojiFont, "Apple Color Emoji", "Segoe UI Emoji", NotoColorEmoji, "Segoe UI Symbol", "Android Emoji", EmojiSymbols;" dir="ltr">

<p>Hi Patrick,</p>

<p><br>

</p>

<p>Sorry for not clearly stating our problem. <span style="font-size: 12pt;">We have the following issue:</span></p>

<p><br>

</p>

<p>temp sensor gets 31(C) -> 31 / 0.255 = 121.57 -> pid uses 127.57 as input for temp sensor -></p>

<p>stepwise fan table output 100 duty -> full speed fan</p>

<p><br>

</p>

<p>As a result, fan will be at full speed while temp is low. Before the commit <span>

fc2e803 [1], this won't happen. The root cause is that, before fc2e803, pid will use config min/max, which is 0 in our case. This would not trigger scaling function, namely <span>scaleSensorReading,</span> in util.cpp. However, after such commit, min/max would

 be non-zero (-128/127 from DBus). This will trigger scaling function.</span></p>

<p><br>

</p>

<p>[1] <a href="https://github.com/openbmc/phosphor-pid-control/commit/fc2e803f5d9256944e18c7c878a441606b1f121c" class="OWAAutoLink" id="LPlnk609236" previewremoved="true">https://github.com/openbmc/phosphor-pid-control/commit/fc2e803f5d9256944e18c7c878a441606b1f121c</a></p>

<p><br>

</p>

<p>Hank</p>

<br>

<div style="color: rgb(0, 0, 0);">

<hr tabindex="-1" style="display:inline-block; width:98%">

<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Hank Liou (劉晉翰)<br>

<b>Sent:</b> Monday, September 2, 2019 4:52 PM<br>

<b>To:</b> Patrick Venture<br>

<b>Cc:</b> James Feist; openbmc@lists.ozlabs.org<br>

<b>Subject:</b> Re: [phosphor-pid-control] scaling issue</font>

<div> </div>

</div>

<div>

<div id="divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">

<p>Hi Patrick,</p>

<p><br>

</p>

<p>Since we use phosphor-sel-logger [1] at the same time, we do assign min and max of temp sensors to Dbus (max: 127, min: -128). So i<span style="font-family:Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols; font-size:16px">n

 the present case, our temp value will be divided by 0.255 (also due to </span><span style="font-family:Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols; font-size:16px">exponent

 is -3 here</span><span style="font-family:Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols; font-size:16px">). </span>This will cause re-scaling problem. <span style="font-size:12pt">Therefore

</span><span style="font-size:12pt">there should be an </span><span style="font-size:12pt">statement to distinguish sensor types. If it is "temp", then one assigns 0 to the min and max from Dbus.</span></p>

<p><br>

</p>

<p>[1] <a href="https://github.com/openbmc/phosphor-sel-logger/blob/3d300fca24b30864b3e9a4f5768cfe5e769458ae/include/sensorutils.hpp#L59" class="OWAAutoLink" id="LPlnk606462" previewremoved="true">https://github.com/openbmc/phosphor-sel-logger/blob/3d300fca24b30864b3e9a4f5768cfe5e769458ae/include/sensorutils.hpp#L59</a><a href="https://g" class="OWAAutoLink" id="LPlnk929597" previewremoved="true"></a></p>

<div id="divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">

<br>

</div>

<p>Hank</p>

<p><br>

</p>

<div style="color:rgb(0,0,0)">

<div>

<hr tabindex="-1" style="display:inline-block; width:98%">

<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Patrick Venture <venture@google.com><br>

<b>Sent:</b> Friday, August 30, 2019 1:47 AM<br>

<b>To:</b> Hank Liou (劉晉翰)<br>

<b>Cc:</b> James Feist; openbmc@lists.ozlabs.org<br>

<b>Subject:</b> Re: [phosphor-pid-control] scaling issue</font>

<div> </div>

</div>

</div>

<font size="2"><span style="font-size:10pt">

<div class="PlainText">PTAL - <a href="https://gerrit.openbmc-project.xyz/24827" id="LPlnk833890" previewremoved="true">

https://gerrit.openbmc-project.xyz/24827</a> - this is merged, and<br>

the srcrev bump should propagate into openbmc/openbmc in a day or two.<br>

<br>

On Wed, Aug 28, 2019 at 11:00 PM Hank Liou (劉晉翰) <Hank.Liou@quantatw.com> wrote:<br>

><br>

> Hi Patrick,<br>

><br>

> I think it's OK to parse the config min&max for temp sensors.<br>

><br>

> Any suggestion?<br>

><br>

> Thanks,<br>

> Hank<br>

><br>

> >-----Original Message-----<br>

> >From: openbmc [<a href=""></a>mailto:openbmc-<br>

> >bounces+hank.liou=quantatw.com@lists.ozlabs.org] On Behalf Of Hank Liou<br>

> >(劉晉翰)<br>

> >Sent: Friday, August 23, 2019 4:31 PM<br>

> >To: Patrick Venture <venture@google.com>; James Feist<br>

> ><james.feist@linux.intel.com><br>

> >Cc: openbmc@lists.ozlabs.org<br>

> >Subject: RE: [phosphor-pid-control] scaling issue<br>

> ><br>

> >Hi Patrick,<br>

> ><br>

> >>-----Original Message-----<br>

> >>From: Patrick Venture [<a href="mailto:venture@google.com">mailto:venture@google.com</a>]<br>

> >>Sent: Wednesday, August 21, 2019 10:32 PM<br>

> >>To: Hank Liou (劉晉翰) <Hank.Liou@quantatw.com>; James Feist<br>

> >><james.feist@linux.intel.com><br>

> >>Cc: openbmc@lists.ozlabs.org<br>

> >>Subject: Re: [phosphor-pid-control] scaling issue<br>

> >><br>

> >>On Wed, Aug 21, 2019 at 1:11 AM Hank Liou (劉晉翰)<br>

> >><Hank.Liou@quantatw.com> wrote:<br>

> >>><br>

> >>> Hi All,<br>

> >>><br>

> >>><br>

> >>> After commit [1], I found my temp sensor reading would be re-scaled<br>

> >>> by<br>

> >>multiplying 1 over 255, making temperature into unfamiliar unit. Also<br>

> >>the fan rpm reading would lie in [0,1] interval, letting the fan input<br>

> >>to be 0 (since the input value of fan is from an integer array [2]). Are these<br>

> >normal behaviors?<br>

> >>Or do I miss something?<br>

> >><br>

> >>Are you using dbus configuration or json?  If json, can you attach your config.<br>

> >>Since you're saying it was working and now isn't, I'm assuming there's<br>

> >>something about the config being treated differently with the code<br>

> >>changes in an unexpected way.<br>

> ><br>

> >I found pid control will first read min and max from dbus and then (before<br>

> >commit [1]) revise them if<br>

> ><br>

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

> ><br>

> >After value initialization, the min and max would be the ones in json file (Json<br>

> >file [3]). However, after commit [1] the min and max values of config would<br>

> >not be fed into the fan control process. The scaling issue is originated from<br>

> >commit [4] with the aim to treat fan rpm as percentage. It seems that commit<br>

> >[1] unexpectedly affects temp sensors in the sense that the temp is the<br>

> >integer reading not percentage. Before commit [1], it would not re-scale the<br>

> >temp reading since my min and max are 0 [6].<br>

> ><br>

> ><br>

> ><br>

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

> >like<br>

> ><br>

> >1500 / 20000 = 0.075<br>

> ><br>

> >where rpm max 20000 is from dbus. However the fan input function would<br>

> >transfer double into int, which is 0 for 0.075 (see [5]). Hence the fan input is 0<br>

> >not percentage. Is there something wrong?<br>

> ><br>

> >[1] <a href="https://github.com/openbmc/phosphor-pid-">https://github.com/openbmc/phosphor-pid-</a><br>

> >control/commit/fc2e803f5d9256944e18c7c878a441606b1f121c<br>

> >[2] <a href="https://github.com/openbmc/phosphor-pid-">https://github.com/openbmc/phosphor-pid-</a><br>

> >control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/pid/fancontrolle<br>

> >r.cpp#L86<br>

> >[3]<br>

> >       {<br>

> >            "name": "fan1",<br>

> >            "type": "fan",<br>

> >            "readPath": "/sys/class/hwmon/hwmon1/fan1_input",<br>

> >            "writePath": "/sys/class/hwmon/hwmon1/pwm1",<br>

> >            "min": 0,<br>

> >            "max": 255<br>

> >        },<br>

> >        {<br>

> >            "name": "temp1",<br>

> >            "type": "temp",<br>

> >            "readPath": "/xyz/openbmc_project/sensors/temperature/temp1",<br>

> >            "writePath": "",<br>

> >            "min": 0,<br>

> >            "max": 0<br>

> >        }<br>

> >[4] <a href="https://github.com/openbmc/phosphor-pid-">https://github.com/openbmc/phosphor-pid-</a><br>

> >control/commit/75eb769d351434547899186f73ff70ae00d7934a<br>

> >[5] <a href="https://github.com/openbmc/phosphor-pid-">https://github.com/openbmc/phosphor-pid-</a><br>

> >control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/pid/fancontrolle<br>

> >r.cpp#L64<br>

> >[6] <a href="https://github.com/openbmc/phosphor-pid-">https://github.com/openbmc/phosphor-pid-</a><br>

> >control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/dbus/dbuspassi<br>

> >ve.cpp#L158<br>

> ><br>

> >><br>

> >>><br>

> >>><br>

> >>> [1]<br>

> >>> <a href="https://github.com/openbmc/phosphor-pid-">https://github.com/openbmc/phosphor-pid-</a><br>

> >>control/commit/fc2e803f5d92569<br>

> >>> 44e18c7c878a441606b1f121c<br>

> >>><br>

> >>> [2]<br>

> >>> <a href="https://github.com/openbmc/phosphor-pid-">https://github.com/openbmc/phosphor-pid-</a><br>

> >>control/blob/a7ec8350d17b70153<br>

> >>> cebe666d3fbe88bddd02a1a/pid/fancontroller.cpp#L86<br>

> >>><br>

> >>><br>

> >>> Thanks,<br>

> >>><br>

> >>><br>

> >>> Hank Liou<br>

> >>><br>

> >>> Quanta Computer Inc.<br>

> >>><br>

> >>><br>

> ><br>

> >Sincerely,<br>

> >Hank<br>

</div>

</span></font></div>

</div>

</div>

</div>

</div>

</body>

</html>