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

Ack. Will send a v2 shortly.

Thanks,
Angelo