Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1255632 > unrolled thread

[PATCH 0/3][v2] Fix thermal problems during suspend/bootup

Started byChen Yu <yu.c.chen@intel.com>
First post2015-10-26 04:20 +0100
Last post2015-10-28 11:40 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/3][v2] Fix thermal problems during suspend/bootup  Chen Yu <yu.c.chen@intel.com> - 2015-10-26 04:20 +0100
    [PATCH 3/3][v2] Thermal: do thermal zone update after a cooling device registered Chen Yu <yu.c.chen@intel.com> - 2015-10-26 04:20 +0100
      Re: [PATCH 3/3][v2] Thermal: do thermal zone update after a cooling  device registered Javi Merino <javi.merino@arm.com> - 2015-10-28 11:50 +0100
        RE: [PATCH 3/3][v2] Thermal: do thermal zone update after a cooling  device registered "Chen, Yu C" <yu.c.chen@intel.com> - 2015-10-30 09:30 +0100
    [PATCH 1/3][v2] Thermal: initialize thermal zone device correctly Chen Yu <yu.c.chen@intel.com> - 2015-10-26 04:20 +0100
    [PATCH 2/3][v2] Thermal: handle thermal zone device properly during system sleep Chen Yu <yu.c.chen@intel.com> - 2015-10-26 04:20 +0100
      Re: [PATCH 2/3][v2] Thermal: handle thermal zone device properly  during system sleep Javi Merino <javi.merino@arm.com> - 2015-10-28 11:40 +0100

#1255632 — [PATCH 0/3][v2] Fix thermal problems during suspend/bootup

FromChen Yu <yu.c.chen@intel.com>
Date2015-10-26 04:20 +0100
Subject[PATCH 0/3][v2] Fix thermal problems during suspend/bootup
Message-ID<qnGue-31H-1@gated-at.bofh.it>
This patch set fixes two problems when system is trying to
suspend and boot up:
1.After system is woken up from suspend, the thermal framework uses
  the dirty 'cached' thermal variables before suspend, which might
  cause expected behavior. 
2.If a cooling device is registered after the thermal zone's registration,
  current thermal framework forgets to update the thermal_zone's status,
  which might bring expected behavior under special cases.

Chen Yu (3):
  Thermal: initialize thermal zone device correctly
  Thermal: handle thermal zone device properly during system sleep
  Thermal: do thermal zone update after a cooling device registered

 drivers/thermal/step_wise.c    | 17 +++++++--
 drivers/thermal/thermal_core.c | 78 +++++++++++++++++++++++++++++++++++++++---
 drivers/thermal/thermal_core.h |  1 +
 include/linux/thermal.h        |  4 +++
 4 files changed, 94 insertions(+), 6 deletions(-)

-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1255633 — [PATCH 3/3][v2] Thermal: do thermal zone update after a cooling device registered

FromChen Yu <yu.c.chen@intel.com>
Date2015-10-26 04:20 +0100
Subject[PATCH 3/3][v2] Thermal: do thermal zone update after a cooling device registered
Message-ID<qnGue-31H-3@gated-at.bofh.it>
In reply to#1255632
When a new cooling device is registered, we need to update the
thermal zone to set the new registered cooling device to a proper
state.

This fixes a problem that the system is cool, while the fan devices
are left running on full speed after boot, if fan device is registered
after thermal zone device.

Here is the history of why current patch looks like this:
https://patchwork.kernel.org/patch/7273041/

CC: <stable@vger.kernel.org> #3.18+
Reference:https://bugzilla.kernel.org/show_bug.cgi?id=92431
Tested-by: Manuel Krause <manuelkrause@netscape.net>
Tested-by: szegad <szegadlo@poczta.onet.pl>
Tested-by: prash <prash.n.rao@gmail.com>
Tested-by: amish <ammdispose-arch@yahoo.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 drivers/thermal/thermal_core.c | 14 +++++++++++++-
 include/linux/thermal.h        |  1 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index abeb995..f36d0bd 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1341,6 +1341,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	if (!result) {
 		list_add_tail(&dev->tz_node, &tz->thermal_instances);
 		list_add_tail(&dev->cdev_node, &cdev->thermal_instances);
+		atomic_set(&tz->need_update, 1);
 	}
 	mutex_unlock(&cdev->lock);
 	mutex_unlock(&tz->lock);
