Re: [PATCH] platform/x86: thinkpad_acpi: Only update profile if successfully converted

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 2/19/2024 09:13, Mark Pearson wrote:
Hi Mario

On Fri, Feb 16, 2024, at 9:23 PM, Mario Limonciello wrote:
Randomly a Lenovo Z13 will trigger a kernel warning traceback from this

if (WARN_ON((profile < 0) || (profile >= ARRAY_SIZE(profile_names))))

This happens because thinkpad-acpi always assumes that
convert_dytc_to_profile() successfully updated the profile. On the
contrary a condition can occur that when dytc_profile_refresh() is called
the profile doesn't get updated as there is a -EOPNOTSUPP branch.

Catch this situation and avoid updating the profile. Also log this into
dynamic debugging in case any other modes should be added in the future.

Fixes: c3bfcd4c6762 ("platform/x86: thinkpad_acpi: Add platform profile
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
BTW - This isn't new.  I've been seeing this a long time, but I just
got annoyed enough by it to find the code that triggered the sequence.

I've never seen this on my systems - is there anything in particular that can be used to reproduce the issue? I'll follow up with the FW team as setting the profile shouldn't (to my knowledge) fail. Agreed it should be handled if it does fail though.

I think it's mostly at bootup I've seen it.

With dynamic debug turned on I can see the "Unknown function" prints 0x0.

  drivers/platform/x86/thinkpad_acpi.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c
index c4895e9bc714..5ecd9d33250d 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -10308,6 +10308,7 @@ static int convert_dytc_to_profile(int
funcmode, int dytcmode,
  		return 0;
  		/* Unknown function */
+		pr_debug("unknown function 0x%x\n", funcmode);
  		return -EOPNOTSUPP;
  	return 0;
@@ -10493,8 +10494,8 @@ static void dytc_profile_refresh(void)

  	perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
-	convert_dytc_to_profile(funcmode, perfmode, &profile);
-	if (profile != dytc_current_profile) {
+	err = convert_dytc_to_profile(funcmode, perfmode, &profile);
+	if (!err && profile != dytc_current_profile) {
  		dytc_current_profile = profile;

Looks good to me. Thank you!
Reviewed-by Mark Pearson <mpearson-lenovo@xxxxxxxxx>


[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux