Re: [PATCH 1/2] thermal: core: add initial support for cold and critical_cold trip point

[Rafael J. Wysocki]

On Tue, Dec 12, 2023 at 11:17 PM Christian Marangi <ansuelsmth@xxxxxxxxx> wrote:
>
> Add initial support for cold and critical_cold trip point. Many if not
> all hwmon and thermal device have normally trip point for hot
> temperature and for cold temperature.
>
> Till now only hot temperature were supported. Add support for also cold
> temperature to permit complete definition of cold trip point in DT.
>
> Thermal driver may use these additional trip point to correctly set
> interrupt for cold temperature values and react based on that with
> various measure like enabling attached heater, forcing higher voltage
> and other specialaized peripherals.
>
> For hwmon drivers this is needed as currently there is a problem with
> setting the full operating range of the device for thermal devices
> defined with hwmon. To better describe the problem, the following
> example is needed:
>
> In the scenario of a simple hwmon with an active trip point declared
> and a cooling device attached, the hwmon subsystem currently set the
> min and max trip point based on the single active trip point.
> Thermal subsystem parse all the trip points and calculate the lowest and
> the highest trip point and calls the .set_trip of hwmon to setup the
> trip points.
>
> The fact that we currently don't have a way to declare the cold/min
> temperature values, makes the thermal subsystem to set the low value as
> -INT_MAX.
> For hwmon drivers that doesn't use clamp_value and actually reject
> invalid values for the trip point, this results in the hwmon settings to
> be rejected.
>
> To permit to pass the correct range of trip point, permit to set in DT
> also cold and critical_cold trip point.
>
> Thermal driver may also define .cold and .critical_cold to act on these
> trip point tripped and apply the required measure.
>
> Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>

Generally speaking, it is kind of late in the cycle for adding
significant new features like this.  We can get to it when 6.8-rc1 is
out, so please resend then.

Also it would be nice to define/document the cold and crtitical_cold
trip points somewhere and is there a better name for critical_cold?

> ---
>  drivers/thermal/thermal_core.c  | 13 +++++++++++++
>  drivers/thermal/thermal_of.c    |  2 ++
>  drivers/thermal/thermal_sysfs.c |  4 ++++
>  drivers/thermal/thermal_trace.h |  4 ++++
>  include/linux/thermal.h         |  2 ++
>  include/uapi/linux/thermal.h    |  2 ++
>  6 files changed, 27 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 9c17d35ccbbd..3c5ab560e72f 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -344,6 +344,17 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
>                 tz->ops->hot(tz);
>  }
>
> +static void handle_critical_cold_trips(struct thermal_zone_device *tz,
> +                                      const struct thermal_trip *trip)
> +{
> +       trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type);
> +
> +       if (trip->type == THERMAL_TRIP_CRITICAL_COLD && tz->ops->critical_cold)
> +               tz->ops->critical_cold(tz);
> +       else if (trip->type == THERMAL_TRIP_COLD && tz->ops->cold)
> +               tz->ops->cold(tz);
> +}
> +
>  static void handle_thermal_trip(struct thermal_zone_device *tz,
>                                 const struct thermal_trip *trip)
>  {
> @@ -365,6 +376,8 @@ static void handle_thermal_trip(struct thermal_zone_device *tz,
>
>         if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT)
>                 handle_critical_trips(tz, trip);
> +       else if (trip->type == THERMAL_TRIP_CRITICAL_COLD || trip->type == THERMAL_TRIP_COLD)
> +               handle_critical_cold_trips(tz, trip);
>         else
>                 handle_non_critical_trips(tz, trip);
>  }
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 1e0655b63259..95bc600bb4b8 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -60,6 +60,8 @@ static const char * const trip_types[] = {
>         [THERMAL_TRIP_PASSIVE]  = "passive",
>         [THERMAL_TRIP_HOT]      = "hot",
>         [THERMAL_TRIP_CRITICAL] = "critical",
> +       [THERMAL_TRIP_COLD]     = "cold",
> +       [THERMAL_TRIP_CRITICAL_COLD] = "critical_cold",
>  };
>
>  /**
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index eef40d4f3063..e1e69e0991c2 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -106,6 +106,10 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr,
>                 return sprintf(buf, "critical\n");
>         case THERMAL_TRIP_HOT:
>                 return sprintf(buf, "hot\n");
> +       case THERMAL_TRIP_COLD:
> +               return sprintf(buf, "cold\n");
> +       case THERMAL_TRIP_CRITICAL_COLD:
> +               return sprintf(buf, "critical_cold\n");
>         case THERMAL_TRIP_PASSIVE:
>                 return sprintf(buf, "passive\n");
>         case THERMAL_TRIP_ACTIVE:
> diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h
> index 459c8ce6cf3b..0a4f96075d7d 100644
> --- a/drivers/thermal/thermal_trace.h
> +++ b/drivers/thermal/thermal_trace.h
> @@ -11,6 +11,8 @@
>
>  TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL);
>  TRACE_DEFINE_ENUM(THERMAL_TRIP_HOT);
> +TRACE_DEFINE_ENUM(THERMAL_TRIP_COLD);
> +TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL_COLD);
>  TRACE_DEFINE_ENUM(THERMAL_TRIP_PASSIVE);
>  TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE);
>
> @@ -18,6 +20,8 @@ TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE);
>         __print_symbolic(type,                                  \
>                          { THERMAL_TRIP_CRITICAL, "CRITICAL"},  \
>                          { THERMAL_TRIP_HOT,      "HOT"},       \
> +                        { THERMAL_TRIP_COLD,      "COLD"},     \
> +                        { THERMAL_TRIP_CRITICAL_COLD, "CRITICAL_COLD"}, \
>                          { THERMAL_TRIP_PASSIVE,  "PASSIVE"},   \
>                          { THERMAL_TRIP_ACTIVE,   "ACTIVE"})
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index cee814d5d1ac..d6345c9ec50d 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -84,6 +84,8 @@ struct thermal_zone_device_ops {
>                           const struct thermal_trip *, enum thermal_trend *);
>         void (*hot)(struct thermal_zone_device *);
>         void (*critical)(struct thermal_zone_device *);
> +       void (*cold)(struct thermal_zone_device *);
> +       void (*critical_cold)(struct thermal_zone_device *);
>  };
>
>  struct thermal_cooling_device_ops {
> diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
> index fc78bf3aead7..7fa1ba0dff05 100644
> --- a/include/uapi/linux/thermal.h
> +++ b/include/uapi/linux/thermal.h
> @@ -14,6 +14,8 @@ enum thermal_trip_type {
>         THERMAL_TRIP_PASSIVE,
>         THERMAL_TRIP_HOT,
>         THERMAL_TRIP_CRITICAL,
> +       THERMAL_TRIP_COLD,
> +       THERMAL_TRIP_CRITICAL_COLD,
>  };
>
>  /* Adding event notification support elements */
> --
> 2.40.1
>