linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/7] Exynos Thermal code improvement
@ 2025-08-13 13:09 Anand Moon
  2025-08-13 13:09 ` [PATCH v7 1/7] thermal/drivers/exynos: Refactor clk_sec initialization inside SOC-specific case Anand Moon
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Anand Moon @ 2025-08-13 13:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Rafael J. Wysocki,
	Daniel Lezcano, Zhang Rui, Lukasz Luba, Alim Akhtar,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	open list:SAMSUNG THERMAL DRIVER,
	open list:SAMSUNG THERMAL DRIVER,
	moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES,
	open list,
	open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b
  Cc: Anand Moon, Mateusz Majewski

Hi All,

This patch series is a rework of my previous patch series [1],
where the code changes were not adequately justified.

In this new series, I have improved the commit subject
and commit message to better explain the changes.

v7: Integrated my RFC patch which improves the IRQ framework
    for all the SoC link below.
    [6] https://lore.kernel.org/all/20250616163831.8138-1-linux.amoon@gmail.com/

v6: Add new patch to use devm_clk_get_enabled
    and Fix few typo in subject as suggested by Daniel.
v5: Drop the guard mutex patch
v4: Tried to address Lukasz review comments.

I dont have any Arm64 device the test and verify
Tested on 32 bit Arch Odroid U3 amd XU4 SoC boards.
Build with clang with W=1 enable.

Please sare the feedback on this.

[5] https://lore.kernel.org/all/20250430123306.15072-1-linux.amoon@gmail.com/
[4] https://lore.kernel.org/all/20250410063754.5483-2-linux.amoon@gmail.com/
[3] https://lore.kernel.org/all/20250310143450.8276-2-linux.amoon@gmail.com/
[2] https://lore.kernel.org/all/20250216195850.5352-2-linux.amoon@gmail.com/
[1] https://lore.kernel.org/all/20220515064126.1424-1-linux.amoon@gmail.com/
[0] https://lore.kernel.org/lkml/CANAwSgS=08fVsqn95WHzSF71WTTyD2-=K2C6-BEz0tY0t6A1-g@mail.gmail.com/T/#m77e57120d230d57f34c29e1422d7fc5f5587ac30

Thanks
-Anand

Anand Moon (7):
  thermal/drivers/exynos: Refactor clk_sec initialization inside
    SOC-specific case
  thermal/drivers/exynos: Use devm_clk_get_enabled() helpers
  thermal/drivers/exynos: Remove redundant IS_ERR() checks for clk_sec
    clock
  thermal/drivers/exynos: Fixed the efuse min max value for exynos5422
  thermal/drivers/exynos: Remove unused base_second mapping and
    references
  thermal/drivers/exynos: Handle temperature threshold IRQs with
    SoC-specific mapping
  thermal/drivers/exynos: Refactor IRQ clear logic using SoC-specific
    config

 drivers/thermal/samsung/exynos_tmu.c | 279 ++++++++++++++++-----------
 1 file changed, 163 insertions(+), 116 deletions(-)


base-commit: 8742b2d8935f476449ef37e263bc4da3295c7b58
-- 
2.50.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v7 1/7] thermal/drivers/exynos: Refactor clk_sec initialization inside SOC-specific case
  2025-08-13 13:09 [PATCH v7 0/7] Exynos Thermal code improvement Anand Moon
@ 2025-08-13 13:09 ` Anand Moon
  2025-08-13 13:09 ` [PATCH v7 2/7] thermal/drivers/exynos: Use devm_clk_get_enabled() helpers Anand Moon
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Anand Moon @ 2025-08-13 13:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Rafael J. Wysocki,
	Daniel Lezcano, Zhang Rui, Lukasz Luba, Alim Akhtar,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	open list:SAMSUNG THERMAL DRIVER,
	open list:SAMSUNG THERMAL DRIVER,
	moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES,
	open list,
	open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b
  Cc: Anand Moon, Mateusz Majewski

Refactor the initialization of the clk_sec clock to be inside the
SOC_ARCH_EXYNOS5420_TRIMINFO case. It ensures that the clk_sec clock
is only initialized for the specified SOC and not for other SOCs,
thereby simplifying the code. The clk_sec clock is used by the TMU
for GPU on the Exynos 542x platform.

Removed redundant IS_ERR() checks for the clk_sec clock since error
handling is already managed internally by clk_unprepare() functions.

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v7: None
v6: Add Rb Lukasz and try to address Daniel review coments.
v5: None
v4: Fix the aligment of code clk for clk_prepare in proper if/else block.
    update the commit for clk_sec used.
    checked to goto clean up all the clks are proper.
    drop IS_ERR() check for clk_sec.
v3: improve the commit message.
---
 drivers/thermal/samsung/exynos_tmu.c | 36 +++++++++++++---------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 47a99b3c5395..04517d52afbd 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -1040,26 +1040,26 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 	if (IS_ERR(data->clk))
 		return dev_err_probe(dev, PTR_ERR(data->clk), "Failed to get clock\n");
 
-	data->clk_sec = devm_clk_get(dev, "tmu_triminfo_apbif");
-	if (IS_ERR(data->clk_sec)) {
-		if (data->soc == SOC_ARCH_EXYNOS5420_TRIMINFO)
-			return dev_err_probe(dev, PTR_ERR(data->clk_sec),
-					     "Failed to get triminfo clock\n");
-	} else {
-		ret = clk_prepare(data->clk_sec);
-		if (ret) {
-			dev_err(dev, "Failed to get clock\n");
-			return ret;
-		}
-	}
-
 	ret = clk_prepare(data->clk);
 	if (ret) {
 		dev_err(dev, "Failed to get clock\n");
-		goto err_clk_sec;
+		return ret;
 	}
 
 	switch (data->soc) {
+	case SOC_ARCH_EXYNOS5420_TRIMINFO:
+		data->clk_sec = devm_clk_get(dev, "tmu_triminfo_apbif");
+		if (IS_ERR(data->clk_sec)) {
+			ret = dev_err_probe(dev, PTR_ERR(data->clk_sec),
+					    "Failed to get clk_sec clock\n");
+			goto err_clk;
+		}
+		ret = clk_prepare(data->clk_sec);
+		if (ret) {
+			dev_err(dev, "Failed to prepare clk_sec clock\n");
+			goto err_clk_sec;
+		}
+		break;
 	case SOC_ARCH_EXYNOS5433:
 	case SOC_ARCH_EXYNOS7:
 		data->sclk = devm_clk_get(dev, "tmu_sclk");
@@ -1112,11 +1112,10 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 
 err_sclk:
 	clk_disable_unprepare(data->sclk);
+err_clk_sec:
+	clk_unprepare(data->clk_sec);
 err_clk:
 	clk_unprepare(data->clk);
-err_clk_sec:
-	if (!IS_ERR(data->clk_sec))
-		clk_unprepare(data->clk_sec);
 	return ret;
 }
 
@@ -1128,8 +1127,7 @@ static void exynos_tmu_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(data->sclk);
 	clk_unprepare(data->clk);
-	if (!IS_ERR(data->clk_sec))
-		clk_unprepare(data->clk_sec);
+	clk_unprepare(data->clk_sec);
 }
 
 #ifdef CONFIG_PM_SLEEP
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 2/7] thermal/drivers/exynos: Use devm_clk_get_enabled() helpers
  2025-08-13 13:09 [PATCH v7 0/7] Exynos Thermal code improvement Anand Moon
  2025-08-13 13:09 ` [PATCH v7 1/7] thermal/drivers/exynos: Refactor clk_sec initialization inside SOC-specific case Anand Moon
