[v4,1/6] thermal/core: Add user thresholds support

Commit Message

Daniel Lezcano Sept. 23, 2024, 9:59 a.m. UTC

The user thresholds mechanism is a way to have the userspace to tell
the thermal framework to send a notification when a temperature limit
is crossed. There is no id, no hysteresis, just the temperature and
the direction of the limit crossing. That means we can be notified
when a threshold is crossed the way up only, or the way down only or
both ways. That allows to create hysteresis values if it is needed.

A threshold can be added, deleted or flushed. The latter means all
thresholds belonging to a thermal zone will be deleted.

When a threshold is added:

 - if the same threshold (temperature and direction) exists, an error
   is returned

 - if a threshold is specified with the same temperature but a
   different direction, the specified direction is added

 - if there is no threshold with the same temperature then it is
   created

When a threshold is deleted:

 - if the same threshold (temperature and direction) exists, it is
   deleted

 - if a threshold is specified with the same temperature but a
   different direction, the specified direction is removed

 - if there is no threshold with the same temperature, then an error
   is returned

When the threshold are flushed:

 - All thresholds related to a thermal zone are deleted

When a threshold is crossed:

 - the userspace does not need to know which threshold(s) have been
   crossed, it will be notified with the current temperature and the
   previous temperature

 - if multiple thresholds have been crossed between two updates only
   one notification will be send to the userspace, it is pointless to
   send a notification per thresholds crossed as the userspace can
   handle that easily when it has the temperature delta information

Signed-off-by: Daniel Lezcano <[e-mail]>
---
 drivers/thermal/Makefile             |   1 +
 drivers/thermal/thermal_core.h       |   2 +
 drivers/thermal/thermal_thresholds.c | 229 +++++++++++++++++++++++++++
 drivers/thermal/thermal_thresholds.h |  19 +++
 include/linux/thermal.h              |   3 +
 include/uapi/linux/thermal.h         |   2 +
 6 files changed, 256 insertions(+)
 create mode 100644 drivers/thermal/thermal_thresholds.c
 create mode 100644 drivers/thermal/thermal_thresholds.h

Comments

Rafael J. Wysocki Oct. 1, 2024, 7:57 p.m. UTC | #1

