| Message ID | 1428373476-14257-2-git-send-email-rui.zhang@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Zhang Rui |
| Headers | show |
On Tue, Apr 07, 2015 at 10:24:34AM +0800, Zhang Rui wrote: > 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> Can you please consider the comments I made on V3? Summary: 1. Change initialized to trend_valid 2. return next_target earlier in the get_target_state, so the behavior is consistent. > --- > 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..fb96b15 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) > > +/* 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.9.1 > > -- > 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
> -----Original Message----- > From: Eduardo Valentin [mailto:edubezval@gmail.com] > Sent: Tuesday, April 7, 2015 10:47 AM > To: Zhang, Rui > Cc: linux-pm@vger.kernel.org > Subject: Re: [PATCH V4 1/3] Thermal: initialize thermal zone device correctly > Importance: High > > On Tue, Apr 07, 2015 at 10:24:34AM +0800, Zhang Rui wrote: > > 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> > > Can you please consider the comments I made on V3? > Sorry, I missed your previous comment. > Summary: > > 1. Change initialized to trend_valid I don't think it is proper to call it "trend_valid" because in the case that cooling device is registered after thermal zone (the problem pointed out in patch 3/3), the new created thermal_instance needs to be set properly, and the trend is valid at this time actually. IMO, thermal_instance->initialized just means if the thermal instance is evaluated for the first time or not, and if yes, we need some special handling. > 2. return next_target earlier in the get_target_state, so the behavior is > consistent. Agreed. Will fix in next version. Thanks, rui > > > --- > > 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..fb96b15 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) > > > > +/* 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.9.1 > > > > -- > > 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 -- 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
Hello Rui, On Wed, Apr 08, 2015 at 01:01:08PM +0000, Zhang, Rui wrote: > > > > -----Original Message----- > > From: Eduardo Valentin [mailto:edubezval@gmail.com] > > Sent: Tuesday, April 7, 2015 10:47 AM > > To: Zhang, Rui > > Cc: linux-pm@vger.kernel.org > > Subject: Re: [PATCH V4 1/3] Thermal: initialize thermal zone device correctly > > Importance: High > > > > On Tue, Apr 07, 2015 at 10:24:34AM +0800, Zhang Rui wrote: > > > 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> > > > > Can you please consider the comments I made on V3? > > > Sorry, I missed your previous comment. > > > Summary: > > > > 1. Change initialized to trend_valid > > I don't think it is proper to call it "trend_valid" because in the case that cooling device > is registered after thermal zone (the problem pointed out in patch 3/3), > the new created thermal_instance needs to be set properly, and the trend is valid > at this time actually. I see your point. But for the case of the problem solved by patch 3/3, shouldn't the core part call then call thermal_zone_device_update only for the thermal_zones that the cooling device has a thermal_instance with ->initialized == false? I think in this case the thermal_core needs to be more aware of this property, instead of leaving for governors to deal with it. And the way it is put today, looks like the property is useful only for step_wise, that is why it makes me feel like it is trend bounded. > > IMO, thermal_instance->initialized just means if the thermal instance is evaluated for > the first time or not, and if yes, we need some special handling. OK. Thanks for clarifying. > > > 2. return next_target earlier in the get_target_state, so the behavior is > > consistent. > > Agreed. Will fix in next version. OK. Thanks. > > Thanks, > rui > > > > > --- > > > 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..fb96b15 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) > > > > > > +/* 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.9.1 > > > > > > -- > > > 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
On Wed, Apr 08, 2015 at 02:01:08PM +0100, Zhang, Rui wrote: > > > > -----Original Message----- > > From: Eduardo Valentin [mailto:edubezval@gmail.com] > > Sent: Tuesday, April 7, 2015 10:47 AM > > To: Zhang, Rui > > Cc: linux-pm@vger.kernel.org > > Subject: Re: [PATCH V4 1/3] Thermal: initialize thermal zone device correctly > > Importance: High > > > > On Tue, Apr 07, 2015 at 10:24:34AM +0800, Zhang Rui wrote: > > > 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> > > > > Can you please consider the comments I made on V3? > > > Sorry, I missed your previous comment. > > > Summary: > > > > 1. Change initialized to trend_valid > > I don't think it is proper to call it "trend_valid" because in the case that cooling device > is registered after thermal zone (the problem pointed out in patch 3/3), > the new created thermal_instance needs to be set properly, and the trend is valid > at this time actually. > > IMO, thermal_instance->initialized just means if the thermal instance is evaluated for > the first time or not, and if yes, we need some special handling. I think the main source of confusion here is the fact that this is only needed for step_wise. Thermal governors that don't care about trend will always have thermal instances with "initialized" being false, which seems counter-intuitive. The only name I can think of is "not_yet_evaluated_by_step_wise" which is ridiculously long name for a variable, but can we get something more descriptive than "initialized"? 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
On Tue, Apr 07, 2015 at 03:24:34AM +0100, Zhang Rui wrote: > 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; CodingStyle says that if one branch of an if statement needs braces, all branches must have braces: if (condition) { do_this(); do_that(); } else { otherwise(); } https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle#n169 > + } > + I pointed out before and I think Eduardo has said it as well. I think you should "return next_target;" at the end of the "if (!instance->initialized)" > switch (trend) { > case THERMAL_TREND_RAISING: > if (throttle) { 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
> -----Original Message----- > From: linux-pm-owner@vger.kernel.org [mailto:linux-pm- > owner@vger.kernel.org] On Behalf Of Eduardo Valentin > Sent: Wednesday, April 8, 2015 11:04 PM > To: Zhang, Rui > Cc: linux-pm@vger.kernel.org > Subject: Re: [PATCH V4 1/3] Thermal: initialize thermal zone device correctly > Importance: High > > Hello Rui, > > On Wed, Apr 08, 2015 at 01:01:08PM +0000, Zhang, Rui wrote: > > > > > > > -----Original Message----- > > > From: Eduardo Valentin [mailto:edubezval@gmail.com] > > > Sent: Tuesday, April 7, 2015 10:47 AM > > > To: Zhang, Rui > > > Cc: linux-pm@vger.kernel.org > > > Subject: Re: [PATCH V4 1/3] Thermal: initialize thermal zone device > > > correctly > > > Importance: High > > > > > > On Tue, Apr 07, 2015 at 10:24:34AM +0800, Zhang Rui wrote: > > > > 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> > > > > > > Can you please consider the comments I made on V3? > > > > > Sorry, I missed your previous comment. > > > > > Summary: > > > > > > 1. Change initialized to trend_valid > > > > I don't think it is proper to call it "trend_valid" because in the > > case that cooling device is registered after thermal zone (the problem > > pointed out in patch 3/3), the new created thermal_instance needs to > > be set properly, and the trend is valid at this time actually. > > I see your point. But for the case of the problem solved by patch 3/3, shouldn't > the core part call then call thermal_zone_device_update only for the > thermal_zones that the cooling device has a thermal_instance with ->initialized > == false? Yes, we can do it in that way. But it seems that we need some extra code for this in thermal governor instead of thermal core. Thus I preferred to use the existing thermal API. Plus, for the fair_share governor, we do need to re-calculate the cooling state for every thermal instance when a new cooling device registered, right? > > I think in this case the thermal_core needs to be more aware of this property, > instead of leaving for governors to deal with it. And the way it is put today, looks > like the property is useful only for step_wise, that is why it makes me feel like it > is trend bounded. Hmm, IMO, it is hard to do this in thermal core in current design, because .throttle() callback is based on trip points rather than thermal_instance, i.e. it evaluates the thermal instances that shares the same trip point in one time, thus I don't think we can handle this without thermal governor change. I agree there are some areas worth being improved, and maybe we could get a better solution some time, but for now, as there are two regression reports, and these patches have been tested to work for them, I really want to push these patches as a urgent fix. :) Thanks, rui > > > > > > IMO, thermal_instance->initialized just means if the thermal instance > > is evaluated for the first time or not, and if yes, we need some special handling. > > OK. Thanks for clarifying. > > > > > > 2. return next_target earlier in the get_target_state, so the > > > behavior is consistent. > > > > Agreed. Will fix in next version. > > OK. Thanks. > > > > > Thanks, > > rui > > > > > > > --- > > > > 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..fb96b15 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) > > > > > > > > +/* 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.9.1 > > > > > > > > -- > > > > 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 -- 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
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogSmF2aSBNZXJpbm8gW21h aWx0bzpqYXZpLm1lcmlub0Bhcm0uY29tXQ0KPiBTZW50OiBUaHVyc2RheSwgQXByaWwgOSwgMjAx NSAxMDo1NyBQTQ0KPiBUbzogWmhhbmcsIFJ1aQ0KPiBDYzogbGludXgtcG1Admdlci5rZXJuZWwu b3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggVjQgMS8zXSBUaGVybWFsOiBpbml0aWFsaXplIHRo ZXJtYWwgem9uZSBkZXZpY2UgY29ycmVjdGx5DQo+IEltcG9ydGFuY2U6IEhpZ2gNCj4gDQo+IE9u IFR1ZSwgQXByIDA3LCAyMDE1IGF0IDAzOjI0OjM0QU0gKzAxMDAsIFpoYW5nIFJ1aSB3cm90ZToN Cj4gPiBBZnRlciB0aGVybWFsIHpvbmUgZGV2aWNlIHJlZ2lzdGVyZWQsIGFzIHdlIGhhdmUgbm90 IHJlYWQgYW55DQo+ID4gdGVtcGVyYXR1cmUgYmVmb3JlLCB0aHVzIHR6LT50ZW1wZXJhdHVyZSBz aG91bGQgbm90IGJlIDAsIHdoaWNoDQo+ID4gYWN0dWFsbHkgbWVhbnMgMEMsIGFuZCB0aGVybWFs IHRyZW5kIGlzIG5vdCBhdmFpbGFibGUuDQo+ID4gSW4gdGhpcyBjYXNlLCB3ZSBuZWVkIHNwZWNp YWxseSBoYW5kbGluZyBmb3IgdGhlIGZpcnN0DQo+ID4gdGhlcm1hbF96b25lX2RldmljZV91cGRh dGUoKS4NCj4gPg0KPiA+IEJvdGggdGhlcm1hbCBjb3JlIGZyYW1ld29yayBhbmQgc3RlcF93aXNl IGdvdmVybm9yIGlzIGVuaGFuY2VkIHRvIGhhbmRsZQ0KPiB0aGlzLg0KPiA+DQo+ID4gQ0M6IDxz dGFibGVAdmdlci5rZXJuZWwub3JnPiAjMy4xOCsNCj4gPiBUZXN0ZWQtYnk6IE1hbnVlbCBLcmF1 c2UgPG1hbnVlbGtyYXVzZUBuZXRzY2FwZS5uZXQ+DQo+ID4gVGVzdGVkLWJ5OiBzemVnYWQgPHN6 ZWdhZGxvQHBvY3p0YS5vbmV0LnBsPg0KPiA+IFRlc3RlZC1ieTogcHJhc2ggPHByYXNoLm4ucmFv QGdtYWlsLmNvbT4NCj4gPiBUZXN0ZWQtYnk6IGFtaXNoIDxhbW1kaXNwb3NlLWFyY2hAeWFob28u Y29tPg0KPiA+IFRlc3RlZC1ieTogTWF0dGhpYXMgPG1vcnBoZXVzeHl6MTIzQHlhaG9vLmRlPg0K PiA+IFNpZ25lZC1vZmYtYnk6IFpoYW5nIFJ1aSA8cnVpLnpoYW5nQGludGVsLmNvbT4NCj4gPiAt LS0NCj4gPiAgZHJpdmVycy90aGVybWFsL3N0ZXBfd2lzZS5jICAgIHwgMTUgKysrKysrKysrKysr Ky0tDQo+ID4gIGRyaXZlcnMvdGhlcm1hbC90aGVybWFsX2NvcmUuYyB8IDE5ICsrKysrKysrKysr KysrKysrLS0NCj4gPiBkcml2ZXJzL3RoZXJtYWwvdGhlcm1hbF9jb3JlLmggfCAgMSArDQo+ID4g IGluY2x1ZGUvbGludXgvdGhlcm1hbC5oICAgICAgICB8ICAzICsrKw0KPiA+ICA0IGZpbGVzIGNo YW5nZWQsIDM0IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pDQo+ID4NCj4gPiBkaWZmIC0t Z2l0IGEvZHJpdmVycy90aGVybWFsL3N0ZXBfd2lzZS5jIGIvZHJpdmVycy90aGVybWFsL3N0ZXBf d2lzZS5jDQo+ID4gaW5kZXggNWEwZjEyZC4uYzJiYjM3YyAxMDA2NDQNCj4gPiAtLS0gYS9kcml2 ZXJzL3RoZXJtYWwvc3RlcF93aXNlLmMNCj4gPiArKysgYi9kcml2ZXJzL3RoZXJtYWwvc3RlcF93 aXNlLmMNCj4gPiBAQCAtNjMsNiArNjMsMTYgQEAgc3RhdGljIHVuc2lnbmVkIGxvbmcgZ2V0X3Rh cmdldF9zdGF0ZShzdHJ1Y3QNCj4gdGhlcm1hbF9pbnN0YW5jZSAqaW5zdGFuY2UsDQo+ID4gIAlu ZXh0X3RhcmdldCA9IGluc3RhbmNlLT50YXJnZXQ7DQo+ID4gIAlkZXZfZGJnKCZjZGV2LT5kZXZp Y2UsICJjdXJfc3RhdGU9JWxkXG4iLCBjdXJfc3RhdGUpOw0KPiA+DQo+ID4gKwlpZiAoIWluc3Rh bmNlLT5pbml0aWFsaXplZCkgew0KPiA+ICsJCWlmICh0aHJvdHRsZSkgew0KPiA+ICsJCQluZXh0 X3RhcmdldCA9IChjdXJfc3RhdGUgKyAxKSA+PSBpbnN0YW5jZS0+dXBwZXIgPw0KPiA+ICsJCQkJ CWluc3RhbmNlLT51cHBlciA6DQo+ID4gKwkJCQkJKChjdXJfc3RhdGUgKyAxKSA8IGluc3RhbmNl LT5sb3dlciA/DQo+ID4gKwkJCQkJaW5zdGFuY2UtPmxvd2VyIDogKGN1cl9zdGF0ZSArIDEpKTsN Cj4gPiArCQl9IGVsc2UNCj4gPiArCQkJbmV4dF90YXJnZXQgPSBUSEVSTUFMX05PX1RBUkdFVDsN Cj4gDQo+IENvZGluZ1N0eWxlIHNheXMgdGhhdCBpZiBvbmUgYnJhbmNoIG9mIGFuIGlmIHN0YXRl bWVudCBuZWVkcyBicmFjZXMsIGFsbCBicmFuY2hlcw0KPiBtdXN0IGhhdmUgYnJhY2VzOg0KPiAN Cj4gaWYgKGNvbmRpdGlvbikgew0KPiAgICAgICAgIGRvX3RoaXMoKTsNCj4gICAgICAgICBkb190 aGF0KCk7DQo+IH0gZWxzZSB7DQo+ICAgICAgICAgb3RoZXJ3aXNlKCk7DQo+IH0NCj4gDQo+IGh0 dHBzOi8vZ2l0Lmtlcm5lbC5vcmcvY2dpdC9saW51eC9rZXJuZWwvZ2l0L3RvcnZhbGRzL2xpbnV4 LmdpdC90cmVlL0RvY3VtZW50YXRpbw0KPiBuL0NvZGluZ1N0eWxlI24xNjkNCj4gDQpUaGFua3Mg Zm9yIHBvaW50aW5nIG91dCwgd2lsbCBmaXggaW4gbmV4dCB2ZXJzaW9uLg0KDQo+ID4gKwl9DQo+ ID4gKw0KPiANCj4gSSBwb2ludGVkIG91dCBiZWZvcmUgYW5kIEkgdGhpbmsgRWR1YXJkbyBoYXMg c2FpZCBpdCBhcyB3ZWxsLiAgSSB0aGluayB5b3Ugc2hvdWxkDQo+ICJyZXR1cm4gbmV4dF90YXJn ZXQ7IiBhdCB0aGUgZW5kIG9mIHRoZSAiaWYgKCFpbnN0YW5jZS0+aW5pdGlhbGl6ZWQpIg0KPiAN ClNvcnJ5LCBJIG1pc3NlZCB0aGVzZSBlbWFpbHMgYXMgbXkgTGludXggd29yayBzdGF0aW9uIHdh cyBicm9rZW4sIGFuZCBJJ20gbm90IHVzZWQgdG8gb3V0bG9vayB5ZXQuDQoNClRoYW5rcywNCnJ1 aQ0KPiA+ICAJc3dpdGNoICh0cmVuZCkgew0KPiA+ICAJY2FzZSBUSEVSTUFMX1RSRU5EX1JBSVNJ Tkc6DQo+ID4gIAkJaWYgKHRocm90dGxlKSB7DQo+IA0KPiBDaGVlcnMsDQo+IEphdmkNCg0K -- 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
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogSmF2aSBNZXJpbm8gW21h aWx0bzpqYXZpLm1lcmlub0Bhcm0uY29tXQ0KPiBTZW50OiBUaHVyc2RheSwgQXByaWwgOSwgMjAx NSAxMDo0NCBQTQ0KPiBUbzogWmhhbmcsIFJ1aQ0KPiBDYzogRWR1YXJkbyBWYWxlbnRpbjsgbGlu dXgtcG1Admdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggVjQgMS8zXSBUaGVy bWFsOiBpbml0aWFsaXplIHRoZXJtYWwgem9uZSBkZXZpY2UgY29ycmVjdGx5DQo+IEltcG9ydGFu Y2U6IEhpZ2gNCj4gDQo+IE9uIFdlZCwgQXByIDA4LCAyMDE1IGF0IDAyOjAxOjA4UE0gKzAxMDAs IFpoYW5nLCBSdWkgd3JvdGU6DQo+ID4NCj4gPg0KPiA+ID4gLS0tLS1PcmlnaW5hbCBNZXNzYWdl LS0tLS0NCj4gPiA+IEZyb206IEVkdWFyZG8gVmFsZW50aW4gW21haWx0bzplZHViZXp2YWxAZ21h aWwuY29tXQ0KPiA+ID4gU2VudDogVHVlc2RheSwgQXByaWwgNywgMjAxNSAxMDo0NyBBTQ0KPiA+ ID4gVG86IFpoYW5nLCBSdWkNCj4gPiA+IENjOiBsaW51eC1wbUB2Z2VyLmtlcm5lbC5vcmcNCj4g PiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggVjQgMS8zXSBUaGVybWFsOiBpbml0aWFsaXplIHRoZXJt YWwgem9uZSBkZXZpY2UNCj4gPiA+IGNvcnJlY3RseQ0KPiA+ID4gSW1wb3J0YW5jZTogSGlnaA0K PiA+ID4NCj4gPiA+IE9uIFR1ZSwgQXByIDA3LCAyMDE1IGF0IDEwOjI0OjM0QU0gKzA4MDAsIFpo YW5nIFJ1aSB3cm90ZToNCj4gPiA+ID4gQWZ0ZXIgdGhlcm1hbCB6b25lIGRldmljZSByZWdpc3Rl cmVkLCBhcyB3ZSBoYXZlIG5vdCByZWFkIGFueQ0KPiA+ID4gPiB0ZW1wZXJhdHVyZSBiZWZvcmUs IHRodXMgdHotPnRlbXBlcmF0dXJlIHNob3VsZCBub3QgYmUgMCwgd2hpY2gNCj4gPiA+ID4gYWN0 dWFsbHkgbWVhbnMgMEMsIGFuZCB0aGVybWFsIHRyZW5kIGlzIG5vdCBhdmFpbGFibGUuDQo+ID4g PiA+IEluIHRoaXMgY2FzZSwgd2UgbmVlZCBzcGVjaWFsbHkgaGFuZGxpbmcgZm9yIHRoZSBmaXJz dA0KPiA+ID4gPiB0aGVybWFsX3pvbmVfZGV2aWNlX3VwZGF0ZSgpLg0KPiA+ID4gPg0KPiA+ID4g PiBCb3RoIHRoZXJtYWwgY29yZSBmcmFtZXdvcmsgYW5kIHN0ZXBfd2lzZSBnb3Zlcm5vciBpcyBl bmhhbmNlZCB0bw0KPiA+ID4gPiBoYW5kbGUNCj4gPiA+IHRoaXMuDQo+ID4gPiA+DQo+ID4gPiA+ IENDOiA8c3RhYmxlQHZnZXIua2VybmVsLm9yZz4gIzMuMTgrDQo+ID4gPiA+IFRlc3RlZC1ieTog TWFudWVsIEtyYXVzZSA8bWFudWVsa3JhdXNlQG5ldHNjYXBlLm5ldD4NCj4gPiA+ID4gVGVzdGVk LWJ5OiBzemVnYWQgPHN6ZWdhZGxvQHBvY3p0YS5vbmV0LnBsPg0KPiA+ID4gPiBUZXN0ZWQtYnk6 IHByYXNoIDxwcmFzaC5uLnJhb0BnbWFpbC5jb20+DQo+ID4gPiA+IFRlc3RlZC1ieTogYW1pc2gg PGFtbWRpc3Bvc2UtYXJjaEB5YWhvby5jb20+DQo+ID4gPiA+IFRlc3RlZC1ieTogTWF0dGhpYXMg PG1vcnBoZXVzeHl6MTIzQHlhaG9vLmRlPg0KPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBaaGFuZyBS dWkgPHJ1aS56aGFuZ0BpbnRlbC5jb20+DQo+ID4gPg0KPiA+ID4gQ2FuIHlvdSBwbGVhc2UgY29u c2lkZXIgdGhlIGNvbW1lbnRzIEkgbWFkZSBvbiBWMz8NCj4gPiA+DQo+ID4gU29ycnksIEkgbWlz c2VkIHlvdXIgcHJldmlvdXMgY29tbWVudC4NCj4gPg0KPiA+ID4gU3VtbWFyeToNCj4gPiA+DQo+ ID4gPiAxLiBDaGFuZ2UgaW5pdGlhbGl6ZWQgdG8gdHJlbmRfdmFsaWQNCj4gPg0KPiA+IEkgZG9u J3QgdGhpbmsgaXQgaXMgcHJvcGVyIHRvIGNhbGwgaXQgInRyZW5kX3ZhbGlkIiBiZWNhdXNlIGlu IHRoZQ0KPiA+IGNhc2UgdGhhdCBjb29saW5nIGRldmljZSBpcyByZWdpc3RlcmVkIGFmdGVyIHRo ZXJtYWwgem9uZSAodGhlIHByb2JsZW0NCj4gPiBwb2ludGVkIG91dCBpbiBwYXRjaCAzLzMpLCB0 aGUgbmV3IGNyZWF0ZWQgdGhlcm1hbF9pbnN0YW5jZSBuZWVkcyB0bw0KPiA+IGJlIHNldCBwcm9w ZXJseSwgYW5kIHRoZSB0cmVuZCBpcyB2YWxpZCBhdCB0aGlzIHRpbWUgYWN0dWFsbHkuDQo+ID4N Cj4gPiBJTU8sIHRoZXJtYWxfaW5zdGFuY2UtPmluaXRpYWxpemVkIGp1c3QgbWVhbnMgaWYgdGhl IHRoZXJtYWwgaW5zdGFuY2UNCj4gPiBpcyBldmFsdWF0ZWQgZm9yIHRoZSBmaXJzdCB0aW1lIG9y IG5vdCwgYW5kIGlmIHllcywgd2UgbmVlZCBzb21lIHNwZWNpYWwgaGFuZGxpbmcuDQo+IA0KPiBJ IHRoaW5rIHRoZSBtYWluIHNvdXJjZSBvZiBjb25mdXNpb24gaGVyZSBpcyB0aGUgZmFjdCB0aGF0 IHRoaXMgaXMgb25seSBuZWVkZWQgZm9yDQo+IHN0ZXBfd2lzZS4gIFRoZXJtYWwgZ292ZXJub3Jz IHRoYXQgZG9uJ3QgY2FyZSBhYm91dCB0cmVuZCB3aWxsIGFsd2F5cyBoYXZlDQo+IHRoZXJtYWwg aW5zdGFuY2VzIHdpdGggImluaXRpYWxpemVkIiBiZWluZyBmYWxzZSwgd2hpY2ggc2VlbXMgY291 bnRlci1pbnR1aXRpdmUuDQoNCkdvb2QgcG9pbnQuDQpGYWlyZV9zaGFyZSBnb3Zlcm5vciAgYWx3 YXlzIHJlLWNhbGN1bGF0ZSB0aGUgY29vbGluZyBzdGF0ZSBvZiBldmVyeSB0aGVybWFsIGluc3Rh bmNlcy4NClVzZXJfc3BhY2UgZ292ZXJub3IgZG9lcyBub3QgY2FyZSBhYm91dCB0aGVybWFsIGlu c3RhbmNlIGF0IGFsbC4NCkJhbmdfYmFuZyBnb3Zlcm5vciBhY3R1YWxseSB1c2VzIFRIRVJNQUxf Tk9fVEFSR0VUIGFzIHRoZSBzaWduIG9mIGEgInVuaW5pdGlhbGl6ZWQiIHRoZXJtYWwgaW5zdGFu Y2UuIEkgY2FuIG1vZGlmeSBiYW5nX2JhbmcgZ292ZXJub3IgYXMgd2VsbCBpZiB5b3UgdGhpbmsg dGhpcyBpcyBjb25mdXNpbmcuDQoNClRoYW5rcywNCnJ1aQ0KPiBUaGUgb25seSBuYW1lIEkgY2Fu IHRoaW5rIG9mIGlzICJub3RfeWV0X2V2YWx1YXRlZF9ieV9zdGVwX3dpc2UiIHdoaWNoIGlzDQo+ IHJpZGljdWxvdXNseSBsb25nIG5hbWUgZm9yIGEgdmFyaWFibGUsIGJ1dCBjYW4gd2UgZ2V0IHNv bWV0aGluZyBtb3JlIGRlc2NyaXB0aXZlDQo+IHRoYW4gImluaXRpYWxpemVkIj8NCg0KDQo+IA0K PiBDaGVlcnMsDQo+IEphdmkNCg0K -- 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 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..fb96b15 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) +/* 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)