thermal: int340x: fix tcc offset on resume

After upgrading to Linux 5.13.3 I noticed my laptop would shutdown due
to overheat (when it should not). It turned out this was due to commit
fe6a6de6692e ("thermal/drivers/int340x/processor_thermal: Fix tcc setting").

What happens is this drivers uses a global variable to keep track of the
tcc offset (tcc_offset_save) and uses it on resume. The issue is this variable is initialized to 0, but is only set in tcc_offset_degree_celsius_store, i.e. when the tcc offset is explicitly
set by userspace. If that does not happen, the resume path will set the
offset to 0 (in my case the h/w default being 3, the offset would become
too low after a suspend/resume cycle).

The issue did not arise before commit fe6a6de6692e, as the function setting the offset would return if the offset was 0. This is no longer
the case (rightfully).

Fix this by not applying the offset if it wasn't saved before, reverting
back to the old logic. A better approach will come later, but this will
be easier to apply to stable kernels.

The logic to restore the offset after a resume was there long before commit fe6a6de6692e, but as a value of 0 was considered invalid I'm referencing the commit that made the issue possible in the Fixes tag instead.

Fixes fe6a6de6692e ("thermal/drivers/int340x/processor_thermal: Fix

tcc setting")

Cc [email protected]

Cc Srinivas Pandruvada <[email protected]>

Signed-off-by Antoine Tenart <[email protected]>

---
 .../thermal/intel/int340x_thermal/processor_thermal_device.c | 5
+++--

 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c index 0f0038af2ad4..fb64acfd5e07 100644
---
a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
+++
b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c

(quoted diff) +0 -0

@@ -107,7 +107,7 @@ static int tcc_offset_update(unsigned int tcc)
         return 0;
  }

drivers/thermal/intel/int340x_thermal/processor_thermal_device.c +1 -1

-static unsigned int tcc_offset_save;
+static int tcc_offset_save = -1;

 static ssize_t tcc_offset_degree_celsius_store(struct device *dev, struct device_attribute *attr, const char *buf,

drivers/thermal/intel/int340x_thermal/processor_thermal_device.c +0 -0

@@ -352,7 +352,8 @@ int proc_thermal_resume(struct device *dev)
         proc_dev = dev_get_drvdata(dev);
         proc_thermal_read_ppcc(proc_dev);

-       tcc_offset_update(tcc_offset_save); +       if (tcc_offset_save >= 0)
+               tcc_offset_update(tcc_offset_save);
        return 0;
 }

Hi Daniel,

This patch is important. Can we send for 5.15 rc release?

I see the previous version of this patch is applied to linux-next. But this series is better as it splits into two patches. The first one can be easily backported and will fix the problem. The second one is an improvement.

Quoted 10 lines · Srinivas Pandruvada

Hi Daniel,

This patch is important. Can we send for 5.15 rc release?

I see the previous version of this patch is applied to linux-next. But this series is better as it splits into two patches. The first one
can be easily backported and will fix the problem. The second one is an
improvement.

Yes, it is in the pipe.

I've applied the patch 1/2 to the fixes branch and the patch 2/2 will land in the next branch as soon as the next -rc is released with the fix
and merged to the next branch.

I've applied the patch 1/2 to the fixes branch and the patch 2/2 will land in the next branch as soon as the next -rc is released with the fix and merged to the next branch.

Hello Daniel,

Quoting Daniel Lezcano (2021-09-24 19:40:13)

Quoted 4 lines

I've applied the patch 1/2 to the fixes branch and the patch 2/2 will land in the next branch as soon as the next -rc is released with the fix and merged to the next branch.

I don't see it in thermal/next even though patch 1 has made it. Not sure if patch 2 has slipped through the cracks or wasn't pushed yet. If it's the later, please ignore this mail.

Quoted 6 lines

Quoting Daniel Lezcano (2021-09-24 19:40:13)

Quoted 4 lines

I've applied the patch 1/2 to the fixes branch and the patch 2/2 will land in the next branch as soon as the next -rc is released with the fix and merged to the next branch.

Quoted 3 lines

I don't see it in thermal/next even though patch 1 has made it. Not sure if patch 2 has slipped through the cracks or wasn't pushed yet. If it's the later, please ignore this mail.

Indeed, I thougth I picked it but it wasn't.

Thanks for the head up, it is applied now.

Hi,

This is a follow-up of [1]. As discussed in the initial thread, the fix is now split into two commits: the two commits combined are fixing the issue in a nice way for 5.15, but only the first one is meant to be applied on stable kernels (to avoid conflicts).

The issue this is fixing is described in patch 1. Tl;dr: a suspend/resume cycle can end-up in setting a wrong tcc offset value, which can lead to machines shutting down because of overheating (when they should not).

I've stripped the Acked-by/Tested-by flags from v1 as the patch was split. But the end result for 5.15+ kernels should be the same.

Thanks,
Antoine

[1]
https://lore.kernel.org/linux-pm/[email protected]/T/

Antoine Tenart (2):
  thermal: int340x: do not set a wrong tcc offset on resume
  thermal: int340x: improve the tcc offset saving for suspend/resume

 .../intel/int340x_thermal/int3401_thermal.c   |  8 +++- .../processor_thermal_device.c                | 39 ++++++++++++++----- .../processor_thermal_device.h                |  1 + .../processor_thermal_device_pci.c            | 18 ++++++++- .../processor_thermal_device_pci_legacy.c     |  8 +++-

 5 files changed, 62 insertions(+), 12 deletions(-)

Hi,

This is a follow-up of [1]. As discussed in the initial thread, the fix is now split into two commits: the two commits combined are fixing the issue in a nice way for 5.15, but only the first one is meant to be applied on stable kernels (to avoid conflicts).

The issue this is fixing is described in patch 1. Tl;dr: a suspend/resume cycle can end-up in setting a wrong tcc offset value, which can lead to machines shutting down because of overheating (when they should not).

I've stripped the Acked-by/Tested-by flags from v1 as the patch was split. But the end result for 5.15+ kernels should be the same.

Thanks,
Antoine

[1] https://lore.kernel.org/linux-pm/[email protected]/T/

Antoine Tenart (2):
thermal: int340x: do not set a wrong tcc offset on resume
thermal: int340x: improve the tcc offset saving for suspend/resume

 .../intel/int340x_thermal/int3401_thermal.c   |  8 +++-
 .../processor_thermal_device.c                | 39 ++++++++++++++-----
 .../processor_thermal_device.h                |  1 +
 .../processor_thermal_device_pci.c            | 18 ++++++++-
 .../processor_thermal_device_pci_legacy.c     |  8 +++-
 5 files changed, 62 insertions(+), 12 deletions(-)