Re: [PATCH v3 1/8] thermal: core: Add mechanism for connecting trips with driver data

From: Daniel Lezcano
Date: Thu Aug 03 2023 - 12:20:21 EST

On 03/08/2023 16:15, Rafael J. Wysocki wrote:
On Thu, Aug 3, 2023 at 3:06 PM Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> wrote:

On 02/08/2023 18:48, Rafael J. Wysocki wrote:

[ ... ]

Let me check if I can do something on top of your series to move it in
the ACPI driver.

It doesn't need to be on top of my series, so if you have an idea,
please just let me know what it is.

It can't be entirely in the ACPI driver AFAICS, though, because
trips[i] need to be modified on updates and they belong to the core.
Hence, the driver needs some help from the core to get to them. It
can be something like "this is my trip tag and please give me the
address of the trip matching it" or similar, but it is needed, because
the driver has to assume that the trip indices used by it initially
may change.

May be I'm missing something but driver_ref does not seems to be used
except when assigning it, no?

It is used on the other side. That is, the value assigned to the trip
field in it is accessed via trip_ref in the driver.

The idea is that the driver puts a pointer to its local struct
thermal_trip_ref into a struct thermal_trip and the core stores the
address of that struct thermal_trip in there, which allows the driver
to access the struct thermal_trip via its local struct
thermal_trip_ref going forward.

Admittedly, this is somewhat convoluted.

I have an alternative approach in the works, just for illustration
purposes if nothing else, but I have encountered a problem that I
would like to ask you about.

Namely, zone disabling is not particularly useful for preventing the
zone from being used while the trips are updated, because it has side
effects. First, it triggers __thermal_zone_device_update() and a
netlink message every time the mode changes, which can be kind of


But second, if the mode is "disabled", it does not actually
prevent things like __thermal_zone_get_trip() from running and the
zone lock is the only thing that can be used for that AFAICS.
So by "disabling" a thermal zone, did you mean changing its mode to
"disabled" or something else?

Yes, that is what I meant.

May be the initial proposal by updating the thermal trips pointer can solve that [1]

IMO we can assume the trip point changes are very rare (if any), so rebuilding a new trip array and update the thermal zone with the pointer may solve the situation.

The routine does a copy of the trips array, so it can reorder it without impacting the array passed as a parameter. And it can take the lock.

We just have to constraint the update function to invalidate arrays with a number of trip points different from the one initially passed when creating the thermal zone.

Alternatively, we can be smarter in the ACPI driver and update the corresponding temperature+hysteresis trip point by using the thermal_zone_set_trip() function.


<> │ Open source software for ARM SoCs

Follow Linaro: <> Facebook |
<!/linaroorg> Twitter |
<> Blog