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

[Brian Norris]

On Wed, Nov 23, 2016 at 10:06:15AM +0800, Caesar Wang wrote:
> å 2016å11æ23æ 05:52, Brian Norris åé:
> >On Tue, Nov 22, 2016 at 12:57:37PM -0800, Brian Norris wrote:

> >>>+	if (temp < table->id[low].temp || temp > table->id[high].temp)
> >>>  		goto exit;
> >I was revisiting the logic here though, and I don't understand your
> >error case. You're treating "too low" and "too high" the same, and in
> >either case, you're choosing a value of ->data_mask. That doesn't make
> >sense to me, especially for ADC_DECREMENT cases like rk3288. In that
> >case, you're programming the trip to the lowest possible temperature.
> 
> I admit that's not perfect, but that should conform to reality.
> 
> Whichever is the adc value, 12it or 10bit.
> #define TSADCV2_DATA_MASK            0xfff
> #define TSADCV3_DATA_MASK            0x3ff
> 
> The "too low" and "too high" are same, that should indicate that temperature is
> invalid or over table range.
> 
> The currect code will return the max analog value to warn it.
> ---
> 
> The temperature {-40C, 125C} is for rockchip SoCs, that should be
> similar with real world's temperature {-INT_MAX, INT_MAX}.

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?

Brian