hwmontempsensor issue in dbus-sensor

[Patrick Williams]

Ed,

I think there are other commits needing the IIO functionality now. There is at least the pending commit for Humidity that we’d like to get merged. I think it is better to move forward with a fix rather than backwards with a revert at this point. 

Kumar/Jayashree,

Can you dig into this and get to the bottom of it?

- Patrick
Sent from my iPhone

> On Jan 3, 2022, at 11:21 AM, Ed Tanous <ed at tanous.net> wrote:
> 
> On Fri, Dec 31, 2021 at 9:31 AM Bruce Mitchell
> <bruce.mitchell at linux.vnet.ibm.com> wrote:
>> 
>>> On 12/20/2021 10:29, Ed Tanous wrote:
>>> On Sun, Dec 19, 2021 at 9:11 PM Kumar Thangavel
>>> <kumarthangavel.hcl at gmail.com> wrote:
>>>> 
>>>> Hi All,
>>>> 
>>>> Gentle Reminder on this.
>>>> 
>>>> Thanks,
>>>> Kumar.
>>>> 
>>>> On Thu, Dec 16, 2021 at 1:56 PM Kumar Thangavel <kumarthangavel.hcl at gmail.com> wrote:
>>>>> 
>>>>> Hi All,
>>>>> 
>>>>> 
>>>>> In dbus-sensors repo, I saw the patch for changes in Hwmon Temp Sensor.
>>>>> https://github.com/openbmc/dbus-sensors/commit/544e7dc58cecef898fe7d684c3ba0fbce178cf2d#diff-bfc8d7ed4ade4b3519ad3b7fe091f9c79a60faf8a26f410c1568e5341b22b7ab
>>> 
>>> Bruce, this is your patch.  Could you take a look at the below and
>>> provide your input?
>>> 
>>>>> 
>>>>> 
>>>>> While testing those changes in my platform, I found that each sensor in hwmon temp has been creating twice.
>>>>> After analyzed the patch, for below 2 paths SP_OUTLET_TEMP sensor is created twice.
>>>>> 
>>>>> 
>>>>> /sys/class/hwmon/hwmon2/temp2_input
>>>>> /sys/class/hwmon/hwmon2/temp1_input
>>>>> 
>>>>> 
>>>>> Before your change, hwmon temp sensor will check whether the path is already exists or not.
>>>>> If it exists, it will skip and proceed with next path.
>>>>> https://github.com/openbmc/dbus-sensors/commit/544e7dc58cecef898fe7d684c3ba0fbce178cf2d#diff-bfc8d7ed4ade4b3519ad3b7fe091f9c79a60faf8a26f410c1568e5341b22b7abL96
>>>>> 
>>>>> 
>>>>> Could you please explain why it is creating twice in hwmon temp ?
>>>>> 
>>>>> 
>>>>> Journalctl log before your changes applied :
>>>>> 
>>>>> 
>>>>> Dec 31 16:01:32 yosemitev2 hwmontempsensor[286]: Path : "/sys/class/hwmon/hwmon2/temp2_input"
>>>>> Dec 31 16:01:32 yosemitev2 hwmontempsensor[286]: Type : xyz.openbmc_project.Configuration.TMP421
>>>>> Dec 31 16:01:32 yosemitev2 hwmontempsensor[286]: Name : SP_OUTLET_TEMP
>>>>> Dec 31 16:01:32 yosemitev2 hwmontempsensor[286]: Dbus path : /xyz/openbmc_project/sensors/temperature/SP_OUTLET_TEMP
>>>>> Dec 31 16:01:32 yosemitev2 hwmontempsensor[286]: Dbus path : /xyz/openbmc_project/sensors/temperature/SP_OUTLET_REMOTE_TEMP
>>>>> Dec 31 16:01:32 yosemitev2 hwmontempsensor[286]: Path : "/sys/class/hwmon/hwmon2/temp1_input"
>>>>> 
>>>>> 
>>>>> 
>>>>> Journalctl log after your changes applied :
>>>>> 
>>>>> 
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Path : "/sys/class/hwmon/hwmon2/temp2_input"
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Sensor type : xyz.openbmc_project.Configuration.TMP421
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Name : SP_OUTLET_TEMP
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Path : /xyz/openbmc_project/sensors/temperature/SP_OUTLET_TEMP
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Path : /xyz/openbmc_project/sensors/temperature/SP_OUTLET_REMOTE_TEMP
>>>>> 
>>>>> 
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Path : "/sys/class/hwmon/hwmon2/temp1_input"
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Sensor type : xyz.openbmc_project.Configuration.TMP421
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Name : SP_OUTLET_TEMP
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Path : /xyz/openbmc_project/sensors/temperature/SP_OUTLET_TEMP
>>>>> Dec 31 16:01:54 yosemitev2 hwmontempsensor[327]: Path : /xyz/openbmc_project/sensors/temperature/SP_OUTLET_REMOTE_TEMP
>>>>> 
>>>>> 
>>>>> 
>>>>> Thanks,
>>>>> 
>>>>> Kumar.
>> 
>> I will give it quick look today.  Also today is my last day at IBM.
>> 
> 
> Do you plan on continuing this beyond your tenure at IBM?  If not, is
> someone taking this feature over for you?  Given it's been a few weeks
> since this regression was reported, and I don't see any fixes in
> review, I'm leaning toward reverting it on master for the time being
> until someone puts up a patch to fix the issue.