@ 2025-08-13 13:09 ` Anand Moon
  2025-08-13 13:09 ` [PATCH v7 3/7] thermal/drivers/exynos: Remove redundant IS_ERR() checks for clk_sec clock Anand Moon
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Anand Moon @ 2025-08-13 13:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Rafael J. Wysocki,
	Daniel Lezcano, Zhang Rui, Lukasz Luba, Alim Akhtar,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	open list:SAMSUNG THERMAL DRIVER,
	open list:SAMSUNG THERMAL DRIVER,
	moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES,
	open list,
	open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b
  Cc: Anand Moon, Mateusz Majewski

Use devm_clk_get_enabled() helper instead of calling devm_clk_get() and
then clk_prepare_enable(). It simplifies the error handling and makes
the code more compact.

Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v7: None
v6: New patch as per Daniel request.
---
 drivers/thermal/samsung/exynos_tmu.c | 77 ++++++++--------------------
 1 file changed, 20 insertions(+), 57 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 04517d52afbd..aa0726b33c84 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -1036,65 +1036,41 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	data->clk = devm_clk_get(dev, "tmu_apbif");
+	data->clk = devm_clk_get_enabled(dev, "tmu_apbif");
 	if (IS_ERR(data->clk))
-		return dev_err_probe(dev, PTR_ERR(data->clk), "Failed to get clock\n");
-
-	ret = clk_prepare(data->clk);
-	if (ret) {
-		dev_err(dev, "Failed to get clock\n");
-		return ret;
-	}
-
-	switch (data->soc) {
-	case SOC_ARCH_EXYNOS5420_TRIMINFO:
-		data->clk_sec = devm_clk_get(dev, "tmu_triminfo_apbif");
-		if (IS_ERR(data->clk_sec)) {
-			ret = dev_err_probe(dev, PTR_ERR(data->clk_sec),
-					    "Failed to get clk_sec clock\n");
-			goto err_clk;
-		}
-		ret = clk_prepare(data->clk_sec);
-		if (ret) {
-			dev_err(dev, "Failed to prepare clk_sec clock\n");
-			goto err_clk_sec;
-		}
-		break;
-	case SOC_ARCH_EXYNOS5433:
-	case SOC_ARCH_EXYNOS7:
-		data->sclk = devm_clk_get(dev, "tmu_sclk");
-		if (IS_ERR(data->sclk)) {
-			ret = dev_err_probe(dev, PTR_ERR(data->sclk), "Failed to get sclk\n");
-			goto err_clk;
-		} else {
-			ret = clk_prepare_enable(data->sclk);
-			if (ret) {
-				dev_err(dev, "Failed to enable sclk\n");
-				goto err_clk;
-			}
-		}
-		break;
-	default:
-		break;
+		return dev_err_probe(dev, PTR_ERR(data->clk),
+				     "Failed to get clock\n");
+
+	if (data->soc == SOC_ARCH_EXYNOS5420_TRIMINFO) {
+		data->clk_sec = devm_clk_get_enabled(dev, "tmu_triminfo_apbif");
+		if (IS_ERR(data->clk_sec))
+			return dev_err_probe(dev, PTR_ERR(data->clk_sec),
+					     "Failed to get clk_sec clock\n");
+	} else if (data->soc == SOC_ARCH_EXYNOS5433 ||
+		   data->soc == SOC_ARCH_EXYNOS7) {
+		data->sclk = devm_clk_get_enabled(dev, "tmu_sclk");
+		if (IS_ERR(data->sclk))
+			return dev_err_probe(dev, PTR_ERR(data->sclk),
+					     "Failed to get sclk\n");
 	}
 
 	ret = exynos_tmu_initialize(pdev);
 	if (ret) {
 		dev_err(dev, "Failed to initialize TMU\n");
-		goto err_sclk;
+		return ret;
 	}
 
 	data->tzd = devm_thermal_of_zone_register(dev, 0, data,
 						  &exynos_sensor_ops);
 	if (IS_ERR(data->tzd)) {
-		ret = dev_err_probe(dev, PTR_ERR(data->tzd), "Failed to register sensor\n");
-		goto err_sclk;
+		return dev_err_probe(dev, PTR_ERR(data->tzd),
+				     "Failed to register sensor\n");
 	}
 
 	ret = exynos_thermal_zone_configure(pdev);
 	if (ret) {
 		dev_err(dev, "Failed to configure the thermal zone\n");
-		goto err_sclk;
+		return ret;
 	}
 
 	ret = devm_request_threaded_irq(dev, data->irq, NULL,
@@ -1104,30 +1080,17 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 					dev_name(dev), data);
 	if (ret) {
 		dev_err(dev, "Failed to request irq: %d\n", data->irq);
-		goto err_sclk;
+		return ret;
 	}
 
 	exynos_tmu_control(pdev, true);
-	return 0;
 
-err_sclk:
-	clk_disable_unprepare(data->sclk);
-err_clk_sec:
-	clk_unprepare(data->clk_sec);
-err_clk:
-	clk_unprepare(data->clk);
 	return ret;
 }
 
 static void exynos_tmu_remove(struct platform_device *pdev)
 {
-	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-
 	exynos_tmu_control(pdev, false);
-
-	clk_disable_unprepare(data->sclk);
-	clk_unprepare(data->clk);
-	clk_unprepare(data->clk_sec);
 }
 
 #ifdef CONFIG_PM_SLEEP
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 3/7] thermal/drivers/exynos: Remove redundant IS_ERR() checks for clk_sec clock
  2025-08-13 13:09 [PATCH v7 0/7] Exynos Thermal code improvement Anand Moon
  2025-08-13 13:09 ` [PATCH v7 1/7] thermal/drivers/exynos: Refactor clk_sec initialization inside SOC-specific case Anand Moon
  2025-08-13 13:09 ` [PATCH v7 2/7] thermal/drivers/exynos: Use devm_clk_get_enabled() helpers Anand Moon
@ 2025-08-13 13:09 ` Anand Moon
  2025-08-13 13:09 ` [PATCH v7 4/7] thermal/drivers/exynos: Fixed the efuse min max value for exynos5422 Anand Moon
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Anand Moon @ 2025-08-13 13:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Rafael J. Wysocki,
	Daniel Lezcano, Zhang Rui, Lukasz Luba, Alim Akhtar,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	open list:SAMSUNG THERMAL DRIVER,
	open list:SAMSUNG THERMAL DRIVER,
	moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES,
	open list,
	open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b
  Cc: Anand Moon, Mateusz Majewski

Remove unnecessary IS_ERR() checks for the clk_sec clock,
the clk_enable() and clk_disable() functions can handle NULL clock
pointers, so the additional checks are redundant and have been removed
to simplify the code.

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v7: None
v6: Add Rb - Lukasz and Fix the typo in the subject
v5: None
v4: drop IE_ERR() for clk_unprepare() as its handle in earlier code.
v3: improve the commit message.
---
 drivers/thermal/samsung/exynos_tmu.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index aa0726b33c84..5f017a78f437 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -258,8 +258,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 
 	mutex_lock(&data->lock);
 	clk_enable(data->clk);
