Re: [PATCH v2 0/9] thermal: Writable trip points handling rework

Rafael J. Wysocki
2024-02-15T17:56:34Z

On Mon, Feb 12, 2024 at 7:44 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> Hi Everyone,
>
> This is an update of
>
> https://lore.kernel.org/linux-pm/3232442.5fSG56mABF@kreacher/
>
> fixing a few bugs and renaming the new trip point flags introduced by it.
>
> The original description of the patch series is still applicable:
>
> "The purpose of this patch series is to allow thermal zone creators
> to specify which properties (temperature or hysteresis) of which
> trip points can be set from user space via sysfs on a per-trip basis
> instead of passing writable trips masks to the thermal zone registration
> function which is both cumbersome and error prone and it doesn't even
> allow to request different treatment of different trip properties.
>
> The writable trip masks used today only affect trip temperatures (that is, if
> a trip point is in a writable trips mask, its temperature can be set via
> sysfs) and they only take effect if the CONFIG_THERMAL_WRITABLE_TRIPS kernel
> configuration option is set, which appears to be assumed by at least some
> of the drivers using writable trips masks. Some other drivers using them
> simply select CONFIG_THERMAL_WRITABLE_TRIPS which pretty much defeats its
> purpose (and imx even sets this option in its defconfig).
>
> For this reasons, patch [1/9] removes CONFIG_THERMAL_WRITABLE_TRIPS and makes
> the writable trips masks always work.
>
> Moreover, trip hysteresis, which is not affected either by the writable trips
> masks or by CONFIG_THERMAL_WRITABLE_TRIPS, can only be set via sysfs if the
> .set_trip_hyst() operation is provided by the given thermal zone, but currently
> this thermal zone operation is used by no one, so effectively trip hysteresis
> cannot be set via sysfs at all. This is not a problem for the majority of
> drivers that want trip temperatures to be set via sysfs, because they also
> don't want trip hysteresis to be changed for any trips (at least as far as I
> can say), but there are use cases in which it is desirable to be able to
> update trip hysteresis as well as trip temperature (for example see
> https://lore.kernel.org/linux-pm/20240106191502.29126-1-quic_manafm@quicinc.com/).
> Those use cases are not addressed here directly, but after this series
> addressing them should be relatively straightforward.
>
> Namely, patch [2/9] adds flags to struct thermal_trip and defines two of them
> to indicate whether or not setting the temperature or hysteresis of the given
> trip via sysfs is allowed. If a writable trips mask is passed to
> thermal_zone_device_register_with_trips(), is it is used to set the
> "writable temperature" flag for the trips covered by it and that flag is
> then consulted by the thermal sysfs code. The "writable hysteresis" trip
> flag is also taken into account by the thermal sysfs code, but it is not set
> automatically in any case.
>
> Patch [3/9] is based on the observation that the .set_trip_hyst() thermal zone
> operation is never used - it simply drops that callback from struct
> thermal_zone_device_ops and adjusts the code checking its presence.
>
> Patches [4-8/9] update drivers using writable trips masks to set the new
> "writable temperature" flag directly instead and some of them are simplified
> a bit as a result. After these patches, all of the callers of
> thermal_zone_device_register_with_trips() pass a zero writable trips mask
> to it, so patch [9/9] drops that mask from the functions argument list and
> adjusts all of its callers accordingly.
>
> After all of the changes in this series, allowing the hysteresis value to be
> set via sysfs for a given trip is a matter of setting its "writable
> hysteresis" flag (and analogously for trip temperature)."

By the lack of comments I gather that this series is not controversial.

It unlocks further development and it should be run through linux-next
for a couple of weeks before the merge window, so reviews are welcome.

Thanks!