On Tue, Aug 1, 2023 at 8:27 PM Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> wrote: > > > Hi Rafael, > > On 25/07/2023 14:02, Rafael J. Wysocki wrote: > > Hi Everyone, > > > > This is the second iteration of the $subject patch series and its original > > description below is still applicable > > > > On Tuesday, July 18, 2023 8:01:20 PM CEST Rafael J. Wysocki wrote: > >> > >> This patch series makes the ACPI thermal driver register thermal zones > >> with the help of thermal_zone_device_register_with_trips(), so it > >> doesn't need to use the thermal zone callbacks related to trip points > >> any more (and they are dropped in the last patch). > >> > >> The approach presented here is quite radically different from the > >> previous attempts, as it doesn't really rearrange the driver's > >> internal data structures, but adds the trip table support on top of > >> them. For this purpose, it uses an additional field in struct thermal_trip > >> introduced in the first patch. > > > > This update is mostly related to the observation that the critical and hot trip > > points never change after initialization, so they don't really need to be > > connected back to the corresponding thermal_trip structures. It also fixes > > an error code path memory leak in patch [5/8]. > > I've been through the series. It is really cool that we can get rid of > the ops usage at the end of the series. > > However, the series introduces a wrapper to the thermal zone lock and > exports that in the public header. That goes in the opposite direction > of the recent cleanups and obviously will give the opportunity to > drivers to do silly things [again]. Because it is necessary to update the trip points in the table under the zone lock, the driver needs to somehow make that happen. I suppose I can pass a callback to thermal_zone_device_update() or similar, but I'm not sure if that's better. > On the other side, the structure thermal_trip introduces a circular > reference, which is usually something to avoid. What do you mean by "a circular reference"? > Apart those two points, the ACPI changes look ok. > > Comments in the different patches will follow Thanks!
