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

Open Source and information security mailing list archives

Message-ID: <b2736d4e-120e-cbb9-3371-838c8f36c752@rock-chips.com>
Date:   Wed, 23 Nov 2016 11:03:33 +0800
From:   Caesar Wang <wxt@...k-chips.com>
To:     Brian Norris <briannorris@...omium.org>
Cc:     Caesar Wang <wxt@...k-chips.com>, heiko@...ech.de,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        smbarber@...omium.org, edubezval@...il.com,
        linux-rockchip@...ts.infradead.org, rui.zhang@...el.com
Subject: Re: [PATCH 3/5] thermal: rockchip: fixes invalid temperature case

在 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@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives