[PATCH 1/5] thermal/drivers/mediatek/lvts: Disable monitor mode during suspend

[Nícolas F. R. A. Prado]

On Tue, Nov 26, 2024 at 10:43:55AM +0100, AngeloGioacchino Del Regno wrote:
> Il 25/11/24 22:20, Nícolas F. R. A. Prado ha scritto:
> > When configured in filtered mode, the LVTS thermal controller will
> > monitor the temperature from the sensors and trigger an interrupt once a
> > thermal threshold is crossed.
> > 
> > Currently this is true even during suspend and resume. The problem with
> > that is that when enabling the internal clock of the LVTS controller in
> > lvts_ctrl_set_enable() during resume, the temperature reading can glitch
> > and appear much higher than the real one, resulting in a spurious
> > interrupt getting generated.
> > 
> > Disable the temperature monitoring and give some time for the signals to
> > stabilize during suspend in order to prevent such spurious interrupts.
> > 
> > Cc: stable at vger.kernel.org
> > Reported-by: Hsin-Te Yuan <yuanhsinte at chromium.org>
> > Closes: https://lore.kernel.org/all/20241108-lvts-v1-1-eee339c6ca20@chromium.org/
> > Fixes: 8137bb90600d ("thermal/drivers/mediatek/lvts_thermal: Add suspend and resume")
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> > ---
> >   drivers/thermal/mediatek/lvts_thermal.c | 36 +++++++++++++++++++++++++++++++--
> >   1 file changed, 34 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> > index 1997e91bb3be94a3059db619238aa5787edc7675..a92ff2325c40704adc537af6995b34f93c3b0650 100644
> > --- a/drivers/thermal/mediatek/lvts_thermal.c
> > +++ b/drivers/thermal/mediatek/lvts_thermal.c
> > @@ -860,6 +860,32 @@ static int lvts_ctrl_init(struct device *dev, struct lvts_domain *lvts_td,
> >   	return 0;
> >   }
> > +static void lvts_ctrl_monitor_enable(struct device *dev, struct lvts_ctrl *lvts_ctrl, bool enable)
> > +{
> > +	/*
> > +	 * Bitmaps to enable each sensor on filtered mode in the MONCTL0
> > +	 * register.
> > +	 */
> > +	u32 sensor_filt_bitmap[] = { BIT(0), BIT(1), BIT(2), BIT(3) };
> > +	u32 sensor_map = 0;
> > +	int i;
> > +
> > +	if (lvts_ctrl->mode != LVTS_MSR_FILTERED_MODE)
> > +		return;
> > +
> 
> That's easier and shorter:
> 
> static void lvts_ctrl_monitor_enable( .... )
> {
> 	/* Bitmap to enable each sensor on filtered mode in the MONCTL0 register */
> 	const u32 sensor_map = GENMASK(3, 0);
> 
> 	if (lvts_ctrl->mode != LVTS_MSR_FILTERED_MODE)
> 		return;
> 
> 	/* Bits 0-3: Sensing points - Bit 9: Single point access flow */
> 	if (enable)
> 		writel(sensor_map | BIT(9), LVTS_MONCTL0(lvts_ctrl->base));

Wait, no, here you're enabling all the sensors in the controller. We only want
to enable ones that are valid, otherwise we might get garbage data and irqs from
sensors that aren't actually there. That's why I use the
lvts_for_each_valid_sensor() helper in this patch.

Thanks,
Nícolas