> Subject: Re: [PATCH V3] hwmon: scmi-hwmon: implement change_mode for
> thermal zones
>
> On Thu, Jan 25, 2024 at 08:16:45AM -0800, Guenter Roeck wrote:
> > On 1/25/24 08:09, Cristian Marussi wrote:
> >
> > > Agreed, but it seems that indeed here the attempt is to make sure
> > > that an accidentally disabled sensor is turned on.
> > >
> >
> > From the patch:
> >
> > +static int scmi_hwmon_thermal_change_mode(struct
> thermal_zone_device *tz,
> > + enum thermal_device_mode
> new_mode) {
> > ...
> > + if (new_mode == THERMAL_DEVICE_ENABLED)
> > + config |= SCMI_SENS_CFG_SENSOR_ENABLED_MASK;
> > + else
> > + config &= ~SCMI_SENS_CFG_SENSOR_ENABLED_MASK;
> >
> > This clearly contradicts your statement. It leaves the sensor in full
> > control by the thermal subsystem. If the thermal subsystem decides to
> > turn it off, it is turned off.
>
> Yes, indeed, and this is wrong as you explained; what I meant is that it seems
> to me now after this discussion, and from the commit message, that the
> reason (and the aim of this patch) is different from how it achieves it (as a
> side effect)
>
> "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."
>
> So when I said:
>
> > > Agreed, but it seems that indeed here the attempt is to make sure
> > > that an accidentally disabled sensor is turned on.
>
> and
>
> >> In this case seems like the sensor is NOT supposed to be ever
> >>disabled (not even temporarily), it it just that you want to ensure
> >>that is enabled, so I would say @Peng, should not that be done in the
> >>fw SCMI server ? (i.e. to turn on, early on, all those resources that
> >>are
> >> exposed to the agent and meant to be always on)
>
> I implied to drop this patch and instead make sure the visible-and-always-
> enabled sensor is default-enabled by the SCMI server running in the firmware,
> given that there won't be any need to ever disable it, from the hwmon
> interface NOR from the thermal subsystem.
>
> Sorry if I have not been clear (but I maybe still well-wrong anyway :D)
Sorry to bring back this old topic.
The tempsensor is disabled at boot, I will check with FW owner to enable it
by default. But the tempsensor will consume some power, if leaving
it always enabled.
Do we need to export a HWMON_T_ENABLE to temperature sensor if
leaving thermal framework only reading temp?
Thanks,
Peng.
>
> Thanks,
> Cristian
>
