[PATCH v2 0/6] Add initial Exynos850 support to the thermal driver

[PATCH v2 0/6] Add initial Exynos850 support to the thermal driver

[Mateusz Majewski]

This series adds initial Exynos850 support to the thermal driver
together with its requirements (sanitize_temp_error fix, adding the new
string to dt-bindings), while also cleaning up a bit (improving power
management support and removing some outdated information from
dt-bindings).

Changelog:
 v2:
   - Reimplemented to use the Exynos850 TMU clock: removed the patch to
     make the clock optional and changed dt-bindings change accordingly
   - Improved the Exynos850 implementation itself (style and one correct
     register offset)
   - Removed conditional compilation in favor of pm_sleep_ptr
   - Shortened dt-bindings description

Mateusz Majewski (6):
  drivers/thermal/exynos: use DEFINE_SIMPLE_DEV_PM_OPS
  drivers/thermal/exynos: use pm_sleep_ptr instead of conditional
    compilation
  drivers/thermal/exynos: improve sanitize_temp_error
  dt-bindings: thermal: samsung,exynos: add exynos850-tmu string
  drivers/thermal/exynos: add initial Exynos850 support
  dt-bindings: thermal: samsung,exynos: remove outdated information on
    trip point count

 .../thermal/samsung,exynos-thermal.yaml       |   8 +-
 drivers/thermal/samsung/exynos_tmu.c          | 219 ++++++++++++++++--
 2 files changed, 199 insertions(+), 28 deletions(-)

-- 
2.45.1

[PATCH v2 1/6] drivers/thermal/exynos: use DEFINE_SIMPLE_DEV_PM_OPS

[Mateusz Majewski]

SIMPLE_DEV_PM_OPS is deprecated, as noted next to its definition.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
---
 drivers/thermal/samsung/exynos_tmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 96cffb2c44ba..9b7ca93a72f1 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -1150,8 +1150,8 @@ static int exynos_tmu_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(exynos_tmu_pm,
-			 exynos_tmu_suspend, exynos_tmu_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(exynos_tmu_pm,
+				exynos_tmu_suspend, exynos_tmu_resume);
 #define EXYNOS_TMU_PM	(&exynos_tmu_pm)
 #else
 #define EXYNOS_TMU_PM	NULL
-- 
2.45.1

[PATCH v2 2/6] drivers/thermal/exynos: use pm_sleep_ptr instead of conditional compilation

[Mateusz Majewski]

Slightly simpler and nothing is lost if _suspend and _resume functions
are built unconditionally.

Suggested-by: Anand Moon <linux.amoon@gmail.com>
Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 9b7ca93a72f1..b68e9755c933 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -1132,7 +1132,6 @@ static void exynos_tmu_remove(struct platform_device *pdev)
 		clk_unprepare(data->clk_sec);
 }
 
-#ifdef CONFIG_PM_SLEEP
 static int exynos_tmu_suspend(struct device *dev)
 {
 	exynos_tmu_control(to_platform_device(dev), false);
@@ -1152,15 +1151,11 @@ static int exynos_tmu_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(exynos_tmu_pm,
 				exynos_tmu_suspend, exynos_tmu_resume);
-#define EXYNOS_TMU_PM	(&exynos_tmu_pm)
-#else
-#define EXYNOS_TMU_PM	NULL
-#endif
 
 static struct platform_driver exynos_tmu_driver = {
 	.driver = {
 		.name   = "exynos-tmu",
-		.pm     = EXYNOS_TMU_PM,
+		.pm     = pm_sleep_ptr(&exynos_tmu_pm),
 		.of_match_table = exynos_tmu_match,
 	},
 	.probe = exynos_tmu_probe,
-- 
2.45.1

[PATCH v2 3/6] drivers/thermal/exynos: improve sanitize_temp_error

[Mateusz Majewski]

There are two minor issues regarding this function.

One is that it attempts to calculate the second calibration value even
if 1-point trimming is being used; in this case, the calculated value is
probably not useful and is never used anyway. Changing this also
requires a minor reordering in Exynos5433 initialization function, so
that we know which type of trimming is used before we call
sanitize_temp_error.

The second issue is that the function is not very consistent when it
comes to the use of Exynos7-specific parameters. This seems to not be an
issue in practice, in part because some of these issues are related to
the mentioned calculation of the second calibration value. However,
fixing this makes the code a bit less confusing, and will be required
for Exynos850 which has 9-bit temperature values and uses 2-point
trimming.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
v1 -> v2: reworked to change shift instead of only mask and to also fix
  the 2-point trimming issue.

 drivers/thermal/samsung/exynos_tmu.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index b68e9755c933..087a09628e23 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -111,6 +111,7 @@
 #define EXYNOS7_TMU_REG_EMUL_CON		0x160
 
 #define EXYNOS7_TMU_TEMP_MASK			0x1ff
+#define EXYNOS7_TMU_TEMP_SHIFT			9
 #define EXYNOS7_PD_DET_EN_SHIFT			23
 #define EXYNOS7_TMU_INTEN_RISE0_SHIFT		0
 #define EXYNOS7_EMUL_DATA_SHIFT			7
@@ -234,20 +235,23 @@ static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
 	u16 tmu_temp_mask =
 		(data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_MASK
 						: EXYNOS_TMU_TEMP_MASK;
+	int tmu_85_shift =
+		(data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_SHIFT
+						: EXYNOS_TRIMINFO_85_SHIFT;
 
 	data->temp_error1 = trim_info & tmu_temp_mask;
-	data->temp_error2 = ((trim_info >> EXYNOS_TRIMINFO_85_SHIFT) &
-				EXYNOS_TMU_TEMP_MASK);
-
 	if (!data->temp_error1 ||
 	    (data->min_efuse_value > data->temp_error1) ||
 	    (data->temp_error1 > data->max_efuse_value))
-		data->temp_error1 = data->efuse_value & EXYNOS_TMU_TEMP_MASK;
+		data->temp_error1 = data->efuse_value & tmu_temp_mask;
 
-	if (!data->temp_error2)
-		data->temp_error2 =
-			(data->efuse_value >> EXYNOS_TRIMINFO_85_SHIFT) &
-			EXYNOS_TMU_TEMP_MASK;
+	if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
+		data->temp_error2 = (trim_info >> tmu_85_shift) & tmu_temp_mask;
+		if (!data->temp_error2)
+			data->temp_error2 =
+				(data->efuse_value >> tmu_85_shift) &
+				tmu_temp_mask;
+	}
 }
 
 static int exynos_tmu_initialize(struct platform_device *pdev)
@@ -510,7 +514,6 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev)
 	int sensor_id, cal_type;
 
 	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
-	sanitize_temp_error(data, trim_info);
 
 	/* Read the temperature sensor id */
 	sensor_id = (trim_info & EXYNOS5433_TRIMINFO_SENSOR_ID_MASK)
@@ -532,6 +535,8 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev)
 		break;
 	}
 
+	sanitize_temp_error(data, trim_info);
+
 	dev_info(&pdev->dev, "Calibration type is %d-point calibration\n",
 			cal_type ?  2 : 1);
 }
-- 
2.45.1

[PATCH v2 4/6] dt-bindings: thermal: samsung,exynos: add exynos850-tmu string

[Mateusz Majewski]

Like most of the SoCs, it requires 1 clock and 1 register.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
v1 -> v2: make the clock required in Exynos850.

 .../devicetree/bindings/thermal/samsung,exynos-thermal.yaml     | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
index 29a08b0729ee..b8c0bb7f4263 100644
--- a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
@@ -27,6 +27,7 @@ properties:
       - samsung,exynos5420-tmu-ext-triminfo
       - samsung,exynos5433-tmu
       - samsung,exynos7-tmu
+      - samsung,exynos850-tmu
 
   clocks:
     minItems: 1
@@ -131,6 +132,7 @@ allOf:
               - samsung,exynos5250-tmu
               - samsung,exynos5260-tmu
               - samsung,exynos5420-tmu
+              - samsung,exynos850-tmu
     then:
       properties:
         clocks:
-- 
2.45.1

[PATCH v2 5/6] drivers/thermal/exynos: add initial Exynos850 support

[Mateusz Majewski]

This is loosely adapted from an implementation available at
https://gitlab.com/Linaro/96boards/e850-96/kernel/-/blob/android-exynos-4.14-linaro/drivers/thermal/samsung/exynos_tmu.c
Some differences from that implementation:
- unlike that implementation, we do not use the ACPM mechanism, instead
  we just access the registers, like we do for other SoCs,
- the SoC is supposed to support multiple sensors inside one unit. The
  vendor implementation uses one kernel device per sensor, we would
  probably prefer to have one device for all sensors, have
  #thermal-sensor-cells = <1> and so on. We implemented this, but we
  could not get the extra sensors to work on our hardware so far. This
  might be due to a misconfiguration and we will probably come back to
  this, however our implementation only supports a single sensor for
  now,
- the vendor implementation supports disabling CPU cores as a cooling
  device. We did not attempt to port this, and this would not really fit
  this driver anyway.

Additionally, some differences from the other SoCs supported by this
driver:
- we do not really constrain the e-fuse information like the other SoCs
  do (data->{min,max}_efuse_value). In our tests, those values (as well
  as the raw sensor values) were much higher than in the other SoCs, to
  the degree that reusing the data->{min,max}_efuse_value from the other
  SoCs would cause instant critical temperature reset on boot,
- this SoC provides more information in the e-fuse data than other SoCs,
  so we read some values inside exynos850_tmu_initialize instead of
  hardcoding them in exynos_map_dt_data.

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
v1 -> v2: rename and reorder some registers, use the correct register
  offset for EXYNOS850_TMU_REG_AVG_CON, make the clock required,
  additionally do some minor style changes.

 drivers/thermal/samsung/exynos_tmu.c | 191 +++++++++++++++++++++++++--
 1 file changed, 182 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 087a09628e23..2618a81fca53 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -117,6 +117,41 @@
 #define EXYNOS7_EMUL_DATA_SHIFT			7
 #define EXYNOS7_EMUL_DATA_MASK			0x1ff
 
+/* Exynos850 specific registers */
+#define EXYNOS850_TMU_REG_CURRENT_TEMP0_1	0x40
+#define EXYNOS850_TMU_REG_THD_TEMP0_RISE	0x50
+#define EXYNOS850_TMU_REG_THD_TEMP0_FALL	0x60
+
+#define EXYNOS850_TMU_TRIMINFO_SHIFT		4
+#define EXYNOS850_TMU_TRIMINFO_OFFSET(n) \
+	(EXYNOS_TMU_REG_TRIMINFO + (n) * EXYNOS850_TMU_TRIMINFO_SHIFT)
+#define EXYNOS850_TMU_T_TRIM0_SHIFT		18
+
+#define EXYNOS850_TMU_REG_CONTROL1		0x24
+#define EXYNOS850_TMU_LPI_MODE_MASK		1
+#define EXYNOS850_TMU_LPI_MODE_SHIFT		10
+
+#define EXYNOS850_TMU_REG_COUNTER_VALUE0	0x30
+#define EXYNOS850_TMU_EN_TEMP_SEN_OFF_MASK	0xffff
+#define EXYNOS850_TMU_EN_TEMP_SEN_OFF_SHIFT	0
+
+#define EXYNOS850_TMU_REG_COUNTER_VALUE1	0x34
+#define EXYNOS850_TMU_CLK_SENSE_ON_MASK		0xffff
+#define EXYNOS850_TMU_CLK_SENSE_ON_SHIFT	16
+
+#define EXYNOS850_TMU_REG_AVG_CON		0x38
+#define EXYNOS850_TMU_AVG_MODE_MASK		0x7
+#define EXYNOS850_TMU_DEM_ENABLE		BIT(4)
+
+#define EXYNOS850_TMU_REG_TRIM0			0x3c
+#define EXYNOS850_TMU_TRIM0_MASK		0xf
+#define EXYNOS850_TMU_VBEI_TRIM_SHIFT		8
+#define EXYNOS850_TMU_VREF_TRIM_SHIFT		12
+#define EXYNOS850_TMU_BGRI_TRIM_SHIFT		20
+
+#define EXYNOS850_TMU_TEM1051X_SENSE_VALUE	0x028a
+#define EXYNOS850_TMU_TEM1456X_SENSE_VALUE	0x0a28
+
 #define EXYNOS_FIRST_POINT_TRIM			25
 #define EXYNOS_SECOND_POINT_TRIM		85
 
