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

[Rafael J. Wysocki]

On Mon, Feb 12, 2024 at 7:44 PM Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> 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@xxxxxxxxxxx/).
>  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!