Re: [PATCH V4 1/3] Thermal: initialize thermal zone device correctly

[Javi Merino <javi.merino@arm.com>]

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