[PATCH 00/11] Improve Exynos thermal driver

[PATCH 00/11] Improve Exynos thermal driver

[Mateusz Majewski]

This work improves Exynos thermal driver in various ways. This is
related to the discussion in
https://lore.kernel.org/all/97201878-3bb8-eac5-7fac-a690322ac43a@linaro.org/

The primary issue being fixed is a lockdep warning, which is fixed by
the thermal: exynos: use set_trips patch. We also handle Exynos 4210 not
supporting falling temperature thresholds by enabling polling for this
SoC, and simplify the code in general.

Mateusz Majewski (11):
  ARM: dts: exynos: enable polling in Exynos 4210
  thermal: exynos: drop id field
  thermal: exynos: switch from workqueue-driven interrupt handling to
    threaded interrupts
  thermal: exynos: remove fine-grained clk management
  thermal: exynos: simplify sclk (de)initialization
  thermal: exynos: simplify regulator (de)initialization
  thermal: exynos: simplify clk_sec (de)initialization
  thermal: exynos: stop using the threshold mechanism on Exynos 4210
  thermal: exynos: split initialization of TMU and the thermal zone
  thermal: exynos: use set_trips
  ARM: dts: exynos: disable polling in Odroid XU3-related devices

 arch/arm/boot/dts/samsung/exynos4210.dtsi     |  10 +-
 .../samsung/exynos5422-odroidxu3-common.dtsi  |  16 +-
 drivers/thermal/samsung/exynos_tmu.c          | 581 ++++++++----------
 3 files changed, 284 insertions(+), 323 deletions(-)

-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 01/11] ARM: dts: exynos: enable polling in Exynos 4210

[Mateusz Majewski]

It seems that thermal in Exynos 4210 is broken without this, as it will
never decrease cooling after increasing it.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 arch/arm/boot/dts/samsung/exynos4210.dtsi | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/samsung/exynos4210.dtsi b/arch/arm/boot/dts/samsung/exynos4210.dtsi
index 0e27c3375e2e..aae185b7f91c 100644
--- a/arch/arm/boot/dts/samsung/exynos4210.dtsi
+++ b/arch/arm/boot/dts/samsung/exynos4210.dtsi
@@ -391,8 +391,14 @@ &cpu_alert2 {
 };
 
 &cpu_thermal {
-	polling-delay-passive = <0>;
-	polling-delay = <0>;
+	/* Exynos 4210 supports thermal interrupts, but only for the rising threshold.
+	 * This means that polling is not needed for preventing overheating, but only
+	 * for decreasing cooling when possible. Hence we poll with a high delay.
+	 * Ideally, we would disable polling for the first trip point, but this isn't
+	 * really possible without outrageous hacks.
+	 */
+	polling-delay-passive = <5000>;
+	polling-delay = <5000>;
 };
 
 &gic {
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 02/11] thermal: exynos: drop id field

[Mateusz Majewski]

This field is not used in code, and seems to not have any meaning; in my
tests, the value was always 0.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 45e5c840d130..70e9c0296ee1 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -138,7 +138,6 @@ enum soc_type {
 /**
  * struct exynos_tmu_data : A structure to hold the private data of the TMU
  *			    driver
- * @id: identifier of the one instance of the TMU controller.
  * @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.
@@ -173,7 +172,6 @@ enum soc_type {
  * @tmu_clear_irqs: SoC specific TMU interrupts clearing method
  */
 struct exynos_tmu_data {
-	int id;
 	void __iomem *base;
 	void __iomem *base_second;
 	int irq;
@@ -866,10 +864,6 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 	if (!data || !pdev->dev.of_node)
 		return -ENODEV;
 
-	data->id = of_alias_get_id(pdev->dev.of_node, "tmuctrl");
-	if (data->id < 0)
-		data->id = 0;
-
 	data->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
 	if (data->irq <= 0) {
 		dev_err(&pdev->dev, "failed to get IRQ\n");
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 03/11] thermal: exynos: switch from workqueue-driven interrupt handling to threaded interrupts

[Mateusz Majewski]

The workqueue boilerplate is mostly one-to-one what the threaded
interrupts do.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 70e9c0296ee1..35b0a55017ad 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -142,7 +142,6 @@ enum soc_type {
  * @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.
- * @irq_work: pointer to the irq work structure.
  * @lock: lock to implement synchronization.
  * @clk: pointer to the clock structure.
  * @clk_sec: pointer to the clock structure for accessing the base_second.
@@ -176,7 +175,6 @@ struct exynos_tmu_data {
 	void __iomem *base_second;
 	int irq;
 	enum soc_type soc;
-	struct work_struct irq_work;
 	struct mutex lock;
 	struct clk *clk, *clk_sec, *sclk;
 	u32 cal_type;
@@ -764,10 +762,9 @@ static int exynos7_tmu_read(struct exynos_tmu_data *data)
 		EXYNOS7_TMU_TEMP_MASK;
 }
 
-static void exynos_tmu_work(struct work_struct *work)
+static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
 {
-	struct exynos_tmu_data *data = container_of(work,
-			struct exynos_tmu_data, irq_work);
+	struct exynos_tmu_data *data = id;
 
 	thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED);
 
@@ -779,7 +776,8 @@ static void exynos_tmu_work(struct work_struct *work)
 
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
-	enable_irq(data->irq);
+
+	return IRQ_HANDLED;
 }
 
 static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data)
@@ -813,16 +811,6 @@ static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data)
 	writel(val_irq, data->base + tmu_intclear);
 }
 
-static irqreturn_t exynos_tmu_irq(int irq, void *id)
-{
-	struct exynos_tmu_data *data = id;
-
-	disable_irq_nosync(irq);
-	schedule_work(&data->irq_work);
-
-	return IRQ_HANDLED;
-}
-
 static const struct of_device_id exynos_tmu_match[] = {
 	{
 		.compatible = "samsung,exynos3250-tmu",
@@ -1024,8 +1012,6 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_sensor;
 
-	INIT_WORK(&data->irq_work, exynos_tmu_work);
-
 	data->clk = devm_clk_get(&pdev->dev, "tmu_apbif");
 	if (IS_ERR(data->clk)) {
 		dev_err(&pdev->dev, "Failed to get clock\n");
@@ -1094,8 +1080,10 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		goto err_sclk;
 	}
 
-	ret = devm_request_irq(&pdev->dev, data->irq, exynos_tmu_irq,
-		IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data);
+	ret = devm_request_threaded_irq(
+		&pdev->dev, data->irq, NULL, exynos_tmu_threaded_irq,
+		IRQF_TRIGGER_RISING | IRQF_SHARED | IRQF_ONESHOT,
+		dev_name(&pdev->dev), data);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to request irq: %d\n", data->irq);
 		goto err_sclk;
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 04/11] thermal: exynos: remove fine-grained clk management

[Mateusz Majewski]

This clock only controls the register operations. The gain in power
efficiency is therefore quite dubious, while there is price of added
complexity that is important to get right (as a register operation might
outright hang the CPU if the clock is not enabled).

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 35b0a55017ad..2c5501704911 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -275,7 +275,6 @@ 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);
 
@@ -305,7 +304,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 		data->tmu_clear_irqs(data);
 	}
 err:
-	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
 	if (!IS_ERR(data->clk_sec))
 		clk_disable(data->clk_sec);
@@ -336,10 +334,8 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 
 	mutex_lock(&data->lock);
-	clk_enable(data->clk);
 	data->tmu_control(pdev, on);
 	data->enabled = on;
-	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
 }
 
@@ -654,7 +650,6 @@ static int exynos_get_temp(struct thermal_zone_device *tz, int *temp)
 		return -EAGAIN;
 
 	mutex_lock(&data->lock);
-	clk_enable(data->clk);
 
 	value = data->tmu_read(data);
 	if (value < 0)
@@ -662,7 +657,6 @@ static int exynos_get_temp(struct thermal_zone_device *tz, int *temp)
 	else
 		*temp = code_to_temp(data, value) * MCELSIUS;
 
-	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
 
 	return ret;
@@ -729,9 +723,7 @@ static int exynos_tmu_set_emulation(struct thermal_zone_device *tz, int temp)
 		goto out;
 
 	mutex_lock(&data->lock);
