Hello Henrik Grimler,

-----Original Message-----
From: Henrik Grimler [mailto:henrik@grimler.se]
Sent: Thursday, September 4, 2025 5:38 PM
To: Shin Son <redacted>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>; Krzysztof Kozlowski
[off-list ref]; Rafael J . Wysocki [off-list ref]; Daniel Lezcano
[off-list ref]; Zhang Rui [off-list ref]; Lukasz Luba
[off-list ref]; Rob Herring [off-list ref]; Conor Dooley
[off-list ref]; Alim Akhtar [off-list ref]; linux-
pm@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/3] thermal: exynos_tmu: Support new hardware and
update TMU interface

Hi Shin,

On Wed, Sep 03, 2025 at 04:36:33PM +0900, Shin Son wrote:

quoted

The Exynos tmu driver's private data structure has been extended to
support the exynosautov920 hardware, which requires per-sensor
interrupt enablement and dual-zone handling:

- Add 'slope_comp' : compensation parameter below 25 degrees.
- Add 'calib_temp' : stores the fused calibaration temperature.
- Add 'tz_count' : reflects the new 1:2 hardware-to-thermal-zone ratio.
- Add 'valid_sensor_bitmap' : bitmap to enable interrupts
			      for each valid sensor.
- Rename 'tzd' -> 'tzd_array' to register multiple thermal zones.

Since splitting this patch causes runtime errors during temperature
emulation or problems where the read temperature feature fails to
retrieve values, I have submitted it as a single commit. To add
support for the exynosautov920 to the exisiting TMU interface, the
following changes are included:

1. Branch 'code_to_temp' and 'temp_to_code' for exynosautov920 SoC

variant.

quoted

2. Loop over 'tz_count' in critical-point setup.
3. Introduce 'update_con_reg' for exynosautov920 control-register

updates.

quoted

4. Add exynosautov920-specific branch in 'exynos_tmu_update_temp'

function.

quoted

5. Skip high & low temperature threshold setup in exynosautov920.
6. Enable interrupts via bitmap in 'exynosautov920_tmu_set_crit_temp'.
7. Initialize all new members during 'exynosautov920_tmu_initialize'.
8. Clear IRQs by iterating the bitamp in exynosautov920.
9. Register each zone with 'devm_thermal_of_zone_register()'
   based on 'tz_count'.

Signed-off-by: Shin Son <redacted>
---
 drivers/thermal/samsung/exynos_tmu.c | 340
++++++++++++++++++++++++---
 1 file changed, 303 insertions(+), 37 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c

b/drivers/thermal/samsung/exynos_tmu.c
index 47a99b3c5395..60d5ab33c593 100644

--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c

[ ... ]

quoted

+#define EXYNOSAUTOV920_TMU_REG_THRESHOLD(p)	(((p)) * 0x50 +

0x00D0)

quoted

+#define EXYNOSAUTOV920_TMU_REG_INTEN(p)		(((p)) * 0x50 +

0x00F0)

quoted

+#define EXYNOSAUTOV920_TMU_REG_INT_PEND(p)	(((p)) * 0x50 + 0x00F8)
+
+#define EXYNOSAUTOV920_CURRENT_TEMP_P1_P0	0x084
+#define EXYNOSAUTOV920_TMU_REG_EMUL_CON		0x0B0
+
+#define EXYNOSAUTOV920_TMU_REG_CONTROL		0x50
+#define EXYNOSAUTOV920_TMU_REG_CONTROL1		0x54
+#define EXYNOSAUTOV920_TMU_REG_AVG_CONTROL	0x58
+#define EXYNOSAUTOV920_TMU_SAMPLING_INTERVAL	0x70
+#define EXYNOSAUTOV920_TMU_REG_COUNTER_VALUE0	0x74
+#define EXYNOSAUTOV920_TMU_REG_COUNTER_VALUE1	0x78
+
+#define EXYNOSAUTOV920_TMU_THERM_TRIP_EN_SHIFT	12

There already is a EXYNOS_TMU_THERM_TRIP_EN_SHIFT constant with the same
value. Is there some fundamental difference between
EXYNOSAUTOV920_TMU_THERM_TRIP_EN_SHIFT and EXYNOS_TMU_THERM_TRIP_EN_SHIFT?