On Mon, Sep 23, 2024 at 12:00 PM Daniel Lezcano
<[e-mail]> wrote:
>
> The user thresholds mechanism is a way to have the userspace to tell
> the thermal framework to send a notification when a temperature limit
> is crossed. There is no id, no hysteresis, just the temperature and
> the direction of the limit crossing. That means we can be notified
> when a threshold is crossed the way up only, or the way down only or
> both ways. That allows to create hysteresis values if it is needed.
>
> A threshold can be added, deleted or flushed. The latter means all
> thresholds belonging to a thermal zone will be deleted.
>
> When a threshold is added:
>
>  - if the same threshold (temperature and direction) exists, an error
>    is returned
>
>  - if a threshold is specified with the same temperature but a
>    different direction, the specified direction is added
>
>  - if there is no threshold with the same temperature then it is
>    created
>
> When a threshold is deleted:
>
>  - if the same threshold (temperature and direction) exists, it is
>    deleted
>
>  - if a threshold is specified with the same temperature but a
>    different direction, the specified direction is removed
>
>  - if there is no threshold with the same temperature, then an error
>    is returned
>
> When the threshold are flushed:
>
>  - All thresholds related to a thermal zone are deleted
>
> When a threshold is crossed:
>
>  - the userspace does not need to know which threshold(s) have been
>    crossed, it will be notified with the current temperature and the
>    previous temperature
>
>  - if multiple thresholds have been crossed between two updates only
>    one notification will be send to the userspace, it is pointless to
>    send a notification per thresholds crossed as the userspace can
>    handle that easily when it has the temperature delta information
>
> Signed-off-by: Daniel Lezcano <[e-mail]>
> ---
>  drivers/thermal/Makefile             |   1 +
>  drivers/thermal/thermal_core.h       |   2 +
>  drivers/thermal/thermal_thresholds.c | 229 +++++++++++++++++++++++++++
>  drivers/thermal/thermal_thresholds.h |  19 +++
>  include/linux/thermal.h              |   3 +
>  include/uapi/linux/thermal.h         |   2 +
>  6 files changed, 256 insertions(+)
>  create mode 100644 drivers/thermal/thermal_thresholds.c
>  create mode 100644 drivers/thermal/thermal_thresholds.h
>
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 41c4d56beb40..1e1559bb971e 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -6,6 +6,7 @@ CFLAGS_thermal_core.o           := -I$(src)
>  obj-$(CONFIG_THERMAL)          += thermal_sys.o
>  thermal_sys-y                  += thermal_core.o thermal_sysfs.o
>  thermal_sys-y                  += thermal_trip.o thermal_helpers.o
> +thermal_sys-y                  += thermal_thresholds.o
>
>  # netlink interface to manage the thermal framework
>  thermal_sys-$(CONFIG_THERMAL_NETLINK)          += thermal_netlink.o
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 50b858aa173a..8f320d17d927 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -13,6 +13,7 @@
>  #include <linux/thermal.h>
>
>  #include "thermal_netlink.h"
> +#include "thermal_thresholds.h"
>  #include "thermal_debugfs.h"
>
>  struct thermal_attr {
> @@ -139,6 +140,7 @@ struct thermal_zone_device {
>  #ifdef CONFIG_THERMAL_DEBUGFS
>         struct thermal_debugfs *debugfs;
>  #endif
> +       struct list_head user_thresholds;
>         struct thermal_trip_desc trips[] __counted_by(num_trips);
>  };
>
> diff --git a/drivers/thermal/thermal_thresholds.c b/drivers/thermal/thermal_thresholds.c
> new file mode 100644
> index 000000000000..f33b6d5474d8
> --- /dev/null
> +++ b/drivers/thermal/thermal_thresholds.c
> @@ -0,0 +1,229 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2024 Linaro Limited
> + *
> + * Author: Daniel Lezcano <[e-mail]>
> + *
> + * Thermal thresholds
> + */
> +#include <linux/list.h>
> +#include <linux/list_sort.h>
> +#include <linux/slab.h>
> +
> +#include "thermal_core.h"
> +#include "thermal_thresholds.h"
> +
> +int thermal_thresholds_init(struct thermal_zone_device *tz)
> +{
> +       INIT_LIST_HEAD(&tz->user_thresholds);
> +
> +       return 0;
> +}
> +
> +void thermal_thresholds_flush(struct thermal_zone_device *tz)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +       struct user_threshold *entry, *tmp;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       list_for_each_entry_safe(entry, tmp, thresholds, list_node) {
> +               list_del(&entry->list_node);
> +               kfree(entry);
> +       }
> +
> +       __thermal_zone_device_update(tz, THERMAL_TZ_FLUSH_THRESHOLDS);
> +}
> +
> +void thermal_thresholds_exit(struct thermal_zone_device *tz)
> +{
> +       thermal_thresholds_flush(tz);
> +}
> +
> +static int __thermal_thresholds_cmp(void *data,
> +                                   const struct list_head *l1,
> +                                   const struct list_head *l2)
> +{
> +       struct user_threshold *t1 = container_of(l1, struct user_threshold, list_node);
> +       struct user_threshold *t2 = container_of(l2, struct user_threshold, list_node);
> +
> +       return t1->temperature - t2->temperature;
> +}
> +
> +static struct user_threshold *__thermal_thresholds_find(const struct list_head *thresholds,
> +                                                       int temperature)
> +{
> +       struct user_threshold *t;
> +
> +       list_for_each_entry(t, thresholds, list_node)
> +               if (t->temperature == temperature)
> +                       return t;
> +
> +       return NULL;
> +}
> +
> +static bool __thermal_threshold_is_crossed(struct user_threshold *threshold, int temperature,
> +                                          int last_temperature, int direction,
> +                                          int *low, int *high)
> +{
> +
> +       if (temperature >= threshold->temperature) {
> +               if (threshold->temperature > *low &&
> +                   THERMAL_THRESHOLD_WAY_DOWN & threshold->direction)
> +                       *low = threshold->temperature;
> +
> +               if (last_temperature < threshold->temperature &&
> +                   threshold->direction & direction)
> +                       return true;
> +       } else {
> +               if (threshold->temperature < *high && THERMAL_THRESHOLD_WAY_UP
> +                   & threshold->direction)
> +                       *high = threshold->temperature;
> +
> +               if (last_temperature >= threshold->temperature &&
> +                   threshold->direction & direction)
> +                       return true;
> +       }
> +
> +       return false;
> +}
> +
> +static bool thermal_thresholds_handle_raising(struct list_head *thresholds, int temperature,
> +                                             int last_temperature, int *low, int *high)
> +{
> +       struct user_threshold *t;
> +
> +       list_for_each_entry(t, thresholds, list_node) {
> +               if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
> +                                                  THERMAL_THRESHOLD_WAY_UP, low, high))
> +                       return true;
> +       }
> +
> +       return false;
> +}
> +
> +static bool thermal_thresholds_handle_dropping(struct list_head *thresholds, int temperature,
> +                                              int last_temperature, int *low, int *high)
> +{
> +       struct user_threshold *t;
> +
> +       list_for_each_entry_reverse(t, thresholds, list_node) {
> +               if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
> +                                                  THERMAL_THRESHOLD_WAY_DOWN, low, high))
> +                       return true;
> +       }
> +
> +       return false;
> +}
> +
> +void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *high)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +
> +       int temperature = tz->temperature;
> +       int last_temperature = tz->last_temperature;
> +       bool notify;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       /*
> +        * We need a second update in order to detect a threshold being crossed
> +        */
> +       if (last_temperature == THERMAL_TEMP_INVALID)
> +               return;
> +
> +       /*
> +        * The temperature is stable, so obviously we can not have
> +        * crossed a threshold.
> +        */
> +       if (last_temperature == temperature)
> +               return;
> +
> +       /*
> +        * Since last update the temperature:
> +        * - increased : thresholds are crossed the way up
> +        * - decreased : thresholds are crossed the way down
> +        */
> +       if (temperature > last_temperature)
> +               notify = thermal_thresholds_handle_raising(thresholds, temperature,
> +                                                          last_temperature, low, high);
> +       else
> +               notify = thermal_thresholds_handle_dropping(thresholds, temperature,
> +                                                           last_temperature, low, high);
> +
> +       if (notify)
> +               pr_debug("A threshold has been crossed the way %s, with a temperature=%d, last_temperature=%d\n",
> +                        temperature > last_temperature ? "up" : "down", temperature, last_temperature);
> +}
> +
> +int thermal_thresholds_add(struct thermal_zone_device *tz, int temperature, int direction)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +       struct user_threshold *t;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       t = __thermal_thresholds_find(thresholds, temperature);
> +       if (t) {
> +               if (t->direction == direction)
> +                       return -EEXIST;
> +
> +               t->direction |= direction;
> +       } else {
> +
> +               t = kmalloc(sizeof(*t), GFP_KERNEL);
> +               if (!t)
> +                       return -ENOMEM;
> +
> +               INIT_LIST_HEAD(&t->list_node);
> +               t->temperature = temperature;
> +               t->direction = direction;
> +               list_add(&t->list_node, thresholds);
> +               list_sort(NULL, thresholds, __thermal_thresholds_cmp);
> +       }
> +
> +       __thermal_zone_device_update(tz, THERMAL_TZ_ADD_THRESHOLD);
> +
> +       return 0;
> +}
> +
> +int thermal_thresholds_delete(struct thermal_zone_device *tz, int temperature, int direction)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +       struct user_threshold *t;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       t = __thermal_thresholds_find(thresholds, temperature);
> +       if (!t)
> +               return -ENOENT;
> +
> +       if (t->direction == direction) {
> +               list_del(&t->list_node);
> +               kfree(t);
> +       } else {
> +               t->direction &= ~direction;
> +       }
> +
> +       __thermal_zone_device_update(tz, THERMAL_TZ_DEL_THRESHOLD);
> +
> +       return 0;
> +}
> +
> +int thermal_thresholds_for_each(struct thermal_zone_device *tz,
> +                               int (*cb)(struct user_threshold *, void *arg), void *arg)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +       struct user_threshold *entry;
> +       int ret;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       list_for_each_entry(entry, thresholds, list_node) {
> +               ret = cb(entry, arg);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> diff --git a/drivers/thermal/thermal_thresholds.h b/drivers/thermal/thermal_thresholds.h
> new file mode 100644
> index 000000000000..232f4e8089af
> --- /dev/null
> +++ b/drivers/thermal/thermal_thresholds.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __THERMAL_THRESHOLDS_H__
> +#define __THERMAL_THRESHOLDS_H__
> +
> +struct user_threshold {
> +       struct list_head list_node;
> +       int temperature;
> +       int direction;
> +};
> +
> +int thermal_thresholds_init(struct thermal_zone_device *tz);
> +void thermal_thresholds_exit(struct thermal_zone_device *tz);
> +void thermal_thresholds_flush(struct thermal_zone_device *tz);
> +void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *high);
> +int thermal_thresholds_add(struct thermal_zone_device *tz, int temperature, int direction);
> +int thermal_thresholds_delete(struct thermal_zone_device *tz, int temperature, int direction);
> +int thermal_thresholds_for_each(struct thermal_zone_device *tz,
> +                               int (*cb)(struct user_threshold *, void *arg), void *arg);
> +#endif
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 25ea8fe2313e..bcaa92732e14 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -56,6 +56,9 @@ enum thermal_notify_event {
>         THERMAL_TZ_UNBIND_CDEV, /* Cooling dev is unbind from the thermal zone */
>         THERMAL_INSTANCE_WEIGHT_CHANGED, /* Thermal instance weight changed */
>         THERMAL_TZ_RESUME, /* Thermal zone is resuming after system sleep */
> +       THERMAL_TZ_ADD_THRESHOLD, /* Threshold added */
> +       THERMAL_TZ_DEL_THRESHOLD, /* Threshold deleted */
> +       THERMAL_TZ_FLUSH_THRESHOLDS, /* All thresholds deleted */
>  };
>
>  /**
> diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
> index fc78bf3aead7..3e7c1c2e71a7 100644
> --- a/include/uapi/linux/thermal.h
> +++ b/include/uapi/linux/thermal.h
> @@ -3,6 +3,8 @@
>  #define _UAPI_LINUX_THERMAL_H
>
>  #define THERMAL_NAME_LENGTH    20
> +#define THERMAL_THRESHOLD_WAY_UP       0x1
> +#define THERMAL_THRESHOLD_WAY_DOWN     0x2

It would be somewhat better to use BIT(0) and BIT(1) here IMO, but
apart from that this patch and patch [2/6] are fine with me (even
though my implementation of threshold crossing detection would be
different).

I still have questions regarding patch [3/6] though, but I'll need to
take a fresh look at it tomorrow.

>
>  enum thermal_device_mode {
>         THERMAL_DEVICE_DISABLED = 0,
> --
> 2.43.0
>

Rafael J. Wysocki Oct. 2, 2024, 12:22 p.m. UTC | #2

On Tue, Oct 1, 2024 at 9:57 PM Rafael J. Wysocki <[e-mail]> wrote:
>
> On Mon, Sep 23, 2024 at 12:00 PM Daniel Lezcano
> <[e-mail]> wrote:
> >
> > The user thresholds mechanism is a way to have the userspace to tell
> > the thermal framework to send a notification when a temperature limit
> > is crossed. There is no id, no hysteresis, just the temperature and
> > the direction of the limit crossing. That means we can be notified
> > when a threshold is crossed the way up only, or the way down only or
> > both ways. That allows to create hysteresis values if it is needed.
> >
> > A threshold can be added, deleted or flushed. The latter means all
> > thresholds belonging to a thermal zone will be deleted.
> >
> > When a threshold is added:
> >
> >  - if the same threshold (temperature and direction) exists, an error
> >    is returned
> >
> >  - if a threshold is specified with the same temperature but a
> >    different direction, the specified direction is added
> >
> >  - if there is no threshold with the same temperature then it is
> >    created
> >
> > When a threshold is deleted:
> >
> >  - if the same threshold (temperature and direction) exists, it is
> >    deleted
> >
> >  - if a threshold is specified with the same temperature but a
> >    different direction, the specified direction is removed
> >
> >  - if there is no threshold with the same temperature, then an error
> >    is returned
> >
> > When the threshold are flushed:
> >
> >  - All thresholds related to a thermal zone are deleted
> >
> > When a threshold is crossed:
> >
> >  - the userspace does not need to know which threshold(s) have been
> >    crossed, it will be notified with the current temperature and the
> >    previous temperature
> >
> >  - if multiple thresholds have been crossed between two updates only
> >    one notification will be send to the userspace, it is pointless to
> >    send a notification per thresholds crossed as the userspace can
> >    handle that easily when it has the temperature delta information
> >
> > Signed-off-by: Daniel Lezcano <[e-mail]>
> > ---
> >  drivers/thermal/Makefile             |   1 +
> >  drivers/thermal/thermal_core.h       |   2 +
> >  drivers/thermal/thermal_thresholds.c | 229 +++++++++++++++++++++++++++
> >  drivers/thermal/thermal_thresholds.h |  19 +++
> >  include/linux/thermal.h              |   3 +
> >  include/uapi/linux/thermal.h         |   2 +
> >  6 files changed, 256 insertions(+)
> >  create mode 100644 drivers/thermal/thermal_thresholds.c
> >  create mode 100644 drivers/thermal/thermal_thresholds.h
> >
> > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> > index 41c4d56beb40..1e1559bb971e 100644
> > --- a/drivers/thermal/Makefile
> > +++ b/drivers/thermal/Makefile
> > @@ -6,6 +6,7 @@ CFLAGS_thermal_core.o           := -I$(src)
> >  obj-$(CONFIG_THERMAL)          += thermal_sys.o
> >  thermal_sys-y                  += thermal_core.o thermal_sysfs.o
> >  thermal_sys-y                  += thermal_trip.o thermal_helpers.o
> > +thermal_sys-y                  += thermal_thresholds.o
> >
> >  # netlink interface to manage the thermal framework
> >  thermal_sys-$(CONFIG_THERMAL_NETLINK)          += thermal_netlink.o
> > diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> > index 50b858aa173a..8f320d17d927 100644
> > --- a/drivers/thermal/thermal_core.h
> > +++ b/drivers/thermal/thermal_core.h
> > @@ -13,6 +13,7 @@
> >  #include <linux/thermal.h>
> >
> >  #include "thermal_netlink.h"
> > +#include "thermal_thresholds.h"
> >  #include "thermal_debugfs.h"
> >
> >  struct thermal_attr {
> > @@ -139,6 +140,7 @@ struct thermal_zone_device {
> >  #ifdef CONFIG_THERMAL_DEBUGFS
> >         struct thermal_debugfs *debugfs;
> >  #endif
> > +       struct list_head user_thresholds;
> >         struct thermal_trip_desc trips[] __counted_by(num_trips);
> >  };
> >
> > diff --git a/drivers/thermal/thermal_thresholds.c b/drivers/thermal/thermal_thresholds.c
> > new file mode 100644
> > index 000000000000..f33b6d5474d8
> > --- /dev/null
> > +++ b/drivers/thermal/thermal_thresholds.c
> > @@ -0,0 +1,229 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2024 Linaro Limited
> > + *
> > + * Author: Daniel Lezcano <[e-mail]>
> > + *
> > + * Thermal thresholds
> > + */
> > +#include <linux/list.h>
> > +#include <linux/list_sort.h>
> > +#include <linux/slab.h>
> > +
> > +#include "thermal_core.h"
> > +#include "thermal_thresholds.h"
> > +
> > +int thermal_thresholds_init(struct thermal_zone_device *tz)
> > +{
> > +       INIT_LIST_HEAD(&tz->user_thresholds);
> > +
> > +       return 0;
> > +}
> > +
> > +void thermal_thresholds_flush(struct thermal_zone_device *tz)
> > +{
> > +       struct list_head *thresholds = &tz->user_thresholds;
> > +       struct user_threshold *entry, *tmp;
> > +
> > +       lockdep_assert_held(&tz->lock);
> > +
> > +       list_for_each_entry_safe(entry, tmp, thresholds, list_node) {
> > +               list_del(&entry->list_node);
> > +               kfree(entry);
> > +       }
> > +
> > +       __thermal_zone_device_update(tz, THERMAL_TZ_FLUSH_THRESHOLDS);
> > +}
> > +
> > +void thermal_thresholds_exit(struct thermal_zone_device *tz)
> > +{
> > +       thermal_thresholds_flush(tz);
> > +}
> > +
> > +static int __thermal_thresholds_cmp(void *data,
> > +                                   const struct list_head *l1,
> > +                                   const struct list_head *l2)
> > +{
> > +       struct user_threshold *t1 = container_of(l1, struct user_threshold, list_node);
> > +       struct user_threshold *t2 = container_of(l2, struct user_threshold, list_node);
> > +
> > +       return t1->temperature - t2->temperature;
> > +}
> > +
> > +static struct user_threshold *__thermal_thresholds_find(const struct list_head *thresholds,
> > +                                                       int temperature)
> > +{
> > +       struct user_threshold *t;
> > +
> > +       list_for_each_entry(t, thresholds, list_node)
> > +               if (t->temperature == temperature)
> > +                       return t;
> > +
> > +       return NULL;
> > +}
> > +
> > +static bool __thermal_threshold_is_crossed(struct user_threshold *threshold, int temperature,
> > +                                          int last_temperature, int direction,
> > +                                          int *low, int *high)
> > +{
> > +
> > +       if (temperature >= threshold->temperature) {
> > +               if (threshold->temperature > *low &&
> > +                   THERMAL_THRESHOLD_WAY_DOWN & threshold->direction)
> > +                       *low = threshold->temperature;
> > +
> > +               if (last_temperature < threshold->temperature &&
> > +                   threshold->direction & direction)
> > +                       return true;
> > +       } else {
> > +               if (threshold->temperature < *high && THERMAL_THRESHOLD_WAY_UP
> > +                   & threshold->direction)
> > +                       *high = threshold->temperature;
> > +
> > +               if (last_temperature >= threshold->temperature &&
> > +                   threshold->direction & direction)
> > +                       return true;
> > +       }
> > +
> > +       return false;
> > +}
> > +
> > +static bool thermal_thresholds_handle_raising(struct list_head *thresholds, int temperature,
> > +                                             int last_temperature, int *low, int *high)
> > +{
> > +       struct user_threshold *t;
> > +
> > +       list_for_each_entry(t, thresholds, list_node) {
> > +               if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
> > +                                                  THERMAL_THRESHOLD_WAY_UP, low, high))
> > +                       return true;
> > +       }
> > +
> > +       return false;
> > +}
> > +
> > +static bool thermal_thresholds_handle_dropping(struct list_head *thresholds, int temperature,
> > +                                              int last_temperature, int *low, int *high)
> > +{
> > +       struct user_threshold *t;
> > +
> > +       list_for_each_entry_reverse(t, thresholds, list_node) {
> > +               if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
> > +                                                  THERMAL_THRESHOLD_WAY_DOWN, low, high))
> > +                       return true;
> > +       }
> > +
> > +       return false;
> > +}
> > +
> > +void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *high)
> > +{
> > +       struct list_head *thresholds = &tz->user_thresholds;
> > +
> > +       int temperature = tz->temperature;
> > +       int last_temperature = tz->last_temperature;
> > +       bool notify;
> > +
> > +       lockdep_assert_held(&tz->lock);
> > +
> > +       /*
> > +        * We need a second update in order to detect a threshold being crossed
> > +        */
> > +       if (last_temperature == THERMAL_TEMP_INVALID)
> > +               return;
> > +
> > +       /*
> > +        * The temperature is stable, so obviously we can not have
> > +        * crossed a threshold.
> > +        */
> > +       if (last_temperature == temperature)
> > +               return;
> > +
> > +       /*
> > +        * Since last update the temperature:
> > +        * - increased : thresholds are crossed the way up
> > +        * - decreased : thresholds are crossed the way down
> > +        */
> > +       if (temperature > last_temperature)
> > +               notify = thermal_thresholds_handle_raising(thresholds, temperature,
> > +                                                          last_temperature, low, high);
> > +       else
> > +               notify = thermal_thresholds_handle_dropping(thresholds, temperature,
> > +                                                           last_temperature, low, high);
> > +
> > +       if (notify)
> > +               pr_debug("A threshold has been crossed the way %s, with a temperature=%d, last_temperature=%d\n",
> > +                        temperature > last_temperature ? "up" : "down", temperature, last_temperature);
> > +}
> > +
> > +int thermal_thresholds_add(struct thermal_zone_device *tz, int temperature, int direction)
> > +{
> > +       struct list_head *thresholds = &tz->user_thresholds;
> > +       struct user_threshold *t;
> > +
> > +       lockdep_assert_held(&tz->lock);
> > +
> > +       t = __thermal_thresholds_find(thresholds, temperature);
> > +       if (t) {
> > +               if (t->direction == direction)
> > +                       return -EEXIST;
> > +
> > +               t->direction |= direction;
> > +       } else {
> > +
> > +               t = kmalloc(sizeof(*t), GFP_KERNEL);
> > +               if (!t)
> > +                       return -ENOMEM;
> > +
> > +               INIT_LIST_HEAD(&t->list_node);
> > +               t->temperature = temperature;
> > +               t->direction = direction;
> > +               list_add(&t->list_node, thresholds);
> > +               list_sort(NULL, thresholds, __thermal_thresholds_cmp);
> > +       }
> > +
> > +       __thermal_zone_device_update(tz, THERMAL_TZ_ADD_THRESHOLD);
> > +
> > +       return 0;
> > +}
> > +
> > +int thermal_thresholds_delete(struct thermal_zone_device *tz, int temperature, int direction)
> > +{
> > +       struct list_head *thresholds = &tz->user_thresholds;
> > +       struct user_threshold *t;
> > +
> > +       lockdep_assert_held(&tz->lock);
> > +
> > +       t = __thermal_thresholds_find(thresholds, temperature);
> > +       if (!t)
> > +               return -ENOENT;
> > +
> > +       if (t->direction == direction) {
> > +               list_del(&t->list_node);
> > +               kfree(t);
> > +       } else {
> > +               t->direction &= ~direction;
> > +       }
> > +
> > +       __thermal_zone_device_update(tz, THERMAL_TZ_DEL_THRESHOLD);
> > +
> > +       return 0;
> > +}
> > +
> > +int thermal_thresholds_for_each(struct thermal_zone_device *tz,
> > +                               int (*cb)(struct user_threshold *, void *arg), void *arg)
> > +{
> > +       struct list_head *thresholds = &tz->user_thresholds;
> > +       struct user_threshold *entry;
> > +       int ret;
> > +
> > +       lockdep_assert_held(&tz->lock);
> > +
> > +       list_for_each_entry(entry, thresholds, list_node) {
> > +               ret = cb(entry, arg);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> > diff --git a/drivers/thermal/thermal_thresholds.h b/drivers/thermal/thermal_thresholds.h
> > new file mode 100644
> > index 000000000000..232f4e8089af
> > --- /dev/null
> > +++ b/drivers/thermal/thermal_thresholds.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __THERMAL_THRESHOLDS_H__
> > +#define __THERMAL_THRESHOLDS_H__
> > +
> > +struct user_threshold {
> > +       struct list_head list_node;
> > +       int temperature;
> > +       int direction;
> > +};
> > +
> > +int thermal_thresholds_init(struct thermal_zone_device *tz);
> > +void thermal_thresholds_exit(struct thermal_zone_device *tz);
> > +void thermal_thresholds_flush(struct thermal_zone_device *tz);
> > +void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *high);
> > +int thermal_thresholds_add(struct thermal_zone_device *tz, int temperature, int direction);
> > +int thermal_thresholds_delete(struct thermal_zone_device *tz, int temperature, int direction);
> > +int thermal_thresholds_for_each(struct thermal_zone_device *tz,
> > +                               int (*cb)(struct user_threshold *, void *arg), void *arg);
> > +#endif
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index 25ea8fe2313e..bcaa92732e14 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -56,6 +56,9 @@ enum thermal_notify_event {
> >         THERMAL_TZ_UNBIND_CDEV, /* Cooling dev is unbind from the thermal zone */
> >         THERMAL_INSTANCE_WEIGHT_CHANGED, /* Thermal instance weight changed */
> >         THERMAL_TZ_RESUME, /* Thermal zone is resuming after system sleep */
> > +       THERMAL_TZ_ADD_THRESHOLD, /* Threshold added */
> > +       THERMAL_TZ_DEL_THRESHOLD, /* Threshold deleted */
> > +       THERMAL_TZ_FLUSH_THRESHOLDS, /* All thresholds deleted */
> >  };
> >
> >  /**
> > diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
> > index fc78bf3aead7..3e7c1c2e71a7 100644
> > --- a/include/uapi/linux/thermal.h
> > +++ b/include/uapi/linux/thermal.h
> > @@ -3,6 +3,8 @@
> >  #define _UAPI_LINUX_THERMAL_H
> >
> >  #define THERMAL_NAME_LENGTH    20
> > +#define THERMAL_THRESHOLD_WAY_UP       0x1
> > +#define THERMAL_THRESHOLD_WAY_DOWN     0x2
>
> It would be somewhat better to use BIT(0) and BIT(1) here IMO, but
> apart from that this patch and patch [2/6] are fine with me (even
> though my implementation of threshold crossing detection would be
> different).

I'm inclined to apply these 2 patches with the change mentioned above,
so that I can base my 6.13 work on them.

I'll send my comments for the [3/6] shortly.

Daniel Lezcano Oct. 9, 2024, 3:40 p.m. UTC | #3

Hi Rafael,

On 02/10/2024 14:22, Rafael J. Wysocki wrote:
> On Tue, Oct 1, 2024 at 9:57 PM Rafael J. Wysocki <[e-mail]> wrote:
>>
>> On Mon, Sep 23, 2024 at 12:00 PM Daniel Lezcano
>> <[e-mail]> wrote:
>>>
>>> The user thresholds mechanism is a way to have the userspace to tell
>>> the thermal framework to send a notification when a temperature limit
>>> is crossed. There is no id, no hysteresis, just the temperature and
>>> the direction of the limit crossing. That means we can be notified
>>> when a threshold is crossed the way up only, or the way down only or
>>> both ways. That allows to create hysteresis values if it is needed.

[ ... ]


> I'm inclined to apply these 2 patches with the change mentioned above,
> so that I can base my 6.13 work on them.

I was expecting you to pick these two patches and do the modifications 
but I don't see them in your tree. Did I misunderstood your comment?

Rafael J. Wysocki Oct. 9, 2024, 3:59 p.m. UTC | #4

Hi Daniel,

On Wed, Oct 9, 2024 at 5:40 PM Daniel Lezcano <[e-mail]> wrote:
>
>
> Hi Rafael,
>
> On 02/10/2024 14:22, Rafael J. Wysocki wrote:
> > On Tue, Oct 1, 2024 at 9:57 PM Rafael J. Wysocki <[e-mail]> wrote:
> >>
> >> On Mon, Sep 23, 2024 at 12:00 PM Daniel Lezcano
> >> <[e-mail]> wrote:
> >>>
> >>> The user thresholds mechanism is a way to have the userspace to tell
> >>> the thermal framework to send a notification when a temperature limit
> >>> is crossed. There is no id, no hysteresis, just the temperature and
> >>> the direction of the limit crossing. That means we can be notified
> >>> when a threshold is crossed the way up only, or the way down only or
> >>> both ways. That allows to create hysteresis values if it is needed.
>
> [ ... ]
>
>
> > I'm inclined to apply these 2 patches with the change mentioned above,
> > so that I can base my 6.13 work on them.
>
> I was expecting you to pick these two patches and do the modifications
> but I don't see them in your tree. Did I misunderstood your comment?

That's still the plan, I've been waiting for you to respond and confirm.

I gather that this is OK then.

Daniel Lezcano Oct. 9, 2024, 4:44 p.m. UTC | #5

On 09/10/2024 17:59, Rafael J. Wysocki wrote:
> Hi Daniel,
> 
> On Wed, Oct 9, 2024 at 5:40 PM Daniel Lezcano <[e-mail]> wrote:
>>
>>
>> Hi Rafael,
>>
>> On 02/10/2024 14:22, Rafael J. Wysocki wrote:
>>> On Tue, Oct 1, 2024 at 9:57 PM Rafael J. Wysocki <[e-mail]> wrote:
>>>>
>>>> On Mon, Sep 23, 2024 at 12:00 PM Daniel Lezcano
>>>> <[e-mail]> wrote:
>>>>>
>>>>> The user thresholds mechanism is a way to have the userspace to tell
>>>>> the thermal framework to send a notification when a temperature limit
>>>>> is crossed. There is no id, no hysteresis, just the temperature and
>>>>> the direction of the limit crossing. That means we can be notified
>>>>> when a threshold is crossed the way up only, or the way down only or
>>>>> both ways. That allows to create hysteresis values if it is needed.
>>
>> [ ... ]
>>
>>
>>> I'm inclined to apply these 2 patches with the change mentioned above,
>>> so that I can base my 6.13 work on them.
>>
>> I was expecting you to pick these two patches and do the modifications
>> but I don't see them in your tree. Did I misunderstood your comment?
> 
> That's still the plan, I've been waiting for you to respond and confirm.
> 
> I gather that this is OK then.

Yes, it is ;)

Rafael J. Wysocki Oct. 10, 2024, 9:44 p.m. UTC | #6

On Wed, Oct 9, 2024 at 6:44 PM Daniel Lezcano <[e-mail]> wrote:
>
> On 09/10/2024 17:59, Rafael J. Wysocki wrote:
> > Hi Daniel,
> >
> > On Wed, Oct 9, 2024 at 5:40 PM Daniel Lezcano <[e-mail]> wrote:
> >>
> >>
> >> Hi Rafael,
> >>
> >> On 02/10/2024 14:22, Rafael J. Wysocki wrote:
> >>> On Tue, Oct 1, 2024 at 9:57 PM Rafael J. Wysocki <[e-mail]> wrote:
> >>>>
> >>>> On Mon, Sep 23, 2024 at 12:00 PM Daniel Lezcano
> >>>> <[e-mail]> wrote:
> >>>>>
> >>>>> The user thresholds mechanism is a way to have the userspace to tell
> >>>>> the thermal framework to send a notification when a temperature limit
> >>>>> is crossed. There is no id, no hysteresis, just the temperature and
> >>>>> the direction of the limit crossing. That means we can be notified
> >>>>> when a threshold is crossed the way up only, or the way down only or
> >>>>> both ways. That allows to create hysteresis values if it is needed.
> >>
> >> [ ... ]
> >>
> >>
> >>> I'm inclined to apply these 2 patches with the change mentioned above,
> >>> so that I can base my 6.13 work on them.
> >>
> >> I was expecting you to pick these two patches and do the modifications
> >> but I don't see them in your tree. Did I misunderstood your comment?
> >
> > That's still the plan, I've been waiting for you to respond and confirm.
> >
> > I gather that this is OK then.
>
> Yes, it is ;)

Applied now, sorry for the delay.

Patch

diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 41c4d56beb40..1e1559bb971e 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -6,6 +6,7 @@  CFLAGS_thermal_core.o		:= -I$(src)
 obj-$(CONFIG_THERMAL)		+= thermal_sys.o
 thermal_sys-y			+= thermal_core.o thermal_sysfs.o
 thermal_sys-y			+= thermal_trip.o thermal_helpers.o
+thermal_sys-y			+= thermal_thresholds.o
 
 # netlink interface to manage the thermal framework
 thermal_sys-$(CONFIG_THERMAL_NETLINK)		+= thermal_netlink.o
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 50b858aa173a..8f320d17d927 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -13,6 +13,7 @@ 
 #include <linux/thermal.h>
 
 #include "thermal_netlink.h"
+#include "thermal_thresholds.h"
 #include "thermal_debugfs.h"
 
 struct thermal_attr {
@@ -139,6 +140,7 @@  struct thermal_zone_device {
 #ifdef CONFIG_THERMAL_DEBUGFS
 	struct thermal_debugfs *debugfs;
 #endif
+	struct list_head user_thresholds;
 	struct thermal_trip_desc trips[] __counted_by(num_trips);
 };
 
diff --git a/drivers/thermal/thermal_thresholds.c b/drivers/thermal/thermal_thresholds.c
new file mode 100644
index 000000000000..f33b6d5474d8
--- /dev/null
+++ b/drivers/thermal/thermal_thresholds.c
@@ -0,0 +1,229 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2024 Linaro Limited
+ *
+ * Author: Daniel Lezcano <[e-mail]>
+ *
+ * Thermal thresholds
+ */
+#include <linux/list.h>
+#include <linux/list_sort.h>
+#include <linux/slab.h>
+
+#include "thermal_core.h"
+#include "thermal_thresholds.h"
+
+int thermal_thresholds_init(struct thermal_zone_device *tz)
+{
+	INIT_LIST_HEAD(&tz->user_thresholds);
+
+	return 0;
+}
+
+void thermal_thresholds_flush(struct thermal_zone_device *tz)
+{
+	struct list_head *thresholds = &tz->user_thresholds;
+	struct user_threshold *entry, *tmp;
+
+	lockdep_assert_held(&tz->lock);
+
+	list_for_each_entry_safe(entry, tmp, thresholds, list_node) {
+		list_del(&entry->list_node);
+		kfree(entry);
+	}
+
+	__thermal_zone_device_update(tz, THERMAL_TZ_FLUSH_THRESHOLDS);
+}
+
+void thermal_thresholds_exit(struct thermal_zone_device *tz)
+{
+	thermal_thresholds_flush(tz);
+}
+
+static int __thermal_thresholds_cmp(void *data,
+				    const struct list_head *l1,
+				    const struct list_head *l2)
+{
+	struct user_threshold *t1 = container_of(l1, struct user_threshold, list_node);
+	struct user_threshold *t2 = container_of(l2, struct user_threshold, list_node);
+
+	return t1->temperature - t2->temperature;
+}
+
+static struct user_threshold *__thermal_thresholds_find(const struct list_head *thresholds,
+							int temperature)
+{
+	struct user_threshold *t;
+
+	list_for_each_entry(t, thresholds, list_node)
+		if (t->temperature == temperature)
+			return t;
+
+	return NULL;
+}
+
+static bool __thermal_threshold_is_crossed(struct user_threshold *threshold, int temperature,
+					   int last_temperature, int direction,
+					   int *low, int *high)
+{
+
+	if (temperature >= threshold->temperature) {
+		if (threshold->temperature > *low &&
+		    THERMAL_THRESHOLD_WAY_DOWN & threshold->direction)
+			*low = threshold->temperature;
+
+		if (last_temperature < threshold->temperature &&
+		    threshold->direction & direction)
+			return true;
+	} else {
+		if (threshold->temperature < *high && THERMAL_THRESHOLD_WAY_UP
+		    & threshold->direction)
+			*high = threshold->temperature;
+
+		if (last_temperature >= threshold->temperature &&
+		    threshold->direction & direction)
+			return true;
+	}
+
+	return false;
+}
+
+static bool thermal_thresholds_handle_raising(struct list_head *thresholds, int temperature,
+					      int last_temperature, int *low, int *high)
+{
+	struct user_threshold *t;
+
+	list_for_each_entry(t, thresholds, list_node) {
+		if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
+						   THERMAL_THRESHOLD_WAY_UP, low, high))
+			return true;
+	}
+
+	return false;
+}
+
+static bool thermal_thresholds_handle_dropping(struct list_head *thresholds, int temperature,
+					       int last_temperature, int *low, int *high)
+{
+	struct user_threshold *t;
+
+	list_for_each_entry_reverse(t, thresholds, list_node) {
+		if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
+						   THERMAL_THRESHOLD_WAY_DOWN, low, high))
+			return true;
+	}
+
+	return false;
+}
+
+void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *high)
+{
+	struct list_head *thresholds = &tz->user_thresholds;
+
+	int temperature = tz->temperature;
+	int last_temperature = tz->last_temperature;
+	bool notify;
+
+	lockdep_assert_held(&tz->lock);
+
+	/*
+	 * We need a second update in order to detect a threshold being crossed
+	 */
+	if (last_temperature == THERMAL_TEMP_INVALID)
+		return;
+
+	/*
+	 * The temperature is stable, so obviously we can not have
+	 * crossed a threshold.
+	 */
+	if (last_temperature == temperature)
+		return;
+
+	/*
+	 * Since last update the temperature:
+	 * - increased : thresholds are crossed the way up
+	 * - decreased : thresholds are crossed the way down
+	 */
+	if (temperature > last_temperature)
+		notify = thermal_thresholds_handle_raising(thresholds, temperature,
+							   last_temperature, low, high);
+	else
+		notify = thermal_thresholds_handle_dropping(thresholds, temperature,
+							    last_temperature, low, high);
+
+	if (notify)
+		pr_debug("A threshold has been crossed the way %s, with a temperature=%d, last_temperature=%d\n",
+			 temperature > last_temperature ? "up" : "down", temperature, last_temperature);
+}
+
+int thermal_thresholds_add(struct thermal_zone_device *tz, int temperature, int direction)
+{
+	struct list_head *thresholds = &tz->user_thresholds;
+	struct user_threshold *t;
+
+	lockdep_assert_held(&tz->lock);
+
+	t = __thermal_thresholds_find(thresholds, temperature);
+	if (t) {
+		if (t->direction == direction)
+			return -EEXIST;
+
+		t->direction |= direction;
+	} else {
+
+		t = kmalloc(sizeof(*t), GFP_KERNEL);
+		if (!t)
+			return -ENOMEM;
+
+		INIT_LIST_HEAD(&t->list_node);
+		t->temperature = temperature;
+		t->direction = direction;
+		list_add(&t->list_node, thresholds);
+		list_sort(NULL, thresholds, __thermal_thresholds_cmp);
+	}
+
+	__thermal_zone_device_update(tz, THERMAL_TZ_ADD_THRESHOLD);
+
+	return 0;
+}
+
+int thermal_thresholds_delete(struct thermal_zone_device *tz, int temperature, int direction)
+{
+	struct list_head *thresholds = &tz->user_thresholds;
+	struct user_threshold *t;
+
+	lockdep_assert_held(&tz->lock);
+
+	t = __thermal_thresholds_find(thresholds, temperature);
+	if (!t)
+		return -ENOENT;
+
+	if (t->direction == direction) {
+		list_del(&t->list_node);
+		kfree(t);
+	} else {
+		t->direction &= ~direction;
+	}
+
+	__thermal_zone_device_update(tz, THERMAL_TZ_DEL_THRESHOLD);
+
+	return 0;
+}
+
+int thermal_thresholds_for_each(struct thermal_zone_device *tz,
+				int (*cb)(struct user_threshold *, void *arg), void *arg)
+{
+	struct list_head *thresholds = &tz->user_thresholds;
+	struct user_threshold *entry;
+	int ret;
+
+	lockdep_assert_held(&tz->lock);
+
+	list_for_each_entry(entry, thresholds, list_node) {
+		ret = cb(entry, arg);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/thermal/thermal_thresholds.h b/drivers/thermal/thermal_thresholds.h
new file mode 100644
index 000000000000..232f4e8089af
--- /dev/null
+++ b/drivers/thermal/thermal_thresholds.h
@@ -0,0 +1,19 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __THERMAL_THRESHOLDS_H__
+#define __THERMAL_THRESHOLDS_H__
+
+struct user_threshold {
+	struct list_head list_node;
+	int temperature;
+	int direction;
+};
+
+int thermal_thresholds_init(struct thermal_zone_device *tz);
+void thermal_thresholds_exit(struct thermal_zone_device *tz);
+void thermal_thresholds_flush(struct thermal_zone_device *tz);
+void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *high);
+int thermal_thresholds_add(struct thermal_zone_device *tz, int temperature, int direction);
+int thermal_thresholds_delete(struct thermal_zone_device *tz, int temperature, int direction);
+int thermal_thresholds_for_each(struct thermal_zone_device *tz,
+				int (*cb)(struct user_threshold *, void *arg), void *arg);
+#endif
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 25ea8fe2313e..bcaa92732e14 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -56,6 +56,9 @@  enum thermal_notify_event {
 	THERMAL_TZ_UNBIND_CDEV, /* Cooling dev is unbind from the thermal zone */
 	THERMAL_INSTANCE_WEIGHT_CHANGED, /* Thermal instance weight changed */
 	THERMAL_TZ_RESUME, /* Thermal zone is resuming after system sleep */
+	THERMAL_TZ_ADD_THRESHOLD, /* Threshold added */
+	THERMAL_TZ_DEL_THRESHOLD, /* Threshold deleted */
+	THERMAL_TZ_FLUSH_THRESHOLDS, /* All thresholds deleted */
 };
 
 /**
diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
index fc78bf3aead7..3e7c1c2e71a7 100644
--- a/include/uapi/linux/thermal.h
+++ b/include/uapi/linux/thermal.h
@@ -3,6 +3,8 @@ 
 #define _UAPI_LINUX_THERMAL_H
 
 #define THERMAL_NAME_LENGTH	20
+#define THERMAL_THRESHOLD_WAY_UP	0x1
+#define THERMAL_THRESHOLD_WAY_DOWN	0x2
 
 enum thermal_device_mode {
 	THERMAL_DEVICE_DISABLED = 0,