All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/3] Thermal: thermal enhancements for boot and system sleep
@ 2015-03-25  7:53 Zhang Rui
  2015-03-25  7:53 ` [PATCH V2 1/3] Thermal: initialize thermal zone device correctly Zhang Rui
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Zhang Rui @ 2015-03-25  7:53 UTC (permalink / raw)
  To: linux-pm; +Cc: Zhang Rui

Currently, there are a couple of problems in thermal core framework after boot
and resume from system sleep state, because the thermal zone devices are not
put into a proper state in these cases.

Details of the problems are described in the patch change logs.

In general, altogether they fix three bugs
https://bugzilla.kernel.org/show_bug.cgi?id=78201
https://bugzilla.kernel.org/show_bug.cgi?id=91411
https://bugzilla.kernel.org/show_bug.cgi?id=92431

Bug 78201 needs patch 1/3 and 2/3.
Bug 91411 and 92431 are regressions caused by
commit 19593a1fb1f6718406afca5b867dab184289d406
Author: Aaron Lu <aaron.lu@intel.com>
Date:   Tue Nov 19 16:59:20 2013 +0800

    ACPI / fan: convert to platform driver

    Convert ACPI fan driver to a platform driver for the purpose of phasing
    out ACPI bus.

    Signed-off-by: Aaron Lu <aaron.lu@intel.com>
    Signed-off-by: Zhang Rui <rui.zhang@intel.com>

which is shipped in 3.18.

Bug 91411 needs patch 1/3, 2/3 to fix, while 92431 needs all three patches.

If possible, I'd like to push these patches into 4.0-rc and 3.18/3.19 stable
kernel as it actually fixes a regression in 3.18.

Any comments are welcome.

thanks,
rui
-----
Changes in V2:
	rename no_thermal_update to in_suspend
	rename thermal_notify to thermal_pm_notify
	declare thermal_pm_nb with callback initialized
	unregister thermal_pm_nb in thermal_exit()

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH V2 1/3] Thermal: initialize thermal zone device correctly
  2015-03-25  7:53 [PATCH V2 0/3] Thermal: thermal enhancements for boot and system sleep Zhang Rui
@ 2015-03-25  7:53 ` Zhang Rui
  2015-03-26 12:40   ` Punit Agrawal
  2015-03-25  7:53 ` [PATCH V2 2/3] Thermal: handle thermal zone device properly during system sleep Zhang Rui
  2015-03-25  7:53 ` [PATCH V2 3/3] Thermal: do thermal zone update after a cooling device registered Zhang Rui
  2 siblings, 1 reply; 7+ messages in thread
From: Zhang Rui @ 2015-03-25  7:53 UTC (permalink / raw)
  To: linux-pm; +Cc: Zhang Rui, stable

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.

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>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/step_wise.c    | 15 +++++++++++++--
 drivers/thermal/thermal_core.c | 19 +++++++++++++++++--
 drivers/thermal/thermal_core.h |  1 +
 include/linux/thermal.h        |  3 +++
 4 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index 5a0f12d..c2bb37c 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -63,6 +63,16 @@ 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;
+	}
+
 	switch (trend) {
 	case THERMAL_TREND_RAISING:
 		if (throttle) {
@@ -149,7 +159,8 @@ 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 +172,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 174d3bc..9d6f71b 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -469,8 +469,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)
@@ -1574,6 +1588,7 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 	if (!tz->ops->get_temp)
 		thermal_zone_device_set_polling(tz, 0);
 
+	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 0531c75..6d9ffa5 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 5eac316..8650b0b 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -40,6 +40,9 @@
 /* No upper/lower limit requirement */
 #define THERMAL_NO_LIMIT	((u32)~0)
 
+/* Invalid/uninitialized temperature */
+#define THERMAL_TEMP_INVALID	-27400
+
 /* Unit conversion macros */
 #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
 				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH V2 2/3] Thermal: handle thermal zone device properly during system sleep
  2015-03-25  7:53 [PATCH V2 0/3] Thermal: thermal enhancements for boot and system sleep Zhang Rui
  2015-03-25  7:53 ` [PATCH V2 1/3] Thermal: initialize thermal zone device correctly Zhang Rui
@ 2015-03-25  7:53 ` Zhang Rui
  2015-03-25  7:53 ` [PATCH V2 3/3] Thermal: do thermal zone update after a cooling device registered Zhang Rui
  2 siblings, 0 replies; 7+ messages in thread
From: Zhang Rui @ 2015-03-25  7:53 UTC (permalink / raw)
  To: linux-pm; +Cc: Zhang Rui, stable

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 19593a1fb1f6718406afca5b867dab184289d406
Author: Aaron Lu <aaron.lu@intel.com>
Date:   Tue Nov 19 16:59:20 2013 +0800

    ACPI / fan: convert to platform driver

    Convert ACPI fan driver to a platform driver for the purpose of phasing
    out ACPI bus.

    Signed-off-by: Aaron Lu <aaron.lu@intel.com>
    Signed-off-by: Zhang Rui <rui.zhang@intel.com>

Because, with the 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>
---
 drivers/thermal/thermal_core.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 9d6f71b..2f3c829 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 bool in_suspend;
+
 static struct thermal_governor *def_governor;
 
 static struct thermal_governor *__find_governor(const char *name)
@@ -491,6 +494,9 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 {
 	int count;
 
+	if (in_suspend)
+		return;
+
 	if (!tz->ops->get_temp)
 		return;
 
@@ -1823,6 +1829,36 @@ static void thermal_unregister_governors(void)
 	thermal_gov_user_space_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:
+		in_suspend = true;
+		break;
+	case PM_POST_HIBERNATION:
+	case PM_POST_RESTORE:
+	case PM_POST_SUSPEND:
+		in_suspend = false;
+		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;
@@ -1843,6 +1879,8 @@ static int __init thermal_init(void)
 	if (result)
 		goto exit_netlink;
 
+	register_pm_notifier(&thermal_pm_nb);
+
 	return 0;
 
 exit_netlink:
@@ -1862,6 +1900,7 @@ static int __init thermal_init(void)
 
 static void __exit thermal_exit(void)
 {
+	unregister_pm_notifier(&thermal_pm_nb);
 	of_thermal_destroy_zones();
 	genetlink_exit();
 	class_unregister(&thermal_class);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH V2 3/3] Thermal: do thermal zone update after a cooling device registered
  2015-03-25  7:53 [PATCH V2 0/3] Thermal: thermal enhancements for boot and system sleep Zhang Rui
  2015-03-25  7:53 ` [PATCH V2 1/3] Thermal: initialize thermal zone device correctly Zhang Rui
  2015-03-25  7:53 ` [PATCH V2 2/3] Thermal: handle thermal zone device properly during system sleep Zhang Rui
@ 2015-03-25  7:53 ` Zhang Rui
  2 siblings, 0 replies; 7+ messages in thread
From: Zhang Rui @ 2015-03-25  7:53 UTC (permalink / raw)
  To: linux-pm; +Cc: Zhang Rui, stable

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.

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>
---
 drivers/thermal/thermal_core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 2f3c829..f872744 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1140,6 +1140,7 @@ __thermal_cooling_device_register(struct device_node *np,
 				  const struct thermal_cooling_device_ops *ops)
 {
 	struct thermal_cooling_device *cdev;
+	struct thermal_instance *pos, *next;
 	int result;
 
 	if (type && strlen(type) >= THERMAL_NAME_LENGTH)
@@ -1184,6 +1185,15 @@ __thermal_cooling_device_register(struct device_node *np,
 	/* Update binding information for 'this' new cdev */
 	bind_cdev(cdev);
 
+	list_for_each_entry_safe(pos, next, &cdev->thermal_instances, cdev_node) {
+			if (next->cdev_node.next == &cdev->thermal_instances) {
+				thermal_zone_device_update(next->tz);
+				break;
+			}
+			if (pos->tz != next->tz)
+				thermal_zone_device_update(pos->tz);
+	}
+
 	return cdev;
 }
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V2 1/3] Thermal: initialize thermal zone device correctly
  2015-03-25  7:53 ` [PATCH V2 1/3] Thermal: initialize thermal zone device correctly Zhang Rui
@ 2015-03-26 12:40   ` Punit Agrawal
  2015-03-30  7:51     ` Zhang, Rui
  2015-04-01  7:21     ` Zhang, Rui
  0 siblings, 2 replies; 7+ messages in thread
From: Punit Agrawal @ 2015-03-26 12:40 UTC (permalink / raw)
  To: Zhang Rui; +Cc: linux-pm, stable

Zhang Rui <rui.zhang@intel.com> writes:

> 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.
>
> 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>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/thermal/step_wise.c    | 15 +++++++++++++--
>  drivers/thermal/thermal_core.c | 19 +++++++++++++++++--
>  drivers/thermal/thermal_core.h |  1 +
>  include/linux/thermal.h        |  3 +++
>  4 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
> index 5a0f12d..c2bb37c 100644
> --- a/drivers/thermal/step_wise.c
> +++ b/drivers/thermal/step_wise.c
> @@ -63,6 +63,16 @@ 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;
> +	}
> +

Instead of checking whether the instance has been initialised, you could
add THERMAL_TREND_INVALID to the thermal_trend enum. That can then be
handled in the switch case below instead of needing special case
handling like above.

What do you think?

>  	switch (trend) {
>  	case THERMAL_TREND_RAISING:
>  		if (throttle) {
> @@ -149,7 +159,8 @@ 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 +172,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.h b/drivers/thermal/thermal_core.h
> index 0531c75..6d9ffa5 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 5eac316..8650b0b 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -40,6 +40,9 @@
>  /* No upper/lower limit requirement */
>  #define THERMAL_NO_LIMIT	((u32)~0)
>  
> +/* Invalid/uninitialized temperature */
> +#define THERMAL_TEMP_INVALID	-27400
> +

Did you mean -274000 (note the missing '0') which is < 0K? A comment
would be helpful for anybody reading the code. Took me a while to parse
it.

>  /* Unit conversion macros */
>  #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
>  				((long)t-2732+5)/10 : ((long)t-2732-5)/10)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH V2 1/3] Thermal: initialize thermal zone device correctly
  2015-03-26 12:40   ` Punit Agrawal
@ 2015-03-30  7:51     ` Zhang, Rui
  2015-04-01  7:21     ` Zhang, Rui
  1 sibling, 0 replies; 7+ messages in thread
From: Zhang, Rui @ 2015-03-30  7:51 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: linux-pm, stable



> -----Original Message-----
> From: Punit Agrawal [mailto:punit.agrawal@arm.com]
> Sent: Thursday, March 26, 2015 8:41 PM
> To: Zhang, Rui
> Cc: linux-pm@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH V2 1/3] Thermal: initialize thermal zone device correctly
> Importance: High
> 
> Zhang Rui <rui.zhang@intel.com> writes:
> 
> > 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.
> >
> > 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>
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/thermal/step_wise.c    | 15 +++++++++++++--
> >  drivers/thermal/thermal_core.c | 19 +++++++++++++++++--
> > drivers/thermal/thermal_core.h |  1 +
> >  include/linux/thermal.h        |  3 +++
> >  4 files changed, 34 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
> > index 5a0f12d..c2bb37c 100644
> > --- a/drivers/thermal/step_wise.c
> > +++ b/drivers/thermal/step_wise.c
> > @@ -63,6 +63,16 @@ 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;
> > +	}
> > +
> 
> Instead of checking whether the instance has been initialised, you could add
> THERMAL_TREND_INVALID to the thermal_trend enum. That can then be
> handled in the switch case below instead of needing special case handling like
> above.
> 
> What do you think?
> 
Well, that's what I did in my initial proposal, but I found it is not enough.
Say, for the problem fixed in patch 3/3, when a cooling device is registered after
the thermal zone, the thermal trend is not THEMAL_TREND_INVALID, but we need
to get a proper cooling state for the newly registered cooling device.
Thus a per thermal_instance flag is introduced instead.

