Message ID | 20231212221301.12581-1-ansuelsmth@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp8034670vqy; Tue, 12 Dec 2023 14:17:06 -0800 (PST) X-Google-Smtp-Source: AGHT+IHMxhFrs502hgvcng2WgkT3jODCzTQm4mQQyhvj7Nro+84URg2y5ssQIEKuRHIatCH7R7WW X-Received: by 2002:a05:6a00:138c:b0:6ce:6beb:9a42 with SMTP id t12-20020a056a00138c00b006ce6beb9a42mr4455794pfg.64.1702419426618; Tue, 12 Dec 2023 14:17:06 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702419426; cv=none; d=google.com; s=arc-20160816; b=niIUCVD4entLv5+E4gz3D8eClB0e4h2T4sfcQpdu/vHtR9v4idWPMVc0+TIzw4VtYo i4BlMR5HXLGlFAkhfoGsjn2fYKC/zSLZrQl7NYKgPRqEYIcDvFzGeK4atL3RBI8/v2L7 Th24xY/EbRnnELkkQLrXhCoknByZzPQhXh0KNYBhrzWif3gqErX0YzvuK1sq3280v060 zSCchK2LAHo643QCOeW36BP6emQ9BOqgaXVPmXxdeusbMyoxCy++hltu/A9Q/O4DLW0z 56g98zhBwdcemLvxTK7t92TDVLWJBWdSjNvvLezl+pcxFb8P7c6BOSggCfFEkwLYOdHg cyGA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:to:from:dkim-signature; bh=mMdH27R0nN/kULVb9qDYym6teaR7gKvUH7s4Uj8Xhzk=; fh=ErNIe1wpWhLZG5o/AQnmhEEYA4RvBSCAQPu9u9yDwTQ=; b=YAch/nhB2O/TGM60TFPjzqqT3H0bPLInLuZPZ3DweuVm/Q162Trk4Pv0xroXD4F0HG IvSeDbdRr3BncKvzYZ/qlmYms/Uf13g064x7ZA1MbesgB80HJveNDuI091rp4fV4j1uD DkZB7Gael0i5sYazotlYaiSuMyk79faIv0bUw0jqSHqCyggoGV1P95hSX4NvybLbe3g4 0s43Gy2p3sj0bRcL+9TBkhIHEmchFVPJ+M/fav3pil6O3oVCHDrVURcpUkclQBieuyFN a4dlgos9JwwA2GwuQFbdT+5Z1brPxnEoocer6pxjSbXNx4EW6XFyVNx4QTiLg4S1aRHS LMLg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=XBEqpIys; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id w189-20020a6362c6000000b005c6b97e40c4si8032208pgb.297.2023.12.12.14.17.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Dec 2023 14:17:06 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=XBEqpIys; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 82B9A8060669; Tue, 12 Dec 2023 14:17:05 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377753AbjLLWQ4 (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Tue, 12 Dec 2023 17:16:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51652 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231980AbjLLWQz (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 12 Dec 2023 17:16:55 -0500 Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AA6F5AC; Tue, 12 Dec 2023 14:17:00 -0800 (PST) Received: by mail-wm1-x335.google.com with SMTP id 5b1f17b1804b1-40c3984f0cdso47326275e9.1; Tue, 12 Dec 2023 14:17:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702419419; x=1703024219; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:from:to:cc:subject:date:message-id:reply-to; bh=mMdH27R0nN/kULVb9qDYym6teaR7gKvUH7s4Uj8Xhzk=; b=XBEqpIysjMlPITg+9VrSdvec9/FPZFZQS2POFtZ4LQJNJHbcvM/NwWnqP+Z+LY8gYR VMLMXMV7qhavqanbvEFLF5odbenSTaHxaGc781lRB2nABjXqJodNruNOuWwcRHx0UZLa ppc+YKw9tzEbcOziaGbf+p6mN4/DbbziV/ntngRbyQgjJWG3CDv0XLtXqW2plFDnpSqT alSrIxo/8jG7ZHJkqNTa7bnY/KSXp9sEkM0ovkyktnb5wJick8jA9LK1G6UVmxjwKDkN CtgD2x9GdIBjgezQewPk1i4mmPVmmsfBVfK3wj4ZT9kNzX0gTnyCBymdTiqxgyTB9sxp tG0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702419419; x=1703024219; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=mMdH27R0nN/kULVb9qDYym6teaR7gKvUH7s4Uj8Xhzk=; b=izs6Q++Md9Ry4nwbyB6ey/r01jk/3FS/uxLWswLcywGMS5NAjiSuJxZaudzLqLox4r Y7QX1m1IDfvpKdM4BsvAsTNSSgPM+Kgjolb9sh0h8VOl2ppDee4WFEocari02vrUMNHv phefhhfckqIlsL00awEyqpx4pCKWytGwjKGveCqS1VhAhd3jhm0mMYxA35o5pAmhmfNa 50FtW4oToTLET441PbHbEv/1iV4j3g8DOOBR++nZyX+S7xdCdGRCvVwGZJ5h2/N8VEue x1TegDGSfENQ3eJAZu0fbL7KGKDt6AxtIIkiJeI+19rK6ktqy723yFEPqk8LU76hDiL2 iXeQ== X-Gm-Message-State: AOJu0YzgwxneTUWHmcm1PCxzz2IXBbRZIx1i/vlZWY7xowayqS6NTjYk N+4IbWEfJoXR62kRsqH20MM= X-Received: by 2002:a7b:ce13:0:b0:40c:311d:c676 with SMTP id m19-20020a7bce13000000b0040c311dc676mr3727486wmc.137.1702419418813; Tue, 12 Dec 2023 14:16:58 -0800 (PST) Received: from localhost.localdomain (93-34-89-13.ip49.fastwebnet.it. [93.34.89.13]) by smtp.googlemail.com with ESMTPSA id m11-20020a056000008b00b0033332524235sm11669113wrx.82.2023.12.12.14.16.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Dec 2023 14:16:58 -0800 (PST) From: Christian Marangi <ansuelsmth@gmail.com> To: "Rafael J. Wysocki" <rafael@kernel.org>, Daniel Lezcano <daniel.lezcano@linaro.org>, Zhang Rui <rui.zhang@intel.com>, Lukasz Luba <lukasz.luba@arm.com>, Christian Marangi <ansuelsmth@gmail.com>, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/2] thermal: core: add initial support for cold and critical_cold trip point Date: Tue, 12 Dec 2023 23:13:00 +0100 Message-Id: <20231212221301.12581-1-ansuelsmth@gmail.com> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 12 Dec 2023 14:17:05 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785116152883855059 X-GMAIL-MSGID: 1785116152883855059 |
Series |
[1/2] thermal: core: add initial support for cold and critical_cold trip point
|
|
Commit Message
Christian Marangi
Dec. 12, 2023, 10:13 p.m. UTC
Add initial support for cold and critical_cold trip point. Many if not
all hwmon and thermal device have normally trip point for hot
temperature and for cold temperature.
Till now only hot temperature were supported. Add support for also cold
temperature to permit complete definition of cold trip point in DT.
Thermal driver may use these additional trip point to correctly set
interrupt for cold temperature values and react based on that with
various measure like enabling attached heater, forcing higher voltage
and other specialaized peripherals.
For hwmon drivers this is needed as currently there is a problem with
setting the full operating range of the device for thermal devices
defined with hwmon. To better describe the problem, the following
example is needed:
In the scenario of a simple hwmon with an active trip point declared
and a cooling device attached, the hwmon subsystem currently set the
min and max trip point based on the single active trip point.
Thermal subsystem parse all the trip points and calculate the lowest and
the highest trip point and calls the .set_trip of hwmon to setup the
trip points.
The fact that we currently don't have a way to declare the cold/min
temperature values, makes the thermal subsystem to set the low value as
-INT_MAX.
For hwmon drivers that doesn't use clamp_value and actually reject
invalid values for the trip point, this results in the hwmon settings to
be rejected.
To permit to pass the correct range of trip point, permit to set in DT
also cold and critical_cold trip point.
Thermal driver may also define .cold and .critical_cold to act on these
trip point tripped and apply the required measure.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/thermal/thermal_core.c | 13 +++++++++++++
drivers/thermal/thermal_of.c | 2 ++
drivers/thermal/thermal_sysfs.c | 4 ++++
drivers/thermal/thermal_trace.h | 4 ++++
include/linux/thermal.h | 2 ++
include/uapi/linux/thermal.h | 2 ++
6 files changed, 27 insertions(+)
Comments
On Tue, Dec 12, 2023 at 11:17 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > Add initial support for cold and critical_cold trip point. Many if not > all hwmon and thermal device have normally trip point for hot > temperature and for cold temperature. > > Till now only hot temperature were supported. Add support for also cold > temperature to permit complete definition of cold trip point in DT. > > Thermal driver may use these additional trip point to correctly set > interrupt for cold temperature values and react based on that with > various measure like enabling attached heater, forcing higher voltage > and other specialaized peripherals. > > For hwmon drivers this is needed as currently there is a problem with > setting the full operating range of the device for thermal devices > defined with hwmon. To better describe the problem, the following > example is needed: > > In the scenario of a simple hwmon with an active trip point declared > and a cooling device attached, the hwmon subsystem currently set the > min and max trip point based on the single active trip point. > Thermal subsystem parse all the trip points and calculate the lowest and > the highest trip point and calls the .set_trip of hwmon to setup the > trip points. > > The fact that we currently don't have a way to declare the cold/min > temperature values, makes the thermal subsystem to set the low value as > -INT_MAX. > For hwmon drivers that doesn't use clamp_value and actually reject > invalid values for the trip point, this results in the hwmon settings to > be rejected. > > To permit to pass the correct range of trip point, permit to set in DT > also cold and critical_cold trip point. > > Thermal driver may also define .cold and .critical_cold to act on these > trip point tripped and apply the required measure. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Generally speaking, it is kind of late in the cycle for adding significant new features like this. We can get to it when 6.8-rc1 is out, so please resend then. Also it would be nice to define/document the cold and crtitical_cold trip points somewhere and is there a better name for critical_cold? > --- > drivers/thermal/thermal_core.c | 13 +++++++++++++ > drivers/thermal/thermal_of.c | 2 ++ > drivers/thermal/thermal_sysfs.c | 4 ++++ > drivers/thermal/thermal_trace.h | 4 ++++ > include/linux/thermal.h | 2 ++ > include/uapi/linux/thermal.h | 2 ++ > 6 files changed, 27 insertions(+) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index 9c17d35ccbbd..3c5ab560e72f 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -344,6 +344,17 @@ static void handle_critical_trips(struct thermal_zone_device *tz, > tz->ops->hot(tz); > } > > +static void handle_critical_cold_trips(struct thermal_zone_device *tz, > + const struct thermal_trip *trip) > +{ > + trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type); > + > + if (trip->type == THERMAL_TRIP_CRITICAL_COLD && tz->ops->critical_cold) > + tz->ops->critical_cold(tz); > + else if (trip->type == THERMAL_TRIP_COLD && tz->ops->cold) > + tz->ops->cold(tz); > +} > + > static void handle_thermal_trip(struct thermal_zone_device *tz, > const struct thermal_trip *trip) > { > @@ -365,6 +376,8 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, > > if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT) > handle_critical_trips(tz, trip); > + else if (trip->type == THERMAL_TRIP_CRITICAL_COLD || trip->type == THERMAL_TRIP_COLD) > + handle_critical_cold_trips(tz, trip); > else > handle_non_critical_trips(tz, trip); > } > diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c > index 1e0655b63259..95bc600bb4b8 100644 > --- a/drivers/thermal/thermal_of.c > +++ b/drivers/thermal/thermal_of.c > @@ -60,6 +60,8 @@ static const char * const trip_types[] = { > [THERMAL_TRIP_PASSIVE] = "passive", > [THERMAL_TRIP_HOT] = "hot", > [THERMAL_TRIP_CRITICAL] = "critical", > + [THERMAL_TRIP_COLD] = "cold", > + [THERMAL_TRIP_CRITICAL_COLD] = "critical_cold", > }; > > /** > diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c > index eef40d4f3063..e1e69e0991c2 100644 > --- a/drivers/thermal/thermal_sysfs.c > +++ b/drivers/thermal/thermal_sysfs.c > @@ -106,6 +106,10 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr, > return sprintf(buf, "critical\n"); > case THERMAL_TRIP_HOT: > return sprintf(buf, "hot\n"); > + case THERMAL_TRIP_COLD: > + return sprintf(buf, "cold\n"); > + case THERMAL_TRIP_CRITICAL_COLD: > + return sprintf(buf, "critical_cold\n"); > case THERMAL_TRIP_PASSIVE: > return sprintf(buf, "passive\n"); > case THERMAL_TRIP_ACTIVE: > diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h > index 459c8ce6cf3b..0a4f96075d7d 100644 > --- a/drivers/thermal/thermal_trace.h > +++ b/drivers/thermal/thermal_trace.h > @@ -11,6 +11,8 @@ > > TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL); > TRACE_DEFINE_ENUM(THERMAL_TRIP_HOT); > +TRACE_DEFINE_ENUM(THERMAL_TRIP_COLD); > +TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL_COLD); > TRACE_DEFINE_ENUM(THERMAL_TRIP_PASSIVE); > TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); > > @@ -18,6 +20,8 @@ TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); > __print_symbolic(type, \ > { THERMAL_TRIP_CRITICAL, "CRITICAL"}, \ > { THERMAL_TRIP_HOT, "HOT"}, \ > + { THERMAL_TRIP_COLD, "COLD"}, \ > + { THERMAL_TRIP_CRITICAL_COLD, "CRITICAL_COLD"}, \ > { THERMAL_TRIP_PASSIVE, "PASSIVE"}, \ > { THERMAL_TRIP_ACTIVE, "ACTIVE"}) > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index cee814d5d1ac..d6345c9ec50d 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -84,6 +84,8 @@ struct thermal_zone_device_ops { > const struct thermal_trip *, enum thermal_trend *); > void (*hot)(struct thermal_zone_device *); > void (*critical)(struct thermal_zone_device *); > + void (*cold)(struct thermal_zone_device *); > + void (*critical_cold)(struct thermal_zone_device *); > }; > > struct thermal_cooling_device_ops { > diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h > index fc78bf3aead7..7fa1ba0dff05 100644 > --- a/include/uapi/linux/thermal.h > +++ b/include/uapi/linux/thermal.h > @@ -14,6 +14,8 @@ enum thermal_trip_type { > THERMAL_TRIP_PASSIVE, > THERMAL_TRIP_HOT, > THERMAL_TRIP_CRITICAL, > + THERMAL_TRIP_COLD, > + THERMAL_TRIP_CRITICAL_COLD, > }; > > /* Adding event notification support elements */ > -- > 2.40.1 >
On Wed, Dec 13, 2023 at 01:01:51PM +0100, Rafael J. Wysocki wrote: > On Tue, Dec 12, 2023 at 11:17 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > > Add initial support for cold and critical_cold trip point. Many if not > > all hwmon and thermal device have normally trip point for hot > > temperature and for cold temperature. > > > > Till now only hot temperature were supported. Add support for also cold > > temperature to permit complete definition of cold trip point in DT. > > > > Thermal driver may use these additional trip point to correctly set > > interrupt for cold temperature values and react based on that with > > various measure like enabling attached heater, forcing higher voltage > > and other specialaized peripherals. > > > > For hwmon drivers this is needed as currently there is a problem with > > setting the full operating range of the device for thermal devices > > defined with hwmon. To better describe the problem, the following > > example is needed: > > > > In the scenario of a simple hwmon with an active trip point declared > > and a cooling device attached, the hwmon subsystem currently set the > > min and max trip point based on the single active trip point. > > Thermal subsystem parse all the trip points and calculate the lowest and > > the highest trip point and calls the .set_trip of hwmon to setup the > > trip points. > > > > The fact that we currently don't have a way to declare the cold/min > > temperature values, makes the thermal subsystem to set the low value as > > -INT_MAX. > > For hwmon drivers that doesn't use clamp_value and actually reject > > invalid values for the trip point, this results in the hwmon settings to > > be rejected. > > > > To permit to pass the correct range of trip point, permit to set in DT > > also cold and critical_cold trip point. > > > > Thermal driver may also define .cold and .critical_cold to act on these > > trip point tripped and apply the required measure. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > Generally speaking, it is kind of late in the cycle for adding > significant new features like this. We can get to it when 6.8-rc1 is > out, so please resend then. > Ok no problem. > Also it would be nice to define/document the cold and crtitical_cold > trip points somewhere and is there a better name for critical_cold? > Ehhh I think critical_cold is the only correct one. Thermal device normally have lowest low high and highest trip point. I think the lowest has to match what we use for highest (critical). Using coldest might be confusing and wouldn't display a critical condition of low temp where the device can't work and immediate actions has to be taken. > > --- > > drivers/thermal/thermal_core.c | 13 +++++++++++++ > > drivers/thermal/thermal_of.c | 2 ++ > > drivers/thermal/thermal_sysfs.c | 4 ++++ > > drivers/thermal/thermal_trace.h | 4 ++++ > > include/linux/thermal.h | 2 ++ > > include/uapi/linux/thermal.h | 2 ++ > > 6 files changed, 27 insertions(+) > > > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > > index 9c17d35ccbbd..3c5ab560e72f 100644 > > --- a/drivers/thermal/thermal_core.c > > +++ b/drivers/thermal/thermal_core.c > > @@ -344,6 +344,17 @@ static void handle_critical_trips(struct thermal_zone_device *tz, > > tz->ops->hot(tz); > > } > > > > +static void handle_critical_cold_trips(struct thermal_zone_device *tz, > > + const struct thermal_trip *trip) > > +{ > > + trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type); > > + > > + if (trip->type == THERMAL_TRIP_CRITICAL_COLD && tz->ops->critical_cold) > > + tz->ops->critical_cold(tz); > > + else if (trip->type == THERMAL_TRIP_COLD && tz->ops->cold) > > + tz->ops->cold(tz); > > +} > > + > > static void handle_thermal_trip(struct thermal_zone_device *tz, > > const struct thermal_trip *trip) > > { > > @@ -365,6 +376,8 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, > > > > if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT) > > handle_critical_trips(tz, trip); > > + else if (trip->type == THERMAL_TRIP_CRITICAL_COLD || trip->type == THERMAL_TRIP_COLD) > > + handle_critical_cold_trips(tz, trip); > > else > > handle_non_critical_trips(tz, trip); > > } > > diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c > > index 1e0655b63259..95bc600bb4b8 100644 > > --- a/drivers/thermal/thermal_of.c > > +++ b/drivers/thermal/thermal_of.c > > @@ -60,6 +60,8 @@ static const char * const trip_types[] = { > > [THERMAL_TRIP_PASSIVE] = "passive", > > [THERMAL_TRIP_HOT] = "hot", > > [THERMAL_TRIP_CRITICAL] = "critical", > > + [THERMAL_TRIP_COLD] = "cold", > > + [THERMAL_TRIP_CRITICAL_COLD] = "critical_cold", > > }; > > > > /** > > diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c > > index eef40d4f3063..e1e69e0991c2 100644 > > --- a/drivers/thermal/thermal_sysfs.c > > +++ b/drivers/thermal/thermal_sysfs.c > > @@ -106,6 +106,10 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr, > > return sprintf(buf, "critical\n"); > > case THERMAL_TRIP_HOT: > > return sprintf(buf, "hot\n"); > > + case THERMAL_TRIP_COLD: > > + return sprintf(buf, "cold\n"); > > + case THERMAL_TRIP_CRITICAL_COLD: > > + return sprintf(buf, "critical_cold\n"); > > case THERMAL_TRIP_PASSIVE: > > return sprintf(buf, "passive\n"); > > case THERMAL_TRIP_ACTIVE: > > diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h > > index 459c8ce6cf3b..0a4f96075d7d 100644 > > --- a/drivers/thermal/thermal_trace.h > > +++ b/drivers/thermal/thermal_trace.h > > @@ -11,6 +11,8 @@ > > > > TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL); > > TRACE_DEFINE_ENUM(THERMAL_TRIP_HOT); > > +TRACE_DEFINE_ENUM(THERMAL_TRIP_COLD); > > +TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL_COLD); > > TRACE_DEFINE_ENUM(THERMAL_TRIP_PASSIVE); > > TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); > > > > @@ -18,6 +20,8 @@ TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); > > __print_symbolic(type, \ > > { THERMAL_TRIP_CRITICAL, "CRITICAL"}, \ > > { THERMAL_TRIP_HOT, "HOT"}, \ > > + { THERMAL_TRIP_COLD, "COLD"}, \ > > + { THERMAL_TRIP_CRITICAL_COLD, "CRITICAL_COLD"}, \ > > { THERMAL_TRIP_PASSIVE, "PASSIVE"}, \ > > { THERMAL_TRIP_ACTIVE, "ACTIVE"}) > > > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > > index cee814d5d1ac..d6345c9ec50d 100644 > > --- a/include/linux/thermal.h > > +++ b/include/linux/thermal.h > > @@ -84,6 +84,8 @@ struct thermal_zone_device_ops { > > const struct thermal_trip *, enum thermal_trend *); > > void (*hot)(struct thermal_zone_device *); > > void (*critical)(struct thermal_zone_device *); > > + void (*cold)(struct thermal_zone_device *); > > + void (*critical_cold)(struct thermal_zone_device *); > > }; > > > > struct thermal_cooling_device_ops { > > diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h > > index fc78bf3aead7..7fa1ba0dff05 100644 > > --- a/include/uapi/linux/thermal.h > > +++ b/include/uapi/linux/thermal.h > > @@ -14,6 +14,8 @@ enum thermal_trip_type { > > THERMAL_TRIP_PASSIVE, > > THERMAL_TRIP_HOT, > > THERMAL_TRIP_CRITICAL, > > + THERMAL_TRIP_COLD, > > + THERMAL_TRIP_CRITICAL_COLD, > > }; > > > > /* Adding event notification support elements */ > > -- > > 2.40.1 > >
On 12/12/2023 23:13, Christian Marangi wrote: > Add initial support for cold and critical_cold trip point. Many if not > all hwmon and thermal device have normally trip point for hot > temperature and for cold temperature. > > Till now only hot temperature were supported. Add support for also cold > temperature to permit complete definition of cold trip point in DT. > > Thermal driver may use these additional trip point to correctly set > interrupt for cold temperature values and react based on that with > various measure like enabling attached heater, forcing higher voltage > and other specialaized peripherals. > > For hwmon drivers this is needed as currently there is a problem with > setting the full operating range of the device for thermal devices > defined with hwmon. To better describe the problem, the following > example is needed: > > In the scenario of a simple hwmon with an active trip point declared > and a cooling device attached, the hwmon subsystem currently set the > min and max trip point based on the single active trip point. > Thermal subsystem parse all the trip points and calculate the lowest and > the highest trip point and calls the .set_trip of hwmon to setup the > trip points. > > The fact that we currently don't have a way to declare the cold/min > temperature values, makes the thermal subsystem to set the low value as > -INT_MAX. > For hwmon drivers that doesn't use clamp_value and actually reject > invalid values for the trip point, this results in the hwmon settings to > be rejected. > > To permit to pass the correct range of trip point, permit to set in DT > also cold and critical_cold trip point. > > Thermal driver may also define .cold and .critical_cold to act on these > trip point tripped and apply the required measure. Agree with the feature but we need to clarify the semantic of the trip points first. What actions do we expect for them in order to have like a mirror reflection of the existing hot trip points. What action do you expect with: - 'cold' ? - 'critical_cold' ? > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Why are there two different names for the same email address? - Christian Marangi <ansuelsmth@gmail.com> - Ansuel Smith <ansuelsmth@gmail.com> > --- > drivers/thermal/thermal_core.c | 13 +++++++++++++ > drivers/thermal/thermal_of.c | 2 ++ > drivers/thermal/thermal_sysfs.c | 4 ++++ > drivers/thermal/thermal_trace.h | 4 ++++ > include/linux/thermal.h | 2 ++ > include/uapi/linux/thermal.h | 2 ++ > 6 files changed, 27 insertions(+) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index 9c17d35ccbbd..3c5ab560e72f 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -344,6 +344,17 @@ static void handle_critical_trips(struct thermal_zone_device *tz, > tz->ops->hot(tz); > } > > +static void handle_critical_cold_trips(struct thermal_zone_device *tz, > + const struct thermal_trip *trip) > +{ > + trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type); > + > + if (trip->type == THERMAL_TRIP_CRITICAL_COLD && tz->ops->critical_cold) > + tz->ops->critical_cold(tz); > + else if (trip->type == THERMAL_TRIP_COLD && tz->ops->cold) > + tz->ops->cold(tz); > +} > + > static void handle_thermal_trip(struct thermal_zone_device *tz, > const struct thermal_trip *trip) > { > @@ -365,6 +376,8 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, > > if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT) > handle_critical_trips(tz, trip); > + else if (trip->type == THERMAL_TRIP_CRITICAL_COLD || trip->type == THERMAL_TRIP_COLD) > + handle_critical_cold_trips(tz, trip); > else > handle_non_critical_trips(tz, trip); > } > diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c > index 1e0655b63259..95bc600bb4b8 100644 > --- a/drivers/thermal/thermal_of.c > +++ b/drivers/thermal/thermal_of.c > @@ -60,6 +60,8 @@ static const char * const trip_types[] = { > [THERMAL_TRIP_PASSIVE] = "passive", > [THERMAL_TRIP_HOT] = "hot", > [THERMAL_TRIP_CRITICAL] = "critical", > + [THERMAL_TRIP_COLD] = "cold", > + [THERMAL_TRIP_CRITICAL_COLD] = "critical_cold", > }; > > /** > diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c > index eef40d4f3063..e1e69e0991c2 100644 > --- a/drivers/thermal/thermal_sysfs.c > +++ b/drivers/thermal/thermal_sysfs.c > @@ -106,6 +106,10 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr, > return sprintf(buf, "critical\n"); > case THERMAL_TRIP_HOT: > return sprintf(buf, "hot\n"); > + case THERMAL_TRIP_COLD: > + return sprintf(buf, "cold\n"); > + case THERMAL_TRIP_CRITICAL_COLD: > + return sprintf(buf, "critical_cold\n"); > case THERMAL_TRIP_PASSIVE: > return sprintf(buf, "passive\n"); > case THERMAL_TRIP_ACTIVE: > diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h > index 459c8ce6cf3b..0a4f96075d7d 100644 > --- a/drivers/thermal/thermal_trace.h > +++ b/drivers/thermal/thermal_trace.h > @@ -11,6 +11,8 @@ > > TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL); > TRACE_DEFINE_ENUM(THERMAL_TRIP_HOT); > +TRACE_DEFINE_ENUM(THERMAL_TRIP_COLD); > +TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL_COLD); > TRACE_DEFINE_ENUM(THERMAL_TRIP_PASSIVE); > TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); > > @@ -18,6 +20,8 @@ TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); > __print_symbolic(type, \ > { THERMAL_TRIP_CRITICAL, "CRITICAL"}, \ > { THERMAL_TRIP_HOT, "HOT"}, \ > + { THERMAL_TRIP_COLD, "COLD"}, \ > + { THERMAL_TRIP_CRITICAL_COLD, "CRITICAL_COLD"}, \ > { THERMAL_TRIP_PASSIVE, "PASSIVE"}, \ > { THERMAL_TRIP_ACTIVE, "ACTIVE"}) > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index cee814d5d1ac..d6345c9ec50d 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -84,6 +84,8 @@ struct thermal_zone_device_ops { > const struct thermal_trip *, enum thermal_trend *); > void (*hot)(struct thermal_zone_device *); > void (*critical)(struct thermal_zone_device *); > + void (*cold)(struct thermal_zone_device *); > + void (*critical_cold)(struct thermal_zone_device *); > }; > > struct thermal_cooling_device_ops { > diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h > index fc78bf3aead7..7fa1ba0dff05 100644 > --- a/include/uapi/linux/thermal.h > +++ b/include/uapi/linux/thermal.h > @@ -14,6 +14,8 @@ enum thermal_trip_type { > THERMAL_TRIP_PASSIVE, > THERMAL_TRIP_HOT, > THERMAL_TRIP_CRITICAL, > + THERMAL_TRIP_COLD, > + THERMAL_TRIP_CRITICAL_COLD, > }; > > /* Adding event notification support elements */
On Wed, Dec 13, 2023 at 03:12:41PM +0100, Daniel Lezcano wrote: > On 12/12/2023 23:13, Christian Marangi wrote: > > Add initial support for cold and critical_cold trip point. Many if not > > all hwmon and thermal device have normally trip point for hot > > temperature and for cold temperature. > > > > Till now only hot temperature were supported. Add support for also cold > > temperature to permit complete definition of cold trip point in DT. > > > > Thermal driver may use these additional trip point to correctly set > > interrupt for cold temperature values and react based on that with > > various measure like enabling attached heater, forcing higher voltage > > and other specialaized peripherals. > > > > For hwmon drivers this is needed as currently there is a problem with > > setting the full operating range of the device for thermal devices > > defined with hwmon. To better describe the problem, the following > > example is needed: > > > > In the scenario of a simple hwmon with an active trip point declared > > and a cooling device attached, the hwmon subsystem currently set the > > min and max trip point based on the single active trip point. > > Thermal subsystem parse all the trip points and calculate the lowest and > > the highest trip point and calls the .set_trip of hwmon to setup the > > trip points. > > > > The fact that we currently don't have a way to declare the cold/min > > temperature values, makes the thermal subsystem to set the low value as > > -INT_MAX. > > For hwmon drivers that doesn't use clamp_value and actually reject > > invalid values for the trip point, this results in the hwmon settings to > > be rejected. > > > > To permit to pass the correct range of trip point, permit to set in DT > > also cold and critical_cold trip point. > > > > Thermal driver may also define .cold and .critical_cold to act on these > > trip point tripped and apply the required measure. > > Agree with the feature but we need to clarify the semantic of the trip > points first. What actions do we expect for them in order to have like a > mirror reflection of the existing hot trip points. > > What action do you expect with: > > - 'cold' ? > > - 'critical_cold' ? > > This is more of a sensible topic but I think it's the thermal driver that needs to implement these. As said in the commit description, examples are setting higher voltage from the attached regulator, enabling some hardware heater. Maybe with critical cold bigger measure can be applied. Currently for critical trip point we shutdown the system (if the critical ops is not declared) but with critical_cold condition I think it won't work... I expect a system in -40°C would just lock up/glitch so rebooting in that condition won't change a thing... Anyway yes we can define a shutdown by default for that but IMHO it wouldn't make much sense. > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > Why are there two different names for the same email address? > > - Christian Marangi <ansuelsmth@gmail.com> > - Ansuel Smith <ansuelsmth@gmail.com> > Stupid mistake in my ""childhood"". Ansuel it's my second name, when I understood things have started to become more serious I fixed the very stupid thing and started using correct naming. I'm extremely sorry for the mistake I did and I know all the problems it does cause doing that. > > > --- > > drivers/thermal/thermal_core.c | 13 +++++++++++++ > > drivers/thermal/thermal_of.c | 2 ++ > > drivers/thermal/thermal_sysfs.c | 4 ++++ > > drivers/thermal/thermal_trace.h | 4 ++++ > > include/linux/thermal.h | 2 ++ > > include/uapi/linux/thermal.h | 2 ++ > > 6 files changed, 27 insertions(+) > > > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > > index 9c17d35ccbbd..3c5ab560e72f 100644 > > --- a/drivers/thermal/thermal_core.c > > +++ b/drivers/thermal/thermal_core.c > > @@ -344,6 +344,17 @@ static void handle_critical_trips(struct thermal_zone_device *tz, > > tz->ops->hot(tz); > > } > > +static void handle_critical_cold_trips(struct thermal_zone_device *tz, > > + const struct thermal_trip *trip) > > +{ > > + trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type); > > + > > + if (trip->type == THERMAL_TRIP_CRITICAL_COLD && tz->ops->critical_cold) > > + tz->ops->critical_cold(tz); > > + else if (trip->type == THERMAL_TRIP_COLD && tz->ops->cold) > > + tz->ops->cold(tz); > > +} > > + > > static void handle_thermal_trip(struct thermal_zone_device *tz, > > const struct thermal_trip *trip) > > { > > @@ -365,6 +376,8 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, > > if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT) > > handle_critical_trips(tz, trip); > > + else if (trip->type == THERMAL_TRIP_CRITICAL_COLD || trip->type == THERMAL_TRIP_COLD) > > + handle_critical_cold_trips(tz, trip); > > else > > handle_non_critical_trips(tz, trip); > > } > > diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c > > index 1e0655b63259..95bc600bb4b8 100644 > > --- a/drivers/thermal/thermal_of.c > > +++ b/drivers/thermal/thermal_of.c > > @@ -60,6 +60,8 @@ static const char * const trip_types[] = { > > [THERMAL_TRIP_PASSIVE] = "passive", > > [THERMAL_TRIP_HOT] = "hot", > > [THERMAL_TRIP_CRITICAL] = "critical", > > + [THERMAL_TRIP_COLD] = "cold", > > + [THERMAL_TRIP_CRITICAL_COLD] = "critical_cold", > > }; > > /** > > diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c > > index eef40d4f3063..e1e69e0991c2 100644 > > --- a/drivers/thermal/thermal_sysfs.c > > +++ b/drivers/thermal/thermal_sysfs.c > > @@ -106,6 +106,10 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr, > > return sprintf(buf, "critical\n"); > > case THERMAL_TRIP_HOT: > > return sprintf(buf, "hot\n"); > > + case THERMAL_TRIP_COLD: > > + return sprintf(buf, "cold\n"); > > + case THERMAL_TRIP_CRITICAL_COLD: > > + return sprintf(buf, "critical_cold\n"); > > case THERMAL_TRIP_PASSIVE: > > return sprintf(buf, "passive\n"); > > case THERMAL_TRIP_ACTIVE: > > diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h > > index 459c8ce6cf3b..0a4f96075d7d 100644 > > --- a/drivers/thermal/thermal_trace.h > > +++ b/drivers/thermal/thermal_trace.h > > @@ -11,6 +11,8 @@ > > TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL); > > TRACE_DEFINE_ENUM(THERMAL_TRIP_HOT); > > +TRACE_DEFINE_ENUM(THERMAL_TRIP_COLD); > > +TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL_COLD); > > TRACE_DEFINE_ENUM(THERMAL_TRIP_PASSIVE); > > TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); > > @@ -18,6 +20,8 @@ TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); > > __print_symbolic(type, \ > > { THERMAL_TRIP_CRITICAL, "CRITICAL"}, \ > > { THERMAL_TRIP_HOT, "HOT"}, \ > > + { THERMAL_TRIP_COLD, "COLD"}, \ > > + { THERMAL_TRIP_CRITICAL_COLD, "CRITICAL_COLD"}, \ > > { THERMAL_TRIP_PASSIVE, "PASSIVE"}, \ > > { THERMAL_TRIP_ACTIVE, "ACTIVE"}) > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > > index cee814d5d1ac..d6345c9ec50d 100644 > > --- a/include/linux/thermal.h > > +++ b/include/linux/thermal.h > > @@ -84,6 +84,8 @@ struct thermal_zone_device_ops { > > const struct thermal_trip *, enum thermal_trend *); > > void (*hot)(struct thermal_zone_device *); > > void (*critical)(struct thermal_zone_device *); > > + void (*cold)(struct thermal_zone_device *); > > + void (*critical_cold)(struct thermal_zone_device *); > > }; > > struct thermal_cooling_device_ops { > > diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h > > index fc78bf3aead7..7fa1ba0dff05 100644 > > --- a/include/uapi/linux/thermal.h > > +++ b/include/uapi/linux/thermal.h > > @@ -14,6 +14,8 @@ enum thermal_trip_type { > > THERMAL_TRIP_PASSIVE, > > THERMAL_TRIP_HOT, > > THERMAL_TRIP_CRITICAL, > > + THERMAL_TRIP_COLD, > > + THERMAL_TRIP_CRITICAL_COLD, > > }; > > /* Adding event notification support elements */ > > -- > <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 >
On Wed, Dec 13, 2023 at 1:06 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > On Wed, Dec 13, 2023 at 01:01:51PM +0100, Rafael J. Wysocki wrote: > > On Tue, Dec 12, 2023 at 11:17 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > > > > Add initial support for cold and critical_cold trip point. Many if not > > > all hwmon and thermal device have normally trip point for hot > > > temperature and for cold temperature. > > > > > > Till now only hot temperature were supported. Add support for also cold > > > temperature to permit complete definition of cold trip point in DT. > > > > > > Thermal driver may use these additional trip point to correctly set > > > interrupt for cold temperature values and react based on that with > > > various measure like enabling attached heater, forcing higher voltage > > > and other specialaized peripherals. > > > > > > For hwmon drivers this is needed as currently there is a problem with > > > setting the full operating range of the device for thermal devices > > > defined with hwmon. To better describe the problem, the following > > > example is needed: > > > > > > In the scenario of a simple hwmon with an active trip point declared > > > and a cooling device attached, the hwmon subsystem currently set the > > > min and max trip point based on the single active trip point. > > > Thermal subsystem parse all the trip points and calculate the lowest and > > > the highest trip point and calls the .set_trip of hwmon to setup the > > > trip points. > > > > > > The fact that we currently don't have a way to declare the cold/min > > > temperature values, makes the thermal subsystem to set the low value as > > > -INT_MAX. > > > For hwmon drivers that doesn't use clamp_value and actually reject > > > invalid values for the trip point, this results in the hwmon settings to > > > be rejected. > > > > > > To permit to pass the correct range of trip point, permit to set in DT > > > also cold and critical_cold trip point. > > > > > > Thermal driver may also define .cold and .critical_cold to act on these > > > trip point tripped and apply the required measure. > > > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > > Generally speaking, it is kind of late in the cycle for adding > > significant new features like this. We can get to it when 6.8-rc1 is > > out, so please resend then. > > > > Ok no problem. > > > Also it would be nice to define/document the cold and crtitical_cold > > trip points somewhere and is there a better name for critical_cold? > > > > Ehhh I think critical_cold is the only correct one. > Thermal device normally have lowest low high and highest trip point. I > think the lowest has to match what we use for highest (critical). Using > coldest might be confusing and wouldn't display a critical condition of > low temp where the device can't work and immediate actions has to be > taken. So at least make it shorter, like crit_cold?
On Wed, Dec 13, 2023 at 3:20 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > On Wed, Dec 13, 2023 at 03:12:41PM +0100, Daniel Lezcano wrote: > > On 12/12/2023 23:13, Christian Marangi wrote: > > > Add initial support for cold and critical_cold trip point. Many if not > > > all hwmon and thermal device have normally trip point for hot > > > temperature and for cold temperature. > > > > > > Till now only hot temperature were supported. Add support for also cold > > > temperature to permit complete definition of cold trip point in DT. > > > > > > Thermal driver may use these additional trip point to correctly set > > > interrupt for cold temperature values and react based on that with > > > various measure like enabling attached heater, forcing higher voltage > > > and other specialaized peripherals. > > > > > > For hwmon drivers this is needed as currently there is a problem with > > > setting the full operating range of the device for thermal devices > > > defined with hwmon. To better describe the problem, the following > > > example is needed: > > > > > > In the scenario of a simple hwmon with an active trip point declared > > > and a cooling device attached, the hwmon subsystem currently set the > > > min and max trip point based on the single active trip point. > > > Thermal subsystem parse all the trip points and calculate the lowest and > > > the highest trip point and calls the .set_trip of hwmon to setup the > > > trip points. > > > > > > The fact that we currently don't have a way to declare the cold/min > > > temperature values, makes the thermal subsystem to set the low value as > > > -INT_MAX. > > > For hwmon drivers that doesn't use clamp_value and actually reject > > > invalid values for the trip point, this results in the hwmon settings to > > > be rejected. > > > > > > To permit to pass the correct range of trip point, permit to set in DT > > > also cold and critical_cold trip point. > > > > > > Thermal driver may also define .cold and .critical_cold to act on these > > > trip point tripped and apply the required measure. > > > > Agree with the feature but we need to clarify the semantic of the trip > > points first. What actions do we expect for them in order to have like a > > mirror reflection of the existing hot trip points. > > > > What action do you expect with: > > > > - 'cold' ? > > > > - 'critical_cold' ? > > > > > > This is more of a sensible topic but I think it's the thermal driver > that needs to implement these. As said in the commit description, > examples are setting higher voltage from the attached regulator, > enabling some hardware heater. So how is it different from an active trip point? There are heating rather than cooling devices associated with it, but other than this?? > Maybe with critical cold bigger measure can be applied. Currently for > critical trip point we shutdown the system (if the critical ops is not > declared) but with critical_cold condition I think it won't work... I > expect a system in -40°C would just lock up/glitch so rebooting in that > condition won't change a thing... > > Anyway yes we can define a shutdown by default for that but IMHO it > wouldn't make much sense. So why do you want to add it at all?
On Wed, Dec 13, 2023 at 3:43 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Wed, Dec 13, 2023 at 3:20 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > > On Wed, Dec 13, 2023 at 03:12:41PM +0100, Daniel Lezcano wrote: > > > On 12/12/2023 23:13, Christian Marangi wrote: > > > > Add initial support for cold and critical_cold trip point. Many if not > > > > all hwmon and thermal device have normally trip point for hot > > > > temperature and for cold temperature. > > > > > > > > Till now only hot temperature were supported. Add support for also cold > > > > temperature to permit complete definition of cold trip point in DT. > > > > > > > > Thermal driver may use these additional trip point to correctly set > > > > interrupt for cold temperature values and react based on that with > > > > various measure like enabling attached heater, forcing higher voltage > > > > and other specialaized peripherals. > > > > > > > > For hwmon drivers this is needed as currently there is a problem with > > > > setting the full operating range of the device for thermal devices > > > > defined with hwmon. To better describe the problem, the following > > > > example is needed: > > > > > > > > In the scenario of a simple hwmon with an active trip point declared > > > > and a cooling device attached, the hwmon subsystem currently set the > > > > min and max trip point based on the single active trip point. > > > > Thermal subsystem parse all the trip points and calculate the lowest and > > > > the highest trip point and calls the .set_trip of hwmon to setup the > > > > trip points. > > > > > > > > The fact that we currently don't have a way to declare the cold/min > > > > temperature values, makes the thermal subsystem to set the low value as > > > > -INT_MAX. > > > > For hwmon drivers that doesn't use clamp_value and actually reject > > > > invalid values for the trip point, this results in the hwmon settings to > > > > be rejected. > > > > > > > > To permit to pass the correct range of trip point, permit to set in DT > > > > also cold and critical_cold trip point. > > > > > > > > Thermal driver may also define .cold and .critical_cold to act on these > > > > trip point tripped and apply the required measure. > > > > > > Agree with the feature but we need to clarify the semantic of the trip > > > points first. What actions do we expect for them in order to have like a > > > mirror reflection of the existing hot trip points. > > > > > > What action do you expect with: > > > > > > - 'cold' ? > > > > > > - 'critical_cold' ? > > > > > > > > > > This is more of a sensible topic but I think it's the thermal driver > > that needs to implement these. As said in the commit description, > > examples are setting higher voltage from the attached regulator, > > enabling some hardware heater. > > So how is it different from an active trip point? There are heating > rather than cooling devices associated with it, but other than this?? And, of course, the mitigation would trigger when crossed on the way down and the hysteresis value would be added to the temperature. So it all looks like a "reverse" active trip point. > > Maybe with critical cold bigger measure can be applied. Currently for > > critical trip point we shutdown the system (if the critical ops is not > > declared) but with critical_cold condition I think it won't work... I > > expect a system in -40°C would just lock up/glitch so rebooting in that > > condition won't change a thing... > > > > Anyway yes we can define a shutdown by default for that but IMHO it > > wouldn't make much sense. > > So why do you want to add it at all?
On Wed, Dec 13, 2023 at 03:43:54PM +0100, Rafael J. Wysocki wrote: > On Wed, Dec 13, 2023 at 3:20 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > > On Wed, Dec 13, 2023 at 03:12:41PM +0100, Daniel Lezcano wrote: > > > On 12/12/2023 23:13, Christian Marangi wrote: > > > > Add initial support for cold and critical_cold trip point. Many if not > > > > all hwmon and thermal device have normally trip point for hot > > > > temperature and for cold temperature. > > > > > > > > Till now only hot temperature were supported. Add support for also cold > > > > temperature to permit complete definition of cold trip point in DT. > > > > > > > > Thermal driver may use these additional trip point to correctly set > > > > interrupt for cold temperature values and react based on that with > > > > various measure like enabling attached heater, forcing higher voltage > > > > and other specialaized peripherals. > > > > > > > > For hwmon drivers this is needed as currently there is a problem with > > > > setting the full operating range of the device for thermal devices > > > > defined with hwmon. To better describe the problem, the following > > > > example is needed: > > > > > > > > In the scenario of a simple hwmon with an active trip point declared > > > > and a cooling device attached, the hwmon subsystem currently set the > > > > min and max trip point based on the single active trip point. > > > > Thermal subsystem parse all the trip points and calculate the lowest and > > > > the highest trip point and calls the .set_trip of hwmon to setup the > > > > trip points. > > > > > > > > The fact that we currently don't have a way to declare the cold/min > > > > temperature values, makes the thermal subsystem to set the low value as > > > > -INT_MAX. > > > > For hwmon drivers that doesn't use clamp_value and actually reject > > > > invalid values for the trip point, this results in the hwmon settings to > > > > be rejected. > > > > > > > > To permit to pass the correct range of trip point, permit to set in DT > > > > also cold and critical_cold trip point. > > > > > > > > Thermal driver may also define .cold and .critical_cold to act on these > > > > trip point tripped and apply the required measure. > > > > > > Agree with the feature but we need to clarify the semantic of the trip > > > points first. What actions do we expect for them in order to have like a > > > mirror reflection of the existing hot trip points. > > > > > > What action do you expect with: > > > > > > - 'cold' ? > > > > > > - 'critical_cold' ? > > > > > > > > > > This is more of a sensible topic but I think it's the thermal driver > > that needs to implement these. As said in the commit description, > > examples are setting higher voltage from the attached regulator, > > enabling some hardware heater. > > So how is it different from an active trip point? There are heating > rather than cooling devices associated with it, but other than this?? > From what I read from documentation, active trip point are used to trigger cooling-device. Cold (and crit_cold) are to setup trip point to the device. The device will normally trigger an interrupt (or even internally with the correct register set autonomously apply some measure to handle the problem) In theory it's possible to have passive trip point for cold condition but still we lack any definition of the lower spectrum of the trip point. > > Maybe with critical cold bigger measure can be applied. Currently for > > critical trip point we shutdown the system (if the critical ops is not > > declared) but with critical_cold condition I think it won't work... I > > expect a system in -40°C would just lock up/glitch so rebooting in that > > condition won't change a thing... > > > > Anyway yes we can define a shutdown by default for that but IMHO it > > wouldn't make much sense. > > So why do you want to add it at all? Again it's really to fill a hole we have from a long time... One example is the qcom tsens driver that have trip point for cold and crit_cold. Those in theory can be set in DT with the trip point but we lack any definition for them. (using passive trip point would be confusing IMHO) Another example is an Aquantia PHY that have register for the cold and critical_cold trip point. Currently defining a critical trip point for the negative temp results in the system shutdown.
On Wed, Dec 13, 2023 at 3:56 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > On Wed, Dec 13, 2023 at 03:43:54PM +0100, Rafael J. Wysocki wrote: > > On Wed, Dec 13, 2023 at 3:20 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > > > > On Wed, Dec 13, 2023 at 03:12:41PM +0100, Daniel Lezcano wrote: > > > > On 12/12/2023 23:13, Christian Marangi wrote: > > > > > Add initial support for cold and critical_cold trip point. Many if not > > > > > all hwmon and thermal device have normally trip point for hot > > > > > temperature and for cold temperature. > > > > > > > > > > Till now only hot temperature were supported. Add support for also cold > > > > > temperature to permit complete definition of cold trip point in DT. > > > > > > > > > > Thermal driver may use these additional trip point to correctly set > > > > > interrupt for cold temperature values and react based on that with > > > > > various measure like enabling attached heater, forcing higher voltage > > > > > and other specialaized peripherals. > > > > > > > > > > For hwmon drivers this is needed as currently there is a problem with > > > > > setting the full operating range of the device for thermal devices > > > > > defined with hwmon. To better describe the problem, the following > > > > > example is needed: > > > > > > > > > > In the scenario of a simple hwmon with an active trip point declared > > > > > and a cooling device attached, the hwmon subsystem currently set the > > > > > min and max trip point based on the single active trip point. > > > > > Thermal subsystem parse all the trip points and calculate the lowest and > > > > > the highest trip point and calls the .set_trip of hwmon to setup the > > > > > trip points. > > > > > > > > > > The fact that we currently don't have a way to declare the cold/min > > > > > temperature values, makes the thermal subsystem to set the low value as > > > > > -INT_MAX. > > > > > For hwmon drivers that doesn't use clamp_value and actually reject > > > > > invalid values for the trip point, this results in the hwmon settings to > > > > > be rejected. > > > > > > > > > > To permit to pass the correct range of trip point, permit to set in DT > > > > > also cold and critical_cold trip point. > > > > > > > > > > Thermal driver may also define .cold and .critical_cold to act on these > > > > > trip point tripped and apply the required measure. > > > > > > > > Agree with the feature but we need to clarify the semantic of the trip > > > > points first. What actions do we expect for them in order to have like a > > > > mirror reflection of the existing hot trip points. > > > > > > > > What action do you expect with: > > > > > > > > - 'cold' ? > > > > > > > > - 'critical_cold' ? > > > > > > > > > > > > > > This is more of a sensible topic but I think it's the thermal driver > > > that needs to implement these. As said in the commit description, > > > examples are setting higher voltage from the attached regulator, > > > enabling some hardware heater. > > > > So how is it different from an active trip point? There are heating > > rather than cooling devices associated with it, but other than this?? > > > > From what I read from documentation, active trip point are used to > trigger cooling-device. Cold (and crit_cold) are to setup trip point to > the device. The device will normally trigger an interrupt Well, that's how thermal sensors work in general IIUC. > (or even internally with the correct register set autonomously apply some measure > to handle the problem) > > In theory it's possible to have passive trip point for cold condition > but still we lack any definition of the lower spectrum of the trip > point. Such that it will increase power of some devices in order to warm the system up? Fair enough. > > > Maybe with critical cold bigger measure can be applied. Currently for > > > critical trip point we shutdown the system (if the critical ops is not > > > declared) but with critical_cold condition I think it won't work... I > > > expect a system in -40°C would just lock up/glitch so rebooting in that > > > condition won't change a thing... > > > > > > Anyway yes we can define a shutdown by default for that but IMHO it > > > wouldn't make much sense. > > > > So why do you want to add it at all? > > Again it's really to fill a hole we have from a long time... One example > is the qcom tsens driver that have trip point for cold and crit_cold. > Those in theory can be set in DT with the trip point but we lack any > definition for them. (using passive trip point would be confusing IMHO) > > Another example is an Aquantia PHY that have register for the cold and > critical_cold trip point. My point is about the crit_cold trips in particular. If there is no common action to trigger when they are crossed, what are they actually good for? > Currently defining a critical trip point for the negative temp results > in the system shutdown. Sure.
On 13/12/2023 15:20, Christian Marangi wrote: > On Wed, Dec 13, 2023 at 03:12:41PM +0100, Daniel Lezcano wrote: >> On 12/12/2023 23:13, Christian Marangi wrote: >>> Add initial support for cold and critical_cold trip point. Many if not >>> all hwmon and thermal device have normally trip point for hot >>> temperature and for cold temperature. >>> >>> Till now only hot temperature were supported. Add support for also cold >>> temperature to permit complete definition of cold trip point in DT. >>> >>> Thermal driver may use these additional trip point to correctly set >>> interrupt for cold temperature values and react based on that with >>> various measure like enabling attached heater, forcing higher voltage >>> and other specialaized peripherals. >>> >>> For hwmon drivers this is needed as currently there is a problem with >>> setting the full operating range of the device for thermal devices >>> defined with hwmon. To better describe the problem, the following >>> example is needed: >>> >>> In the scenario of a simple hwmon with an active trip point declared >>> and a cooling device attached, the hwmon subsystem currently set the >>> min and max trip point based on the single active trip point. >>> Thermal subsystem parse all the trip points and calculate the lowest and >>> the highest trip point and calls the .set_trip of hwmon to setup the >>> trip points. >>> >>> The fact that we currently don't have a way to declare the cold/min >>> temperature values, makes the thermal subsystem to set the low value as >>> -INT_MAX. >>> For hwmon drivers that doesn't use clamp_value and actually reject >>> invalid values for the trip point, this results in the hwmon settings to >>> be rejected. >>> >>> To permit to pass the correct range of trip point, permit to set in DT >>> also cold and critical_cold trip point. >>> >>> Thermal driver may also define .cold and .critical_cold to act on these >>> trip point tripped and apply the required measure. >> >> Agree with the feature but we need to clarify the semantic of the trip >> points first. What actions do we expect for them in order to have like a >> mirror reflection of the existing hot trip points. >> >> What action do you expect with: >> >> - 'cold' ? >> >> - 'critical_cold' ? >> >> > > This is more of a sensible topic but I think it's the thermal driver > that needs to implement these. As said in the commit description, > examples are setting higher voltage from the attached regulator, > enabling some hardware heater. That is a warming device. In the thermal framework design it is part of the mitigation device actors like a cooling device. The driver does not have to deal with that. > Maybe with critical cold bigger measure can be applied. Currently for > critical trip point we shutdown the system (if the critical ops is not > declared) but with critical_cold condition I think it won't work... I > expect a system in -40°C would just lock up/glitch so rebooting in that > condition won't change a thing... From my POV, a critical trip point is for a too hot or too cold trip point. The temperature should be set before the system will be malfunctioning, so a halt (or reboot if set) should work. I'm not in favor of adding more callbacks if we can avoid them. Passing the trip point to the critical callback makes more sense to me. > Anyway yes we can define a shutdown by default for that but IMHO it > wouldn't make much sense. Why? If the device is about to go to out of the functioning range, then it makes sense to shut it down. IIRC, electric signals lose their stability below the lower bound temperature. There is the point about the mitigation to stay above a certain temperature. If the devices do not have any kind a 'warming' device, then an alternative would be to have a generic warming device mirroring the cooling device with a duty cycles at different performance states. With this case, we may need another trip point type for a governor which should handle that. So we end up with the question: shall we add trip point types or trip point properties? 1. Trip point types - active / passive : mitigate - hot : notify userspace - critical : halt by default - cold : do something - cold_crit : do something else with a callback 2. Trip point types + property - active / passive : mitigate - hot : cool down - cold : warm up - hot : notify userspace - cold : notify userspace - critical: - hot : shutdown (or callback + trip) - cold : shutdown (or callback + trip) That implies there are two models: 1. We assume cold / hot trip points are symmetric features of the thermal management. So we do mitigation using governors, if that mitigation fails then we end up with critical actions. A consistent behavior between temperature increase or decrease. From my POV, I prefer this approach because it reflects nicely the functioning range temperature. 2. We handle the cold situation differently by doing a on/off action on a specific device. That is an asymmetric approach.
On Wed, Dec 13, 2023 at 4:10 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 13/12/2023 15:20, Christian Marangi wrote: > > On Wed, Dec 13, 2023 at 03:12:41PM +0100, Daniel Lezcano wrote: > >> On 12/12/2023 23:13, Christian Marangi wrote: > >>> Add initial support for cold and critical_cold trip point. Many if not > >>> all hwmon and thermal device have normally trip point for hot > >>> temperature and for cold temperature. > >>> > >>> Till now only hot temperature were supported. Add support for also cold > >>> temperature to permit complete definition of cold trip point in DT. > >>> > >>> Thermal driver may use these additional trip point to correctly set > >>> interrupt for cold temperature values and react based on that with > >>> various measure like enabling attached heater, forcing higher voltage > >>> and other specialaized peripherals. > >>> > >>> For hwmon drivers this is needed as currently there is a problem with > >>> setting the full operating range of the device for thermal devices > >>> defined with hwmon. To better describe the problem, the following > >>> example is needed: > >>> > >>> In the scenario of a simple hwmon with an active trip point declared > >>> and a cooling device attached, the hwmon subsystem currently set the > >>> min and max trip point based on the single active trip point. > >>> Thermal subsystem parse all the trip points and calculate the lowest and > >>> the highest trip point and calls the .set_trip of hwmon to setup the > >>> trip points. > >>> > >>> The fact that we currently don't have a way to declare the cold/min > >>> temperature values, makes the thermal subsystem to set the low value as > >>> -INT_MAX. > >>> For hwmon drivers that doesn't use clamp_value and actually reject > >>> invalid values for the trip point, this results in the hwmon settings to > >>> be rejected. > >>> > >>> To permit to pass the correct range of trip point, permit to set in DT > >>> also cold and critical_cold trip point. > >>> > >>> Thermal driver may also define .cold and .critical_cold to act on these > >>> trip point tripped and apply the required measure. > >> > >> Agree with the feature but we need to clarify the semantic of the trip > >> points first. What actions do we expect for them in order to have like a > >> mirror reflection of the existing hot trip points. > >> > >> What action do you expect with: > >> > >> - 'cold' ? > >> > >> - 'critical_cold' ? > >> > >> > > > > This is more of a sensible topic but I think it's the thermal driver > > that needs to implement these. As said in the commit description, > > examples are setting higher voltage from the attached regulator, > > enabling some hardware heater. > > That is a warming device. In the thermal framework design it is part of > the mitigation device actors like a cooling device. The driver does not > have to deal with that. > > > Maybe with critical cold bigger measure can be applied. Currently for > > critical trip point we shutdown the system (if the critical ops is not > > declared) but with critical_cold condition I think it won't work... I > > expect a system in -40°C would just lock up/glitch so rebooting in that > > condition won't change a thing... > > From my POV, a critical trip point is for a too hot or too cold trip > point. The temperature should be set before the system will be > malfunctioning, so a halt (or reboot if set) should work. > > I'm not in favor of adding more callbacks if we can avoid them. Passing > the trip point to the critical callback makes more sense to me. > > > Anyway yes we can define a shutdown by default for that but IMHO it > > wouldn't make much sense. > > Why? If the device is about to go to out of the functioning range, then > it makes sense to shut it down. IIRC, electric signals lose their > stability below the lower bound temperature. > > There is the point about the mitigation to stay above a certain > temperature. If the devices do not have any kind a 'warming' device, > then an alternative would be to have a generic warming device mirroring > the cooling device with a duty cycles at different performance states. > With this case, we may need another trip point type for a governor which > should handle that. > > So we end up with the question: shall we add trip point types or trip > point properties? > > 1. Trip point types > > - active / passive : mitigate > - hot : notify userspace > - critical : halt by default > - cold : do something > - cold_crit : do something else with a callback > > 2. Trip point types + property > > - active / passive : mitigate > - hot : cool down > - cold : warm up > > - hot : notify userspace > - cold : notify userspace > > - critical: > - hot : shutdown (or callback + trip) > - cold : shutdown (or callback + trip) > > That implies there are two models: > > 1. We assume cold / hot trip points are symmetric features of the > thermal management. So we do mitigation using governors, if that > mitigation fails then we end up with critical actions. A consistent > behavior between temperature increase or decrease. From my POV, I prefer > this approach because it reflects nicely the functioning range temperature. I agree here, FWIW. > 2. We handle the cold situation differently by doing a on/off action on > a specific device. That is an asymmetric approach.
On 13/12/2023 15:56, Christian Marangi wrote: > On Wed, Dec 13, 2023 at 03:43:54PM +0100, Rafael J. Wysocki wrote: >> On Wed, Dec 13, 2023 at 3:20 PM Christian Marangi <ansuelsmth@gmail.com> wrote: >>> >>> On Wed, Dec 13, 2023 at 03:12:41PM +0100, Daniel Lezcano wrote: >>>> On 12/12/2023 23:13, Christian Marangi wrote: >>>>> Add initial support for cold and critical_cold trip point. Many if not >>>>> all hwmon and thermal device have normally trip point for hot >>>>> temperature and for cold temperature. >>>>> >>>>> Till now only hot temperature were supported. Add support for also cold >>>>> temperature to permit complete definition of cold trip point in DT. >>>>> >>>>> Thermal driver may use these additional trip point to correctly set >>>>> interrupt for cold temperature values and react based on that with >>>>> various measure like enabling attached heater, forcing higher voltage >>>>> and other specialaized peripherals. >>>>> >>>>> For hwmon drivers this is needed as currently there is a problem with >>>>> setting the full operating range of the device for thermal devices >>>>> defined with hwmon. To better describe the problem, the following >>>>> example is needed: >>>>> >>>>> In the scenario of a simple hwmon with an active trip point declared >>>>> and a cooling device attached, the hwmon subsystem currently set the >>>>> min and max trip point based on the single active trip point. >>>>> Thermal subsystem parse all the trip points and calculate the lowest and >>>>> the highest trip point and calls the .set_trip of hwmon to setup the >>>>> trip points. >>>>> >>>>> The fact that we currently don't have a way to declare the cold/min >>>>> temperature values, makes the thermal subsystem to set the low value as >>>>> -INT_MAX. >>>>> For hwmon drivers that doesn't use clamp_value and actually reject >>>>> invalid values for the trip point, this results in the hwmon settings to >>>>> be rejected. >>>>> >>>>> To permit to pass the correct range of trip point, permit to set in DT >>>>> also cold and critical_cold trip point. >>>>> >>>>> Thermal driver may also define .cold and .critical_cold to act on these >>>>> trip point tripped and apply the required measure. >>>> >>>> Agree with the feature but we need to clarify the semantic of the trip >>>> points first. What actions do we expect for them in order to have like a >>>> mirror reflection of the existing hot trip points. >>>> >>>> What action do you expect with: >>>> >>>> - 'cold' ? >>>> >>>> - 'critical_cold' ? >>>> >>>> >>> >>> This is more of a sensible topic but I think it's the thermal driver >>> that needs to implement these. As said in the commit description, >>> examples are setting higher voltage from the attached regulator, >>> enabling some hardware heater. >> >> So how is it different from an active trip point? There are heating >> rather than cooling devices associated with it, but other than this?? >> > > From what I read from documentation, active trip point are used to > trigger cooling-device. Cold (and crit_cold) are to setup trip point to > the device. The device will normally trigger an interrupt (or even > internally with the correct register set autonomously apply some measure > to handle the problem) Actually what specifies an active cooling device is it requires energy in order to operate. More precisely, the goal of an active cooling device is too move the heat from one place to another place. So the system, instead of relying on the natural convection thermal transfer, will force this transfer. So the "active" means external system + energy. > In theory it's possible to have passive trip point for cold condition > but still we lack any definition of the lower spectrum of the trip > point. Yes, absolutely :) And that is why I think we should clarify that to conform to the general semantic of the thermal management. If we define things in the thermal framework but having a different meaning in the thermal management vocabulary. That will be really odd and look amateur work :) In the lower spectrum, an external warming device where we use energy to provide some heat is active. But if we use some kind of software solution (like what suggested before), we indeed use energy, but the solution is internal to the system, so I do believe we can consider it as passive. IMO, we should see, especially on mobile, passive trip point for too hot, and active trip point for too cold. That would not surprising as the former has too much energy generated and the latter not enough energy. (BTW, as a side note, active or passive trip points do not really make sense to me. It is the mitigation devices which are active or passive). For the systems which do not have a dedicated warming up hardware, we should implement a "warming device" as a passive one (which is a different story from your proposal I agree). >>> Maybe with critical cold bigger measure can be applied. Currently for >>> critical trip point we shutdown the system (if the critical ops is not >>> declared) but with critical_cold condition I think it won't work... I >>> expect a system in -40°C would just lock up/glitch so rebooting in that >>> condition won't change a thing... >>> >>> Anyway yes we can define a shutdown by default for that but IMHO it >>> wouldn't make much sense. >> >> So why do you want to add it at all? > > Again it's really to fill a hole we have from a long time... One example > is the qcom tsens driver that have trip point for cold and crit_cold. > Those in theory can be set in DT with the trip point but we lack any > definition for them. (using passive trip point would be confusing IMHO) > > Another example is an Aquantia PHY that have register for the cold and > critical_cold trip point. > > Currently defining a critical trip point for the negative temp results > in the system shutdown. Yes, and the more I think about it, the more I'm inclined to have: * trip (active|passive) + hot|cold * trip cold (meaning "really too cold") * trip hot (meaning "really too hot") * trip critical (meaning "I'm about to collapse") We may have also active devices with multiple level of warm up, so it will need to be managed by a governor like the step wise.
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 9c17d35ccbbd..3c5ab560e72f 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -344,6 +344,17 @@ static void handle_critical_trips(struct thermal_zone_device *tz, tz->ops->hot(tz); } +static void handle_critical_cold_trips(struct thermal_zone_device *tz, + const struct thermal_trip *trip) +{ + trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type); + + if (trip->type == THERMAL_TRIP_CRITICAL_COLD && tz->ops->critical_cold) + tz->ops->critical_cold(tz); + else if (trip->type == THERMAL_TRIP_COLD && tz->ops->cold) + tz->ops->cold(tz); +} + static void handle_thermal_trip(struct thermal_zone_device *tz, const struct thermal_trip *trip) { @@ -365,6 +376,8 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT) handle_critical_trips(tz, trip); + else if (trip->type == THERMAL_TRIP_CRITICAL_COLD || trip->type == THERMAL_TRIP_COLD) + handle_critical_cold_trips(tz, trip); else handle_non_critical_trips(tz, trip); } diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 1e0655b63259..95bc600bb4b8 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -60,6 +60,8 @@ static const char * const trip_types[] = { [THERMAL_TRIP_PASSIVE] = "passive", [THERMAL_TRIP_HOT] = "hot", [THERMAL_TRIP_CRITICAL] = "critical", + [THERMAL_TRIP_COLD] = "cold", + [THERMAL_TRIP_CRITICAL_COLD] = "critical_cold", }; /** diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index eef40d4f3063..e1e69e0991c2 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -106,6 +106,10 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr, return sprintf(buf, "critical\n"); case THERMAL_TRIP_HOT: return sprintf(buf, "hot\n"); + case THERMAL_TRIP_COLD: + return sprintf(buf, "cold\n"); + case THERMAL_TRIP_CRITICAL_COLD: + return sprintf(buf, "critical_cold\n"); case THERMAL_TRIP_PASSIVE: return sprintf(buf, "passive\n"); case THERMAL_TRIP_ACTIVE: diff --git a/drivers/thermal/thermal_trace.h b/drivers/thermal/thermal_trace.h index 459c8ce6cf3b..0a4f96075d7d 100644 --- a/drivers/thermal/thermal_trace.h +++ b/drivers/thermal/thermal_trace.h @@ -11,6 +11,8 @@ TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL); TRACE_DEFINE_ENUM(THERMAL_TRIP_HOT); +TRACE_DEFINE_ENUM(THERMAL_TRIP_COLD); +TRACE_DEFINE_ENUM(THERMAL_TRIP_CRITICAL_COLD); TRACE_DEFINE_ENUM(THERMAL_TRIP_PASSIVE); TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); @@ -18,6 +20,8 @@ TRACE_DEFINE_ENUM(THERMAL_TRIP_ACTIVE); __print_symbolic(type, \ { THERMAL_TRIP_CRITICAL, "CRITICAL"}, \ { THERMAL_TRIP_HOT, "HOT"}, \ + { THERMAL_TRIP_COLD, "COLD"}, \ + { THERMAL_TRIP_CRITICAL_COLD, "CRITICAL_COLD"}, \ { THERMAL_TRIP_PASSIVE, "PASSIVE"}, \ { THERMAL_TRIP_ACTIVE, "ACTIVE"}) diff --git a/include/linux/thermal.h b/include/linux/thermal.h index cee814d5d1ac..d6345c9ec50d 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -84,6 +84,8 @@ struct thermal_zone_device_ops { const struct thermal_trip *, enum thermal_trend *); void (*hot)(struct thermal_zone_device *); void (*critical)(struct thermal_zone_device *); + void (*cold)(struct thermal_zone_device *); + void (*critical_cold)(struct thermal_zone_device *); }; struct thermal_cooling_device_ops { diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h index fc78bf3aead7..7fa1ba0dff05 100644 --- a/include/uapi/linux/thermal.h +++ b/include/uapi/linux/thermal.h @@ -14,6 +14,8 @@ enum thermal_trip_type { THERMAL_TRIP_PASSIVE, THERMAL_TRIP_HOT, THERMAL_TRIP_CRITICAL, + THERMAL_TRIP_COLD, + THERMAL_TRIP_CRITICAL_COLD, }; /* Adding event notification support elements */