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

[Caesar Wang <wxt@rock-chips.com>]

在 2016年11月23日 10:33, 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?

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;
....
}
or
static void rk_tsadcv2_tshut_temp(const struct chip_tsadc_table *table,
                   int chn, void __iomem *regs, int temp)
{
     u32 tshut_value, val;

     /* Make sure the value is valid */
     tshut_value = rk_tsadcv2_temp_to_code(table, temp);
     if (tshut_value == table->data_mask)
         return;
...
}

>
> Brian
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip