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/[email protected]/
* rebased against v6.3-rc1
Changes since PATCHv2:
* https://lore.kernel.org/all/[email protected]/
* 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/[email protected]/
* 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
Use dev_err_probe to simplify error printing in the driver's probe
routine.
Reviewed-by: Heiko Stuebner <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
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
Replace the channel ID lookup table by a simple offset, since
the channel IDs are consecutive.
Signed-off-by: Sebastian Reichel <[email protected]>
---
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
Add a new compatible for the thermal sensor device on RK3588 SoCs.
Reviewed-by: Heiko Stuebner <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
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
From: Finley Xiao <[email protected]>
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 <[email protected]>
[rebase, squash fixes]
Reviewed-by: Heiko Stuebner <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
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
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 <[email protected]>
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
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 <[email protected]>
>
> 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/[email protected]/
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
>
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 <[email protected]>
>>
>> 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/[email protected]/
>
> 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
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 <[email protected]>
> >>
> >> 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/[email protected]/
> >
> > 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 <[email protected]>
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
> >>
>
>
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 <[email protected]>
>>>>
>>>> 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/[email protected]/
>>>
>>> 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