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

[PATCH v3 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:
 v3:
   - Reworded the commit message of the dt-binding information removal
     change
 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 driver-specific
    information

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

-- 
2.45.1

[PATCH v3 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 v3 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>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
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 v3 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.

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
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

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

[Daniel Lezcano]

On 07/08/2024 10:48, Mateusz Majewski 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.

May I suggest to convert this function to a specific soc ops to be put 
in the struct exynos_tmu_data ?

> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> 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);
>   }


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

[Mateusz Majewski]

Hello :)

> May I suggest to convert this function to a specific soc ops to be put
> in the struct exynos_tmu_data ?

Like this?

static void exynos4210_sanitize_temp_error(struct exynos_tmu_data *data,
					   u32 trim_info)
{
	data->temp_error1 = trim_info & EXYNOS_TMU_TEMP_MASK;
	if (!data->temp_error1 ||
	    (data->min_efuse_value > data->temp_error1) ||
	    (data->temp_error1 > data->max_efuse_value))
		data->temp_error1 = data->efuse_value & EXYNOS_TMU_TEMP_MASK;
	WARN_ON_ONCE(data->cal_type == TYPE_TWO_POINT_TRIMMING);
}

static void exynos5433_sanitize_temp_error(struct exynos_tmu_data *data,
					   u32 trim_info)
{
	data->temp_error1 = trim_info & EXYNOS_TMU_TEMP_MASK;
	if (!data->temp_error1 ||
	    (data->min_efuse_value > data->temp_error1) ||
	    (data->temp_error1 > data->max_efuse_value))
		data->temp_error1 = data->efuse_value & EXYNOS_TMU_TEMP_MASK;

	if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
		data->temp_error2 = (trim_info >> EXYNOS_TRIMINFO_85_SHIFT) &
				    EXYNOS_TMU_TEMP_MASK;
		if (!data->temp_error2)
			data->temp_error2 = (data->efuse_value >>
					     EXYNOS_TRIMINFO_85_SHIFT) &
					    EXYNOS_TMU_TEMP_MASK;
	}
}

static void exynos7_sanitize_temp_error(struct exynos_tmu_data *data,
					u32 trim_info)
{
	data->temp_error1 = trim_info & EXYNOS7_TMU_TEMP_MASK;
	if (!data->temp_error1 ||
	    (data->min_efuse_value > data->temp_error1) ||
	    (data->temp_error1 > data->max_efuse_value))
		data->temp_error1 = data->efuse_value & EXYNOS7_TMU_TEMP_MASK;
	WARN_ON_ONCE(data->cal_type == TYPE_TWO_POINT_TRIMMING);
}

static void exynos850_sanitize_temp_error(struct exynos_tmu_data *data,
					   u32 trim_info)
{
	data->temp_error1 = trim_info & EXYNOS7_TMU_TEMP_MASK;
	if (!data->temp_error1 ||
	    (data->min_efuse_value > data->temp_error1) ||
	    (data->temp_error1 > data->max_efuse_value))
		data->temp_error1 = data->efuse_value & EXYNOS7_TMU_TEMP_MASK;

	if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
		data->temp_error2 = (trim_info >> EXYNOS7_TMU_TEMP_SHIFT) &
				    EXYNOS7_TMU_TEMP_MASK;
		if (!data->temp_error2)
			data->temp_error2 = (data->efuse_value >>
					     EXYNOS7_TMU_TEMP_SHIFT) &
					    EXYNOS_TMU_TEMP_MASK;
	}
}

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

static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
{
	data->temp_error1 = trim_info & data->tmu_temp_mask;
	if (!data->temp_error1 || (data->min_efuse_value > data->temp_error1) ||
	    (data->temp_error1 > data->max_efuse_value))
		data->temp_error1 = data->efuse_value & data->tmu_temp_mask;

	if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
		data->temp_error2 = (trim_info >> data->tmu_85_shift) &
				    data->tmu_temp_mask;
		if (!data->temp_error2)
			data->temp_error2 =
				(data->efuse_value >> data->tmu_85_shift) &
				data->tmu_temp_mask;
	}
}

Thank you,
Mateusz

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

[Daniel Lezcano]

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

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

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

Either way

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

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


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v3 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.

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
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

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

[Krzysztof Kozlowski]

On 07/08/2024 10:48, Mateusz Majewski wrote:
> Like most of the SoCs, it requires 1 clock and 1 register.
> 
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---
> v1 -> v2: make the clock required in Exynos850.
> 

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

Best regards,
Krzysztof

[PATCH v3 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.

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
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 v3 6/6] dt-bindings: thermal: samsung,exynos: remove driver-specific information

[Mateusz Majewski]

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 of
the details of the driver implementation.

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
---
v2 -> v3: reword the commit message to be easier to understand in
  context of dt-bindings.
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 v3 6/6] dt-bindings: thermal: samsung,exynos: remove driver-specific information

[Krzysztof Kozlowski]

On 07/08/2024 10:48, Mateusz Majewski wrote:
> 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 of
> the details of the driver implementation.
> 
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---

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

Best regards,
Krzysztof

Re: [PATCH v3 6/6] dt-bindings: thermal: samsung,exynos: remove driver-specific information

[Rob Herring (Arm)]

On Wed, 07 Aug 2024 10:48:25 +0200, Mateusz Majewski wrote:
> 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 of
> the details of the driver implementation.
> 
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
> ---
> v2 -> v3: reword the commit message to be easier to understand in
>   context of dt-bindings.
> v1 -> v2: remove an unnecessary sentence.
> 
>  .../devicetree/bindings/thermal/samsung,exynos-thermal.yaml | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>