[<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