linux-kernel - Re: [PATCH v2 1/5] thermal: Add support for hardware-tracked trip points

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160525164420.GB11807@e104805>
Date:	Wed, 25 May 2016 17:44:21 +0100
From:	Javi Merino <javi.merino@....com>
To:	Caesar Wang <caesar.upstream@...il.com>
Cc:	Caesar Wang <wxt@...k-chips.com>, huangtao@...k-chips.com,
	Jonathan Corbet <corbet@....net>, Ni Wade <wni@...dia.com>,
	Durgadoss R <durgadoss.r@...el.com>,
	Heiko Stuebner <heiko@...ech.de>, linux-pm@...r.kernel.org,
	Sascha Hauer <s.hauer@...gutronix.de>,
	dmitry.torokhov@...il.com, linux-doc@...r.kernel.org,
	dianders@...omium.org, linux-kernel@...r.kernel.org,
	edubezval@...il.com, linux-rockchip@...ts.infradead.org,
	Laxman Dewangan <ldewangan@...dia.com>, smbarber@...gle.com,
	Leo Yan <leo.yan@...aro.org>, cf@...k-chips.com,
	briannorris@...gle.com, Zhang Rui <rui.zhang@...el.com>,
	Andy Champ <andycham@...zon.com>
Subject: Re: [PATCH v2 1/5] thermal: Add support for hardware-tracked trip
 points

Hi Caesar,

On Wed, May 25, 2016 at 11:27:24AM +0800, Caesar Wang wrote:
> On 2016年05月24日 20:57, Javi Merino wrote:
> >On Tue, May 03, 2016 at 05:33:29PM +0800, Caesar Wang wrote:
> >>From: Sascha Hauer <s.hauer@...gutronix.de>
> >>
> >>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 .set_trips
> >>callback is then called with the temperatures. If there is no trip
> >>point above or below the current temperature, the passed trip
> >>temperature will be -INT_MAX or INT_MAX 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 .set_trips is not implemented, the framework behaves as before.
> >>
> >>This patch is based on an earlier version from Mikko Perttunen
> >><mikko.perttunen@...si.fi>
> >>
> >>Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
> >>Signed-off-by: Caesar Wang <wxt@...k-chips.com>
> >>Cc: Zhang Rui <rui.zhang@...el.com>
> >>Cc: Eduardo Valentin <edubezval@...il.com>
> >>Cc: linux-pm@...r.kernel.org
> >>
> >>---
> >>
> >>Changes in v2:
> >>- update the sysfs-api.txt for set_trips
> >>
> >>  Documentation/thermal/sysfs-api.txt |  7 +++++
> >>  drivers/thermal/thermal_core.c      | 52 +++++++++++++++++++++++++++++++++++++
> >>  include/linux/thermal.h             |  3 +++
> >>  3 files changed, 62 insertions(+)
> <cut..>
> >>+	/*
> >>+	 * Set a temperature window. When this window is left the driver
> >>+	 * must inform the thermal core via thermal_zone_device_update.
> >>+	 */
> >>+	ret = tz->ops->set_trips(tz, low, high);
> >>+	if (ret)
> >>+		dev_err(&tz->device, "Failed to set trips: %d\n", ret);
> >This function can be called at the same time from multiple places so
> >it should be reentrant.  I think you should call mutex_lock(tz->lock)
> >before "if (tz->prev_low_trip == low && ..." and unlock it here.
> 
> Sound reasonable, fixes it in next version.
> 
> >>+}
> >>+
> >>  static void update_temperature(struct thermal_zone_device *tz)
> >>  {
> >>  	int temp, ret;
> >>@@ -569,6 +614,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
> >>  	update_temperature(tz);
> >>+	thermal_zone_set_trips(tz);
> >>+
> >>  	for (count = 0; count < tz->trips; count++)
> >>  		handle_thermal_trip(tz, count);
> >>  }
> >>@@ -754,6 +801,9 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr,
> >>  	 */
> >>  	ret = tz->ops->set_trip_hyst(tz, trip, temperature);
> >>+	if (!ret)
> >>+		thermal_zone_set_trips(tz);
> >>+
> >You should add a similar call to thermal_zone_set_trips() in trip_point_temp_store()
> 
> No, this patch has been done.
> 
> if you see the linux next kernel.
> 72f3ada UPSTREAM: thermal: trip_point_temp_store() calls
> thermal_zone_device_update()
> 
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -733,8 +733,12 @@ trip_point_temp_store(struct device *dev,
> struct device_attribute *attr,
> return -EINVAL;
> 
> ret = tz->ops->set_trip_temp(tz, trip, temperature);
> + if (ret)
> + return ret;
> 
> - return ret ? ret : count;
> + thermal_zone_device_update(tz);
> +
> + return count;
> }
> 
> So the "thermal_zone_set_trips(tz)" have been set in
> thermal_zone_device_update.

Ah, right, I missed that.  Thanks!
Javi