Re: [PATCH V3] hwmon: scmi-hwmon: implement change_mode for thermal zones

From: Guenter Roeck
Date: Thu Jan 25 2024 - 09:25:13 EST


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@xxxxxxx>

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@xxxxxxx>
Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
---

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@imx95-19x19-lpddr5-evk:/sys/class/hwmon/hwmon0# cat temp3_input
cat: temp3_input: Protocol error

For enabled, it will report value:
root@imx95-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