| Message ID | 1443332908-13948-1-git-send-email-yu.c.chen@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Zhang Rui |
| Headers | show |
On Sun, Sep 27, 2015 at 06:48:28AM +0100, Chen Yu wrote: > 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. It may be worth pointing out that 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> > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > Signed-off-by: Chen Yu <yu.c.chen@intel.com> One minor nit below, other than that, Reviewed-by: Javi Merino <javi.merino@arm.com> > --- > drivers/thermal/step_wise.c | 18 ++++++++++++++++-- > drivers/thermal/thermal_core.c | 19 +++++++++++++++++-- > drivers/thermal/thermal_core.h | 1 + > include/linux/thermal.h | 3 +++ > 4 files changed, 37 insertions(+), 4 deletions(-) > > diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c > index 2f9f708..405de04 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,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) nit: it fits in one line, if (instance->initialized && old_target == instance->target) is 77 characters. Cheers, Javi -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Javi, > -----Original Message----- > From: Javi Merino [mailto:javi.merino@arm.com] > Sent: Monday, September 28, 2015 10:28 PM > To: Chen, Yu C > Cc: linux-pm@vger.kernel.org; edubezval@gmail.com; Zhang, Rui; linux- > kernel@vger.kernel.org; stable@vger.kernel.org > Subject: Re: [PATCH 1/3] Thermal: initialize thermal zone device correctly > > On Sun, Sep 27, 2015 at 06:48:28AM +0100, Chen Yu wrote: > > 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. > > It may be worth pointing out that the step_wise governor is the only one > that uses trends, so it's the only thermal governor that needs to be updated. > OK, will add. > > > - if (old_target == instance->target) > > + if (instance->initialized && > > + old_target == instance->target) > > nit: it fits in one line, > > if (instance->initialized && old_target == instance->target) > > is 77 characters. > Not sure if the limit for one line is 75 in checkpatch.pl, I'll have a try. Thanks! Best Regards, Yu
On Mon, Sep 28, 2015 at 06:30:30PM +0100, Chen, Yu C wrote: > Hi, Javi, > > > -----Original Message----- > > From: Javi Merino [mailto:javi.merino@arm.com] > > Sent: Monday, September 28, 2015 10:28 PM > > To: Chen, Yu C > > Cc: linux-pm@vger.kernel.org; edubezval@gmail.com; Zhang, Rui; linux- > > kernel@vger.kernel.org; stable@vger.kernel.org > > Subject: Re: [PATCH 1/3] Thermal: initialize thermal zone device correctly > > > > On Sun, Sep 27, 2015 at 06:48:28AM +0100, Chen Yu wrote: > > > 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. > > > > It may be worth pointing out that the step_wise governor is the only one > > that uses trends, so it's the only thermal governor that needs to be updated. > > > OK, will add. > > > > > - if (old_target == instance->target) > > > + if (instance->initialized && > > > + old_target == instance->target) > > > > nit: it fits in one line, > > > > if (instance->initialized && old_target == instance->target) > > > > is 77 characters. > > > Not sure if the limit for one line is 75 in checkpatch.pl, I'll have a try. The limit is 80: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle#n79 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/scripts/checkpatch.pl#n47 Cheers, Javi -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c index 2f9f708..405de04 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,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 +175,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)