Re: [PATCH] thermal: core: ignore invalid trip temperature

[Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>]

On Fri, 2014-11-14 at 12:50 +0100, Lukasz Majewski wrote:
> Hi Srinivas,
> 
> > Hi Lukasz,
> > 
> > Thanks for your review and comment.
> > 
> > On Thu, 2014-11-13 at 10:23 +0100, Lukasz Majewski wrote:
> > > Hi Srinivas,
> > > 
> > > > Ignore invalid trip temperature less or equal to zero. Some
> > > > buggy systems have invalid trips, causing system shutdown.
> > > > 
> > > > Signed-off-by: Srinivas Pandruvada
> > > > <srinivas.pandruvada@linux.intel.com> Acked-by: Zhang Rui
> > > > <rui.zhang@intel.com> ---
> > > >  drivers/thermal/thermal_core.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/thermal/thermal_core.c
> > > > b/drivers/thermal/thermal_core.c index 9bf10aa..fbf301a 100644
> > > > --- a/drivers/thermal/thermal_core.c
> > > > +++ b/drivers/thermal/thermal_core.c
> > > > @@ -368,7 +368,7 @@ static void handle_critical_trips(struct
> > > > thermal_zone_device *tz, tz->ops->get_trip_temp(tz, trip,
> > > > &trip_temp); 
> > > >  	/* If we have not crossed the trip_temp, we do not care.
> > > > */
> > > > -	if (tz->temperature < trip_temp)
> > > > +	if (trip_temp <= 0 || tz->temperature < trip_temp)
> > > 
> > > To be honest, I regard this as a feature :-), not bug.
> > 
> > I am not saying bug is in the code, saying buggy system with invalid
> > configuration of critical trip.
> > 
> > > In this way I know that on some systems the regulator for thermal
> > > unit is not enabled (which results in read temp of 0xFF).
> > > 
> > So are you saying that trip temp of <= 0 is a valid trip in your
> > system?
> 
> No. Please find below explanation.
> 
> I have 4 available trip points in my system (e.g. Eynos4/5). The last
> trip point is critical (e.g. 120 C degrees).
So this change will still work for you, since your trip > 0.
> 
> When everything is properly configured (i.e. power supply is provided
> to thermal unit, clocks are setup) and we pass the critical trip point
> then interrupt is triggered and my board is shut down unconditionally by
> internal SoC's power management IP block.
> 
> However, when something is misconfigured, wrong value of temperature is
> read (max possible value - 0xFF) and passed to the driver.
> 
> In such a situation, I see that something is wrong, since I cannot fully
> boot up my system.
> 
> I don't mind the patch, but please pay a note that on some systems
> value of -1 is also expected.
If critical trip is -1, then this change will simply return as no need
to evaluate temp for trip for a given temp change.

I am forwarding this to linux-pm list, let me see if others have a
problem.

Thanks,
Srinivas

> 
> > 
> > Thanks,
> > Srinivas
> > 
> > > I'd prefer to keep this as is.
> > > 
> > > >  		return;
> > > >  
> > > >  	trace_thermal_zone_trip(tz, trip, trip_type);
> > > 
> > > 
> > > 
> > 
> > 
> 
> 
>