linux-kernel - Re: [PATCH v3 3/6] drivers/thermal/exynos: improve sanitize_temp

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: <2c7f43b1-48e7-48f2-bbe2-c0006ac6e0e7@linaro.org>
Date: Fri, 6 Sep 2024 12:04:14 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Mateusz Majewski <m.majewski2@...sung.com>
Cc: 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, 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>,
 Sam Protsenko <semen.protsenko@...aro.org>,
 Anand Moon <linux.amoon@...il.com>
Subject: Re: [PATCH v3 3/6] drivers/thermal/exynos: improve
 sanitize_temp_error

On 03/09/2024 09:19, Mateusz Majewski wrote:
> Hello :)
> 
>> May I suggest to convert this function to a specific soc ops to be put
>> in the struct exynos_tmu_data ?
> 
> Like this?
> 
> static void exynos4210_sanitize_temp_error(struct exynos_tmu_data *data,
> 					   u32 trim_info)
> {
> 	data->temp_error1 = trim_info & EXYNOS_TMU_TEMP_MASK;
> 	if (!data->temp_error1 ||
> 	    (data->min_efuse_value > data->temp_error1) ||
> 	    (data->temp_error1 > data->max_efuse_value))
> 		data->temp_error1 = data->efuse_value & EXYNOS_TMU_TEMP_MASK;
> 	WARN_ON_ONCE(data->cal_type == TYPE_TWO_POINT_TRIMMING);
> }
> 
> static void exynos5433_sanitize_temp_error(struct exynos_tmu_data *data,
> 					   u32 trim_info)
> {
> 	data->temp_error1 = trim_info & EXYNOS_TMU_TEMP_MASK;
> 	if (!data->temp_error1 ||
> 	    (data->min_efuse_value > data->temp_error1) ||
> 	    (data->temp_error1 > data->max_efuse_value))
> 		data->temp_error1 = data->efuse_value & EXYNOS_TMU_TEMP_MASK;
> 
> 	if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
> 		data->temp_error2 = (trim_info >> EXYNOS_TRIMINFO_85_SHIFT) &
> 				    EXYNOS_TMU_TEMP_MASK;
> 		if (!data->temp_error2)
> 			data->temp_error2 = (data->efuse_value >>
> 					     EXYNOS_TRIMINFO_85_SHIFT) &
> 					    EXYNOS_TMU_TEMP_MASK;
> 	}
> }
> 
> static void exynos7_sanitize_temp_error(struct exynos_tmu_data *data,
> 					u32 trim_info)
> {
> 	data->temp_error1 = trim_info & EXYNOS7_TMU_TEMP_MASK;
> 	if (!data->temp_error1 ||
> 	    (data->min_efuse_value > data->temp_error1) ||
> 	    (data->temp_error1 > data->max_efuse_value))
> 		data->temp_error1 = data->efuse_value & EXYNOS7_TMU_TEMP_MASK;
> 	WARN_ON_ONCE(data->cal_type == TYPE_TWO_POINT_TRIMMING);
> }
> 
> static void exynos850_sanitize_temp_error(struct exynos_tmu_data *data,
> 					   u32 trim_info)
> {
> 	data->temp_error1 = trim_info & EXYNOS7_TMU_TEMP_MASK;
> 	if (!data->temp_error1 ||
> 	    (data->min_efuse_value > data->temp_error1) ||
> 	    (data->temp_error1 > data->max_efuse_value))
> 		data->temp_error1 = data->efuse_value & EXYNOS7_TMU_TEMP_MASK;
> 
> 	if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
> 		data->temp_error2 = (trim_info >> EXYNOS7_TMU_TEMP_SHIFT) &
> 				    EXYNOS7_TMU_TEMP_MASK;
> 		if (!data->temp_error2)
> 			data->temp_error2 = (data->efuse_value >>
> 					     EXYNOS7_TMU_TEMP_SHIFT) &
> 					    EXYNOS_TMU_TEMP_MASK;
> 	}
> }

Yes, something like that but may be with more code factoring, like a 
common routine to pass the temp_mask and the specific chunk of code.

> Or maybe we could put tmu_temp_mask and tmu_85_shift in data instead,
> and have one function like this:

Either way

It would be nice if the code can be commented to explain how the sensor 
works in order to understand what means "sanitize the temp error"

> static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
> {
> 	data->temp_error1 = trim_info & data->tmu_temp_mask;
> 	if (!data->temp_error1 || (data->min_efuse_value > data->temp_error1) ||
> 	    (data->temp_error1 > data->max_efuse_value))
> 		data->temp_error1 = data->efuse_value & data->tmu_temp_mask;
> 
> 	if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
> 		data->temp_error2 = (trim_info >> data->tmu_85_shift) &
> 				    data->tmu_temp_mask;
> 		if (!data->temp_error2)
> 			data->temp_error2 =
> 				(data->efuse_value >> data->tmu_85_shift) &
> 				data->tmu_temp_mask;
> 	}
> }
> 
> Thank you,
> Mateusz


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