-	if (!IS_ERR(data->clk_sec))
-		clk_enable(data->clk_sec);
+	clk_enable(data->clk_sec);
 
 	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
 	if (!status) {
@@ -269,8 +268,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 		data->tmu_clear_irqs(data);
 	}
 
-	if (!IS_ERR(data->clk_sec))
-		clk_disable(data->clk_sec);
+	clk_disable(data->clk_sec);
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 4/7] thermal/drivers/exynos: Fixed the efuse min max value for exynos5422
  2025-08-13 13:09 [PATCH v7 0/7] Exynos Thermal code improvement Anand Moon
                   ` (2 preceding siblings ...)
  2025-08-13 13:09 ` [PATCH v7 3/7] thermal/drivers/exynos: Remove redundant IS_ERR() checks for clk_sec clock Anand Moon
@ 2025-08-13 13:09 ` Anand Moon
  2025-08-13 13:09 ` [PATCH v7 5/7] thermal/drivers/exynos: Remove unused base_second mapping and references Anand Moon
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Anand Moon @ 2025-08-13 13:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Rafael J. Wysocki,
	Daniel Lezcano, Zhang Rui, Lukasz Luba, Alim Akhtar,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	open list:SAMSUNG THERMAL DRIVER,
	open list:SAMSUNG THERMAL DRIVER,
	moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES,
	open list,
	open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b
  Cc: Anand Moon, Mateusz Majewski

As per Exynos5422 user manual e-Fuse range min~max range is 16~76.
if e-Fuse value is out of this range, then thermal sensor may not
sense thermal data properly. Additionally, refactors the efuse
initialization logic in exynos_map_dt_data() by replacing nested
if-else blocks with a switch statement for better readability
and maintainability. Ensures correct efuse setup based on SoC type.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v7: drop the Rb Llukasz, as we dropped the nested switch to set efuse.
v6: Add Rb Lukasz and fix typo in subject
v5: None
V4: None
---
 drivers/thermal/samsung/exynos_tmu.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 5f017a78f437..3d12e95703bf 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -884,6 +884,22 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 	case SOC_ARCH_EXYNOS4412:
 	case SOC_ARCH_EXYNOS5250:
 	case SOC_ARCH_EXYNOS5260:
+		data->tmu_set_low_temp = exynos4412_tmu_set_low_temp;
+		data->tmu_set_high_temp = exynos4412_tmu_set_high_temp;
+		data->tmu_disable_low = exynos4412_tmu_disable_low;
+		data->tmu_disable_high = exynos4210_tmu_disable_high;
+		data->tmu_set_crit_temp = exynos4412_tmu_set_crit_temp;
+		data->tmu_initialize = exynos4412_tmu_initialize;
+		data->tmu_control = exynos4210_tmu_control;
+		data->tmu_read = exynos4412_tmu_read;
+		data->tmu_set_emulation = exynos4412_tmu_set_emulation;
+		data->tmu_clear_irqs = exynos4210_tmu_clear_irqs;
+		data->gain = 8;
+		data->reference_voltage = 16;
+		data->efuse_value = 55;
+		data->min_efuse_value = 0;
+		data->max_efuse_value = 100;
+		break;
 	case SOC_ARCH_EXYNOS5420:
 	case SOC_ARCH_EXYNOS5420_TRIMINFO:
 		data->tmu_set_low_temp = exynos4412_tmu_set_low_temp;
@@ -899,12 +915,8 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->gain = 8;
 		data->reference_voltage = 16;
 		data->efuse_value = 55;
-		if (data->soc != SOC_ARCH_EXYNOS5420 &&
-		    data->soc != SOC_ARCH_EXYNOS5420_TRIMINFO)
-			data->min_efuse_value = 40;
-		else
-			data->min_efuse_value = 0;
-		data->max_efuse_value = 100;
+		data->min_efuse_value = 16;
+		data->max_efuse_value = 76;
 		break;
 	case SOC_ARCH_EXYNOS5433:
 		data->tmu_set_low_temp = exynos5433_tmu_set_low_temp;
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 5/7] thermal/drivers/exynos: Remove unused base_second mapping and references
  2025-08-13 13:09 [PATCH v7 0/7] Exynos Thermal code improvement Anand Moon
                   ` (3 preceding siblings ...)
  2025-08-13 13:09 ` [PATCH v7 4/7] thermal/drivers/exynos: Fixed the efuse min max value for exynos5422 Anand Moon
@ 2025-08-13 13:09 ` Anand Moon
  2025-08-13 13:09 ` [PATCH v7 6/7] thermal/drivers/exynos: Handle temperature threshold IRQs with SoC-specific mapping Anand Moon
  2025-08-13 13:09 ` [PATCH v7 7/7] thermal/drivers/exynos: Refactor IRQ clear logic using SoC-specific config Anand Moon
  6 siblings, 0 replies; 13+ messages in thread
From: Anand Moon @ 2025-08-13 13:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Rafael J. Wysocki,
	Daniel Lezcano, Zhang Rui, Lukasz Luba, Alim Akhtar,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	open list:SAMSUNG THERMAL DRIVER,
	open list:SAMSUNG THERMAL DRIVER,
	moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES,
	open list,
	open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b
  Cc: Anand Moon, Mateusz Majewski

The base_second field has been removed from struct exynos_tmu_data
because it was unused. This cleanup also eliminates its mapping in
exynos_map_dt_data() and ensures that TRIMINFO access in
exynos4412_tmu_initialize() consistently uses the base field across
all SoCs. This streamlines the code and optimizes memory usage.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v7: new patch in this series,
   Improve the commit message
   simplify the logic to TRIMINFO for all SoC.
---
 drivers/thermal/samsung/exynos_tmu.c | 30 +++++++---------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 3d12e95703bf..146f29fadea9 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -139,12 +139,11 @@ enum soc_type {
  * struct exynos_tmu_data : A structure to hold the private data of the TMU
  *			    driver
  * @base: base address of the single instance of the TMU controller.
- * @base_second: base address of the common registers of the TMU controller.
  * @irq: irq number of the TMU controller.
  * @soc: id of the SOC type.
  * @lock: lock to implement synchronization.
  * @clk: pointer to the clock structure.
- * @clk_sec: pointer to the clock structure for accessing the base_second.
+ * @clk_sec: pointer to the clock structure for accessing the gpu clk.
  * @sclk: pointer to the clock structure for accessing the tmu special clk.
  * @cal_type: calibration type for temperature
  * @efuse_value: SoC defined fuse value
@@ -172,7 +171,6 @@ enum soc_type {
  */
 struct exynos_tmu_data {
 	void __iomem *base;
-	void __iomem *base_second;
 	int irq;
 	enum soc_type soc;
 	struct mutex lock;
@@ -444,24 +442,17 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev)
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	unsigned int trim_info, ctrl;
 
-	if (data->soc == SOC_ARCH_EXYNOS3250 ||
-	    data->soc == SOC_ARCH_EXYNOS4412 ||
-	    data->soc == SOC_ARCH_EXYNOS5250) {
-		if (data->soc == SOC_ARCH_EXYNOS3250) {
-			ctrl = readl(data->base + EXYNOS_TMU_TRIMINFO_CON1);
-			ctrl |= EXYNOS_TRIMINFO_RELOAD_ENABLE;
-			writel(ctrl, data->base + EXYNOS_TMU_TRIMINFO_CON1);
-		}
+	if (data->soc == SOC_ARCH_EXYNOS3250) {
+		ctrl = readl(data->base + EXYNOS_TMU_TRIMINFO_CON1);
+		ctrl |= EXYNOS_TRIMINFO_RELOAD_ENABLE;
+		writel(ctrl, data->base + EXYNOS_TMU_TRIMINFO_CON1);
+	} else {
 		ctrl = readl(data->base + EXYNOS_TMU_TRIMINFO_CON2);
 		ctrl |= EXYNOS_TRIMINFO_RELOAD_ENABLE;
 		writel(ctrl, data->base + EXYNOS_TMU_TRIMINFO_CON2);
 	}
 
-	/* On exynos5420 the triminfo register is in the shared space */
-	if (data->soc == SOC_ARCH_EXYNOS5420_TRIMINFO)
-		trim_info = readl(data->base_second + EXYNOS_TMU_REG_TRIMINFO);
-	else
-		trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
+	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
 
 	sanitize_temp_error(data, trim_info);
 }
@@ -974,13 +965,6 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	data->base_second = devm_ioremap(&pdev->dev, res.start,
-					resource_size(&res));
-	if (!data->base_second) {
-		dev_err(&pdev->dev, "Failed to ioremap memory\n");
-		return -ENOMEM;
-	}
-
 	return 0;
 }
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 6/7] thermal/drivers/exynos: Handle temperature threshold IRQs with SoC-specific mapping
  2025-08-13 13:09 [PATCH v7 0/7] Exynos Thermal code improvement Anand Moon
                   ` (4 preceding siblings ...)
  2025-08-13 13:09 ` [PATCH v7 5/7] thermal/drivers/exynos: Remove unused base_second mapping and references Anand Moon
@ 2025-08-13 13:09 ` Anand Moon
       [not found]   ` <CGME20250819131732eucas1p26bd491e9b6b747a4857905bfd50420a9@eucas1p2.samsung.com>
  2025-08-13 13:09 ` [PATCH v7 7/7] thermal/drivers/exynos: Refactor IRQ clear logic using SoC-specific config Anand Moon
  6 siblings, 1 reply; 13+ messages in thread
