linux-kernel - Re: [PATCH v7 RESEND 2/3] thermal: exynos_tmu: Support new hardware and update TMU interface

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: <12346382-7718-4942-a497-4de278b1d5a0@linaro.org>
Date: Mon, 24 Nov 2025 12:43:22 +0200
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: 손신 <shin.son@...sung.com>,
 'Daniel Lezcano' <daniel.lezcano@...aro.org>,
 'Bartlomiej Zolnierkiewicz' <bzolnier@...il.com>,
 'Krzysztof Kozlowski' <krzk@...nel.org>,
 "'Rafael J . Wysocki'" <rafael@...nel.org>, 'Zhang Rui'
 <rui.zhang@...el.com>, 'Lukasz Luba' <lukasz.luba@....com>,
 'Rob Herring' <robh@...nel.org>, 'Conor Dooley' <conor+dt@...nel.org>,
 'Alim Akhtar' <alim.akhtar@...sung.com>
Cc: 'Henrik Grimler' <henrik@...mler.se>, linux-pm@...r.kernel.org,
 linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Peter Griffin <peter.griffin@...aro.org>,
 André Draszik <andre.draszik@...aro.org>,
 William McVicker <willmcvicker@...gle.com>, jyescas@...gle.com
Subject: Re: [PATCH v7 RESEND 2/3] thermal: exynos_tmu: Support new hardware
 and update TMU interface

Hi, Shin,

On 11/24/25 12:06 PM, 손신 wrote:
>>> +static void update_con_reg(struct exynos_tmu_data *data) {
>>> +	u32 val, t_buf_vref_sel, t_buf_slope_sel;
>>> +
>>> +	val = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
>>> +	t_buf_vref_sel = (val >> EXYNOSAUTOV920_TMU_T_BUF_VREF_SEL_SHIFT)
>>> +				& EXYNOSAUTOV920_TMU_T_BUF_VREF_SEL_MASK;
>>> +	t_buf_slope_sel = (val >> EXYNOSAUTOV920_TMU_T_BUF_SLOPE_SEL_SHIFT)
>>> +				& EXYNOSAUTOV920_TMU_T_BUF_SLOPE_SEL_MASK;
>>> +
>>> +	val = readl(data->base +  EXYNOSAUTOV920_TMU_REG_CONTROL);
>>> +	val &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK <<
>> EXYNOS_TMU_REF_VOLTAGE_SHIFT);
>>> +	val |= (t_buf_vref_sel << EXYNOS_TMU_REF_VOLTAGE_SHIFT);
>>> +	val &= ~(EXYNOS_TMU_BUF_SLOPE_SEL_MASK <<
>> EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT);
>>> +	val |= (t_buf_slope_sel << EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT);
>>> +	writel(val, data->base + EXYNOSAUTOV920_TMU_REG_CONTROL);
>>> +
>>> +	val = readl(data->base + EXYNOSAUTOV920_TMU_REG_CONTROL1);
>>> +	val &= ~(EXYNOSAUTOV920_TMU_NUM_PROBE_MASK <<
>> EXYNOSAUTOV920_TMU_NUM_PROBE_SHIFT);
>>> +	val &= ~(EXYNOSAUTOV920_TMU_LPI_MODE_MASK <<
>> EXYNOSAUTOV920_TMU_LPI_MODE_SHIFT);
>>> +	val |= (data->sensor_count << EXYNOSAUTOV920_TMU_NUM_PROBE_SHIFT);
>>> +	writel(val, data->base + EXYNOSAUTOV920_TMU_REG_CONTROL1);
>>> +
>>> +	writel(1, data->base + EXYNOSAUTOV920_TMU_SAMPLING_INTERVAL);
>>> +	writel(EXYNOSAUTOV920_TMU_AVG_CON_UPDATE, data->base +
>> EXYNOSAUTOV920_TMU_REG_AVG_CONTROL);
>>> +	writel(EXYNOSAUTOV920_TMU_COUNTER_VALUE0_UPDATE,
>>> +	       data->base + EXYNOSAUTOV920_TMU_REG_COUNTER_VALUE0);
>>> +	writel(EXYNOSAUTOV920_TMU_COUNTER_VALUE1_UPDATE,
>>> +	       data->base + EXYNOSAUTOV920_TMU_REG_COUNTER_VALUE1);
>>> +}
>>> +
>> This is unreadable; please make it understandable for those who don’t have
>> the documentation (explicit static inline functions, comments, etc ...).
> I'll restructure this code by introducing explicit static inline helper functions and proper comments to improve readability.

We likely shouldn't use inlines here, see Linus's reply from 2006:
https://lore.kernel.org/all/Pine.LNX.4.64.0601021105000.3668@g5.osdl.org/T/#u

I guess you can make this easier to read if you use FIELD_GET/SET from
bitfield.h. Other improvement would be using the regmap api.

Shin, a bit unrelated with the patch, but I wanted to let you know that
I started looking at the GS101 TMU. I assume it's very similar with the 
TMU on exynosautov920. Do you know if they share the same IP version?

I noticed GS101 uses ACPM calls to communicate with the TMU. Why did you
choose to not use ACPM for exynosautov920 TMU?

Thanks!
ta