@@ -1450,6 +1451,7 @@ __thermal_cooling_device_register(struct device_node *np,
 				  const struct thermal_cooling_device_ops *ops)
 {
 	struct thermal_cooling_device *cdev;
+	struct thermal_zone_device *pos = NULL;
 	int result;
 
 	if (type && strlen(type) >= THERMAL_NAME_LENGTH)
@@ -1494,6 +1496,12 @@ __thermal_cooling_device_register(struct device_node *np,
 	/* Update binding information for 'this' new cdev */
 	bind_cdev(cdev);
 
+	mutex_lock(&thermal_list_lock);
+	list_for_each_entry(pos, &thermal_tz_list, node)
+		if (atomic_cmpxchg(&pos->need_update, 1, 0))
+			thermal_zone_device_update(pos);
+	mutex_unlock(&thermal_list_lock);
+
 	return cdev;
 }
 
@@ -1826,6 +1834,8 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 	tz->trips = trips;
 	tz->passive_delay = passive_delay;
 	tz->polling_delay = polling_delay;
+	/* A new thermal zone needs to be updated anyway. */
+	atomic_set(&tz->need_update, 1);
 
 	dev_set_name(&tz->device, "thermal_zone%d", tz->id);
 	result = device_register(&tz->device);
@@ -1921,7 +1931,9 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 	INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);
 
 	thermal_zone_device_reset(tz);
-	thermal_zone_device_update(tz);
+	/* Update the new thermal zone and mark it as already updated. */
+	if (atomic_cmpxchg(&tz->need_update, 1, 0))
+		thermal_zone_device_update(tz);
 
 	return tz;
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5bcabc7..4298418 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -195,6 +195,7 @@ struct thermal_zone_device {
 	int emul_temperature;
 	int passive;
 	unsigned int forced_passive;
+	atomic_t need_update;
 	struct thermal_zone_device_ops *ops;
 	struct thermal_zone_params *tzp;
 	struct thermal_governor *governor;
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1257896 — Re: [PATCH 3/3][v2] Thermal: do thermal zone update after a cooling device registered

FromJavi Merino <javi.merino@arm.com>
Date2015-10-28 11:50 +0100
SubjectRe: [PATCH 3/3][v2] Thermal: do thermal zone update after a cooling device registered
Message-ID<qowsO-1tK-15@gated-at.bofh.it>
In reply to#1255633
Hi Yu,

One minor nit below.  Other than that you can add my

Reviewed-by: Javi Merino <javi.merino@arm.com>

On Mon, Oct 26, 2015 at 11:15:30AM +0800, Chen Yu wrote:
> When a new cooling device is registered, we need to update the
> thermal zone to set the new registered cooling device to a proper
> state.
> 
> This fixes a problem that the system is cool, while the fan devices
> are left running on full speed after boot, if fan device is registered
> after thermal zone device.
> 
> Here is the history of why current patch looks like this:
> https://patchwork.kernel.org/patch/7273041/
> 
> CC: <stable@vger.kernel.org> #3.18+
> Reference:https://bugzilla.kernel.org/show_bug.cgi?id=92431
> Tested-by: Manuel Krause <manuelkrause@netscape.net>
> Tested-by: szegad <szegadlo@poczta.onet.pl>
> Tested-by: prash <prash.n.rao@gmail.com>
> Tested-by: amish <ammdispose-arch@yahoo.com>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  drivers/thermal/thermal_core.c | 14 +++++++++++++-
>  include/linux/thermal.h        |  1 +
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index abeb995..f36d0bd 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1341,6 +1341,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>  	if (!result) {
>  		list_add_tail(&dev->tz_node, &tz->thermal_instances);
>  		list_add_tail(&dev->cdev_node, &cdev->thermal_instances);
> +		atomic_set(&tz->need_update, 1);
>  	}
>  	mutex_unlock(&cdev->lock);
>  	mutex_unlock(&tz->lock);
> @@ -1450,6 +1451,7 @@ __thermal_cooling_device_register(struct device_node *np,
>  				  const struct thermal_cooling_device_ops *ops)
>  {
>  	struct thermal_cooling_device *cdev;
> +	struct thermal_zone_device *pos = NULL;
>  	int result;
>  
>  	if (type && strlen(type) >= THERMAL_NAME_LENGTH)
> @@ -1494,6 +1496,12 @@ __thermal_cooling_device_register(struct device_node *np,
>  	/* Update binding information for 'this' new cdev */
>  	bind_cdev(cdev);
>  
> +	mutex_lock(&thermal_list_lock);
> +	list_for_each_entry(pos, &thermal_tz_list, node)
> +		if (atomic_cmpxchg(&pos->need_update, 1, 0))
> +			thermal_zone_device_update(pos);
> +	mutex_unlock(&thermal_list_lock);
> +
>  	return cdev;
>  }
>  
> @@ -1826,6 +1834,8 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>  	tz->trips = trips;
>  	tz->passive_delay = passive_delay;
>  	tz->polling_delay = polling_delay;
> +	/* A new thermal zone needs to be updated anyway. */
> +	atomic_set(&tz->need_update, 1);
>  
>  	dev_set_name(&tz->device, "thermal_zone%d", tz->id);
>  	result = device_register(&tz->device);
> @@ -1921,7 +1931,9 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>  	INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);
>  
>  	thermal_zone_device_reset(tz);
> -	thermal_zone_device_update(tz);
> +	/* Update the new thermal zone and mark it as already updated. */
> +	if (atomic_cmpxchg(&tz->need_update, 1, 0))
> +		thermal_zone_device_update(tz);
>  
>  	return tz;
>  
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 5bcabc7..4298418 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -195,6 +195,7 @@ struct thermal_zone_device {
>  	int emul_temperature;
>  	int passive;
>  	unsigned int forced_passive;
> +	atomic_t need_update;

Please document this field in the kernel-doc comment for struct
thermal_zone_device.

Cheers,
Javi

>  	struct thermal_zone_device_ops *ops;
>  	struct thermal_zone_params *tzp;
>  	struct thermal_governor *governor;
> -- 
> 1.8.4.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1259296 — RE: [PATCH 3/3][v2] Thermal: do thermal zone update after a cooling device registered

From"Chen, Yu C" <yu.c.chen@intel.com>
Date2015-10-30 09:30 +0100
SubjectRE: [PATCH 3/3][v2] Thermal: do thermal zone update after a cooling device registered
Message-ID<qpdep-3t7-1@gated-at.bofh.it>
In reply to#1257896
SGksIEphdmksDQp0aGFua3MgZm9yIHlvdXIgcmV2aWV3LA0Kd2lsbCBzZW5kIGEgdmVyc2lvbiAz
IG91dCBzb29uLg0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEphdmkg
TWVyaW5vIFttYWlsdG86amF2aS5tZXJpbm9AYXJtLmNvbV0NCj4gU2VudDogV2VkbmVzZGF5LCBP
Y3RvYmVyIDI4LCAyMDE1IDY6NDAgUE0NCj4gVG86IENoZW4sIFl1IEMNCj4gQ2M6IFpoYW5nLCBS
dWk7IGVkdWJlenZhbEBnbWFpbC5jb207IGxpbnV4LXBtQHZnZXIua2VybmVsLm9yZzsgbGludXgt
DQo+IGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcNCj4gU3Vi
amVjdDogUmU6IFtQQVRDSCAzLzNdW3YyXSBUaGVybWFsOiBkbyB0aGVybWFsIHpvbmUgdXBkYXRl
IGFmdGVyIGEgY29vbGluZw0KPiBkZXZpY2UgcmVnaXN0ZXJlZA0KPiANCj4gSGkgWXUsDQo+IA0K
PiBPbmUgbWlub3Igbml0IGJlbG93LiAgT3RoZXIgdGhhbiB0aGF0IHlvdSBjYW4gYWRkIG15DQo+
IA0KPiBSZXZpZXdlZC1ieTogSmF2aSBNZXJpbm8gPGphdmkubWVyaW5vQGFybS5jb20+DQo+IA0K
PiBPbiBNb24sIE9jdCAyNiwgMjAxNSBhdCAxMToxNTozMEFNICswODAwLCBDaGVuIFl1IHdyb3Rl
Og0KPiA+IFdoZW4gYSBuZXcgY29vbGluZyBkZXZpY2UgaXMgcmVnaXN0ZXJlZCwgd2UgbmVlZCB0
byB1cGRhdGUgdGhlIHRoZXJtYWwNCj4gPiB6b25lIHRvIHNldCB0aGUgbmV3IHJlZ2lzdGVyZWQg
Y29vbGluZyBkZXZpY2UgdG8gYSBwcm9wZXIgc3RhdGUuDQo+ID4NCj4gPiBUaGlzIGZpeGVzIGEg
cHJvYmxlbSB0aGF0IHRoZSBzeXN0ZW0gaXMgY29vbCwgd2hpbGUgdGhlIGZhbiBkZXZpY2VzDQo+
ID4gYXJlIGxlZnQgcnVubmluZyBvbiBmdWxsIHNwZWVkIGFmdGVyIGJvb3QsIGlmIGZhbiBkZXZp
Y2UgaXMgcmVnaXN0ZXJlZA0KPiA+IGFmdGVyIHRoZXJtYWwgem9uZSBkZXZpY2UuDQo+ID4NCj4g
PiBIZXJlIGlzIHRoZSBoaXN0b3J5IG9mIHdoeSBjdXJyZW50IHBhdGNoIGxvb2tzIGxpa2UgdGhp
czoNCj4gPiBodHRwczovL3BhdGNod29yay5rZXJuZWwub3JnL3BhdGNoLzcyNzMwNDEvDQo+ID4N
Cj4gPiBDQzogPHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmc+ICMzLjE4Kw0KPiA+IFJlZmVyZW5jZTpo
dHRwczovL2J1Z3ppbGxhLmtlcm5lbC5vcmcvc2hvd19idWcuY2dpP2lkPTkyNDMxDQo+ID4gVGVz
dGVkLWJ5OiBNYW51ZWwgS3JhdXNlIDxtYW51ZWxrcmF1c2VAbmV0c2NhcGUubmV0Pg0KPiA+IFRl
c3RlZC1ieTogc3plZ2FkIDxzemVnYWRsb0Bwb2N6dGEub25ldC5wbD4NCj4gPiBUZXN0ZWQtYnk6
IHByYXNoIDxwcmFzaC5uLnJhb0BnbWFpbC5jb20+DQo+ID4gVGVzdGVkLWJ5OiBhbWlzaCA8YW1t
ZGlzcG9zZS1hcmNoQHlhaG9vLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBaaGFuZyBSdWkgPHJ1
aS56aGFuZ0BpbnRlbC5jb20+DQo+ID4gU2lnbmVkLW9mZi1ieTogQ2hlbiBZdSA8eXUuYy5jaGVu
QGludGVsLmNvbT4NCj4gPiAtLS0NCj4gPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9saW51eC90aGVy
bWFsLmggYi9pbmNsdWRlL2xpbnV4L3RoZXJtYWwuaCBpbmRleA0KPiA+IDViY2FiYzcuLjQyOTg0
MTggMTAwNjQ0DQo+ID4gLS0tIGEvaW5jbHVkZS9saW51eC90aGVybWFsLmgNCj4gPiArKysgYi9p
bmNsdWRlL2xpbnV4L3RoZXJtYWwuaA0KPiA+IEBAIC0xOTUsNiArMTk1LDcgQEAgc3RydWN0IHRo
ZXJtYWxfem9uZV9kZXZpY2Ugew0KPiA+ICAJaW50IGVtdWxfdGVtcGVyYXR1cmU7DQo+ID4gIAlp
bnQgcGFzc2l2ZTsNCj4gPiAgCXVuc2lnbmVkIGludCBmb3JjZWRfcGFzc2l2ZTsNCj4gPiArCWF0
b21pY190IG5lZWRfdXBkYXRlOw0KPiANCj4gUGxlYXNlIGRvY3VtZW50IHRoaXMgZmllbGQgaW4g
dGhlIGtlcm5lbC1kb2MgY29tbWVudCBmb3Igc3RydWN0DQo+IHRoZXJtYWxfem9uZV9kZXZpY2Uu
DQo+IA0KDQpCZXN0IFJlZ2FyZHMsDQpZdQ0KDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1255634 — [PATCH 1/3][v2] Thermal: initialize thermal zone device correctly

FromChen Yu <yu.c.chen@intel.com>
Date2015-10-26 04:20 +0100
Subject[PATCH 1/3][v2] Thermal: initialize thermal zone device correctly
Message-ID<qnGue-31H-5@gated-at.bofh.it>
In reply to#1255632
From: Zhang Rui <rui.zhang@intel.com>

After thermal zone device registered, as we have not read any
temperature before, thus tz->temperature should not be 0,
which actually means 0C, and thermal trend is not available.
In this case, we need specially handling for the first
thermal_zone_device_update().

Both thermal core framework and step_wise governor is
enhanced to handle this. And since the step_wise governor
is the only one that uses trends, so it's the only thermal
governor that needs to be updated.

CC: <stable@vger.kernel.org> #3.18+
Tested-by: Manuel Krause <manuelkrause@netscape.net>
Tested-by: szegad <szegadlo@poczta.onet.pl>
Tested-by: prash <prash.n.rao@gmail.com>
Tested-by: amish <ammdispose-arch@yahoo.com>
Tested-by: Matthias <morpheusxyz123@yahoo.de>
Reviewed-by: Javi Merino <javi.merino@arm.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 drivers/thermal/step_wise.c    | 17 +++++++++++++++--
 drivers/thermal/thermal_core.c | 19 +++++++++++++++++--
 drivers/thermal/thermal_core.h |  1 +
 include/linux/thermal.h        |  3 +++
 4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index 2f9f708..ea9366a 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -63,6 +63,19 @@ static unsigned long get_target_state(struct thermal_instance *instance,
 	next_target = instance->target;
 	dev_dbg(&cdev->device, "cur_state=%ld\n", cur_state);
 
+	if (!instance->initialized) {
+		if (throttle) {
+			next_target = (cur_state + 1) >= instance->upper ?
+					instance->upper :
+					((cur_state + 1) < instance->lower ?
+					instance->lower : (cur_state + 1));
+		} else {
+			next_target = THERMAL_NO_TARGET;
+		}
+
+		return next_target;
+	}
+
 	switch (trend) {
 	case THERMAL_TREND_RAISING:
 		if (throttle) {
@@ -149,7 +162,7 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 		dev_dbg(&instance->cdev->device, "old_target=%d, target=%d\n",
 					old_target, (int)instance->target);
 
-		if (old_target == instance->target)
+		if (instance->initialized && old_target == instance->target)
 			continue;
 
 		/* Activate a passive thermal instance */
@@ -161,7 +174,7 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 			instance->target == THERMAL_NO_TARGET)
 			update_passive_instance(tz, trip_type, -1);
 
-
+		instance->initialized = true;
 		instance->cdev->updated = false; /* cdev needs update */
 	}
 
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index d9e525c..682bc1e 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -532,8 +532,22 @@ static void update_temperature(struct thermal_zone_device *tz)
 	mutex_unlock(&tz->lock);
 
 	trace_thermal_temperature(tz);
-	dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
-				tz->last_temperature, tz->temperature);
+	if (tz->last_temperature == THERMAL_TEMP_INVALID)
+		dev_dbg(&tz->device, "last_temperature N/A, current_temperature=%d\n",
+			tz->temperature);
+	else
+		dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
+			tz->last_temperature, tz->temperature);
+}
+
+static void thermal_zone_device_reset(struct thermal_zone_device *tz)
+{
+	struct thermal_instance *pos;
+
+	tz->temperature = THERMAL_TEMP_INVALID;
+	tz->passive = 0;
+	list_for_each_entry(pos, &tz->thermal_instances, tz_node)
+		pos->initialized = false;
 }
 
 void thermal_zone_device_update(struct thermal_zone_device *tz)
