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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161123023325.GB122654@google.com>
Date:   Tue, 22 Nov 2016 18:33:26 -0800
From:   Brian Norris <briannorris@...omium.org>
To:     Caesar Wang <wxt@...k-chips.com>
Cc:     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

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