quoted

+#define EXYNOSAUTOV920_TMU_T_BUF_VREF_SEL_SHIFT		8
+#define EXYNOSAUTOV920_TMU_T_BUF_VREF_SEL_MASK		0x1f
+#define EXYNOSAUTOV920_TMU_T_BUF_SLOPE_SEL_SHIFT	3
+#define EXYNOSAUTOV920_TMU_T_BUF_SLOPE_SEL_MASK		0xf
+#define EXYNOSAUTOV920_TMU_NUM_PROBE_MASK		0xf
+#define EXYNOSAUTOV920_TMU_NUM_PROBE_SHIFT		16
+#define EXYNOSAUTOV920_TMU_LPI_MODE_MASK		1
+#define EXYNOSAUTOV920_TMU_LPI_MODE_SHIFT		10
+
+#define EXYNOSAUTOV920_TMU_AVG_CON_UPDATE		0x0008011A
+#define EXYNOSAUTOV920_TMU_COUNTER_VALUE0_UPDATE	0x030003C0
+#define EXYNOSAUTOV920_TMU_COUNTER_VALUE1_UPDATE	0x03C0004D

If I am not mistaken lowercase letters is preferred in defines. The file
already has a mix, but let's not make it worse. Please change to
0x03c0004d and so on in constants above.

quoted

 #define MCELSIUS	1000