@@ -1900,6 +1914,7 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 
 	INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);
 
+	thermal_zone_device_reset(tz);
 	thermal_zone_device_update(tz);
 
 	return tz;
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index d7ac1fc..749d41a 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -41,6 +41,7 @@ struct thermal_instance {
 	struct thermal_zone_device *tz;
 	struct thermal_cooling_device *cdev;
 	int trip;
+	bool initialized;
 	unsigned long upper;	/* Highest cooling state for this trip point */
 	unsigned long lower;	/* Lowest cooling state for this trip point */
 	unsigned long target;	/* expected cooling state */
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 157d366..5bcabc7 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -43,6 +43,9 @@
 /* Default weight of a bound cooling device */
 #define THERMAL_WEIGHT_DEFAULT 0
 
+/* use value, which < 0K, to indicate an invalid/uninitialized temperature */
+#define THERMAL_TEMP_INVALID	-274000
+
 /* Unit conversion macros */
 #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
 				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1255636 — [PATCH 2/3][v2] Thermal: handle thermal zone device properly during system sleep

FromChen Yu <yu.c.chen@intel.com>
Date2015-10-26 04:20 +0100
Subject[PATCH 2/3][v2] Thermal: handle thermal zone device properly during system sleep
Message-ID<qnGue-31H-13@gated-at.bofh.it>
In reply to#1255632
From: Zhang Rui <rui.zhang@intel.com>

Current thermal code does not handle system sleep well because
1. the cooling device cooling state may be changed during suspend
2. the previous temperature reading becomes invalid after resumed because
   it is got before system sleep
3. updating thermal zone device during suspending/resuming
   is wrong because some devices may have already been suspended
   or may have not been resumed.

Thus, the proper way to do this is to cancel all thermal zone
device update requirements during suspend/resume, and after all
the devices have been resumed, reset and update every registered
thermal zone devices.

This also fixes a regression introduced by:
Commit 19593a1fb1f6 ("ACPI / fan: convert to platform driver")
Because, with above commit applied, all the fan devices are attached
to the acpi_general_pm_domain, and they are turned on by the pm_domain
automatically after resume, without the awareness of thermal core.

CC: <stable@vger.kernel.org> #3.18+
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=78201
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=91411
Tested-by: Manuel Krause <manuelkrause@netscape.net>
Tested-by: szegad <szegadlo@poczta.onet.pl>
Tested-by: prash <prash.n.rao@gmail.com>
Tested-by: amish <ammdispose-arch@yahoo.com>
Tested-by: Matthias <morpheusxyz123@yahoo.de>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 drivers/thermal/thermal_core.c | 45 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 682bc1e..abeb995 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -37,6 +37,7 @@
 #include <linux/of.h>
 #include <net/netlink.h>
 #include <net/genetlink.h>
+#include <linux/suspend.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/thermal.h>
@@ -59,6 +60,8 @@ static LIST_HEAD(thermal_governor_list);
 static DEFINE_MUTEX(thermal_list_lock);
 static DEFINE_MUTEX(thermal_governor_lock);
 
+static atomic_t in_suspend;
+
 static struct thermal_governor *def_governor;
 
 static struct thermal_governor *__find_governor(const char *name)
@@ -554,6 +557,9 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 {
 	int count;
 
+	if (atomic_read(&in_suspend))
+		return;
+
 	if (!tz->ops->get_temp)
 		return;
 
@@ -2155,9 +2161,39 @@ static void thermal_unregister_governors(void)
 	thermal_gov_power_allocator_unregister();
 }
 
+static int thermal_pm_notify(struct notifier_block *nb,
+				unsigned long mode, void *_unused)
+{
+	struct thermal_zone_device *tz;
+
+	switch (mode) {
+	case PM_HIBERNATION_PREPARE:
+	case PM_RESTORE_PREPARE:
+	case PM_SUSPEND_PREPARE:
+		atomic_set(&in_suspend, 1);
+		break;
+	case PM_POST_HIBERNATION:
+	case PM_POST_RESTORE:
+	case PM_POST_SUSPEND:
+		atomic_set(&in_suspend, 0);
+		list_for_each_entry(tz, &thermal_tz_list, node) {
+			thermal_zone_device_reset(tz);
+			thermal_zone_device_update(tz);
+		}
+		break;
+	default:
+		break;
+	}
+	return 0;
+}
+
+static struct notifier_block thermal_pm_nb = {
+	.notifier_call = thermal_pm_notify,
+};
+
 static int __init thermal_init(void)
 {
-	int result;
+	int result, notifier_result;
 
 	result = thermal_register_governors();
 	if (result)
@@ -2175,6 +2211,12 @@ static int __init thermal_init(void)
 	if (result)
 		goto exit_netlink;
 
+	notifier_result = register_pm_notifier(&thermal_pm_nb);
+	if (notifier_result)
+		pr_err("Thermal: Can not register suspend notifier"
+			"for thermal framework, return %d\n",
+			notifier_result);
+
 	return 0;
 
 exit_netlink:
@@ -2194,6 +2236,7 @@ error:
 
 static void __exit thermal_exit(void)
 {
+	unregister_pm_notifier(&thermal_pm_nb);
 	of_thermal_destroy_zones();
 	genetlink_exit();
 	class_unregister(&thermal_class);
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1257891 — Re: [PATCH 2/3][v2] Thermal: handle thermal zone device properly during system sleep

FromJavi Merino <javi.merino@arm.com>
Date2015-10-28 11:40 +0100
SubjectRe: [PATCH 2/3][v2] Thermal: handle thermal zone device properly during system sleep
Message-ID<qowj8-1pQ-15@gated-at.bofh.it>
In reply to#1255636
Hi Yu,

some minor nits below.  Other than that, you can add my

Reviewed-by: Javi Merino <javi.merino@arm.com>

On Mon, Oct 26, 2015 at 11:14:31AM +0800, Chen Yu wrote:
> From: Zhang Rui <rui.zhang@intel.com>
> 
> Current thermal code does not handle system sleep well because
> 1. the cooling device cooling state may be changed during suspend
> 2. the previous temperature reading becomes invalid after resumed because
>    it is got before system sleep
> 3. updating thermal zone device during suspending/resuming
>    is wrong because some devices may have already been suspended
>    or may have not been resumed.
> 
> Thus, the proper way to do this is to cancel all thermal zone
> device update requirements during suspend/resume, and after all
> the devices have been resumed, reset and update every registered
> thermal zone devices.
> 
> This also fixes a regression introduced by:
> Commit 19593a1fb1f6 ("ACPI / fan: convert to platform driver")
> Because, with above commit applied, all the fan devices are attached
> to the acpi_general_pm_domain, and they are turned on by the pm_domain
> automatically after resume, without the awareness of thermal core.
> 
> CC: <stable@vger.kernel.org> #3.18+
> Reference: https://bugzilla.kernel.org/show_bug.cgi?id=78201
> Reference: https://bugzilla.kernel.org/show_bug.cgi?id=91411
> Tested-by: Manuel Krause <manuelkrause@netscape.net>
> Tested-by: szegad <szegadlo@poczta.onet.pl>
> Tested-by: prash <prash.n.rao@gmail.com>
> Tested-by: amish <ammdispose-arch@yahoo.com>
> Tested-by: Matthias <morpheusxyz123@yahoo.de>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  drivers/thermal/thermal_core.c | 45 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 682bc1e..abeb995 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -37,6 +37,7 @@
>  #include <linux/of.h>
>  #include <net/netlink.h>
>  #include <net/genetlink.h>
> +#include <linux/suspend.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/thermal.h>
> @@ -59,6 +60,8 @@ static LIST_HEAD(thermal_governor_list);
>  static DEFINE_MUTEX(thermal_list_lock);
>  static DEFINE_MUTEX(thermal_governor_lock);
>  
> +static atomic_t in_suspend;
> +
>  static struct thermal_governor *def_governor;
>  
>  static struct thermal_governor *__find_governor(const char *name)
> @@ -554,6 +557,9 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>  {
>  	int count;
>  
> +	if (atomic_read(&in_suspend))
> +		return;
> +
>  	if (!tz->ops->get_temp)
>  		return;
>  
> @@ -2155,9 +2161,39 @@ static void thermal_unregister_governors(void)
>  	thermal_gov_power_allocator_unregister();
>  }
>  
> +static int thermal_pm_notify(struct notifier_block *nb,
> +				unsigned long mode, void *_unused)
> +{
> +	struct thermal_zone_device *tz;
> +
> +	switch (mode) {
> +	case PM_HIBERNATION_PREPARE:
> +	case PM_RESTORE_PREPARE:
> +	case PM_SUSPEND_PREPARE:
> +		atomic_set(&in_suspend, 1);
> +		break;
> +	case PM_POST_HIBERNATION:
> +	case PM_POST_RESTORE:
> +	case PM_POST_SUSPEND:
> +		atomic_set(&in_suspend, 0);
> +		list_for_each_entry(tz, &thermal_tz_list, node) {
> +			thermal_zone_device_reset(tz);
> +			thermal_zone_device_update(tz);
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +	return 0;
> +}
> +
> +static struct notifier_block thermal_pm_nb = {
> +	.notifier_call = thermal_pm_notify,
> +};
> +
>  static int __init thermal_init(void)
>  {
> -	int result;
> +	int result, notifier_result;
>  
>  	result = thermal_register_governors();
>  	if (result)
> @@ -2175,6 +2211,12 @@ static int __init thermal_init(void)
>  	if (result)
>  		goto exit_netlink;
>  
> +	notifier_result = register_pm_notifier(&thermal_pm_nb);

No need to define a new variable, you can use result.

> +	if (notifier_result)
> +		pr_err("Thermal: Can not register suspend notifier"

pr_warn ?

> +			"for thermal framework, return %d\n",

Please keep the string in the same line, don't break it.  I know that
checkpatch.pl complains, but the CodingStyle says don't break them:

"However, never break user-visible strings such as printk messages,
because that breaks the ability to grep for them"

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle#n86

Cheers,
Javi

> +			notifier_result);
> +
>  	return 0;
>  
>  exit_netlink:
> @@ -2194,6 +2236,7 @@ error:
>  
>  static void __exit thermal_exit(void)
>  {
> +	unregister_pm_notifier(&thermal_pm_nb);
>  	of_thermal_destroy_zones();
>  	genetlink_exit();
>  	class_unregister(&thermal_class);
> -- 
> 1.8.4.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web