This is a bunch of patches to support the MT8186 and MT8188 thermal
sensor configurations.
Since the patches of v3 were applied except those related to the SoC
device trees, this series includes mainly patches for 'mt8186.dtsi'
and 'mt8188.dtsi'. Due to some thermal zone renaming in these 2 device
trees, the related definitions were also renamed in the dt-bindings and
in the driver.
[RFC] When PATCH 1/6 and 2/6 are squashed, checkpatch raises this WARNING:
"DT binding docs and includes should be a separate patch." That's why I
split them in this v5. The problem is that the driver can't be compiled
any more at PATCH 1/6. It needs PATCH 2/6 to be compiled. Should the
checkpatch warning be ignored here ? Should I finally squash PATCH 1/6
and PATCH 2/6 ?
[NOTE] Before being applied, PATCH 6/6 needs a 'gpu' node in 'mt8188.dtsi',
but this node does not exist yet. A series will be submitted by Angelo to
add this GPU support in MT8188.
Signed-off-by: Julien Panis <[email protected]>
---
Changes in v5:
- Rename some thermal zones
(mfg -> gpu / soc1 -> adsp / soc2 -> vdo / soc3 -> infra).
- Add cooling-device for GPUs.
- Link to v4: https://lore.kernel.org/r/[email protected]
Changes in v4:
- Fix wrong thermal zone names.
- Lower 'polling-delay-passive' values.
- Set 'hysteresis' value to 0 for 'critical' trips.
- Add a 'hot' trip point in between 'passive' and 'critical' trips.
- Link to v3: https://lore.kernel.org/all/[email protected]/
Changes in v3:
- use meaningful name for binding index definitions
- reuse LVTS_COEFF_*_MT7988 on MT8186 per reviewer request
- do similarly for MT8188 that now reuses LVTS_COEFF_*_MT8195
- use thermal zone names the svs driver wants
- adjust some DT node names and iospace length
- remove variable .hw_tshut_temp as it is constant across all SOCs
- Link to v2: https://lore.kernel.org/all/[email protected]/
Changes in v2:
- renamed CPU cluster thermal zones in DT
- fixed logic to cope with empty controller slots at the beginning
- isolated bindings to their own patches
- added MT8188 default thermal zones
- Link to v1: https://lore.kernel.org/all/[email protected]/T/
---
Julien Panis (2):
dt-bindings: thermal: mediatek: Rename thermal zone definitions for MT8186 and MT8188
thermal/drivers/mediatek/lvts_thermal: Use renamed thermal zone definitions for MT8186 and MT8188
Nicolas Pitre (4):
arm64: dts: mediatek: mt8186: add lvts definitions
arm64: dts: mediatek: mt8186: add default thermal zones
arm64: dts: mediatek: mt8188: add lvts definitions
arm64: dts: mediatek: mt8188: add default thermal zones
arch/arm64/boot/dts/mediatek/mt8186.dtsi | 316 ++++++++++++++
arch/arm64/boot/dts/mediatek/mt8188.dtsi | 481 +++++++++++++++++++++
drivers/thermal/mediatek/lvts_thermal.c | 12 +-
.../dt-bindings/thermal/mediatek,lvts-thermal.h | 12 +-
4 files changed, 809 insertions(+), 12 deletions(-)
---
base-commit: 632483ea8004edfadd035de36e1ab2c7c4f53158
change-id: 20240520-mtk-thermal-mt818x-dtsi-eca378f9b6a0
Best regards,
--
Julien Panis <[email protected]>
Use thermal zone names that make more sense.
Signed-off-by: Julien Panis <[email protected]>
---
include/dt-bindings/thermal/mediatek,lvts-thermal.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/dt-bindings/thermal/mediatek,lvts-thermal.h b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
index bf95309d2525..ddc7302a510a 100644
--- a/include/dt-bindings/thermal/mediatek,lvts-thermal.h
+++ b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
@@ -24,7 +24,7 @@
#define MT8186_BIG_CPU1 5
#define MT8186_NNA 6
#define MT8186_ADSP 7
-#define MT8186_MFG 8
+#define MT8186_GPU 8
#define MT8188_MCU_LITTLE_CPU0 0
#define MT8188_MCU_LITTLE_CPU1 1
@@ -34,11 +34,11 @@
#define MT8188_MCU_BIG_CPU1 5
#define MT8188_AP_APU 0
-#define MT8188_AP_GPU1 1
-#define MT8188_AP_GPU2 2
-#define MT8188_AP_SOC1 3
-#define MT8188_AP_SOC2 4
-#define MT8188_AP_SOC3 5
+#define MT8188_AP_GPU0 1
+#define MT8188_AP_GPU1 2
+#define MT8188_AP_ADSP 3
+#define MT8188_AP_VDO 4
+#define MT8188_AP_INFRA 5
#define MT8188_AP_CAM1 6
#define MT8188_AP_CAM2 7
--
2.37.3
Use thermal zone names that make more sense.
Signed-off-by: Julien Panis <[email protected]>
---
drivers/thermal/mediatek/lvts_thermal.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 0bb3a495b56e..89fb92666b81 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -1436,7 +1436,7 @@ static const struct lvts_ctrl_data mt8186_lvts_data_ctrl[] = {
.cal_offsets = { 29, 30, 31 } },
{ .dt_id = MT8186_ADSP,
.cal_offsets = { 34, 35, 28 } },
- { .dt_id = MT8186_MFG,
+ { .dt_id = MT8186_GPU,
.cal_offsets = { 39, 32, 33 } }
},
VALID_SENSOR_MAP(1, 1, 1, 0),
@@ -1487,11 +1487,11 @@ static const struct lvts_ctrl_data mt8188_lvts_ap_data_ctrl[] = {
},
{
.lvts_sensor = {
- { .dt_id = MT8188_AP_GPU1,
+ { .dt_id = MT8188_AP_GPU0,
.cal_offsets = { 43, 44, 45 } },
- { .dt_id = MT8188_AP_GPU2,
+ { .dt_id = MT8188_AP_GPU1,
.cal_offsets = { 46, 47, 48 } },
- { .dt_id = MT8188_AP_SOC1,
+ { .dt_id = MT8188_AP_ADSP,
.cal_offsets = { 49, 50, 51 } },
},
VALID_SENSOR_MAP(1, 1, 1, 0),
@@ -1500,9 +1500,9 @@ static const struct lvts_ctrl_data mt8188_lvts_ap_data_ctrl[] = {
},
{
.lvts_sensor = {
- { .dt_id = MT8188_AP_SOC2,
+ { .dt_id = MT8188_AP_VDO,
.cal_offsets = { 52, 53, 54 } },
- { .dt_id = MT8188_AP_SOC3,
+ { .dt_id = MT8188_AP_INFRA,
.cal_offsets = { 55, 56, 57 } },
},
VALID_SENSOR_MAP(1, 1, 0, 0),
--
2.37.3
From: Nicolas Pitre <[email protected]>
Various values extracted from the vendor's kernel driver.
Signed-off-by: Nicolas Pitre <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[Angelo: Fixed wrong nvmem-cell-names]
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8188.dtsi | 35 ++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
index b4315c9214dc..a9f1b9db54a6 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
@@ -11,6 +11,7 @@
#include <dt-bindings/phy/phy.h>
#include <dt-bindings/pinctrl/mediatek,mt8188-pinfunc.h>
#include <dt-bindings/power/mediatek,mt8188-power.h>
+#include <dt-bindings/reset/mt8188-resets.h>
/ {
compatible = "mediatek,mt8188";
@@ -357,6 +358,7 @@ infracfg_ao: syscon@10001000 {
compatible = "mediatek,mt8188-infracfg-ao", "syscon";
reg = <0 0x10001000 0 0x1000>;
#clock-cells = <1>;
+ #reset-cells = <1>;
};
pericfg: syscon@10003000 {
@@ -491,6 +493,17 @@ spi0: spi@1100a000 {
status = "disabled";
};
+ lvts_ap: thermal-sensor@1100b000 {
+ compatible = "mediatek,mt8188-lvts-ap";
+ reg = <0 0x1100b000 0 0xc00>;
+ interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
+ resets = <&infracfg_ao MT8188_INFRA_RST1_THERMAL_CTRL_RST>;
+ nvmem-cells = <&lvts_efuse_data1>;
+ nvmem-cell-names = "lvts-calib-data-1";
+ #thermal-sensor-cells = <1>;
+ };
+
spi1: spi@11010000 {
compatible = "mediatek,mt8188-spi-ipm", "mediatek,spi-ipm";
#address-cells = <1>;
@@ -604,6 +617,17 @@ mmc1: mmc@11240000 {
status = "disabled";
};
+ lvts_mcu: thermal-sensor@11278000 {
+ compatible = "mediatek,mt8188-lvts-mcu";
+ reg = <0 0x11278000 0 0x1000>;
+ interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
+ resets = <&infracfg_ao MT8188_INFRA_RST1_THERMAL_MCU_RST>;
+ nvmem-cells = <&lvts_efuse_data1>;
+ nvmem-cell-names = "lvts-calib-data-1";
+ #thermal-sensor-cells = <1>;
+ };
+
i2c0: i2c@11280000 {
compatible = "mediatek,mt8188-i2c";
reg = <0 0x11280000 0 0x1000>,
@@ -827,6 +851,17 @@ imp_iic_wrap_en: clock-controller@11ec2000 {
#clock-cells = <1>;
};
+ efuse: efuse@11f20000 {
+ compatible = "mediatek,mt8188-efuse", "mediatek,efuse";
+ reg = <0 0x11f20000 0 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ lvts_efuse_data1: lvts1-calib@1ac {
+ reg = <0x1ac 0x40>;
+ };
+ };
+
mfgcfg: clock-controller@13fbf000 {
compatible = "mediatek,mt8188-mfgcfg";
reg = <0 0x13fbf000 0 0x1000>;
--
2.37.3
From: Nicolas Pitre <[email protected]>
Values extracted from vendor source tree.
Signed-off-by: Nicolas Pitre <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[Angelo: Fixed validation and quality issues]
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8186.dtsi | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
index 4763ed5dc86c..caec83f5eece 100644
--- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
@@ -1361,6 +1361,17 @@ spi0: spi@1100a000 {
status = "disabled";
};
+ lvts: thermal-sensor@1100b000 {
+ compatible = "mediatek,mt8186-lvts";
+ reg = <0 0x1100b000 0 0x1000>;
+ interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
+ resets = <&infracfg_ao MT8186_INFRA_THERMAL_CTRL_RST>;
+ nvmem-cells = <&lvts_efuse_data1 &lvts_efuse_data2>;
+ nvmem-cell-names = "lvts-calib-data-1", "lvts-calib-data-2";
+ #thermal-sensor-cells = <1>;
+ };
+
pwm0: pwm@1100e000 {
compatible = "mediatek,mt8186-disp-pwm", "mediatek,mt8183-disp-pwm";
reg = <0 0x1100e000 0 0x1000>;
@@ -1676,6 +1687,14 @@ efuse: efuse@11cb0000 {
#address-cells = <1>;
#size-cells = <1>;
+ lvts_efuse_data1: lvts1-calib@1cc {
+ reg = <0x1cc 0x14>;
+ };
+
+ lvts_efuse_data2: lvts2-calib@2f8 {
+ reg = <0x2f8 0x14>;
+ };
+
gpu_speedbin: gpu-speedbin@59c {
reg = <0x59c 0x4>;
bits = <0 3>;
--
2.37.3
From: Nicolas Pitre <[email protected]>
Inspired by the vendor kernel but adapted to the upstream thermal
driver version.
Signed-off-by: Nicolas Pitre <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8186.dtsi | 297 +++++++++++++++++++++++++++++++
1 file changed, 297 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
index caec83f5eece..95fe5a05f0d7 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";
@@ -2197,4 +2199,299 @@ larb19: smi@1c10f000 {
power-domains = <&spm MT8186_POWER_DOMAIN_IPE>;
};
};
+
+ thermal_zones: thermal-zones {
+ cpu-little0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <150>;
+ thermal-sensors = <&lvts MT8186_LITTLE_CPU0>;
+
+ trips {
+ cpu_little0_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_little0_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_little0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_little0_alert0>;
+ 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-little1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <150>;
+ thermal-sensors = <&lvts MT8186_LITTLE_CPU1>;
+
+ trips {
+ cpu_little1_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_little1_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_little1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_little1_alert0>;
+ 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-little2-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <150>;
+ thermal-sensors = <&lvts MT8186_LITTLE_CPU2>;
+
+ trips {
+ cpu_little2_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_little2_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_little2_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_little2_alert0>;
+ 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_CAM>;
+
+ trips {
+ cam_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cam_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cam_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ nna-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_NNA>;
+
+ trips {
+ nna_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ nna_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ nna_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ adsp-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_ADSP>;
+
+ trips {
+ adsp_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ adsp_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ adsp_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ gpu-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts MT8186_GPU>;
+
+ trips {
+ gpu_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ gpu_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ gpu_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&gpu_alert0>;
+ cooling-device = <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu-big0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <100>;
+ thermal-sensors = <&lvts MT8186_BIG_CPU0>;
+
+ trips {
+ cpu_big0_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_big0_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_big0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_big0_alert0>;
+ 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 = <100>;
+ thermal-sensors = <&lvts MT8186_BIG_CPU1>;
+
+ trips {
+ cpu_big1_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_big1_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_big1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_big1_alert0>;
+ cooling-device = <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+ };
};
--
2.37.3
From: Nicolas Pitre <[email protected]>
Inspired by the vendor kernel but adapted to the upstream thermal
driver version.
Signed-off-by: Nicolas Pitre <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8188.dtsi | 446 +++++++++++++++++++++++++++++++
1 file changed, 446 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
index a9f1b9db54a6..6ab4ccc245b9 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
@@ -12,6 +12,8 @@
#include <dt-bindings/pinctrl/mediatek,mt8188-pinfunc.h>
#include <dt-bindings/power/mediatek,mt8188-power.h>
#include <dt-bindings/reset/mt8188-resets.h>
+#include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/thermal/mediatek,lvts-thermal.h>
/ {
compatible = "mediatek,mt8188";
@@ -311,6 +313,450 @@ psci {
method = "smc";
};
+ thermal_zones: thermal-zones {
+ cpu-little0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <150>;
+ thermal-sensors = <&lvts_mcu MT8188_MCU_LITTLE_CPU0>;
+
+ trips {
+ cpu_little0_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_little0_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_little0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_little0_alert0>;
+ 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-little1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <150>;
+ thermal-sensors = <&lvts_mcu MT8188_MCU_LITTLE_CPU1>;
+
+ trips {
+ cpu_little1_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_little1_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_little1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_little1_alert0>;
+ 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-little2-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <150>;
+ thermal-sensors = <&lvts_mcu MT8188_MCU_LITTLE_CPU2>;
+
+ trips {
+ cpu_little2_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_little2_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_little2_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_little2_alert0>;
+ 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-little3-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <150>;
+ thermal-sensors = <&lvts_mcu MT8188_MCU_LITTLE_CPU3>;
+
+ trips {
+ cpu_little3_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_little3_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_little3_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_little3_alert0>;
+ 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-big0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <100>;
+ thermal-sensors = <&lvts_mcu MT8188_MCU_BIG_CPU0>;
+
+ trips {
+ cpu_big0_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_big0_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_big0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_big0_alert0>;
+ 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 = <100>;
+ thermal-sensors = <&lvts_mcu MT8188_MCU_BIG_CPU1>;
+
+ trips {
+ cpu_big1_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_big1_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cpu_big1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu_big1_alert0>;
+ cooling-device = <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ apu-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8188_AP_APU>;
+
+ trips {
+ apu_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ apu_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ apu_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ gpu-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8188_AP_GPU0>;
+
+ trips {
+ gpu_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ gpu_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ gpu_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&gpu_alert0>;
+ cooling-device = <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ gpu1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8188_AP_GPU1>;
+
+ trips {
+ gpu1_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ gpu1_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ gpu1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&gpu1_alert0>;
+ cooling-device = <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ adsp-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8188_AP_ADSP>;
+
+ trips {
+ soc_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ soc_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ soc_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ vdo-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8188_AP_VDO>;
+
+ trips {
+ soc1_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ soc1_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ soc1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ infra-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8188_AP_INFRA>;
+
+ trips {
+ soc2_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ soc2_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ soc2_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ cam1-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8188_AP_CAM1>;
+
+ trips {
+ cam1_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cam1_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cam1_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ cam2-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_ap MT8188_AP_CAM2>;
+
+ trips {
+ cam2_alert0: trip-alert0 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cam2_alert1: trip-alert1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+
+ cam2_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+ };
+
timer: timer {
compatible = "arm,armv8-timer";
interrupt-parent = <&gic>;
--
2.37.3
On Fri, 24 May 2024, Julien Panis wrote:
> [RFC] When PATCH 1/6 and 2/6 are squashed, checkpatch raises this WARNING:
> "DT binding docs and includes should be a separate patch." That's why I
> split them in this v5. The problem is that the driver can't be compiled
> any more at PATCH 1/6. It needs PATCH 2/6 to be compiled. Should the
> checkpatch warning be ignored here ? Should I finally squash PATCH 1/6
> and PATCH 2/6 ?
IMHO it might be preferable to preserve successful compilation across
bisection than to appeal to checkpatch in this case.
Nicolas
On Fri, May 24, 2024 at 01:04:38PM -0400, Nicolas Pitre wrote:
> On Fri, 24 May 2024, Julien Panis wrote:
>
> > [RFC] When PATCH 1/6 and 2/6 are squashed, checkpatch raises this WARNING:
> > "DT binding docs and includes should be a separate patch." That's why I
> > split them in this v5. The problem is that the driver can't be compiled
> > any more at PATCH 1/6. It needs PATCH 2/6 to be compiled. Should the
> > checkpatch warning be ignored here ? Should I finally squash PATCH 1/6
> > and PATCH 2/6 ?
>
> IMHO it might be preferable to preserve successful compilation across
> bisection than to appeal to checkpatch in this case.
Or, patch 1 adds the new definitions, subsequent patches convert the
users, and the last patch removes the old, now unused, definitions.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Fri, May 24, 2024 at 11:04:34AM +0200, Julien Panis wrote:
> Use thermal zone names that make more sense.
>
> Signed-off-by: Julien Panis <[email protected]>
Removing the defines is an ABI break. If these are all the same devices,
but with more accurate naming, then keep the old defines and add new
ones. However, the GPU1 define changes in the course of this patch which
is more problematic.
Why do these names even make more sense? Where did the old names come
from and where do the new?
Thanks,
Conor.
> ---
> include/dt-bindings/thermal/mediatek,lvts-thermal.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/dt-bindings/thermal/mediatek,lvts-thermal.h b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
> index bf95309d2525..ddc7302a510a 100644
> --- a/include/dt-bindings/thermal/mediatek,lvts-thermal.h
> +++ b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
> @@ -24,7 +24,7 @@
> #define MT8186_BIG_CPU1 5
> #define MT8186_NNA 6
> #define MT8186_ADSP 7
> -#define MT8186_MFG 8
> +#define MT8186_GPU 8
>
> #define MT8188_MCU_LITTLE_CPU0 0
> #define MT8188_MCU_LITTLE_CPU1 1
> @@ -34,11 +34,11 @@
> #define MT8188_MCU_BIG_CPU1 5
>
> #define MT8188_AP_APU 0
> -#define MT8188_AP_GPU1 1
> -#define MT8188_AP_GPU2 2
> -#define MT8188_AP_SOC1 3
> -#define MT8188_AP_SOC2 4
> -#define MT8188_AP_SOC3 5
> +#define MT8188_AP_GPU0 1
> +#define MT8188_AP_GPU1 2
> +#define MT8188_AP_ADSP 3
> +#define MT8188_AP_VDO 4
> +#define MT8188_AP_INFRA 5
> #define MT8188_AP_CAM1 6
> #define MT8188_AP_CAM2 7
>
>
> --
> 2.37.3
>
On Fri, May 24, 2024 at 07:24:47PM +0100, Conor Dooley wrote:
> On Fri, May 24, 2024 at 11:04:34AM +0200, Julien Panis wrote:
> > Use thermal zone names that make more sense.
> >
> > Signed-off-by: Julien Panis <[email protected]>
>
> Removing the defines is an ABI break. If these are all the same devices,
> but with more accurate naming, then keep the old defines and add new
> ones. However, the GPU1 define changes in the course of this patch which
> is more problematic.
> > [RFC] When PATCH 1/6 and 2/6 are squashed, checkpatch raises this WARNING:
> > "DT binding docs and includes should be a separate patch." That's why I
> > split them in this v5. The problem is that the driver can't be compiled
> > any more at PATCH 1/6. It needs PATCH 2/6 to be compiled. Should the
> > checkpatch warning be ignored here ? Should I finally squash PATCH 1/6
> > and PATCH 2/6 ?
Heh, and there's just one of the issues caused by your ABI break...
> Why do these names even make more sense? Where did the old names come
> from and where do the new?
Thanks,
Conor
On 5/24/24 20:24, Conor Dooley wrote:
> On Fri, May 24, 2024 at 11:04:34AM +0200, Julien Panis wrote:
>> Use thermal zone names that make more sense.
>>
>> Signed-off-by: Julien Panis <[email protected]>
> Removing the defines is an ABI break. If these are all the same devices,
> but with more accurate naming, then keep the old defines and add new
> ones. However, the GPU1 define changes in the course of this patch which
> is more problematic.
>
> Why do these names even make more sense? Where did the old names come
> from and where do the new?
>
> Thanks,
> Conor.
Thanks for your comment.
For mt8188, the old name 'soc' came from a document by MTK, which is not
public: MT8188G Functional Specification. This document does not explain
what 'soc' are/do exactly, it just says that some temperature sensors are
located on them.
Then, there was a comment about these 'soc' names:
https://lore.kernel.org/all/[email protected]/
So, I had a discussion with MTK to understand what these 'soc1/2/3'
are/do. The new names comes from this discussion and were given by MTK.
For the other SoC of this series (mt8186), the same kind of document exists
(MT8186G Functional Specification) and explains what 'soc' are/do exactly:
('cam', 'dsp', 'nna'...which are used in 'mt8186.dtsi').
Using the same logic in 'mt8188.dtsi' would be more consistent. Besides,
these names are more explicit.
Julien
>
>> ---
>> include/dt-bindings/thermal/mediatek,lvts-thermal.h | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/dt-bindings/thermal/mediatek,lvts-thermal.h b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
>> index bf95309d2525..ddc7302a510a 100644
>> --- a/include/dt-bindings/thermal/mediatek,lvts-thermal.h
>> +++ b/include/dt-bindings/thermal/mediatek,lvts-thermal.h
>> @@ -24,7 +24,7 @@
>> #define MT8186_BIG_CPU1 5
>> #define MT8186_NNA 6
>> #define MT8186_ADSP 7
>> -#define MT8186_MFG 8
>> +#define MT8186_GPU 8
>>
>> #define MT8188_MCU_LITTLE_CPU0 0
>> #define MT8188_MCU_LITTLE_CPU1 1
>> @@ -34,11 +34,11 @@
>> #define MT8188_MCU_BIG_CPU1 5
>>
>> #define MT8188_AP_APU 0
>> -#define MT8188_AP_GPU1 1
>> -#define MT8188_AP_GPU2 2
>> -#define MT8188_AP_SOC1 3
>> -#define MT8188_AP_SOC2 4
>> -#define MT8188_AP_SOC3 5
>> +#define MT8188_AP_GPU0 1
>> +#define MT8188_AP_GPU1 2
>> +#define MT8188_AP_ADSP 3
>> +#define MT8188_AP_VDO 4
>> +#define MT8188_AP_INFRA 5
>> #define MT8188_AP_CAM1 6
>> #define MT8188_AP_CAM2 7
>>
>>
>> --
>> 2.37.3
>>
On 5/24/24 19:19, Russell King (Oracle) wrote:
> On Fri, May 24, 2024 at 01:04:38PM -0400, Nicolas Pitre wrote:
>> On Fri, 24 May 2024, Julien Panis wrote:
>>
>>> [RFC] When PATCH 1/6 and 2/6 are squashed, checkpatch raises this WARNING:
>>> "DT binding docs and includes should be a separate patch." That's why I
>>> split them in this v5. The problem is that the driver can't be compiled
>>> any more at PATCH 1/6. It needs PATCH 2/6 to be compiled. Should the
>>> checkpatch warning be ignored here ? Should I finally squash PATCH 1/6
>>> and PATCH 2/6 ?
>> IMHO it might be preferable to preserve successful compilation across
>> bisection than to appeal to checkpatch in this case.
> Or, patch 1 adds the new definitions, subsequent patches convert the
> users, and the last patch removes the old, now unused, definitions.
>
Thanks for this suggestion. I'll do something like that in next version.
On 5/24/24 20:27, Conor Dooley wrote:
> On Fri, May 24, 2024 at 07:24:47PM +0100, Conor Dooley wrote:
>> On Fri, May 24, 2024 at 11:04:34AM +0200, Julien Panis wrote:
>>> Use thermal zone names that make more sense.
>>>
>>> Signed-off-by: Julien Panis <[email protected]>
>> Removing the defines is an ABI break. If these are all the same devices,
>> but with more accurate naming, then keep the old defines and add new
>> ones. However, the GPU1 define changes in the course of this patch which
>> is more problematic.
>>> [RFC] When PATCH 1/6 and 2/6 are squashed, checkpatch raises this WARNING:
>>> "DT binding docs and includes should be a separate patch." That's why I
>>> split them in this v5. The problem is that the driver can't be compiled
>>> any more at PATCH 1/6. It needs PATCH 2/6 to be compiled. Should the
>>> checkpatch warning be ignored here ? Should I finally squash PATCH 1/6
>>> and PATCH 2/6 ?
> Heh, and there's just one of the issues caused by your ABI break...
Conor,
Would Russell's suggestion be acceptable for you ?
I mean, this one:
https://lore.kernel.org/all/[email protected]/
I could implement it, but before submitting it I would like to make
sure that it suits everyone.
Julien
>
>> Why do these names even make more sense? Where did the old names come
>> from and where do the new?
>
> Thanks,
> Conor
On Mon, May 27, 2024 at 05:25:35PM +0200, Julien Panis wrote:
> On 5/24/24 20:27, Conor Dooley wrote:
> > On Fri, May 24, 2024 at 07:24:47PM +0100, Conor Dooley wrote:
> > > On Fri, May 24, 2024 at 11:04:34AM +0200, Julien Panis wrote:
> > > > Use thermal zone names that make more sense.
> > > >
> > > > Signed-off-by: Julien Panis <[email protected]>
> > > Removing the defines is an ABI break. If these are all the same devices,
> > > but with more accurate naming, then keep the old defines and add new
> > > ones. However, the GPU1 define changes in the course of this patch which
> > > is more problematic.
> > > > [RFC] When PATCH 1/6 and 2/6 are squashed, checkpatch raises this WARNING:
> > > > "DT binding docs and includes should be a separate patch." That's why I
> > > > split them in this v5. The problem is that the driver can't be compiled
> > > > any more at PATCH 1/6. It needs PATCH 2/6 to be compiled. Should the
> > > > checkpatch warning be ignored here ? Should I finally squash PATCH 1/6
> > > > and PATCH 2/6 ?
> > Heh, and there's just one of the issues caused by your ABI break...
>
> Conor,
>
> Would Russell's suggestion be acceptable for you ?
> I mean, this one:
> https://lore.kernel.org/all/[email protected]/
>
> I could implement it, but before submitting it I would like to make
> sure that it suits everyone.
How's that going to work? MT8188_AP_GPU1 currently means 1, after your
series it means 2.
You're gonna need to pick a different naming for the new defines to
avoid that. Additionally, why even delete the old ones? Just define
new names with the same numbering and you don't need to worry about
any compatibility issues.
Thanks,
Conor.
On Mon, 27 May 2024, Conor Dooley wrote:
> On Mon, May 27, 2024 at 05:25:35PM +0200, Julien Panis wrote:
> > On 5/24/24 20:27, Conor Dooley wrote:
> > > On Fri, May 24, 2024 at 07:24:47PM +0100, Conor Dooley wrote:
> > > > On Fri, May 24, 2024 at 11:04:34AM +0200, Julien Panis wrote:
> > > > > Use thermal zone names that make more sense.
> > > > >
> > > > > Signed-off-by: Julien Panis <[email protected]>
> > > > Removing the defines is an ABI break. If these are all the same devices,
> > > > but with more accurate naming, then keep the old defines and add new
> > > > ones. However, the GPU1 define changes in the course of this patch which
> > > > is more problematic.
> > > > > [RFC] When PATCH 1/6 and 2/6 are squashed, checkpatch raises this WARNING:
> > > > > "DT binding docs and includes should be a separate patch." That's why I
> > > > > split them in this v5. The problem is that the driver can't be compiled
> > > > > any more at PATCH 1/6. It needs PATCH 2/6 to be compiled. Should the
> > > > > checkpatch warning be ignored here ? Should I finally squash PATCH 1/6
> > > > > and PATCH 2/6 ?
> > > Heh, and there's just one of the issues caused by your ABI break...
> >
> > Conor,
> >
> > Would Russell's suggestion be acceptable for you ?
> > I mean, this one:
> > https://lore.kernel.org/all/[email protected]/
> >
> > I could implement it, but before submitting it I would like to make
> > sure that it suits everyone.
>
> How's that going to work? MT8188_AP_GPU1 currently means 1, after your
> series it means 2.
> You're gonna need to pick a different naming for the new defines to
> avoid that. Additionally, why even delete the old ones? Just define
> new names with the same numbering and you don't need to worry about
> any compatibility issues.
Isn't this making a mountain out of a molehill here?
Seriously... none of this was present in a released kernel. The naming
can be adjusted "atomically" so compilation doesn't break, and
it is within maintainers' discretion to bypass the checkpatch warning in
such particular case.
Nicolas
On Mon, May 27, 2024 at 04:57:15PM -0400, Nicolas Pitre wrote:
> On Mon, 27 May 2024, Conor Dooley wrote:
> > > Would Russell's suggestion be acceptable for you ?
> > > I mean, this one:
> > > https://lore.kernel.org/all/[email protected]/
> > >
> > > I could implement it, but before submitting it I would like to make
> > > sure that it suits everyone.
> >
> > How's that going to work? MT8188_AP_GPU1 currently means 1, after your
> > series it means 2.
> > You're gonna need to pick a different naming for the new defines to
> > avoid that. Additionally, why even delete the old ones? Just define
> > new names with the same numbering and you don't need to worry about
> > any compatibility issues.
>
> Isn't this making a mountain out of a molehill here?
>
> Seriously... none of this was present in a released kernel. The naming
> can be adjusted "atomically" so compilation doesn't break, and
> it is within maintainers' discretion to bypass the checkpatch warning in
> such particular case.
If that's the case, then great. Provide a fixes tag, and a better commit
message than "Use thermal zone names that make more sense." that
actually explains why it is okay to change the definitions. There'd've
been neither mountain nor molehill were a sufficient commit message
provided.