* [PATCH RFC 2/4] thermal/core: Add critical and hot ops
2020-12-09 15:34 [PATCH RFC 1/4] thermal/core: Emit a warning if the thermal zone is updated without ops Daniel Lezcano
@ 2020-12-09 15:34 ` Daniel Lezcano
2020-12-10 9:28 ` Kai-Heng Feng
2020-12-09 15:34 ` [PATCH RFC 3/4] thermal/drivers/acpi: Use hot and critical ops Daniel Lezcano
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2020-12-09 15:34 UTC (permalink / raw)
To: daniel.lezcano, rui.zhang
Cc: linux-kernel, linux-pm, lukasz.luba, srinivas.pandruvada, kai.heng.feng
Currently there is no way to the sensors to directly call an ops in
interrupt mode without calling thermal_zone_device_update assuming all
the trip points are defined.
A sensor may want to do something special if a trip point is hot or
critical.
This patch adds the critical and hot ops to the thermal zone device,
so a sensor can directly invoke them or let the thermal framework to
call the sensor specific ones.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/thermal/thermal_core.c | 42 +++++++++++++++++++++-------------
include/linux/thermal.h | 3 +++
2 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index afc02e7d1045..0366f3f076cc 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -375,6 +375,24 @@ static void thermal_emergency_poweroff(void)
msecs_to_jiffies(poweroff_delay_ms));
}
+void thermal_zone_device_critical(struct thermal_zone_device *tz)
+{
+ dev_emerg(&tz->device, "%s: critical temperature reached, "
+ "shutting down\n", tz->type);
+
+ mutex_lock(&poweroff_lock);
+ if (!power_off_triggered) {
+ /*
+ * Queue a backup emergency shutdown in the event of
+ * orderly_poweroff failure
+ */
+ thermal_emergency_poweroff();
+ orderly_poweroff(true);
+ power_off_triggered = true;
+ }
+ mutex_unlock(&poweroff_lock);
+}
+
static void handle_critical_trips(struct thermal_zone_device *tz,
int trip, enum thermal_trip_type trip_type)
{
@@ -391,22 +409,10 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
if (tz->ops->notify)
tz->ops->notify(tz, trip, trip_type);
- if (trip_type == THERMAL_TRIP_CRITICAL) {
- dev_emerg(&tz->device,
- "critical temperature reached (%d C), shutting down\n",
- tz->temperature / 1000);
- mutex_lock(&poweroff_lock);
- if (!power_off_triggered) {
- /*
- * Queue a backup emergency shutdown in the event of
- * orderly_poweroff failure
- */
- thermal_emergency_poweroff();
- orderly_poweroff(true);
- power_off_triggered = true;
- }
- mutex_unlock(&poweroff_lock);
- }
+ if (trip_type == THERMAL_TRIP_HOT && tz->ops->hot)
+ tz->ops->hot(tz);
+ else if (trip_type == THERMAL_TRIP_CRITICAL)
+ tz->ops->critical(tz);
}
static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
@@ -1331,6 +1337,10 @@ thermal_zone_device_register(const char *type, int trips, int mask,
tz->id = id;
strlcpy(tz->type, type, sizeof(tz->type));
+
+ if (!ops->critical)
+ ops->critical = thermal_zone_device_critical;
+
tz->ops = ops;
tz->tzp = tzp;
tz->device.class = &thermal_class;
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index f23a388ded15..125c8a4d52e6 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -79,6 +79,8 @@ struct thermal_zone_device_ops {
enum thermal_trend *);
int (*notify) (struct thermal_zone_device *, int,
enum thermal_trip_type);
+ void (*hot)(struct thermal_zone_device *);
+ void (*critical)(struct thermal_zone_device *);
};
struct thermal_cooling_device_ops {
@@ -399,6 +401,7 @@ void thermal_cdev_update(struct thermal_cooling_device *);
void thermal_notify_framework(struct thermal_zone_device *, int);
int thermal_zone_device_enable(struct thermal_zone_device *tz);
int thermal_zone_device_disable(struct thermal_zone_device *tz);
+void thermal_zone_device_critical(struct thermal_zone_device *tz);
#else
static inline struct thermal_zone_device *thermal_zone_device_register(
const char *type, int trips, int mask, void *devdata,
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH RFC 2/4] thermal/core: Add critical and hot ops
2020-12-09 15:34 ` [PATCH RFC 2/4] thermal/core: Add critical and hot ops Daniel Lezcano
@ 2020-12-10 9:28 ` Kai-Heng Feng
0 siblings, 0 replies; 6+ messages in thread
From: Kai-Heng Feng @ 2020-12-10 9:28 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Zhang Rui, open list, open list:THERMAL, lukasz.luba,
srinivas.pandruvada
> On Dec 9, 2020, at 23:34, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> Currently there is no way to the sensors to directly call an ops in
> interrupt mode without calling thermal_zone_device_update assuming all
> the trip points are defined.
>
> A sensor may want to do something special if a trip point is hot or
> critical.
>
> This patch adds the critical and hot ops to the thermal zone device,
> so a sensor can directly invoke them or let the thermal framework to
> call the sensor specific ones.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Thanks. This can solve my issue once .critical callbacks are added in thermal drivers.
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/thermal/thermal_core.c | 42 +++++++++++++++++++++-------------
> include/linux/thermal.h | 3 +++
> 2 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index afc02e7d1045..0366f3f076cc 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -375,6 +375,24 @@ static void thermal_emergency_poweroff(void)
> msecs_to_jiffies(poweroff_delay_ms));
> }
>
> +void thermal_zone_device_critical(struct thermal_zone_device *tz)
> +{
> + dev_emerg(&tz->device, "%s: critical temperature reached, "
> + "shutting down\n", tz->type);
> +
> + mutex_lock(&poweroff_lock);
> + if (!power_off_triggered) {
> + /*
> + * Queue a backup emergency shutdown in the event of
> + * orderly_poweroff failure
> + */
> + thermal_emergency_poweroff();
> + orderly_poweroff(true);
> + power_off_triggered = true;
> + }
> + mutex_unlock(&poweroff_lock);
> +}
> +
> static void handle_critical_trips(struct thermal_zone_device *tz,
> int trip, enum thermal_trip_type trip_type)
> {
> @@ -391,22 +409,10 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
> if (tz->ops->notify)
> tz->ops->notify(tz, trip, trip_type);
>
> - if (trip_type == THERMAL_TRIP_CRITICAL) {
> - dev_emerg(&tz->device,
> - "critical temperature reached (%d C), shutting down\n",
> - tz->temperature / 1000);
> - mutex_lock(&poweroff_lock);
> - if (!power_off_triggered) {
> - /*
> - * Queue a backup emergency shutdown in the event of
> - * orderly_poweroff failure
> - */
> - thermal_emergency_poweroff();
> - orderly_poweroff(true);
> - power_off_triggered = true;
> - }
> - mutex_unlock(&poweroff_lock);
> - }
> + if (trip_type == THERMAL_TRIP_HOT && tz->ops->hot)
> + tz->ops->hot(tz);
> + else if (trip_type == THERMAL_TRIP_CRITICAL)
> + tz->ops->critical(tz);
> }
>
> static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
> @@ -1331,6 +1337,10 @@ thermal_zone_device_register(const char *type, int trips, int mask,
>
> tz->id = id;
> strlcpy(tz->type, type, sizeof(tz->type));
> +
> + if (!ops->critical)
> + ops->critical = thermal_zone_device_critical;
> +
> tz->ops = ops;
> tz->tzp = tzp;
> tz->device.class = &thermal_class;
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index f23a388ded15..125c8a4d52e6 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -79,6 +79,8 @@ struct thermal_zone_device_ops {
> enum thermal_trend *);
> int (*notify) (struct thermal_zone_device *, int,
> enum thermal_trip_type);
> + void (*hot)(struct thermal_zone_device *);
> + void (*critical)(struct thermal_zone_device *);
> };
>
> struct thermal_cooling_device_ops {
> @@ -399,6 +401,7 @@ void thermal_cdev_update(struct thermal_cooling_device *);
> void thermal_notify_framework(struct thermal_zone_device *, int);
> int thermal_zone_device_enable(struct thermal_zone_device *tz);
> int thermal_zone_device_disable(struct thermal_zone_device *tz);
> +void thermal_zone_device_critical(struct thermal_zone_device *tz);
> #else
> static inline struct thermal_zone_device *thermal_zone_device_register(
> const char *type, int trips, int mask, void *devdata,
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RFC 3/4] thermal/drivers/acpi: Use hot and critical ops
2020-12-09 15:34 [PATCH RFC 1/4] thermal/core: Emit a warning if the thermal zone is updated without ops Daniel Lezcano
2020-12-09 15:34 ` [PATCH RFC 2/4] thermal/core: Add critical and hot ops Daniel Lezcano
@ 2020-12-09 15:34 ` Daniel Lezcano
2020-12-09 15:34 ` [PATCH RFC 4/4] thermal/core: Remove notify ops Daniel Lezcano
2020-12-09 15:53 ` [PATCH RFC 1/4] thermal/core: Emit a warning if the thermal zone is updated without ops Lukasz Luba
3 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2020-12-09 15:34 UTC (permalink / raw)
To: daniel.lezcano, rui.zhang
Cc: linux-kernel, linux-pm, lukasz.luba, srinivas.pandruvada, kai.heng.feng
The acpi driver wants to do a netlink notification in case of a hot or
critical trip point. Implement the corresponding ops to be used for
the thermal zone and use them instead of the notify ops.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/acpi/thermal.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 12c0ece746f0..b5e4bc9e3282 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -677,27 +677,24 @@ static int thermal_get_trend(struct thermal_zone_device *thermal,
return 0;
}
-
-static int thermal_notify(struct thermal_zone_device *thermal, int trip,
- enum thermal_trip_type trip_type)
+static void acpi_thermal_zone_device_hot(struct thermal_zone_device *thermal)
{
- u8 type = 0;
struct acpi_thermal *tz = thermal->devdata;
- if (trip_type == THERMAL_TRIP_CRITICAL)
- type = ACPI_THERMAL_NOTIFY_CRITICAL;
- else if (trip_type == THERMAL_TRIP_HOT)
- type = ACPI_THERMAL_NOTIFY_HOT;
- else
- return 0;
-
acpi_bus_generate_netlink_event(tz->device->pnp.device_class,
- dev_name(&tz->device->dev), type, 1);
+ dev_name(&tz->device->dev),
+ ACPI_THERMAL_NOTIFY_HOT, 1);
+}
- if (trip_type == THERMAL_TRIP_CRITICAL && nocrt)
- return 1;
+static void acpi_thermal_zone_device_critical(struct thermal_zone_device *thermal)
+{
+ struct acpi_thermal *tz = thermal->devdata;
- return 0;
+ acpi_bus_generate_netlink_event(tz->device->pnp.device_class,
+ dev_name(&tz->device->dev),
+ ACPI_THERMAL_NOTIFY_CRITICAL, 1);
+
+ thermal_zone_device_critical(thermal);
}
static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
@@ -812,7 +809,8 @@ static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
.get_trip_temp = thermal_get_trip_temp,
.get_crit_temp = thermal_get_crit_temp,
.get_trend = thermal_get_trend,
- .notify = thermal_notify,
+ .hot = acpi_thermal_zone_device_hot,
+ .critical = acpi_thermal_zone_device_critical,
};
static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH RFC 4/4] thermal/core: Remove notify ops
2020-12-09 15:34 [PATCH RFC 1/4] thermal/core: Emit a warning if the thermal zone is updated without ops Daniel Lezcano
2020-12-09 15:34 ` [PATCH RFC 2/4] thermal/core: Add critical and hot ops Daniel Lezcano
2020-12-09 15:34 ` [PATCH RFC 3/4] thermal/drivers/acpi: Use hot and critical ops Daniel Lezcano
@ 2020-12-09 15:34 ` Daniel Lezcano
2020-12-09 15:53 ` [PATCH RFC 1/4] thermal/core: Emit a warning if the thermal zone is updated without ops Lukasz Luba
3 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2020-12-09 15:34 UTC (permalink / raw)
To: daniel.lezcano, rui.zhang
Cc: linux-kernel, linux-pm, lukasz.luba, srinivas.pandruvada, kai.heng.feng
With the remove of the notify user in a previous patch, the ops is no
longer needed, remove it.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/thermal/thermal_core.c | 3 ---
include/linux/thermal.h | 2 --
2 files changed, 5 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 0366f3f076cc..8a47369f0432 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -406,9 +406,6 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
trace_thermal_zone_trip(tz, trip, trip_type);
- if (tz->ops->notify)
- tz->ops->notify(tz, trip, trip_type);
-
if (trip_type == THERMAL_TRIP_HOT && tz->ops->hot)
tz->ops->hot(tz);
else if (trip_type == THERMAL_TRIP_CRITICAL)
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 125c8a4d52e6..7e051b4cf715 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -77,8 +77,6 @@ struct thermal_zone_device_ops {
int (*set_emul_temp) (struct thermal_zone_device *, int);
int (*get_trend) (struct thermal_zone_device *, int,
enum thermal_trend *);
- int (*notify) (struct thermal_zone_device *, int,
- enum thermal_trip_type);
void (*hot)(struct thermal_zone_device *);
void (*critical)(struct thermal_zone_device *);
};
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH RFC 1/4] thermal/core: Emit a warning if the thermal zone is updated without ops
2020-12-09 15:34 [PATCH RFC 1/4] thermal/core: Emit a warning if the thermal zone is updated without ops Daniel Lezcano
` (2 preceding siblings ...)
2020-12-09 15:34 ` [PATCH RFC 4/4] thermal/core: Remove notify ops Daniel Lezcano
@ 2020-12-09 15:53 ` Lukasz Luba
3 siblings, 0 replies; 6+ messages in thread
From: Lukasz Luba @ 2020-12-09 15:53 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rui.zhang, linux-kernel, linux-pm, srinivas.pandruvada, kai.heng.feng
Hi Daniel,
On 12/9/20 3:34 PM, Daniel Lezcano wrote:
> The actual code is silently ignoring a thermal zone update when a
> driver is requesting it without a get_temp ops set.
>
> That looks not correct, as the caller should not have called this
> function if the thermal zone is unable to read the temperature.
>
> That makes the code less robust as the check won't detect the driver
> is inconsistently using the thermal API and that does not help to
> improve the framework as these circumvolutions hide the problem at the
> source.
>
> In order to detect the situation when it happens, let's add a warning
> when the update is requested without the get_temp() ops set.
>
> Any warning emitted will have to be fixed at the source of the
> problem: the caller must not call thermal_zone_device_update if there
> is not get_temp callback set.
>
> Cc: Thara Gopinath <thara.gopinath@linaro.org>
> Cc: Amit Kucheria <amitk@kernel.org>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/thermal/thermal_core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index dee40ff41177..afc02e7d1045 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -548,7 +548,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz,
> if (atomic_read(&in_suspend))
> return;
>
> - if (!tz->ops->get_temp)
> + if (WARN_ONCE(!tz->ops->get_temp, "'%s' must not be called without "
> + "'get_temp' ops set\n", __FUNCTION__))
> return;
>
> update_temperature(tz);
>
With this RFC and the link that you gave me in previous thread, I see
the motivation.
I would change __FUNCTION__ into __func__, but other than that LGTM.
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Regards,
Lukasz
^ permalink raw reply [flat|nested] 6+ messages in thread