Re: [PATCH 0/6] Add initial Exynos 850 support to the thermal driver

[Mateusz Majewski <m.majewski2@xxxxxxxxxxx>]

Hi :)

> Thank you for the contribution! Did you by chance test it on any
> hardware, perhaps on E850-96 board? Just noticed there are no dts
> changes in this series (or as separate patches). If no -- I'll be glad
> to assist you on that, if you can share dts definitions for E850-96
> and the testing instructions with me.

I did test it on our copy of E850-96. I used this for testing:

diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
index f1c8b4613cbc..cffc173fd059 100644
--- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
@@ -617,6 +617,53 @@ sysreg_cmgp: syscon@11c20000 {
 			clocks = <&cmu_cmgp CLK_GOUT_SYSREG_CMGP_PCLK>;
 		};
 
+		tmuctrl_0: tmu@10070000{
+			compatible = "samsung,exynos850-tmu";
+			reg = <0x10070000 0x800>;
+			interrupts = <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>;
+			#thermal-sensor-cells = <0>;
+		};
+
+		thermal-zones {
+			test_thermal: test-thermal{
+				polling-delay-passive = <0>;
+				polling-delay = <0>;
+				thermal-sensors = <&tmuctrl_0>;
+				trips {
+					test_40000: test-40000 {
+						temperature = <40000>;
+						hysteresis = <1000>;
+						type = "passive";
+					};
+					test_50000: test-50000 {
+						temperature = <50000>;
+						hysteresis = <1000>;
+						type = "passive";
+					};
+					test_60000: test-60000 {
+						temperature = <60000>;
+						hysteresis = <1000>;
+						type = "passive";
+					};
+					test_70000: test-70000 {
+						temperature = <70000>;
+						hysteresis = <1000>;
+						type = "passive";
+					};
+					test_80000: test-80000 {
+						temperature = <80000>;
+						hysteresis = <1000>;
+						type = "passive";
+					};
+					test_crit: test-crit {
+						temperature = <90000>;
+						hysteresis = <1000>;
+						type = "critical";
+					};
+				};
+			};
+		};
+
 		usbdrd: usb@13600000 {
 			compatible = "samsung,exynos850-dwusb3";
 			ranges = <0x0 0x13600000 0x10000>;

I did not post this because it would probably need some more thought,
and it probably wouldn't get merged until this series does anyway I
think? During testing I couldn't get readings higher than 35C, but the
values reacted to CPU load as expected. Also, Marek had physical access
to the board while I was testing it and has confirmed that the values
are realistic. Some more testing was done with some combination of
lowering the trip points, emul_temp and those temporary changes:

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index bd52663f1a5a..8db3f9039e7a 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -941,6 +941,7 @@ static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
 {
 	struct exynos_tmu_data *data = id;
 
+	pr_info("interrupt\n");
 	thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED);
 
 	mutex_lock(&data->lock);
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 72b302bf914e..0864179526e2 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -232,13 +232,11 @@ emul_temp_store(struct device *dev, struct device_attribute *attr,
 
 	mutex_lock(&tz->lock);
 
-	if (!tz->ops.set_emul_temp)
+	if (!tz->ops.set_emul_temp) {
 		tz->emul_temperature = temperature;
-	else
-		ret = tz->ops.set_emul_temp(tz, temperature);
-
-	if (!ret)
 		__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+	} else
+		ret = tz->ops.set_emul_temp(tz, temperature);
 
 	mutex_unlock(&tz->lock);