From: Anand Moon @ 2025-08-13 13:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Rafael J. Wysocki,
	Daniel Lezcano, Zhang Rui, Lukasz Luba, Alim Akhtar,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	open list:SAMSUNG THERMAL DRIVER,
	open list:SAMSUNG THERMAL DRIVER,
	moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES,
	open list,
	open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b
  Cc: Anand Moon, Mateusz Majewski

The Exynos TMU interrupt handling mechanism has been refined to utilize
SoC-specific mappings for interrupt clear registers, targeting rising and
falling edge events. This change introduces a tmu_irq_map structure that
defines these edge-specific interrupt bits, improving the accuracy of
thermal event handling by ensuring that only relevant interrupts are
acknowledged and cleared. The exynos4210_tmu_clear_irqs() function has
been refactored to incorporate this mapping. Notably, for Exynos4210,
a check has been added to prevent clearing unsupported falling edge
interrupt bits.

As per user manuals, specific mappings for interrupt status and clear
registers include:

Exynos4412: Falling edge bits at 20, 16, 12, and
		rising edge bits at 8, 4, 0.
Exynos5422: Falling edge bits at 24, 20, 16, and
		rising edge bits at 8, 4, 0.
Exynos5433: Falling edge bits at 23, 17, 16, and
		rising edge bits at 7, 1, 0.

Cc: Mateusz Majewski <m.majewski2@samsung.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v7: New patch in this series
    simpilfy the logic for set and clear rising and failling
    edges of the IRQ clear register.
[0] https://lore.kernel.org/all/20250624075815.132207-1-m.majewski2@samsung.com/
---
 drivers/thermal/samsung/exynos_tmu.c | 70 ++++++++++++++++++++++++----
 1 file changed, 60 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 146f29fadea9..5e581055e3f3 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -197,6 +197,12 @@ struct exynos_tmu_data {
 	void (*tmu_clear_irqs)(struct exynos_tmu_data *data);
 };
 
+/* Map Rise and Falling edges for IRQ Clean */
+struct tmu_irq_map {
+	u32 fall[3];
+	u32 rise[3];
+};
+
 /*
  * TMU treats temperature as a mapped temperature code.
  * The temperature is converted differently depending on the calibration type.
@@ -765,8 +771,9 @@ static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
 
 static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data)
 {
-	unsigned int val_irq;
+	unsigned int val_irq, clear_irq = 0;
 	u32 tmu_intstat, tmu_intclear;
+	struct tmu_irq_map irq_map = {0};
 
 	if (data->soc == SOC_ARCH_EXYNOS5260) {
 		tmu_intstat = EXYNOS5260_TMU_REG_INTSTAT;
@@ -783,15 +790,58 @@ static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data)
 	}
 
 	val_irq = readl(data->base + tmu_intstat);
-	/*
-	 * Clear the interrupts.  Please note that the documentation for
-	 * Exynos3250, Exynos4412, Exynos5250 and Exynos5260 incorrectly
-	 * states that INTCLEAR register has a different placing of bits
-	 * responsible for FALL IRQs than INTSTAT register.  Exynos5420
-	 * and Exynos5440 documentation is correct (Exynos4210 doesn't
-	 * support FALL IRQs at all).
-	 */
-	writel(val_irq, data->base + tmu_intclear);
+
+	/* Exynos4210 doesn't support FALL interrupts */
+	if (data->soc == SOC_ARCH_EXYNOS4210) {
+		writel(val_irq, data->base + tmu_intclear);
+		return;
+	}
+
+	/* Set SoC-specific interrupt bit mappings */
+	switch (data->soc) {
+	case SOC_ARCH_EXYNOS3250:
+	case SOC_ARCH_EXYNOS4412:
+	case SOC_ARCH_EXYNOS5250:
+	case SOC_ARCH_EXYNOS5260:
+		irq_map.fall[2] = BIT(20);
+		irq_map.fall[1] = BIT(16);
+		irq_map.fall[0] = BIT(12);
+		irq_map.rise[2] = BIT(8);
+		irq_map.rise[1] = BIT(4);
+		irq_map.rise[0] = BIT(0);
+		break;
+	case SOC_ARCH_EXYNOS5420:
+	case SOC_ARCH_EXYNOS5420_TRIMINFO:
+		irq_map.fall[2] = BIT(24);
+		irq_map.fall[1] = BIT(20);
+		irq_map.fall[0] = BIT(16);
+		irq_map.rise[2] = BIT(8);
+		irq_map.rise[1] = BIT(4);
+		irq_map.rise[0] = BIT(0);
+		break;
+	case SOC_ARCH_EXYNOS5433:
+	case SOC_ARCH_EXYNOS7:
+		irq_map.fall[2] = BIT(23);
+		irq_map.fall[1] = BIT(17);
+		irq_map.fall[0] = BIT(16);
+		irq_map.rise[2] = BIT(7);
+		irq_map.rise[1] = BIT(1);
+		irq_map.rise[0] = BIT(0);
+		break;
+	default:
+		pr_warn("exynos-tmu: Unknown SoC type %d, using fallback IRQ mapping\n", soc);
+		break;
+
+	/* Map active INTSTAT bits to INTCLEAR */
+	for (int i = 0; i < 3; i++) {
+		if (val_irq & irq_map.fall[i])
+			clear_irq |= irq_map.fall[i];
+		if (val_irq & irq_map.rise[i])
+			clear_irq |= irq_map.rise[i];
+	}
+
+	if (clear_irq)
+		writel(clear_irq, data->base + tmu_intclear);
 }
 
 static const struct of_device_id exynos_tmu_match[] = {
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 7/7] thermal/drivers/exynos: Refactor IRQ clear logic using SoC-specific config
  2025-08-13 13:09 [PATCH v7 0/7] Exynos Thermal code improvement Anand Moon
                   ` (5 preceding siblings ...)
  2025-08-13 13:09 ` [PATCH v7 6/7] thermal/drivers/exynos: Handle temperature threshold IRQs with SoC-specific mapping Anand Moon
