[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a9af415d-9fd0-dcea-79ee-0fb90f45045e@linaro.org>
Date: Mon, 18 May 2020 18:37:07 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
rui.zhang@...el.com, amit.kucheria@...durent.com
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [RFC][PATCH 3/5] thermal: Add support for setting notification
thresholds
On 04/05/2020 20:16, Srinivas Pandruvada wrote:
> Add new attributes in thermal syfs when a thermal drivers provides
> callbacks for them and CONFIG_THERMAL_USER_EVENT_INTERFACE is defined.
>
> These attribute allow user space to stop polling for temperature.
>
> These attributes are:
> - temp_thres_low: Specify a notification temperature for a low
> temperature threshold event.
> temp_thres_high: Specify a notification temperature for a high
> temperature threshold event.
> temp_thres_hyst: Specify a change in temperature to send notification
> again.
>
> This is implemented by adding additional sysfs attribute group. The
> changes in this patch are trivial to add new attributes in thermal
> sysfs as done for other attributes.
Isn't it duplicate with the trip point?
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> ---
> drivers/thermal/thermal_sysfs.c | 136 +++++++++++++++++++++++++++++++-
> include/linux/thermal.h | 10 ++-
> 2 files changed, 143 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index aa99edb4dff7..aa85424c3ac4 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -215,6 +215,125 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
> return ret ? ret : sprintf(buf, "%d\n", temperature);
> }
>
> +#if IS_ENABLED(CONFIG_THERMAL_USER_EVENT_INTERFACE)
> +
> +#define create_thres_attr(name) \
> + static ssize_t \
> + name##_show(struct device *dev, struct device_attribute *devattr, \
> + char *buf) \
> + { \
> + struct thermal_zone_device *tz = to_thermal_zone(dev); \
> + int temperature, ret; \
> + \
> + ret = tz->ops->get_##name(tz, &temperature); \
> + \
> + return ret ? ret : sprintf(buf, "%d\n", temperature); \
> + } \
> + \
> + static ssize_t \
> + name##_store(struct device *dev, struct device_attribute *devattr, \
> + const char *buf, size_t count) \
> + { \
> + struct thermal_zone_device *tz = to_thermal_zone(dev); \
> + int temperature, ret; \
> + \
> + if (kstrtoint(buf, 10, &temperature)) \
> + return -EINVAL; \
> + \
> + ret = tz->ops->set_##name(tz, temperature); \
> + return ret ? ret : count; \
> + }
> +
> +create_thres_attr(temp_thres_low);
> +create_thres_attr(temp_thres_high);
> +create_thres_attr(temp_thres_hyst);
> +
> +static int create_user_events_attrs(struct thermal_zone_device *tz)
> +{
> + struct attribute **attrs;
> + int index = 0;
> +
> + if (tz->ops->get_temp_thres_low)
> + ++index;
> + if (tz->ops->get_temp_thres_high)
> + ++index;
> + if (tz->ops->get_temp_thres_high)
> + ++index;
> +
> + /* One additional space for NULL */
> + attrs = kcalloc(index + 1, sizeof(*attrs), GFP_KERNEL);
> + if (!attrs)
> + return -ENOMEM;
> +
> + tz->threshold_attrs = kcalloc(index, sizeof(*tz->threshold_attrs), GFP_KERNEL);
> + if (!tz->threshold_attrs) {
> + kfree(attrs);
> + return -ENOMEM;
> + }
> +
> + index = 0;
> +
> + if (tz->ops->get_temp_thres_low) {
> + snprintf(tz->threshold_attrs[index].name, THERMAL_NAME_LENGTH,
> + "temp_thres_low");
> +
> + sysfs_attr_init(&tz->threshold_attrs[index].attr.attr);
> + tz->threshold_attrs[index].attr.attr.name =
> + tz->threshold_attrs[index].name;
> + tz->threshold_attrs[index].attr.attr.mode = S_IWUSR | S_IRUGO;
> + tz->threshold_attrs[index].attr.show = temp_thres_low_show;
> + tz->threshold_attrs[index].attr.store = temp_thres_low_store;
> + attrs[index] = &tz->threshold_attrs[index].attr.attr;
> + ++index;
> + }
> + if (tz->ops->get_temp_thres_high) {
> + snprintf(tz->threshold_attrs[index].name, THERMAL_NAME_LENGTH,
> + "temp_thres_high");
> +
> + sysfs_attr_init(&tz->threshold_attrs[index].attr.attr);
> + tz->threshold_attrs[index].attr.attr.name =
> + tz->threshold_attrs[index].name;
> + tz->threshold_attrs[index].attr.attr.mode = S_IWUSR | S_IRUGO;
> + tz->threshold_attrs[index].attr.show = temp_thres_high_show;
> + tz->threshold_attrs[index].attr.store = temp_thres_high_store;
> + attrs[index] = &tz->threshold_attrs[index].attr.attr;
> + ++index;
> + }
> + if (tz->ops->get_temp_thres_hyst) {
> + snprintf(tz->threshold_attrs[index].name, THERMAL_NAME_LENGTH,
> + "temp_thres_hyst");
> +
> + sysfs_attr_init(&tz->threshold_attrs[index].attr.attr);
> + tz->threshold_attrs[index].attr.attr.name =
> + tz->threshold_attrs[index].name;
> + tz->threshold_attrs[index].attr.attr.mode = S_IWUSR | S_IRUGO;
> + tz->threshold_attrs[index].attr.show = temp_thres_hyst_show;
> + tz->threshold_attrs[index].attr.store = temp_thres_hyst_store;
> + attrs[index] = &tz->threshold_attrs[index].attr.attr;
> + ++index;
> + }
> + attrs[index] = NULL;
> + tz->threshold_attribute_group.attrs = attrs;
> +
> + return 0;
> +}
> +
> +static void delete_user_events_attrs(struct thermal_zone_device *tz)
> +{
> + kfree(tz->threshold_attrs);
> + kfree(tz->threshold_attribute_group.attrs);
> +}
> +#else
> +static int create_user_events_attrs(struct thermal_zone_device *tz)
> +{
> + return -EINVAL;
> +}
> +
> +static void delete_user_events_attrs(struct thermal_zone_device *tz)
> +{
> +}
> +#endif
> +
> static ssize_t
> passive_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> @@ -625,16 +744,27 @@ int thermal_zone_create_device_groups(struct thermal_zone_device *tz,
> {
> const struct attribute_group **groups;
> int i, size, result;
> + int start = 0;
>
> /* we need one extra for trips and the NULL to terminate the array */
> size = ARRAY_SIZE(thermal_zone_attribute_groups) + 2;
> +
> + result = create_user_events_attrs(tz);
> + if (!result) {
> + ++size;
> + ++start;
> + }
> +
> /* This also takes care of API requirement to be NULL terminated */
> groups = kcalloc(size, sizeof(*groups), GFP_KERNEL);
> if (!groups)
> return -ENOMEM;
>
> - for (i = 0; i < size - 2; i++)
> - groups[i] = thermal_zone_attribute_groups[i];
> + if (start)
> + groups[0] = &tz->threshold_attribute_group;
> +
> + for (i = 0; i < size - 2 - start; i++)
> + groups[i + start] = thermal_zone_attribute_groups[i];
>
> if (tz->trips) {
> result = create_trip_attrs(tz, mask);
> @@ -660,6 +790,8 @@ void thermal_zone_destroy_device_groups(struct thermal_zone_device *tz)
> if (tz->trips)
> destroy_trip_attrs(tz);
>
> + delete_user_events_attrs(tz);
> +
> kfree(tz->device.groups);
> }
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index f5e1e7c6a9a2..ee9d79ace7ce 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -102,6 +102,12 @@ struct thermal_zone_device_ops {
> enum thermal_trend *);
> int (*notify) (struct thermal_zone_device *, int,
> enum thermal_trip_type);
> + int (*set_temp_thres_low)(struct thermal_zone_device *, int);
> + int (*set_temp_thres_high)(struct thermal_zone_device *, int);
> + int (*set_temp_thres_hyst)(struct thermal_zone_device *, int);
> + int (*get_temp_thres_low)(struct thermal_zone_device *, int *);
> + int (*get_temp_thres_high)(struct thermal_zone_device *, int *);
> + int (*get_temp_thres_hyst)(struct thermal_zone_device *, int *);
> };
>
> struct thermal_cooling_device_ops {
> @@ -208,6 +214,8 @@ struct thermal_zone_device {
> struct list_head node;
> struct delayed_work poll_queue;
> enum thermal_notify_event notify_event;
> + struct attribute_group threshold_attribute_group;
> + struct thermal_attr *threshold_attrs;
> };
>
> /**
> @@ -558,7 +566,7 @@ enum thermal_device_events {
> THERMAL_PERF_CHANGED,
> };
>
> -#ifdef CONFIG_THERMAL_USER_EVENT_INTERFACE
> +#if IS_ENABLED(CONFIG_THERMAL_USER_EVENT_INTERFACE)
> int thermal_dev_send_event(int zone_id, enum thermal_device_events event, u64 event_data);
> #else
> int thermal_dev_send_event(int zone_id, enum thermal_device_events event, u64 event_data)
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Powered by blists - more mailing lists