From: swarren@wwwdotorg•org (Stephen Warren)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 1/6] thermal: of: Add support for hardware-tracked trip points
Date: Mon, 30 Jun 2014 15:08:24 -0600 [thread overview]
Message-ID: <53B1D1C8.5010907@wwwdotorg.org> (raw)
In-Reply-To: <1403856699-2140-2-git-send-email-mperttunen@nvidia.com>
On 06/27/2014 02:11 AM, Mikko Perttunen wrote:
> This adds support for hardware-tracked trip points to the device tree
> thermal sensor framework.
>
> The framework supports an arbitrary number of trip points. Whenever
> the current temperature is updated, the trip points immediately
> below and above the current temperature are found. A sensor driver
> callback `set_trips' is then called with the temperatures.
> If there is no trip point above or below the current temperature,
> the passed trip temperature will be LONG_MAX or LONG_MIN respectively.
> In this callback, the driver should program the hardware such that
> it is notified when either of these trip points are triggered.
> When a trip point is triggered, the driver should call
> `thermal_zone_device_update' for the respective thermal zone. This
> will cause the trip points to be updated again.
>
> If the `set_trips' callback is not implemented (is NULL), the framework
> behaves as before.
Is there no "core thermal" code? I would have expected this new feature
to be implemented in "core" code rather than of/dt "support" code.
Perhaps there would also be some additions to the of/dt code, but I'd
still expect the bulk of the feature to be complete independant of
of/dt. Systems still using board files or ACPI or ... would surely
benefit from this too?
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> +static int of_thermal_set_trips(struct thermal_zone_device *tz, long temp)
s/tz/tzd/ or s/tz/tzdev/ ? Since "tz" says "thermal zone" to me, but
it's actually a "thermal zone device".
> + struct __thermal_zone *data = tz->devdata;
s/data/tz/ ? "data" is a rather generic term, and "tz" seems like a good
abbreviation for a __thermal_zone.
> + for (i = 0; i < data->ntrips; ++i) {
> + struct __thermal_trip *trip = data->trips + i;
> + long trip_low = trip->temperature - trip->hysteresis;
> +
> + if (trip_low < temp && trip_low > low)
> + low = trip_low;
> +
> + if (trip->temperature > temp && trip->temperature < high)
> + high = trip->temperature;
> + }
That seems to always apply hysteresis to the low end of a trip object.
Don't you need to apply the hysteresis to either the low or high end of
the range, depending on whether the temperature is currently below/above
the range, and hence which direction the edge will be crossed?
Similar comments elsewhere. Perhaps the patch is consistent with some
existing confusing naming style though?
> +static int of_thermal_update_trips(struct thermal_zone_device *tz)
> +{
> + long temp;
> + int err;
> +
> + err = of_thermal_get_temp(tz, &temp);
> + if (err)
> + return err;
> +
> + err = of_thermal_set_trips(tz, temp);
Doesn't this patch modify of_thermal_get_temp() to call
of_thermal_set_trips() itself?
> @@ -384,7 +467,8 @@ thermal_zone_of_add_sensor(struct device_node *zone,
> struct thermal_zone_device *
> thermal_zone_of_sensor_register(struct device *dev, int sensor_id,
> void *data, int (*get_temp)(void *, long *),
> - int (*get_trend)(void *, long *))
> + int (*get_trend)(void *, long *),
> + int (*set_trips)(void *, long, long))
Passing in a struct containing fields for all the ops seem better than
forever extending this function with more and more function pointers.
next prev parent reply other threads:[~2014-06-30 21:08 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-27 8:11 [PATCH 0/6] of-thermal hardware trip points + Tegra124 SOCTHERM driver Mikko Perttunen
2014-06-27 8:11 ` [PATCH 1/6] thermal: of: Add support for hardware-tracked trip points Mikko Perttunen
2014-06-30 21:08 ` Stephen Warren [this message]
2014-07-01 7:27 ` Mikko Perttunen
2014-07-01 18:15 ` Stephen Warren
2014-07-03 14:15 ` Mikko Perttunen
2014-07-30 14:16 ` Eduardo Valentin
2014-08-01 11:42 ` Mikko Perttunen
2014-08-01 13:15 ` edubezval at gmail.com
2014-06-27 8:11 ` [PATCH 2/6] of: Add bindings for nvidia,tegra124-soctherm Mikko Perttunen
2014-06-30 20:40 ` Stephen Warren
2014-06-27 8:11 ` [PATCH 3/6] ARM: tegra: Add thermal trip points for Jetson TK1 Mikko Perttunen
2014-06-30 20:45 ` Stephen Warren
2014-06-27 8:11 ` [PATCH 4/6] ARM: tegra: Add soctherm and thermal zones to Tegra124 device tree Mikko Perttunen
2014-06-30 20:48 ` Stephen Warren
2014-07-01 7:49 ` Mikko Perttunen
2014-07-21 23:13 ` Matthew Longnecker
2014-06-27 8:11 ` [PATCH 5/6] clk: tegra: Add soctherm and tsensor clocks to Tegra124 init table Mikko Perttunen
2014-06-27 12:18 ` Peter De Schrijver
2014-06-27 8:11 ` [PATCH 6/6] thermal: Add Tegra SOCTHERM thermal management driver Mikko Perttunen
2014-06-30 21:23 ` Stephen Warren
2014-07-01 8:06 ` Mikko Perttunen
2014-07-01 18:26 ` Stephen Warren
2014-07-03 13:51 ` Mikko Perttunen
2014-07-01 23:47 ` Tuomas Tynkkynen
2014-07-04 8:43 ` Wei Ni
2014-07-04 11:52 ` Mikko Perttunen
2014-07-21 7:42 ` [PATCH 0/6] of-thermal hardware trip points + Tegra124 SOCTHERM driver Zhang Rui
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53B1D1C8.5010907@wwwdotorg.org \
--to=swarren@wwwdotorg$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox