[RESEND] [PATCHv3 0/7] RK3588 Thermal Support

[RESEND] [PATCHv3 0/7] RK3588 Thermal Support

[Sebastian Reichel]

Hi,

This adds thermal support for the new RK3588(s) SoC series. The
series has been tested on the RK3588 EVB1 board.

Can this be applied? The series received absolutely no feedback
for a full release cycle.

Changes since PATCHv3:
 * https://lore.kernel.org/all/20230105180340.29140-1-sebastian.reichel@collabora.com/
 * rebased against v6.3-rc1

Changes since PATCHv2:
 * https://lore.kernel.org/all/20221031175058.175698-1-sebastian.reichel@collabora.com/
 * rebased against v6.2-rc1
 * drop useless cast from patch 1
 * add Heiko's reviewed-by to patches 1-3 & 5
 * The discussion around patch 4 died, so I kept it unchanged

Changes since PATCHv1:
 * https://lore.kernel.org/all/20221021174721.92468-1-sebastian.reichel@collabora.com/
 * Collect Reviewed-by/Acked-by
 * Use TRM channel info in commit message (Daniel Lezcano)
 * Add patch removing channel id lookup table (Daniel Lezcano)
 * Add patch allocating sensors array dynamiccaly (Daniel Lezcano)
 * I also added patches simplifying up the probe routine a bit

-- Sebastian

Finley Xiao (1):
  thermal: rockchip: Support RK3588 SoC in the thermal driver

Sebastian Reichel (6):
  thermal: rockchip: Simplify getting match data
  thermal: rockchip: Simplify clock logic
  thermal: rockchip: Use dev_err_probe
  thermal: rockchip: Simplify channel id logic
  thermal: rockchip: Support dynamic sized sensor array
  dt-bindings: rockchip-thermal: Support the RK3588 SoC compatible

 .../bindings/thermal/rockchip-thermal.yaml    |   1 +
 drivers/thermal/rockchip_thermal.c            | 322 ++++++++++++------
 2 files changed, 226 insertions(+), 97 deletions(-)

-- 
2.39.2

[RESEND] [PATCHv3 1/7] thermal: rockchip: Simplify getting match data

[Sebastian Reichel]

It's possible to directly get the match data in a generic
way nowadays.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/thermal/rockchip_thermal.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 4b7c43f34d1a..7e88525fe8d0 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -1353,15 +1353,10 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct rockchip_thermal_data *thermal;
-	const struct of_device_id *match;
 	int irq;
 	int i;
 	int error;
 
-	match = of_match_node(of_rockchip_thermal_match, np);
-	if (!match)
-		return -ENXIO;
-
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return -EINVAL;
@@ -1373,7 +1368,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 
 	thermal->pdev = pdev;
 
-	thermal->chip = (const struct rockchip_tsadc_chip *)match->data;
+	thermal->chip = device_get_match_data(&pdev->dev);
 	if (!thermal->chip)
 		return -EINVAL;
 
-- 
2.39.2

Re: [RESEND] [PATCHv3 1/7] thermal: rockchip: Simplify getting match data

[Daniel Lezcano]

On 08/03/2023 12:22, Sebastian Reichel wrote:
> It's possible to directly get the match data in a generic
> way nowadays.
> 
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---

Applied, thanks


-- 
<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

[RESEND] [PATCHv3 2/7] thermal: rockchip: Simplify clock logic

[Sebastian Reichel]

By using devm_clk_get_enabled() the clock acquisition and
enabling can be done in one step with automatic error
handling.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/thermal/rockchip_thermal.c | 33 +++++-------------------------
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 7e88525fe8d0..6235c033995b 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -1383,14 +1383,14 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 		return error;
 	}
 
-	thermal->clk = devm_clk_get(&pdev->dev, "tsadc");
+	thermal->clk = devm_clk_get_enabled(&pdev->dev, "tsadc");
 	if (IS_ERR(thermal->clk)) {
 		error = PTR_ERR(thermal->clk);
 		dev_err(&pdev->dev, "failed to get tsadc clock: %d\n", error);
 		return error;
 	}
 
-	thermal->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
+	thermal->pclk = devm_clk_get_enabled(&pdev->dev, "apb_pclk");
 	if (IS_ERR(thermal->pclk)) {
 		error = PTR_ERR(thermal->pclk);
 		dev_err(&pdev->dev, "failed to get apb_pclk clock: %d\n",
@@ -1398,26 +1398,13 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 		return error;
 	}
 
-	error = clk_prepare_enable(thermal->clk);
-	if (error) {
-		dev_err(&pdev->dev, "failed to enable converter clock: %d\n",
-			error);
-		return error;
-	}
-
-	error = clk_prepare_enable(thermal->pclk);
-	if (error) {
-		dev_err(&pdev->dev, "failed to enable pclk: %d\n", error);
-		goto err_disable_clk;
-	}
-
 	rockchip_thermal_reset_controller(thermal->reset);
 
 	error = rockchip_configure_from_dt(&pdev->dev, np, thermal);
 	if (error) {
 		dev_err(&pdev->dev, "failed to parse device tree data: %d\n",
 			error);
-		goto err_disable_pclk;
+		return error;
 	}
 
 	thermal->chip->initialize(thermal->grf, thermal->regs,
@@ -1431,7 +1418,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev,
 				"failed to register sensor[%d] : error = %d\n",
 				i, error);
-			goto err_disable_pclk;
+			return error;
 		}
 	}
 
@@ -1442,7 +1429,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 	if (error) {
 		dev_err(&pdev->dev,
 			"failed to request tsadc irq: %d\n", error);
-		goto err_disable_pclk;
+		return error;
 	}
 
 	thermal->chip->control(thermal->regs, true);
@@ -1460,13 +1447,6 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, thermal);
 
 	return 0;
-
-err_disable_pclk:
-	clk_disable_unprepare(thermal->pclk);
-err_disable_clk:
-	clk_disable_unprepare(thermal->clk);
-
-	return error;
 }
 
 static int rockchip_thermal_remove(struct platform_device *pdev)
@@ -1483,9 +1463,6 @@ static int rockchip_thermal_remove(struct platform_device *pdev)
 
 	thermal->chip->control(thermal->regs, false);
 
-	clk_disable_unprepare(thermal->pclk);
-	clk_disable_unprepare(thermal->clk);
-
 	return 0;
 }
 
-- 
2.39.2

[RESEND] [PATCHv3 3/7] thermal: rockchip: Use dev_err_probe

[Sebastian Reichel]

Use dev_err_probe to simplify error printing in the driver's probe
routine.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/thermal/rockchip_thermal.c | 50 +++++++++++-------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 6235c033995b..9ed45b318344 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -1377,35 +1377,26 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 		return PTR_ERR(thermal->regs);
 
 	thermal->reset = devm_reset_control_array_get(&pdev->dev, false, false);
-	if (IS_ERR(thermal->reset)) {
-		error = PTR_ERR(thermal->reset);
-		dev_err(&pdev->dev, "failed to get tsadc reset: %d\n", error);
-		return error;
-	}
+	if (IS_ERR(thermal->reset))
+		return dev_err_probe(&pdev->dev, PTR_ERR(thermal->reset),
+				     "failed to get tsadc reset.\n");
 
 	thermal->clk = devm_clk_get_enabled(&pdev->dev, "tsadc");
-	if (IS_ERR(thermal->clk)) {
-		error = PTR_ERR(thermal->clk);
-		dev_err(&pdev->dev, "failed to get tsadc clock: %d\n", error);
-		return error;
-	}
+	if (IS_ERR(thermal->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(thermal->clk),
+				     "failed to get tsadc clock.\n");
 
 	thermal->pclk = devm_clk_get_enabled(&pdev->dev, "apb_pclk");
-	if (IS_ERR(thermal->pclk)) {
-		error = PTR_ERR(thermal->pclk);
-		dev_err(&pdev->dev, "failed to get apb_pclk clock: %d\n",
-			error);
-		return error;
-	}
+	if (IS_ERR(thermal->pclk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(thermal->pclk),
+				     "failed to get apb_pclk clock.\n");
 
 	rockchip_thermal_reset_controller(thermal->reset);
 
 	error = rockchip_configure_from_dt(&pdev->dev, np, thermal);
-	if (error) {
-		dev_err(&pdev->dev, "failed to parse device tree data: %d\n",
-			error);
-		return error;
-	}
+	if (error)
+		return dev_err_probe(&pdev->dev, error,
+				"failed to parse device tree data\n");
 
 	thermal->chip->initialize(thermal->grf, thermal->regs,
 				  thermal->tshut_polarity);
@@ -1414,23 +1405,18 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 		error = rockchip_thermal_register_sensor(pdev, thermal,
 						&thermal->sensors[i],
 						thermal->chip->chn_id[i]);
-		if (error) {
-			dev_err(&pdev->dev,
-				"failed to register sensor[%d] : error = %d\n",
-				i, error);
-			return error;
-		}
+		if (error)
+			return dev_err_probe(&pdev->dev, error,
+				"failed to register sensor[%d].\n", i);
 	}
 
 	error = devm_request_threaded_irq(&pdev->dev, irq, NULL,
 					  &rockchip_thermal_alarm_irq_thread,
 					  IRQF_ONESHOT,
 					  "rockchip_thermal", thermal);
-	if (error) {
-		dev_err(&pdev->dev,
-			"failed to request tsadc irq: %d\n", error);
-		return error;
-	}
+	if (error)
+		return dev_err_probe(&pdev->dev, error,
+				     "failed to request tsadc irq.\n");
 
 	thermal->chip->control(thermal->regs, true);
 
-- 
2.39.2

[RESEND] [PATCHv3 4/7] thermal: rockchip: Simplify channel id logic

[Sebastian Reichel]

Replace the channel ID lookup table by a simple offset, since
the channel IDs are consecutive.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/thermal/rockchip_thermal.c | 48 +++++++++++++-----------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 9ed45b318344..bcbdd618daae 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -39,15 +39,6 @@ enum tshut_polarity {
 	TSHUT_HIGH_ACTIVE,
 };
 
-/*
- * The system has two Temperature Sensors.
- * sensor0 is for CPU, and sensor1 is for GPU.
- */
-enum sensor_id {
-	SENSOR_CPU = 0,
-	SENSOR_GPU,
-};
-
 /*
  * The conversion table has the adc value and temperature.
  * ADC_DECREMENT: the adc value is of diminishing.(e.g. rk3288_code_table)
@@ -82,7 +73,7 @@ struct chip_tsadc_table {
 
 /**
  * struct rockchip_tsadc_chip - hold the private data of tsadc chip
- * @chn_id: array of sensor ids of chip corresponding to the channel
+ * @chn_offset: the channel offset of the first channel
  * @chn_num: the channel number of tsadc chip
  * @tshut_temp: the hardware-controlled shutdown temperature value
  * @tshut_mode: the hardware-controlled shutdown mode (0:CRU 1:GPIO)
@@ -98,7 +89,7 @@ struct chip_tsadc_table {
  */
 struct rockchip_tsadc_chip {
 	/* The sensor id of chip correspond to the ADC channel */
-	int chn_id[SOC_MAX_SENSORS];
+	int chn_offset;
 	int chn_num;
 
 	/* The hardware-controlled tshut property */
@@ -925,8 +916,8 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
 }
 
 static const struct rockchip_tsadc_chip px30_tsadc_data = {
-	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
-	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
+	/* cpu, gpu */
+	.chn_offset = 0,
 	.chn_num = 2, /* 2 channels for tsadc */
 
 	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
@@ -949,7 +940,8 @@ static const struct rockchip_tsadc_chip px30_tsadc_data = {
 };
 
 static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
-	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
+	/* cpu */
+	.chn_offset = 0,
 	.chn_num = 1, /* one channel for tsadc */
 
 	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
@@ -973,7 +965,8 @@ static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
 };
 
 static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
-	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
+	/* cpu */
+	.chn_offset = 0,
 	.chn_num = 1, /* one channel for tsadc */
 
 	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
@@ -997,8 +990,8 @@ static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
 };
 
 static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
-	.chn_id[SENSOR_CPU] = 1, /* cpu sensor is channel 1 */
-	.chn_id[SENSOR_GPU] = 2, /* gpu sensor is channel 2 */
+	/* cpu, gpu */
+	.chn_offset = 1,
 	.chn_num = 2, /* two channels for tsadc */
 
 	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
@@ -1022,7 +1015,8 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
 };
 
 static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
-	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
+	/* cpu */
+	.chn_offset = 0,
 	.chn_num = 1, /* one channels for tsadc */
 
 	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
@@ -1045,8 +1039,8 @@ static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
 };
 
 static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
-	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
-	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
+	/* cpu, gpu */
+	.chn_offset = 0,
 	.chn_num = 2, /* two channels for tsadc */
 
 	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
@@ -1070,8 +1064,8 @@ static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
 };
 
 static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
-	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
-	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
+	/* cpu, gpu */
+	.chn_offset = 0,
 	.chn_num = 2, /* two channels for tsadc */
 
 	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
@@ -1095,8 +1089,8 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
 };
 
 static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
-	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
-	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
+	/* cpu, gpu */
+	.chn_offset = 0,
 	.chn_num = 2, /* two channels for tsadc */
 
 	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
