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. > 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. > >
