Re: [PATCH v2 01/14] thermal/core: Change thermal_zone_ops to thermal_sensor_ops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Rafael,

sorry for the delay, I was OoO.

On 17/05/2022 17:42, Rafael J. Wysocki wrote:
On Sat, May 7, 2022 at 2:55 PM Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> wrote:

A thermal zone is software abstraction of a sensor associated with
properties and cooling devices if any.

The fact that we have thermal_zone and thermal_zone_ops mixed is
confusing and does not clearly identify the different components
entering in the thermal management process. A thermal zone appears to
be a sensor while it is not.

Well, the majority of the operations in thermal_zone_ops don't apply
to thermal sensors.  For example, ->set_trips(), ->get_trip_type(),
->get_trip_temp().

The set_trips is necessary to set the sensor interrupt to fire when the trip temperature is crossed the way up or down.

In order to set the scene for multiple thermal sensors aggregated into
a single thermal zone. Rename the thermal_zone_ops to
thermal_sensor_ops, that will appear clearyl the thermal zone is not a
sensor but an abstraction of one [or multiple] sensor(s).

So I'm not convinced that the renaming mentioned above is particularly
clean either.

IMV the way to go would be to split the thermal sensor operations,
like ->get_temp(), out of thermal_zone_ops.

Probably, we should first replace all the calls to ops->get_temp with a function. Then create the ops for the sensor:

 - get_trend
 - get_temp
 - set_trips
 - bind / unbind

But then it is not clear what a thermal zone with multiple sensors in
it really means.  I guess it would require an aggregation function to
combine the thermal sensors in it that would produce an effective
temperature to check against the trip points.

Yes, that is why the above ops->get_temp should be wrapped into a function which can evolve to an aggregation function.

Honestly, I don't think that setting a separate set of trips for each
sensor in a thermal zone would make a lot of sense.

I agree the set_trips is for the interrupt mode only.


--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux