On Tue, May 17, 2022 at 6:51 PM srinivas pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote: > > On Tue, 2022-05-17 at 17:42 +0200, 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(). > > > In past we discussed adding thermal sensor sysfs with threshold to > notify temperature. > > So sensor can have set/get_threshold() functions instead of the > set/get_trip for zones. > > Like we have /sys/class/thermal_zone* we can have > /sys/class/thermal_sensor*. Exactly, so renaming thermal_zone_ops as thermal_sensor_ops isn't quite helpful in this respect. IMO there should be operations for sensors and there should be operations for thermal zones and those two sets of operations should be different. > Thermal sensor(s) are bound to thermal zones. So I think that this binding should be analogous to the binding between thermal zones and cooling devices. > This can also include multiple sensors in a zone and can create a virtual sensor also. It can. However, what's the difference between a thermal zone with multiple sensors and a thermal zone with one virtual sensor being an aggregate of multiple physical sensors? Both involve some type of aggregation of temperature values measured by the physical sensors. > > > 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. > > > > 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. > > > > 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. >