@ 2025-08-13 13:09 ` Anand Moon
       [not found]   ` <CGME20250819131814eucas1p2c57ccc084cf6736fed01a8a5c0b35fab@eucas1p2.samsung.com>
  6 siblings, 1 reply; 13+ messages in thread
From: Anand Moon @ 2025-08-13 13:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Rafael J. Wysocki,
	Daniel Lezcano, Zhang Rui, Lukasz Luba, Alim Akhtar,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	open list:SAMSUNG THERMAL DRIVER,
	open list:SAMSUNG THERMAL DRIVER,
	moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES,
	open list,
	open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b
  Cc: Anand Moon, Mateusz Majewski

The Exynos TMU driver's IRQ clear logic has been refactored for improved
maintainability and reduced code duplication. A unified
exynos4210_tmu_clear_irqs() implementation now replaces the previous
reliance on SoC-specific functions and hardcoded register mappings.
This new implementation leverages SoC-specific configuration fields
(tmu_intstat, tmu_intclear, and IRQ bit mappings) stored within
exynos_tmu_data. These fields are populated during device setup within
exynos_map_dt_data(), thereby streamlining new SoC integration, ensuring
correct interrupt handling, and improving code clarity. This refactor
simplifies the addition of new SoC support, ensures correct interrupt
handling across platforms, and improves overall code clarity.

Cc: Mateusz Majewski <m.majewski2@samsung.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v7: new patch in this series
    split the IRQ function handler per SoC.
---
 drivers/thermal/samsung/exynos_tmu.c | 150 +++++++++++++++++----------
 1 file changed, 96 insertions(+), 54 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 5e581055e3f3..9f94c58e1e74 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -158,6 +158,8 @@ enum soc_type {
  *	0 < reference_voltage <= 31
  * @tzd: pointer to thermal_zone_device structure
  * @enabled: current status of TMU device
+ * @tmu_intstat: interrupt status register
+ * @tmu_intclear: interrupt clear register
  * @tmu_set_low_temp: SoC specific method to set trip (falling threshold)
  * @tmu_set_high_temp: SoC specific method to set trip (rising threshold)
  * @tmu_set_crit_temp: SoC specific method to set critical temperature
@@ -184,6 +186,8 @@ struct exynos_tmu_data {
 	u8 reference_voltage;
 	struct thermal_zone_device *tzd;
 	bool enabled;
+	u32 tmu_intstat;
+	u32 tmu_intclear;
 
 	void (*tmu_set_low_temp)(struct exynos_tmu_data *data, u8 temp);
 	void (*tmu_set_high_temp)(struct exynos_tmu_data *data, u8 temp);
@@ -770,67 +774,90 @@ static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
 }
 
 static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data)
