[PATCH] thermal/drivers/mediatek/lvts_thermal: Make reset optional for MT8196

[PATCH] thermal/drivers/mediatek/lvts_thermal: Make reset optional for MT8196

[AngeloGioacchino Del Regno]

Depending on the SoC+Firmware combination, the LVTS hardware may be
may be actively used by one or even multiple concurrent MCUs!
In this case, resetting it may produce either a severe slowdown of
the entire system, or even a thermal protection AP reset, as some
MCU(s) may be reading a very high or very low temperature while the
LVTS is being reset.

On those, don't fail if no reset is found as that may be omitted on
purpose, but still check if there's one, because some board(s) may
be running on a different bootchain with reduced firmwares or using
firmwares with reduced functionality.

Add a new "optional_reset" member to lvts_data and use it to check
whether the resets should be mandatory or not.

Also, while at it, since devm_reset_control_get_by_index() is now
deprecated, change the probe function to instead call function
devm_reset_control_get_exclusive_by_index(), which does the same.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/thermal/mediatek/lvts_thermal.c | 29 ++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 92711896ce24..6670b5458f2c 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -160,6 +160,7 @@ struct lvts_data {
 	int gt_calib_bit_offset;
 	unsigned int def_calibration;
 	u16 msr_offset;
+	bool optional_reset;
 };
 
 struct lvts_sensor {
@@ -1473,9 +1474,29 @@ static int lvts_probe(struct platform_device *pdev)
 	if (IS_ERR(lvts_td->base))
 		return dev_err_probe(dev, PTR_ERR(lvts_td->base), "Failed to map io resource\n");
 
-	lvts_td->reset = devm_reset_control_get_by_index(dev, 0);
-	if (IS_ERR(lvts_td->reset))
-		return dev_err_probe(dev, PTR_ERR(lvts_td->reset), "Failed to get reset control\n");
+	/*
+	 * Depending on the SoC+Firmware combination, the LVTS hardware may be
+	 * may be actively used by one or even multiple concurrent MCUs!
+	 * In this case, resetting it may produce either a severe slowdown of
+	 * the entire system, or even a thermal protection AP reset, as some
+	 * MCU(s) may be reading a very high or very low temperature while the
+	 * LVTS is being reset.
+	 *
+	 * On those, don't fail if no reset is found as that may be omitted on
+	 * purpose, but still check if there's one, because some board(s) may
+	 * be running on a different bootchain with reduced firmwares or using
+	 * firmwares with reduced functionality.
+	 */
+	lvts_td->reset = devm_reset_control_get_exclusive_by_index(dev, 0);
+	if (IS_ERR(lvts_td->reset)) {
+		if (lvts_data->optional_reset) {
+			dev_dbg(dev, "No reset found. LVTS may be used by firmware.\n");
+			lvts_td->reset = NULL;
+		} else {
+			return dev_err_probe(dev, PTR_ERR(lvts_td->reset),
+					     "Failed to get reset control\n");
+		}
+	}
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -2163,6 +2184,7 @@ static const struct lvts_data mt8196_lvts_mcu_data = {
 	.num_cal_offsets = LVTS_NUM_CAL_OFFSETS_MT8196,
 	.msr_offset = LVTS_MSR_OFFSET_MT8196,
 	.ops = &lvts_platform_ops_mt8196,
+	.optional_reset = true,
 };
 
 static const struct lvts_data mt8196_lvts_ap_data = {
@@ -2175,6 +2197,7 @@ static const struct lvts_data mt8196_lvts_ap_data = {
 	.num_cal_offsets = LVTS_NUM_CAL_OFFSETS_MT8196,
 	.msr_offset = LVTS_MSR_OFFSET_MT8196,
 	.ops = &lvts_platform_ops_mt8196,
+	.optional_reset = true,
 };
 
 static const struct of_device_id lvts_of_match[] = {
-- 
2.55.0

Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Make reset optional for MT8196

[Philipp Zabel]

On Mo, 2026-07-20 at 16:42 +0200, AngeloGioacchino Del Regno wrote:
> Depending on the SoC+Firmware combination, the LVTS hardware may be
> may be actively used by one or even multiple concurrent MCUs!
> In this case, resetting it may produce either a severe slowdown of
> the entire system, or even a thermal protection AP reset, as some
> MCU(s) may be reading a very high or very low temperature while the
> LVTS is being reset.
> 
> On those, don't fail if no reset is found as that may be omitted on
> purpose, but still check if there's one, because some board(s) may
> be running on a different bootchain with reduced firmwares or using
> firmwares with reduced functionality.
> 
> Add a new "optional_reset" member to lvts_data and use it to check
> whether the resets should be mandatory or not.
> 
> Also, while at it, since devm_reset_control_get_by_index() is now
> deprecated, change the probe function to instead call function
> devm_reset_control_get_exclusive_by_index(), which does the same.

Why is this using _by_index() at all? mediatek,lvts-thermal.yaml
specifies a single reset, so the driver should just
devm_reset_control_get_exclusive(dev, NULL).

> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/thermal/mediatek/lvts_thermal.c | 29 ++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index 92711896ce24..6670b5458f2c 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -160,6 +160,7 @@ struct lvts_data {
>  	int gt_calib_bit_offset;
>  	unsigned int def_calibration;
>  	u16 msr_offset;
> +	bool optional_reset;

No need to complicate things. It's not the driver's business to check
device tree correctness. Just make the reset optional, as the commit
message says.

>  };
>  
>  struct lvts_sensor {
> @@ -1473,9 +1474,29 @@ static int lvts_probe(struct platform_device *pdev)
>  	if (IS_ERR(lvts_td->base))
>  		return dev_err_probe(dev, PTR_ERR(lvts_td->base), "Failed to map io resource\n");
>  
> -	lvts_td->reset = devm_reset_control_get_by_index(dev, 0);
> -	if (IS_ERR(lvts_td->reset))
> -		return dev_err_probe(dev, PTR_ERR(lvts_td->reset), "Failed to get reset control\n");
> +	/*
> +	 * Depending on the SoC+Firmware combination, the LVTS hardware may be
> +	 * may be actively used by one or even multiple concurrent MCUs!
> +	 * In this case, resetting it may produce either a severe slowdown of
> +	 * the entire system, or even a thermal protection AP reset, as some
> +	 * MCU(s) may be reading a very high or very low temperature while the
> +	 * LVTS is being reset.
> +	 *
> +	 * On those, don't fail if no reset is found as that may be omitted on
> +	 * purpose, but still check if there's one, because some board(s) may
> +	 * be running on a different bootchain with reduced firmwares or using
> +	 * firmwares with reduced functionality.
> +	 */
> +	lvts_td->reset = devm_reset_control_get_exclusive_by_index(dev, 0);

So this should be:

	lvts_td->reset = devm_reset_control_get_optional_exclusive(dev, NULL);

> +	if (IS_ERR(lvts_td->reset)) {
> +		if (lvts_data->optional_reset) {
> +			dev_dbg(dev, "No reset found. LVTS may be used by firmware.\n");
> +			lvts_td->reset = NULL;

This is not necessary then.

> +		} else {
> +			return dev_err_probe(dev, PTR_ERR(lvts_td->reset),
> +					     "Failed to get reset control\n");
> +		}

And this part could be kept as before.

> +	}
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0)
> @@ -2163,6 +2184,7 @@ static const struct lvts_data mt8196_lvts_mcu_data = {
>  	.num_cal_offsets = LVTS_NUM_CAL_OFFSETS_MT8196,
>  	.msr_offset = LVTS_MSR_OFFSET_MT8196,
>  	.ops = &lvts_platform_ops_mt8196,
> +	.optional_reset = true,

But mediatek,lvts-thermal.yaml still specifies the reset as required
for mediatek,mt8196-lvts-ap. That should be changed instead.

>  };
>  
>  static const struct lvts_data mt8196_lvts_ap_data = {
> @@ -2175,6 +2197,7 @@ static const struct lvts_data mt8196_lvts_ap_data = {
>  	.num_cal_offsets = LVTS_NUM_CAL_OFFSETS_MT8196,
>  	.msr_offset = LVTS_MSR_OFFSET_MT8196,
>  	.ops = &lvts_platform_ops_mt8196,
> +	.optional_reset = true,

Same as above, but for mediatek,mt8196-lvts-mcu.

regards
Philipp

Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Make reset optional for MT8196

[AngeloGioacchino Del Regno]

On 7/21/26 09:53, Philipp Zabel wrote:
> On Mo, 2026-07-20 at 16:42 +0200, AngeloGioacchino Del Regno wrote:
>> Depending on the SoC+Firmware combination, the LVTS hardware may be
>> may be actively used by one or even multiple concurrent MCUs!
>> In this case, resetting it may produce either a severe slowdown of
>> the entire system, or even a thermal protection AP reset, as some
>> MCU(s) may be reading a very high or very low temperature while the
>> LVTS is being reset.
>>
>> On those, don't fail if no reset is found as that may be omitted on
>> purpose, but still check if there's one, because some board(s) may
>> be running on a different bootchain with reduced firmwares or using
>> firmwares with reduced functionality.
>>
>> Add a new "optional_reset" member to lvts_data and use it to check
>> whether the resets should be mandatory or not.
>>
>> Also, while at it, since devm_reset_control_get_by_index() is now
>> deprecated, change the probe function to instead call function
>> devm_reset_control_get_exclusive_by_index(), which does the same.
> 
> Why is this using _by_index() at all? mediatek,lvts-thermal.yaml
> specifies a single reset, so the driver should just
> devm_reset_control_get_exclusive(dev, NULL).
> 
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/thermal/mediatek/lvts_thermal.c | 29 ++++++++++++++++++++++---
>>   1 file changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
>> index 92711896ce24..6670b5458f2c 100644
>> --- a/drivers/thermal/mediatek/lvts_thermal.c
>> +++ b/drivers/thermal/mediatek/lvts_thermal.c
>> @@ -160,6 +160,7 @@ struct lvts_data {
>>   	int gt_calib_bit_offset;
>>   	unsigned int def_calibration;
>>   	u16 msr_offset;
>> +	bool optional_reset;
> 
> No need to complicate things. It's not the driver's business to check
> device tree correctness. Just make the reset optional, as the commit
> message says.
> 
>>   };
>>   
>>   struct lvts_sensor {
>> @@ -1473,9 +1474,29 @@ static int lvts_probe(struct platform_device *pdev)
>>   	if (IS_ERR(lvts_td->base))
>>   		return dev_err_probe(dev, PTR_ERR(lvts_td->base), "Failed to map io resource\n");
>>   
>> -	lvts_td->reset = devm_reset_control_get_by_index(dev, 0);
>> -	if (IS_ERR(lvts_td->reset))
>> -		return dev_err_probe(dev, PTR_ERR(lvts_td->reset), "Failed to get reset control\n");
>> +	/*
>> +	 * Depending on the SoC+Firmware combination, the LVTS hardware may be
>> +	 * may be actively used by one or even multiple concurrent MCUs!
>> +	 * In this case, resetting it may produce either a severe slowdown of
>> +	 * the entire system, or even a thermal protection AP reset, as some
>> +	 * MCU(s) may be reading a very high or very low temperature while the
>> +	 * LVTS is being reset.
>> +	 *
>> +	 * On those, don't fail if no reset is found as that may be omitted on
>> +	 * purpose, but still check if there's one, because some board(s) may
>> +	 * be running on a different bootchain with reduced firmwares or using
>> +	 * firmwares with reduced functionality.
>> +	 */
>> +	lvts_td->reset = devm_reset_control_get_exclusive_by_index(dev, 0);
> 
> So this should be:
> 
> 	lvts_td->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
> 
>> +	if (IS_ERR(lvts_td->reset)) {
>> +		if (lvts_data->optional_reset) {
>> +			dev_dbg(dev, "No reset found. LVTS may be used by firmware.\n");
>> +			lvts_td->reset = NULL;
> 
> This is not necessary then.
> 
>> +		} else {
>> +			return dev_err_probe(dev, PTR_ERR(lvts_td->reset),
>> +					     "Failed to get reset control\n");
>> +		}
> 
> And this part could be kept as before.
> 
>> +	}
>>   
>>   	irq = platform_get_irq(pdev, 0);
>>   	if (irq < 0)
>> @@ -2163,6 +2184,7 @@ static const struct lvts_data mt8196_lvts_mcu_data = {
>>   	.num_cal_offsets = LVTS_NUM_CAL_OFFSETS_MT8196,
>>   	.msr_offset = LVTS_MSR_OFFSET_MT8196,
>>   	.ops = &lvts_platform_ops_mt8196,
>> +	.optional_reset = true,
> 
> But mediatek,lvts-thermal.yaml still specifies the reset as required
> for mediatek,mt8196-lvts-ap. That should be changed instead.
> 
>>   };
>>   
>>   static const struct lvts_data mt8196_lvts_ap_data = {
>> @@ -2175,6 +2197,7 @@ static const struct lvts_data mt8196_lvts_ap_data = {
>>   	.num_cal_offsets = LVTS_NUM_CAL_OFFSETS_MT8196,
>>   	.msr_offset = LVTS_MSR_OFFSET_MT8196,
>>   	.ops = &lvts_platform_ops_mt8196,
>> +	.optional_reset = true,
> 
> Same as above, but for mediatek,mt8196-lvts-mcu.
> 

Ack. Will send a v2 shortly.

Thanks,
Angelo