@@ -134,6 +169,7 @@ enum soc_type {
 	SOC_ARCH_EXYNOS5420_TRIMINFO,
 	SOC_ARCH_EXYNOS5433,
 	SOC_ARCH_EXYNOS7,
+	SOC_ARCH_EXYNOS850,
 };
 
 /**
@@ -232,12 +268,14 @@ static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code)
 
 static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
 {
-	u16 tmu_temp_mask =
-		(data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_MASK
-						: EXYNOS_TMU_TEMP_MASK;
-	int tmu_85_shift =
-		(data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_SHIFT
-						: EXYNOS_TRIMINFO_85_SHIFT;
+	u16 tmu_temp_mask = (data->soc == SOC_ARCH_EXYNOS7 ||
+			     data->soc == SOC_ARCH_EXYNOS850) ?
+				    EXYNOS7_TMU_TEMP_MASK :
+				    EXYNOS_TMU_TEMP_MASK;
+	int tmu_85_shift = (data->soc == SOC_ARCH_EXYNOS7 ||
+			    data->soc == SOC_ARCH_EXYNOS850) ?
+				   EXYNOS7_TMU_TEMP_SHIFT :
+				   EXYNOS_TRIMINFO_85_SHIFT;
 
 	data->temp_error1 = trim_info & tmu_temp_mask;
 	if (!data->temp_error1 ||
@@ -587,6 +625,114 @@ static void exynos7_tmu_initialize(struct platform_device *pdev)
 	sanitize_temp_error(data, trim_info);
 }
 
+static void exynos850_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	exynos_tmu_update_temp(data, EXYNOS850_TMU_REG_THD_TEMP0_FALL + 12, 0,
+			       temp);
+	exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_FALL0_SHIFT + 0, true);
+}
+
+static void exynos850_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	exynos_tmu_update_temp(data, EXYNOS850_TMU_REG_THD_TEMP0_RISE + 12, 16,
+			       temp);
+	exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
+			      EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, true);
+}
+
+static void exynos850_tmu_disable_low(struct exynos_tmu_data *data)
+{
+	exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
+			      EXYNOS_TMU_INTEN_FALL0_SHIFT + 0, false);
+}
+
+static void exynos850_tmu_disable_high(struct exynos_tmu_data *data)
+{
+	exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
+			      EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, false);
+}
+
+static void exynos850_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp)
+{
+	exynos_tmu_update_temp(data, EXYNOS850_TMU_REG_THD_TEMP0_RISE + 0, 16,
+			       temp);
+	exynos_tmu_update_bit(data, EXYNOS_TMU_REG_CONTROL,
+			      EXYNOS_TMU_THERM_TRIP_EN_SHIFT, true);
+	exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
+			      EXYNOS7_TMU_INTEN_RISE0_SHIFT + 7, true);
+}
+
+static void exynos850_tmu_initialize(struct platform_device *pdev)
+{
+	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
+	u32 cal_type, avg_mode, reg, bgri, vref, vbei;
+
+	reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(0));
+	cal_type = (reg & EXYNOS5433_TRIMINFO_CALIB_SEL_MASK) >>
+		   EXYNOS5433_TRIMINFO_CALIB_SEL_SHIFT;
+	data->reference_voltage = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) &
+				  EXYNOS_TMU_REF_VOLTAGE_MASK;
+	reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(1));
+	data->gain = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) &
+		     EXYNOS_TMU_BUF_SLOPE_SEL_MASK;
+	reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(2));
+	avg_mode = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) &
+		   EXYNOS850_TMU_AVG_MODE_MASK;
+	reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(3));
+	bgri = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) & EXYNOS850_TMU_TRIM0_MASK;
+	reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(4));
+	vref = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) & EXYNOS850_TMU_TRIM0_MASK;
+	reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(5));
+	vbei = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) & EXYNOS850_TMU_TRIM0_MASK;
+
+	data->cal_type = cal_type == EXYNOS5433_TRIMINFO_TWO_POINT_TRIMMING ?
+				 TYPE_TWO_POINT_TRIMMING :
+				 TYPE_ONE_POINT_TRIMMING;
+
+	reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(0));
+	sanitize_temp_error(data, reg);
+
+	dev_info(&pdev->dev, "Calibration type is %d-point calibration\n",
+		 cal_type ? 2 : 1);
+
+	reg = readl(data->base + EXYNOS850_TMU_REG_AVG_CON);
+	reg &= ~EXYNOS850_TMU_AVG_MODE_MASK;
+	reg &= ~EXYNOS850_TMU_DEM_ENABLE;
+	if (avg_mode) {
+		reg |= avg_mode;
+		reg |= EXYNOS850_TMU_DEM_ENABLE;
+	}
+	writel(reg, data->base + EXYNOS850_TMU_REG_AVG_CON);
+
+	reg = readl(data->base + EXYNOS850_TMU_REG_COUNTER_VALUE0);
+	reg &= ~(EXYNOS850_TMU_EN_TEMP_SEN_OFF_MASK
+		 << EXYNOS850_TMU_EN_TEMP_SEN_OFF_SHIFT);
+	reg |= EXYNOS850_TMU_TEM1051X_SENSE_VALUE
+	       << EXYNOS850_TMU_EN_TEMP_SEN_OFF_SHIFT;
+	writel(reg, data->base + EXYNOS850_TMU_REG_COUNTER_VALUE0);
+
+	reg = readl(data->base + EXYNOS850_TMU_REG_COUNTER_VALUE1);
+	reg &= ~(EXYNOS850_TMU_CLK_SENSE_ON_MASK
+		 << EXYNOS850_TMU_CLK_SENSE_ON_SHIFT);
+	reg |= EXYNOS850_TMU_TEM1051X_SENSE_VALUE
+	       << EXYNOS850_TMU_CLK_SENSE_ON_SHIFT;
+	writel(reg, data->base + EXYNOS850_TMU_REG_COUNTER_VALUE1);
+
+	reg = readl(data->base + EXYNOS850_TMU_REG_TRIM0);
+	reg &= ~(EXYNOS850_TMU_TRIM0_MASK << EXYNOS850_TMU_BGRI_TRIM_SHIFT);
+	reg &= ~(EXYNOS850_TMU_TRIM0_MASK << EXYNOS850_TMU_VREF_TRIM_SHIFT);
+	reg &= ~(EXYNOS850_TMU_TRIM0_MASK << EXYNOS850_TMU_VBEI_TRIM_SHIFT);
+	reg |= bgri << EXYNOS850_TMU_BGRI_TRIM_SHIFT;
+	reg |= vref << EXYNOS850_TMU_VREF_TRIM_SHIFT;
+	reg |= vbei << EXYNOS850_TMU_VBEI_TRIM_SHIFT;
+	writel(reg, data->base + EXYNOS850_TMU_REG_TRIM0);
+
+	reg = readl(data->base + EXYNOS850_TMU_REG_CONTROL1);
+	reg &= ~(EXYNOS850_TMU_LPI_MODE_MASK << EXYNOS850_TMU_LPI_MODE_SHIFT);
+	writel(reg, data->base + EXYNOS850_TMU_REG_CONTROL1);
+}
+
 static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
@@ -676,7 +822,8 @@ static u32 get_emul_con_reg(struct exynos_tmu_data *data, unsigned int val,
 
 		val &= ~(EXYNOS_EMUL_TIME_MASK << EXYNOS_EMUL_TIME_SHIFT);
 		val |= (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT);
-		if (data->soc == SOC_ARCH_EXYNOS7) {
+		if (data->soc == SOC_ARCH_EXYNOS7 ||
+		    data->soc == SOC_ARCH_EXYNOS850) {
 			val &= ~(EXYNOS7_EMUL_DATA_MASK <<
 				EXYNOS7_EMUL_DATA_SHIFT);
 			val |= (temp_to_code(data, temp) <<
@@ -706,7 +853,8 @@ static void exynos4412_tmu_set_emulation(struct exynos_tmu_data *data,
 		emul_con = EXYNOS5260_EMUL_CON;
 	else if (data->soc == SOC_ARCH_EXYNOS5433)
 		emul_con = EXYNOS5433_TMU_EMUL_CON;
-	else if (data->soc == SOC_ARCH_EXYNOS7)
+	else if (data->soc == SOC_ARCH_EXYNOS7 ||
+		 data->soc == SOC_ARCH_EXYNOS850)
 		emul_con = EXYNOS7_TMU_REG_EMUL_CON;
 	else
 		emul_con = EXYNOS_EMUL_CON;
@@ -761,6 +909,12 @@ static int exynos7_tmu_read(struct exynos_tmu_data *data)
 		EXYNOS7_TMU_TEMP_MASK;
 }
 
+static int exynos850_tmu_read(struct exynos_tmu_data *data)
+{
+	return readw(data->base + EXYNOS850_TMU_REG_CURRENT_TEMP0_1) &
+	       EXYNOS7_TMU_TEMP_MASK;
+}
+
 static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
 {
 	struct exynos_tmu_data *data = id;
@@ -787,7 +941,8 @@ static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data)
 	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) {
+	} else if (data->soc == SOC_ARCH_EXYNOS7 ||
+		   data->soc == SOC_ARCH_EXYNOS850) {
 		tmu_intstat = EXYNOS7_TMU_REG_INTPEND;
 		tmu_intclear = EXYNOS7_TMU_REG_INTPEND;
 	} else if (data->soc == SOC_ARCH_EXYNOS5433) {
@@ -838,6 +993,9 @@ static const struct of_device_id exynos_tmu_match[] = {
 	}, {
 		.compatible = "samsung,exynos7-tmu",
 		.data = (const void *)SOC_ARCH_EXYNOS7,
+	}, {
+		.compatible = "samsung,exynos850-tmu",
+		.data = (const void *)SOC_ARCH_EXYNOS850,
 	},
 	{ },
 };
@@ -950,6 +1108,21 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->min_efuse_value = 15;
 		data->max_efuse_value = 100;
 		break;
+	case SOC_ARCH_EXYNOS850:
+		data->tmu_set_low_temp = exynos850_tmu_set_low_temp;
+		data->tmu_set_high_temp = exynos850_tmu_set_high_temp;
+		data->tmu_disable_low = exynos850_tmu_disable_low;
+		data->tmu_disable_high = exynos850_tmu_disable_high;
+		data->tmu_set_crit_temp = exynos850_tmu_set_crit_temp;
+		data->tmu_initialize = exynos850_tmu_initialize;
+		data->tmu_control = exynos4210_tmu_control;
+		data->tmu_read = exynos850_tmu_read;
+		data->tmu_set_emulation = exynos4412_tmu_set_emulation;
+		data->tmu_clear_irqs = exynos4210_tmu_clear_irqs;
+		data->efuse_value = 55;
+		data->min_efuse_value = 0;
+		data->max_efuse_value = 511;
+		break;
 	default:
 		dev_err(&pdev->dev, "Platform not supported\n");
 		return -EINVAL;
-- 
2.45.1

[PATCH v2 6/6] dt-bindings: thermal: samsung,exynos: remove outdated information on trip point count

[Mateusz Majewski]

This is not true as of commit 5314b1543787 ("thermal/drivers/exynos: Use
set_trips ops").

Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
v1 -> v2: remove an unnecessary sentence.

 .../devicetree/bindings/thermal/samsung,exynos-thermal.yaml | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
index b8c0bb7f4263..b85b4c420cd3 100644
--- a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
@@ -40,11 +40,7 @@ properties:
   interrupts:
     description: |
       The Exynos TMU supports generating interrupts when reaching given
-      temperature thresholds. Number of supported thermal trip points depends
-      on the SoC (only first trip points defined in DT will be configured)::
-       - most of SoC: 4
-       - samsung,exynos5433-tmu: 8
-       - samsung,exynos7-tmu: 8
+      temperature thresholds.
     maxItems: 1
 
   reg:
-- 
2.45.1

RE: [PATCH v2 0/6] Add initial Exynos850 support to the thermal driver

[Mateusz Majewski]

Forgot to mention it in the cover letter, but as discussed in v1 in
https://lore.kernel.org/lkml/CAPLW+4nfEjP4FDjRJORyyKk46x4VfFAcMuK88jXUT_LJoP1N_g@mail.gmail.com,
this requires support for the TMU clock to run, available in
https://lore.kernel.org/lkml/20240723163311.28654-2-semen.protsenko@linaro.org.
This series builds fine without this, only it is not possible to write a
devicetree source for this without the mentioned series, so as I
understand it is ok for this to be in review anyway?

By the way, I am going to have some more time to help with the upstream
kernel, and have access to most of the supported SoCs. If you feel that
it is appropriate, I would be very happy to become one of the
maintainers of this driver :)

Re: [PATCH v2 0/6] Add initial Exynos850 support to the thermal driver

[Krzysztof Kozlowski]

On 26/07/2024 17:03, Mateusz Majewski wrote:
> Forgot to mention it in the cover letter, but as discussed in v1 in
> https://lore.kernel.org/lkml/CAPLW+4nfEjP4FDjRJORyyKk46x4VfFAcMuK88jXUT_LJoP1N_g@mail.gmail.com,
> this requires support for the TMU clock to run, available in
> https://lore.kernel.org/lkml/20240723163311.28654-2-semen.protsenko@linaro.org.
> This series builds fine without this, only it is not possible to write a
> devicetree source for this without the mentioned series, so as I
> understand it is ok for this to be in review anyway?

It's okay. Also okay for merging via thermal tree, after merge window.
Only DTS will depend on the clock binding patch, which you will have to
mention in cover letter or patch changelog (---).

> 
> By the way, I am going to have some more time to help with the upstream
> kernel, and have access to most of the supported SoCs. If you feel that
> it is appropriate, I would be very happy to become one of the
> maintainers of this driver :)

If you have time, I think it would be great. +1

Reviews, tests, cleanups and any other non-developer activities are
welcomed anyway, regardless whether you are listed as maintainer or not.
Just set yourself a lei filter for specific keywords (e.g. samsung-soc
list or dfn: for paths) and just review all the code on the lists.

Best regards,
Krzysztof

Re: [PATCH v2 2/6] drivers/thermal/exynos: use pm_sleep_ptr instead of conditional compilation

[Sam Protsenko]

On Fri, Jul 26, 2024 at 6:01 AM Mateusz Majewski
<m.majewski2@samsung.com> wrote:
>
> Slightly simpler and nothing is lost if _suspend and _resume functions
> are built unconditionally.
>
> Suggested-by: Anand Moon <linux.amoon@gmail.com>
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

>  drivers/thermal/samsung/exynos_tmu.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 9b7ca93a72f1..b68e9755c933 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -1132,7 +1132,6 @@ static void exynos_tmu_remove(struct platform_device *pdev)
>                 clk_unprepare(data->clk_sec);
>  }
>
> -#ifdef CONFIG_PM_SLEEP
>  static int exynos_tmu_suspend(struct device *dev)
>  {
>         exynos_tmu_control(to_platform_device(dev), false);
> @@ -1152,15 +1151,11 @@ static int exynos_tmu_resume(struct device *dev)
>
>  static DEFINE_SIMPLE_DEV_PM_OPS(exynos_tmu_pm,
>                                 exynos_tmu_suspend, exynos_tmu_resume);
> -#define EXYNOS_TMU_PM  (&exynos_tmu_pm)
> -#else
> -#define EXYNOS_TMU_PM  NULL
> -#endif
>
>  static struct platform_driver exynos_tmu_driver = {
>         .driver = {
>                 .name   = "exynos-tmu",
> -               .pm     = EXYNOS_TMU_PM,
> +               .pm     = pm_sleep_ptr(&exynos_tmu_pm),
>                 .of_match_table = exynos_tmu_match,
>         },
>         .probe = exynos_tmu_probe,
> --
> 2.45.1
>

Re: [PATCH v2 3/6] drivers/thermal/exynos: improve sanitize_temp_error

[Sam Protsenko]

On Fri, Jul 26, 2024 at 6:01 AM Mateusz Majewski
<m.majewski2@samsung.com> wrote:
>
> There are two minor issues regarding this function.
>
> One is that it attempts to calculate the second calibration value even
> if 1-point trimming is being used; in this case, the calculated value is
> probably not useful and is never used anyway. Changing this also
> requires a minor reordering in Exynos5433 initialization function, so
> that we know which type of trimming is used before we call
> sanitize_temp_error.
>
> The second issue is that the function is not very consistent when it
> comes to the use of Exynos7-specific parameters. This seems to not be an
> issue in practice, in part because some of these issues are related to
> the mentioned calculation of the second calibration value. However,
> fixing this makes the code a bit less confusing, and will be required
> for Exynos850 which has 9-bit temperature values and uses 2-point
> trimming.
>
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

> v1 -> v2: reworked to change shift instead of only mask and to also fix
>   the 2-point trimming issue.
>
>  drivers/thermal/samsung/exynos_tmu.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index b68e9755c933..087a09628e23 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -111,6 +111,7 @@
>  #define EXYNOS7_TMU_REG_EMUL_CON               0x160
>
>  #define EXYNOS7_TMU_TEMP_MASK                  0x1ff
> +#define EXYNOS7_TMU_TEMP_SHIFT                 9
>  #define EXYNOS7_PD_DET_EN_SHIFT                        23
>  #define EXYNOS7_TMU_INTEN_RISE0_SHIFT          0
>  #define EXYNOS7_EMUL_DATA_SHIFT                        7
> @@ -234,20 +235,23 @@ static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
>         u16 tmu_temp_mask =
>                 (data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_MASK
>                                                 : EXYNOS_TMU_TEMP_MASK;
> +       int tmu_85_shift =
> +               (data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_SHIFT
> +                                               : EXYNOS_TRIMINFO_85_SHIFT;
>
>         data->temp_error1 = trim_info & tmu_temp_mask;
> -       data->temp_error2 = ((trim_info >> EXYNOS_TRIMINFO_85_SHIFT) &
> -                               EXYNOS_TMU_TEMP_MASK);
> -
>         if (!data->temp_error1 ||
>             (data->min_efuse_value > data->temp_error1) ||
>             (data->temp_error1 > data->max_efuse_value))
> -               data->temp_error1 = data->efuse_value & EXYNOS_TMU_TEMP_MASK;
> +               data->temp_error1 = data->efuse_value & tmu_temp_mask;
>
> -       if (!data->temp_error2)
> -               data->temp_error2 =
> -                       (data->efuse_value >> EXYNOS_TRIMINFO_85_SHIFT) &
> -                       EXYNOS_TMU_TEMP_MASK;
> +       if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
> +               data->temp_error2 = (trim_info >> tmu_85_shift) & tmu_temp_mask;
> +               if (!data->temp_error2)
> +                       data->temp_error2 =
> +                               (data->efuse_value >> tmu_85_shift) &
> +                               tmu_temp_mask;
> +       }
>  }
>
>  static int exynos_tmu_initialize(struct platform_device *pdev)
> @@ -510,7 +514,6 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev)
>         int sensor_id, cal_type;
>
>         trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
> -       sanitize_temp_error(data, trim_info);
>
>         /* Read the temperature sensor id */
>         sensor_id = (trim_info & EXYNOS5433_TRIMINFO_SENSOR_ID_MASK)
> @@ -532,6 +535,8 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev)
>                 break;
>         }
>
> +       sanitize_temp_error(data, trim_info);
> +
>         dev_info(&pdev->dev, "Calibration type is %d-point calibration\n",
>                         cal_type ?  2 : 1);
>  }
> --
> 2.45.1
>

