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] [day] [month] [year] [list]
Message-ID: <0b37e5e1-6dd6-49fc-b874-741e75c8d56a@linaro.org>
Date: Tue, 11 Feb 2025 09:08:06 +0100
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Dragan Simic <dsimic@...jaro.org>, Trevor Woerner <twoerner@...il.com>
Cc: linux-kernel@...r.kernel.org, "Rafael J. Wysocki" <rafael@...nel.org>,
 Zhang Rui <rui.zhang@...el.com>, Lukasz Luba <lukasz.luba@....com>,
 Heiko Stuebner <heiko@...ech.de>, Caesar Wang <wxt@...k-chips.com>,
 Rocky Hao <rocky.hao@...k-chips.com>, linux-pm@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 stable@...r.kernel.org
Subject: Re: [PATCH v2] thermal/drivers/rockchip: add missing rk3328 mapping
 entry

On 11/02/2025 02:40, Dragan Simic wrote:
> Hello Trevor,
> 
> On 2025-02-07 18:50, Trevor Woerner wrote:
>> The mapping table for the rk3328 is missing the entry for -25C which is
>> found in the TRM section 9.5.2 "Temperature-to-code mapping".
>>
>> NOTE: the kernel uses the tsadc_q_sel=1'b1 mode which is defined as:
>>       4096-<code in table>. Whereas the table in the TRM gives the code
>>       "3774" for -25C, the kernel uses 4096-3774=322.
> 
> After going through the RK3308 and RK3328 TRMs, as well as through the
> downstream kernel code, it seems we may have some troubles at our hands.
> Let me explain, please.
> 
> To sum it up, part 1 of the RK3308 TRM v1.1 says on page 538 that the
> equation for the output when tsadc_q_sel equals 1 is (4096 - tsadc_q),
> while part 1 of the RK3328 TRM v1.2 says that the output equation is
> (1024 - tsadc_q) in that case.
> 
> The downstream kernel code, however, treats the RK3308 and RK3328
> tables and their values as being the same.  It even mentions 1024 as
> the "offset" value in a comment block for the rk_tsadcv3_control()
> function, just like the upstream code does, which is obviously wrong
> "offset" value when correlated with the table on page 544 of part 1
> of the RK3308 TRM v1.1.
> 
> With all this in mind, it's obvious that more work is needed to make
> it clear where's the actual mistake (it could be that the TRM is wrong),
> which I'll volunteer for as part of the SoC binning project.  In the
> meantime, this patch looks fine as-is to me, by offering what's a clear
> improvement to the current state of the upstream code, so please feel
> free to include:
> 
> Reviewed-by: Dragan Simic <dsimic@...jaro.org>
> 
> However, it would be good to include some additional notes into the
> patch description in the v3, which would briefly sum up the above-
> described issues and discrepancies, for future reference.


Applied and added the additional notes in the patch description.

Thanks

   -- D.


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