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@xxxxxxxxxxxxx>
> ---
>  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