-	clk_enable(data->clk);
 	data->tmu_set_emulation(data, temp);
-	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
 	return 0;
 out:
@@ -769,12 +761,10 @@ static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
 	thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED);
 
 	mutex_lock(&data->lock);
-	clk_enable(data->clk);
 
 	/* TODO: take action based on particular interrupt */
 	data->tmu_clear_irqs(data);
 
-	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
 
 	return IRQ_HANDLED;
@@ -1012,7 +1002,7 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_sensor;
 
-	data->clk = devm_clk_get(&pdev->dev, "tmu_apbif");
+	data->clk = devm_clk_get_enabled(&pdev->dev, "tmu_apbif");
 	if (IS_ERR(data->clk)) {
 		dev_err(&pdev->dev, "Failed to get clock\n");
 		ret = PTR_ERR(data->clk);
@@ -1034,12 +1024,6 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		}
 	}
 
-	ret = clk_prepare(data->clk);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to get clock\n");
-		goto err_clk_sec;
-	}
-
 	switch (data->soc) {
 	case SOC_ARCH_EXYNOS5433:
 	case SOC_ARCH_EXYNOS7:
@@ -1047,12 +1031,12 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		if (IS_ERR(data->sclk)) {
 			dev_err(&pdev->dev, "Failed to get sclk\n");
 			ret = PTR_ERR(data->sclk);
-			goto err_clk;
+			goto err_clk_sec;
 		} else {
 			ret = clk_prepare_enable(data->sclk);
 			if (ret) {
 				dev_err(&pdev->dev, "Failed to enable sclk\n");
-				goto err_clk;
+				goto err_clk_sec;
 			}
 		}
 		break;
@@ -1094,8 +1078,6 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 
 err_sclk:
 	clk_disable_unprepare(data->sclk);
-err_clk:
-	clk_unprepare(data->clk);
 err_clk_sec:
 	if (!IS_ERR(data->clk_sec))
 		clk_unprepare(data->clk_sec);
@@ -1113,7 +1095,6 @@ static int exynos_tmu_remove(struct platform_device *pdev)
 	exynos_tmu_control(pdev, false);
 
 	clk_disable_unprepare(data->sclk);
-	clk_unprepare(data->clk);
 	if (!IS_ERR(data->clk_sec))
 		clk_unprepare(data->clk_sec);
 
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 05/11] thermal: exynos: simplify sclk (de)initialization

[Mateusz Majewski]

There's no need to enable the clock separately, and this also allows us
to delegate the deinitialization to devm entirely.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 2c5501704911..49e9157c3dc7 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -1027,17 +1027,11 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 	switch (data->soc) {
 	case SOC_ARCH_EXYNOS5433:
 	case SOC_ARCH_EXYNOS7:
-		data->sclk = devm_clk_get(&pdev->dev, "tmu_sclk");
+		data->sclk = devm_clk_get_enabled(&pdev->dev, "tmu_sclk");
 		if (IS_ERR(data->sclk)) {
 			dev_err(&pdev->dev, "Failed to get sclk\n");
 			ret = PTR_ERR(data->sclk);
 			goto err_clk_sec;
-		} else {
-			ret = clk_prepare_enable(data->sclk);
-			if (ret) {
-				dev_err(&pdev->dev, "Failed to enable sclk\n");
-				goto err_clk_sec;
-			}
 		}
 		break;
 	default:
@@ -1055,13 +1049,13 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		if (ret != -EPROBE_DEFER)
 			dev_err(&pdev->dev, "Failed to register sensor: %d\n",
 				ret);
-		goto err_sclk;
+		goto err_clk_sec;
 	}
 
 	ret = exynos_tmu_initialize(pdev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to initialize TMU\n");
-		goto err_sclk;
+		goto err_clk_sec;
 	}
 
 	ret = devm_request_threaded_irq(
@@ -1070,14 +1064,12 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		dev_name(&pdev->dev), data);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to request irq: %d\n", data->irq);
-		goto err_sclk;
+		goto err_clk_sec;
 	}
 
 	exynos_tmu_control(pdev, true);
 	return 0;
 
-err_sclk:
-	clk_disable_unprepare(data->sclk);
 err_clk_sec:
 	if (!IS_ERR(data->clk_sec))
 		clk_unprepare(data->clk_sec);
@@ -1094,7 +1086,6 @@ static int exynos_tmu_remove(struct platform_device *pdev)
 
 	exynos_tmu_control(pdev, false);
 
-	clk_disable_unprepare(data->sclk);
 	if (!IS_ERR(data->clk_sec))
 		clk_unprepare(data->clk_sec);
 
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 06/11] thermal: exynos: simplify regulator (de)initialization

[Mateusz Majewski]

This does reduce the error granularity a bit, but the code
simplification seems to be worth it.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 33 +++++++---------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 49e9157c3dc7..5c08212ff8ac 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -157,7 +157,6 @@ enum soc_type {
  * @reference_voltage: reference voltage of amplifier
  *	in the positive-TC generator block
  *	0 < reference_voltage <= 31
- * @regulator: pointer to the TMU regulator structure.
  * @reg_conf: pointer to structure to register with core thermal.
  * @tzd: pointer to thermal_zone_device structure
  * @ntrip: number of supported trip points.
@@ -184,7 +183,6 @@ struct exynos_tmu_data {
 	u16 temp_error1, temp_error2;
 	u8 gain;
 	u8 reference_voltage;
-	struct regulator *regulator;
 	struct thermal_zone_device *tzd;
 	unsigned int ntrip;
 	bool enabled;
@@ -985,42 +983,34 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 	 * TODO: Add regulator as an SOC feature, so that regulator enable
 	 * is a compulsory call.
 	 */
-	data->regulator = devm_regulator_get_optional(&pdev->dev, "vtmu");
-	if (!IS_ERR(data->regulator)) {
-		ret = regulator_enable(data->regulator);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to enable vtmu\n");
-			return ret;
-		}
-	} else {
-		if (PTR_ERR(data->regulator) == -EPROBE_DEFER)
+	ret = devm_regulator_get_enable_optional(&pdev->dev, "vtmu");
+	if (ret) {
+		if (ret == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
-		dev_info(&pdev->dev, "Regulator node (vtmu) not found\n");
+		dev_info(&pdev->dev, "Failed to get regulator node (vtmu)\n");
 	}
 
 	ret = exynos_map_dt_data(pdev);
 	if (ret)
-		goto err_sensor;
+		return ret;
 
 	data->clk = devm_clk_get_enabled(&pdev->dev, "tmu_apbif");
 	if (IS_ERR(data->clk)) {
 		dev_err(&pdev->dev, "Failed to get clock\n");
-		ret = PTR_ERR(data->clk);
-		goto err_sensor;
+		return PTR_ERR(data->clk);
 	}
 
 	data->clk_sec = devm_clk_get(&pdev->dev, "tmu_triminfo_apbif");
 	if (IS_ERR(data->clk_sec)) {
 		if (data->soc == SOC_ARCH_EXYNOS5420_TRIMINFO) {
 			dev_err(&pdev->dev, "Failed to get triminfo clock\n");
-			ret = PTR_ERR(data->clk_sec);
-			goto err_sensor;
+			return PTR_ERR(data->clk_sec);
 		}
 	} else {
 		ret = clk_prepare(data->clk_sec);
 		if (ret) {
 			dev_err(&pdev->dev, "Failed to get clock\n");
-			goto err_sensor;
+			return ret;
 		}
 	}
 
@@ -1073,10 +1063,6 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 err_clk_sec:
 	if (!IS_ERR(data->clk_sec))
 		clk_unprepare(data->clk_sec);
-err_sensor:
-	if (!IS_ERR(data->regulator))
-		regulator_disable(data->regulator);
-
 	return ret;
 }
 
@@ -1089,9 +1075,6 @@ static int exynos_tmu_remove(struct platform_device *pdev)
 	if (!IS_ERR(data->clk_sec))
 		clk_unprepare(data->clk_sec);
 
-	if (!IS_ERR(data->regulator))
-		regulator_disable(data->regulator);
-
 	return 0;
 }
 
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 07/11] thermal: exynos: simplify clk_sec (de)initialization

