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

[Guenter Roeck]

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