On Tue, Nov 26, 2024 at 5:21 AM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
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.
This sounds weird to me. On my end, the symptom is that the device sometimes cannot suspend. To be more precise, `echo mem > /sys/power/state` returns almost immediately. I think the irq is more likely to be triggered during suspension.
Disable the temperature monitoring and give some time for the signals to stabilize during suspend in order to prevent such spurious interrupts.
Cc: stable@vger.kernel.org Reported-by: Hsin-Te Yuan yuanhsinte@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@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;if (enable) {lvts_for_each_valid_sensor(i, lvts_ctrl)sensor_map |= sensor_filt_bitmap[i];}/** Bits:* 9: Single point access flow* 0-3: Enable sensing point 0-3*/writel(sensor_map | BIT(9), LVTS_MONCTL0(lvts_ctrl->base));+}
/*
- At this point the configuration register is the only place in the
- driver where we write multiple values. Per hardware constraint,
@@ -1381,8 +1407,11 @@ static int lvts_suspend(struct device *dev)
lvts_td = dev_get_drvdata(dev);
for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {lvts_ctrl_monitor_enable(dev, &lvts_td->lvts_ctrl[i], false);usleep_range(100, 200); lvts_ctrl_set_enable(&lvts_td->lvts_ctrl[i], false);} clk_disable_unprepare(lvts_td->clk);@@ -1400,8 +1429,11 @@ static int lvts_resume(struct device *dev) if (ret) return ret;
for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
for (i = 0; i < lvts_td->num_lvts_ctrl; i++) { lvts_ctrl_set_enable(&lvts_td->lvts_ctrl[i], true);usleep_range(100, 200);lvts_ctrl_monitor_enable(dev, &lvts_td->lvts_ctrl[i], true);} return 0;}
-- 2.47.0