[Mateusz Majewski]

We only now grab the clock where it is actually being used.
Additionally, we remove the fine-grained clock management by enabling
the clock during initialization.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 45 ++++++++--------------------
 1 file changed, 12 insertions(+), 33 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 5c08212ff8ac..5da44f43715d 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -273,8 +273,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	}
 
 	mutex_lock(&data->lock);
-	if (!IS_ERR(data->clk_sec))
-		clk_enable(data->clk_sec);
 
 	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
 	if (!status) {
@@ -303,8 +301,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	}
 err:
 	mutex_unlock(&data->lock);
-	if (!IS_ERR(data->clk_sec))
-		clk_disable(data->clk_sec);
 out:
 	return ret;
 }
@@ -1000,28 +996,21 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		return PTR_ERR(data->clk);
 	}
 
-	data->clk_sec = devm_clk_get(&pdev->dev, "tmu_triminfo_apbif");
-	if (IS_ERR(data->clk_sec)) {
-		if (data->soc == SOC_ARCH_EXYNOS5420_TRIMINFO) {
-			dev_err(&pdev->dev, "Failed to get triminfo clock\n");
-			return PTR_ERR(data->clk_sec);
-		}
-	} else {
-		ret = clk_prepare(data->clk_sec);
-		if (ret) {
-			dev_err(&pdev->dev, "Failed to get clock\n");
-			return ret;
-		}
-	}
-
 	switch (data->soc) {
 	case SOC_ARCH_EXYNOS5433:
 	case SOC_ARCH_EXYNOS7:
 		data->sclk = devm_clk_get_enabled(&pdev->dev, "tmu_sclk");
 		if (IS_ERR(data->sclk)) {
 			dev_err(&pdev->dev, "Failed to get sclk\n");
-			ret = PTR_ERR(data->sclk);
-			goto err_clk_sec;
+			return PTR_ERR(data->sclk);
+		}
+		break;
+	case SOC_ARCH_EXYNOS5420_TRIMINFO:
+		data->clk_sec =
+			devm_clk_get_enabled(&pdev->dev, "tmu_triminfo_apbif");
+		if (IS_ERR(data->clk_sec)) {
+			dev_err(&pdev->dev, "Failed to get triminfo clock\n");
+			return PTR_ERR(data->clk_sec);
 		}
 		break;
 	default:
@@ -1039,13 +1028,13 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		if (ret != -EPROBE_DEFER)
 			dev_err(&pdev->dev, "Failed to register sensor: %d\n",
 				ret);
-		goto err_clk_sec;
+		return ret;
 	}
 
 	ret = exynos_tmu_initialize(pdev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to initialize TMU\n");
-		goto err_clk_sec;
+		return ret;
 	}
 
 	ret = devm_request_threaded_irq(
@@ -1054,27 +1043,17 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		dev_name(&pdev->dev), data);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to request irq: %d\n", data->irq);
-		goto err_clk_sec;
+		return ret;
 	}
 
 	exynos_tmu_control(pdev, true);
 	return 0;
-
-err_clk_sec:
-	if (!IS_ERR(data->clk_sec))
-		clk_unprepare(data->clk_sec);
-	return ret;
 }
 
 static int exynos_tmu_remove(struct platform_device *pdev)
 {
-	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-
 	exynos_tmu_control(pdev, false);
 
-	if (!IS_ERR(data->clk_sec))
-		clk_unprepare(data->clk_sec);
-
 	return 0;
 }
 
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 08/11] thermal: exynos: stop using the threshold mechanism on Exynos 4210

[Mateusz Majewski]

Exynos 4210 supports setting a base threshold value, which is added to
all trip points. This might be useful, but is not really necessary in
our usecase, so we always set it to 0 to simplify the code a bit.

Additionally, this change makes it so that we convert the value to the
calibrated one in a slightly different place. This is more correct
morally, though it does not make any change when single-point
calibration is being used (which is the case currently).

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 5da44f43715d..6c107fe9fd27 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -336,20 +336,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 static void exynos4210_tmu_set_trip_temp(struct exynos_tmu_data *data,
 					 int trip_id, u8 temp)
 {
-	struct thermal_trip trip;
-	u8 ref, th_code;
-
-	if (thermal_zone_get_trip(data->tzd, 0, &trip))
-		return;
-
-	ref = trip.temperature / MCELSIUS;
-
-	if (trip_id == 0) {
-		th_code = temp_to_code(data, ref);
-		writeb(th_code, data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP);
-	}
-
-	temp -= ref;
+	temp = temp_to_code(data, temp);
 	writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + trip_id * 4);
 }
 
@@ -364,6 +351,8 @@ static void exynos4210_tmu_initialize(struct platform_device *pdev)
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 
 	sanitize_temp_error(data, readl(data->base + EXYNOS_TMU_REG_TRIMINFO));
+
+	writeb(0, data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP);
 }
 
 static void exynos4412_tmu_set_trip_temp(struct exynos_tmu_data *data,
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 09/11] thermal: exynos: split initialization of TMU and the thermal zone

[Mateusz Majewski]

This will be needed in the future, as the thermal zone subsystem might
call our callbacks right after devm_thermal_of_zone_register. Currently
we just make get_temp return EAGAIN in such case, but this will not be
possible with state-modifying callbacks, for instance set_trips.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 78 ++++++++++++++++------------
 1 file changed, 45 insertions(+), 33 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 6c107fe9fd27..aa3de367b3c7 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -250,20 +250,43 @@ static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
 }
 
 static int exynos_tmu_initialize(struct platform_device *pdev)
+{
+	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
+	unsigned int status;
+	int ret = 0;
+
+	mutex_lock(&data->lock);
+
+	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
+	if (!status) {
+		ret = -EBUSY;
+	} else {
+		data->tmu_initialize(pdev);
+		data->tmu_clear_irqs(data);
+	}
+
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int exynos_thermal_zone_configure(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tzd = data->tzd;
 	int num_trips = thermal_zone_get_num_trips(tzd);
-	unsigned int status;
 	int ret = 0, temp;
 
 	ret = thermal_zone_get_crit_temp(tzd, &temp);
+
 	if (ret && data->soc != SOC_ARCH_EXYNOS5433) { /* FIXME */
 		dev_err(&pdev->dev,
 			"No CRITICAL trip point defined in device tree!\n");
-		goto out;
+		goto err;
 	}
 
+	mutex_lock(&data->lock);
+
 	if (num_trips > data->ntrip) {
 		dev_info(&pdev->dev,
 			 "More trip points than supported by this TMU.\n");
@@ -272,36 +295,23 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 			 num_trips - data->ntrip);
 	}
 
-	mutex_lock(&data->lock);
+	int i, ntrips = min_t(int, num_trips, data->ntrip);
 
-	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
-	if (!status) {
-		ret = -EBUSY;
-	} else {
-		int i, ntrips =
-			min_t(int, num_trips, data->ntrip);
+	/* Write temperature code for rising and falling threshold */
+	for (i = 0; i < ntrips; i++) {
+		struct thermal_trip trip;
 
-		data->tmu_initialize(pdev);
+		ret = thermal_zone_get_trip(tzd, i, &trip);
+		if (ret)
+			goto err;
 
-		/* Write temperature code for rising and falling threshold */
-		for (i = 0; i < ntrips; i++) {
-
-			struct thermal_trip trip;
-
-			ret = thermal_zone_get_trip(tzd, i, &trip);
-			if (ret)
-				goto err;
-
-			data->tmu_set_trip_temp(data, i, trip.temperature / MCELSIUS);
-			data->tmu_set_trip_hyst(data, i, trip.temperature / MCELSIUS,
-						trip.hysteresis / MCELSIUS);
-		}
-
-		data->tmu_clear_irqs(data);
+		data->tmu_set_trip_temp(data, i, trip.temperature / MCELSIUS);
+		data->tmu_set_trip_hyst(data, i, trip.temperature / MCELSIUS,
+					trip.hysteresis / MCELSIUS);
 	}
-err:
+
 	mutex_unlock(&data->lock);
-out:
+err:
 	return ret;
 }
 
@@ -1006,10 +1016,12 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		break;
 	}
 
-	/*
-	 * data->tzd must be registered before calling exynos_tmu_initialize(),
-	 * requesting irq and calling exynos_tmu_control().
-	 */
+	ret = exynos_tmu_initialize(pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to initialize TMU\n");
+		return ret;
+	}
+
 	data->tzd = devm_thermal_of_zone_register(&pdev->dev, 0, data,
 						  &exynos_sensor_ops);
 	if (IS_ERR(data->tzd)) {
@@ -1020,9 +1032,9 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = exynos_tmu_initialize(pdev);
+	ret = exynos_thermal_zone_configure(pdev);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to initialize TMU\n");
+		dev_err(&pdev->dev, "Failed to configure the thermal zone\n");
 		return ret;
 	}
 
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 10/11] thermal: exynos: use set_trips

[Mateusz Majewski]

Currently, each trip point defined in the device tree corresponds to a
single hardware interrupt. This commit instead switches to using two
hardware interrupts, whose values are set dynamically using the
set_trips callback. Additionally, the critical temperature threshold is
handled specifically.

Setting interrupts in this way also fixes a long-standing lockdep
warning, which was caused by calling thermal_zone_get_trips with our
lock being held. Do note that this requires TMU initialization to be
split into two parts, as done by the parent commit: parts of the
initialization call into the thermal_zone_device structure and so must
be done after its registration, but the initialization is also
responsible for setting up calibration, which must be done before
thermal_zone_device registration, which will call set_trips for the
first time; if the calibration is not done in time, the interrupt values
will be silently wrong!

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 390 +++++++++++++++------------
 1 file changed, 217 insertions(+), 173 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index aa3de367b3c7..5b35c51fca79 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -159,7 +159,6 @@ enum soc_type {
  *	0 < reference_voltage <= 31
  * @reg_conf: pointer to structure to register with core thermal.
  * @tzd: pointer to thermal_zone_device structure
- * @ntrip: number of supported trip points.
  * @enabled: current status of TMU device
  * @tmu_set_trip_temp: SoC specific method to set trip (rising threshold)
  * @tmu_set_trip_hyst: SoC specific to set hysteresis (falling threshold)
@@ -184,13 +183,13 @@ struct exynos_tmu_data {
 	u8 gain;
 	u8 reference_voltage;
 	struct thermal_zone_device *tzd;
-	unsigned int ntrip;
 	bool enabled;
 
-	void (*tmu_set_trip_temp)(struct exynos_tmu_data *data, int trip,
-				 u8 temp);
-	void (*tmu_set_trip_hyst)(struct exynos_tmu_data *data, int trip,
-				 u8 temp, u8 hyst);
+	void (*tmu_set_low_temp)(struct exynos_tmu_data *data, u8 temp);
+	void (*tmu_set_high_temp)(struct exynos_tmu_data *data, u8 temp);
+	void (*tmu_set_crit_temp)(struct exynos_tmu_data *data, u8 temp);
+	void (*tmu_disable_low)(struct exynos_tmu_data *data);
+	void (*tmu_disable_high)(struct exynos_tmu_data *data);
 	void (*tmu_initialize)(struct platform_device *pdev);
 	void (*tmu_control)(struct platform_device *pdev, bool on);
 	int (*tmu_read)(struct exynos_tmu_data *data);
@@ -274,43 +273,24 @@ static int exynos_thermal_zone_configure(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tzd = data->tzd;
-	int num_trips = thermal_zone_get_num_trips(tzd);
 	int ret = 0, temp;
 
 	ret = thermal_zone_get_crit_temp(tzd, &temp);
-
-	if (ret && data->soc != SOC_ARCH_EXYNOS5433) { /* FIXME */
-		dev_err(&pdev->dev,
-			"No CRITICAL trip point defined in device tree!\n");
-		goto err;
-	}
-
-	mutex_lock(&data->lock);
-
-	if (num_trips > data->ntrip) {
-		dev_info(&pdev->dev,
-			 "More trip points than supported by this TMU.\n");
-		dev_info(&pdev->dev,
-			 "%d trip points should be configured in polling mode.\n",
-			 num_trips - data->ntrip);
-	}
-
-	int i, ntrips = min_t(int, num_trips, data->ntrip);
-
-	/* Write temperature code for rising and falling threshold */
-	for (i = 0; i < ntrips; i++) {
-		struct thermal_trip trip;
-
-		ret = thermal_zone_get_trip(tzd, i, &trip);
-		if (ret)
+	if (ret) {
+		if (data->soc ==
+		    SOC_ARCH_EXYNOS5433) { /* FIXME: Remove this and the check below */
+			temp = INT_MAX;
+			ret = 0;
+		} else {
+			dev_err(&pdev->dev,
+				"No CRITICAL trip point defined in device tree!\n");
 			goto err;
-
-		data->tmu_set_trip_temp(data, i, trip.temperature / MCELSIUS);
-		data->tmu_set_trip_hyst(data, i, trip.temperature / MCELSIUS,
-					trip.hysteresis / MCELSIUS);
+		}
 	}
 
-	mutex_unlock(&data->lock);
+	if (temp < INT_MAX)
+		data->tmu_set_crit_temp(data, temp / MCELSIUS);
+
 err:
 	return ret;
 }
@@ -343,17 +323,56 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	mutex_unlock(&data->lock);
 }
 
-static void exynos4210_tmu_set_trip_temp(struct exynos_tmu_data *data,
-					 int trip_id, u8 temp)
+static void exynos_tmu_update_bit(struct exynos_tmu_data *data, int reg_off,
+				  int bit_off, bool enable)
 {
-	temp = temp_to_code(data, temp);
-	writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + trip_id * 4);
+	u32 interrupt_en;
+
+	interrupt_en = readl(data->base + reg_off);
+	if (enable)
+		interrupt_en |= 1 << bit_off;
+	else
+		interrupt_en &= ~(1 << bit_off);
+	writel(interrupt_en, data->base + reg_off);
 }
 
-/* failing thresholds are not supported on Exynos4210 */
-static void exynos4210_tmu_set_trip_hyst(struct exynos_tmu_data *data,
-					 int trip, u8 temp, u8 hyst)
+static void exynos4210_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp)
 {
+	/* Failing thresholds are not supported on Exynos 4210.
+	 * We use polling instead.
+	 */
+}
+
+static void exynos4210_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	temp = temp_to_code(data, temp);
+	writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + 4);
+	exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_RISE0_SHIFT + 4, true);
+}
+
+static void exynos4210_tmu_disable_low(struct exynos_tmu_data *data)
+{
+	/* Again, this is handled by polling. */
+}
+
+static void exynos4210_tmu_disable_high(struct exynos_tmu_data *data)
+{
+	exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_RISE0_SHIFT + 4, false);
+}
+
+static void exynos4210_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	/* Hardware critical temperature handling is not supported on Exynos 4210.
+	 * We still set the critical temperature threshold, but this is only to
+	 * make sure it is handled as soon as possible. It is just a normal interrupt.
+	 */
+
+	temp = temp_to_code(data, temp);
+	writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + 12);
+	exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_RISE0_SHIFT + 12, true);
 }
 
 static void exynos4210_tmu_initialize(struct platform_device *pdev)
@@ -365,33 +384,49 @@ static void exynos4210_tmu_initialize(struct platform_device *pdev)
 	writeb(0, data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP);
 }
 
-static void exynos4412_tmu_set_trip_temp(struct exynos_tmu_data *data,
-					 int trip, u8 temp)
-{
-	u32 th, con;
-
-	th = readl(data->base + EXYNOS_THD_TEMP_RISE);
-	th &= ~(0xff << 8 * trip);
-	th |= temp_to_code(data, temp) << 8 * trip;
-	writel(th, data->base + EXYNOS_THD_TEMP_RISE);
-
-	if (trip == 3) {
-		con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
-		con |= (1 << EXYNOS_TMU_THERM_TRIP_EN_SHIFT);
-		writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
-	}
-}
-
-static void exynos4412_tmu_set_trip_hyst(struct exynos_tmu_data *data,
-					 int trip, u8 temp, u8 hyst)
+static void exynos4412_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp)
 {
 	u32 th;
 
 	th = readl(data->base + EXYNOS_THD_TEMP_FALL);
-	th &= ~(0xff << 8 * trip);
-	if (hyst)
-		th |= temp_to_code(data, temp - hyst) << 8 * trip;
+	th &= ~(0xff << 0);
+	th |= temp_to_code(data, temp) << 0;
 	writel(th, data->base + EXYNOS_THD_TEMP_FALL);
+
+	exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_FALL0_SHIFT, true);
+}
+
+static void exynos4412_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	u32 th;
+
+	th = readl(data->base + EXYNOS_THD_TEMP_RISE);
+	th &= ~(0xff << 8);
+	th |= temp_to_code(data, temp) << 8;
+	writel(th, data->base + EXYNOS_THD_TEMP_RISE);
+
+	exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_RISE0_SHIFT + 4, true);
+}
+
+static void exynos4412_tmu_disable_low(struct exynos_tmu_data *data)
+{
+	exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_FALL0_SHIFT, false);
+}
+
+static void exynos4412_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	u32 th;
+
+	th = readl(data->base + EXYNOS_THD_TEMP_RISE);
+	th &= ~(0xff << 24);
+	th |= temp_to_code(data, temp) << 24;
+	writel(th, data->base + EXYNOS_THD_TEMP_RISE);
+
+	exynos_tmu_update_bit(data, EXYNOS_TMU_REG_CONTROL,
+			      EXYNOS_TMU_THERM_TRIP_EN_SHIFT, true);
 }
 
 static void exynos4412_tmu_initialize(struct platform_device *pdev)
@@ -421,44 +456,57 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev)
 	sanitize_temp_error(data, trim_info);
 }
 
-static void exynos5433_tmu_set_trip_temp(struct exynos_tmu_data *data,
-					 int trip, u8 temp)
+static void exynos5433_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp)
 {
-	unsigned int reg_off, j;
 	u32 th;
 
-	if (trip > 3) {
-		reg_off = EXYNOS5433_THD_TEMP_RISE7_4;
-		j = trip - 4;
-	} else {
-		reg_off = EXYNOS5433_THD_TEMP_RISE3_0;
-		j = trip;
-	}
+	th = readl(data->base + EXYNOS5433_THD_TEMP_FALL3_0);
+	th &= ~(0xff << 0);
+	th |= temp_to_code(data, temp) << 0;
+	writel(th, data->base + EXYNOS5433_THD_TEMP_FALL3_0);
 
-	th = readl(data->base + reg_off);
-	th &= ~(0xff << j * 8);
-	th |= (temp_to_code(data, temp) << j * 8);
-	writel(th, data->base + reg_off);
+	exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_FALL0_SHIFT, true);
 }
 
-static void exynos5433_tmu_set_trip_hyst(struct exynos_tmu_data *data,
-					 int trip, u8 temp, u8 hyst)
+static void exynos5433_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp)
 {
-	unsigned int reg_off, j;
 	u32 th;
 
-	if (trip > 3) {
-		reg_off = EXYNOS5433_THD_TEMP_FALL7_4;
-		j = trip - 4;
-	} else {
-		reg_off = EXYNOS5433_THD_TEMP_FALL3_0;
-		j = trip;
-	}
+	th = readl(data->base + EXYNOS5433_THD_TEMP_RISE3_0);
+	th &= ~(0xff << 8);
+	th |= temp_to_code(data, temp) << 8;
+	writel(th, data->base + EXYNOS5433_THD_TEMP_RISE3_0);
 
-	th = readl(data->base + reg_off);
-	th &= ~(0xff << j * 8);
-	th |= (temp_to_code(data, temp - hyst) << j * 8);
-	writel(th, data->base + reg_off);
+	exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN,
+			      EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, true);
+}
+
+static void exynos5433_tmu_disable_low(struct exynos_tmu_data *data)
+{
+	exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_FALL0_SHIFT, false);
+}
+
+static void exynos5433_tmu_disable_high(struct exynos_tmu_data *data)
+{
+	exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN,
+			      EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, false);
+}
+
+static void exynos5433_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	u32 th;
+
+	th = readl(data->base + EXYNOS5433_THD_TEMP_RISE7_4);
+	th &= ~(0xff << 24);
+	th |= temp_to_code(data, temp) << 24;
+	writel(th, data->base + EXYNOS5433_THD_TEMP_RISE7_4);
+
+	exynos_tmu_update_bit(data, EXYNOS_TMU_REG_CONTROL,
+			      EXYNOS_TMU_THERM_TRIP_EN_SHIFT, true);
+	exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN,
+			      EXYNOS7_TMU_INTEN_RISE0_SHIFT + 7, true);
 }
 
 static void exynos5433_tmu_initialize(struct platform_device *pdev)
@@ -494,34 +542,47 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev)
 			cal_type ?  2 : 1);
 }
 
-static void exynos7_tmu_set_trip_temp(struct exynos_tmu_data *data,
-				      int trip, u8 temp)
+static void exynos7_tmu_update_temp(struct exynos_tmu_data *data, u8 temp,
+				    int idx, bool rise)
 {
 	unsigned int reg_off, bit_off;
 	u32 th;
+	void __iomem *reg;
 
-	reg_off = ((7 - trip) / 2) * 4;
-	bit_off = ((8 - trip) % 2);
+	reg_off = ((7 - idx) / 2) * 4;
+	bit_off = ((8 - idx) % 2);
 
-	th = readl(data->base + EXYNOS7_THD_TEMP_RISE7_6 + reg_off);
+	reg = data->base +
+	      (rise ? EXYNOS7_THD_TEMP_RISE7_6 : EXYNOS7_THD_TEMP_FALL7_6) +
+	      reg_off;
+	th = readl(reg);
 	th &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off));
 	th |= temp_to_code(data, temp) << (16 * bit_off);
-	writel(th, data->base + EXYNOS7_THD_TEMP_RISE7_6 + reg_off);
+	writel(th, reg);
+
+	exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN,
+			      (rise ? EXYNOS7_TMU_INTEN_RISE0_SHIFT :
+				      EXYNOS_TMU_INTEN_FALL0_SHIFT) +
+				      idx,
+			      true);
 }
 
-static void exynos7_tmu_set_trip_hyst(struct exynos_tmu_data *data,
-				      int trip, u8 temp, u8 hyst)
+static void exynos7_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp)
 {
-	unsigned int reg_off, bit_off;
-	u32 th;
+	exynos7_tmu_update_temp(data, temp, 0, false);
+}
 
-	reg_off = ((7 - trip) / 2) * 4;
-	bit_off = ((8 - trip) % 2);
+static void exynos7_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	exynos7_tmu_update_temp(data, temp, 1, true);
+}
 
-	th = readl(data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off);
-	th &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off));
-	th |= temp_to_code(data, temp - hyst) << (16 * bit_off);
-	writel(th, data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off);
+static void exynos7_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	/* Like Exynos 4210, Exynos 7 does not seem to support critical temperature
+	 * handling in hardware. Again, we still set a separate interrupt for it.
+	 */
+	exynos7_tmu_update_temp(data, temp, 7, true);
 }
 
 static void exynos7_tmu_initialize(struct platform_device *pdev)
@@ -536,87 +597,44 @@ static void exynos7_tmu_initialize(struct platform_device *pdev)
 static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-	struct thermal_zone_device *tz = data->tzd;
-	struct thermal_trip trip;
-	unsigned int con, interrupt_en = 0, i;
+	unsigned int con;
 
 	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
 
-	if (on) {
-		for (i = 0; i < data->ntrip; i++) {
-			if (thermal_zone_get_trip(tz, i, &trip))
-				continue;
-
-			interrupt_en |=
-				(1 << (EXYNOS_TMU_INTEN_RISE0_SHIFT + i * 4));
-		}
-
-		if (data->soc != SOC_ARCH_EXYNOS4210)
-			interrupt_en |=
-				interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
-
+	if (on)
 		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
-	} else {
+	else
 		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
-	}
 
