Re: [PATCH] thermal: power_allocator: Use temperature reading from tz

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

On Wed, Oct 14, 2015 at 05:28:49AM +0000, Chen, Yu C wrote:
> > -----Original Message-----
> > From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> > owner@vger.kernel.org] On Behalf Of Kapileshwar Singh
> > Sent: Tuesday, October 13, 2015 7:30 PM
> > To: linux-pm@vger.kernel.org
> > Cc: Kapileshwar Singh; Javi Merino; Eduardo Valentin; Daniel Kurtz; Zhang, Rui;
> > Dmitry Torokhov; Sascha Hauer; Andrea Arcangeli
> > Subject: [PATCH] thermal: power_allocator: Use temperature reading from
> > tz
> > 
> > All thermal governors use the temperature value stored in struct
> > thermal_zone_device.
> > 
> >    thermal_zone_device->temperature
> > 
> > power_allocator governor should not deviate from this and use the same.
> > 
> Just my 2 cents:
> I wonder if tz->temperature would vary during power_allocator_throttle? 
> because we don't have tz->lock to protect here.

True, tz->temperature could vary but I don't think it's problematic.
tz->temperature changing would mean that it doesn't pass this condition:

	if (!ret && (tz->temperature < switch_on_temp)) {

and then, the temperature changes to a value below switch_on_temp
before the call to allocate_power().  allocate_power() would still
work and make an appropriate decision.

All calls inside allocate_power() are protected by tz->lock, so the
temperature used in pid_controller() is the same as the one reported
to ftrace.

In summary, I don't think it has any impact on functionality.  Thanks
a lot for the review,
Javi