Re: [PATCH v2 4/6] dt-bindings: thermal: samsung,exynos: add exynos850-tmu string

[Sam Protsenko]

On Fri, Jul 26, 2024 at 6:01 AM Mateusz Majewski
<m.majewski2@samsung.com> wrote:
>
> Like most of the SoCs, it requires 1 clock and 1 register.
>
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

> v1 -> v2: make the clock required in Exynos850.
>
>  .../devicetree/bindings/thermal/samsung,exynos-thermal.yaml     | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
> index 29a08b0729ee..b8c0bb7f4263 100644
> --- a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
> @@ -27,6 +27,7 @@ properties:
>        - samsung,exynos5420-tmu-ext-triminfo
>        - samsung,exynos5433-tmu
>        - samsung,exynos7-tmu
> +      - samsung,exynos850-tmu
>
>    clocks:
>      minItems: 1
> @@ -131,6 +132,7 @@ allOf:
>                - samsung,exynos5250-tmu
>                - samsung,exynos5260-tmu
>                - samsung,exynos5420-tmu
> +              - samsung,exynos850-tmu
>      then:
>        properties:
>          clocks:
> --
> 2.45.1
>

Re: [PATCH v2 6/6] dt-bindings: thermal: samsung,exynos: remove outdated information on trip point count