+{
+	unsigned int val_irq;
+	u32 tmu_intstat = data->tmu_intstat;
+	u32 tmu_intclear = data->tmu_intclear;
+
+	val_irq = readl(data->base + tmu_intstat);
+
+	/* Exynos4210 doesn't support FALL interrupts */
+	writel(val_irq, data->base + tmu_intclear);
+}
+
+static void exynos4412_tmu_clear_irqs(struct exynos_tmu_data *data)
 {
 	unsigned int val_irq, clear_irq = 0;
-	u32 tmu_intstat, tmu_intclear;
+	u32 tmu_intstat = data->tmu_intstat;
+	u32 tmu_intclear = data->tmu_intclear;
 	struct tmu_irq_map irq_map = {0};
 
-	if (data->soc == SOC_ARCH_EXYNOS5260) {
-		tmu_intstat = EXYNOS5260_TMU_REG_INTSTAT;
-		tmu_intclear = EXYNOS5260_TMU_REG_INTCLEAR;
-	} else if (data->soc == SOC_ARCH_EXYNOS7) {
-		tmu_intstat = EXYNOS7_TMU_REG_INTPEND;
-		tmu_intclear = EXYNOS7_TMU_REG_INTPEND;
-	} else if (data->soc == SOC_ARCH_EXYNOS5433) {
-		tmu_intstat = EXYNOS5433_TMU_REG_INTPEND;
-		tmu_intclear = EXYNOS5433_TMU_REG_INTPEND;
-	} else {
-		tmu_intstat = EXYNOS_TMU_REG_INTSTAT;
-		tmu_intclear = EXYNOS_TMU_REG_INTCLEAR;
+	val_irq = readl(data->base + tmu_intstat);
+
+	/* Set SoC-specific interrupt bit mappings */
+	irq_map.fall[2] = BIT(20);
+	irq_map.fall[1] = BIT(16);
+	irq_map.fall[0] = BIT(12);
+	irq_map.rise[2] = BIT(8);
+	irq_map.rise[1] = BIT(4);
+	irq_map.rise[0] = BIT(0);
+
+	/* Map active INTSTAT bits to INTCLEAR */
+	for (int i = 0; i < 3; i++) {
+		if (val_irq & irq_map.fall[i])
+			clear_irq |= irq_map.fall[i];
+		if (val_irq & irq_map.rise[i])
+			clear_irq |= irq_map.rise[i];
 	}
 
+	if (clear_irq)
+		writel(clear_irq, data->base + tmu_intclear);
+}
+
+static void exynos5420_tmu_clear_irqs(struct exynos_tmu_data *data)
+{
+	unsigned int val_irq, clear_irq = 0;
+	u32 tmu_intstat = data->tmu_intstat;
+	u32 tmu_intclear = data->tmu_intclear;
+	struct tmu_irq_map irq_map = {0};
+
 	val_irq = readl(data->base + tmu_intstat);
 
-	/* Exynos4210 doesn't support FALL interrupts */
-	if (data->soc == SOC_ARCH_EXYNOS4210) {
-		writel(val_irq, data->base + tmu_intclear);
-		return;
+	/* Set SoC-specific interrupt bit mappings */
+	irq_map.fall[2] = BIT(24);
+	irq_map.fall[1] = BIT(20);
+	irq_map.fall[0] = BIT(16);
+	irq_map.rise[2] = BIT(8);
+	irq_map.rise[1] = BIT(4);
+	irq_map.rise[0] = BIT(0);
+
+	for (int i = 0; i < 3; i++) {
+		if (val_irq & irq_map.fall[i])
+			clear_irq |= irq_map.fall[i];
+		if (val_irq & irq_map.rise[i])
+			clear_irq |= irq_map.rise[i];
 	}
 
+	if (clear_irq)
+		writel(clear_irq, data->base + tmu_intclear);
+}
+
+static void exynos5433_tmu_clear_irqs(struct exynos_tmu_data *data)
+{
+	unsigned int val_irq, clear_irq = 0;
+	u32 tmu_intstat = data->tmu_intstat;
+	u32 tmu_intclear = data->tmu_intclear;
+	struct tmu_irq_map irq_map = {0};
+
+	val_irq = readl(data->base + tmu_intstat);
+
 	/* Set SoC-specific interrupt bit mappings */
-	switch (data->soc) {
-	case SOC_ARCH_EXYNOS3250:
-	case SOC_ARCH_EXYNOS4412:
-	case SOC_ARCH_EXYNOS5250:
-	case SOC_ARCH_EXYNOS5260:
-		irq_map.fall[2] = BIT(20);
-		irq_map.fall[1] = BIT(16);
-		irq_map.fall[0] = BIT(12);
-		irq_map.rise[2] = BIT(8);
-		irq_map.rise[1] = BIT(4);
-		irq_map.rise[0] = BIT(0);
-		break;
-	case SOC_ARCH_EXYNOS5420:
-	case SOC_ARCH_EXYNOS5420_TRIMINFO:
-		irq_map.fall[2] = BIT(24);
-		irq_map.fall[1] = BIT(20);
-		irq_map.fall[0] = BIT(16);
-		irq_map.rise[2] = BIT(8);
-		irq_map.rise[1] = BIT(4);
-		irq_map.rise[0] = BIT(0);
-		break;
-	case SOC_ARCH_EXYNOS5433:
-	case SOC_ARCH_EXYNOS7:
-		irq_map.fall[2] = BIT(23);
-		irq_map.fall[1] = BIT(17);
-		irq_map.fall[0] = BIT(16);
-		irq_map.rise[2] = BIT(7);
-		irq_map.rise[1] = BIT(1);
-		irq_map.rise[0] = BIT(0);
-		break;
-	default:
-		pr_warn("exynos-tmu: Unknown SoC type %d, using fallback IRQ mapping\n", soc);
-		break;
+	irq_map.fall[2] = BIT(23);
+	irq_map.fall[1] = BIT(17);
+	irq_map.fall[0] = BIT(16);
+	irq_map.rise[2] = BIT(7);
+	irq_map.rise[1] = BIT(1);
+	irq_map.rise[0] = BIT(0);
 
 	/* Map active INTSTAT bits to INTCLEAR */
 	for (int i = 0; i < 3; i++) {
@@ -915,6 +942,8 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->tmu_control = exynos4210_tmu_control;
 		data->tmu_read = exynos4210_tmu_read;
 		data->tmu_clear_irqs = exynos4210_tmu_clear_irqs;
+		data->tmu_intstat = EXYNOS_TMU_REG_INTSTAT;
+		data->tmu_intclear = EXYNOS_TMU_REG_INTCLEAR;
 		data->gain = 15;
 		data->reference_voltage = 7;
 		data->efuse_value = 55;
@@ -934,7 +963,14 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->tmu_control = exynos4210_tmu_control;
 		data->tmu_read = exynos4412_tmu_read;
 		data->tmu_set_emulation = exynos4412_tmu_set_emulation;
-		data->tmu_clear_irqs = exynos4210_tmu_clear_irqs;
+		data->tmu_clear_irqs = exynos4412_tmu_clear_irqs;
+		if (data->soc == SOC_ARCH_EXYNOS5260) {
+			data->tmu_intstat = EXYNOS5260_TMU_REG_INTSTAT;
+			data->tmu_intclear = EXYNOS5260_TMU_REG_INTCLEAR;
+		} else {
+			data->tmu_intstat = EXYNOS_TMU_REG_INTSTAT;
+			data->tmu_intclear = EXYNOS_TMU_REG_INTCLEAR;
+		}
 		data->gain = 8;
 		data->reference_voltage = 16;
 		data->efuse_value = 55;
@@ -952,7 +988,9 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->tmu_control = exynos4210_tmu_control;
 		data->tmu_read = exynos4412_tmu_read;
 		data->tmu_set_emulation = exynos4412_tmu_set_emulation;
-		data->tmu_clear_irqs = exynos4210_tmu_clear_irqs;
+		data->tmu_clear_irqs = exynos5420_tmu_clear_irqs;
+		data->tmu_intstat = EXYNOS_TMU_REG_INTSTAT;
+		data->tmu_intclear = EXYNOS_TMU_REG_INTCLEAR;
 		data->gain = 8;
 		data->reference_voltage = 16;
 		data->efuse_value = 55;
@@ -969,7 +1007,9 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->tmu_control = exynos5433_tmu_control;
 		data->tmu_read = exynos4412_tmu_read;
 		data->tmu_set_emulation = exynos4412_tmu_set_emulation;
-		data->tmu_clear_irqs = exynos4210_tmu_clear_irqs;
+		data->tmu_clear_irqs = exynos5433_tmu_clear_irqs;
+		data->tmu_intstat = EXYNOS5433_TMU_REG_INTPEND;
+		data->tmu_intclear = EXYNOS5433_TMU_REG_INTPEND;
 		data->gain = 8;
 		if (res.start == EXYNOS5433_G3D_BASE)
 			data->reference_voltage = 23;
@@ -989,7 +1029,9 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->tmu_control = exynos7_tmu_control;
 		data->tmu_read = exynos7_tmu_read;
 		data->tmu_set_emulation = exynos4412_tmu_set_emulation;
-		data->tmu_clear_irqs = exynos4210_tmu_clear_irqs;
+		data->tmu_clear_irqs = exynos5433_tmu_clear_irqs;
+		data->tmu_intstat = EXYNOS7_TMU_REG_INTPEND;
+		data->tmu_intclear = EXYNOS7_TMU_REG_INTPEND;
 		data->gain = 9;
 		data->reference_voltage = 17;
 		data->efuse_value = 75;
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 6/7] thermal/drivers/exynos: Handle temperature threshold IRQs with SoC-specific mapping
       [not found]   ` <CGME20250819131732eucas1p26bd491e9b6b747a4857905bfd50420a9@eucas1p2.samsung.com>
@ 2025-08-19 13:17     ` Mateusz Majewski
       [not found]       ` <CGME20250819134804eucas1p1ed14f9680e66327a86af4e98319eed11@eucas1p1.samsung.com>
  2025-08-20 13:28       ` Anand Moon
  0 siblings, 2 replies; 13+ messages in thread
From: Mateusz Majewski @ 2025-08-19 13:17 UTC (permalink / raw)
  To: linux.amoon
  Cc: alim.akhtar, bzolnier, daniel.lezcano, justinstitt, krzk,
	linux-arm-kernel, linux-kernel, linux-pm, linux-samsung-soc, llvm,
	lukasz.luba, m.majewski2, morbo, nathan, nick.desaulniers+lkml,
	rafael, rui.zhang

Hello :)

> +/* Map Rise and Falling edges for IRQ Clean */
> +struct tmu_irq_map {
> +	u32 fall[3];
> +	u32 rise[3];
> +};

Hmm, we can probably get away with less interrupts. We actually only
enable one fall interrupt in tmu_set_low_temp and one rise interrupt in
tmu_set_high_temp.

Regarding tmu_set_crit_temp, on SoCs that have hardware thermal tripping
there is nothing to clear. On others we will reboot immediately anyway,
though maybe there is nothing wrong with clearing the interrupt
beforehand? Regardless of this, there is only a rise critical
temperature interrupt, we never set a matching fall interrupt.

Maybe it would also be good to add a bool to this struct containing
information about whether a fall interrupt is in use, and reuse
the same logic for 4210?

(Nitpick: I am not a native speaker of English, but I think "clean" and
"clear" have slightly different meanings, and the rest of the code
consistently uses "clear", so it would be worthwhile to also use "clear"
here.)

> +	/* Set SoC-specific interrupt bit mappings */
> +	switch (data->soc) {
> +	case SOC_ARCH_EXYNOS3250:
> +	case SOC_ARCH_EXYNOS4412:
> +	case SOC_ARCH_EXYNOS5250:
> +	case SOC_ARCH_EXYNOS5260:
> +		irq_map.fall[2] = BIT(20);
> +		irq_map.fall[1] = BIT(16);
> +		irq_map.fall[0] = BIT(12);
> +		irq_map.rise[2] = BIT(8);
> +		irq_map.rise[1] = BIT(4);
> +		irq_map.rise[0] = BIT(0);
> +		break;
> +	case SOC_ARCH_EXYNOS5420:
> +	case SOC_ARCH_EXYNOS5420_TRIMINFO:
> +		irq_map.fall[2] = BIT(24);
> +		irq_map.fall[1] = BIT(20);
> +		irq_map.fall[0] = BIT(16);
> +		irq_map.rise[2] = BIT(8);
> +		irq_map.rise[1] = BIT(4);
> +		irq_map.rise[0] = BIT(0);
> +		break;
> +	case SOC_ARCH_EXYNOS5433:
> +	case SOC_ARCH_EXYNOS7:
> +		irq_map.fall[2] = BIT(23);
> +		irq_map.fall[1] = BIT(17);
> +		irq_map.fall[0] = BIT(16);
> +		irq_map.rise[2] = BIT(7);
> +		irq_map.rise[1] = BIT(1);
> +		irq_map.rise[0] = BIT(0);
> +		break;
> +	default:
> +		pr_warn("exynos-tmu: Unknown SoC type %d, using fallback IRQ mapping\n", soc);
> +		break;

Maybe put irq_map inside exynos_tmu_data? exynos_map_dt_data has a
switch block that is quite similar, in that it also matches on the SoC
type. This way also there is no need to have a fallback.

Kind regards,
Mateusz Majewski

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 7/7] thermal/drivers/exynos: Refactor IRQ clear logic using SoC-specific config
       [not found]   ` <CGME20250819131814eucas1p2c57ccc084cf6736fed01a8a5c0b35fab@eucas1p2.samsung.com>
@ 2025-08-19 13:18     ` Mateusz Majewski
  2025-08-20 13:28       ` Anand Moon
  0 siblings, 1 reply; 13+ messages in thread
From: Mateusz Majewski @ 2025-08-19 13:18 UTC (permalink / raw)
  To: linux.amoon
  Cc: alim.akhtar, bzolnier, daniel.lezcano, justinstitt, krzk,
	linux-arm-kernel, linux-kernel, linux-pm, linux-samsung-soc, llvm,
	lukasz.luba, m.majewski2, morbo, nathan, nick.desaulniers+lkml,
	rafael, rui.zhang

> A unified
> exynos4210_tmu_clear_irqs() implementation now replaces the previous
> reliance on SoC-specific functions and hardcoded register mappings.

Well, right now we actually add exynos{4412,5420,5433}_tmu_clear_irqs :)
But those are quite similar except irq_map values, so maybe they can be
unified into one?

Kind regards,
Mateusz Majewski

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 6/7] thermal/drivers/exynos: Handle temperature threshold IRQs with SoC-specific mapping
       [not found]       ` <CGME20250819134804eucas1p1ed14f9680e66327a86af4e98319eed11@eucas1p1.samsung.com>
@ 2025-08-19 13:47         ` Mateusz Majewski
  0 siblings, 0 replies; 13+ messages in thread
From: Mateusz Majewski @ 2025-08-19 13:47 UTC (permalink / raw)
  To: m.majewski2
  Cc: alim.akhtar, bzolnier, daniel.lezcano, justinstitt, krzk,
	linux-arm-kernel, linux-kernel, linux-pm, linux-samsung-soc,
	linux.amoon, llvm, lukasz.luba, morbo, nathan,
	nick.desaulniers+lkml, rafael, rui.zhang

> +		pr_warn("exynos-tmu: Unknown SoC type %d, using fallback IRQ mapping\n", soc);

I missed this when writing the previous reply, but this doesn't build:
"soc" should be "data->soc". This line disappears in 7/7 though, so 7/7
builds just fine for me.

Kind regards,
Mateusz Majewski

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 6/7] thermal/drivers/exynos: Handle temperature threshold IRQs with SoC-specific mapping
  2025-08-19 13:17     ` Mateusz Majewski
       [not found]       ` <CGME20250819134804eucas1p1ed14f9680e66327a86af4e98319eed11@eucas1p1.samsung.com>
@ 2025-08-20 13:28       ` Anand Moon
  1 sibling, 0 replies; 13+ messages in thread
From: Anand Moon @ 2025-08-20 13:28 UTC (permalink / raw)
  To: Mateusz Majewski
  Cc: alim.akhtar, bzolnier, daniel.lezcano, justinstitt, krzk,
	linux-arm-kernel, linux-kernel, linux-pm, linux-samsung-soc, llvm,
	lukasz.luba, morbo, nathan, nick.desaulniers+lkml, rafael,
	rui.zhang

Hi Mateusz,

Thanks for your review comments..

On Tue, 19 Aug 2025 at 18:47, Mateusz Majewski <m.majewski2@samsung.com> wrote:
>
> Hello :)
>
> > +/* Map Rise and Falling edges for IRQ Clean */
> > +struct tmu_irq_map {
> > +     u32 fall[3];
> > +     u32 rise[3];
> > +};
>
Ops, this is an overkill approach that could be simplified further.

static void exynos4412_tmu_clear_irqs(struct exynos_tmu_data *data)
{
        unsigned int val_irq, clear_irq = 0;
        u32 tmu_intstat = data->tmu_intstat;
        u32 tmu_intclear = data->tmu_intclear;
        static const u32 irq_bits[] = {
                BIT(0), BIT(4), BIT(8),   /* Rising edge */
                BIT(12), BIT(16), BIT(20) /* Falling edge */
        };

        val_irq = readl(data->base + tmu_intstat);

        /* Set SoC-specific interrupt bit mappings */
        for (int i = 0; i < ARRAY_SIZE(irq_bits); i++) {
                if (val_irq & irq_bits[i])
                        clear_irq |= irq_bits[i];
        }

        if (clear_irq)
                writel(clear_irq, data->base + tmu_intclear);
}

> Hmm, we can probably get away with less interrupts. We actually only
> enable one fall interrupt in tmu_set_low_temp and one rise interrupt in
> tmu_set_high_temp.

Got it — we need to enable INTEN for both rising and falling edges,
specific to each SoC.
So each SoC has a one-to-one mapping of INTEN with INTSTAT and INTCLEAR bits
for rising and falling edge detection:.

Exynos4412
   Falling edge: bits 20, 16, 12
   Rising edge: bits 8, 4, 0
Exynos5422
  Falling edge: bits 24, 20, 16
  Rising edge: bits 8, 4, 0
Exynos5433
  Falling edge: bits 23, 17, 16
  Rising edge: bits 7, 1, 0

So we need to enable INTEN like below.

 static void exynos4412_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp)
 {
        exynos_tmu_update_temp(data, EXYNOS_THD_TEMP_FALL, 0, temp);
-       exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
-                             EXYNOS_TMU_INTEN_FALL0_SHIFT, true);
+       for (int i = 0; i < 3; i++)
+               exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
+                                     EXYNOS_TMU_INTEN_FALL0_SHIFT + i
* 4, true);
 }

 static void exynos4412_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp)
 {
        exynos_tmu_update_temp(data, EXYNOS_THD_TEMP_RISE, 8, temp);
-       exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
-                             EXYNOS_TMU_INTEN_RISE0_SHIFT + 4, true);
+       for (int i = 0; i < 3; i++)
+               exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
+                             EXYNOS_TMU_INTEN_RISE0_SHIFT + 1 * 4, true);
 }

