linux-kernel - Re: [PATCH V3] hwmon: scmi-hwmon: implement change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <540cf4b3-ebf6-4a85-84c4-c012a69db416@roeck-us.net>
Date: Thu, 25 Jan 2024 06:25:00 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Peng Fan <peng.fan@....com>, "Peng Fan (OSS)" <peng.fan@....nxp.com>,
 "groeck7@...il.com" <groeck7@...il.com>,
 "sudeep.holla@....com" <sudeep.holla@....com>,
 "cristian.marussi@....com" <cristian.marussi@....com>,
 "jdelvare@...e.com" <jdelvare@...e.com>
Cc: "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3] hwmon: scmi-hwmon: implement change_mode for thermal
 zones

On 1/24/24 23:06, Peng Fan wrote:
> Hi Guenter,
> 
>> Subject: Re: [PATCH V3] hwmon: scmi-hwmon: implement change_mode for
>> thermal zones
>>
>> On 1/24/24 22:44, Peng Fan (OSS) wrote:
>>> From: Peng Fan <peng.fan@....com>
>>>
>>> The thermal sensors maybe disabled before kernel boot, so add
>>> change_mode for thermal zones to support configuring the thermal
>>> sensor to enabled state. If reading the temperature when the sensor is
>>> disabled, there will be error reported.
>>>
>>> The cost is an extra config_get all to SCMI firmware to get the status
>>> of the thermal sensor. No function level impact.
>>>
>>> Reviewed-by: Cristian Marussi <cristian.marussi@....com>
>>> Signed-off-by: Peng Fan <peng.fan@....com>
>>> ---
>>>
>>> V3:
>>>    Update commit log to show it only applys to thermal
>>>    Add comments in code
>>>    Add R-b from Cristian
>>>
>>
>> You didn't address my question regarding the behavior of hwmon attributes if
>> a sensor is disabled.
> 
> Would you please share a bit more on what attributes?
> You mean the files under /sys/class/hwmon/hwmon0?
> 
> If the sensor is disabled, when cat temp[x]_input, it will
> report:
> root@...95-19x19-lpddr5-evk:/sys/class/hwmon/hwmon0# cat temp3_input
> cat: temp3_input: Protocol error
> 
> For enabled, it will report value:
> root@...95-19x19-lpddr5-evk:/sys/class/hwmon/hwmon0# cat temp1_input
> 31900
> 

This is wrong. If the sensor is disabled, there should either be no
sensor attribute (if the condition is permanent), or there should be
tempX_enable attribute(s) which return the sensor status (and, if
appropriate, permit it to be enabled). If the condition is not
permanent, attempts to read the sensor value should return -ENODATA.

Overall, hwmon functionality is orthogonal to thermal subsystem use.
It is not appropriate for the thermal subsystem to disable any
temperature sensors in the hwmon subsystem because the user might
expect to be able to read temperatures from hwmon devices even
if sensors are not in use by the thermal subsystem.

Thanks,
Guenter