@@ -1120,8 +1114,8 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
 };
 
 static const struct rockchip_tsadc_chip rk3568_tsadc_data = {
-	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
-	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
+	/* cpu, gpu */
+	.chn_offset = 0,
 	.chn_num = 2, /* two channels for tsadc */
 
 	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
@@ -1404,7 +1398,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 	for (i = 0; i < thermal->chip->chn_num; i++) {
 		error = rockchip_thermal_register_sensor(pdev, thermal,
 						&thermal->sensors[i],
-						thermal->chip->chn_id[i]);
+						thermal->chip->chn_offset + i);
 		if (error)
 			return dev_err_probe(&pdev->dev, error,
 				"failed to register sensor[%d].\n", i);
-- 
2.39.2

Re: [RESEND] [PATCHv3 4/7] thermal: rockchip: Simplify channel id logic

[Daniel Lezcano]

On 08/03/2023 12:22, Sebastian Reichel wrote:
> Replace the channel ID lookup table by a simple offset, since
> the channel IDs are consecutive.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

As all the other patches are reviewed by Heiko, is the tag missing here?


> ---
>   drivers/thermal/rockchip_thermal.c | 48 +++++++++++++-----------------
>   1 file changed, 21 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> index 9ed45b318344..bcbdd618daae 100644
> --- a/drivers/thermal/rockchip_thermal.c
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -39,15 +39,6 @@ enum tshut_polarity {
>   	TSHUT_HIGH_ACTIVE,
>   };
>   
> -/*
> - * The system has two Temperature Sensors.
> - * sensor0 is for CPU, and sensor1 is for GPU.
> - */
> -enum sensor_id {
> -	SENSOR_CPU = 0,
> -	SENSOR_GPU,
> -};
> -
>   /*
>    * The conversion table has the adc value and temperature.
>    * ADC_DECREMENT: the adc value is of diminishing.(e.g. rk3288_code_table)
> @@ -82,7 +73,7 @@ struct chip_tsadc_table {
>   
>   /**
>    * struct rockchip_tsadc_chip - hold the private data of tsadc chip
> - * @chn_id: array of sensor ids of chip corresponding to the channel
> + * @chn_offset: the channel offset of the first channel
>    * @chn_num: the channel number of tsadc chip
>    * @tshut_temp: the hardware-controlled shutdown temperature value
>    * @tshut_mode: the hardware-controlled shutdown mode (0:CRU 1:GPIO)
> @@ -98,7 +89,7 @@ struct chip_tsadc_table {
>    */
>   struct rockchip_tsadc_chip {
>   	/* The sensor id of chip correspond to the ADC channel */
> -	int chn_id[SOC_MAX_SENSORS];
> +	int chn_offset;
>   	int chn_num;
>   
>   	/* The hardware-controlled tshut property */
> @@ -925,8 +916,8 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
>   }
>   
>   static const struct rockchip_tsadc_chip px30_tsadc_data = {
> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> +	/* cpu, gpu */
> +	.chn_offset = 0,
>   	.chn_num = 2, /* 2 channels for tsadc */
>   
>   	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> @@ -949,7 +940,8 @@ static const struct rockchip_tsadc_chip px30_tsadc_data = {
>   };
>   
>   static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> +	/* cpu */
> +	.chn_offset = 0,
>   	.chn_num = 1, /* one channel for tsadc */
>   
>   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> @@ -973,7 +965,8 @@ static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
>   };
>   
>   static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> +	/* cpu */
> +	.chn_offset = 0,
>   	.chn_num = 1, /* one channel for tsadc */
>   
>   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> @@ -997,8 +990,8 @@ static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
>   };
>   
>   static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
> -	.chn_id[SENSOR_CPU] = 1, /* cpu sensor is channel 1 */
> -	.chn_id[SENSOR_GPU] = 2, /* gpu sensor is channel 2 */
> +	/* cpu, gpu */
> +	.chn_offset = 1,
>   	.chn_num = 2, /* two channels for tsadc */
>   
>   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> @@ -1022,7 +1015,8 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
>   };
>   
>   static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> +	/* cpu */
> +	.chn_offset = 0,
>   	.chn_num = 1, /* one channels for tsadc */
>   
>   	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> @@ -1045,8 +1039,8 @@ static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
>   };
>   
>   static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> +	/* cpu, gpu */
> +	.chn_offset = 0,
>   	.chn_num = 2, /* two channels for tsadc */
>   
>   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> @@ -1070,8 +1064,8 @@ static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
>   };
>   
>   static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> +	/* cpu, gpu */
> +	.chn_offset = 0,
>   	.chn_num = 2, /* two channels for tsadc */
>   
>   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> @@ -1095,8 +1089,8 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
>   };
>   
>   static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> +	/* cpu, gpu */
> +	.chn_offset = 0,
>   	.chn_num = 2, /* two channels for tsadc */
>   
>   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> @@ -1120,8 +1114,8 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
>   };
>   
>   static const struct rockchip_tsadc_chip rk3568_tsadc_data = {
> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> +	/* cpu, gpu */
> +	.chn_offset = 0,
>   	.chn_num = 2, /* two channels for tsadc */
>   
>   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> @@ -1404,7 +1398,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
>   	for (i = 0; i < thermal->chip->chn_num; i++) {
>   		error = rockchip_thermal_register_sensor(pdev, thermal,
>   						&thermal->sensors[i],
> -						thermal->chip->chn_id[i]);
> +						thermal->chip->chn_offset + i);
>   		if (error)
>   			return dev_err_probe(&pdev->dev, error,
>   				"failed to register sensor[%d].\n", i);

-- 
<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: [RESEND] [PATCHv3 4/7] thermal: rockchip: Simplify channel id logic

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 7073 bytes --]

Hi Daniel,

On Wed, Mar 08, 2023 at 07:13:22PM +0100, Daniel Lezcano wrote:
> On 08/03/2023 12:22, Sebastian Reichel wrote:
> > Replace the channel ID lookup table by a simple offset, since
> > the channel IDs are consecutive.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> As all the other patches are reviewed by Heiko, is the tag missing here?

Heiko was not happy with this in PATCHv2, when he reviewed most
of the patches:

https://lore.kernel.org/all/3601039.e9J7NaK4W3@phil/

I replied, but never got a response, so I kept it as is:

https://lore.kernel.org/all/20221206170232.xsm4kcbfwrmlrriw@mercury.elektranox.org/

FWIW it is essential for the series and cannot be dropped, because
RK3588 has more than 2 channels.

Greetings,

-- Sebastian

> > ---
> >   drivers/thermal/rockchip_thermal.c | 48 +++++++++++++-----------------
> >   1 file changed, 21 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> > index 9ed45b318344..bcbdd618daae 100644
> > --- a/drivers/thermal/rockchip_thermal.c
> > +++ b/drivers/thermal/rockchip_thermal.c
> > @@ -39,15 +39,6 @@ enum tshut_polarity {
> >   	TSHUT_HIGH_ACTIVE,
> >   };
> > -/*
> > - * The system has two Temperature Sensors.
> > - * sensor0 is for CPU, and sensor1 is for GPU.
> > - */
> > -enum sensor_id {
> > -	SENSOR_CPU = 0,
> > -	SENSOR_GPU,
> > -};
> > -
> >   /*
> >    * The conversion table has the adc value and temperature.
> >    * ADC_DECREMENT: the adc value is of diminishing.(e.g. rk3288_code_table)
> > @@ -82,7 +73,7 @@ struct chip_tsadc_table {
> >   /**
> >    * struct rockchip_tsadc_chip - hold the private data of tsadc chip
> > - * @chn_id: array of sensor ids of chip corresponding to the channel
> > + * @chn_offset: the channel offset of the first channel
> >    * @chn_num: the channel number of tsadc chip
> >    * @tshut_temp: the hardware-controlled shutdown temperature value
> >    * @tshut_mode: the hardware-controlled shutdown mode (0:CRU 1:GPIO)
> > @@ -98,7 +89,7 @@ struct chip_tsadc_table {
> >    */
> >   struct rockchip_tsadc_chip {
> >   	/* The sensor id of chip correspond to the ADC channel */
> > -	int chn_id[SOC_MAX_SENSORS];
> > +	int chn_offset;
> >   	int chn_num;
> >   	/* The hardware-controlled tshut property */
> > @@ -925,8 +916,8 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
> >   }
> >   static const struct rockchip_tsadc_chip px30_tsadc_data = {
> > -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> > -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> > +	/* cpu, gpu */
> > +	.chn_offset = 0,
> >   	.chn_num = 2, /* 2 channels for tsadc */
> >   	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> > @@ -949,7 +940,8 @@ static const struct rockchip_tsadc_chip px30_tsadc_data = {
> >   };
> >   static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
> > -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> > +	/* cpu */
> > +	.chn_offset = 0,
> >   	.chn_num = 1, /* one channel for tsadc */
> >   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> > @@ -973,7 +965,8 @@ static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
> >   };
> >   static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
> > -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> > +	/* cpu */
> > +	.chn_offset = 0,
> >   	.chn_num = 1, /* one channel for tsadc */
> >   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> > @@ -997,8 +990,8 @@ static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
> >   };
> >   static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
> > -	.chn_id[SENSOR_CPU] = 1, /* cpu sensor is channel 1 */
> > -	.chn_id[SENSOR_GPU] = 2, /* gpu sensor is channel 2 */
> > +	/* cpu, gpu */
> > +	.chn_offset = 1,
> >   	.chn_num = 2, /* two channels for tsadc */
> >   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> > @@ -1022,7 +1015,8 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
> >   };
> >   static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
> > -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> > +	/* cpu */
> > +	.chn_offset = 0,
> >   	.chn_num = 1, /* one channels for tsadc */
> >   	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> > @@ -1045,8 +1039,8 @@ static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
> >   };
> >   static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
> > -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> > -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> > +	/* cpu, gpu */
> > +	.chn_offset = 0,
> >   	.chn_num = 2, /* two channels for tsadc */
> >   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> > @@ -1070,8 +1064,8 @@ static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
> >   };
> >   static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
> > -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> > -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> > +	/* cpu, gpu */
> > +	.chn_offset = 0,
> >   	.chn_num = 2, /* two channels for tsadc */
> >   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> > @@ -1095,8 +1089,8 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
> >   };
> >   static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
> > -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> > -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> > +	/* cpu, gpu */
> > +	.chn_offset = 0,
> >   	.chn_num = 2, /* two channels for tsadc */
> >   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> > @@ -1120,8 +1114,8 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
> >   };
> >   static const struct rockchip_tsadc_chip rk3568_tsadc_data = {
> > -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> > -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> > +	/* cpu, gpu */
> > +	.chn_offset = 0,
> >   	.chn_num = 2, /* two channels for tsadc */
> >   	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> > @@ -1404,7 +1398,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
> >   	for (i = 0; i < thermal->chip->chn_num; i++) {
> >   		error = rockchip_thermal_register_sensor(pdev, thermal,
> >   						&thermal->sensors[i],
> > -						thermal->chip->chn_id[i]);
> > +						thermal->chip->chn_offset + i);
> >   		if (error)
> >   			return dev_err_probe(&pdev->dev, error,
> >   				"failed to register sensor[%d].\n", i);
> 
> -- 
> <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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND] [PATCHv3 4/7] thermal: rockchip: Simplify channel id logic

[Daniel Lezcano]

Hi Heiko,

On 08/03/2023 19:42, Sebastian Reichel wrote:
> Hi Daniel,
> 
> On Wed, Mar 08, 2023 at 07:13:22PM +0100, Daniel Lezcano wrote:
>> On 08/03/2023 12:22, Sebastian Reichel wrote:
>>> Replace the channel ID lookup table by a simple offset, since
>>> the channel IDs are consecutive.
>>>
>>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>>
>> As all the other patches are reviewed by Heiko, is the tag missing here?
> 
> Heiko was not happy with this in PATCHv2, when he reviewed most
> of the patches:
> 
> https://lore.kernel.org/all/3601039.e9J7NaK4W3@phil/
> 
> I replied, but never got a response, so I kept it as is:
> 
> https://lore.kernel.org/all/20221206170232.xsm4kcbfwrmlrriw@mercury.elektranox.org/
> 
> FWIW it is essential for the series and cannot be dropped, because
> RK3588 has more than 2 channels.

Do you have a suggestion to improve the proposed change ?

Thanks
    -- Daniel


>>> ---
>>>    drivers/thermal/rockchip_thermal.c | 48 +++++++++++++-----------------
>>>    1 file changed, 21 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
>>> index 9ed45b318344..bcbdd618daae 100644
>>> --- a/drivers/thermal/rockchip_thermal.c
>>> +++ b/drivers/thermal/rockchip_thermal.c
>>> @@ -39,15 +39,6 @@ enum tshut_polarity {
>>>    	TSHUT_HIGH_ACTIVE,
>>>    };
>>> -/*
>>> - * The system has two Temperature Sensors.
>>> - * sensor0 is for CPU, and sensor1 is for GPU.
>>> - */
>>> -enum sensor_id {
>>> -	SENSOR_CPU = 0,
>>> -	SENSOR_GPU,
>>> -};
>>> -
>>>    /*
>>>     * The conversion table has the adc value and temperature.
>>>     * ADC_DECREMENT: the adc value is of diminishing.(e.g. rk3288_code_table)
>>> @@ -82,7 +73,7 @@ struct chip_tsadc_table {
>>>    /**
>>>     * struct rockchip_tsadc_chip - hold the private data of tsadc chip
>>> - * @chn_id: array of sensor ids of chip corresponding to the channel
>>> + * @chn_offset: the channel offset of the first channel
>>>     * @chn_num: the channel number of tsadc chip
>>>     * @tshut_temp: the hardware-controlled shutdown temperature value
>>>     * @tshut_mode: the hardware-controlled shutdown mode (0:CRU 1:GPIO)
>>> @@ -98,7 +89,7 @@ struct chip_tsadc_table {
>>>     */
>>>    struct rockchip_tsadc_chip {
>>>    	/* The sensor id of chip correspond to the ADC channel */
>>> -	int chn_id[SOC_MAX_SENSORS];
>>> +	int chn_offset;
>>>    	int chn_num;
>>>    	/* The hardware-controlled tshut property */
>>> @@ -925,8 +916,8 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
>>>    }
>>>    static const struct rockchip_tsadc_chip px30_tsadc_data = {
>>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
>>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
>>> +	/* cpu, gpu */
>>> +	.chn_offset = 0,
>>>    	.chn_num = 2, /* 2 channels for tsadc */
>>>    	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
>>> @@ -949,7 +940,8 @@ static const struct rockchip_tsadc_chip px30_tsadc_data = {
>>>    };
>>>    static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
>>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
>>> +	/* cpu */
>>> +	.chn_offset = 0,
>>>    	.chn_num = 1, /* one channel for tsadc */
>>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
>>> @@ -973,7 +965,8 @@ static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
>>>    };
>>>    static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
>>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
>>> +	/* cpu */
>>> +	.chn_offset = 0,
>>>    	.chn_num = 1, /* one channel for tsadc */
>>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
>>> @@ -997,8 +990,8 @@ static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
>>>    };
>>>    static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
>>> -	.chn_id[SENSOR_CPU] = 1, /* cpu sensor is channel 1 */
>>> -	.chn_id[SENSOR_GPU] = 2, /* gpu sensor is channel 2 */
>>> +	/* cpu, gpu */
>>> +	.chn_offset = 1,
>>>    	.chn_num = 2, /* two channels for tsadc */
>>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
>>> @@ -1022,7 +1015,8 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
>>>    };
>>>    static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
>>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
>>> +	/* cpu */
>>> +	.chn_offset = 0,
>>>    	.chn_num = 1, /* one channels for tsadc */
>>>    	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
>>> @@ -1045,8 +1039,8 @@ static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
>>>    };
>>>    static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
>>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
>>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
>>> +	/* cpu, gpu */
>>> +	.chn_offset = 0,
>>>    	.chn_num = 2, /* two channels for tsadc */
>>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
>>> @@ -1070,8 +1064,8 @@ static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
>>>    };
>>>    static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
>>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
>>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
>>> +	/* cpu, gpu */
>>> +	.chn_offset = 0,
>>>    	.chn_num = 2, /* two channels for tsadc */
>>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
>>> @@ -1095,8 +1089,8 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
>>>    };
>>>    static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
>>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
>>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
>>> +	/* cpu, gpu */
>>> +	.chn_offset = 0,
>>>    	.chn_num = 2, /* two channels for tsadc */
>>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
>>> @@ -1120,8 +1114,8 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
>>>    };
>>>    static const struct rockchip_tsadc_chip rk3568_tsadc_data = {
>>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
>>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
>>> +	/* cpu, gpu */
>>> +	.chn_offset = 0,
>>>    	.chn_num = 2, /* two channels for tsadc */
>>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
>>> @@ -1404,7 +1398,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
>>>    	for (i = 0; i < thermal->chip->chn_num; i++) {
>>>    		error = rockchip_thermal_register_sensor(pdev, thermal,
>>>    						&thermal->sensors[i],
>>> -						thermal->chip->chn_id[i]);
>>> +						thermal->chip->chn_offset + i);
>>>    		if (error)
>>>    			return dev_err_probe(&pdev->dev, error,
>>>    				"failed to register sensor[%d].\n", i);
>>
>> -- 
>> <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
>>

-- 
<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: [RESEND] [PATCHv3 4/7] thermal: rockchip: Simplify channel id logic

[Heiko Stübner]

Am Donnerstag, 16. März 2023, 11:05:25 CEST schrieb Daniel Lezcano:
> 
> Hi Heiko,
> 
> On 08/03/2023 19:42, Sebastian Reichel wrote:
> > Hi Daniel,
> > 
> > On Wed, Mar 08, 2023 at 07:13:22PM +0100, Daniel Lezcano wrote:
> >> On 08/03/2023 12:22, Sebastian Reichel wrote:
> >>> Replace the channel ID lookup table by a simple offset, since
> >>> the channel IDs are consecutive.
> >>>
> >>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> >>
> >> As all the other patches are reviewed by Heiko, is the tag missing here?
> > 
> > Heiko was not happy with this in PATCHv2, when he reviewed most
> > of the patches:
> > 
> > https://lore.kernel.org/all/3601039.e9J7NaK4W3@phil/
> > 
> > I replied, but never got a response, so I kept it as is:
> > 
> > https://lore.kernel.org/all/20221206170232.xsm4kcbfwrmlrriw@mercury.elektranox.org/
> > 
> > FWIW it is essential for the series and cannot be dropped, because
> > RK3588 has more than 2 channels.
> 
> Do you have a suggestion to improve the proposed change ?

I guess it's fine after all.

Sebastian's response makes sense and there is not really a reason to keep
infrastructure around for a hypothetical case that may never happen.

If that really changes with some SoC in the far future we can always
re-evaluate.


So,

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Sorry for dropping the ball on this
Heiko


> >>> ---
> >>>    drivers/thermal/rockchip_thermal.c | 48 +++++++++++++-----------------
> >>>    1 file changed, 21 insertions(+), 27 deletions(-)
> >>>
> >>> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> >>> index 9ed45b318344..bcbdd618daae 100644
> >>> --- a/drivers/thermal/rockchip_thermal.c
> >>> +++ b/drivers/thermal/rockchip_thermal.c
> >>> @@ -39,15 +39,6 @@ enum tshut_polarity {
> >>>    	TSHUT_HIGH_ACTIVE,
> >>>    };
> >>> -/*
> >>> - * The system has two Temperature Sensors.
> >>> - * sensor0 is for CPU, and sensor1 is for GPU.
> >>> - */
> >>> -enum sensor_id {
> >>> -	SENSOR_CPU = 0,
> >>> -	SENSOR_GPU,
> >>> -};
> >>> -
> >>>    /*
> >>>     * The conversion table has the adc value and temperature.
> >>>     * ADC_DECREMENT: the adc value is of diminishing.(e.g. rk3288_code_table)
> >>> @@ -82,7 +73,7 @@ struct chip_tsadc_table {
> >>>    /**
> >>>     * struct rockchip_tsadc_chip - hold the private data of tsadc chip
> >>> - * @chn_id: array of sensor ids of chip corresponding to the channel
> >>> + * @chn_offset: the channel offset of the first channel
> >>>     * @chn_num: the channel number of tsadc chip
> >>>     * @tshut_temp: the hardware-controlled shutdown temperature value
> >>>     * @tshut_mode: the hardware-controlled shutdown mode (0:CRU 1:GPIO)
> >>> @@ -98,7 +89,7 @@ struct chip_tsadc_table {
> >>>     */
> >>>    struct rockchip_tsadc_chip {
> >>>    	/* The sensor id of chip correspond to the ADC channel */
> >>> -	int chn_id[SOC_MAX_SENSORS];
> >>> +	int chn_offset;
> >>>    	int chn_num;
> >>>    	/* The hardware-controlled tshut property */
> >>> @@ -925,8 +916,8 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
> >>>    }
> >>>    static const struct rockchip_tsadc_chip px30_tsadc_data = {
> >>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> >>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> >>> +	/* cpu, gpu */
> >>> +	.chn_offset = 0,
> >>>    	.chn_num = 2, /* 2 channels for tsadc */
> >>>    	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> >>> @@ -949,7 +940,8 @@ static const struct rockchip_tsadc_chip px30_tsadc_data = {
> >>>    };
> >>>    static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
> >>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> >>> +	/* cpu */
> >>> +	.chn_offset = 0,
> >>>    	.chn_num = 1, /* one channel for tsadc */
> >>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> >>> @@ -973,7 +965,8 @@ static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
> >>>    };
> >>>    static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
> >>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> >>> +	/* cpu */
> >>> +	.chn_offset = 0,
> >>>    	.chn_num = 1, /* one channel for tsadc */
> >>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> >>> @@ -997,8 +990,8 @@ static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
> >>>    };
> >>>    static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
> >>> -	.chn_id[SENSOR_CPU] = 1, /* cpu sensor is channel 1 */
> >>> -	.chn_id[SENSOR_GPU] = 2, /* gpu sensor is channel 2 */
> >>> +	/* cpu, gpu */
> >>> +	.chn_offset = 1,
> >>>    	.chn_num = 2, /* two channels for tsadc */
> >>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> >>> @@ -1022,7 +1015,8 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
> >>>    };
> >>>    static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
> >>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> >>> +	/* cpu */
> >>> +	.chn_offset = 0,
> >>>    	.chn_num = 1, /* one channels for tsadc */
> >>>    	.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> >>> @@ -1045,8 +1039,8 @@ static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
> >>>    };
> >>>    static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
> >>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> >>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> >>> +	/* cpu, gpu */
> >>> +	.chn_offset = 0,
> >>>    	.chn_num = 2, /* two channels for tsadc */
> >>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> >>> @@ -1070,8 +1064,8 @@ static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
> >>>    };
> >>>    static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
> >>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> >>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> >>> +	/* cpu, gpu */
> >>> +	.chn_offset = 0,
> >>>    	.chn_num = 2, /* two channels for tsadc */
> >>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> >>> @@ -1095,8 +1089,8 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
> >>>    };
> >>>    static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
> >>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> >>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> >>> +	/* cpu, gpu */
> >>> +	.chn_offset = 0,
> >>>    	.chn_num = 2, /* two channels for tsadc */
> >>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> >>> @@ -1120,8 +1114,8 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
> >>>    };
> >>>    static const struct rockchip_tsadc_chip rk3568_tsadc_data = {
> >>> -	.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
> >>> -	.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
> >>> +	/* cpu, gpu */
> >>> +	.chn_offset = 0,
> >>>    	.chn_num = 2, /* two channels for tsadc */
> >>>    	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> >>> @@ -1404,7 +1398,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
> >>>    	for (i = 0; i < thermal->chip->chn_num; i++) {
> >>>    		error = rockchip_thermal_register_sensor(pdev, thermal,
> >>>    						&thermal->sensors[i],
> >>> -						thermal->chip->chn_id[i]);
> >>> +						thermal->chip->chn_offset + i);
> >>>    		if (error)
> >>>    			return dev_err_probe(&pdev->dev, error,
> >>>    				"failed to register sensor[%d].\n", i);
> >>
> >>
> >> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> >> <http://twitter.com/#!/linaroorg> Twitter |
> >> <http://www.linaro.org/linaro-blog/> Blog
> >>
> 
> 

Re: [RESEND] [PATCHv3 4/7] thermal: rockchip: Simplify channel id logic

[Daniel Lezcano]

On 30/03/2023 19:07, Heiko Stübner wrote:
> Am Donnerstag, 16. März 2023, 11:05:25 CEST schrieb Daniel Lezcano:
>>
>> Hi Heiko,
>>
>> On 08/03/2023 19:42, Sebastian Reichel wrote:
>>> Hi Daniel,
>>>
>>> On Wed, Mar 08, 2023 at 07:13:22PM +0100, Daniel Lezcano wrote:
>>>> On 08/03/2023 12:22, Sebastian Reichel wrote:
>>>>> Replace the channel ID lookup table by a simple offset, since
>>>>> the channel IDs are consecutive.
>>>>>
>>>>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>>>>
>>>> As all the other patches are reviewed by Heiko, is the tag missing here?
>>>
>>> Heiko was not happy with this in PATCHv2, when he reviewed most
>>> of the patches:
>>>
>>> https://lore.kernel.org/all/3601039.e9J7NaK4W3@phil/
>>>
>>> I replied, but never got a response, so I kept it as is:
>>>
>>> https://lore.kernel.org/all/20221206170232.xsm4kcbfwrmlrriw@mercury.elektranox.org/
>>>
>>> FWIW it is essential for the series and cannot be dropped, because
>>> RK3588 has more than 2 channels.
>>
>> Do you have a suggestion to improve the proposed change ?
> 
> I guess it's fine after all.
> 
> Sebastian's response makes sense and there is not really a reason to keep
> infrastructure around for a hypothetical case that may never happen.
> 
> If that really changes with some SoC in the far future we can always
> re-evaluate.

Thanks, I've added your tag


-- 
<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

[RESEND] [PATCHv3 5/7] thermal: rockchip: Support dynamic sized sensor array

[Sebastian Reichel]

Dynamically allocate the sensors array based on the amount
of platform sensors in preparation for rk3588 support, which
needs 7 sensors.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/thermal/rockchip_thermal.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index bcbdd618daae..1a1435a2836b 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -51,12 +51,6 @@ enum adc_sort_mode {
 
 #include "thermal_hwmon.h"
 
-/*
- * The max sensors is two in rockchip SoCs.
- * Two sensors: CPU and GPU sensor.
- */
-#define SOC_MAX_SENSORS	2
-
 /**
  * struct chip_tsadc_table - hold information about chip-specific differences
  * @id: conversion table
@@ -147,7 +141,7 @@ struct rockchip_thermal_data {
 	struct platform_device *pdev;
 	struct reset_control *reset;
 
-	struct rockchip_thermal_sensor sensors[SOC_MAX_SENSORS];
+	struct rockchip_thermal_sensor *sensors;
 
 	struct clk *clk;
 	struct clk *pclk;
@@ -1366,6 +1360,11 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
 	if (!thermal->chip)
 		return -EINVAL;
 
+	thermal->sensors = devm_kcalloc(&pdev->dev, thermal->chip->chn_num,
+					sizeof(*thermal->sensors), GFP_KERNEL);
+	if (!thermal->sensors)
+		return -ENOMEM;
+
 	thermal->regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
 	if (IS_ERR(thermal->regs))
 		return PTR_ERR(thermal->regs);
-- 
2.39.2

[RESEND] [PATCHv3 6/7] thermal: rockchip: Support RK3588 SoC in the thermal driver

[Sebastian Reichel]

From: Finley Xiao <finley.xiao@rock-chips.com>

The RK3588 SoC has seven temperature sensor ADC channels:

- Chip Center
- CPU Cluster 1 (Dual A76 "Big" Cores)
- CPU Cluster 2 (Dual A76 "Big" Cores)
- CPU Cluster 0 (Quad A55 "Little" Cores)
- Power Domain Center
- Graphics Processing Unit
- Neural Processing Unit

Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
[rebase, squash fixes]
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/thermal/rockchip_thermal.c | 177 +++++++++++++++++++++++++++++
 1 file changed, 177 insertions(+)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 1a1435a2836b..e69c10a2a748 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -165,29 +165,49 @@ struct rockchip_thermal_data {
 #define TSADCV2_AUTO_CON			0x04
 #define TSADCV2_INT_EN				0x08
 #define TSADCV2_INT_PD				0x0c
+#define TSADCV3_AUTO_SRC_CON			0x0c
+#define TSADCV3_HT_INT_EN			0x14
+#define TSADCV3_HSHUT_GPIO_INT_EN		0x18
+#define TSADCV3_HSHUT_CRU_INT_EN		0x1c
+#define TSADCV3_INT_PD				0x24
+#define TSADCV3_HSHUT_PD			0x28
 #define TSADCV2_DATA(chn)			(0x20 + (chn) * 0x04)
 #define TSADCV2_COMP_INT(chn)		        (0x30 + (chn) * 0x04)
 #define TSADCV2_COMP_SHUT(chn)		        (0x40 + (chn) * 0x04)
+#define TSADCV3_DATA(chn)			(0x2c + (chn) * 0x04)
+#define TSADCV3_COMP_INT(chn)		        (0x6c + (chn) * 0x04)
+#define TSADCV3_COMP_SHUT(chn)		        (0x10c + (chn) * 0x04)
 #define TSADCV2_HIGHT_INT_DEBOUNCE		0x60
 #define TSADCV2_HIGHT_TSHUT_DEBOUNCE		0x64
+#define TSADCV3_HIGHT_INT_DEBOUNCE		0x14c
+#define TSADCV3_HIGHT_TSHUT_DEBOUNCE		0x150
 #define TSADCV2_AUTO_PERIOD			0x68
 #define TSADCV2_AUTO_PERIOD_HT			0x6c
+#define TSADCV3_AUTO_PERIOD			0x154
+#define TSADCV3_AUTO_PERIOD_HT			0x158
 
 #define TSADCV2_AUTO_EN				BIT(0)
+#define TSADCV2_AUTO_EN_MASK			BIT(16)
 #define TSADCV2_AUTO_SRC_EN(chn)		BIT(4 + (chn))
+#define TSADCV3_AUTO_SRC_EN(chn)		BIT(chn)
+#define TSADCV3_AUTO_SRC_EN_MASK(chn)		BIT(16 + chn)
 #define TSADCV2_AUTO_TSHUT_POLARITY_HIGH	BIT(8)
+#define TSADCV2_AUTO_TSHUT_POLARITY_MASK	BIT(24)
 
 #define TSADCV3_AUTO_Q_SEL_EN			BIT(1)
 
 #define TSADCV2_INT_SRC_EN(chn)			BIT(chn)
+#define TSADCV2_INT_SRC_EN_MASK(chn)		BIT(16 + (chn))
 #define TSADCV2_SHUT_2GPIO_SRC_EN(chn)		BIT(4 + (chn))
 #define TSADCV2_SHUT_2CRU_SRC_EN(chn)		BIT(8 + (chn))
 
 #define TSADCV2_INT_PD_CLEAR_MASK		~BIT(8)
 #define TSADCV3_INT_PD_CLEAR_MASK		~BIT(16)
+#define TSADCV4_INT_PD_CLEAR_MASK		0xffffffff
 
 #define TSADCV2_DATA_MASK			0xfff
 #define TSADCV3_DATA_MASK			0x3ff
+#define TSADCV4_DATA_MASK			0x1ff
 
 #define TSADCV2_HIGHT_INT_DEBOUNCE_COUNT	4
 #define TSADCV2_HIGHT_TSHUT_DEBOUNCE_COUNT	4
@@ -198,6 +218,8 @@ struct rockchip_thermal_data {
 
 #define TSADCV5_AUTO_PERIOD_TIME		1622 /* 2.5ms */
 #define TSADCV5_AUTO_PERIOD_HT_TIME		1622 /* 2.5ms */
+#define TSADCV6_AUTO_PERIOD_TIME		5000 /* 2.5ms */
+#define TSADCV6_AUTO_PERIOD_HT_TIME		5000 /* 2.5ms */
 
 #define TSADCV2_USER_INTER_PD_SOC		0x340 /* 13 clocks */
 #define TSADCV5_USER_INTER_PD_SOC		0xfc0 /* 97us, at least 90us */
@@ -214,6 +236,12 @@ struct rockchip_thermal_data {
 #define RK3568_GRF_TSADC_ANA_REG2		(0x10001 << 2)
 #define RK3568_GRF_TSADC_TSEN			(0x10001 << 8)
 
+#define RK3588_GRF0_TSADC_CON			0x0100
+
+#define RK3588_GRF0_TSADC_TRM			(0xff0077 << 0)
+#define RK3588_GRF0_TSADC_SHUT_2CRU		(0x30003 << 10)
+#define RK3588_GRF0_TSADC_SHUT_2GPIO		(0x70007 << 12)
+
 #define GRF_SARADC_TESTBIT_ON			(0x10001 << 2)
 #define GRF_TSADC_TESTBIT_H_ON			(0x10001 << 2)
 #define GRF_TSADC_VCM_EN_L			(0x10001 << 7)
@@ -508,6 +536,15 @@ static const struct tsadc_table rk3568_code_table[] = {
 	{TSADCV2_DATA_MASK, 125000},
 };
 
+static const struct tsadc_table rk3588_code_table[] = {
+	{0, -40000},
+	{215, -40000},
+	{285, 25000},
+	{350, 85000},
+	{395, 125000},
+	{TSADCV4_DATA_MASK, 125000},
+};
+
 static u32 rk_tsadcv2_temp_to_code(const struct chip_tsadc_table *table,
 				   int temp)
 {
@@ -778,6 +815,25 @@ static void rk_tsadcv7_initialize(struct regmap *grf, void __iomem *regs,
 	}
 }
 
+static void rk_tsadcv8_initialize(struct regmap *grf, void __iomem *regs,
+				  enum tshut_polarity tshut_polarity)
+{
+	writel_relaxed(TSADCV6_AUTO_PERIOD_TIME, regs + TSADCV3_AUTO_PERIOD);
+	writel_relaxed(TSADCV6_AUTO_PERIOD_HT_TIME,
+		       regs + TSADCV3_AUTO_PERIOD_HT);
+	writel_relaxed(TSADCV2_HIGHT_INT_DEBOUNCE_COUNT,
+		       regs + TSADCV3_HIGHT_INT_DEBOUNCE);
+	writel_relaxed(TSADCV2_HIGHT_TSHUT_DEBOUNCE_COUNT,
+		       regs + TSADCV3_HIGHT_TSHUT_DEBOUNCE);
+	if (tshut_polarity == TSHUT_HIGH_ACTIVE)
+		writel_relaxed(TSADCV2_AUTO_TSHUT_POLARITY_HIGH |
+			       TSADCV2_AUTO_TSHUT_POLARITY_MASK,
+			       regs + TSADCV2_AUTO_CON);
+	else
+		writel_relaxed(TSADCV2_AUTO_TSHUT_POLARITY_MASK,
+			       regs + TSADCV2_AUTO_CON);
+}
+
 static void rk_tsadcv2_irq_ack(void __iomem *regs)
 {
 	u32 val;
@@ -794,6 +850,17 @@ static void rk_tsadcv3_irq_ack(void __iomem *regs)
 	writel_relaxed(val & TSADCV3_INT_PD_CLEAR_MASK, regs + TSADCV2_INT_PD);
 }
 
+static void rk_tsadcv4_irq_ack(void __iomem *regs)
+{
+	u32 val;
+
+	val = readl_relaxed(regs + TSADCV3_INT_PD);
+	writel_relaxed(val & TSADCV4_INT_PD_CLEAR_MASK, regs + TSADCV3_INT_PD);
+	val = readl_relaxed(regs + TSADCV3_HSHUT_PD);
+	writel_relaxed(val & TSADCV3_INT_PD_CLEAR_MASK,
+		       regs + TSADCV3_HSHUT_PD);
+}
+
 static void rk_tsadcv2_control(void __iomem *regs, bool enable)
 {
 	u32 val;
@@ -829,6 +896,18 @@ static void rk_tsadcv3_control(void __iomem *regs, bool enable)
 	writel_relaxed(val, regs + TSADCV2_AUTO_CON);
 }
 
+static void rk_tsadcv4_control(void __iomem *regs, bool enable)
+{
+	u32 val;
+
+	if (enable)
+		val = TSADCV2_AUTO_EN | TSADCV2_AUTO_EN_MASK;
+	else
+		val = TSADCV2_AUTO_EN_MASK;
+
+	writel_relaxed(val, regs + TSADCV2_AUTO_CON);
+}
+
 static int rk_tsadcv2_get_temp(const struct chip_tsadc_table *table,
 			       int chn, void __iomem *regs, int *temp)
 {
@@ -839,6 +918,16 @@ static int rk_tsadcv2_get_temp(const struct chip_tsadc_table *table,
 	return rk_tsadcv2_code_to_temp(table, val, temp);
 }
 
+static int rk_tsadcv4_get_temp(const struct chip_tsadc_table *table,
+			       int chn, void __iomem *regs, int *temp)
+{
+	u32 val;
+
+	val = readl_relaxed(regs + TSADCV3_DATA(chn));
+
+	return rk_tsadcv2_code_to_temp(table, val, temp);
+}
+
 static int rk_tsadcv2_alarm_temp(const struct chip_tsadc_table *table,
 				 int chn, void __iomem *regs, int temp)
 {
@@ -873,6 +962,33 @@ static int rk_tsadcv2_alarm_temp(const struct chip_tsadc_table *table,
 	return 0;
 }
 
+static int rk_tsadcv3_alarm_temp(const struct chip_tsadc_table *table,
+				 int chn, void __iomem *regs, int temp)
+{
+	u32 alarm_value;
+
+	/*
+	 * In some cases, some sensors didn't need the trip points, the
+	 * set_trips will pass {-INT_MAX, INT_MAX} to trigger tsadc alarm
+	 * in the end, ignore this case and disable the high temperature
+	 * interrupt.
+	 */
+	if (temp == INT_MAX) {
+		writel_relaxed(TSADCV2_INT_SRC_EN_MASK(chn),
+			       regs + TSADCV3_HT_INT_EN);
+		return 0;
+	}
+	/* Make sure the value is valid */
+	alarm_value = rk_tsadcv2_temp_to_code(table, temp);
+	if (alarm_value == table->data_mask)
+		return -ERANGE;
+	writel_relaxed(alarm_value & table->data_mask,
+		       regs + TSADCV3_COMP_INT(chn));
+	writel_relaxed(TSADCV2_INT_SRC_EN(chn) | TSADCV2_INT_SRC_EN_MASK(chn),
+		       regs + TSADCV3_HT_INT_EN);
+	return 0;
+}
+
 static int rk_tsadcv2_tshut_temp(const struct chip_tsadc_table *table,
 				 int chn, void __iomem *regs, int temp)
 {
@@ -892,6 +1008,25 @@ static int rk_tsadcv2_tshut_temp(const struct chip_tsadc_table *table,
 	return 0;
 }
 
+static int rk_tsadcv3_tshut_temp(const struct chip_tsadc_table *table,
+				 int chn, void __iomem *regs, int temp)
+{
+	u32 tshut_value;
+
+	/* Make sure the value is valid */
+	tshut_value = rk_tsadcv2_temp_to_code(table, temp);
+	if (tshut_value == table->data_mask)
+		return -ERANGE;
+
+	writel_relaxed(tshut_value, regs + TSADCV3_COMP_SHUT(chn));
+
+	/* TSHUT will be valid */
+	writel_relaxed(TSADCV3_AUTO_SRC_EN(chn) | TSADCV3_AUTO_SRC_EN_MASK(chn),
+		       regs + TSADCV3_AUTO_SRC_CON);
+
+	return 0;
+}
+
 static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
 				  enum tshut_mode mode)
 {
@@ -909,6 +1044,22 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
 	writel_relaxed(val, regs + TSADCV2_INT_EN);
 }
 
+static void rk_tsadcv3_tshut_mode(int chn, void __iomem *regs,
+				  enum tshut_mode mode)
+{
+	u32 val_gpio, val_cru;
+
+	if (mode == TSHUT_MODE_GPIO) {
+		val_gpio = TSADCV2_INT_SRC_EN(chn) | TSADCV2_INT_SRC_EN_MASK(chn);
+		val_cru = TSADCV2_INT_SRC_EN_MASK(chn);
+	} else {
+		val_cru = TSADCV2_INT_SRC_EN(chn) | TSADCV2_INT_SRC_EN_MASK(chn);
+		val_gpio = TSADCV2_INT_SRC_EN_MASK(chn);
+	}
+	writel_relaxed(val_gpio, regs + TSADCV3_HSHUT_GPIO_INT_EN);
+	writel_relaxed(val_cru, regs + TSADCV3_HSHUT_CRU_INT_EN);
+}
+
 static const struct rockchip_tsadc_chip px30_tsadc_data = {
 	/* cpu, gpu */
 	.chn_offset = 0,
@@ -1132,6 +1283,28 @@ static const struct rockchip_tsadc_chip rk3568_tsadc_data = {
 	},
 };
 
+static const struct rockchip_tsadc_chip rk3588_tsadc_data = {
+	/* top, big_core0, big_core1, little_core, center, gpu, npu */
+	.chn_offset = 0,
+	.chn_num = 7, /* seven channels for tsadc */
+	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
+	.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
+	.tshut_temp = 95000,
+	.initialize = rk_tsadcv8_initialize,
+	.irq_ack = rk_tsadcv4_irq_ack,
+	.control = rk_tsadcv4_control,
+	.get_temp = rk_tsadcv4_get_temp,
+	.set_alarm_temp = rk_tsadcv3_alarm_temp,
+	.set_tshut_temp = rk_tsadcv3_tshut_temp,
+	.set_tshut_mode = rk_tsadcv3_tshut_mode,
+	.table = {
+		.id = rk3588_code_table,
+		.length = ARRAY_SIZE(rk3588_code_table),
+		.data_mask = TSADCV4_DATA_MASK,
+		.mode = ADC_INCREMENT,
+	},
+};
+
 static const struct of_device_id of_rockchip_thermal_match[] = {
 	{	.compatible = "rockchip,px30-tsadc",
 		.data = (void *)&px30_tsadc_data,
@@ -1168,6 +1341,10 @@ static const struct of_device_id of_rockchip_thermal_match[] = {
 		.compatible = "rockchip,rk3568-tsadc",
 		.data = (void *)&rk3568_tsadc_data,
 	},
+	{
+		.compatible = "rockchip,rk3588-tsadc",
+		.data = (void *)&rk3588_tsadc_data,
+	},
 	{ /* end */ },
 };
 MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
-- 
2.39.2

[RESEND] [PATCHv3 7/7] dt-bindings: rockchip-thermal: Support the RK3588 SoC compatible

[Sebastian Reichel]

Add a new compatible for the thermal sensor device on RK3588 SoCs.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 Documentation/devicetree/bindings/thermal/rockchip-thermal.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/thermal/rockchip-thermal.yaml b/Documentation/devicetree/bindings/thermal/rockchip-thermal.yaml
index f6c1be226aaa..55f8ec0bec01 100644
--- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.yaml
@@ -19,6 +19,7 @@ properties:
       - rockchip,rk3368-tsadc
       - rockchip,rk3399-tsadc
       - rockchip,rk3568-tsadc
+      - rockchip,rk3588-tsadc
       - rockchip,rv1108-tsadc
 
   reg:
-- 
2.39.2