On Tue, May 17, 2022 at 9:02 PM srinivas pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote: > > On Tue, 2022-05-17 at 20:53 +0200, Rafael J. Wysocki wrote: > > 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? > > > Either way is fine. A thermal sensor can be aggregate of other sensors. Agreed. But the point is that if we go with thermal zones bound to multiple physical sensors, I don't see much point in using virtual sensors. And the other way around. Daniel seems to be preferring the thermal zones bound to multiple physical sensors approach. > > 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. > > > >
