* [PATCH 1/9] thermal/drivers/mediatek/lvts_thermal: retrieve all calibration bytes
2024-01-11 22:29 [PATCH 0/9] Mediatek thermal sensor driver support for MT8186 and MT8188 Nicolas Pitre
@ 2024-01-11 22:29 ` Nicolas Pitre
2024-01-11 22:29 ` [PATCH 2/9] thermal/drivers/mediatek/lvts_thermal: move comment Nicolas Pitre
` (7 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-11 22:29 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
From: Nicolas Pitre <npitre@baylibre.com>
Calibration values are 24-bit wide. Those values so far appear to span
only 16 bits but let's not push our luck.
Found while looking at the original Mediatek driver code.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
drivers/thermal/mediatek/lvts_thermal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 98d9c80bd4..8aa6a8675b 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -679,7 +679,7 @@ static int lvts_calibration_init(struct device *dev, struct lvts_ctrl *lvts_ctrl
for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++)
memcpy(&lvts_ctrl->calibration[i],
- efuse_calibration + lvts_ctrl_data->cal_offset[i], 2);
+ efuse_calibration + lvts_ctrl_data->cal_offset[i], 3);
return 0;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 2/9] thermal/drivers/mediatek/lvts_thermal: move comment
2024-01-11 22:29 [PATCH 0/9] Mediatek thermal sensor driver support for MT8186 and MT8188 Nicolas Pitre
2024-01-11 22:29 ` [PATCH 1/9] thermal/drivers/mediatek/lvts_thermal: retrieve all calibration bytes Nicolas Pitre
@ 2024-01-11 22:29 ` Nicolas Pitre
2024-01-11 22:30 ` [PATCH 3/9] thermal/drivers/mediatek/lvts_thermal: use offsets for every calibration byte Nicolas Pitre
` (6 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-11 22:29 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
From: Nicolas Pitre <npitre@baylibre.com>
Move efuse data interpretation inside lvts_golden_temp_init() alongside
the actual code retrieving wanted value.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
drivers/thermal/mediatek/lvts_thermal.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 8aa6a8675b..73ca2be0f5 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -732,11 +732,15 @@ static int lvts_calibration_read(struct device *dev, struct lvts_domain *lvts_td
return 0;
}
-static int lvts_golden_temp_init(struct device *dev, u32 *value, int temp_offset)
+static int lvts_golden_temp_init(struct device *dev, u8 *calib, int temp_offset)
{
u32 gt;
- gt = (*value) >> 24;
+ /*
+ * The golden temp information is contained in the 4th byte (index = 3)
+ * of efuse data.
+ */
+ gt = calib[3];
if (gt && gt < LVTS_GOLDEN_TEMP_MAX)
golden_temp = gt;
@@ -760,11 +764,7 @@ static int lvts_ctrl_init(struct device *dev, struct lvts_domain *lvts_td,
if (ret)
return ret;
- /*
- * The golden temp information is contained in the first chunk
- * of efuse data.
- */
- ret = lvts_golden_temp_init(dev, (u32 *)lvts_td->calib, lvts_data->temp_offset);
+ ret = lvts_golden_temp_init(dev, lvts_td->calib, lvts_data->temp_offset);
if (ret)
return ret;
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 3/9] thermal/drivers/mediatek/lvts_thermal: use offsets for every calibration byte
2024-01-11 22:29 [PATCH 0/9] Mediatek thermal sensor driver support for MT8186 and MT8188 Nicolas Pitre
2024-01-11 22:29 ` [PATCH 1/9] thermal/drivers/mediatek/lvts_thermal: retrieve all calibration bytes Nicolas Pitre
2024-01-11 22:29 ` [PATCH 2/9] thermal/drivers/mediatek/lvts_thermal: move comment Nicolas Pitre
@ 2024-01-11 22:30 ` Nicolas Pitre
2024-01-11 22:30 ` [PATCH 4/9] thermal/drivers/mediatek/lvts_thermal: guard against efuse data buffer overflow Nicolas Pitre
` (5 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-11 22:30 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
From: Nicolas Pitre <npitre@baylibre.com>
Current code assumes calibration values are always stored contiguously
in host endian order. A future patch will prove this wrong.
Let's specify the offset for each calibration byte instead.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
drivers/thermal/mediatek/lvts_thermal.c | 165 ++++++++++++++----------
1 file changed, 99 insertions(+), 66 deletions(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 73ca2be0f5..2c346ea7c6 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -102,11 +102,11 @@ static int golden_temp_offset;
struct lvts_sensor_data {
int dt_id;
+ u8 cal_offsets[3];
};
struct lvts_ctrl_data {
struct lvts_sensor_data lvts_sensor[LVTS_SENSOR_MAX];
- int cal_offset[LVTS_SENSOR_MAX];
int hw_tshut_temp;
int num_lvts_sensor;
int offset;
@@ -668,8 +668,9 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
* <-----ap--tc#3-----> <-----sensor#7-----> <-----sensor#8----->
* 0x40 | 0x41 | 0x42 | 0x43 | 0x44 | 0x45 | 0x46 | 0x47 | 0x48
*
- * The data description gives the offset of the calibration data in
- * this bytes stream for each sensor.
+ * Note: In some cases, values don't strictly follow a little endian ordering.
+ * The data description gives byte offsets constituting each calibration value
+ * for each sensor.
*/
static int lvts_calibration_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
const struct lvts_ctrl_data *lvts_ctrl_data,
@@ -677,9 +678,15 @@ static int lvts_calibration_init(struct device *dev, struct lvts_ctrl *lvts_ctrl
{
int i;
- for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++)
- memcpy(&lvts_ctrl->calibration[i],
- efuse_calibration + lvts_ctrl_data->cal_offset[i], 3);
+ for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++) {
+ const struct lvts_sensor_data *sensor =
+ &lvts_ctrl_data->lvts_sensor[i];
+
+ lvts_ctrl->calibration[i] =
+ (efuse_calibration[sensor->cal_offsets[0]] << 0) +
+ (efuse_calibration[sensor->cal_offsets[1]] << 8) +
+ (efuse_calibration[sensor->cal_offsets[2]] << 16);
+ }
return 0;
}
@@ -1300,24 +1307,30 @@ static void lvts_remove(struct platform_device *pdev)
static const struct lvts_ctrl_data mt7988_lvts_ap_data_ctrl[] = {
{
- .cal_offset = { 0x00, 0x04, 0x08, 0x0c },
.lvts_sensor = {
- { .dt_id = MT7988_CPU_0 },
- { .dt_id = MT7988_CPU_1 },
- { .dt_id = MT7988_ETH2P5G_0 },
- { .dt_id = MT7988_ETH2P5G_1 }
+ { .dt_id = MT7988_CPU_0,
+ .cal_offsets = { 0x00, 0x01, 0x02 } },
+ { .dt_id = MT7988_CPU_1,
+ .cal_offsets = { 0x04, 0x05, 0x06 } },
+ { .dt_id = MT7988_ETH2P5G_0,
+ .cal_offsets = { 0x08, 0x09, 0x0a } },
+ { .dt_id = MT7988_ETH2P5G_1,
+ .cal_offsets = { 0x0c, 0x0d, 0x0e } }
},
.num_lvts_sensor = 4,
.offset = 0x0,
.hw_tshut_temp = LVTS_HW_SHUTDOWN_MT7988,
},
{
- .cal_offset = { 0x14, 0x18, 0x1c, 0x20 },
.lvts_sensor = {
- { .dt_id = MT7988_TOPS_0},
- { .dt_id = MT7988_TOPS_1},
- { .dt_id = MT7988_ETHWARP_0},
- { .dt_id = MT7988_ETHWARP_1}
+ { .dt_id = MT7988_TOPS_0,
+ .cal_offsets = { 0x14, 0x15, 0x16 } },
+ { .dt_id = MT7988_TOPS_1,
+ .cal_offsets = { 0x18, 0x19, 0x1a } },
+ { .dt_id = MT7988_ETHWARP_0,
+ .cal_offsets = { 0x1c, 0x1d, 0x1e } },
+ { .dt_id = MT7988_ETHWARP_1,
+ .cal_offsets = { 0x20, 0x21, 0x22 } }
},
.num_lvts_sensor = 4,
.offset = 0x100,
@@ -1359,10 +1372,11 @@ static int lvts_resume(struct device *dev)
static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
{
- .cal_offset = { 0x04, 0x08 },
.lvts_sensor = {
- { .dt_id = MT8192_MCU_BIG_CPU0 },
- { .dt_id = MT8192_MCU_BIG_CPU1 }
+ { .dt_id = MT8192_MCU_BIG_CPU0,
+ .cal_offsets = { 0x04, 0x05, 0x06 } },
+ { .dt_id = MT8192_MCU_BIG_CPU1,
+ .cal_offsets = { 0x08, 0x09, 0x0a } }
},
.num_lvts_sensor = 2,
.offset = 0x0,
@@ -1370,10 +1384,11 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
.mode = LVTS_MSR_FILTERED_MODE,
},
{
- .cal_offset = { 0x0c, 0x10 },
.lvts_sensor = {
- { .dt_id = MT8192_MCU_BIG_CPU2 },
- { .dt_id = MT8192_MCU_BIG_CPU3 }
+ { .dt_id = MT8192_MCU_BIG_CPU2,
+ .cal_offsets = { 0x0c, 0x0d, 0x0e } },
+ { .dt_id = MT8192_MCU_BIG_CPU3,
+ .cal_offsets = { 0x10, 0x11, 0x12 } }
},
.num_lvts_sensor = 2,
.offset = 0x100,
@@ -1381,12 +1396,15 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
.mode = LVTS_MSR_FILTERED_MODE,
},
{
- .cal_offset = { 0x14, 0x18, 0x1c, 0x20 },
.lvts_sensor = {
- { .dt_id = MT8192_MCU_LITTLE_CPU0 },
- { .dt_id = MT8192_MCU_LITTLE_CPU1 },
- { .dt_id = MT8192_MCU_LITTLE_CPU2 },
- { .dt_id = MT8192_MCU_LITTLE_CPU3 }
+ { .dt_id = MT8192_MCU_LITTLE_CPU0,
+ .cal_offsets = { 0x14, 0x15, 0x16 } },
+ { .dt_id = MT8192_MCU_LITTLE_CPU1,
+ .cal_offsets = { 0x18, 0x19, 0x1a } },
+ { .dt_id = MT8192_MCU_LITTLE_CPU2,
+ .cal_offsets = { 0x1c, 0x1d, 0x1e } },
+ { .dt_id = MT8192_MCU_LITTLE_CPU3,
+ .cal_offsets = { 0x20, 0x21, 0x22 } }
},
.num_lvts_sensor = 4,
.offset = 0x200,
@@ -1396,42 +1414,47 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
};
static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
- {
- .cal_offset = { 0x24, 0x28 },
+ {
.lvts_sensor = {
- { .dt_id = MT8192_AP_VPU0 },
- { .dt_id = MT8192_AP_VPU1 }
+ { .dt_id = MT8192_AP_VPU0,
+ .cal_offsets = { 0x24, 0x25, 0x26 } },
+ { .dt_id = MT8192_AP_VPU1,
+ .cal_offsets = { 0x28, 0x29, 0x2a } }
},
.num_lvts_sensor = 2,
.offset = 0x0,
.hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
},
{
- .cal_offset = { 0x2c, 0x30 },
.lvts_sensor = {
- { .dt_id = MT8192_AP_GPU0 },
- { .dt_id = MT8192_AP_GPU1 }
+ { .dt_id = MT8192_AP_GPU0,
+ .cal_offsets = { 0x2c, 0x2d, 0x2e } },
+ { .dt_id = MT8192_AP_GPU1,
+ .cal_offsets = { 0x30, 0x31, 0x32 } }
},
.num_lvts_sensor = 2,
.offset = 0x100,
.hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
},
{
- .cal_offset = { 0x34, 0x38 },
.lvts_sensor = {
- { .dt_id = MT8192_AP_INFRA },
- { .dt_id = MT8192_AP_CAM },
+ { .dt_id = MT8192_AP_INFRA,
+ .cal_offsets = { 0x34, 0x35, 0x36 } },
+ { .dt_id = MT8192_AP_CAM,
+ .cal_offsets = { 0x38, 0x39, 0x3a } },
},
.num_lvts_sensor = 2,
.offset = 0x200,
.hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
},
{
- .cal_offset = { 0x3c, 0x40, 0x44 },
.lvts_sensor = {
- { .dt_id = MT8192_AP_MD0 },
- { .dt_id = MT8192_AP_MD1 },
- { .dt_id = MT8192_AP_MD2 }
+ { .dt_id = MT8192_AP_MD0,
+ .cal_offsets = { 0x3c, 0x3d, 0x3e } },
+ { .dt_id = MT8192_AP_MD1,
+ .cal_offsets = { 0x40, 0x41, 0x42 } },
+ { .dt_id = MT8192_AP_MD2,
+ .cal_offsets = { 0x44, 0x45, 0x46 } }
},
.num_lvts_sensor = 3,
.offset = 0x300,
@@ -1441,32 +1464,37 @@ static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
static const struct lvts_ctrl_data mt8195_lvts_mcu_data_ctrl[] = {
{
- .cal_offset = { 0x04, 0x07 },
.lvts_sensor = {
- { .dt_id = MT8195_MCU_BIG_CPU0 },
- { .dt_id = MT8195_MCU_BIG_CPU1 }
+ { .dt_id = MT8195_MCU_BIG_CPU0,
+ .cal_offsets = { 0x04, 0x05, 0x06 } },
+ { .dt_id = MT8195_MCU_BIG_CPU1,
+ .cal_offsets = { 0x07, 0x08, 0x09 } }
},
.num_lvts_sensor = 2,
.offset = 0x0,
.hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
},
{
- .cal_offset = { 0x0d, 0x10 },
.lvts_sensor = {
- { .dt_id = MT8195_MCU_BIG_CPU2 },
- { .dt_id = MT8195_MCU_BIG_CPU3 }
+ { .dt_id = MT8195_MCU_BIG_CPU2,
+ .cal_offsets = { 0x0d, 0x0e, 0x0f } },
+ { .dt_id = MT8195_MCU_BIG_CPU3,
+ .cal_offsets = { 0x10, 0x11, 0x12 } }
},
.num_lvts_sensor = 2,
.offset = 0x100,
.hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
},
{
- .cal_offset = { 0x16, 0x19, 0x1c, 0x1f },
.lvts_sensor = {
- { .dt_id = MT8195_MCU_LITTLE_CPU0 },
- { .dt_id = MT8195_MCU_LITTLE_CPU1 },
- { .dt_id = MT8195_MCU_LITTLE_CPU2 },
- { .dt_id = MT8195_MCU_LITTLE_CPU3 }
+ { .dt_id = MT8195_MCU_LITTLE_CPU0,
+ .cal_offsets = { 0x16, 0x17, 0x18 } },
+ { .dt_id = MT8195_MCU_LITTLE_CPU1,
+ .cal_offsets = { 0x19, 0x1a, 0x1b } },
+ { .dt_id = MT8195_MCU_LITTLE_CPU2,
+ .cal_offsets = { 0x1c, 0x1d, 0x1e } },
+ { .dt_id = MT8195_MCU_LITTLE_CPU3,
+ .cal_offsets = { 0x1f, 0x20, 0x21 } }
},
.num_lvts_sensor = 4,
.offset = 0x200,
@@ -1475,42 +1503,47 @@ static const struct lvts_ctrl_data mt8195_lvts_mcu_data_ctrl[] = {
};
static const struct lvts_ctrl_data mt8195_lvts_ap_data_ctrl[] = {
- {
- .cal_offset = { 0x25, 0x28 },
+ {
.lvts_sensor = {
- { .dt_id = MT8195_AP_VPU0 },
- { .dt_id = MT8195_AP_VPU1 }
+ { .dt_id = MT8195_AP_VPU0,
+ .cal_offsets = { 0x25, 0x26, 0x27 } },
+ { .dt_id = MT8195_AP_VPU1,
+ .cal_offsets = { 0x28, 0x29, 0x2a } }
},
.num_lvts_sensor = 2,
.offset = 0x0,
.hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
},
{
- .cal_offset = { 0x2e, 0x31 },
.lvts_sensor = {
- { .dt_id = MT8195_AP_GPU0 },
- { .dt_id = MT8195_AP_GPU1 }
+ { .dt_id = MT8195_AP_GPU0,
+ .cal_offsets = { 0x2e, 0x2f, 0x30 } },
+ { .dt_id = MT8195_AP_GPU1,
+ .cal_offsets = { 0x31, 0x32, 0x33 } }
},
.num_lvts_sensor = 2,
.offset = 0x100,
.hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
},
{
- .cal_offset = { 0x37, 0x3a, 0x3d },
.lvts_sensor = {
- { .dt_id = MT8195_AP_VDEC },
- { .dt_id = MT8195_AP_IMG },
- { .dt_id = MT8195_AP_INFRA },
+ { .dt_id = MT8195_AP_VDEC,
+ .cal_offsets = { 0x37, 0x38, 0x39 } },
+ { .dt_id = MT8195_AP_IMG,
+ .cal_offsets = { 0x3a, 0x3b, 0x3c } },
+ { .dt_id = MT8195_AP_INFRA,
+ .cal_offsets = { 0x3d, 0x3e, 0x3f } }
},
.num_lvts_sensor = 3,
.offset = 0x200,
.hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
},
{
- .cal_offset = { 0x43, 0x46 },
.lvts_sensor = {
- { .dt_id = MT8195_AP_CAM0 },
- { .dt_id = MT8195_AP_CAM1 }
+ { .dt_id = MT8195_AP_CAM0,
+ .cal_offsets = { 0x43, 0x44, 0x45 } },
+ { .dt_id = MT8195_AP_CAM1,
+ .cal_offsets = { 0x46, 0x47, 0x48 } }
},
.num_lvts_sensor = 2,
.offset = 0x300,
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 4/9] thermal/drivers/mediatek/lvts_thermal: guard against efuse data buffer overflow
2024-01-11 22:29 [PATCH 0/9] Mediatek thermal sensor driver support for MT8186 and MT8188 Nicolas Pitre
` (2 preceding siblings ...)
2024-01-11 22:30 ` [PATCH 3/9] thermal/drivers/mediatek/lvts_thermal: use offsets for every calibration byte Nicolas Pitre
@ 2024-01-11 22:30 ` Nicolas Pitre
2024-01-11 22:30 ` [PATCH 5/9] thermal/drivers/mediatek/lvts_thermal: add MT8186 support Nicolas Pitre
` (4 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-11 22:30 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
From: Nicolas Pitre <npitre@baylibre.com>
We don't want to silently fetch garbage past the actual buffer.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
drivers/thermal/mediatek/lvts_thermal.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 2c346ea7c6..ed1888fb24 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -674,7 +674,8 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
*/
static int lvts_calibration_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
const struct lvts_ctrl_data *lvts_ctrl_data,
- u8 *efuse_calibration)
+ u8 *efuse_calibration,
+ size_t calib_len)
{
int i;
@@ -682,6 +683,11 @@ static int lvts_calibration_init(struct device *dev, struct lvts_ctrl *lvts_ctrl
const struct lvts_sensor_data *sensor =
&lvts_ctrl_data->lvts_sensor[i];
+ if (sensor->cal_offsets[0] >= calib_len ||
+ sensor->cal_offsets[1] >= calib_len ||
+ sensor->cal_offsets[2] >= calib_len)
+ return -EINVAL;
+
lvts_ctrl->calibration[i] =
(efuse_calibration[sensor->cal_offsets[0]] << 0) +
(efuse_calibration[sensor->cal_offsets[1]] << 8) +
@@ -791,7 +797,8 @@ static int lvts_ctrl_init(struct device *dev, struct lvts_domain *lvts_td,
ret = lvts_calibration_init(dev, &lvts_ctrl[i],
&lvts_data->lvts_ctrl[i],
- lvts_td->calib);
+ lvts_td->calib,
+ lvts_td->calib_len);
if (ret)
return ret;
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 5/9] thermal/drivers/mediatek/lvts_thermal: add MT8186 support
2024-01-11 22:29 [PATCH 0/9] Mediatek thermal sensor driver support for MT8186 and MT8188 Nicolas Pitre
` (3 preceding siblings ...)
2024-01-11 22:30 ` [PATCH 4/9] thermal/drivers/mediatek/lvts_thermal: guard against efuse data buffer overflow Nicolas Pitre
@ 2024-01-11 22:30 ` Nicolas Pitre
2024-01-11 22:30 ` [PATCH 6/9] arm64: dts: mediatek: mt8186: add default thermal zones Nicolas Pitre
` (3 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-11 22:30 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
From: Nicolas Pitre <npitre@baylibre.com>
Various values extracted from the vendor's kernel driver.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
.../thermal/mediatek,lvts-thermal.yaml | 2 +
arch/arm64/boot/dts/mediatek/mt8186.dtsi | 20 ++++++
drivers/thermal/mediatek/lvts_thermal.c | 67 +++++++++++++++++++
.../thermal/mediatek,lvts-thermal.h | 10 +++
4 files changed, 99 insertions(+)
diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
index e6665af52e..4173bae530 100644
--- a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
@@ -19,6 +19,7 @@ properties:
compatible:
enum:
- mediatek,mt7988-lvts-ap
+ - mediatek,mt8186-lvts
- mediatek,mt8192-lvts-ap
- mediatek,mt8192-lvts-mcu
- mediatek,mt8195-lvts-ap
@@ -75,6 +76,7 @@ allOf:
compatible:
contains:
enum:
+ - mediatek,mt8186-lvts
- mediatek,mt8195-lvts-ap
- mediatek,mt8195-lvts-mcu
then:
diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
index df0c04f2ba..8fc563dce6 100644
--- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
@@ -1355,6 +1355,18 @@ spi0: spi@1100a000 {
status = "disabled";
};
+ lvts: lvts@1100b000 {
+ compatible = "mediatek,mt8186-lvts";
+ #thermal-sensor-cells = <1>;
+ reg = <0 0x1100b000 0 0x1000>;
+ interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
+ clock-names = "lvts_clk";
+ resets = <&infracfg_ao MT8186_INFRA_THERMAL_CTRL_RST>;
+ nvmem-cells = <&lvts_e_data1 &lvts_e_data2>;
+ nvmem-cell-names = "e_data1","e_data2";
+ };
+
pwm0: pwm@1100e000 {
compatible = "mediatek,mt8186-disp-pwm", "mediatek,mt8183-disp-pwm";
reg = <0 0x1100e000 0 0x1000>;
@@ -1668,6 +1680,14 @@ efuse: efuse@11cb0000 {
#address-cells = <1>;
#size-cells = <1>;
+ lvts_e_data1: data1 {
+ reg = <0x1cc 0x14>;
+ };
+
+ lvts_e_data2: data1-1 {
+ reg = <0x2f8 0x14>;
+ };
+
gpu_speedbin: gpu-speedbin@59c {
reg = <0x59c 0x4>;
bits = <0 3>;
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index ed1888fb24..e923d22c17 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -80,6 +80,8 @@
#define LVTS_SENSOR_MAX 4
#define LVTS_GOLDEN_TEMP_MAX 62
#define LVTS_GOLDEN_TEMP_DEFAULT 50
+#define LVTS_COEFF_A_MT8186 -204650
+#define LVTS_COEFF_B_MT8186 204650
#define LVTS_COEFF_A_MT8195 -250460
#define LVTS_COEFF_B_MT8195 250460
#define LVTS_COEFF_A_MT7988 -204650
@@ -92,6 +94,7 @@
#define LVTS_MSR_READ_WAIT_US (LVTS_MSR_READ_TIMEOUT_US / 2)
#define LVTS_HW_SHUTDOWN_MT7988 105000
+#define LVTS_HW_SHUTDOWN_MT8186 105000
#define LVTS_HW_SHUTDOWN_MT8192 105000
#define LVTS_HW_SHUTDOWN_MT8195 105000
@@ -1377,6 +1380,62 @@ static int lvts_resume(struct device *dev)
return 0;
}
+/*
+ * The MT8186 calibration data is stored as packed 3-byte little-endian
+ * values using a weird layout that makes sense only when viewed as a 32-bit
+ * hexadecimal word dump. Let's suppose SxBy where x = sensor number and
+ * y = byte number where the LSB is y=0. We then have:
+ *
+ * [S0B2-S0B1-S0B0-S1B2] [S1B1-S1B0-S2B2-S2B1] [S2B0-S3B2-S3B1-S3B0]
+ *
+ * However, when considering a byte stream, those appear as follows:
+ *
+ * [S1B2] [S0B0[ [S0B1] [S0B2] [S2B1] [S2B2] [S1B0] [S1B1] [S3B0] [S3B1] [S3B2] [S2B0]
+ *
+ * Hence the rather confusing offsets provided below.
+ */
+static const struct lvts_ctrl_data mt8186_lvts_data_ctrl[] = {
+ {
+ .lvts_sensor = {
+ { .dt_id = MT8186_TS1_0,
+ .cal_offsets = { 5, 6, 7 } },
+ { .dt_id = MT8186_TS1_1,
+ .cal_offsets = { 10, 11, 4 } },
+ { .dt_id = MT8186_TS1_2,
+ .cal_offsets = { 15, 8, 9 } },
+ { .dt_id = MT8186_TS1_3,
+ .cal_offsets = { 12, 13, 14 } }
+ },
+ .num_lvts_sensor = 4,
+ .offset = 0x0,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8186,
+ },
+ {
+ .lvts_sensor = {
+ { .dt_id = MT8186_TS2_0,
+ .cal_offsets = { 22, 23, 16 } },
+ { .dt_id = MT8186_TS2_1,
+ .cal_offsets = { 27, 20, 21 } }
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x100,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8186,
+ },
+ {
+ .lvts_sensor = {
+ { .dt_id = MT8186_TS3_0,
+ .cal_offsets = { 29, 30, 31 } },
+ { .dt_id = MT8186_TS3_1,
+ .cal_offsets = { 34, 35, 28 } },
+ { .dt_id = MT8186_TS3_2,
+ .cal_offsets = { 39, 32, 33 } }
+ },
+ .num_lvts_sensor = 3,
+ .offset = 0x200,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8186,
+ }
+};
+
static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
{
.lvts_sensor = {
@@ -1565,6 +1624,13 @@ static const struct lvts_data mt7988_lvts_ap_data = {
.temp_offset = LVTS_COEFF_B_MT7988,
};
+static const struct lvts_data mt8186_lvts_data = {
+ .lvts_ctrl = mt8186_lvts_data_ctrl,
+ .num_lvts_ctrl = ARRAY_SIZE(mt8186_lvts_data_ctrl),
+ .temp_factor = LVTS_COEFF_A_MT8186,
+ .temp_offset = LVTS_COEFF_B_MT8186,
+};
+
static const struct lvts_data mt8192_lvts_mcu_data = {
.lvts_ctrl = mt8192_lvts_mcu_data_ctrl,
.num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_mcu_data_ctrl),
@@ -1591,6 +1657,7 @@ static const struct lvts_data mt8195_lvts_ap_data = {
static const struct of_device_id lvts_of_match[] = {
{ .compatible = "mediatek,mt7988-lvts-ap", .data = &mt7988_lvts_ap_data },
+ { .compatible = "mediatek,mt8186-lvts", .data = &mt8186_lvts_data },
{ .compatible = "mediatek,mt8192-lvts-mcu", .data = &mt8192_lvts_mcu_data },
{ .compatible = "mediatek,mt8192-lvts-ap", .data = &mt8192_lvts_ap_data },
{ .compatible = "mediatek,mt8195-lvts-mcu", .data = &mt8195_lvts_mcu_data },
diff --git a/include/dt-bindings/thermal/mediatek,lvts-thermal.h b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
index 997e2f5512..3197ca6087 100644
--- a/include/dt-bindings/thermal/mediatek,lvts-thermal.h
+++ b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
@@ -16,6 +16,16 @@
#define MT7988_ETHWARP_0 6
#define MT7988_ETHWARP_1 7
+#define MT8186_TS1_0 0
+#define MT8186_TS1_1 1
+#define MT8186_TS1_2 2
+#define MT8186_TS1_3 3
+#define MT8186_TS2_0 4
+#define MT8186_TS2_1 5
+#define MT8186_TS3_0 6
+#define MT8186_TS3_1 7
+#define MT8186_TS3_2 8
+
#define MT8195_MCU_BIG_CPU0 0
#define MT8195_MCU_BIG_CPU1 1
#define MT8195_MCU_BIG_CPU2 2
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 6/9] arm64: dts: mediatek: mt8186: add default thermal zones
2024-01-11 22:29 [PATCH 0/9] Mediatek thermal sensor driver support for MT8186 and MT8188 Nicolas Pitre
` (4 preceding siblings ...)
2024-01-11 22:30 ` [PATCH 5/9] thermal/drivers/mediatek/lvts_thermal: add MT8186 support Nicolas Pitre
@ 2024-01-11 22:30 ` Nicolas Pitre
2024-01-12 11:21 ` Krzysztof Kozlowski
2024-01-11 22:30 ` [PATCH 7/9] thermal/drivers/mediatek/lvts_thermal: provision for gt variable location Nicolas Pitre
` (2 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-11 22:30 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
From: Nicolas Pitre <npitre@baylibre.com>
Inspired by the vendor kernel but adapted to the upstream thermal
driver version.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
arch/arm64/boot/dts/mediatek/mt8186.dtsi | 236 +++++++++++++++++++++++
1 file changed, 236 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
index 8fc563dce6..91b902a9f0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
@@ -13,6 +13,8 @@
#include <dt-bindings/power/mt8186-power.h>
#include <dt-bindings/phy/phy.h>
#include <dt-bindings/reset/mt8186-resets.h>
+#include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/thermal/mediatek,lvts-thermal.h>
/ {
compatible = "mediatek,mt8186";
@@ -2115,4 +2117,238 @@ larb19: smi@1c10f000 {
power-domains = <&spm MT8186_POWER_DOMAIN_IPE>;
};
};
+
+ thermal_zones: thermal-zones {
+ cpu_zone0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_TS1_0>;
+
+ trips {
+ cpu_zone0_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_zone0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_zone0_alert>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu_zone1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_TS1_1>;
+
+ trips {
+ cpu_zone1_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_zone1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_zone1_alert>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu_zone2-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_TS1_2>;
+
+ trips {
+ cpu_zone2_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_zone2_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_zone2_alert>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cam-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_TS1_3>;
+
+ trips {
+ cam_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cam_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ nna-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_TS3_0>;
+
+ trips {
+ nna_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ nna_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ adsp-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_TS3_1>;
+
+ trips {
+ adsp_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ adsp_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ mfg-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_TS3_2>;
+
+ trips {
+ mfg_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ mfg_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ cpu_big0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_TS2_0>;
+
+ trips {
+ cpu_big0_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_big0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_big0_alert>;
+ cooling-device = <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu_big1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_TS2_1>;
+
+ trips {
+ cpu_big1_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_big1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_big1_alert>;
+ cooling-device = <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+ };
};
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 6/9] arm64: dts: mediatek: mt8186: add default thermal zones
2024-01-11 22:30 ` [PATCH 6/9] arm64: dts: mediatek: mt8186: add default thermal zones Nicolas Pitre
@ 2024-01-12 11:21 ` Krzysztof Kozlowski
2024-01-12 16:52 ` Nicolas Pitre
2024-01-15 17:46 ` Nicolas Pitre
0 siblings, 2 replies; 21+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-12 11:21 UTC (permalink / raw)
To: Nicolas Pitre, Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
On 11/01/2024 23:30, Nicolas Pitre wrote:
> From: Nicolas Pitre <npitre@baylibre.com>
>
> Inspired by the vendor kernel but adapted to the upstream thermal
> driver version.
DTS should not go via PM tree, so you need to Cc Mediatek SoC
maintainers. b4 would do this automatically. Other way is using
get_maintainers.pl.
>
> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> ---
> arch/arm64/boot/dts/mediatek/mt8186.dtsi | 236 +++++++++++++++++++++++
> 1 file changed, 236 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> index 8fc563dce6..91b902a9f0 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> @@ -13,6 +13,8 @@
> #include <dt-bindings/power/mt8186-power.h>
> #include <dt-bindings/phy/phy.h>
> #include <dt-bindings/reset/mt8186-resets.h>
> +#include <dt-bindings/thermal/thermal.h>
> +#include <dt-bindings/thermal/mediatek,lvts-thermal.h>
>
> / {
> compatible = "mediatek,mt8186";
> @@ -2115,4 +2117,238 @@ larb19: smi@1c10f000 {
> power-domains = <&spm MT8186_POWER_DOMAIN_IPE>;
> };
> };
> +
> + thermal_zones: thermal-zones {
> + cpu_zone0-thermal {
No underscores in node names. Could one CPU have multiple names? If not,
then it is just "cpu0-thermal".
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 6/9] arm64: dts: mediatek: mt8186: add default thermal zones
2024-01-12 11:21 ` Krzysztof Kozlowski
@ 2024-01-12 16:52 ` Nicolas Pitre
2024-01-15 17:46 ` Nicolas Pitre
1 sibling, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-12 16:52 UTC (permalink / raw)
To: Krzysztof Kozlowski; +Cc: Daniel Lezcano, linux-pm
On Fri, 12 Jan 2024, Krzysztof Kozlowski wrote:
> On 11/01/2024 23:30, Nicolas Pitre wrote:
> > From: Nicolas Pitre <npitre@baylibre.com>
> >
> > Inspired by the vendor kernel but adapted to the upstream thermal
> > driver version.
>
> DTS should not go via PM tree, so you need to Cc Mediatek SoC
> maintainers. b4 would do this automatically. Other way is using
> get_maintainers.pl.
Sure. But I'd prefer to have reviews making sure those patches are sane
enough before spamming more people with this.
Nicolas
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 6/9] arm64: dts: mediatek: mt8186: add default thermal zones
2024-01-12 11:21 ` Krzysztof Kozlowski
2024-01-12 16:52 ` Nicolas Pitre
@ 2024-01-15 17:46 ` Nicolas Pitre
2024-01-15 17:52 ` Krzysztof Kozlowski
1 sibling, 1 reply; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-15 17:46 UTC (permalink / raw)
To: Krzysztof Kozlowski; +Cc: Daniel Lezcano, linux-pm
On Fri, 12 Jan 2024, Krzysztof Kozlowski wrote:
> On 11/01/2024 23:30, Nicolas Pitre wrote:
>
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> > index 8fc563dce6..91b902a9f0 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> > @@ -2115,4 +2117,238 @@ larb19: smi@1c10f000 {
> > power-domains = <&spm MT8186_POWER_DOMAIN_IPE>;
> > };
> > };
> > +
> > + thermal_zones: thermal-zones {
> > + cpu_zone0-thermal {
>
> No underscores in node names. Could one CPU have multiple names? If not,
> then it is just "cpu0-thermal".
Well... I'm not completely clear about this given the available info,
but this thermal zone would not be matching a single CPU but a few of
them, hence several "zones of CPUs".
Nicolas
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 6/9] arm64: dts: mediatek: mt8186: add default thermal zones
2024-01-15 17:46 ` Nicolas Pitre
@ 2024-01-15 17:52 ` Krzysztof Kozlowski
2024-01-19 17:04 ` Daniel Lezcano
0 siblings, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-15 17:52 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Daniel Lezcano, linux-pm
On 15/01/2024 18:46, Nicolas Pitre wrote:
>>> @@ -2115,4 +2117,238 @@ larb19: smi@1c10f000 {
>>> power-domains = <&spm MT8186_POWER_DOMAIN_IPE>;
>>> };
>>> };
>>> +
>>> + thermal_zones: thermal-zones {
>>> + cpu_zone0-thermal {
>>
>> No underscores in node names. Could one CPU have multiple names? If not,
>> then it is just "cpu0-thermal".
>
> Well... I'm not completely clear about this given the available info,
> but this thermal zone would not be matching a single CPU but a few of
> them, hence several "zones of CPUs".
OK, then just: cpu-zone0-thermal
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 6/9] arm64: dts: mediatek: mt8186: add default thermal zones
2024-01-15 17:52 ` Krzysztof Kozlowski
@ 2024-01-19 17:04 ` Daniel Lezcano
0 siblings, 0 replies; 21+ messages in thread
From: Daniel Lezcano @ 2024-01-19 17:04 UTC (permalink / raw)
To: Krzysztof Kozlowski, Nicolas Pitre; +Cc: linux-pm
On 15/01/2024 18:52, Krzysztof Kozlowski wrote:
> On 15/01/2024 18:46, Nicolas Pitre wrote:
>>>> @@ -2115,4 +2117,238 @@ larb19: smi@1c10f000 {
>>>> power-domains = <&spm MT8186_POWER_DOMAIN_IPE>;
>>>> };
>>>> };
>>>> +
>>>> + thermal_zones: thermal-zones {
>>>> + cpu_zone0-thermal {
>>>
>>> No underscores in node names. Could one CPU have multiple names? If not,
>>> then it is just "cpu0-thermal".
>>
>> Well... I'm not completely clear about this given the available info,
>> but this thermal zone would not be matching a single CPU but a few of
>> them, hence several "zones of CPUs".
>
> OK, then just: cpu-zone0-thermal
Given the node name is used as the thermal zone name. The 'zone' is
duplicate in the namespace.
/sys/class/thermal/thermal_zone0/type = cpu-zone0-thermal
Furthermore, if other thermal zones are registered before, that could be:
/sys/class/thermal/thermal_zone4/type = cpu-zone0-thermal
Kind of inconsistent :/
If it is a group of CPUS, they are probably belonging to the same
performance domain, may be we can use the name 'cluster' here:
cluster0-thermal
--
<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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 7/9] thermal/drivers/mediatek/lvts_thermal: provision for gt variable location
2024-01-11 22:29 [PATCH 0/9] Mediatek thermal sensor driver support for MT8186 and MT8188 Nicolas Pitre
` (5 preceding siblings ...)
2024-01-11 22:30 ` [PATCH 6/9] arm64: dts: mediatek: mt8186: add default thermal zones Nicolas Pitre
@ 2024-01-11 22:30 ` Nicolas Pitre
2024-01-11 22:30 ` [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots Nicolas Pitre
2024-01-11 22:30 ` [PATCH 9/9] thermal/drivers/mediatek/lvts_thermal: add MT8188 support Nicolas Pitre
8 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-11 22:30 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
From: Nicolas Pitre <npitre@baylibre.com>
The golden temperature calibration value in nvram is not always the
3rd byte. A future commit will prove this assumption wrong.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
drivers/thermal/mediatek/lvts_thermal.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index e923d22c17..b20b70fd36 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -121,6 +121,7 @@ struct lvts_data {
int num_lvts_ctrl;
int temp_factor;
int temp_offset;
+ int gt_calib_bit_offset;
};
struct lvts_sensor {
@@ -748,20 +749,21 @@ static int lvts_calibration_read(struct device *dev, struct lvts_domain *lvts_td
return 0;
}
-static int lvts_golden_temp_init(struct device *dev, u8 *calib, int temp_offset)
+static int lvts_golden_temp_init(struct device *dev, u8 *calib,
+ const struct lvts_data *lvts_data)
{
u32 gt;
/*
- * The golden temp information is contained in the 4th byte (index = 3)
- * of efuse data.
+ * The golden temp information is contained in the first 32-bit
+ * word of efuse data at a specific bit offset.
*/
- gt = calib[3];
+ gt = (((u32 *)calib)[0] >> lvts_data->gt_calib_bit_offset) & 0xff;
if (gt && gt < LVTS_GOLDEN_TEMP_MAX)
golden_temp = gt;
- golden_temp_offset = golden_temp * 500 + temp_offset;
+ golden_temp_offset = golden_temp * 500 + lvts_data->temp_offset;
return 0;
}
@@ -780,7 +782,7 @@ static int lvts_ctrl_init(struct device *dev, struct lvts_domain *lvts_td,
if (ret)
return ret;
- ret = lvts_golden_temp_init(dev, lvts_td->calib, lvts_data->temp_offset);
+ ret = lvts_golden_temp_init(dev, lvts_td->calib, lvts_data);
if (ret)
return ret;
@@ -1622,6 +1624,7 @@ static const struct lvts_data mt7988_lvts_ap_data = {
.num_lvts_ctrl = ARRAY_SIZE(mt7988_lvts_ap_data_ctrl),
.temp_factor = LVTS_COEFF_A_MT7988,
.temp_offset = LVTS_COEFF_B_MT7988,
+ .gt_calib_bit_offset = 24,
};
static const struct lvts_data mt8186_lvts_data = {
@@ -1629,16 +1632,19 @@ static const struct lvts_data mt8186_lvts_data = {
.num_lvts_ctrl = ARRAY_SIZE(mt8186_lvts_data_ctrl),
.temp_factor = LVTS_COEFF_A_MT8186,
.temp_offset = LVTS_COEFF_B_MT8186,
+ .gt_calib_bit_offset = 24,
};
static const struct lvts_data mt8192_lvts_mcu_data = {
.lvts_ctrl = mt8192_lvts_mcu_data_ctrl,
.num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_mcu_data_ctrl),
+ .gt_calib_bit_offset = 24,
};
static const struct lvts_data mt8192_lvts_ap_data = {
.lvts_ctrl = mt8192_lvts_ap_data_ctrl,
.num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_ap_data_ctrl),
+ .gt_calib_bit_offset = 24,
};
static const struct lvts_data mt8195_lvts_mcu_data = {
@@ -1646,6 +1652,7 @@ static const struct lvts_data mt8195_lvts_mcu_data = {
.num_lvts_ctrl = ARRAY_SIZE(mt8195_lvts_mcu_data_ctrl),
.temp_factor = LVTS_COEFF_A_MT8195,
.temp_offset = LVTS_COEFF_B_MT8195,
+ .gt_calib_bit_offset = 24,
};
static const struct lvts_data mt8195_lvts_ap_data = {
@@ -1653,6 +1660,7 @@ static const struct lvts_data mt8195_lvts_ap_data = {
.num_lvts_ctrl = ARRAY_SIZE(mt8195_lvts_ap_data_ctrl),
.temp_factor = LVTS_COEFF_A_MT8195,
.temp_offset = LVTS_COEFF_B_MT8195,
+ .gt_calib_bit_offset = 24,
};
static const struct of_device_id lvts_of_match[] = {
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots
2024-01-11 22:29 [PATCH 0/9] Mediatek thermal sensor driver support for MT8186 and MT8188 Nicolas Pitre
` (6 preceding siblings ...)
2024-01-11 22:30 ` [PATCH 7/9] thermal/drivers/mediatek/lvts_thermal: provision for gt variable location Nicolas Pitre
@ 2024-01-11 22:30 ` Nicolas Pitre
2024-01-19 16:29 ` Daniel Lezcano
2024-01-11 22:30 ` [PATCH 9/9] thermal/drivers/mediatek/lvts_thermal: add MT8188 support Nicolas Pitre
8 siblings, 1 reply; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-11 22:30 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
From: Nicolas Pitre <npitre@baylibre.com>
Some systems don't always populate sensor controller slots starting
at slot 0.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
drivers/thermal/mediatek/lvts_thermal.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index b20b70fd36..473ef91ea3 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -112,6 +112,7 @@ struct lvts_ctrl_data {
struct lvts_sensor_data lvts_sensor[LVTS_SENSOR_MAX];
int hw_tshut_temp;
int num_lvts_sensor;
+ int skipped_sensors;
int offset;
int mode;
};
@@ -555,6 +556,7 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
const struct lvts_ctrl_data *lvts_ctrl_data)
{
struct lvts_sensor *lvts_sensor = lvts_ctrl->sensors;
+
void __iomem *msr_regs[] = {
LVTS_MSR0(lvts_ctrl->base),
LVTS_MSR1(lvts_ctrl->base),
@@ -569,7 +571,7 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
LVTS_IMMD3(lvts_ctrl->base)
};
- int i;
+ int i, skip;
for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++) {
@@ -604,8 +606,9 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
/*
* Each sensor has its own register address to read from.
*/
+ skip = lvts_ctrl_data->skipped_sensors;
lvts_sensor[i].msr = lvts_ctrl_data->mode == LVTS_MSR_IMMEDIATE_MODE ?
- imm_regs[i] : msr_regs[i];
+ imm_regs[i + skip] : msr_regs[i + skip];
lvts_sensor[i].low_thresh = INT_MIN;
lvts_sensor[i].high_thresh = INT_MIN;
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots
2024-01-11 22:30 ` [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots Nicolas Pitre
@ 2024-01-19 16:29 ` Daniel Lezcano
2024-01-19 16:53 ` Nicolas Pitre
0 siblings, 1 reply; 21+ messages in thread
From: Daniel Lezcano @ 2024-01-19 16:29 UTC (permalink / raw)
To: Nicolas Pitre, linux-pm; +Cc: Nicolas Pitre
Hi Nico,
On 11/01/2024 23:30, Nicolas Pitre wrote:
> From: Nicolas Pitre <npitre@baylibre.com>
>
> Some systems don't always populate sensor controller slots starting
> at slot 0.
>
> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> ---
> drivers/thermal/mediatek/lvts_thermal.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index b20b70fd36..473ef91ea3 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -112,6 +112,7 @@ struct lvts_ctrl_data {
> struct lvts_sensor_data lvts_sensor[LVTS_SENSOR_MAX];
> int hw_tshut_temp;
> int num_lvts_sensor;
> + int skipped_sensors;
> int offset;
> int mode;
> };
> @@ -555,6 +556,7 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
> const struct lvts_ctrl_data *lvts_ctrl_data)
> {
> struct lvts_sensor *lvts_sensor = lvts_ctrl->sensors;
> +
> void __iomem *msr_regs[] = {
> LVTS_MSR0(lvts_ctrl->base),
> LVTS_MSR1(lvts_ctrl->base),
> @@ -569,7 +571,7 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
> LVTS_IMMD3(lvts_ctrl->base)
> };
>
> - int i;
> + int i, skip;
>
> for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++) {
>
> @@ -604,8 +606,9 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
> /*
> * Each sensor has its own register address to read from.
> */
> + skip = lvts_ctrl_data->skipped_sensors;
> lvts_sensor[i].msr = lvts_ctrl_data->mode == LVTS_MSR_IMMEDIATE_MODE ?
> - imm_regs[i] : msr_regs[i];
> + imm_regs[i + skip] : msr_regs[i + skip];
Overall the series look ok but this changes is hard to understand.
Could you propose a different approach to have the resulting code easier
to understand ?
> lvts_sensor[i].low_thresh = INT_MIN;
> lvts_sensor[i].high_thresh = INT_MIN;
--
<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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots
2024-01-19 16:29 ` Daniel Lezcano
@ 2024-01-19 16:53 ` Nicolas Pitre
2024-01-22 11:55 ` Daniel Lezcano
0 siblings, 1 reply; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-19 16:53 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-pm
On Fri, 19 Jan 2024, Daniel Lezcano wrote:
>
> Hi Nico,
>
> On 11/01/2024 23:30, Nicolas Pitre wrote:
> > From: Nicolas Pitre <npitre@baylibre.com>
> >
> > Some systems don't always populate sensor controller slots starting
> > at slot 0.
> >
> > Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> > ---
> > drivers/thermal/mediatek/lvts_thermal.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/mediatek/lvts_thermal.c
> > b/drivers/thermal/mediatek/lvts_thermal.c
> > index b20b70fd36..473ef91ea3 100644
> > --- a/drivers/thermal/mediatek/lvts_thermal.c
> > +++ b/drivers/thermal/mediatek/lvts_thermal.c
> > @@ -112,6 +112,7 @@ struct lvts_ctrl_data {
> > struct lvts_sensor_data lvts_sensor[LVTS_SENSOR_MAX];
> > int hw_tshut_temp;
> > int num_lvts_sensor;
> > + int skipped_sensors;
> > int offset;
> > int mode;
> > };
> > @@ -555,6 +556,7 @@ static int lvts_sensor_init(struct device *dev, struct
> > @@ lvts_ctrl *lvts_ctrl,
> > const struct lvts_ctrl_data
> > *lvts_ctrl_data)
> > {
> > struct lvts_sensor *lvts_sensor = lvts_ctrl->sensors;
> > +
> > void __iomem *msr_regs[] = {
> > LVTS_MSR0(lvts_ctrl->base),
> > LVTS_MSR1(lvts_ctrl->base),
> > @@ -569,7 +571,7 @@ static int lvts_sensor_init(struct device *dev, struct
> > @@ lvts_ctrl *lvts_ctrl,
> > LVTS_IMMD3(lvts_ctrl->base)
> > };
> > - int i;
> > + int i, skip;
> >
> > for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++) {
> >
> > @@ -604,8 +606,9 @@ static int lvts_sensor_init(struct device *dev, struct
> > @@ lvts_ctrl *lvts_ctrl,
> > /*
> > * Each sensor has its own register address to read from.
> > */
> > + skip = lvts_ctrl_data->skipped_sensors;
> > lvts_sensor[i].msr = lvts_ctrl_data->mode ==
> > LVTS_MSR_IMMEDIATE_MODE ?
> > - imm_regs[i] : msr_regs[i];
> > + imm_regs[i + skip] : msr_regs[i + skip];
>
> Overall the series look ok but this changes is hard to understand.
>
> Could you propose a different approach to have the resulting code easier to
> understand ?
I'm not sure how I could make it simpler. Maybe a comment is in order
though?
The sensor controller has 4 slots. Those slots are accessible either
through imm_regs[<slot_number>] oe msr_regs[<slot_number>]. If, say,
slot 0 is unpopulated then sensor 0 (i = 0) needs to address slot 1 (i =
0, skip = 1), sensor 1 is in slot 2 (i = 1, skip = 1), etc. Does this
make sense?
Nicolas
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots
2024-01-19 16:53 ` Nicolas Pitre
@ 2024-01-22 11:55 ` Daniel Lezcano
2024-01-22 15:23 ` Nicolas Pitre
0 siblings, 1 reply; 21+ messages in thread
From: Daniel Lezcano @ 2024-01-22 11:55 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: linux-pm
Hi Nico,
On 19/01/2024 17:53, Nicolas Pitre wrote:
[ ... ]
>>> + skip = lvts_ctrl_data->skipped_sensors;
>>> lvts_sensor[i].msr = lvts_ctrl_data->mode ==
>>> LVTS_MSR_IMMEDIATE_MODE ?
>>> - imm_regs[i] : msr_regs[i];
>>> + imm_regs[i + skip] : msr_regs[i + skip];
>>
>> Overall the series look ok but this changes is hard to understand.
>>
>> Could you propose a different approach to have the resulting code easier to
>> understand ?
>
> I'm not sure how I could make it simpler. Maybe a comment is in order
> though?
>
> The sensor controller has 4 slots. Those slots are accessible either
> through imm_regs[<slot_number>] oe msr_regs[<slot_number>]. If, say,
> slot 0 is unpopulated then sensor 0 (i = 0) needs to address slot 1 (i =
> 0, skip = 1), sensor 1 is in slot 2 (i = 1, skip = 1), etc. Does this
> make sense?
Why not keep the sensor id = slot id and declare the ones which are
disabled with a mask?
--
<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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots
2024-01-22 11:55 ` Daniel Lezcano
@ 2024-01-22 15:23 ` Nicolas Pitre
2024-01-22 16:03 ` Daniel Lezcano
0 siblings, 1 reply; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-22 15:23 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-pm
On Mon, 22 Jan 2024, Daniel Lezcano wrote:
>
> Hi Nico,
>
> On 19/01/2024 17:53, Nicolas Pitre wrote:
>
> [ ... ]
>
> >>> + skip = lvts_ctrl_data->skipped_sensors;
> >>> lvts_sensor[i].msr = lvts_ctrl_data->mode ==
> >>> LVTS_MSR_IMMEDIATE_MODE ?
> >>> - imm_regs[i] : msr_regs[i];
> >>> + imm_regs[i + skip] : msr_regs[i + skip];
> >>
> >> Overall the series look ok but this changes is hard to understand.
> >>
> >> Could you propose a different approach to have the resulting code easier to
> >> understand ?
> >
> > I'm not sure how I could make it simpler. Maybe a comment is in order
> > though?
> >
> > The sensor controller has 4 slots. Those slots are accessible either
> > through imm_regs[<slot_number>] oe msr_regs[<slot_number>]. If, say,
> > slot 0 is unpopulated then sensor 0 (i = 0) needs to address slot 1 (i =
> > 0, skip = 1), sensor 1 is in slot 2 (i = 1, skip = 1), etc. Does this
> > make sense?
>
> Why not keep the sensor id = slot id and declare the ones which are disabled
> with a mask?
Then what do you do with the empty sensor 0? Do we want to present dead
sensor IDs to users?
Nicolas
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots
2024-01-22 15:23 ` Nicolas Pitre
@ 2024-01-22 16:03 ` Daniel Lezcano
2024-01-22 16:13 ` Nicolas Pitre
0 siblings, 1 reply; 21+ messages in thread
From: Daniel Lezcano @ 2024-01-22 16:03 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: linux-pm
On 22/01/2024 16:23, Nicolas Pitre wrote:
> On Mon, 22 Jan 2024, Daniel Lezcano wrote:
>
>>
>> Hi Nico,
>>
>> On 19/01/2024 17:53, Nicolas Pitre wrote:
>>
>> [ ... ]
>>
>>>>> + skip = lvts_ctrl_data->skipped_sensors;
>>>>> lvts_sensor[i].msr = lvts_ctrl_data->mode ==
>>>>> LVTS_MSR_IMMEDIATE_MODE ?
>>>>> - imm_regs[i] : msr_regs[i];
>>>>> + imm_regs[i + skip] : msr_regs[i + skip];
>>>>
>>>> Overall the series look ok but this changes is hard to understand.
>>>>
>>>> Could you propose a different approach to have the resulting code easier to
>>>> understand ?
>>>
>>> I'm not sure how I could make it simpler. Maybe a comment is in order
>>> though?
>>>
>>> The sensor controller has 4 slots. Those slots are accessible either
>>> through imm_regs[<slot_number>] oe msr_regs[<slot_number>]. If, say,
>>> slot 0 is unpopulated then sensor 0 (i = 0) needs to address slot 1 (i =
>>> 0, skip = 1), sensor 1 is in slot 2 (i = 1, skip = 1), etc. Does this
>>> make sense?
>>
>> Why not keep the sensor id = slot id and declare the ones which are disabled
>> with a mask?
>
> Then what do you do with the empty sensor 0? Do we want to present dead
> sensor IDs to users?
You can skip it somehow in lvts_ctrl_start(), right?
--
<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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots
2024-01-22 16:03 ` Daniel Lezcano
@ 2024-01-22 16:13 ` Nicolas Pitre
0 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-22 16:13 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-pm
On Mon, 22 Jan 2024, Daniel Lezcano wrote:
> On 22/01/2024 16:23, Nicolas Pitre wrote:
> > On Mon, 22 Jan 2024, Daniel Lezcano wrote:
> >
> >>
> >> Hi Nico,
> >>
> >> On 19/01/2024 17:53, Nicolas Pitre wrote:
> >>
> >> [ ... ]
> >>
> >>>>> + skip = lvts_ctrl_data->skipped_sensors;
> >>>>> lvts_sensor[i].msr = lvts_ctrl_data->mode ==
> >>>>> LVTS_MSR_IMMEDIATE_MODE ?
> >>>>> - imm_regs[i] : msr_regs[i];
> >>>>> + imm_regs[i + skip] : msr_regs[i + skip];
> >>>>
> >>>> Overall the series look ok but this changes is hard to understand.
> >>>>
> >>>> Could you propose a different approach to have the resulting code easier
> >>>> to
> >>>> understand ?
> >>>
> >>> I'm not sure how I could make it simpler. Maybe a comment is in order
> >>> though?
> >>>
> >>> The sensor controller has 4 slots. Those slots are accessible either
> >>> through imm_regs[<slot_number>] oe msr_regs[<slot_number>]. If, say,
> >>> slot 0 is unpopulated then sensor 0 (i = 0) needs to address slot 1 (i =
> >>> 0, skip = 1), sensor 1 is in slot 2 (i = 1, skip = 1), etc. Does this
> >>> make sense?
> >>
> >> Why not keep the sensor id = slot id and declare the ones which are
> >> disabled
> >> with a mask?
> >
> > Then what do you do with the empty sensor 0? Do we want to present dead
> > sensor IDs to users?
>
> You can skip it somehow in lvts_ctrl_start(), right?
I see what you mean. Indeed that would be better.
Nicolas
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 9/9] thermal/drivers/mediatek/lvts_thermal: add MT8188 support
2024-01-11 22:29 [PATCH 0/9] Mediatek thermal sensor driver support for MT8186 and MT8188 Nicolas Pitre
` (7 preceding siblings ...)
2024-01-11 22:30 ` [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots Nicolas Pitre
@ 2024-01-11 22:30 ` Nicolas Pitre
8 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2024-01-11 22:30 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: Nicolas Pitre
From: Nicolas Pitre <npitre@baylibre.com>
Various values extracted from the vendor's kernel driver.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
.../thermal/mediatek,lvts-thermal.yaml | 4 +
drivers/thermal/mediatek/lvts_thermal.c | 101 ++++++++++++++++++
.../thermal/mediatek,lvts-thermal.h | 16 +++
3 files changed, 121 insertions(+)
diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
index 4173bae530..331cf4e662 100644
--- a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
@@ -20,6 +20,8 @@ properties:
enum:
- mediatek,mt7988-lvts-ap
- mediatek,mt8186-lvts
+ - mediatek,mt8188-lvts-ap
+ - mediatek,mt8188-lvts-mcu
- mediatek,mt8192-lvts-ap
- mediatek,mt8192-lvts-mcu
- mediatek,mt8195-lvts-ap
@@ -61,6 +63,8 @@ allOf:
compatible:
contains:
enum:
+ - mediatek,mt8188-lvts-ap
+ - mediatek,mt8188-lvts-mcu
- mediatek,mt8192-lvts-ap
- mediatek,mt8192-lvts-mcu
then:
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 473ef91ea3..8942b50d84 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -1441,6 +1441,89 @@ static const struct lvts_ctrl_data mt8186_lvts_data_ctrl[] = {
}
};
+static const struct lvts_ctrl_data mt8188_lvts_mcu_data_ctrl[] = {
+ {
+ .lvts_sensor = {
+ { .dt_id = MT8188_MCU_TS1_0,
+ .cal_offsets = { 22, 23, 24 } },
+ { .dt_id = MT8188_MCU_TS1_1,
+ .cal_offsets = { 25, 26, 27 } },
+ { .dt_id = MT8188_MCU_TS1_2,
+ .cal_offsets = { 28, 29, 30 } },
+ { .dt_id = MT8188_MCU_TS1_3,
+ .cal_offsets = { 31, 32, 33 } },
+ },
+ .num_lvts_sensor = 4,
+ .offset = 0x0,
+ .hw_tshut_temp = 117000,
+ .mode = LVTS_MSR_FILTERED_MODE,
+ },
+ {
+ .lvts_sensor = {
+ { .dt_id = MT8188_MCU_TS2_0,
+ .cal_offsets = { 34, 35, 36 } },
+ { .dt_id = MT8188_MCU_TS2_1,
+ .cal_offsets = { 37, 38, 39 } },
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x100,
+ .hw_tshut_temp = 117000,
+ .mode = LVTS_MSR_FILTERED_MODE,
+ }
+};
+
+static const struct lvts_ctrl_data mt8188_lvts_ap_data_ctrl[] = {
+ {
+ .lvts_sensor = {
+ { .dt_id = MT8188_AP_TS3_1,
+ .cal_offsets = { 40, 41, 42 } },
+ },
+ .num_lvts_sensor = 1,
+ .skipped_sensors = 1,
+ .offset = 0x0,
+ .hw_tshut_temp = 117000,
+ .mode = LVTS_MSR_FILTERED_MODE,
+ },
+ {
+ .lvts_sensor = {
+ { .dt_id = MT8188_AP_TS4_0,
+ .cal_offsets = { 43, 44, 45 } },
+ { .dt_id = MT8188_AP_TS4_1,
+ .cal_offsets = { 46, 47, 48 } },
+ { .dt_id = MT8188_AP_TS4_2,
+ .cal_offsets = { 49, 50, 51 } },
+ },
+ .num_lvts_sensor = 3,
+ .offset = 0x100,
+ .hw_tshut_temp = 117000,
+ .mode = LVTS_MSR_FILTERED_MODE,
+ },
+ {
+ .lvts_sensor = {
+ { .dt_id = MT8188_AP_TS5_0,
+ .cal_offsets = { 52, 53, 54 } },
+ { .dt_id = MT8188_AP_TS5_1,
+ .cal_offsets = { 55, 56, 57 } },
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x200,
+ .hw_tshut_temp = 117000,
+ .mode = LVTS_MSR_FILTERED_MODE,
+ },
+ {
+ .lvts_sensor = {
+ { .dt_id = MT8188_AP_TS6_0,
+ .cal_offsets = { 58, 59, 60 } },
+ { .dt_id = MT8188_AP_TS6_1,
+ .cal_offsets = { 61, 62, 63 } },
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x300,
+ .hw_tshut_temp = 117000,
+ .mode = LVTS_MSR_FILTERED_MODE,
+ }
+};
+
static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
{
.lvts_sensor = {
@@ -1638,6 +1721,22 @@ static const struct lvts_data mt8186_lvts_data = {
.gt_calib_bit_offset = 24,
};
+static const struct lvts_data mt8188_lvts_mcu_data = {
+ .lvts_ctrl = mt8188_lvts_mcu_data_ctrl,
+ .num_lvts_ctrl = ARRAY_SIZE(mt8188_lvts_mcu_data_ctrl),
+ .temp_factor = -250460,
+ .temp_offset = 250460,
+ .gt_calib_bit_offset = 20,
+};
+
+static const struct lvts_data mt8188_lvts_ap_data = {
+ .lvts_ctrl = mt8188_lvts_ap_data_ctrl,
+ .num_lvts_ctrl = ARRAY_SIZE(mt8188_lvts_ap_data_ctrl),
+ .temp_factor = -250460,
+ .temp_offset = 250460,
+ .gt_calib_bit_offset = 20,
+};
+
static const struct lvts_data mt8192_lvts_mcu_data = {
.lvts_ctrl = mt8192_lvts_mcu_data_ctrl,
.num_lvts_ctrl = ARRAY_SIZE(mt8192_lvts_mcu_data_ctrl),
@@ -1669,6 +1768,8 @@ static const struct lvts_data mt8195_lvts_ap_data = {
static const struct of_device_id lvts_of_match[] = {
{ .compatible = "mediatek,mt7988-lvts-ap", .data = &mt7988_lvts_ap_data },
{ .compatible = "mediatek,mt8186-lvts", .data = &mt8186_lvts_data },
+ { .compatible = "mediatek,mt8188-lvts-mcu", .data = &mt8188_lvts_mcu_data },
+ { .compatible = "mediatek,mt8188-lvts-ap", .data = &mt8188_lvts_ap_data },
{ .compatible = "mediatek,mt8192-lvts-mcu", .data = &mt8192_lvts_mcu_data },
{ .compatible = "mediatek,mt8192-lvts-ap", .data = &mt8192_lvts_ap_data },
{ .compatible = "mediatek,mt8195-lvts-mcu", .data = &mt8195_lvts_mcu_data },
diff --git a/include/dt-bindings/thermal/mediatek,lvts-thermal.h b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
index 3197ca6087..04fa9d7821 100644
--- a/include/dt-bindings/thermal/mediatek,lvts-thermal.h
+++ b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
@@ -26,6 +26,22 @@
#define MT8186_TS3_1 7
#define MT8186_TS3_2 8
+#define MT8188_MCU_TS1_0 0
+#define MT8188_MCU_TS1_1 1
+#define MT8188_MCU_TS1_2 2
+#define MT8188_MCU_TS1_3 3
+#define MT8188_MCU_TS2_0 4
+#define MT8188_MCU_TS2_1 5
+
+#define MT8188_AP_TS3_1 0
+#define MT8188_AP_TS4_0 1
+#define MT8188_AP_TS4_1 2
+#define MT8188_AP_TS4_2 3
+#define MT8188_AP_TS5_0 4
+#define MT8188_AP_TS5_1 5
+#define MT8188_AP_TS6_0 6
+#define MT8188_AP_TS6_1 7
+
#define MT8195_MCU_BIG_CPU0 0
#define MT8195_MCU_BIG_CPU1 1
#define MT8195_MCU_BIG_CPU2 2
--
2.42.0
^ permalink raw reply related [flat|nested] 21+ messages in thread