>
> Regarding tmu_set_crit_temp, on SoCs that have hardware thermal tripping
> there is nothing to clear. On others we will reboot immediately anyway,
> though maybe there is nothing wrong with clearing the interrupt
> beforehand? Regardless of this, there is only a rise critical
> temperature interrupt, we never set a matching fall interrupt.
>
As per my understanding, it depends on the calibration consists of reading the
measured data from e-fuse and wrote the calibrated threshold temperature to
generate interrupts into trigger level 0-3

THRES_TEMP_RISE and THRES_TEMP_FALL.

When the current temperature exceeds a threshold rise temperature,
 then it generates the corresponding interrupt (INTREQ_RISE[2:0]).
When the current temperature goes below a threshold fall temperature, t
hen it generates a corresponding interrupt (INTREQ_FALL[2:0].

It also depends on the sample interval

> Maybe it would also be good to add a bool to this struct containing
> information about whether a fall interrupt is in use, and reuse
> the same logic for 4210?
>
> (Nitpick: I am not a native speaker of English, but I think "clean" and
> "clear" have slightly different meanings, and the rest of the code
> consistently uses "clear", so it would be worthwhile to also use "clear"
> here.)
>
> > +             break;
>
> Maybe put irq_map inside exynos_tmu_data? exynos_map_dt_data has a
> switch block that is quite similar, in that it also matches on the SoC
> type. This way also there is no need to have a fallback.
>
I want to avoid this If you have any design feedback, plz let me know.

I had attempted to refactor exynos_tmu_data by splitting it per-SoC,
assigning dedicated callback functions for each variant. The goal was
to eliminate
the need for data->soc checks and make the code more modular.
However, the approach didn’t work as expected—devm_thermal_of_zone_register()
wouldn’t bind the sensor-specific callbacks correctly.

static const struct thermal_zone_device_ops exynos_sensor_ops = {
.get_temp = exynos_get_temp,
.set_emul_temp = exynos_tmu_set_emulation,
.set_trips = exynos_set_trips,
};

> Kind regards,
> Mateusz Majewski

Thanks
-Anand

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 7/7] thermal/drivers/exynos: Refactor IRQ clear logic using SoC-specific config
  2025-08-19 13:18     ` Mateusz Majewski
@ 2025-08-20 13:28       ` Anand Moon
  0 siblings, 0 replies; 13+ messages in thread
