Re: [PATCH 3/5] thermal: rockchip: fixes invalid temperature case

[Brian Norris]

On Wed, Nov 23, 2016 at 11:03:33AM +0800, Caesar Wang wrote:
> å 2016å11æ23æ 10:33, Brian Norris åé:
> >IIUC, "too high" should not be interpreted as TSADCV2_DATA_MASK on
> >rk3288, should it? That corresponds to -40C, which means you'll be
> >triggering the alarm temperature at a very *low* temperature, not a very
> >high one, no?
> 
> The "too high" will correspond to -40C on rk3288, but shouldn't
> trigger the alarm temperature.
> 
> Due to the alarm or tshut function will handle it.
> 
> e.g.:
> static void rk_tsadcv2_alarm_temp(const struct chip_tsadc_table *table,
>                   int chn, void __iomem *regs, int temp)
> {
>     u32 alarm_value, int_en;
> 
>     /* Make sure the value is valid */
>     alarm_value = rk_tsadcv2_temp_to_code(table, temp);
>     if (alarm_value == table->data_mask)
>         return;

Ah, right. I keep forgetting about this odd error handling.

That's still the wrong error handling though; the right response is
never to avoid doing anything (and therefore returning "success" to the
thermal core). You need to either program a high (or low) trip value, or
else report an error (i.e., allow rk_tsadcv2_alarm_temp() to return an
error code back to the calling function). Otherwise, this:

echo -45000 > trip_0_temp

will succeed without error, and:

cat trip_0_temp
-45000

will return the cached temperature from of-thermal, even though the trip
point is programmed to something else entirely.

Brian