+#define EXYNOS_DEFAULT_TZ_COUNT		1
+#define EXYNOS_MAX_TZ_COUNT		2
+
 enum soc_type {
 	SOC_ARCH_EXYNOS3250 = 1,
 	SOC_ARCH_EXYNOS4210,

@@ -133,6 +179,7 @@ enum soc_type {
 	SOC_ARCH_EXYNOS5420_TRIMINFO,
 	SOC_ARCH_EXYNOS5433,
 	SOC_ARCH_EXYNOS7,
+	SOC_ARCH_EXYNOSAUTOV920,
 };

 /**

@@ -150,6 +197,8 @@ enum soc_type {
  * @efuse_value: SoC defined fuse value
  * @min_efuse_value: minimum valid trimming data
  * @max_efuse_value: maximum valid trimming data
+ * @slope_comp: allocated value of the slope compensation.
+ * @calib_temp: calibration temperature of the TMU.
  * @temp_error1: fused value of the first point trim.
  * @temp_error2: fused value of the second point trim.
  * @gain: gain of amplifier in the positive-TC generator block @@
-157,7 +206,9 @@ enum soc_type {
  * @reference_voltage: reference voltage of amplifier
  *	in the positive-TC generator block
  *	0 < reference_voltage <= 31
- * @tzd: pointer to thermal_zone_device structure
+ * @tz_count: The allocated number of the thermal zone
+ * @tzd_array: pointer array of thermal_zone_device structure
+ * @valid_sensor_bitmap: The enabled sensor of the TMU device
  * @enabled: current status of TMU device
  * @tmu_set_low_temp: SoC specific method to set trip (falling

threshold)

quoted

  * @tmu_set_high_temp: SoC specific method to set trip (rising
threshold) @@ -181,10 +232,14 @@ struct exynos_tmu_data {
 	u32 efuse_value;
 	u32 min_efuse_value;
 	u32 max_efuse_value;
+	u16 slope_comp;
+	u16 calib_temp;
 	u16 temp_error1, temp_error2;
 	u8 gain;
 	u8 reference_voltage;
-	struct thermal_zone_device *tzd;
+	u8 tz_count;
+	unsigned long valid_sensor_bitmap;
+	struct thermal_zone_device *tzd_array[EXYNOS_MAX_TZ_COUNT];
 	bool enabled;

 	void (*tmu_set_low_temp)(struct exynos_tmu_data *data, u8 temp); @@
-208,10 +263,25 @@ static int temp_to_code(struct exynos_tmu_data *data,

u8 temp)

quoted

 	if (data->cal_type == TYPE_ONE_POINT_TRIMMING)
 		return temp + data->temp_error1 - EXYNOS_FIRST_POINT_TRIM;

-	return (temp - EXYNOS_FIRST_POINT_TRIM) *
-		(data->temp_error2 - data->temp_error1) /
-		(EXYNOS_SECOND_POINT_TRIM - EXYNOS_FIRST_POINT_TRIM) +
-		data->temp_error1;
+	if (data->soc == SOC_ARCH_EXYNOSAUTOV920) {
+		if ((temp - EXYNOS_FIRST_POINT_TRIM) >= 0) {
+			return (temp - EXYNOS_FIRST_POINT_TRIM) *
+				(data->temp_error2 - data->temp_error1) /
+				(data->calib_temp -

EXYNOS_FIRST_POINT_TRIM) +

quoted

+				data->temp_error1;
+		} else {
+			return ((temp - EXYNOS_FIRST_POINT_TRIM) *
+				(data->temp_error2 - data->temp_error1) /
+				(data->calib_temp -

EXYNOS_FIRST_POINT_TRIM) *

quoted

+				((57 + data->slope_comp) * 1000 / 65)) /

1000 +

quoted

+				data->temp_error1;
+		}
+	} else {
+		return (temp - EXYNOS_FIRST_POINT_TRIM) *
+			(data->temp_error2 - data->temp_error1) /
+			(EXYNOS_SECOND_POINT_TRIM -

EXYNOS_FIRST_POINT_TRIM) +

quoted

+			data->temp_error1;

This is essentially the same as the first return in the
SOC_ARCH_EXYNOSAUTOV920 path. How about putting EXYNOS_SECOND_POINT_TRIM
in the calib_temp field for the non autov920 SoCs, then we can simplify
temp_to_code and code_to_temp to something more readable like:

static int temp_to_code(struct exynos_tmu_data *data, u8 temp) {
	if (data->cal_type == TYPE_ONE_POINT_TRIMMING)
		return temp + data->temp_error1 - EXYNOS_FIRST_POINT_TRIM;

	int coeff = (data->temp_error2 - data->temp_error1) /
			(data->calib_temp - EXYNOS_FIRST_POINT_TRIM);

	if (data->soc == SOC_ARCH_EXYNOSAUTOV920 &&
	    temp < EXYNOS_FIRST_POINT_TRIM)
		coeff *= (57 + data->slope_comp) * 1000 / 65)) / 1000;

	return (temp - EXYNOS_FIRST_POINT_TRIM) * coeff + data-

quoted

temp_error1; }

Thanks for your advice. I will reflect it and revise the code accordingly.

quoted

 }

 /*

@@ -223,10 +293,25 @@ static int code_to_temp(struct exynos_tmu_data

*data, u16 temp_code)

quoted

 	if (data->cal_type == TYPE_ONE_POINT_TRIMMING)
 		return temp_code - data->temp_error1 +

EXYNOS_FIRST_POINT_TRIM;

quoted

-	return (temp_code - data->temp_error1) *
-		(EXYNOS_SECOND_POINT_TRIM - EXYNOS_FIRST_POINT_TRIM) /
-		(data->temp_error2 - data->temp_error1) +
-		EXYNOS_FIRST_POINT_TRIM;
+	if (data->soc == SOC_ARCH_EXYNOSAUTOV920) {
+		if ((temp_code - data->temp_error1) >= 0) {
+			return (temp_code - data->temp_error1) *
+				(data->calib_temp -

EXYNOS_FIRST_POINT_TRIM) /

quoted

+				(data->temp_error2 - data->temp_error1) +
+				EXYNOS_FIRST_POINT_TRIM;
+		} else {
+			return ((temp_code - data->temp_error1) *
+				(data->calib_temp -

EXYNOS_FIRST_POINT_TRIM) /

quoted

+				(data->temp_error2 - data->temp_error1) *
+				(65 * 1000 / (57 + data->slope_comp))) /

1000 +

quoted

+				EXYNOS_FIRST_POINT_TRIM;
+		}
+	} else {
+		return (temp_code - data->temp_error1) *
+			(EXYNOS_SECOND_POINT_TRIM -

EXYNOS_FIRST_POINT_TRIM) /

quoted

+			(data->temp_error2 - data->temp_error1) +
+			EXYNOS_FIRST_POINT_TRIM;
+	}

Similar suggestion as for temp_to_code applies here as well.

Best regards,
Henrik Grimler

Thanks,
Best regards
Shin son