Re: [PATCH v4 2/3] thermal: exynos_tmu: Support new hardware and update TMU interface

[Henrik Grimler <henrik@grimler.se>]

Hi Shin,

On Tue, Sep 23, 2025 at 09:53:53AM +0900, �ս� wrote:
> Hello Henrik Grimler
> 
> > -----Original Message-----
> > From: Henrik Grimler [mailto:henrik@grimler.se]
> > Sent: Tuesday, September 23, 2025 5:05 AM
> > To: Shin Son <shin.son@samsung.com>
> > Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>; Krzysztof Kozlowski
> > <krzk@kernel.org>; Rafael J . Wysocki <rafael@kernel.org>; Daniel Lezcano
> > <daniel.lezcano@linaro.org>; Zhang Rui <rui.zhang@intel.com>; Lukasz Luba
> > <lukasz.luba@arm.com>; Rob Herring <robh@kernel.org>; Conor Dooley
> > <conor+dt@kernel.org>; Alim Akhtar <alim.akhtar@samsung.com>; 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 v4 2/3] thermal: exynos_tmu: Support new hardware and
> > update TMU interface
> > 
> > Hi Shin,
> > 
> > On Mon, Sep 22, 2025 at 01:18:56PM +0900, Shin Son wrote:
> > > The Exynos tmu driver's private data structure has been extended to
> > > support the exynosautov920 hardware, which requires per-sensor
> > > interrupt enablement and multiple-zone handling:
> > >
> > > - Add 'slope_comp' : compensation parameter below 25 degrees.
> > > - Add 'calib_temp' : stores the fused calibaration temperature.
> > > - Add 'sensor_count' : reflects the maximum sensor numbers.
> > > - 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. Simplify "temp_to_code" and "code_to_temp" to one computation path
> > >    by normalizing calib_temp.
> > > 2. Loop over 'sensor_count' in critical-point setup.
> > > 3. Introduce 'update_con_reg' for exynosautov920 control-register
> > updates.
> > > 4. Add exynosautov920-specific branch in 'exynos_tmu_update_temp'
> > function.
> > > 5. Skip high & low temperature threshold setup in exynosautov920.
> > > 6. Enable interrupts via sensor_count in exynosautov920.
> > > 7. Initialize all new members during 'exynosautov920_tmu_initialize'.
> > > 8. Clear IRQs by iterating the sensor_count in exynosautov920.
> > > 9. Register each zone with 'devm_thermal_of_zone_register()'
> > >    based on 'sensor_count'.
> > >
> > > Signed-off-by: Shin Son <shin.son@samsung.com>

[ ... ]

> > > @@ -952,6 +1183,14 @@ static int exynos_map_dt_data(struct
> > > platform_device *pdev)
> > >
> > >  	data->cal_type = TYPE_ONE_POINT_TRIMMING;
> > >
> > > +	if (data->soc == SOC_ARCH_EXYNOSAUTOV920) {
> > > +		if (of_property_read_u32(pdev->dev.of_node,
> > "samsung,sensors",
> > > +					 &data->sensor_count)) {
> > > +			dev_err(&pdev->dev, "failed to get sensor count\n");
> > > +			return -ENODEV;
> > > +		}
> > > +	}
> > 
> > Do we really need the `if (data->soc == SOC_ARCH_EXYNOSAUTOV920)` here, I
> > am sure there will be more socs that use samsung,sensors. Can't we simply
> > read samsung,sensors for all socs and use EXYNOS_DEFAULT_SENSOR_COUNT if
> > it fails, or would it be potentially dangerous if samsung,sensors is
> > missing for autov920 dtb and default value of 1 is used?
> > 
> > Best regards,
> > Henrik Grimler
> > 
> 
> Yes. Incorrect remote-sensor settings can affect TMU operation. For
> example, when the sensor count is set to 1,
> The thermal zone doesn't function properly and the hardware trip doesn't
> assert on the v920 variant.
> I consider that configuration unsafe, so I added variant-specific handling
> for that SoC.
> Meanwhile, the other variant legitimately uses only a single sensor.

I see, thanks for the explanation!

Best regards,
Henrik Grimler