[Sam Protsenko]

On Fri, Jul 26, 2024 at 6:01 AM Mateusz Majewski
<m.majewski2@samsung.com> wrote:
>
> This is not true as of commit 5314b1543787 ("thermal/drivers/exynos: Use
> set_trips ops").
>
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

> v1 -> v2: remove an unnecessary sentence.
>
>  .../devicetree/bindings/thermal/samsung,exynos-thermal.yaml | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
> index b8c0bb7f4263..b85b4c420cd3 100644
> --- a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
> @@ -40,11 +40,7 @@ properties:
>    interrupts:
>      description: |
>        The Exynos TMU supports generating interrupts when reaching given
> -      temperature thresholds. Number of supported thermal trip points depends
> -      on the SoC (only first trip points defined in DT will be configured)::
> -       - most of SoC: 4
> -       - samsung,exynos5433-tmu: 8
> -       - samsung,exynos7-tmu: 8
> +      temperature thresholds.
>      maxItems: 1
>
>    reg:
> --
> 2.45.1
>

Re: [PATCH v2 5/6] drivers/thermal/exynos: add initial Exynos850 support

[Sam Protsenko]

On Fri, Jul 26, 2024 at 6:01 AM Mateusz Majewski
<m.majewski2@samsung.com> wrote:
>
> This is loosely adapted from an implementation available at
> https://gitlab.com/Linaro/96boards/e850-96/kernel/-/blob/android-exynos-4.14-linaro/drivers/thermal/samsung/exynos_tmu.c
> Some differences from that implementation:
> - unlike that implementation, we do not use the ACPM mechanism, instead
>   we just access the registers, like we do for other SoCs,
> - the SoC is supposed to support multiple sensors inside one unit. The
>   vendor implementation uses one kernel device per sensor, we would
>   probably prefer to have one device for all sensors, have
>   #thermal-sensor-cells = <1> and so on. We implemented this, but we
>   could not get the extra sensors to work on our hardware so far. This
>   might be due to a misconfiguration and we will probably come back to
>   this, however our implementation only supports a single sensor for
>   now,
> - the vendor implementation supports disabling CPU cores as a cooling
>   device. We did not attempt to port this, and this would not really fit
>   this driver anyway.
>
> Additionally, some differences from the other SoCs supported by this
> driver:
> - we do not really constrain the e-fuse information like the other SoCs
>   do (data->{min,max}_efuse_value). In our tests, those values (as well
>   as the raw sensor values) were much higher than in the other SoCs, to
>   the degree that reusing the data->{min,max}_efuse_value from the other
>   SoCs would cause instant critical temperature reset on boot,
> - this SoC provides more information in the e-fuse data than other SoCs,
>   so we read some values inside exynos850_tmu_initialize instead of
>   hardcoding them in exynos_map_dt_data.
>
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

> v1 -> v2: rename and reorder some registers, use the correct register
>   offset for EXYNOS850_TMU_REG_AVG_CON, make the clock required,
>   additionally do some minor style changes.
>
>  drivers/thermal/samsung/exynos_tmu.c | 191 +++++++++++++++++++++++++--
>  1 file changed, 182 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 087a09628e23..2618a81fca53 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -117,6 +117,41 @@
>  #define EXYNOS7_EMUL_DATA_SHIFT                        7
>  #define EXYNOS7_EMUL_DATA_MASK                 0x1ff
>
> +/* Exynos850 specific registers */
> +#define EXYNOS850_TMU_REG_CURRENT_TEMP0_1      0x40
> +#define EXYNOS850_TMU_REG_THD_TEMP0_RISE       0x50
> +#define EXYNOS850_TMU_REG_THD_TEMP0_FALL       0x60
> +
> +#define EXYNOS850_TMU_TRIMINFO_SHIFT           4
> +#define EXYNOS850_TMU_TRIMINFO_OFFSET(n) \
> +       (EXYNOS_TMU_REG_TRIMINFO + (n) * EXYNOS850_TMU_TRIMINFO_SHIFT)
> +#define EXYNOS850_TMU_T_TRIM0_SHIFT            18
> +
> +#define EXYNOS850_TMU_REG_CONTROL1             0x24
> +#define EXYNOS850_TMU_LPI_MODE_MASK            1
> +#define EXYNOS850_TMU_LPI_MODE_SHIFT           10
> +
> +#define EXYNOS850_TMU_REG_COUNTER_VALUE0       0x30
> +#define EXYNOS850_TMU_EN_TEMP_SEN_OFF_MASK     0xffff
> +#define EXYNOS850_TMU_EN_TEMP_SEN_OFF_SHIFT    0
> +
> +#define EXYNOS850_TMU_REG_COUNTER_VALUE1       0x34
> +#define EXYNOS850_TMU_CLK_SENSE_ON_MASK                0xffff
> +#define EXYNOS850_TMU_CLK_SENSE_ON_SHIFT       16
> +
> +#define EXYNOS850_TMU_REG_AVG_CON              0x38
> +#define EXYNOS850_TMU_AVG_MODE_MASK            0x7
> +#define EXYNOS850_TMU_DEM_ENABLE               BIT(4)
> +
> +#define EXYNOS850_TMU_REG_TRIM0                        0x3c
> +#define EXYNOS850_TMU_TRIM0_MASK               0xf
> +#define EXYNOS850_TMU_VBEI_TRIM_SHIFT          8
> +#define EXYNOS850_TMU_VREF_TRIM_SHIFT          12
> +#define EXYNOS850_TMU_BGRI_TRIM_SHIFT          20
> +
> +#define EXYNOS850_TMU_TEM1051X_SENSE_VALUE     0x028a
> +#define EXYNOS850_TMU_TEM1456X_SENSE_VALUE     0x0a28
> +
>  #define EXYNOS_FIRST_POINT_TRIM                        25
>  #define EXYNOS_SECOND_POINT_TRIM               85
>
> @@ -134,6 +169,7 @@ enum soc_type {
>         SOC_ARCH_EXYNOS5420_TRIMINFO,
>         SOC_ARCH_EXYNOS5433,
>         SOC_ARCH_EXYNOS7,
> +       SOC_ARCH_EXYNOS850,
>  };
>
>  /**
> @@ -232,12 +268,14 @@ static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code)
>
>  static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
>  {
> -       u16 tmu_temp_mask =
> -               (data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_MASK
> -                                               : EXYNOS_TMU_TEMP_MASK;
> -       int tmu_85_shift =
> -               (data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_SHIFT
> -                                               : EXYNOS_TRIMINFO_85_SHIFT;
> +       u16 tmu_temp_mask = (data->soc == SOC_ARCH_EXYNOS7 ||
> +                            data->soc == SOC_ARCH_EXYNOS850) ?
> +                                   EXYNOS7_TMU_TEMP_MASK :
> +                                   EXYNOS_TMU_TEMP_MASK;
> +       int tmu_85_shift = (data->soc == SOC_ARCH_EXYNOS7 ||
> +                           data->soc == SOC_ARCH_EXYNOS850) ?
> +                                  EXYNOS7_TMU_TEMP_SHIFT :
> +                                  EXYNOS_TRIMINFO_85_SHIFT;
>
>         data->temp_error1 = trim_info & tmu_temp_mask;
>         if (!data->temp_error1 ||
> @@ -587,6 +625,114 @@ static void exynos7_tmu_initialize(struct platform_device *pdev)
>         sanitize_temp_error(data, trim_info);
>  }
>
> +static void exynos850_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp)
> +{
> +       exynos_tmu_update_temp(data, EXYNOS850_TMU_REG_THD_TEMP0_FALL + 12, 0,
> +                              temp);
> +       exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
> +                             EXYNOS_TMU_INTEN_FALL0_SHIFT + 0, true);
> +}
> +
> +static void exynos850_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp)
> +{
> +       exynos_tmu_update_temp(data, EXYNOS850_TMU_REG_THD_TEMP0_RISE + 12, 16,
> +                              temp);
> +       exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
> +                             EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, true);
> +}
> +
> +static void exynos850_tmu_disable_low(struct exynos_tmu_data *data)
> +{
> +       exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
> +                             EXYNOS_TMU_INTEN_FALL0_SHIFT + 0, false);
> +}
> +
> +static void exynos850_tmu_disable_high(struct exynos_tmu_data *data)
> +{
> +       exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
> +                             EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, false);
> +}
> +
> +static void exynos850_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp)
> +{
> +       exynos_tmu_update_temp(data, EXYNOS850_TMU_REG_THD_TEMP0_RISE + 0, 16,
> +                              temp);
> +       exynos_tmu_update_bit(data, EXYNOS_TMU_REG_CONTROL,
> +                             EXYNOS_TMU_THERM_TRIP_EN_SHIFT, true);
> +       exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN,
> +                             EXYNOS7_TMU_INTEN_RISE0_SHIFT + 7, true);
> +}
> +
> +static void exynos850_tmu_initialize(struct platform_device *pdev)
> +{
> +       struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> +       u32 cal_type, avg_mode, reg, bgri, vref, vbei;
> +
> +       reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(0));
> +       cal_type = (reg & EXYNOS5433_TRIMINFO_CALIB_SEL_MASK) >>
> +                  EXYNOS5433_TRIMINFO_CALIB_SEL_SHIFT;
> +       data->reference_voltage = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) &
> +                                 EXYNOS_TMU_REF_VOLTAGE_MASK;
> +       reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(1));
> +       data->gain = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) &
> +                    EXYNOS_TMU_BUF_SLOPE_SEL_MASK;
> +       reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(2));
> +       avg_mode = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) &
> +                  EXYNOS850_TMU_AVG_MODE_MASK;
> +       reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(3));
> +       bgri = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) & EXYNOS850_TMU_TRIM0_MASK;
> +       reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(4));
> +       vref = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) & EXYNOS850_TMU_TRIM0_MASK;
> +       reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(5));
> +       vbei = (reg >> EXYNOS850_TMU_T_TRIM0_SHIFT) & EXYNOS850_TMU_TRIM0_MASK;
> +
> +       data->cal_type = cal_type == EXYNOS5433_TRIMINFO_TWO_POINT_TRIMMING ?
> +                                TYPE_TWO_POINT_TRIMMING :
> +                                TYPE_ONE_POINT_TRIMMING;
> +
> +       reg = readl(data->base + EXYNOS850_TMU_TRIMINFO_OFFSET(0));
> +       sanitize_temp_error(data, reg);
> +
> +       dev_info(&pdev->dev, "Calibration type is %d-point calibration\n",
> +                cal_type ? 2 : 1);
> +
> +       reg = readl(data->base + EXYNOS850_TMU_REG_AVG_CON);
> +       reg &= ~EXYNOS850_TMU_AVG_MODE_MASK;
> +       reg &= ~EXYNOS850_TMU_DEM_ENABLE;
> +       if (avg_mode) {
> +               reg |= avg_mode;
> +               reg |= EXYNOS850_TMU_DEM_ENABLE;
> +       }
> +       writel(reg, data->base + EXYNOS850_TMU_REG_AVG_CON);
> +
> +       reg = readl(data->base + EXYNOS850_TMU_REG_COUNTER_VALUE0);
> +       reg &= ~(EXYNOS850_TMU_EN_TEMP_SEN_OFF_MASK
> +                << EXYNOS850_TMU_EN_TEMP_SEN_OFF_SHIFT);
> +       reg |= EXYNOS850_TMU_TEM1051X_SENSE_VALUE
> +              << EXYNOS850_TMU_EN_TEMP_SEN_OFF_SHIFT;
> +       writel(reg, data->base + EXYNOS850_TMU_REG_COUNTER_VALUE0);
> +
> +       reg = readl(data->base + EXYNOS850_TMU_REG_COUNTER_VALUE1);
> +       reg &= ~(EXYNOS850_TMU_CLK_SENSE_ON_MASK
> +                << EXYNOS850_TMU_CLK_SENSE_ON_SHIFT);
> +       reg |= EXYNOS850_TMU_TEM1051X_SENSE_VALUE
> +              << EXYNOS850_TMU_CLK_SENSE_ON_SHIFT;
> +       writel(reg, data->base + EXYNOS850_TMU_REG_COUNTER_VALUE1);
> +
> +       reg = readl(data->base + EXYNOS850_TMU_REG_TRIM0);
> +       reg &= ~(EXYNOS850_TMU_TRIM0_MASK << EXYNOS850_TMU_BGRI_TRIM_SHIFT);
> +       reg &= ~(EXYNOS850_TMU_TRIM0_MASK << EXYNOS850_TMU_VREF_TRIM_SHIFT);
> +       reg &= ~(EXYNOS850_TMU_TRIM0_MASK << EXYNOS850_TMU_VBEI_TRIM_SHIFT);
> +       reg |= bgri << EXYNOS850_TMU_BGRI_TRIM_SHIFT;
> +       reg |= vref << EXYNOS850_TMU_VREF_TRIM_SHIFT;
> +       reg |= vbei << EXYNOS850_TMU_VBEI_TRIM_SHIFT;
> +       writel(reg, data->base + EXYNOS850_TMU_REG_TRIM0);
> +
> +       reg = readl(data->base + EXYNOS850_TMU_REG_CONTROL1);
> +       reg &= ~(EXYNOS850_TMU_LPI_MODE_MASK << EXYNOS850_TMU_LPI_MODE_SHIFT);
> +       writel(reg, data->base + EXYNOS850_TMU_REG_CONTROL1);
> +}
> +
>  static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
>  {
>         struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> @@ -676,7 +822,8 @@ static u32 get_emul_con_reg(struct exynos_tmu_data *data, unsigned int val,
>
>                 val &= ~(EXYNOS_EMUL_TIME_MASK << EXYNOS_EMUL_TIME_SHIFT);
>                 val |= (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT);
> -               if (data->soc == SOC_ARCH_EXYNOS7) {
> +               if (data->soc == SOC_ARCH_EXYNOS7 ||
> +                   data->soc == SOC_ARCH_EXYNOS850) {
>                         val &= ~(EXYNOS7_EMUL_DATA_MASK <<
>                                 EXYNOS7_EMUL_DATA_SHIFT);
>                         val |= (temp_to_code(data, temp) <<
> @@ -706,7 +853,8 @@ static void exynos4412_tmu_set_emulation(struct exynos_tmu_data *data,
>                 emul_con = EXYNOS5260_EMUL_CON;
>         else if (data->soc == SOC_ARCH_EXYNOS5433)
>                 emul_con = EXYNOS5433_TMU_EMUL_CON;
> -       else if (data->soc == SOC_ARCH_EXYNOS7)
> +       else if (data->soc == SOC_ARCH_EXYNOS7 ||
> +                data->soc == SOC_ARCH_EXYNOS850)
>                 emul_con = EXYNOS7_TMU_REG_EMUL_CON;
>         else
>                 emul_con = EXYNOS_EMUL_CON;
> @@ -761,6 +909,12 @@ static int exynos7_tmu_read(struct exynos_tmu_data *data)
>                 EXYNOS7_TMU_TEMP_MASK;
>  }
>
> +static int exynos850_tmu_read(struct exynos_tmu_data *data)
> +{
> +       return readw(data->base + EXYNOS850_TMU_REG_CURRENT_TEMP0_1) &
> +              EXYNOS7_TMU_TEMP_MASK;
> +}
> +
>  static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
>  {
>         struct exynos_tmu_data *data = id;
> @@ -787,7 +941,8 @@ static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data)
>         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) {
> +       } else if (data->soc == SOC_ARCH_EXYNOS7 ||
> +                  data->soc == SOC_ARCH_EXYNOS850) {
>                 tmu_intstat = EXYNOS7_TMU_REG_INTPEND;
>                 tmu_intclear = EXYNOS7_TMU_REG_INTPEND;
>         } else if (data->soc == SOC_ARCH_EXYNOS5433) {
> @@ -838,6 +993,9 @@ static const struct of_device_id exynos_tmu_match[] = {
>         }, {
>                 .compatible = "samsung,exynos7-tmu",
>                 .data = (const void *)SOC_ARCH_EXYNOS7,
> +       }, {
> +               .compatible = "samsung,exynos850-tmu",
> +               .data = (const void *)SOC_ARCH_EXYNOS850,
>         },
>         { },
>  };
> @@ -950,6 +1108,21 @@ static int exynos_map_dt_data(struct platform_device *pdev)
>                 data->min_efuse_value = 15;
>                 data->max_efuse_value = 100;
>                 break;
> +       case SOC_ARCH_EXYNOS850:
> +               data->tmu_set_low_temp = exynos850_tmu_set_low_temp;
> +               data->tmu_set_high_temp = exynos850_tmu_set_high_temp;
> +               data->tmu_disable_low = exynos850_tmu_disable_low;
> +               data->tmu_disable_high = exynos850_tmu_disable_high;
> +               data->tmu_set_crit_temp = exynos850_tmu_set_crit_temp;
> +               data->tmu_initialize = exynos850_tmu_initialize;
> +               data->tmu_control = exynos4210_tmu_control;
> +               data->tmu_read = exynos850_tmu_read;
> +               data->tmu_set_emulation = exynos4412_tmu_set_emulation;
> +               data->tmu_clear_irqs = exynos4210_tmu_clear_irqs;
> +               data->efuse_value = 55;
> +               data->min_efuse_value = 0;
> +               data->max_efuse_value = 511;
> +               break;
>         default:
>                 dev_err(&pdev->dev, "Platform not supported\n");
>                 return -EINVAL;
> --
> 2.45.1
>

Re: [PATCH v2 6/6] dt-bindings: thermal: samsung,exynos: remove outdated information on trip point count

[Rob Herring]

On Fri, Jul 26, 2024 at 01:01:10PM +0200, Mateusz Majewski wrote:
> This is not true as of commit 5314b1543787 ("thermal/drivers/exynos: Use
> set_trips ops").

What is not true?

How can the h/w change? I already asked that. Please make your commit 
message summarize prior discussions so that the patch stands on its own 
and you don't get the same response again. Assume the reviewers have 0 
recollection of the prior versions because we don't. This is just one of 
100s of patches a week...

> 
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---
> v1 -> v2: remove an unnecessary sentence.
> 
>  .../devicetree/bindings/thermal/samsung,exynos-thermal.yaml | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
> index b8c0bb7f4263..b85b4c420cd3 100644
> --- a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yaml
> @@ -40,11 +40,7 @@ properties:
>    interrupts:
>      description: |
>        The Exynos TMU supports generating interrupts when reaching given
> -      temperature thresholds. Number of supported thermal trip points depends
> -      on the SoC (only first trip points defined in DT will be configured)::
> -       - most of SoC: 4
> -       - samsung,exynos5433-tmu: 8
> -       - samsung,exynos7-tmu: 8
> +      temperature thresholds.
>      maxItems: 1
>  
>    reg:
> -- 
> 2.45.1
> 

Re: [PATCH v2 6/6] dt-bindings: thermal: samsung,exynos: remove outdated information on trip point count

[Sam Protsenko]

On Tue, Jul 30, 2024 at 11:17 AM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Jul 26, 2024 at 01:01:10PM +0200, Mateusz Majewski wrote:
> > This is not true as of commit 5314b1543787 ("thermal/drivers/exynos: Use
> > set_trips ops").
>
> What is not true?
>
> How can the h/w change? I already asked that. Please make your commit
> message summarize prior discussions so that the patch stands on its own
> and you don't get the same response again. Assume the reviewers have 0
> recollection of the prior versions because we don't. This is just one of
> 100s of patches a week...
>

Hi Mateusz,

Do I understand it correctly that the patch actually removes an
outdated description of *driver* implementation, and not outdated
hardware description? If so, then maybe it makes sense to rework the
patch title and commit message in a way Rob suggests. I.e. rather than
stating that the patch removes an outdated information, instead
mention it removes *software* (driver) description which was
incorrectly added earlier. Because bindings are only meant for
hardware description and should be completely independent of driver's
side of things. Also in that case it probably doesn't make much sense
referencing that commit for using set_trips ops. Just my two cents.

Thanks!

[snip]

Re: [PATCH v2 6/6] dt-bindings: thermal: samsung,exynos: remove outdated information on trip point count

[Mateusz Majewski]

> Do I understand it correctly that the patch actually removes an
> outdated description of *driver* implementation, and not outdated
> hardware description?

Correct.

> If so, then maybe it makes sense to rework the
> patch title and commit message in a way Rob suggests. I.e. rather than
> stating that the patch removes an outdated information, instead
> mention it removes *software* (driver) description which was
> incorrectly added earlier. Because bindings are only meant for
> hardware description and should be completely independent of driver's
> side of things. Also in that case it probably doesn't make much sense
> referencing that commit for using set_trips ops. Just my two cents.

Makes sense, what do you think about this?

dt-bindings: thermal: samsung,exynos: remove driver-specific information

The number of supported trip points was only limited by the driver
implementation at the time, which mapped each trip point defined in the
devicetree source file to a hardware trip point. An implementation that
does not have this limitation is possible; indeed, that is how the
driver works currently. Therefore, this information should be removed
from the bindings description, which are meant to be independent from
the details of the driver implementation.

Re: [PATCH v2 6/6] dt-bindings: thermal: samsung,exynos: remove outdated information on trip point count

[Sam Protsenko]

On Wed, Jul 31, 2024 at 4:14 PM Mateusz Majewski
<m.majewski2@samsung.com> wrote:
>
> > Do I understand it correctly that the patch actually removes an
> > outdated description of *driver* implementation, and not outdated
> > hardware description?
>
> Correct.
>
> > If so, then maybe it makes sense to rework the
> > patch title and commit message in a way Rob suggests. I.e. rather than
> > stating that the patch removes an outdated information, instead
> > mention it removes *software* (driver) description which was
> > incorrectly added earlier. Because bindings are only meant for
> > hardware description and should be completely independent of driver's
> > side of things. Also in that case it probably doesn't make much sense
> > referencing that commit for using set_trips ops. Just my two cents.
>
> Makes sense, what do you think about this?
>
> dt-bindings: thermal: samsung,exynos: remove driver-specific information
>
> The number of supported trip points was only limited by the driver
> implementation at the time, which mapped each trip point defined in the
> devicetree source file to a hardware trip point. An implementation that
> does not have this limitation is possible; indeed, that is how the
> driver works currently. Therefore, this information should be removed
> from the bindings description, which are meant to be independent from
> the details of the driver implementation.

Looks good to me. But you already have my R-b tag :) More important if
it's ok with Rob.

Re: [PATCH v2 6/6] dt-bindings: thermal: samsung,exynos: remove outdated information on trip point count

[Rob Herring]

On Wed, Jul 31, 2024 at 11:14:42PM +0200, Mateusz Majewski wrote:
> > Do I understand it correctly that the patch actually removes an
> > outdated description of *driver* implementation, and not outdated
> > hardware description?
> 
> Correct.
> 
> > If so, then maybe it makes sense to rework the
> > patch title and commit message in a way Rob suggests. I.e. rather than
> > stating that the patch removes an outdated information, instead
> > mention it removes *software* (driver) description which was
> > incorrectly added earlier. Because bindings are only meant for
> > hardware description and should be completely independent of driver's
> > side of things. Also in that case it probably doesn't make much sense
> > referencing that commit for using set_trips ops. Just my two cents.
> 
> Makes sense, what do you think about this?
> 
> dt-bindings: thermal: samsung,exynos: remove driver-specific information
> 
> The number of supported trip points was only limited by the driver
> implementation at the time, which mapped each trip point defined in the
> devicetree source file to a hardware trip point. An implementation that
> does not have this limitation is possible; indeed, that is how the
> driver works currently. Therefore, this information should be removed
> from the bindings description, which are meant to be independent from
> the details of the driver implementation.

LGTM