From: Anand Moon @ 2025-08-20 13:28 UTC (permalink / raw)
  To: Mateusz Majewski
  Cc: alim.akhtar, bzolnier, daniel.lezcano, justinstitt, krzk,
	linux-arm-kernel, linux-kernel, linux-pm, linux-samsung-soc, llvm,
	lukasz.luba, morbo, nathan, nick.desaulniers+lkml, rafael,
	rui.zhang

Hi Mateusz

Thanks for your review comments..
On Tue, 19 Aug 2025 at 18:48, Mateusz Majewski <m.majewski2@samsung.com> wrote:
>
> > A unified
> > exynos4210_tmu_clear_irqs() implementation now replaces the previous
> > reliance on SoC-specific functions and hardcoded register mappings.
>
> Well, right now we actually add exynos{4412,5420,5433}_tmu_clear_irqs :)
> But those are quite similar except irq_map values, so maybe they can be
> unified into one?
>
No, the BIT fields for RISE and FALL are a bit different for SoC
So that's the reason for splitting, the IRQ handle should be clean and compact.
So make them simple.

> Kind regards,
> Mateusz Majewski
Thanks
-Anand

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-08-20 13:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-13 13:09 [PATCH v7 0/7] Exynos Thermal code improvement Anand Moon
2025-08-13 13:09 ` [PATCH v7 1/7] thermal/drivers/exynos: Refactor clk_sec initialization inside SOC-specific case Anand Moon
2025-08-13 13:09 ` [PATCH v7 2/7] thermal/drivers/exynos: Use devm_clk_get_enabled() helpers Anand Moon
2025-08-13 13:09 ` [PATCH v7 3/7] thermal/drivers/exynos: Remove redundant IS_ERR() checks for clk_sec clock Anand Moon
2025-08-13 13:09 ` [PATCH v7 4/7] thermal/drivers/exynos: Fixed the efuse min max value for exynos5422 Anand Moon
2025-08-13 13:09 ` [PATCH v7 5/7] thermal/drivers/exynos: Remove unused base_second mapping and references Anand Moon
2025-08-13 13:09 ` [PATCH v7 6/7] thermal/drivers/exynos: Handle temperature threshold IRQs with SoC-specific mapping Anand Moon
     [not found]   ` <CGME20250819131732eucas1p26bd491e9b6b747a4857905bfd50420a9@eucas1p2.samsung.com>
2025-08-19 13:17     ` Mateusz Majewski
     [not found]       ` <CGME20250819134804eucas1p1ed14f9680e66327a86af4e98319eed11@eucas1p1.samsung.com>
2025-08-19 13:47         ` Mateusz Majewski
2025-08-20 13:28       ` Anand Moon
2025-08-13 13:09 ` [PATCH v7 7/7] thermal/drivers/exynos: Refactor IRQ clear logic using SoC-specific config Anand Moon
     [not found]   ` <CGME20250819131814eucas1p2c57ccc084cf6736fed01a8a5c0b35fab@eucas1p2.samsung.com>
2025-08-19 13:18     ` Mateusz Majewski
2025-08-20 13:28       ` Anand Moon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).