-	writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
 }
 
 static void exynos5433_tmu_control(struct platform_device *pdev, bool on)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-	struct thermal_zone_device *tz = data->tzd;
-	struct thermal_trip trip;
-	unsigned int con, interrupt_en = 0, pd_det_en, i;
+	unsigned int con, pd_det_en;
 
 	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
 
-	if (on) {
-		for (i = 0; i < data->ntrip; i++) {
-			if (thermal_zone_get_trip(tz, i, &trip))
-				continue;
-
-			interrupt_en |=
-				(1 << (EXYNOS7_TMU_INTEN_RISE0_SHIFT + i));
-		}
-
-		interrupt_en |=
-			interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
-
+	if (on)
 		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
-	} else
+	else
 		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
 
 	pd_det_en = on ? EXYNOS5433_PD_DET_EN : 0;
 
 	writel(pd_det_en, data->base + EXYNOS5433_TMU_PD_DET_EN);
-	writel(interrupt_en, data->base + EXYNOS5433_TMU_REG_INTEN);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
 }
 
 static void exynos7_tmu_control(struct platform_device *pdev, bool on)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-	struct thermal_zone_device *tz = data->tzd;
-	struct thermal_trip trip;
-	unsigned int con, interrupt_en = 0, i;
+	unsigned int con;
 
 	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
 
 	if (on) {
-		for (i = 0; i < data->ntrip; i++) {
-			if (thermal_zone_get_trip(tz, i, &trip))
-				continue;
-
-			interrupt_en |=
-				(1 << (EXYNOS7_TMU_INTEN_RISE0_SHIFT + i));
-		}
-
-		interrupt_en |=
-			interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
-
 		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
 		con |= (1 << EXYNOS7_PD_DET_EN_SHIFT);
 	} else {
@@ -624,7 +642,6 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on)
 		con &= ~(1 << EXYNOS7_PD_DET_EN_SHIFT);
 	}
 
-	writel(interrupt_en, data->base + EXYNOS7_TMU_REG_INTEN);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
 }
 
@@ -856,13 +873,15 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 
 	switch (data->soc) {
 	case SOC_ARCH_EXYNOS4210:
-		data->tmu_set_trip_temp = exynos4210_tmu_set_trip_temp;
-		data->tmu_set_trip_hyst = exynos4210_tmu_set_trip_hyst;
+		data->tmu_set_low_temp = exynos4210_tmu_set_low_temp;
+		data->tmu_set_high_temp = exynos4210_tmu_set_high_temp;
+		data->tmu_disable_low = exynos4210_tmu_disable_low;
+		data->tmu_disable_high = exynos4210_tmu_disable_high;
+		data->tmu_set_crit_temp = exynos4210_tmu_set_crit_temp;
 		data->tmu_initialize = exynos4210_tmu_initialize;
 		data->tmu_control = exynos4210_tmu_control;
 		data->tmu_read = exynos4210_tmu_read;
 		data->tmu_clear_irqs = exynos4210_tmu_clear_irqs;
-		data->ntrip = 4;
 		data->gain = 15;
 		data->reference_voltage = 7;
 		data->efuse_value = 55;
@@ -875,14 +894,16 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 	case SOC_ARCH_EXYNOS5260:
 	case SOC_ARCH_EXYNOS5420:
 	case SOC_ARCH_EXYNOS5420_TRIMINFO:
-		data->tmu_set_trip_temp = exynos4412_tmu_set_trip_temp;
-		data->tmu_set_trip_hyst = exynos4412_tmu_set_trip_hyst;
+		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->ntrip = 4;
 		data->gain = 8;
 		data->reference_voltage = 16;
 		data->efuse_value = 55;
@@ -894,14 +915,16 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->max_efuse_value = 100;
 		break;
 	case SOC_ARCH_EXYNOS5433:
-		data->tmu_set_trip_temp = exynos5433_tmu_set_trip_temp;
-		data->tmu_set_trip_hyst = exynos5433_tmu_set_trip_hyst;
+		data->tmu_set_low_temp = exynos5433_tmu_set_low_temp;
+		data->tmu_set_high_temp = exynos5433_tmu_set_high_temp;
+		data->tmu_disable_low = exynos5433_tmu_disable_low;
+		data->tmu_disable_high = exynos5433_tmu_disable_high;
+		data->tmu_set_crit_temp = exynos5433_tmu_set_crit_temp;
 		data->tmu_initialize = exynos5433_tmu_initialize;
 		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->ntrip = 8;
 		data->gain = 8;
 		if (res.start == EXYNOS5433_G3D_BASE)
 			data->reference_voltage = 23;
@@ -912,14 +935,16 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->max_efuse_value = 150;
 		break;
 	case SOC_ARCH_EXYNOS7:
-		data->tmu_set_trip_temp = exynos7_tmu_set_trip_temp;
-		data->tmu_set_trip_hyst = exynos7_tmu_set_trip_hyst;
+		data->tmu_set_low_temp = exynos7_tmu_set_low_temp;
+		data->tmu_set_high_temp = exynos7_tmu_set_high_temp;
+		data->tmu_disable_low = exynos5433_tmu_disable_low;
+		data->tmu_disable_high = exynos5433_tmu_disable_high;
+		data->tmu_set_crit_temp = exynos7_tmu_set_crit_temp;
 		data->tmu_initialize = exynos7_tmu_initialize;
 		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->ntrip = 8;
 		data->gain = 9;
 		data->reference_voltage = 17;
 		data->efuse_value = 75;
@@ -955,9 +980,28 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 	return 0;
 }
 
+static int exynos_set_trips(struct thermal_zone_device *tz, int low, int high)
+{
+	struct exynos_tmu_data *data = thermal_zone_device_priv(tz);
+
+	mutex_lock(&data->lock);
+	if (low > INT_MIN)
+		data->tmu_set_low_temp(data, low / MCELSIUS);
+	else
+		data->tmu_disable_low(data);
+	if (high < INT_MAX)
+		data->tmu_set_high_temp(data, high / MCELSIUS);
+	else
+		data->tmu_disable_high(data);
+	mutex_unlock(&data->lock);
+
+	return 0;
+}
+
 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,
 };
 
 static int exynos_tmu_probe(struct platform_device *pdev)
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 11/11] ARM: dts: exynos: disable polling in Odroid XU3-related devices

[Mateusz Majewski]

