| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <65799e2f.5d0a0220.1213a.f035@mx.google.com>
Date: Wed, 13 Dec 2023 13:06:05 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Daniel Lezcano <daniel.lezcano@...aro.org>,
Zhang Rui <rui.zhang@...el.com>,
Lukasz Luba <lukasz.luba@....com>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] thermal: core: add initial support for cold and
critical_cold trip point
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@...il.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@...il.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
> >
--
Ansuel
Powered by blists - more mailing lists