Thanks,
rui

> >  	switch (trend) {
> >  	case THERMAL_TREND_RAISING:
> >  		if (throttle) {
> > @@ -149,7 +159,8 @@ 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 +172,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.h
> > b/drivers/thermal/thermal_core.h index 0531c75..6d9ffa5 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
> > 5eac316..8650b0b 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -40,6 +40,9 @@
> >  /* No upper/lower limit requirement */
> >  #define THERMAL_NO_LIMIT	((u32)~0)
> >
> > +/* Invalid/uninitialized temperature */
> > +#define THERMAL_TEMP_INVALID	-27400
> > +
> 
> Did you mean -274000 (note the missing '0') which is < 0K? A comment would be
> helpful for anybody reading the code. Took me a while to parse it.
> 
> >  /* Unit conversion macros */
> >  #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
> >  				((long)t-2732+5)/10 : ((long)t-2732-5)/10)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH V2 1/3] Thermal: initialize thermal zone device correctly
  2015-03-26 12:40   ` Punit Agrawal
  2015-03-30  7:51     ` Zhang, Rui
@ 2015-04-01  7:21     ` Zhang, Rui
  1 sibling, 0 replies; 7+ messages in thread
From: Zhang, Rui @ 2015-04-01  7:21 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: linux-pm, stable

> > diff --git a/drivers/thermal/thermal_core.h
> > b/drivers/thermal/thermal_core.h index 0531c75..6d9ffa5 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
> > 5eac316..8650b0b 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -40,6 +40,9 @@
> >  /* No upper/lower limit requirement */
> >  #define THERMAL_NO_LIMIT	((u32)~0)
> >
> > +/* Invalid/uninitialized temperature */
> > +#define THERMAL_TEMP_INVALID	-27400
> > +
> 
> Did you mean -274000 (note the missing '0') which is < 0K? A comment would be
> helpful for anybody reading the code. Took me a while to parse it.
> 
Oops, I missed this one.
Yes, I want to use -274000 which is < 0K to indicate an invalid/uninitialized temperature.
Updated in patch V4. Thanks for pointing it out.

-rui

> >  /* Unit conversion macros */
> >  #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
> >  				((long)t-2732+5)/10 : ((long)t-2732-5)/10)

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-04-01  7:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-25  7:53 [PATCH V2 0/3] Thermal: thermal enhancements for boot and system sleep Zhang Rui
2015-03-25  7:53 ` [PATCH V2 1/3] Thermal: initialize thermal zone device correctly Zhang Rui
2015-03-26 12:40   ` Punit Agrawal
2015-03-30  7:51     ` Zhang, Rui
2015-04-01  7:21     ` Zhang, Rui
2015-03-25  7:53 ` [PATCH V2 2/3] Thermal: handle thermal zone device properly during system sleep Zhang Rui
2015-03-25  7:53 ` [PATCH V2 3/3] Thermal: do thermal zone update after a cooling device registered Zhang Rui

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.