After having switched to dynamic trip points, we no longer have a
hardware limit for trip point count and can support as many as we want
without polling.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 .../dts/samsung/exynos5422-odroidxu3-common.dtsi | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/samsung/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/samsung/exynos5422-odroidxu3-common.dtsi
index b4a851aa8881..4a4c55a4beb3 100644
--- a/arch/arm/boot/dts/samsung/exynos5422-odroidxu3-common.dtsi
+++ b/arch/arm/boot/dts/samsung/exynos5422-odroidxu3-common.dtsi
@@ -55,7 +55,7 @@ fan0: pwm-fan {
 	thermal-zones {
 		cpu0_thermal: cpu0-thermal {
 			thermal-sensors = <&tmu_cpu0>;
-			polling-delay-passive = <250>;
+			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			trips {
 				cpu0_alert0: cpu-alert-0 {
@@ -78,12 +78,6 @@ cpu0_crit0: cpu-crit-0 {
 					hysteresis = <0>; /* millicelsius */
 					type = "critical";
 				};
-				/*
-				 * Exynos542x supports only 4 trip-points
-				 * so for these polling mode is required.
-				 * Start polling at temperature level of last
-				 * interrupt-driven trip: cpu0_alert2
-				 */
 				cpu0_alert3: cpu-alert-3 {
 					temperature = <70000>; /* millicelsius */
 					hysteresis = <10000>; /* millicelsius */
@@ -144,7 +138,7 @@ cpu0_cooling_map4: map4 {
 		};
 		cpu1_thermal: cpu1-thermal {
 			thermal-sensors = <&tmu_cpu1>;
-			polling-delay-passive = <250>;
+			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			trips {
 				cpu1_alert0: cpu-alert-0 {
@@ -217,7 +211,7 @@ cpu1_cooling_map4: map4 {
 		};
 		cpu2_thermal: cpu2-thermal {
 			thermal-sensors = <&tmu_cpu2>;
-			polling-delay-passive = <250>;
+			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			trips {
 				cpu2_alert0: cpu-alert-0 {
@@ -290,7 +284,7 @@ cpu2_cooling_map4: map4 {
 		};
 		cpu3_thermal: cpu3-thermal {
 			thermal-sensors = <&tmu_cpu3>;
-			polling-delay-passive = <250>;
+			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			trips {
 				cpu3_alert0: cpu-alert-0 {
@@ -363,7 +357,7 @@ cpu3_cooling_map4: map4 {
 		};
 		gpu_thermal: gpu-thermal {
 			thermal-sensors = <&tmu_gpu>;
-			polling-delay-passive = <250>;
+			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			trips {
 				gpu_alert0: gpu-alert-0 {
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/11] ARM: dts: exynos: enable polling in Exynos 4210

[Krzysztof Kozlowski]

On 29/08/2023 11:18, Mateusz Majewski wrote:
> It seems that thermal in Exynos 4210 is broken without this, as it will
> never decrease cooling after increasing it.
> 
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---
>  arch/arm/boot/dts/samsung/exynos4210.dtsi | 10 ++++++++--

Please split unrelated patches for different subsystems into separate
patchsets.

>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/samsung/exynos4210.dtsi b/arch/arm/boot/dts/samsung/exynos4210.dtsi
> index 0e27c3375e2e..aae185b7f91c 100644
> --- a/arch/arm/boot/dts/samsung/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/samsung/exynos4210.dtsi
> @@ -391,8 +391,14 @@ &cpu_alert2 {
>  };
>  
>  &cpu_thermal {
> -	polling-delay-passive = <0>;
> -	polling-delay = <0>;
> +	/* Exynos 4210 supports thermal interrupts, but only for the rising threshold.

Linux coding style comments are:

/*
 * Foo bar


Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 02/11] thermal: exynos: drop id field

[Krzysztof Kozlowski]

On 29/08/2023 11:18, Mateusz Majewski wrote:
> This field is not used in code, and seems to not have any meaning; in my
> tests, the value was always 0.
> 
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---

Please drop also remaining alias from DTS.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 03/11] thermal: exynos: switch from workqueue-driven interrupt handling to threaded interrupts

[Krzysztof Kozlowski]

On 29/08/2023 11:18, Mateusz Majewski wrote:
> The workqueue boilerplate is mostly one-to-one what the threaded
> interrupts do.
> 
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>


>  	data->clk = devm_clk_get(&pdev->dev, "tmu_apbif");
>  	if (IS_ERR(data->clk)) {
>  		dev_err(&pdev->dev, "Failed to get clock\n");
> @@ -1094,8 +1080,10 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>  		goto err_sclk;
>  	}
>  
> -	ret = devm_request_irq(&pdev->dev, data->irq, exynos_tmu_irq,
> -		IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data);
> +	ret = devm_request_threaded_irq(
> +		&pdev->dev, data->irq, NULL, exynos_tmu_threaded_irq,

This does not look properly aligned.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 04/11] thermal: exynos: remove fine-grained clk management

[Krzysztof Kozlowski]

On 29/08/2023 11:18, Mateusz Majewski wrote:
> This clock only controls the register operations. The gain in power
> efficiency is therefore quite dubious, while there is price of added
> complexity that is important to get right (as a register operation might
> outright hang the CPU if the clock is not enabled).

So once it is done right, this stops being argument. The benefit is to
keep this clock disabled most of the time, which now we lost.

I don't find this patch correct approach.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 08/11] thermal: exynos: stop using the threshold mechanism on Exynos 4210

[Krzysztof Kozlowski]

On 29/08/2023 11:18, Mateusz Majewski wrote:
> Exynos 4210 supports setting a base threshold value, which is added to
> all trip points. This might be useful, but is not really necessary in
> our usecase, so we always set it to 0 to simplify the code a bit.
> 
> Additionally, this change makes it so that we convert the value to the
> calibrated one in a slightly different place. This is more correct
> morally, though it does not make any change when single-point

I don't think code placement is an aspect of morality, yet okay:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 11/11] ARM: dts: exynos: disable polling in Odroid XU3-related devices

[Krzysztof Kozlowski]

On 29/08/2023 11:18, Mateusz Majewski wrote:
> After having switched to dynamic trip points, we no longer have a
> hardware limit for trip point count and can support as many as we want
> without polling.
> 
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---
>  .../dts/samsung/exynos5422-odroidxu3-common.dtsi | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 

This will need to wait for release after driver changes get accepted.
You sent your patchset just after merge window started, so unfortunately
this means it will get in 2 releases.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 10/11] thermal: exynos: use set_trips

[kernel test robot]

Hi Mateusz,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/thermal]
[also build test WARNING on krzk/for-next arm/for-next arm/fixes arm64/for-next/core clk/clk-next kvmarm/next rockchip/for-next shawnguo/for-next soc/for-next linus/master v6.5 next-20230829]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mateusz-Majewski/ARM-dts-exynos-enable-polling-in-Exynos-4210/20230829-172850
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal
patch link:    https://lore.kernel.org/r/20230829091853.626011-11-m.majewski2%40samsung.com
patch subject: [PATCH 10/11] thermal: exynos: use set_trips
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230829/202308291857.W4jDgr7Y-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230829/202308291857.W4jDgr7Y-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308291857.W4jDgr7Y-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/thermal/samsung/exynos_tmu.c:198: warning: Function parameter or member 'tmu_set_low_temp' not described in 'exynos_tmu_data'
>> drivers/thermal/samsung/exynos_tmu.c:198: warning: Function parameter or member 'tmu_set_high_temp' not described in 'exynos_tmu_data'
>> drivers/thermal/samsung/exynos_tmu.c:198: warning: Function parameter or member 'tmu_set_crit_temp' not described in 'exynos_tmu_data'
>> drivers/thermal/samsung/exynos_tmu.c:198: warning: Function parameter or member 'tmu_disable_low' not described in 'exynos_tmu_data'
>> drivers/thermal/samsung/exynos_tmu.c:198: warning: Function parameter or member 'tmu_disable_high' not described in 'exynos_tmu_data'


vim +198 drivers/thermal/samsung/exynos_tmu.c

7efd18a2a181551 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  137  
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  138  /**
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  139   * struct exynos_tmu_data : A structure to hold the private data of the TMU
9625e9e694e7470 drivers/thermal/samsung/exynos_tmu.c Amit Kucheria             2019-11-20  140   *			    driver
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  141   * @base: base address of the single instance of the TMU controller.
9025d563cd9bd14 drivers/thermal/samsung/exynos_tmu.c Naveen Krishna Chatradhi  2013-12-19  142   * @base_second: base address of the common registers of the TMU controller.
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  143   * @irq: irq number of the TMU controller.
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  144   * @soc: id of the SOC type.
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  145   * @lock: lock to implement synchronization.
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  146   * @clk: pointer to the clock structure.
14a11dc7e0dbf4a drivers/thermal/samsung/exynos_tmu.c Naveen Krishna Chatradhi  2013-12-19  147   * @clk_sec: pointer to the clock structure for accessing the base_second.
6c247393cfdd669 drivers/thermal/samsung/exynos_tmu.c Abhilash Kesavan          2015-01-27  148   * @sclk: pointer to the clock structure for accessing the tmu special clk.
199b3e3c860cdf3 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  149   * @cal_type: calibration type for temperature
e3ed36499bc9565 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  150   * @efuse_value: SoC defined fuse value
e3ed36499bc9565 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  151   * @min_efuse_value: minimum valid trimming data
e3ed36499bc9565 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  152   * @max_efuse_value: maximum valid trimming data
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  153   * @temp_error1: fused value of the first point trim.
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  154   * @temp_error2: fused value of the second point trim.
fccfe0993b5dc55 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  155   * @gain: gain of amplifier in the positive-TC generator block
fccfe0993b5dc55 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  156   *	0 < gain <= 15
61020d189dbc4a7 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  157   * @reference_voltage: reference voltage of amplifier
61020d189dbc4a7 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  158   *	in the positive-TC generator block
61020d189dbc4a7 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  159   *	0 < reference_voltage <= 31
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  160   * @reg_conf: pointer to structure to register with core thermal.
9625e9e694e7470 drivers/thermal/samsung/exynos_tmu.c Amit Kucheria             2019-11-20  161   * @tzd: pointer to thermal_zone_device structure
88fc6f73fddf64e drivers/thermal/samsung/exynos_tmu.c Marek Szyprowski          2018-04-16  162   * @enabled: current status of TMU device
9625e9e694e7470 drivers/thermal/samsung/exynos_tmu.c Amit Kucheria             2019-11-20  163   * @tmu_set_trip_temp: SoC specific method to set trip (rising threshold)
9625e9e694e7470 drivers/thermal/samsung/exynos_tmu.c Amit Kucheria             2019-11-20  164   * @tmu_set_trip_hyst: SoC specific to set hysteresis (falling threshold)
72d1100b736d2ff drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13  165   * @tmu_initialize: SoC specific TMU initialization method
37f9034f99c3c1b drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13  166   * @tmu_control: SoC specific TMU control method
b79985ca74b2592 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13  167   * @tmu_read: SoC specific TMU temperature read method
285d994a51e45ca drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13  168   * @tmu_set_emulation: SoC specific TMU emulation setting method
a7331f72d3eb2bf drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13  169   * @tmu_clear_irqs: SoC specific TMU interrupts clearing method
cebe7373a7e659d drivers/thermal/samsung/exynos_tmu.c Amit Daniel Kachhap       2013-06-24  170   */
f22d9c03ccc9339 drivers/thermal/exynos_thermal.c     Amit Daniel Kachhap       2012-08-16  171  struct exynos_tmu_data {
9d97e5c81e15afa drivers/hwmon/exynos4_tmu.c          Donggeun Kim              2011-09-07  172  	void __iomem *base;
9025d563cd9bd14 drivers/thermal/samsung/exynos_tmu.c Naveen Krishna Chatradhi  2013-12-19  173  	void __iomem *base_second;
9d97e5c81e15afa drivers/hwmon/exynos4_tmu.c          Donggeun Kim              2011-09-07  174  	int irq;
f22d9c03ccc9339 drivers/thermal/exynos_thermal.c     Amit Daniel Kachhap       2012-08-16  175  	enum soc_type soc;
9d97e5c81e15afa drivers/hwmon/exynos4_tmu.c          Donggeun Kim              2011-09-07  176  	struct mutex lock;
6c247393cfdd669 drivers/thermal/samsung/exynos_tmu.c Abhilash Kesavan          2015-01-27  177  	struct clk *clk, *clk_sec, *sclk;
199b3e3c860cdf3 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  178  	u32 cal_type;
e3ed36499bc9565 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  179  	u32 efuse_value;
e3ed36499bc9565 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  180  	u32 min_efuse_value;
e3ed36499bc9565 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  181  	u32 max_efuse_value;
6c247393cfdd669 drivers/thermal/samsung/exynos_tmu.c Abhilash Kesavan          2015-01-27  182  	u16 temp_error1, temp_error2;
fccfe0993b5dc55 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  183  	u8 gain;
61020d189dbc4a7 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-16  184  	u8 reference_voltage;
3b6a1a805f34247 drivers/thermal/samsung/exynos_tmu.c Lukasz Majewski           2015-01-23  185  	struct thermal_zone_device *tzd;
88fc6f73fddf64e drivers/thermal/samsung/exynos_tmu.c Marek Szyprowski          2018-04-16  186  	bool enabled;
3b6a1a805f34247 drivers/thermal/samsung/exynos_tmu.c Lukasz Majewski           2015-01-23  187  
93106a41fa1d04f drivers/thermal/samsung/exynos_tmu.c Mateusz Majewski          2023-08-29  188  	void (*tmu_set_low_temp)(struct exynos_tmu_data *data, u8 temp);
93106a41fa1d04f drivers/thermal/samsung/exynos_tmu.c Mateusz Majewski          2023-08-29  189  	void (*tmu_set_high_temp)(struct exynos_tmu_data *data, u8 temp);
93106a41fa1d04f drivers/thermal/samsung/exynos_tmu.c Mateusz Majewski          2023-08-29  190  	void (*tmu_set_crit_temp)(struct exynos_tmu_data *data, u8 temp);
93106a41fa1d04f drivers/thermal/samsung/exynos_tmu.c Mateusz Majewski          2023-08-29  191  	void (*tmu_disable_low)(struct exynos_tmu_data *data);
93106a41fa1d04f drivers/thermal/samsung/exynos_tmu.c Mateusz Majewski          2023-08-29  192  	void (*tmu_disable_high)(struct exynos_tmu_data *data);
c35268f589d545f drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2018-04-26  193  	void (*tmu_initialize)(struct platform_device *pdev);
37f9034f99c3c1b drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13  194  	void (*tmu_control)(struct platform_device *pdev, bool on);
b79985ca74b2592 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13  195  	int (*tmu_read)(struct exynos_tmu_data *data);
17e8351a77397e8 drivers/thermal/samsung/exynos_tmu.c Sascha Hauer              2015-07-24  196  	void (*tmu_set_emulation)(struct exynos_tmu_data *data, int temp);
a7331f72d3eb2bf drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13  197  	void (*tmu_clear_irqs)(struct exynos_tmu_data *data);
9d97e5c81e15afa drivers/hwmon/exynos4_tmu.c          Donggeun Kim              2011-09-07 @198  };
9d97e5c81e15afa drivers/hwmon/exynos4_tmu.c          Donggeun Kim              2011-09-07  199  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 04/11] thermal: exynos: remove fine-grained clk management

[Marek Szyprowski]

On 29.08.2023 11:56, Krzysztof Kozlowski wrote:
> On 29/08/2023 11:18, Mateusz Majewski wrote:
>> This clock only controls the register operations. The gain in power
>> efficiency is therefore quite dubious, while there is price of added
>> complexity that is important to get right (as a register operation might
>> outright hang the CPU if the clock is not enabled).
> So once it is done right, this stops being argument. The benefit is to
> keep this clock disabled most of the time, which now we lost.
>
> I don't find this patch correct approach.

I've suggested this change while playing with this driver.

For me turning AHB clock on/off during normal driver operation seems to 
be over-engineering and really gives no real power saving benefits, 
especially if thermal driver is the only one that does such fine-grained 
clock management (none of the Exynos supported in mainline does that). 
Removing it simplifies code and makes it easier to understand or read, 
as the current code already was somehow problematic to understand and 
unintuitive:

https://lore.kernel.org/all/c3258cb2-9a56-d048-5738-1132331a157d@linaro.org/

Taking into account that the driver is not really maintained, making it 
simpler without noticeable feature loss counts as a benefit for me.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 04/11] thermal: exynos: remove fine-grained clk management

[Krzysztof Kozlowski]

On 01/09/2023 10:40, Marek Szyprowski wrote:
> On 29.08.2023 11:56, Krzysztof Kozlowski wrote:
>> On 29/08/2023 11:18, Mateusz Majewski wrote:
>>> This clock only controls the register operations. The gain in power
>>> efficiency is therefore quite dubious, while there is price of added
>>> complexity that is important to get right (as a register operation might
>>> outright hang the CPU if the clock is not enabled).
>> So once it is done right, this stops being argument. The benefit is to
>> keep this clock disabled most of the time, which now we lost.
>>
>> I don't find this patch correct approach.
> 
> I've suggested this change while playing with this driver.
> 
> For me turning AHB clock on/off during normal driver operation seems to 
> be over-engineering and really gives no real power saving benefits, 
> especially if thermal driver is the only one that does such fine-grained 
> clock management (none of the Exynos supported in mainline does that). 
> Removing it simplifies code and makes it easier to understand or read, 
> as the current code already was somehow problematic to understand and 
> unintuitive:
> 
> https://lore.kernel.org/all/c3258cb2-9a56-d048-5738-1132331a157d@linaro.org/
> 
> Taking into account that the driver is not really maintained, making it 
> simpler without noticeable feature loss counts as a benefit for me.

Hm, ok, let it be, although I bet once someone will come and start
adding runtime PM for clock handling...

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel