add LVTS support for mt7988
Subsystems (3)
cover add LVTS support for mt7988
From: Frank Wunderlich <[email protected]>
This series makes allows soc specific temperature coefficient and adds support for mt7988 which has a different one.
Frank Wunderlich (3):
dt-bindings: thermal: mediatek: add mt7988 compatible thermal/drivers/mediatek/lvts_thermal: make coeff configurable thermal/drivers/mediatek/lvts_thermal: add mt7988 support
.../thermal/mediatek,lvts-thermal.yaml | 1 + drivers/thermal/mediatek/lvts_thermal.c | 129 ++++++++++++++++-- 2 files changed, 115 insertions(+), 15 deletions(-)
Signature · 1 line
2.34.1
RFC 1/3 dt-bindings: thermal: mediatek: add mt7988 compatible
From: Frank Wunderlich <[email protected]>
Add compatible string for mt7988.
--- .../devicetree/bindings/thermal/mediatek,lvts-thermal.yaml | 1 + 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml +1 -0
@@ -18,6 +18,7 @@ description: | properties: compatible: enum: + - mediatek,mt7988-lvts - mediatek,mt8192-lvts-ap - mediatek,mt8192-lvts-mcu - mediatek,mt8195-lvts-ap
Signature · 1 line
2.34.1
Re: [RFC v1 1/3] dt-bindings: thermal: mediatek: add mt7988 compatible
Quoted 9 lines · Frank Wunderlich
From: Frank Wunderlich <[email protected]>
Add compatible string for mt7988.
Signed-off-by Frank Wunderlich <[email protected]>--- .../devicetree/bindings/thermal/mediatek,lvts-thermal.yaml | 1 + 1 file changed, 1 insertion(+)
AngeloGioacchino Del Regno
maintainerRe: [RFC v1 1/3] dt-bindings: thermal: mediatek: add mt7988 compatible
In reply to Frank WunderlichIl 11/09/23 20:33, Frank Wunderlich ha scritto:
Quoted 14 lines
From: Frank Wunderlich <[email protected]>
Add compatible string for mt7988.
Signed-off-by Frank Wunderlich <[email protected]>--- .../devicetree/bindings/thermal/mediatek,lvts-thermal.yaml | 1 + 1 file changed, 1 insertion(+)Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml +0
-0
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml description: | · line 21
Quoted 4 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml +1
-0@@ -18,6 +18,7 @@ description: | properties: compatible: enum: + - mediatek,mt7988-lvts
Are you sure that MT7988 has only one LVTS controller, and that it is global?
Quoted 3 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml +0
-0- mediatek,mt8192-lvts-ap - mediatek,mt8192-lvts-mcu - mediatek,mt8195-lvts-ap
Re: [RFC v1 1/3] dt-bindings: thermal: mediatek: add mt7988 compatible
Am 13. September 2023 09:49:08 MESZ schrieb AngeloGioacchino Del Regno <[email protected]>:
Quoted 10 lines
Il 11/09/23 20:33, Frank Wunderlich ha scritto:
Quoted 9 lines
From: Frank Wunderlich <[email protected]>
Add compatible string for mt7988.
Signed-off-by Frank Wunderlich <[email protected]>--- .../devicetree/bindings/thermal/mediatek,lvts-thermal.yaml | 1 + 1 file changed, 1 insertion(+)
Quoted 11 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml description: | · line 21
Quoted 9 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml +1
-0@@ -18,6 +18,7 @@ description: | properties: compatible: enum: + - mediatek,mt7988-lvtsAre you sure that MT7988 has only one LVTS controller, and that it is global?
Based on the information i have it is only 1 lvts device (dts node) with 2 internal controllers. Do i need to define it in different way?
Quoted 4 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml description: | · line 24
Quoted 3 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml +0
-0- mediatek,mt8192-lvts-ap - mediatek,mt8192-lvts-mcu - mediatek,mt8195-lvts-ap
regards Frank
AngeloGioacchino Del Regno
maintainerRe: [RFC v1 1/3] dt-bindings: thermal: mediatek: add mt7988 compatible
Il 13/09/23 12:57, Frank Wunderlich ha scritto:
Quoted 23 lines
Am 13. September 2023 09:49:08 MESZ schrieb AngeloGioacchino Del Regno <[email protected]>:
Quoted 10 lines
Il 11/09/23 20:33, Frank Wunderlich ha scritto:
Quoted 9 lines
From: Frank Wunderlich <[email protected]>
Add compatible string for mt7988.
Signed-off-by Frank Wunderlich <[email protected]>--- .../devicetree/bindings/thermal/mediatek,lvts-thermal.yaml | 1 + 1 file changed, 1 insertion(+)Quoted 11 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml description: | · line 21
Quoted 9 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml +1
-0@@ -18,6 +18,7 @@ description: | properties: compatible: enum: + - mediatek,mt7988-lvtsAre you sure that MT7988 has only one LVTS controller, and that it is global?
Quoted 2 lines
Based on the information i have it is only 1 lvts device (dts node) with 2 internal controllers. Do i need to define it in different way?
In the MediaTek BSP, I can see that the controller at 0x1100a000 is referenced to as MT7988_AP_DOMAIN... this means that this controller effectively is the LVTS-AP one.
This means that the compatible here should be "mediatek,mt7988-lvts-ap" :-)
Regards,
Angelo
Quoted 7 lines
Quoted 4 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml description: | · line 24
Quoted 3 lines
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml +0
-0- mediatek,mt8192-lvts-ap - mediatek,mt8192-lvts-mcu - mediatek,mt8195-lvts-apregards Frank
RFC 2/3 thermal/drivers/mediatek/lvts_thermal: make coeff configurable
In reply to Frank WunderlichFrom: Frank Wunderlich <[email protected]>
The upcoming mt7988 has different temperature coefficients so we cannot use constants in the functions lvts_golden_temp_init, lvts_golden_temp_init and lvts_raw_to_temp anymore.
Add a field in the lvts_ctrl pointing to the lvts_data which now contains the soc-specific temperature coefficents.
--- drivers/thermal/mediatek/lvts_thermal.c | 56 ++++++++++++++++++------- 1 file changed, 41 insertions(+), 15 deletions(-)
drivers/thermal/mediatek/lvts_thermal.c +41 -15
@@ -80,8 +80,8 @@#define LVTS_SENSOR_MAX 4#define LVTS_GOLDEN_TEMP_MAX 62#define LVTS_GOLDEN_TEMP_DEFAULT 50 -#define LVTS_COEFF_A -250460 -#define LVTS_COEFF_B 250460 +#define LVTS_COEFF_A_MT8195 -250460 +#define LVTS_COEFF_B_MT8195 250460#define LVTS_MSR_IMMEDIATE_MODE 0#define LVTS_MSR_FILTERED_MODE 1
@@ -94,7 +94,7 @@#define LVTS_MINIMUM_THRESHOLD 20000static int golden_temp =LVTS_GOLDEN_TEMP_DEFAULT ; -static int coeff_b =LVTS_COEFF_B ; +static int coeff_b;struct lvts_sensor_data {int dt_id;
@@ -109,9 +109,15 @@ struct lvts_ctrl_data {int mode; }; +struct formula_coeff { +int a; +int b; +}; +struct lvts_data {const struct lvts_ctrl_data *lvts_ctrl;int num_lvts_ctrl; +struct formula_coeff coeff; };struct lvts_sensor {
@@ -126,6 +132,7 @@ struct lvts_sensor {struct lvts_ctrl {struct lvts_sensor sensors[LVTS_SENSOR_MAX ]; +const struct lvts_data *lvts_data;u32 calibration[LVTS_SENSOR_MAX ];u32 hw_tshut_raw_temp;int num_lvts_sensor;
@@ -247,21 +254,21 @@ static void lvts_debugfs_exit(struct lvts_domain *lvts_td) { }#endif -static int lvts_raw_to_temp (u32 raw_temp) +static int lvts_raw_to_temp (u32 raw_temp,struct formula_coeff coeff) {int temperature; - temperature = ((s64)(raw_temp &0xFFFF ) *LVTS_COEFF_A ) >>14 ; + temperature = ((s64)(raw_temp &0xFFFF ) * coeff.a) >>14 ; temperature += coeff_b;return temperature; } -static u32 lvts_temp_to_raw (int temperature) +static u32 lvts_temp_to_raw (int temperature,struct formula_coeff coeff) {u32 raw_temp = ((s64)(coeff_b - temperature)) <<14 ; - raw_temp =div_s64 (raw_temp, -LVTS_COEFF_A ); + raw_temp =div_s64 (raw_temp, -coeff.a);return raw_temp; }
@@ -269,6 +276,9 @@ static u32 lvts_temp_to_raw(int temperature)static int lvts_get_temp (struct thermal_zone_device *tz,int *temp) {struct lvts_sensor *lvts_sensor =thermal_zone_device_priv (tz); + struct lvts_ctrl *lvts_ctrl = container_of(lvts_sensor, struct lvts_ctrl, + sensors[lvts_sensor->id]); +const struct lvts_data *lvts_data = lvts_ctrl->lvts_data;void __iomem *msr = lvts_sensor->msr;u32 value;int rc;
@@ -301,7 +311,7 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)if (rc)return -EAGAIN ; - *temp =lvts_raw_to_temp (value &0xFFFF ); + *temp =lvts_raw_to_temp (value &0xFFFF , lvts_data->coeff);return 0 ; }
@@ -348,10 +358,13 @@ static bool lvts_should_update_thresh(struct lvts_ctrl *lvts_ctrl, int high)static int lvts_set_trips (struct thermal_zone_device *tz,int low,int high) {struct lvts_sensor *lvts_sensor =thermal_zone_device_priv (tz); - struct lvts_ctrl *lvts_ctrl = container_of(lvts_sensor, struct lvts_ctrl, sensors[lvts_sensor->id]); + struct lvts_ctrl *lvts_ctrl = container_of(lvts_sensor, struct lvts_ctrl, + sensors[lvts_sensor->id]); +const struct lvts_data *lvts_data = lvts_ctrl->lvts_data;void __iomem *base = lvts_sensor->base; -u32 raw_low =lvts_temp_to_raw (low != -INT_MAX ? low :LVTS_MINIMUM_THRESHOLD ); -u32 raw_high =lvts_temp_to_raw (high); +u32 raw_low =lvts_temp_to_raw (low != -INT_MAX ? low :LVTS_MINIMUM_THRESHOLD , + lvts_data->coeff); +u32 raw_high =lvts_temp_to_raw (high, lvts_data->coeff);bool should_update_thresh; lvts_sensor->low_thresh = low;
@@ -692,7 +705,7 @@ static int lvts_calibration_read(struct device *dev, struct lvts_domain *lvts_tdreturn 0 ; } -static int lvts_golden_temp_init (struct device *dev,u32 *value) +static int lvts_golden_temp_init (struct device *dev,u32 *value,struct formula_coeff coeff) {u32 gt;
@@ -701,7 +714,7 @@ static int lvts_golden_temp_init(struct device *dev, u32 *value)if (gt && gt <LVTS_GOLDEN_TEMP_MAX ) golden_temp = gt; - coeff_b = golden_temp *500 +LVTS_COEFF_B ; + coeff_b = golden_temp *500 + coeff.b;return 0 ; }
@@ -724,7 +737,7 @@ static int lvts_ctrl_init(struct device *dev, struct lvts_domain *lvts_td,* The golden temp information is contained in the first chunk * of efuse data. */ - ret =lvts_golden_temp_init (dev, (u32 *)lvts_td->calib); + ret =lvts_golden_temp_init (dev, (u32 *)lvts_td->calib, lvts_data->coeff);if (ret)return ret;
@@ -735,6 +748,7 @@ static int lvts_ctrl_init(struct device *dev, struct lvts_domain *lvts_td,for (i =0 ; i < lvts_data->num_lvts_ctrl; i++) { lvts_ctrl[i].base = lvts_td->base + lvts_data->lvts_ctrl[i].offset; + lvts_ctrl[i].lvts_data = lvts_data; ret =lvts_sensor_init (dev, &lvts_ctrl[i], &lvts_data->lvts_ctrl[i]);
@@ -758,7 +772,7 @@ static int lvts_ctrl_init(struct device *dev, struct lvts_domain *lvts_td,* after initializing the calibration. */ lvts_ctrl[i].hw_tshut_raw_temp = -lvts_temp_to_raw (lvts_data->lvts_ctrl[i].hw_tshut_temp); +lvts_temp_to_raw (lvts_data->lvts_ctrl[i].hw_tshut_temp, lvts_data->coeff); lvts_ctrl[i].low_thresh =INT_MIN ; lvts_ctrl[i].high_thresh =INT_MIN ;
@@ -1223,6 +1237,8 @@ static int lvts_probe(struct platform_device *pdev)if (irq <0 )return irq; + coeff_b = lvts_data->coeff.b; + ret =lvts_domain_init (dev, lvts_td, lvts_data);if (ret)return dev_err_probe (dev, ret,"Failed to initialize the lvts domain\n" );
@@ -1338,11 +1354,21 @@ static const struct lvts_ctrl_data mt8195_lvts_ap_data_ctrl[] = {static const struct lvts_data mt8195_lvts_mcu_data = { .lvts_ctrl = mt8195_lvts_mcu_data_ctrl, .num_lvts_ctrl =ARRAY_SIZE (mt8195_lvts_mcu_data_ctrl), + + .coeff = { + .a =LVTS_COEFF_A_MT8195 , + .b =LVTS_COEFF_B_MT8195 , + }, };static const struct lvts_data mt8195_lvts_ap_data = { .lvts_ctrl = mt8195_lvts_ap_data_ctrl, .num_lvts_ctrl =ARRAY_SIZE (mt8195_lvts_ap_data_ctrl), + + .coeff = { + .a =LVTS_COEFF_A_MT8195 , + .b =LVTS_COEFF_B_MT8195 , + }, };static const struct of_device_id lvts_of_match[] = {
Signature · 1 line
2.34.1
AngeloGioacchino Del Regno
maintainerRe: [RFC v1 2/3] thermal/drivers/mediatek/lvts_thermal: make coeff configurable
Il 11/09/23 20:33, Frank Wunderlich ha scritto:
Quoted 30 lines
From: Frank Wunderlich <[email protected]>
The upcoming mt7988 has different temperature coefficients so we cannot use constants in the functions
lvts_golden_temp_init,lvts_golden_temp_initandlvts_raw_to_tempanymore.Add a field in the
lvts_ctrlpointing to thelvts_datawhich now contains the soc-specific temperature coefficents.Signed-off-by Frank Wunderlich <[email protected]>--- drivers/thermal/mediatek/lvts_thermal.c | 56 ++++++++++++++++++------- 1 file changed, 41 insertions(+), 15 deletions(-)drivers/thermal/mediatek/lvts_thermal.c +2
-2@@ -80,8 +80,8 @@#define LVTS_SENSOR_MAX 4#define LVTS_GOLDEN_TEMP_MAX 62#define LVTS_GOLDEN_TEMP_DEFAULT 50 -#define LVTS_COEFF_A -250460 -#define LVTS_COEFF_B 250460 +#define LVTS_COEFF_A_MT8195 -250460 +#define LVTS_COEFF_B_MT8195 250460#define LVTS_MSR_IMMEDIATE_MODE 0#define LVTS_MSR_FILTERED_MODE 1
drivers/thermal/mediatek/lvts_thermal.c line 97
Quoted 5 lines
drivers/thermal/mediatek/lvts_thermal.c +1
-1@@ -94,7 +94,7 @@#define LVTS_MINIMUM_THRESHOLD 20000static int golden_temp =LVTS_GOLDEN_TEMP_DEFAULT ; -static int coeff_b =LVTS_COEFF_B ; +static int coeff_b;
This could be renamed to golden_temp_offset
Quoted 6 lines
drivers/thermal/mediatek/lvts_thermal.c +0
-0struct lvts_sensor_data {int dt_id;@@ -109,9 +109,15 @@ struct lvts_ctrl_data {int mode; };
drivers/thermal/mediatek/lvts_thermal.c struct lvts_ctrl_data { · line 120
Quoted 10 lines
drivers/thermal/mediatek/lvts_thermal.c +6
-0+struct formula_coeff { +int a; +int b; +}; +struct lvts_data {const struct lvts_ctrl_data *lvts_ctrl;int num_lvts_ctrl; +struct formula_coeff coeff;
You can just add the coefficients here directly... and while at it you can also make it self explanatory, because the "A" coefficient is a factor while the "B" coefficient is just an offset.
int temp_factor; <--- coeff_a
int temp_offset; <--- coeff_b
Quoted 12 lines
drivers/thermal/mediatek/lvts_thermal.c +1
-0};struct lvts_sensor {@@ -126,6 +132,7 @@ struct lvts_sensor {struct lvts_ctrl {struct lvts_sensor sensors[LVTS_SENSOR_MAX ]; +const struct lvts_data *lvts_data;u32 calibration[LVTS_SENSOR_MAX ];u32 hw_tshut_raw_temp;int num_lvts_sensor;
drivers/thermal/mediatek/lvts_thermal.c static void lvts_debugfs_exit(struct lvts_domain *lvts_td) { } · line 261
Quoted 10 lines
drivers/thermal/mediatek/lvts_thermal.c +2
-2@@ -247,21 +254,21 @@ static void lvts_debugfs_exit(struct lvts_domain *lvts_td) { }#endif -static int lvts_raw_to_temp (u32 raw_temp) +static int lvts_raw_to_temp (u32 raw_temp,struct formula_coeff coeff) {int temperature; - temperature = ((s64)(raw_temp &0xFFFF ) *LVTS_COEFF_A ) >>14 ; + temperature = ((s64)(raw_temp &0xFFFF ) * coeff.a) >>14 ;
so that this also becomes more readable:
static int lvts_raw_to_temp(u32 raw_temp, int temp_factor)
{
int temperature;
temperature = ((s64)(raw_temp & 0xFFFF) * temp_factor) >> 14;
temperature += golden_temp_offset;
return temperature;
}where temp_factor is lvts_data.temp_factor, and golden_temp_offset is a rename of static int coeff_b.
Cheers,
Angelo
Aw: Re: [RFC v1 2/3] thermal/drivers/mediatek/lvts_thermal: make coeff configurable
Hi
Quoted 62 lines
Gesendet: Mittwoch, 13. September 2023 um 10:03 Uhr
Von: "AngeloGioacchino Del Regno" <[email protected]> Il 11/09/23 20:33, Frank Wunderlich ha scritto:Quoted 30 lines
From: Frank Wunderlich <[email protected]>
The upcoming mt7988 has different temperature coefficients so we cannot use constants in the functions
lvts_golden_temp_init,lvts_golden_temp_initandlvts_raw_to_tempanymore.Add a field in the
lvts_ctrlpointing to thelvts_datawhich now contains the soc-specific temperature coefficents.Signed-off-by Frank Wunderlich <[email protected]>--- drivers/thermal/mediatek/lvts_thermal.c | 56 ++++++++++++++++++------- 1 file changed, 41 insertions(+), 15 deletions(-)drivers/thermal/mediatek/lvts_thermal.c +2
-2@@ -80,8 +80,8 @@#define LVTS_SENSOR_MAX 4#define LVTS_GOLDEN_TEMP_MAX 62#define LVTS_GOLDEN_TEMP_DEFAULT 50 -#define LVTS_COEFF_A -250460 -#define LVTS_COEFF_B 250460 +#define LVTS_COEFF_A_MT8195 -250460 +#define LVTS_COEFF_B_MT8195 250460#define LVTS_MSR_IMMEDIATE_MODE 0#define LVTS_MSR_FILTERED_MODE 1drivers/thermal/mediatek/lvts_thermal.c line 97
Quoted 5 lines
drivers/thermal/mediatek/lvts_thermal.c +1
-1@@ -94,7 +94,7 @@#define LVTS_MINIMUM_THRESHOLD 20000static int golden_temp =LVTS_GOLDEN_TEMP_DEFAULT ; -static int coeff_b =LVTS_COEFF_B ; +static int coeff_b;This could be renamed to
golden_temp_offsetQuoted 6 lines
drivers/thermal/mediatek/lvts_thermal.c +0
-0struct lvts_sensor_data {int dt_id;@@ -109,9 +109,15 @@ struct lvts_ctrl_data {int mode; };drivers/thermal/mediatek/lvts_thermal.c struct lvts_ctrl_data { · line 120
Quoted 10 lines
drivers/thermal/mediatek/lvts_thermal.c +6
-0+struct formula_coeff { +int a; +int b; +}; +struct lvts_data {const struct lvts_ctrl_data *lvts_ctrl;int num_lvts_ctrl; +struct formula_coeff coeff;You can just add the coefficients here directly... and while at it you can also make it self explanatory, because the "A" coefficient is a factor while the "B" coefficient is just an offset.
Quoted 2 lines
int
temp_factor; <---coeff_a
inttemp_offset; <---coeff_b
makes sense...imho very good idea.
originally i took the naming based on the Constants which maybe should be changed too?
Quoted 35 lines
Quoted 12 lines
drivers/thermal/mediatek/lvts_thermal.c +1
-0};struct lvts_sensor {@@ -126,6 +132,7 @@ struct lvts_sensor {struct lvts_ctrl {struct lvts_sensor sensors[LVTS_SENSOR_MAX ]; +const struct lvts_data *lvts_data;u32 calibration[LVTS_SENSOR_MAX ];u32 hw_tshut_raw_temp;int num_lvts_sensor;drivers/thermal/mediatek/lvts_thermal.c static void lvts_debugfs_exit(struct lvts_domain *lvts_td) { } · line 261
Quoted 10 lines
drivers/thermal/mediatek/lvts_thermal.c +2
-2@@ -247,21 +254,21 @@ static void lvts_debugfs_exit(struct lvts_domain *lvts_td) { }#endif -static int lvts_raw_to_temp (u32 raw_temp) +static int lvts_raw_to_temp (u32 raw_temp,struct formula_coeff coeff) {int temperature; - temperature = ((s64)(raw_temp &0xFFFF ) *LVTS_COEFF_A ) >>14 ; + temperature = ((s64)(raw_temp &0xFFFF ) * coeff.a) >>14 ;so that this also becomes more readable:
static int
lvts_raw_to_temp(u32 raw_temp, int temp_factor){ int temperature; temperature = ((s64)(raw_temp & 0xFFFF) * temp_factor) >> 14; temperature += golden_temp_offset; return temperature; }
Quoted 2 lines
where
temp_factoris lvts_data.temp_factor, andgolden_temp_offsetis a rename ofstatic int coeff_b.
right and passing an int (instead of struct) is easier and more readable too.
Quoted 4 lines
Cheers,
Angelo
RFC 3/3 thermal/drivers/mediatek/lvts_thermal: add mt7988 support
In reply to Frank WunderlichFrom: Frank Wunderlich <[email protected]>
Add Support for mediatek fologic 880/MT7988.
--- drivers/thermal/mediatek/lvts_thermal.c | 73 +++++++++++++++++++++++++ 1 file changed, 73 insertions(+)
drivers/thermal/mediatek/lvts_thermal.c +73 -0
@@ -82,6 +82,8 @@#define LVTS_GOLDEN_TEMP_DEFAULT 50#define LVTS_COEFF_A_MT8195 -250460#define LVTS_COEFF_B_MT8195 250460 +#define LVTS_COEFF_A_MT7988 -204650 +#define LVTS_COEFF_B_MT7988 204650#define LVTS_MSR_IMMEDIATE_MODE 0#define LVTS_MSR_FILTERED_MODE 1
@@ -1272,6 +1274,67 @@ static int lvts_remove(struct platform_device *pdev)return 0 ; } +/* +* LVTS MT7988 +*/ +#define LVTS_HW_SHUTDOWN_MT7988 117000 +//enum mt7988_lvts_domain { MT7988_AP_DOMAIN, MT7988_NUM_DOMAIN }; + +enum mt7988_lvts_sensor_enum { +MT7988_TS3_0 , +MT7988_TS3_1 , +MT7988_TS3_2 , +MT7988_TS3_3 , +MT7988_TS4_0 , +MT7988_TS4_1 , +MT7988_TS4_2 , +MT7988_TS4_3 , +MT7988_NUM_TS +}; + +//Efuse base : 0x11f50000 +//lvts offset in efuse : 0x918 (set in efuse dts node as calibration data) +//offsets: +//918 = LVTS_3_0_COUNT_R +//91C = LVTS_3_1_COUNT_R +//920 = LVTS_3_2_COUNT_R +//924 = LVTS_3_3_COUNT_R +//928 = LVTS_3_COUNT_RC + +//92C = LVTS_4_0_COUNT_R +//930 = LVTS_4_1_COUNT_R +//934 = LVTS_4_2_COUNT_R +//938 = LVTS_4_3_COUNT_R +//93C = LVTS_4_COUNT_RC + +static const struct lvts_ctrl_data mt7988_lvts_data_ctrl[] = { + { + .cal_offset = {0x00 ,0x04 ,0x08 ,0x0c },//918,91C,920,924 + .lvts_sensor = { + { .dt_id =MT7988_TS3_0 }, + { .dt_id =MT7988_TS3_1 }, + { .dt_id =MT7988_TS3_2 }, + { .dt_id =MT7988_TS3_3 } + }, + .num_lvts_sensor =4 , + .offset =0x0 , + .hw_tshut_temp =LVTS_HW_SHUTDOWN_MT7988 , + }, + { + .cal_offset = {0x14 ,0x18 ,0x1c ,0x20 },//92C,930,934,938 + .lvts_sensor = { + { .dt_id =MT7988_TS4_0 }, + { .dt_id =MT7988_TS4_1 }, + { .dt_id =MT7988_TS4_2 }, + { .dt_id =MT7988_TS4_3 } + }, + .num_lvts_sensor =4 , + .offset =0x100 , + .hw_tshut_temp =LVTS_HW_SHUTDOWN_MT7988 , + } +}; + +//MT8195 static const struct lvts_ctrl_data mt8195_lvts_mcu_data_ctrl[] = { { .cal_offset = {0x04 ,0x07 },
@@ -1351,6 +1414,15 @@ static const struct lvts_ctrl_data mt8195_lvts_ap_data_ctrl[] = { } }; +static const struct lvts_data mt7988_lvts_data = { + .lvts_ctrl = mt7988_lvts_data_ctrl, + .num_lvts_ctrl =ARRAY_SIZE (mt7988_lvts_data_ctrl), + .coeff = { + .a =LVTS_COEFF_A_MT7988 , + .b =LVTS_COEFF_B_MT7988 , + }, +}; +static const struct lvts_data mt8195_lvts_mcu_data = { .lvts_ctrl = mt8195_lvts_mcu_data_ctrl, .num_lvts_ctrl =ARRAY_SIZE (mt8195_lvts_mcu_data_ctrl),
@@ -1372,6 +1444,7 @@ static const struct lvts_data mt8195_lvts_ap_data = { };static const struct of_device_id lvts_of_match[] = { + { .compatible ="mediatek,mt7988-lvts" , .data = &mt7988_lvts_data }, { .compatible ="mediatek,mt8195-lvts-mcu" , .data = &mt8195_lvts_mcu_data }, { .compatible ="mediatek,mt8195-lvts-ap" , .data = &mt8195_lvts_ap_data }, {},
Signature · 1 line
2.34.1
AngeloGioacchino Del Regno
maintainerRe: [RFC v1 3/3] thermal/drivers/mediatek/lvts_thermal: add mt7988 support
Il 11/09/23 20:33, Frank Wunderlich ha scritto:
Quoted 23 lines
From: Frank Wunderlich <[email protected]>
Add Support for mediatek fologic 880/MT7988.
Signed-off-by Frank Wunderlich <[email protected]>--- drivers/thermal/mediatek/lvts_thermal.c | 73 +++++++++++++++++++++++++ 1 file changed, 73 insertions(+)drivers/thermal/mediatek/lvts_thermal.c +2
-0@@ -82,6 +82,8 @@#define LVTS_GOLDEN_TEMP_DEFAULT 50#define LVTS_COEFF_A_MT8195 -250460#define LVTS_COEFF_B_MT8195 250460 +#define LVTS_COEFF_A_MT7988 -204650 +#define LVTS_COEFF_B_MT7988 204650#define LVTS_MSR_IMMEDIATE_MODE 0#define LVTS_MSR_FILTERED_MODE 1
drivers/thermal/mediatek/lvts_thermal.c static int lvts_remove(struct platform_device *pdev) · line 1280
Quoted 7 lines
drivers/thermal/mediatek/lvts_thermal.c +4
-0@@ -1272,6 +1274,67 @@ static int lvts_remove(struct platform_device *pdev)return 0 ; } +/* +* LVTS MT7988 +*/ +#define LVTS_HW_SHUTDOWN_MT7988 117000
Are you sure that this chip's Tj is >117°C ?!
Looks a bit high... if it is exactly 117°C, I would suggest cutting earlier, either at 110 (safe side) or 115: after all, this is a life-saver feature and the chip is actually never meant to *constantly* work at 110°C (as it would degrade fast and say goodbye earlier than "planned").
drivers/thermal/mediatek/lvts_thermal.c static int lvts_remove(struct platform_device *pdev) · line 1293
Quoted 13 lines
drivers/thermal/mediatek/lvts_thermal.c +13
-0+//enum mt7988_lvts_domain { MT7988_AP_DOMAIN, MT7988_NUM_DOMAIN }; + +enum mt7988_lvts_sensor_enum { +MT7988_TS3_0 , +MT7988_TS3_1 , +MT7988_TS3_2 , +MT7988_TS3_3 , +MT7988_TS4_0 , +MT7988_TS4_1 , +MT7988_TS4_2 , +MT7988_TS4_3 , +MT7988_NUM_TS +};
This enumeration should be definitions in bindings (mediatek,lvts-thermal.h).
Besides, the LVTS is about internal temperatures, so those TS3_x and 4_x can be renamed like what was done for MT8192 and MT8195: this is because you will never see TS3_2 being CPU2 on a board and CPU4 on another, being those - again - internal to the SoC, hence unchangeable.
Another reason is that you'll anyway have to refer to those sensors in the devicetree to configure thermal trips and such, so... :-)
Regards,
Angelo
Re: [RFC v1 3/3] thermal/drivers/mediatek/lvts_thermal: add mt7988 support
Am 13. September 2023 10:16:51 MESZ schrieb AngeloGioacchino Del Regno <[email protected]>:
Hi angelo,
thanks for first look
Quoted 32 lines
Il 11/09/23 20:33, Frank Wunderlich ha scritto:
Quoted 22 lines
From: Frank Wunderlich <[email protected]>
Add Support for mediatek fologic 880/MT7988.
Signed-off-by Frank Wunderlich <[email protected]>--- drivers/thermal/mediatek/lvts_thermal.c | 73 +++++++++++++++++++++++++ 1 file changed, 73 insertions(+)drivers/thermal/mediatek/lvts_thermal.c +2
-0@@ -82,6 +82,8 @@#define LVTS_GOLDEN_TEMP_DEFAULT 50#define LVTS_COEFF_A_MT8195 -250460#define LVTS_COEFF_B_MT8195 250460 +#define LVTS_COEFF_A_MT7988 -204650 +#define LVTS_COEFF_B_MT7988 204650#define LVTS_MSR_IMMEDIATE_MODE 0#define LVTS_MSR_FILTERED_MODE 1drivers/thermal/mediatek/lvts_thermal.c static int lvts_remove(struct platform_device *pdev) · line 1279
Quoted 6 lines
drivers/thermal/mediatek/lvts_thermal.c +3
-0@@ -1272,6 +1274,67 @@ static int lvts_remove(struct platform_device *pdev)return 0 ; } +/* +* LVTS MT7988 +*/ +#define LVTS_HW_SHUTDOWN_MT7988 117000Are you sure that this chip's Tj is >117°C ?!
Quoted 4 lines
Looks a bit high... if it is exactly 117°C, I would suggest cutting earlier, either at 110 (safe side) or 115: after all, this is a life-saver feature and the chip is actually never meant to *constantly* work at 110°C (as it would degrade fast and say goodbye earlier than "planned").
I took values from SDK
Quoted 16 lines
drivers/thermal/mediatek/lvts_thermal.c static int lvts_remove(struct platform_device *pdev) · line 1293
Quoted 13 lines
drivers/thermal/mediatek/lvts_thermal.c +13
-0+//enum mt7988_lvts_domain { MT7988_AP_DOMAIN, MT7988_NUM_DOMAIN }; + +enum mt7988_lvts_sensor_enum { +MT7988_TS3_0 , +MT7988_TS3_1 , +MT7988_TS3_2 , +MT7988_TS3_3 , +MT7988_TS4_0 , +MT7988_TS4_1 , +MT7988_TS4_2 , +MT7988_TS4_3 , +MT7988_NUM_TS +};This enumeration should be definitions in bindings (mediatek,lvts-thermal.h).
Quoted 4 lines
Besides, the LVTS is about internal temperatures, so those
TS3_xand 4_x can be renamed like what was done for MT8192 and MT8195: this is because you will never seeTS3_2being CPU2 on a board and CPU4 on another, being those - again - internal to the SoC, hence unchangeable.
Right these sensors are internally only and i took naming from sdk to avoid confusion. And i have not more information about these internal sensors (special meaning),but their values are packed together to get the resulting (average) temperature.
Quoted 2 lines
Another reason is that you'll anyway have to refer to those sensors in the devicetree to configure thermal trips and such, so... :-)
In device tree it will look like this:
Daniel has also defined thermal trips there,but these are untested atm. I only verified temperature itself i get from sysfs as far as i can (start at ~40°C and reaching ~70 while running).
Quoted 2 lines
Regards,
Angelo
regards Frank
AngeloGioacchino Del Regno
maintainerRe: [RFC v1 3/3] thermal/drivers/mediatek/lvts_thermal: add mt7988 support
Il 13/09/23 12:52, Frank Wunderlich ha scritto:
Quoted 37 lines
Am 13. September 2023 10:16:51 MESZ schrieb AngeloGioacchino Del Regno <[email protected]>:
Hi angelo,thanks for first look
Quoted 32 lines
Il 11/09/23 20:33, Frank Wunderlich ha scritto:
Quoted 22 lines
From: Frank Wunderlich <[email protected]>
Add Support for mediatek fologic 880/MT7988.
Signed-off-by Frank Wunderlich <[email protected]>--- drivers/thermal/mediatek/lvts_thermal.c | 73 +++++++++++++++++++++++++ 1 file changed, 73 insertions(+)drivers/thermal/mediatek/lvts_thermal.c +2
-0@@ -82,6 +82,8 @@#define LVTS_GOLDEN_TEMP_DEFAULT 50#define LVTS_COEFF_A_MT8195 -250460#define LVTS_COEFF_B_MT8195 250460 +#define LVTS_COEFF_A_MT7988 -204650 +#define LVTS_COEFF_B_MT7988 204650#define LVTS_MSR_IMMEDIATE_MODE 0#define LVTS_MSR_FILTERED_MODE 1drivers/thermal/mediatek/lvts_thermal.c static int lvts_remove(struct platform_device *pdev) · line 1279
Quoted 6 lines
drivers/thermal/mediatek/lvts_thermal.c +3
-0@@ -1272,6 +1274,67 @@ static int lvts_remove(struct platform_device *pdev)return 0 ; } +/* +* LVTS MT7988 +*/ +#define LVTS_HW_SHUTDOWN_MT7988 117000Are you sure that this chip's Tj is >117°C ?!
Quoted 9 lines
Quoted 4 lines
Looks a bit high... if it is exactly 117°C, I would suggest cutting earlier, either at 110 (safe side) or 115: after all, this is a life-saver feature and the chip is actually never meant to *constantly* work at 110°C (as it would degrade fast and say goodbye earlier than "planned").
I took values from SDK
That kernel also defines 117°C for MT8195, which leaves me a bit dubious.
For safety, I would recommend using the same 105°C hw shutdown temperature for 7988 as well: after all if you think about it, reaching that kind of temperature means that there's a real problem (fan stopped working and/or heatsink detached) that needs attention.
This is because you'll have trip points in devicetree that should throttle the CPU in case it reaches at least a dangerously high temperature (read:
a temperature outside the recommended continuous operation range), bringing the temperatures down because of the throttling action; I would imagine throttling the CPU a bit down at 80°C, then a bit more at 90°C - but then, if the temps won't drop like that, there's clearly a HW-related issue that must be addressed (like the fan/heatsink scenario that I just described).
Though, take this as an advice; I'll respect whichever decision you'll take.
Quoted 23 lines
Quoted 16 lines
drivers/thermal/mediatek/lvts_thermal.c static int lvts_remove(struct platform_device *pdev) · line 1293
Quoted 13 lines
drivers/thermal/mediatek/lvts_thermal.c +13
-0+//enum mt7988_lvts_domain { MT7988_AP_DOMAIN, MT7988_NUM_DOMAIN }; + +enum mt7988_lvts_sensor_enum { +MT7988_TS3_0 , +MT7988_TS3_1 , +MT7988_TS3_2 , +MT7988_TS3_3 , +MT7988_TS4_0 , +MT7988_TS4_1 , +MT7988_TS4_2 , +MT7988_TS4_3 , +MT7988_NUM_TS +};This enumeration should be definitions in bindings (mediatek,lvts-thermal.h).
Quoted 4 lines
Besides, the LVTS is about internal temperatures, so those
TS3_xand 4_x can be renamed like what was done for MT8192 and MT8195: this is because you will never seeTS3_2being CPU2 on a board and CPU4 on another, being those - again - internal to the SoC, hence unchangeable.Right these sensors are internally only and i took naming from sdk to avoid confusion. And i have not more information about these internal sensors (special meaning),but their values are packed together to get the resulting (average) temperature.
Quoted 9 lines
Quoted 2 lines
Another reason is that you'll anyway have to refer to those sensors in the devicetree to configure thermal trips and such, so... :-)
In device tree it will look like this:
Daniel has also defined thermal trips there,but these are untested atm. I only verified temperature itself i get from sysfs as far as i can (start at ~40°C and reaching ~70 while running).
Check how it's done in mt8192.dtsi and mt8195.dtsi: we're using the definitions from the bindings for thermal zones.
At least for consistency (apart from other even more valid considerations), that's how it should be done: please do it like that.
Besides, I think that you can definitely guess what TS is CPU related: checking the driver and devicetree from the downstream kernel that you provided, what I understand is:
1. Your naming TS3,4 corresponds to TS2,3 downstream (so it's wrong?)
2. Downstream TS2 is related to the CPU cores, so it should be
- TS2_0 - CPU0
- TS2_1 - CPU1
- TS2_2 - CPU2
- TS2_3 - CPU3
The other set of thermal sensors seem to be completely unused, so we cannot guess just by looking at the code. Since you have the hardware, you may be able to take a position about what they are by checking what sensor heats up for each "action" (could be ethernet controller or infra or whatever else); if in doubt, just leave them out, but add a comment saying that there are more sensors and say where, so that if anyone finds what they're for, it'll be easy to add them.
In any case, adding thermal sensors that you don't know about is meaningless, as the sense of all this is:
- Monitoring temperatures of the hardware
- Taking action to prevent temperature overrun
but if you don't know what you're reading, you can't interpret temperatures for something unknown, hence you can't as well take action to prevent overruns, as you won't know what's the maximum temperature for "unknown thing" :-)
Regards,
Angelo
Aw: Re: [RFC v1 3/3] thermal/drivers/mediatek/lvts_thermal: add mt7988 support
Hi,
Quoted 51 lines
Gesendet: Mittwoch, 13. September 2023 um 13:43 Uhr
Von: "AngeloGioacchino Del Regno" <[email protected]>
An: [email protected], "Frank Wunderlich" <[email protected]>, [email protected] Il 13/09/23 12:52, Frank Wunderlich ha scritto:Quoted 37 lines
Am 13. September 2023 10:16:51 MESZ schrieb AngeloGioacchino Del Regno <[email protected]>:
Hi angelo,thanks for first look
Quoted 32 lines
Il 11/09/23 20:33, Frank Wunderlich ha scritto:
Quoted 22 lines
From: Frank Wunderlich <[email protected]>
Add Support for mediatek fologic 880/MT7988.
Signed-off-by Frank Wunderlich <[email protected]>--- drivers/thermal/mediatek/lvts_thermal.c | 73 +++++++++++++++++++++++++ 1 file changed, 73 insertions(+)drivers/thermal/mediatek/lvts_thermal.c +2
-0@@ -82,6 +82,8 @@#define LVTS_GOLDEN_TEMP_DEFAULT 50#define LVTS_COEFF_A_MT8195 -250460#define LVTS_COEFF_B_MT8195 250460 +#define LVTS_COEFF_A_MT7988 -204650 +#define LVTS_COEFF_B_MT7988 204650#define LVTS_MSR_IMMEDIATE_MODE 0#define LVTS_MSR_FILTERED_MODE 1drivers/thermal/mediatek/lvts_thermal.c static int lvts_remove(struct platform_device *pdev) · line 1279
Quoted 6 lines
drivers/thermal/mediatek/lvts_thermal.c +3
-0@@ -1272,6 +1274,67 @@ static int lvts_remove(struct platform_device *pdev)return 0 ; } +/* +* LVTS MT7988 +*/ +#define LVTS_HW_SHUTDOWN_MT7988 117000Are you sure that this chip's Tj is >117°C ?!
Quoted 9 lines
Quoted 4 lines
Looks a bit high... if it is exactly 117°C, I would suggest cutting earlier, either at 110 (safe side) or 115: after all, this is a life-saver feature and the chip is actually never meant to *constantly* work at 110°C (as it would degrade fast and say goodbye earlier than "planned").
I took values from SDK
Quoted 6 lines
That kernel also defines 117°C for MT8195, which leaves me a bit dubious.
For safety, I would recommend using the same 105°C hw shutdown temperature for 7988 as well: after all if you think about it, reaching that kind of temperature means that there's a real problem (fan stopped working and/or heatsink detached) that needs attention.
ack, will change to same value and put the define on top, abve the mt8195
Quoted 52 lines
This is because you'll have trip points in devicetree that should throttle the CPU in case it reaches at least a dangerously high temperature (read:
a temperature outside the recommended continuous operation range), bringing the temperatures down because of the throttling action; I would imagine throttling the CPU a bit down at 80°C, then a bit more at 90°C - but then, if the temps won't drop like that, there's clearly a HW-related issue that must be addressed (like the fan/heatsink scenario that I just described).Though, take this as an advice; I'll respect whichever decision you'll take.
Quoted 23 lines
Quoted 16 lines
drivers/thermal/mediatek/lvts_thermal.c static int lvts_remove(struct platform_device *pdev) · line 1293
Quoted 13 lines
drivers/thermal/mediatek/lvts_thermal.c +13
-0+//enum mt7988_lvts_domain { MT7988_AP_DOMAIN, MT7988_NUM_DOMAIN }; + +enum mt7988_lvts_sensor_enum { +MT7988_TS3_0 , +MT7988_TS3_1 , +MT7988_TS3_2 , +MT7988_TS3_3 , +MT7988_TS4_0 , +MT7988_TS4_1 , +MT7988_TS4_2 , +MT7988_TS4_3 , +MT7988_NUM_TS +};This enumeration should be definitions in bindings (mediatek,lvts-thermal.h).
Quoted 4 lines
Besides, the LVTS is about internal temperatures, so those
TS3_xand 4_x can be renamed like what was done for MT8192 and MT8195: this is because you will never seeTS3_2being CPU2 on a board and CPU4 on another, being those - again - internal to the SoC, hence unchangeable.Right these sensors are internally only and i took naming from sdk to avoid confusion. And i have not more information about these internal sensors (special meaning),but their values are packed together to get the resulting (average) temperature.
Quoted 9 lines
Quoted 2 lines
Another reason is that you'll anyway have to refer to those sensors in the devicetree to configure thermal trips and such, so... :-)
In device tree it will look like this:
Daniel has also defined thermal trips there,but these are untested atm. I only verified temperature itself i get from sysfs as far as i can (start at ~40°C and reaching ~70 while running).
Check how it's done in
mt8192.dtsiandmt8195.dtsi: we're using the definitions from the bindings for thermal zones.
At least for consistency (apart from other even more valid considerations), that's how it should be done: please do it like that.Besides, I think that you can definitely guess what
TSis CPU related: checking the driver and devicetree from the downstream kernel that you provided, what I understand is:
Quoted 6 lines
1. Your naming TS3,4 corresponds to TS2,3 downstream (so it's wrong?)
2. Downstream TS2 is related to the CPU cores, so it should be
-TS2_0- CPU0
-TS2_1- CPU1
-TS2_2- CPU2
-TS2_3- CPU3
i took the names from efuse names (which are listed as comment above), had 2/3 before. need to ask mtk here. same for the second controller and if it is really a "lvts-ap".
Quoted 7 lines
The other set of thermal sensors seem to be completely unused, so we cannot guess just by looking at the code. Since you have the hardware, you may be able to take a position about what they are by checking what sensor heats up for each "action" (could be ethernet controller or infra or whatever else); if in doubt, just leave them out, but add a comment saying that there are more sensors and say where, so that if anyone finds what they're for, it'll be easy to add them.
Quoted 7 lines
In any case, adding thermal sensors that you don't know about is meaningless, as the sense of all this is:
- Monitoring temperatures of the hardware
- Taking action to prevent temperature overrun
but if you don't know what you're reading, you can't interpret temperatures for something unknown, hence you can't as well take action to prevent overruns, as you won't know what's the maximum temperature for "unknown thing" :-)
yes i have only the downstream kernel and some additional information like interupt number and the efuse offsets for calibration. And for the 8 sensors i get this information:
"8 sensors are distributed across the whole soc in order to get the correct average temperature. chip designer didn’t disclose more detailed info to us."
so i thought all 8 sensors are taken into account with both controllers to calculate 1 resulting temperature. In downstream i saw also only 1 controller in dts and i have only 1 interrupt.
and the compatible for both (mt7988 and mt8195) are without "ap" there, so i took the same.
Where have you seen the mt7988-AP ?
i have only seen 1 sensor which should be the SOC itself and this chip has metal surface which makes it hard to get real temperature (infrared thermometer gives wrong temp).
Quoted 2 lines
Regards,
Angelo