* [RFC][PATCH 1/5] thermal: Add support for /dev/thermal_notify
2020-05-04 18:16 [RFC][PATCH 0/5] thermal: Add new mechanism to get thermal notification Srinivas Pandruvada
@ 2020-05-04 18:16 ` Srinivas Pandruvada
2020-05-20 4:45 ` Amit Kucheria
2020-05-04 18:16 ` [RFC][PATCH 2/5] thermal: Add notification for zone creation and deletion Srinivas Pandruvada
` (3 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-04 18:16 UTC (permalink / raw)
To: rui.zhang, daniel.lezcano, amit.kucheria
Cc: linux-kernel, linux-pm, Srinivas Pandruvada
This change adds an optional feature to add a new device entry
/dev/thermal_notify.
When config CONFIG_THERMAL_USER_EVENT_INTERFACE is selected, this new
device entry is created.
Thermal core or any thermal driver can use thermal_dev_send_event() interface
to send events. Each user events follows a standard format:
- zone_id
- event_id
- event_data
- reserved for future, currently 0s
User space can basically:
fd = open ("/dev/thermal_notify")
In a loop
read (fd)
read and process event
or
fd = open ("/dev/thermal_notify")
Set the fs as non blocking
In a loop
Use poll() and wait
read and process event
There are predefined events added to thermal.h. Based on need they can
be extended.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/thermal/Kconfig | 9 ++
drivers/thermal/Makefile | 3 +
drivers/thermal/thermal_dev_if.c | 195 +++++++++++++++++++++++++++++++
include/linux/thermal.h | 24 ++++
4 files changed, 231 insertions(+)
create mode 100644 drivers/thermal/thermal_dev_if.c
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 91af271e9bb0..27d05d62458e 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -78,6 +78,15 @@ config THERMAL_WRITABLE_TRIPS
Say 'Y' here if you would like to allow userspace tools to
change trip temperatures.
+config THERMAL_USER_EVENT_INTERFACE
+ bool "Allow user space to read thermal events from a dev file"
+ help
+ This option allows a user space program to read thermal events
+ via /dev/thermal_notify file.
+
+ Say 'Y' here if you would like to allow userspace programs to
+ read thermal events.
+
choice
prompt "Default Thermal governor"
default THERMAL_DEFAULT_GOV_STEP_WISE
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 8c8ed7b79915..8f65832d755a 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -11,6 +11,9 @@ thermal_sys-y += thermal_core.o thermal_sysfs.o \
thermal_sys-$(CONFIG_THERMAL_HWMON) += thermal_hwmon.o
thermal_sys-$(CONFIG_THERMAL_OF) += of-thermal.o
+# Thermal user space events
+obj-$(CONFIG_THERMAL_USER_EVENT_INTERFACE) += thermal_dev_if.o
+
# governors
thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE) += fair_share.o
thermal_sys-$(CONFIG_THERMAL_GOV_BANG_BANG) += gov_bang_bang.o
diff --git a/drivers/thermal/thermal_dev_if.c b/drivers/thermal/thermal_dev_if.c
new file mode 100644
index 000000000000..763bfe9eef9d
--- /dev/null
+++ b/drivers/thermal/thermal_dev_if.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Thermal device file interface
+ * Copyright (c) 2020, Intel Corporation.
+ * All rights reserved.
+ *
+ * Author: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
+ */
+
+#include <linux/init.h>
+#include <linux/kfifo.h>
+#include <linux/miscdevice.h>
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/thermal.h>
+#include <linux/wait.h>
+
+#define THERMAL_DEV_FIFO_SIZE 1024
+
+struct thermal_chdev_sample {
+ int zone_id;
+ int event;
+ u64 event_data;
+ u64 reserved;
+};
+
+struct thermal_chdev {
+ struct miscdevice therm_dev;
+ struct kfifo data_fifo;
+ unsigned long misc_opened;
+ wait_queue_head_t wait;
+};
+
+static DEFINE_MUTEX(thermal_chdev_mutex);
+static struct thermal_chdev *thermal_chdev;
+
+static int thermal_chdev_open(struct inode *inode, struct file *file)
+{
+ struct thermal_chdev *chdev;
+
+ chdev = container_of(file->private_data, struct thermal_chdev, therm_dev);
+
+ /* We essentially have single reader and writer */
+ if (test_and_set_bit(0, &chdev->misc_opened))
+ return -EBUSY;
+
+ return stream_open(inode, file);
+}
+
+static int thermal_chdev_release(struct inode *inode, struct file *file)
+{
+ struct thermal_chdev *chdev;
+
+ chdev = container_of(file->private_data, struct thermal_chdev, therm_dev);
+
+ clear_bit(0, &chdev->misc_opened);
+
+ return 0;
+}
+
+static __poll_t thermal_chdev_poll(struct file *file, struct poll_table_struct *wait)
+{
+ struct thermal_chdev *chdev;
+ __poll_t mask = 0;
+
+ chdev = container_of(file->private_data, struct thermal_chdev, therm_dev);
+
+ poll_wait(file, &chdev->wait, wait);
+
+ if (!kfifo_is_empty(&chdev->data_fifo))
+ mask = EPOLLIN | EPOLLRDNORM;
+
+ return mask;
+}
+
+static ssize_t thermal_chdev_read(struct file *file, char __user *buf, size_t count, loff_t *f_ps)
+{
+ struct thermal_chdev *chdev;
+ unsigned int copied;
+ int ret;
+
+ chdev = container_of(file->private_data, struct thermal_chdev, therm_dev);
+
+ if (count < sizeof(struct thermal_chdev_sample))
+ return -EINVAL;
+
+ do {
+ if (kfifo_is_empty(&chdev->data_fifo)) {
+ if (file->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+
+ ret = wait_event_interruptible(chdev->wait, !kfifo_is_empty(&chdev->data_fifo));
+ if (ret)
+ return ret;
+ }
+ ret = kfifo_to_user(&chdev->data_fifo, buf, count, &copied);
+ if (ret)
+ return ret;
+
+ } while (copied == 0);
+
+ return copied;
+}
+
+static int thermal_chdev_capture_sample(struct thermal_chdev *chdev, struct thermal_chdev_sample *sample)
+{
+ int ret = 0;
+
+ if (!test_bit(0, &chdev->misc_opened))
+ return 0;
+
+ mutex_lock(&thermal_chdev_mutex);
+ if (kfifo_avail(&chdev->data_fifo) >= sizeof(*sample)) {
+ kfifo_in(&chdev->data_fifo, (unsigned char *)sample, sizeof(*sample));
+ wake_up(&chdev->wait);
+ } else {
+ ret = -ENOMEM;
+ }
+ mutex_unlock(&thermal_chdev_mutex);
+
+ return ret;
+}
+
+static const struct file_operations thermal_chdev_fops = {
+ .open = thermal_chdev_open,
+ .read = thermal_chdev_read,
+ .release = thermal_chdev_release,
+ .poll = thermal_chdev_poll,
+ .llseek = noop_llseek,
+};
+
+static int thermal_dev_if_add(void)
+{
+ int ret;
+
+ if (thermal_chdev)
+ return 0;
+
+ thermal_chdev = kzalloc(sizeof(*thermal_chdev), GFP_KERNEL);
+ if (!thermal_chdev)
+ return -ENOMEM;
+
+ ret = kfifo_alloc(&thermal_chdev->data_fifo, THERMAL_DEV_FIFO_SIZE, GFP_KERNEL);
+ if (ret)
+ goto free_mem;
+
+ init_waitqueue_head(&thermal_chdev->wait);
+
+ thermal_chdev->therm_dev.minor = MISC_DYNAMIC_MINOR;
+ thermal_chdev->therm_dev.name = "thermal_notify";
+ thermal_chdev->therm_dev.fops = &thermal_chdev_fops;
+ ret = misc_register(&thermal_chdev->therm_dev);
+ if (ret) {
+ kfifo_free(&thermal_chdev->data_fifo);
+ goto free_mem;
+ }
+
+ return 0;
+
+free_mem:
+ kfree(thermal_chdev);
+ thermal_chdev = NULL;
+ return ret;
+}
+
+/**
+ * thermal_dev_send_event() - Send thermal event to user space
+ * @zone_id: Zone id of the caller
+ * @event: A predefined thermal event
+ * @event_data: Event specific data
+ *
+ * An interface to send event to user space with an optional associated
+ * data.
+ *
+ * Return: 0 on success, other values on error.
+ */
+int thermal_dev_send_event(int zone_id, enum thermal_device_events event, u64 event_data)
+{
+ struct thermal_chdev_sample sample;
+
+ if (!thermal_chdev) {
+ int ret;
+
+ ret = thermal_dev_if_add();
+ if (ret)
+ return ret;
+ }
+
+ sample.zone_id = zone_id;
+ sample.event = event;
+ sample.event_data = event_data;
+ sample.reserved = 0;
+ return thermal_chdev_capture_sample(thermal_chdev, &sample);
+}
+EXPORT_SYMBOL_GPL(thermal_dev_send_event);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index c91b1e344d56..f5e1e7c6a9a2 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -543,4 +543,28 @@ static inline void thermal_notify_framework(struct thermal_zone_device *tz,
{ }
#endif /* CONFIG_THERMAL */
+enum thermal_device_events {
+ THERMAL_TEMP_SAMPLE = 0,
+ THERMAL_ZONE_CREATE,
+ THERMAL_ZONE_DELETE,
+ THERMAL_ZONE_DISABLED,
+ THERMAL_ZONE_ENABLED,
+ THERMAL_TEMP_LOW_THRES,
+ THERMAL_TEMP_HIGH_THRES,
+ THERMAL_TRIP_ADD,
+ THERMAL_TRIP_DELETE,
+ THERMAL_TRIP_UPDATE,
+ THERMAL_TRIP_REACHED,
+ THERMAL_PERF_CHANGED,
+};
+
+#ifdef 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)
+{
+ return 0;
+}
+#endif
+
#endif /* __THERMAL_H__ */
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC][PATCH 1/5] thermal: Add support for /dev/thermal_notify
2020-05-04 18:16 ` [RFC][PATCH 1/5] thermal: Add support for /dev/thermal_notify Srinivas Pandruvada
@ 2020-05-20 4:45 ` Amit Kucheria
0 siblings, 0 replies; 19+ messages in thread
From: Amit Kucheria @ 2020-05-20 4:45 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Zhang Rui, Daniel Lezcano, LKML, Linux PM list
On Mon, May 4, 2020 at 11:47 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> This change adds an optional feature to add a new device entry
> /dev/thermal_notify.
>
> When config CONFIG_THERMAL_USER_EVENT_INTERFACE is selected, this new
> device entry is created.
>
> Thermal core or any thermal driver can use thermal_dev_send_event() interface
Do you have any particular use case in mind where a platform driver
will use this interface to send platform-specific events?
IMO, we should probably try to keep this restricted to messages from
thermal core if we are to have any hope of having a standard library
in userspace capable of parsing these thermal events.
> to send events. Each user events follows a standard format:
> - zone_id
> - event_id
> - event_data
> - reserved for future, currently 0s
>
> User space can basically:
> fd = open ("/dev/thermal_notify")
> In a loop
> read (fd)
> read and process event
>
> or
> fd = open ("/dev/thermal_notify")
> Set the fs as non blocking
> In a loop
> Use poll() and wait
> read and process event
>
> There are predefined events added to thermal.h. Based on need they can
> be extended.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/thermal/Kconfig | 9 ++
> drivers/thermal/Makefile | 3 +
> drivers/thermal/thermal_dev_if.c | 195 +++++++++++++++++++++++++++++++
> include/linux/thermal.h | 24 ++++
> 4 files changed, 231 insertions(+)
> create mode 100644 drivers/thermal/thermal_dev_if.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 91af271e9bb0..27d05d62458e 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -78,6 +78,15 @@ config THERMAL_WRITABLE_TRIPS
> Say 'Y' here if you would like to allow userspace tools to
> change trip temperatures.
>
> +config THERMAL_USER_EVENT_INTERFACE
> + bool "Allow user space to read thermal events from a dev file"
> + help
> + This option allows a user space program to read thermal events
> + via /dev/thermal_notify file.
> +
> + Say 'Y' here if you would like to allow userspace programs to
> + read thermal events.
> +
> choice
> prompt "Default Thermal governor"
> default THERMAL_DEFAULT_GOV_STEP_WISE
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 8c8ed7b79915..8f65832d755a 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -11,6 +11,9 @@ thermal_sys-y += thermal_core.o thermal_sysfs.o \
> thermal_sys-$(CONFIG_THERMAL_HWMON) += thermal_hwmon.o
> thermal_sys-$(CONFIG_THERMAL_OF) += of-thermal.o
>
> +# Thermal user space events
> +obj-$(CONFIG_THERMAL_USER_EVENT_INTERFACE) += thermal_dev_if.o
> +
> # governors
> thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE) += fair_share.o
> thermal_sys-$(CONFIG_THERMAL_GOV_BANG_BANG) += gov_bang_bang.o
> diff --git a/drivers/thermal/thermal_dev_if.c b/drivers/thermal/thermal_dev_if.c
> new file mode 100644
> index 000000000000..763bfe9eef9d
> --- /dev/null
> +++ b/drivers/thermal/thermal_dev_if.c
> @@ -0,0 +1,195 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Thermal device file interface
> + * Copyright (c) 2020, Intel Corporation.
> + * All rights reserved.
> + *
> + * Author: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> + */
> +
> +#include <linux/init.h>
> +#include <linux/kfifo.h>
> +#include <linux/miscdevice.h>
> +#include <linux/poll.h>
> +#include <linux/slab.h>
> +#include <linux/thermal.h>
> +#include <linux/wait.h>
> +
> +#define THERMAL_DEV_FIFO_SIZE 1024
> +
> +struct thermal_chdev_sample {
> + int zone_id;
> + int event;
> + u64 event_data;
> + u64 reserved;
> +};
> +
> +struct thermal_chdev {
> + struct miscdevice therm_dev;
> + struct kfifo data_fifo;
> + unsigned long misc_opened;
> + wait_queue_head_t wait;
> +};
> +
> +static DEFINE_MUTEX(thermal_chdev_mutex);
> +static struct thermal_chdev *thermal_chdev;
> +
> +static int thermal_chdev_open(struct inode *inode, struct file *file)
> +{
> + struct thermal_chdev *chdev;
> +
> + chdev = container_of(file->private_data, struct thermal_chdev, therm_dev);
> +
> + /* We essentially have single reader and writer */
> + if (test_and_set_bit(0, &chdev->misc_opened))
> + return -EBUSY;
> +
> + return stream_open(inode, file);
> +}
> +
> +static int thermal_chdev_release(struct inode *inode, struct file *file)
> +{
> + struct thermal_chdev *chdev;
> +
> + chdev = container_of(file->private_data, struct thermal_chdev, therm_dev);
> +
> + clear_bit(0, &chdev->misc_opened);
> +
> + return 0;
> +}
> +
> +static __poll_t thermal_chdev_poll(struct file *file, struct poll_table_struct *wait)
> +{
> + struct thermal_chdev *chdev;
> + __poll_t mask = 0;
> +
> + chdev = container_of(file->private_data, struct thermal_chdev, therm_dev);
> +
> + poll_wait(file, &chdev->wait, wait);
> +
> + if (!kfifo_is_empty(&chdev->data_fifo))
> + mask = EPOLLIN | EPOLLRDNORM;
> +
> + return mask;
> +}
> +
> +static ssize_t thermal_chdev_read(struct file *file, char __user *buf, size_t count, loff_t *f_ps)
> +{
> + struct thermal_chdev *chdev;
> + unsigned int copied;
> + int ret;
> +
> + chdev = container_of(file->private_data, struct thermal_chdev, therm_dev);
> +
> + if (count < sizeof(struct thermal_chdev_sample))
> + return -EINVAL;
> +
> + do {
> + if (kfifo_is_empty(&chdev->data_fifo)) {
> + if (file->f_flags & O_NONBLOCK)
> + return -EAGAIN;
> +
> + ret = wait_event_interruptible(chdev->wait, !kfifo_is_empty(&chdev->data_fifo));
> + if (ret)
> + return ret;
> + }
> + ret = kfifo_to_user(&chdev->data_fifo, buf, count, &copied);
> + if (ret)
> + return ret;
> +
> + } while (copied == 0);
> +
> + return copied;
> +}
> +
> +static int thermal_chdev_capture_sample(struct thermal_chdev *chdev, struct thermal_chdev_sample *sample)
> +{
> + int ret = 0;
> +
> + if (!test_bit(0, &chdev->misc_opened))
> + return 0;
> +
> + mutex_lock(&thermal_chdev_mutex);
> + if (kfifo_avail(&chdev->data_fifo) >= sizeof(*sample)) {
> + kfifo_in(&chdev->data_fifo, (unsigned char *)sample, sizeof(*sample));
> + wake_up(&chdev->wait);
> + } else {
> + ret = -ENOMEM;
> + }
> + mutex_unlock(&thermal_chdev_mutex);
> +
> + return ret;
> +}
> +
> +static const struct file_operations thermal_chdev_fops = {
> + .open = thermal_chdev_open,
> + .read = thermal_chdev_read,
> + .release = thermal_chdev_release,
> + .poll = thermal_chdev_poll,
> + .llseek = noop_llseek,
> +};
> +
> +static int thermal_dev_if_add(void)
> +{
> + int ret;
> +
> + if (thermal_chdev)
> + return 0;
> +
> + thermal_chdev = kzalloc(sizeof(*thermal_chdev), GFP_KERNEL);
> + if (!thermal_chdev)
> + return -ENOMEM;
> +
> + ret = kfifo_alloc(&thermal_chdev->data_fifo, THERMAL_DEV_FIFO_SIZE, GFP_KERNEL);
> + if (ret)
> + goto free_mem;
> +
> + init_waitqueue_head(&thermal_chdev->wait);
> +
> + thermal_chdev->therm_dev.minor = MISC_DYNAMIC_MINOR;
> + thermal_chdev->therm_dev.name = "thermal_notify";
> + thermal_chdev->therm_dev.fops = &thermal_chdev_fops;
> + ret = misc_register(&thermal_chdev->therm_dev);
> + if (ret) {
> + kfifo_free(&thermal_chdev->data_fifo);
> + goto free_mem;
> + }
> +
> + return 0;
> +
> +free_mem:
> + kfree(thermal_chdev);
> + thermal_chdev = NULL;
> + return ret;
> +}
> +
> +/**
> + * thermal_dev_send_event() - Send thermal event to user space
> + * @zone_id: Zone id of the caller
> + * @event: A predefined thermal event
> + * @event_data: Event specific data
> + *
> + * An interface to send event to user space with an optional associated
> + * data.
> + *
> + * Return: 0 on success, other values on error.
> + */
> +int thermal_dev_send_event(int zone_id, enum thermal_device_events event, u64 event_data)
> +{
> + struct thermal_chdev_sample sample;
> +
> + if (!thermal_chdev) {
> + int ret;
> +
> + ret = thermal_dev_if_add();
> + if (ret)
> + return ret;
> + }
> +
> + sample.zone_id = zone_id;
> + sample.event = event;
> + sample.event_data = event_data;
> + sample.reserved = 0;
> + return thermal_chdev_capture_sample(thermal_chdev, &sample);
> +}
> +EXPORT_SYMBOL_GPL(thermal_dev_send_event);
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index c91b1e344d56..f5e1e7c6a9a2 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -543,4 +543,28 @@ static inline void thermal_notify_framework(struct thermal_zone_device *tz,
> { }
> #endif /* CONFIG_THERMAL */
>
> +enum thermal_device_events {
> + THERMAL_TEMP_SAMPLE = 0,
> + THERMAL_ZONE_CREATE,
> + THERMAL_ZONE_DELETE,
> + THERMAL_ZONE_DISABLED,
> + THERMAL_ZONE_ENABLED,
> + THERMAL_TEMP_LOW_THRES,
> + THERMAL_TEMP_HIGH_THRES,
> + THERMAL_TRIP_ADD,
> + THERMAL_TRIP_DELETE,
> + THERMAL_TRIP_UPDATE,
> + THERMAL_TRIP_REACHED,
> + THERMAL_PERF_CHANGED,
> +};
> +
> +#ifdef 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)
> +{
> + return 0;
> +}
> +#endif
> +
> #endif /* __THERMAL_H__ */
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC][PATCH 2/5] thermal: Add notification for zone creation and deletion
2020-05-04 18:16 [RFC][PATCH 0/5] thermal: Add new mechanism to get thermal notification Srinivas Pandruvada
2020-05-04 18:16 ` [RFC][PATCH 1/5] thermal: Add support for /dev/thermal_notify Srinivas Pandruvada
@ 2020-05-04 18:16 ` Srinivas Pandruvada
2020-05-04 18:16 ` [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds Srinivas Pandruvada
` (2 subsequent siblings)
4 siblings, 0 replies; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-04 18:16 UTC (permalink / raw)
To: rui.zhang, daniel.lezcano, amit.kucheria
Cc: linux-kernel, linux-pm, Srinivas Pandruvada
Whenever thermal_zone_device_register() is called and zone is created
successfully, send a notification "THERMAL_ZONE_CREATE".
Similarly send "THERMAL_ZONE_DELETE" when thermal_zone_device_unregister
is called.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/thermal/thermal_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 9a321dc548c8..14770d882d42 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1350,6 +1350,8 @@ thermal_zone_device_register(const char *type, int trips, int mask,
if (atomic_cmpxchg(&tz->need_update, 1, 0))
thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+ thermal_dev_send_event(tz->id, THERMAL_ZONE_CREATE, 0);
+
return tz;
unregister:
@@ -1379,6 +1381,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
if (!tz)
return;
+ thermal_dev_send_event(tz->id, THERMAL_ZONE_DELETE, 0);
+
tzp = tz->tzp;
mutex_lock(&thermal_list_lock);
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds
2020-05-04 18:16 [RFC][PATCH 0/5] thermal: Add new mechanism to get thermal notification Srinivas Pandruvada
2020-05-04 18:16 ` [RFC][PATCH 1/5] thermal: Add support for /dev/thermal_notify Srinivas Pandruvada
2020-05-04 18:16 ` [RFC][PATCH 2/5] thermal: Add notification for zone creation and deletion Srinivas Pandruvada
@ 2020-05-04 18:16 ` Srinivas Pandruvada
2020-05-18 16:37 ` Daniel Lezcano
2020-05-04 18:16 ` [RFC][PATCH 4/5] thermal: Add support for setting polling interval Srinivas Pandruvada
2020-05-04 18:16 ` [RFC][PATCH 5/5] thermal: int340x: Use new device interface Srinivas Pandruvada
4 siblings, 1 reply; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-04 18:16 UTC (permalink / raw)
To: rui.zhang, daniel.lezcano, amit.kucheria
Cc: linux-kernel, linux-pm, Srinivas Pandruvada
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.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.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)
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds
2020-05-04 18:16 ` [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds Srinivas Pandruvada
@ 2020-05-18 16:37 ` Daniel Lezcano
2020-05-18 23:40 ` Srinivas Pandruvada
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Lezcano @ 2020-05-18 16:37 UTC (permalink / raw)
To: Srinivas Pandruvada, rui.zhang, amit.kucheria; +Cc: linux-kernel, linux-pm
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@linux.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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds
2020-05-18 16:37 ` Daniel Lezcano
@ 2020-05-18 23:40 ` Srinivas Pandruvada
2020-05-20 4:28 ` Amit Kucheria
0 siblings, 1 reply; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-18 23:40 UTC (permalink / raw)
To: Daniel Lezcano, rui.zhang, amit.kucheria; +Cc: linux-kernel, linux-pm
On Mon, 2020-05-18 at 18:37 +0200, Daniel Lezcano wrote:
> 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?
A trip point is where an in-kernel governor takes some action. This is
not same as a notification temperature. For example at trip point
configured by ACPI at 85C, the thermal governor may start aggressive
throttling.
But a user space can set a notification threshold at 80C and start some
active controls like activate some fan to reduce the impact of passive
control on performance.
We need a way to distinguish between temperature notification threshold
and actual trip point. Changing a trip point means that user wants
kernel to throttle at temperature.
Thanks,
Srinivas
>
>
>
>
> > Signed-off-by: Srinivas Pandruvada <
> > srinivas.pandruvada@linux.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)
> >
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds
2020-05-18 23:40 ` Srinivas Pandruvada
@ 2020-05-20 4:28 ` Amit Kucheria
2020-05-20 18:16 ` Srinivas Pandruvada
0 siblings, 1 reply; 19+ messages in thread
From: Amit Kucheria @ 2020-05-20 4:28 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Daniel Lezcano, Zhang Rui, LKML, Linux PM list
On Tue, May 19, 2020 at 5:10 AM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Mon, 2020-05-18 at 18:37 +0200, Daniel Lezcano wrote:
> > 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?
> A trip point is where an in-kernel governor takes some action. This is
> not same as a notification temperature. For example at trip point
> configured by ACPI at 85C, the thermal governor may start aggressive
> throttling.
> But a user space can set a notification threshold at 80C and start some
> active controls like activate some fan to reduce the impact of passive
> control on performance.
Then what is the use of thermal trip type "ACTIVE" ?
> We need a way to distinguish between temperature notification threshold
> and actual trip point. Changing a trip point means that user wants
> kernel to throttle at temperature.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds
2020-05-20 4:28 ` Amit Kucheria
@ 2020-05-20 18:16 ` Srinivas Pandruvada
2020-05-21 5:11 ` Amit Kucheria
0 siblings, 1 reply; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-20 18:16 UTC (permalink / raw)
To: Amit Kucheria; +Cc: Daniel Lezcano, Zhang Rui, LKML, Linux PM list
On Wed, 2020-05-20 at 09:58 +0530, Amit Kucheria wrote:
> On Tue, May 19, 2020 at 5:10 AM Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > On Mon, 2020-05-18 at 18:37 +0200, Daniel Lezcano wrote:
> > > 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?
> > A trip point is where an in-kernel governor takes some action. This
> > is
> > not same as a notification temperature. For example at trip point
> > configured by ACPI at 85C, the thermal governor may start
> > aggressive
> > throttling.
> > But a user space can set a notification threshold at 80C and start
> > some
> > active controls like activate some fan to reduce the impact of
> > passive
> > control on performance.
>
> Then what is the use of thermal trip type "ACTIVE" ?
This is an example.
The defaults are set by the OEMs via ACPI. User can't modify that if
they want to optimize for their usage on Linux. There are fan control
daemon's which user use on top.
Thanks,
Srinivas
>
> > We need a way to distinguish between temperature notification
> > threshold
> > and actual trip point. Changing a trip point means that user wants
> > kernel to throttle at temperature.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds
2020-05-20 18:16 ` Srinivas Pandruvada
@ 2020-05-21 5:11 ` Amit Kucheria
2020-05-21 19:11 ` Srinivas Pandruvada
0 siblings, 1 reply; 19+ messages in thread
From: Amit Kucheria @ 2020-05-21 5:11 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Daniel Lezcano, Zhang Rui, LKML, Linux PM list
Hi Srinivas,
On Wed, May 20, 2020 at 11:46 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Wed, 2020-05-20 at 09:58 +0530, Amit Kucheria wrote:
> > On Tue, May 19, 2020 at 5:10 AM Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com> wrote:
> > > On Mon, 2020-05-18 at 18:37 +0200, Daniel Lezcano wrote:
> > > > 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?
> > > A trip point is where an in-kernel governor takes some action. This
> > > is
> > > not same as a notification temperature. For example at trip point
> > > configured by ACPI at 85C, the thermal governor may start
> > > aggressive
> > > throttling.
> > > But a user space can set a notification threshold at 80C and start
> > > some
> > > active controls like activate some fan to reduce the impact of
> > > passive
> > > control on performance.
> >
> > Then what is the use of thermal trip type "ACTIVE" ?
> This is an example.
> The defaults are set by the OEMs via ACPI. User can't modify that if
> they want to optimize for their usage on Linux. There are fan control
> daemon's which user use on top.
-ENOPARSE. Are you saying users "can" modify these?
In any case, how is what you described earlier not possible with an
ACTIVE trip point directly wired to the fan as a cooling device or
with a HOT trip point that causes the platform driver to send
notification to userspace where a fan control daemon can do what it
needs to?
Basically, I think the issue of polling is orthogonal to the
introduction of the new attributes introduced in this patch and I
don't understand the reason for these attributes from your commit
description.
> > > We need a way to distinguish between temperature notification
> > > threshold
> > > and actual trip point. Changing a trip point means that user wants
> > > kernel to throttle at temperature.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds
2020-05-21 5:11 ` Amit Kucheria
@ 2020-05-21 19:11 ` Srinivas Pandruvada
0 siblings, 0 replies; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-21 19:11 UTC (permalink / raw)
To: Amit Kucheria; +Cc: Daniel Lezcano, Zhang Rui, LKML, Linux PM list
Hi Amit,
On Thu, 2020-05-21 at 10:41 +0530, Amit Kucheria wrote:
> Hi Srinivas,
>
> On Wed, May 20, 2020 at 11:46 PM Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > On Wed, 2020-05-20 at 09:58 +0530, Amit Kucheria wrote:
> > > On Tue, May 19, 2020 at 5:10 AM Srinivas Pandruvada
> > > <srinivas.pandruvada@linux.intel.com> wrote:
> > > > On Mon, 2020-05-18 at 18:37 +0200, Daniel Lezcano wrote:
> > > > > 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?
> > > > A trip point is where an in-kernel governor takes some action.
> > > > This
> > > > is
> > > > not same as a notification temperature. For example at trip
> > > > point
> > > > configured by ACPI at 85C, the thermal governor may start
> > > > aggressive
> > > > throttling.
> > > > But a user space can set a notification threshold at 80C and
> > > > start
> > > > some
> > > > active controls like activate some fan to reduce the impact of
> > > > passive
> > > > control on performance.
> > >
> > > Then what is the use of thermal trip type "ACTIVE" ?
> > This is an example.
> > The defaults are set by the OEMs via ACPI. User can't modify that
> > if
> > they want to optimize for their usage on Linux. There are fan
> > control
> > daemon's which user use on top.
>
> -ENOPARSE. Are you saying users "can" modify these?
Most of the x86 laptops will not have an active trip as the fan control
is done by embedded controller. This is a safety and regulatory issue.
Even when you have an active trip it will be read only and also ACPI
fan cooling device will have few fix states to control.
There are fine grain controls on top are available outside of thermal
drivers via hwmon or others.
https://wiki.archlinux.org/index.php/Fan_speed_control#ThinkPad_laptops
Like in thermald we have XML config, which can be used to set different
speed levels at different temperatures. Instead of polling of
temperature, these attributes allow notification of temperature
threshold. We currently mimic this behavior via adding a RW passive
trip (The RW passive trips has a well defined usage different than what
we are using for).
There can be already existing RO passive/active trips in that zone
already bound to some cooling device. So from user space we search for
some RW passive trip and hope this is will give notifications. This I
believe is a hack to use a fake trip point for notifications for
temperature thresholds.
Thanks,
Srinivas
>
> In any case, how is what you described earlier not possible with an
> ACTIVE trip point directly wired to the fan as a cooling device or
> with a HOT trip point that causes the platform driver to send
> notification to userspace where a fan control daemon can do what it
> needs to?
>
> Basically, I think the issue of polling is orthogonal to the
> introduction of the new attributes introduced in this patch and I
> don't understand the reason for these attributes from your commit
> description.
>
> > > > We need a way to distinguish between temperature notification
> > > > threshold
> > > > and actual trip point. Changing a trip point means that user
> > > > wants
> > > > kernel to throttle at temperature.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC][PATCH 4/5] thermal: Add support for setting polling interval
2020-05-04 18:16 [RFC][PATCH 0/5] thermal: Add new mechanism to get thermal notification Srinivas Pandruvada
` (2 preceding siblings ...)
2020-05-04 18:16 ` [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds Srinivas Pandruvada
@ 2020-05-04 18:16 ` Srinivas Pandruvada
2020-05-18 16:51 ` Daniel Lezcano
2020-05-20 4:38 ` Amit Kucheria
2020-05-04 18:16 ` [RFC][PATCH 5/5] thermal: int340x: Use new device interface Srinivas Pandruvada
4 siblings, 2 replies; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-04 18:16 UTC (permalink / raw)
To: rui.zhang, daniel.lezcano, amit.kucheria
Cc: linux-kernel, linux-pm, Srinivas Pandruvada
Add new attribute in the thermal syfs for setting temperature sampling
interval when CONFIG_THERMAL_USER_EVENT_INTERFACE is defined. The default
value is 0, which means no polling.
At this interval user space will get an event THERMAL_TEMP_SAMPLE with
temperature sample. This reuses existing polling mecahnism when polling
or passive delay is specified during zone registry. To avoid interference
with passive and polling delay, this new polling attribute can't be used
for those zones.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/thermal/thermal_core.c | 7 +++++++
drivers/thermal/thermal_sysfs.c | 36 +++++++++++++++++++++++++++++++--
include/linux/thermal.h | 1 +
3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 14770d882d42..17cd799b0073 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -313,6 +313,8 @@ static void monitor_thermal_zone(struct thermal_zone_device *tz)
thermal_zone_device_set_polling(tz, tz->passive_delay);
else if (tz->polling_delay)
thermal_zone_device_set_polling(tz, tz->polling_delay);
+ else if (tz->temp_polling_delay)
+ thermal_zone_device_set_polling(tz, tz->temp_polling_delay);
else
thermal_zone_device_set_polling(tz, 0);
@@ -446,6 +448,11 @@ static void update_temperature(struct thermal_zone_device *tz)
tz->temperature = temp;
mutex_unlock(&tz->lock);
+ if (tz->temp_polling_delay) {
+ thermal_dev_send_event(tz->id, THERMAL_TEMP_SAMPLE, temp);
+ monitor_thermal_zone(tz);
+ }
+
trace_thermal_temperature(tz);
if (tz->last_temperature == THERMAL_TEMP_INVALID)
dev_dbg(&tz->device, "last_temperature N/A, current_temperature=%d\n",
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index aa85424c3ac4..0df7997993fe 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -248,6 +248,36 @@ create_thres_attr(temp_thres_low);
create_thres_attr(temp_thres_high);
create_thres_attr(temp_thres_hyst);
+static ssize_t
+temp_polling_delay_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct thermal_zone_device *tz = to_thermal_zone(dev);
+ int val;
+
+ if (kstrtoint(buf, 10, &val))
+ return -EINVAL;
+
+ if (val && val < 1000)
+ return -EINVAL;
+
+ tz->temp_polling_delay = val;
+ thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+
+ return count;
+}
+
+static ssize_t
+temp_polling_delay_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct thermal_zone_device *tz = to_thermal_zone(dev);
+
+ return sprintf(buf, "%d\n", tz->temp_polling_delay);
+}
+
+static DEVICE_ATTR_RW(temp_polling_delay);
+
static int create_user_events_attrs(struct thermal_zone_device *tz)
{
struct attribute **attrs;
@@ -260,8 +290,8 @@ static int create_user_events_attrs(struct thermal_zone_device *tz)
if (tz->ops->get_temp_thres_high)
++index;
- /* One additional space for NULL */
- attrs = kcalloc(index + 1, sizeof(*attrs), GFP_KERNEL);
+ /* One additional space for NULL and temp_pollling_delay */
+ attrs = kcalloc(index + 2, sizeof(*attrs), GFP_KERNEL);
if (!attrs)
return -ENOMEM;
@@ -312,6 +342,8 @@ static int create_user_events_attrs(struct thermal_zone_device *tz)
attrs[index] = &tz->threshold_attrs[index].attr.attr;
++index;
}
+ if (!tz->polling_delay && !tz->passive_delay)
+ attrs[index++] = &dev_attr_temp_polling_delay.attr;
attrs[index] = NULL;
tz->threshold_attribute_group.attrs = attrs;
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index ee9d79ace7ce..0ec4bd8c9c5c 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -216,6 +216,7 @@ struct thermal_zone_device {
enum thermal_notify_event notify_event;
struct attribute_group threshold_attribute_group;
struct thermal_attr *threshold_attrs;
+ int temp_polling_delay;
};
/**
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC][PATCH 4/5] thermal: Add support for setting polling interval
2020-05-04 18:16 ` [RFC][PATCH 4/5] thermal: Add support for setting polling interval Srinivas Pandruvada
@ 2020-05-18 16:51 ` Daniel Lezcano
2020-05-18 23:46 ` Srinivas Pandruvada
2020-05-20 4:38 ` Amit Kucheria
1 sibling, 1 reply; 19+ messages in thread
From: Daniel Lezcano @ 2020-05-18 16:51 UTC (permalink / raw)
To: Srinivas Pandruvada, rui.zhang, amit.kucheria; +Cc: linux-kernel, linux-pm
On 04/05/2020 20:16, Srinivas Pandruvada wrote:
> Add new attribute in the thermal syfs for setting temperature sampling
> interval when CONFIG_THERMAL_USER_EVENT_INTERFACE is defined. The default
> value is 0, which means no polling.
>
> At this interval user space will get an event THERMAL_TEMP_SAMPLE with
> temperature sample. This reuses existing polling mecahnism when polling
> or passive delay is specified during zone registry. To avoid interference
> with passive and polling delay, this new polling attribute can't be used
> for those zones.
The userspace can get the temperature whenever it wants via the
temperature file. The polling is designed for a specific hardware and
the slope of the temperature graphic.
The userspace has the alternative of reading the temperature based on
its own timer or wait for (and stick to) the thermal framework sampling
rate. Adding a notification in the update is enough IMO.
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/thermal/thermal_core.c | 7 +++++++
> drivers/thermal/thermal_sysfs.c | 36 +++++++++++++++++++++++++++++++--
> include/linux/thermal.h | 1 +
> 3 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 14770d882d42..17cd799b0073 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -313,6 +313,8 @@ static void monitor_thermal_zone(struct thermal_zone_device *tz)
> thermal_zone_device_set_polling(tz, tz->passive_delay);
> else if (tz->polling_delay)
> thermal_zone_device_set_polling(tz, tz->polling_delay);
> + else if (tz->temp_polling_delay)
> + thermal_zone_device_set_polling(tz, tz->temp_polling_delay);
> else
> thermal_zone_device_set_polling(tz, 0);
>
> @@ -446,6 +448,11 @@ static void update_temperature(struct thermal_zone_device *tz)
> tz->temperature = temp;
> mutex_unlock(&tz->lock);
>
> + if (tz->temp_polling_delay) {
> + thermal_dev_send_event(tz->id, THERMAL_TEMP_SAMPLE, temp);
> + monitor_thermal_zone(tz);
> + }
> +
> trace_thermal_temperature(tz);
> if (tz->last_temperature == THERMAL_TEMP_INVALID)
> dev_dbg(&tz->device, "last_temperature N/A, current_temperature=%d\n",
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index aa85424c3ac4..0df7997993fe 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -248,6 +248,36 @@ create_thres_attr(temp_thres_low);
> create_thres_attr(temp_thres_high);
> create_thres_attr(temp_thres_hyst);
>
> +static ssize_t
> +temp_polling_delay_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct thermal_zone_device *tz = to_thermal_zone(dev);
> + int val;
> +
> + if (kstrtoint(buf, 10, &val))
> + return -EINVAL;
> +
> + if (val && val < 1000)
> + return -EINVAL;
> +
> + tz->temp_polling_delay = val;
> + thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> +
> + return count;
> +}
> +
> +static ssize_t
> +temp_polling_delay_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct thermal_zone_device *tz = to_thermal_zone(dev);
> +
> + return sprintf(buf, "%d\n", tz->temp_polling_delay);
> +}
> +
> +static DEVICE_ATTR_RW(temp_polling_delay);
> +
> static int create_user_events_attrs(struct thermal_zone_device *tz)
> {
> struct attribute **attrs;
> @@ -260,8 +290,8 @@ static int create_user_events_attrs(struct thermal_zone_device *tz)
> if (tz->ops->get_temp_thres_high)
> ++index;
>
> - /* One additional space for NULL */
> - attrs = kcalloc(index + 1, sizeof(*attrs), GFP_KERNEL);
> + /* One additional space for NULL and temp_pollling_delay */
> + attrs = kcalloc(index + 2, sizeof(*attrs), GFP_KERNEL);
> if (!attrs)
> return -ENOMEM;
>
> @@ -312,6 +342,8 @@ static int create_user_events_attrs(struct thermal_zone_device *tz)
> attrs[index] = &tz->threshold_attrs[index].attr.attr;
> ++index;
> }
> + if (!tz->polling_delay && !tz->passive_delay)
> + attrs[index++] = &dev_attr_temp_polling_delay.attr;
> attrs[index] = NULL;
> tz->threshold_attribute_group.attrs = attrs;
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index ee9d79ace7ce..0ec4bd8c9c5c 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -216,6 +216,7 @@ struct thermal_zone_device {
> enum thermal_notify_event notify_event;
> struct attribute_group threshold_attribute_group;
> struct thermal_attr *threshold_attrs;
> + int temp_polling_delay;
> };
>
> /**
>
--
<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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC][PATCH 4/5] thermal: Add support for setting polling interval
2020-05-18 16:51 ` Daniel Lezcano
@ 2020-05-18 23:46 ` Srinivas Pandruvada
2020-05-19 10:25 ` Daniel Lezcano
0 siblings, 1 reply; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-18 23:46 UTC (permalink / raw)
To: Daniel Lezcano, rui.zhang, amit.kucheria; +Cc: linux-kernel, linux-pm
On Mon, 2020-05-18 at 18:51 +0200, Daniel Lezcano wrote:
> On 04/05/2020 20:16, Srinivas Pandruvada wrote:
> > Add new attribute in the thermal syfs for setting temperature
> > sampling
> > interval when CONFIG_THERMAL_USER_EVENT_INTERFACE is defined. The
> > default
> > value is 0, which means no polling.
> >
> > At this interval user space will get an event THERMAL_TEMP_SAMPLE
> > with
> > temperature sample. This reuses existing polling mecahnism when
> > polling
> > or passive delay is specified during zone registry. To avoid
> > interference
> > with passive and polling delay, this new polling attribute can't be
> > used
> > for those zones.
>
> The userspace can get the temperature whenever it wants via the
> temperature file. The polling is designed for a specific hardware and
> the slope of the temperature graphic.
>
> The userspace has the alternative of reading the temperature based on
> its own timer or wait for (and stick to) the thermal framework
> sampling
> rate. Adding a notification in the update is enough IMO.
>
The problem with this approach is that the user can't change sampling
interval. Those polling intervals are fixed during thermal-zone
register. Is there any way to change those defaults from user space?
Kernel can start with some long polling interval and user space can
change close to some trip.
Thanks,
Srinivas
>
> > Signed-off-by: Srinivas Pandruvada <
> > srinivas.pandruvada@linux.intel.com>
> > ---
> > drivers/thermal/thermal_core.c | 7 +++++++
> > drivers/thermal/thermal_sysfs.c | 36
> > +++++++++++++++++++++++++++++++--
> > include/linux/thermal.h | 1 +
> > 3 files changed, 42 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/thermal_core.c
> > b/drivers/thermal/thermal_core.c
> > index 14770d882d42..17cd799b0073 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -313,6 +313,8 @@ static void monitor_thermal_zone(struct
> > thermal_zone_device *tz)
> > thermal_zone_device_set_polling(tz, tz->passive_delay);
> > else if (tz->polling_delay)
> > thermal_zone_device_set_polling(tz, tz->polling_delay);
> > + else if (tz->temp_polling_delay)
> > + thermal_zone_device_set_polling(tz, tz-
> > >temp_polling_delay);
> > else
> > thermal_zone_device_set_polling(tz, 0);
> >
> > @@ -446,6 +448,11 @@ static void update_temperature(struct
> > thermal_zone_device *tz)
> > tz->temperature = temp;
> > mutex_unlock(&tz->lock);
> >
> > + if (tz->temp_polling_delay) {
> > + thermal_dev_send_event(tz->id, THERMAL_TEMP_SAMPLE,
> > temp);
> > + monitor_thermal_zone(tz);
> > + }
> > +
> > trace_thermal_temperature(tz);
> > if (tz->last_temperature == THERMAL_TEMP_INVALID)
> > dev_dbg(&tz->device, "last_temperature N/A,
> > current_temperature=%d\n",
> > diff --git a/drivers/thermal/thermal_sysfs.c
> > b/drivers/thermal/thermal_sysfs.c
> > index aa85424c3ac4..0df7997993fe 100644
> > --- a/drivers/thermal/thermal_sysfs.c
> > +++ b/drivers/thermal/thermal_sysfs.c
> > @@ -248,6 +248,36 @@ create_thres_attr(temp_thres_low);
> > create_thres_attr(temp_thres_high);
> > create_thres_attr(temp_thres_hyst);
> >
> > +static ssize_t
> > +temp_polling_delay_store(struct device *dev, struct
> > device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct thermal_zone_device *tz = to_thermal_zone(dev);
> > + int val;
> > +
> > + if (kstrtoint(buf, 10, &val))
> > + return -EINVAL;
> > +
> > + if (val && val < 1000)
> > + return -EINVAL;
> > +
> > + tz->temp_polling_delay = val;
> > + thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> > +
> > + return count;
> > +}
> > +
> > +static ssize_t
> > +temp_polling_delay_show(struct device *dev, struct
> > device_attribute *attr,
> > + char *buf)
> > +{
> > + struct thermal_zone_device *tz = to_thermal_zone(dev);
> > +
> > + return sprintf(buf, "%d\n", tz->temp_polling_delay);
> > +}
> > +
> > +static DEVICE_ATTR_RW(temp_polling_delay);
> > +
> > static int create_user_events_attrs(struct thermal_zone_device
> > *tz)
> > {
> > struct attribute **attrs;
> > @@ -260,8 +290,8 @@ static int create_user_events_attrs(struct
> > thermal_zone_device *tz)
> > if (tz->ops->get_temp_thres_high)
> > ++index;
> >
> > - /* One additional space for NULL */
> > - attrs = kcalloc(index + 1, sizeof(*attrs), GFP_KERNEL);
> > + /* One additional space for NULL and temp_pollling_delay */
> > + attrs = kcalloc(index + 2, sizeof(*attrs), GFP_KERNEL);
> > if (!attrs)
> > return -ENOMEM;
> >
> > @@ -312,6 +342,8 @@ static int create_user_events_attrs(struct
> > thermal_zone_device *tz)
> > attrs[index] = &tz->threshold_attrs[index].attr.attr;
> > ++index;
> > }
> > + if (!tz->polling_delay && !tz->passive_delay)
> > + attrs[index++] = &dev_attr_temp_polling_delay.attr;
> > attrs[index] = NULL;
> > tz->threshold_attribute_group.attrs = attrs;
> >
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index ee9d79ace7ce..0ec4bd8c9c5c 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -216,6 +216,7 @@ struct thermal_zone_device {
> > enum thermal_notify_event notify_event;
> > struct attribute_group threshold_attribute_group;
> > struct thermal_attr *threshold_attrs;
> > + int temp_polling_delay;
> > };
> >
> > /**
> >
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC][PATCH 4/5] thermal: Add support for setting polling interval
2020-05-18 23:46 ` Srinivas Pandruvada
@ 2020-05-19 10:25 ` Daniel Lezcano
2020-05-21 22:26 ` Srinivas Pandruvada
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Lezcano @ 2020-05-19 10:25 UTC (permalink / raw)
To: Srinivas Pandruvada, rui.zhang, amit.kucheria; +Cc: linux-kernel, linux-pm
On 19/05/2020 01:46, Srinivas Pandruvada wrote:
> On Mon, 2020-05-18 at 18:51 +0200, Daniel Lezcano wrote:
>> On 04/05/2020 20:16, Srinivas Pandruvada wrote:
>>> Add new attribute in the thermal syfs for setting temperature
>>> sampling
>>> interval when CONFIG_THERMAL_USER_EVENT_INTERFACE is defined. The
>>> default
>>> value is 0, which means no polling.
>>>
>>> At this interval user space will get an event THERMAL_TEMP_SAMPLE
>>> with
>>> temperature sample. This reuses existing polling mecahnism when
>>> polling
>>> or passive delay is specified during zone registry. To avoid
>>> interference
>>> with passive and polling delay, this new polling attribute can't be
>>> used
>>> for those zones.
>>
>> The userspace can get the temperature whenever it wants via the
>> temperature file. The polling is designed for a specific hardware and
>> the slope of the temperature graphic.
>>
>> The userspace has the alternative of reading the temperature based on
>> its own timer or wait for (and stick to) the thermal framework
>> sampling
>> rate. Adding a notification in the update is enough IMO.
>>
> The problem with this approach is that the user can't change sampling
> interval. Those polling intervals are fixed during thermal-zone
> register. Is there any way to change those defaults from user space?
No, we can't but the userspace can decide when to read the temperature
(via sysfs or netlink) and thus decide its own sampling rate.
Otherwise, we are talking about an userspace governor, so the platform
is setup with the desired sampling rate + userspace governor.
> Kernel can start with some long polling interval and user space can
> change close to some trip.
Ok, let me rephrase it. This (big) comment encompass also patch 3/5.
I understood now the initial need of adding user trip points.
There are platforms where the interrupt mode does not exist so setting
an user trip point does not set the interrupt for the closer
temperature, hence we end up with a kernel sampling rate and in this
case adding a trip point + new user sampling rate is pointless as the
userspace can poll the temperature at its convenient rate.
If we summarize the different combinations we have:
1. monitoring : interrupt mode, mitigation : interrupt mode
There are no thermal zone update until an interrupt fires. The
mitigation is based on trip point crossed.
2. monitoring : interrupt mode, mitigation : polling
There are no thermal zone update until an interrupt fires. The
mitigation happens with a sampling rate specified with the polling rate.
3. monitoring : polling, mitigation : polling
The thermal zone is updated at the polling rate, the mitigation occurs
with an update at the second polling rate.
IIUC, the RFC proposes to add a new type of temperature threshold,
followed a new polling rate to update the userspace.
IMHO, it is not a good thing to delegate to the kernel what the
userspace can handle easily.
I suggest:
- Not add another polling rate. If the thermal zone has a polling rate
or supports the interrupt mode, then the user trip point setup succeed
otherwise it fails and up to the userspace to read the temperature at
its convenient rate. (Note multiple process may want to get temperature,
so one should not set the rate of others).
- Not add another temp threshold structure but add a new trip type
"user" and keep using the existing trip structures, so the notification
can happen in the handle_trip_point function. The sysfs only reflects
the setup via the "trip_point_x_hyst", "trip_point_0_temp",
"trip_point_x_type"
- Do not use sysfs for setup but rely on the genetlink for one message
setup instead of multiple sysfs file writing. Adding a trip point will
be straighforward.
What do you think?
--
<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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC][PATCH 4/5] thermal: Add support for setting polling interval
2020-05-19 10:25 ` Daniel Lezcano
@ 2020-05-21 22:26 ` Srinivas Pandruvada
0 siblings, 0 replies; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-21 22:26 UTC (permalink / raw)
To: Daniel Lezcano, rui.zhang, amit.kucheria; +Cc: linux-kernel, linux-pm
On Tue, 2020-05-19 at 12:25 +0200, Daniel Lezcano wrote:
> On 19/05/2020 01:46, Srinivas Pandruvada wrote:
> > On Mon, 2020-05-18 at 18:51 +0200, Daniel Lezcano wrote:
> > > On 04/05/2020 20:16, Srinivas Pandruvada wrote:
> > > > Add new attribute in the thermal syfs for setting temperature
> > > > sampling
> > > > interval when CONFIG_THERMAL_USER_EVENT_INTERFACE is defined.
> > > > The
> > > > default
> > > > value is 0, which means no polling.
> > > >
> > > > At this interval user space will get an event
> > > > THERMAL_TEMP_SAMPLE
> > > > with
> > > > temperature sample. This reuses existing polling mecahnism when
> > > > polling
> > > > or passive delay is specified during zone registry. To avoid
> > > > interference
> > > > with passive and polling delay, this new polling attribute
> > > > can't be
> > > > used
> > > > for those zones.
> > >
> > > The userspace can get the temperature whenever it wants via the
> > > temperature file. The polling is designed for a specific hardware
> > > and
> > > the slope of the temperature graphic.
> > >
> > > The userspace has the alternative of reading the temperature
> > > based on
> > > its own timer or wait for (and stick to) the thermal framework
> > > sampling
> > > rate. Adding a notification in the update is enough IMO.
> > >
> > The problem with this approach is that the user can't change
> > sampling
> > interval. Those polling intervals are fixed during thermal-zone
> > register. Is there any way to change those defaults from user
> > space?
>
> No, we can't but the userspace can decide when to read the
> temperature
> (via sysfs or netlink) and thus decide its own sampling rate.
Yes, if we poll for temperature from user space, no change is required
neighter netlink nor kfifo.
The average time to read CPU temperature and convert to int from sysfs
takes 45us vs 7us for push via Kfifo. I haven't looked at your patches
and checked this time. If it is comparable then netlink is better.
>
> Otherwise, we are talking about an userspace governor, so the
> platform
> is setup with the desired sampling rate + userspace governor.
>
> > Kernel can start with some long polling interval and user space can
> > change close to some trip.
>
> Ok, let me rephrase it. This (big) comment encompass also patch 3/5.
>
> I understood now the initial need of adding user trip points.
>
> There are platforms where the interrupt mode does not exist so
> setting
> an user trip point does not set the interrupt for the closer
> temperature, hence we end up with a kernel sampling rate and in this
> case adding a trip point + new user sampling rate is pointless as the
> userspace can poll the temperature at its convenient rate.
>
> If we summarize the different combinations we have:
>
> 1. monitoring : interrupt mode, mitigation : interrupt mode
>
> There are no thermal zone update until an interrupt fires. The
> mitigation is based on trip point crossed.
Yes. Basically daemon sleeps, till it gets a netlink notification
currently.
>
> 2. monitoring : interrupt mode, mitigation : polling
>
> There are no thermal zone update until an interrupt fires. The
> mitigation happens with a sampling rate specified with the polling
> rate.
>
More complex than this. interrupt fires but it will fire flood for any
change (+/- around threshold) so after the first trigger, just disable
interrupt for an interval and start polling from user space as this is
more efficient.
> 3. monitoring : polling, mitigation : polling
>
> The thermal zone is updated at the polling rate, the mitigation
> occurs
> with an update at the second polling rate.
>
Yes.
> IIUC, the RFC proposes to add a new type of temperature threshold,
> followed a new polling rate to update the userspace.
>
> IMHO, it is not a good thing to delegate to the kernel what the
> userspace can handle easily.
>
> I suggest:
>
> - Not add another polling rate. If the thermal zone has a polling
> rate
> or supports the interrupt mode, then the user trip point setup
> succeed
> otherwise it fails and up to the userspace to read the temperature at
> its convenient rate. (Note multiple process may want to get
> temperature,
> so one should not set the rate of others).
Fine.
>
> - Not add another temp threshold structure but add a new trip type
> "user" and keep using the existing trip structures, so the
> notification
> can happen in the handle_trip_point function. The sysfs only reflects
> the setup via the "trip_point_x_hyst", "trip_point_0_temp",
> "trip_point_x_type"
Fine. It is better than what we re-purpose a passive trip currently.
>
> - Do not use sysfs for setup but rely on the genetlink for one
> message
> setup instead of multiple sysfs file writing. Adding a trip point
> will
Fine.
>
>
> What do you think?
Looks good.
Thanks,
Srinivas
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC][PATCH 4/5] thermal: Add support for setting polling interval
2020-05-04 18:16 ` [RFC][PATCH 4/5] thermal: Add support for setting polling interval Srinivas Pandruvada
2020-05-18 16:51 ` Daniel Lezcano
@ 2020-05-20 4:38 ` Amit Kucheria
1 sibling, 0 replies; 19+ messages in thread
From: Amit Kucheria @ 2020-05-20 4:38 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Zhang Rui, Daniel Lezcano, LKML, Linux PM list
On Mon, May 4, 2020 at 11:47 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> Add new attribute in the thermal syfs for setting temperature sampling
> interval when CONFIG_THERMAL_USER_EVENT_INTERFACE is defined. The default
> value is 0, which means no polling.
>
> At this interval user space will get an event THERMAL_TEMP_SAMPLE with
> temperature sample. This reuses existing polling mecahnism when polling
> or passive delay is specified during zone registry. To avoid interference
> with passive and polling delay, this new polling attribute can't be used
> for those zones.
Why should the kernel periodically emit events for userspace when the
userspace is perfectly capable of deciding how frequently it wants to
poll a file for changes?
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/thermal/thermal_core.c | 7 +++++++
> drivers/thermal/thermal_sysfs.c | 36 +++++++++++++++++++++++++++++++--
> include/linux/thermal.h | 1 +
> 3 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 14770d882d42..17cd799b0073 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -313,6 +313,8 @@ static void monitor_thermal_zone(struct thermal_zone_device *tz)
> thermal_zone_device_set_polling(tz, tz->passive_delay);
> else if (tz->polling_delay)
> thermal_zone_device_set_polling(tz, tz->polling_delay);
> + else if (tz->temp_polling_delay)
> + thermal_zone_device_set_polling(tz, tz->temp_polling_delay);
> else
> thermal_zone_device_set_polling(tz, 0);
>
> @@ -446,6 +448,11 @@ static void update_temperature(struct thermal_zone_device *tz)
> tz->temperature = temp;
> mutex_unlock(&tz->lock);
>
> + if (tz->temp_polling_delay) {
> + thermal_dev_send_event(tz->id, THERMAL_TEMP_SAMPLE, temp);
> + monitor_thermal_zone(tz);
> + }
> +
> trace_thermal_temperature(tz);
> if (tz->last_temperature == THERMAL_TEMP_INVALID)
> dev_dbg(&tz->device, "last_temperature N/A, current_temperature=%d\n",
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index aa85424c3ac4..0df7997993fe 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -248,6 +248,36 @@ create_thres_attr(temp_thres_low);
> create_thres_attr(temp_thres_high);
> create_thres_attr(temp_thres_hyst);
>
> +static ssize_t
> +temp_polling_delay_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct thermal_zone_device *tz = to_thermal_zone(dev);
> + int val;
> +
> + if (kstrtoint(buf, 10, &val))
> + return -EINVAL;
> +
> + if (val && val < 1000)
> + return -EINVAL;
> +
> + tz->temp_polling_delay = val;
> + thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> +
> + return count;
> +}
> +
> +static ssize_t
> +temp_polling_delay_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct thermal_zone_device *tz = to_thermal_zone(dev);
> +
> + return sprintf(buf, "%d\n", tz->temp_polling_delay);
> +}
> +
> +static DEVICE_ATTR_RW(temp_polling_delay);
> +
> static int create_user_events_attrs(struct thermal_zone_device *tz)
> {
> struct attribute **attrs;
> @@ -260,8 +290,8 @@ static int create_user_events_attrs(struct thermal_zone_device *tz)
> if (tz->ops->get_temp_thres_high)
> ++index;
>
> - /* One additional space for NULL */
> - attrs = kcalloc(index + 1, sizeof(*attrs), GFP_KERNEL);
> + /* One additional space for NULL and temp_pollling_delay */
> + attrs = kcalloc(index + 2, sizeof(*attrs), GFP_KERNEL);
> if (!attrs)
> return -ENOMEM;
>
> @@ -312,6 +342,8 @@ static int create_user_events_attrs(struct thermal_zone_device *tz)
> attrs[index] = &tz->threshold_attrs[index].attr.attr;
> ++index;
> }
> + if (!tz->polling_delay && !tz->passive_delay)
> + attrs[index++] = &dev_attr_temp_polling_delay.attr;
> attrs[index] = NULL;
> tz->threshold_attribute_group.attrs = attrs;
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index ee9d79ace7ce..0ec4bd8c9c5c 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -216,6 +216,7 @@ struct thermal_zone_device {
> enum thermal_notify_event notify_event;
> struct attribute_group threshold_attribute_group;
> struct thermal_attr *threshold_attrs;
> + int temp_polling_delay;
> };
>
> /**
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC][PATCH 5/5] thermal: int340x: Use new device interface
2020-05-04 18:16 [RFC][PATCH 0/5] thermal: Add new mechanism to get thermal notification Srinivas Pandruvada
` (3 preceding siblings ...)
2020-05-04 18:16 ` [RFC][PATCH 4/5] thermal: Add support for setting polling interval Srinivas Pandruvada
@ 2020-05-04 18:16 ` Srinivas Pandruvada
2020-05-20 4:49 ` Amit Kucheria
4 siblings, 1 reply; 19+ messages in thread
From: Srinivas Pandruvada @ 2020-05-04 18:16 UTC (permalink / raw)
To: rui.zhang, daniel.lezcano, amit.kucheria
Cc: linux-kernel, linux-pm, Srinivas Pandruvada
Use the new framework to send notifications for:
- Setting temperature threshold for notification to avoid polling
- Send THERMAL_TRIP_REACHED event on reaching threshold
- Send THERMAL_TRIP_UPDATE when firmware change the the existing trip
temperature
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
.../intel/int340x_thermal/int3403_thermal.c | 3 ++
.../int340x_thermal/int340x_thermal_zone.c | 29 +++++++++++++++++++
.../int340x_thermal/int340x_thermal_zone.h | 7 +++++
.../processor_thermal_device.c | 1 +
4 files changed, 40 insertions(+)
diff --git a/drivers/thermal/intel/int340x_thermal/int3403_thermal.c b/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
index f86cbb125e2f..77c014a113a4 100644
--- a/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
@@ -63,15 +63,18 @@ static void int3403_notify(acpi_handle handle,
switch (event) {
case INT3403_PERF_CHANGED_EVENT:
+ int340x_thermal_send_user_event(obj->int340x_zone, THERMAL_PERF_CHANGED, 0);
break;
case INT3403_THERMAL_EVENT:
int340x_thermal_zone_device_update(obj->int340x_zone,
THERMAL_TRIP_VIOLATED);
+ int340x_thermal_send_user_event(obj->int340x_zone, THERMAL_TRIP_REACHED, 0);
break;
case INT3403_PERF_TRIP_POINT_CHANGED:
int340x_thermal_read_trips(obj->int340x_zone);
int340x_thermal_zone_device_update(obj->int340x_zone,
THERMAL_TRIP_CHANGED);
+ int340x_thermal_send_user_event(obj->int340x_zone, THERMAL_TRIP_UPDATE, 0);
break;
default:
dev_err(&priv->pdev->dev, "Unsupported event [0x%x]\n", event);
diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
index 432213272f1e..9568a2db7afd 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -146,12 +146,41 @@ static int int340x_thermal_get_trip_hyst(struct thermal_zone_device *zone,
return 0;
}
+static int int340x_thermal_get_thres_low(struct thermal_zone_device *zone, int *temp)
+{
+ struct int34x_thermal_zone *d = zone->devdata;
+
+ *temp = d->aux_trips[0];
+
+ return 0;
+}
+
+static int int340x_thermal_set_thres_low(struct thermal_zone_device *zone, int temp)
+{
+ struct int34x_thermal_zone *d = zone->devdata;
+ acpi_status status;
+
+ if (d->override_ops && d->override_ops->set_trip_temp)
+ return d->override_ops->set_trip_temp(zone, 0, temp);
+
+ status = acpi_execute_simple_method(d->adev->handle, "PAT0",
+ millicelsius_to_deci_kelvin(temp));
+ if (ACPI_FAILURE(status))
+ return -EIO;
+
+ d->aux_trips[0] = temp;
+
+ return 0;
+}
+
static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
.get_temp = int340x_thermal_get_zone_temp,
.get_trip_temp = int340x_thermal_get_trip_temp,
.get_trip_type = int340x_thermal_get_trip_type,
.set_trip_temp = int340x_thermal_set_trip_temp,
.get_trip_hyst = int340x_thermal_get_trip_hyst,
+ .set_temp_thres_low = int340x_thermal_set_thres_low,
+ .get_temp_thres_low = int340x_thermal_get_thres_low,
};
static int int340x_thermal_get_trip_config(acpi_handle handle, char *name,
diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
index 3b4971df1b33..142027e4955f 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
@@ -58,4 +58,11 @@ static inline void int340x_thermal_zone_device_update(
thermal_zone_device_update(tzone->zone, event);
}
+static inline void int340x_thermal_send_user_event(
+ struct int34x_thermal_zone *tzone,
+ enum thermal_device_events event,
+ u64 data)
+{
+ thermal_dev_send_event(tzone->zone->id, event, data);
+}
#endif
diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
index 297db1d2d960..e25f01948d33 100644
--- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
+++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
@@ -340,6 +340,7 @@ static void proc_thermal_notify(acpi_handle handle, u32 event, void *data)
proc_thermal_read_ppcc(proc_priv);
int340x_thermal_zone_device_update(proc_priv->int340x_zone,
THERMAL_DEVICE_POWER_CAPABILITY_CHANGED);
+ int340x_thermal_send_user_event(proc_priv->int340x_zone, THERMAL_PERF_CHANGED, 0);
break;
default:
dev_dbg(proc_priv->dev, "Unsupported event [0x%x]\n", event);
--
2.25.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC][PATCH 5/5] thermal: int340x: Use new device interface
2020-05-04 18:16 ` [RFC][PATCH 5/5] thermal: int340x: Use new device interface Srinivas Pandruvada
@ 2020-05-20 4:49 ` Amit Kucheria
0 siblings, 0 replies; 19+ messages in thread
From: Amit Kucheria @ 2020-05-20 4:49 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Zhang Rui, Daniel Lezcano, LKML, Linux PM list
On Mon, May 4, 2020 at 11:47 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> Use the new framework to send notifications for:
> - Setting temperature threshold for notification to avoid polling
> - Send THERMAL_TRIP_REACHED event on reaching threshold
> - Send THERMAL_TRIP_UPDATE when firmware change the the existing trip
> temperature
I am a little confused here. I would've expected the thermal core to
send the THERMAL_TRIP_* notifications, not platform drivers. Why
shouldn't this be done in thermal core?
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> .../intel/int340x_thermal/int3403_thermal.c | 3 ++
> .../int340x_thermal/int340x_thermal_zone.c | 29 +++++++++++++++++++
> .../int340x_thermal/int340x_thermal_zone.h | 7 +++++
> .../processor_thermal_device.c | 1 +
> 4 files changed, 40 insertions(+)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int3403_thermal.c b/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
> index f86cbb125e2f..77c014a113a4 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
> @@ -63,15 +63,18 @@ static void int3403_notify(acpi_handle handle,
>
> switch (event) {
> case INT3403_PERF_CHANGED_EVENT:
> + int340x_thermal_send_user_event(obj->int340x_zone, THERMAL_PERF_CHANGED, 0);
> break;
> case INT3403_THERMAL_EVENT:
> int340x_thermal_zone_device_update(obj->int340x_zone,
> THERMAL_TRIP_VIOLATED);
> + int340x_thermal_send_user_event(obj->int340x_zone, THERMAL_TRIP_REACHED, 0);
> break;
> case INT3403_PERF_TRIP_POINT_CHANGED:
> int340x_thermal_read_trips(obj->int340x_zone);
> int340x_thermal_zone_device_update(obj->int340x_zone,
> THERMAL_TRIP_CHANGED);
> + int340x_thermal_send_user_event(obj->int340x_zone, THERMAL_TRIP_UPDATE, 0);
> break;
> default:
> dev_err(&priv->pdev->dev, "Unsupported event [0x%x]\n", event);
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> index 432213272f1e..9568a2db7afd 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> @@ -146,12 +146,41 @@ static int int340x_thermal_get_trip_hyst(struct thermal_zone_device *zone,
> return 0;
> }
>
> +static int int340x_thermal_get_thres_low(struct thermal_zone_device *zone, int *temp)
> +{
> + struct int34x_thermal_zone *d = zone->devdata;
> +
> + *temp = d->aux_trips[0];
> +
> + return 0;
> +}
> +
> +static int int340x_thermal_set_thres_low(struct thermal_zone_device *zone, int temp)
> +{
> + struct int34x_thermal_zone *d = zone->devdata;
> + acpi_status status;
> +
> + if (d->override_ops && d->override_ops->set_trip_temp)
> + return d->override_ops->set_trip_temp(zone, 0, temp);
> +
> + status = acpi_execute_simple_method(d->adev->handle, "PAT0",
> + millicelsius_to_deci_kelvin(temp));
> + if (ACPI_FAILURE(status))
> + return -EIO;
> +
> + d->aux_trips[0] = temp;
> +
> + return 0;
> +}
> +
> static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
> .get_temp = int340x_thermal_get_zone_temp,
> .get_trip_temp = int340x_thermal_get_trip_temp,
> .get_trip_type = int340x_thermal_get_trip_type,
> .set_trip_temp = int340x_thermal_set_trip_temp,
> .get_trip_hyst = int340x_thermal_get_trip_hyst,
> + .set_temp_thres_low = int340x_thermal_set_thres_low,
> + .get_temp_thres_low = int340x_thermal_get_thres_low,
> };
>
> static int int340x_thermal_get_trip_config(acpi_handle handle, char *name,
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> index 3b4971df1b33..142027e4955f 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> @@ -58,4 +58,11 @@ static inline void int340x_thermal_zone_device_update(
> thermal_zone_device_update(tzone->zone, event);
> }
>
> +static inline void int340x_thermal_send_user_event(
> + struct int34x_thermal_zone *tzone,
> + enum thermal_device_events event,
> + u64 data)
> +{
> + thermal_dev_send_event(tzone->zone->id, event, data);
> +}
> #endif
> diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> index 297db1d2d960..e25f01948d33 100644
> --- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> +++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> @@ -340,6 +340,7 @@ static void proc_thermal_notify(acpi_handle handle, u32 event, void *data)
> proc_thermal_read_ppcc(proc_priv);
> int340x_thermal_zone_device_update(proc_priv->int340x_zone,
> THERMAL_DEVICE_POWER_CAPABILITY_CHANGED);
> + int340x_thermal_send_user_event(proc_priv->int340x_zone, THERMAL_PERF_CHANGED, 0);
> break;
> default:
> dev_dbg(proc_priv->dev, "Unsupported event [0x%x]\n", event);
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 19+ messages in thread