Re: [PATCH 1/6] platform/x86: msi-wmi-platform: Move guard out of switch in platform_write()
From: Mario Limonciello
Date: Mon Oct 26 2026 - 11:27:32 EST
On 10/24/26 04:24, Antheas Kapenekakis wrote:
On Wed, 21 Oct 2026 at 05:26, Derek John Clark
<derekjohn.clark@xxxxxxxxx> wrote:
On Tue, Oct 20, 2026 at 2:53 PM Antheas Kapenekakis <lkml@xxxxxxxxxxx> wrote:
On Tue, 18 Aug 2026 at 17:26, Antheas Kapenekakis <lkml@xxxxxxxxxxx> wrote:
On Tue, 18 Aug 2026 at 16:42, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On 8/18/26 07:12, Ilpo Järvinen wrote:
On Mon, 3 Aug 2026, Derek J. Clark wrote:
The ML patch set uses a guard within a switch case that violates cleanup
rules. Move the guard outside the switch so all return paths are covered.
Signed-off-by: Derek J. Clark <derekjohn.clark@xxxxxxxxx>
---
drivers/platform/x86/msi-wmi-platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/msi-wmi-platform.c b/drivers/platform/x86/msi-wmi-platform.c
index dfb65ac8fbf6..33acd448f674 100644
--- a/drivers/platform/x86/msi-wmi-platform.c
+++ b/drivers/platform/x86/msi-wmi-platform.c
@@ -675,11 +675,11 @@ static int msi_wmi_platform_write(struct device *dev, enum hwmon_sensor_types ty
u8 buffer[32] = { };
int ret;
+ guard(mutex)(&data->wmi_lock);
switch (type) {
case hwmon_pwm:
switch (attr) {
case hwmon_pwm_enable:
- guard(mutex)(&data->wmi_lock);
While I cannot see the code easily, one could also add braces to inside
the case to avoid this problem.
I don't see the code either, but wouldn't this be a use case for
scoped_guard() ?
Hi, not explicitly stated in the cover letter, but this is essentially
a series on top of the series I sent last year but I did not clean up
due to a variety of issues with MSI firmware that would have made the
driver unsuitable for upstreaming (to my standards at least). These
issues span Intel driver problems, HID controller instability, and
varying WMI implementations esp. in regards to PLx setting between AMD
/ Intel variants, shift mode behavior not acting correctly, and fan
curve application / resetting being weird (the fan curve on off bit
does not seem to change whether an automatic curve is used but how
close the device alings to the set curve? requiring us to back up and
restore the original curve? half of the devices keep the curve during
sleep and half don't? but if the fan curve is so finicky do we really
want a resume handler for it? Perhaps we can kick the timing quirks to
userspace)
For this reason, I focused on other manufacturers such as Ayaneo and
OneXPlayer. I will revisit this series per request from Derek as he
seems to want this for steamos and hid-msi is already queued for 7.3?
Although, to be honest, compared to the controller firmware issues,
the WMI implementation is pristine.
So I will need to go through these issues 1 by 1 and see which can be
fixed and either drop certain features in the beginning.
Particularly for the fan curves, if we say that resume handlers are
required to be in the kernel moving forward, I do not know how
realistic it is to make those reliable for this WMI device family.
This driver still needs work. I am getting bug reports about
inconsistent TDP application on the EX. A8 is broken as well. Even
with your patches Derek. I need to grab a new version of center M and
see what they are doing.
Does this driver work in your experience? What is that SMT toggle you added
I've got a few reports of some issues, but nothing with much detail. I
have the 8 AI+, 8 EX AI+, and A8 in hand and all three had effective
TDP changes in SteamOS with these patches applied and SteamOS Manager.
I haven't done extensive testing over multiple suspends or longer
sessions myself. I'm also not aware if there have been BIOS updates
since my testing.
By SMT toggle i think you mean the STAPM toggle for the A8? It was
referenced in API_UserScenario.dll (setPowerLimit_AMDSTAMP) which I
reviewed using ILSpy. After toggling that TDP started reporting
correctly and being settable.
Hope this helps. LMK if you need anything tested or additional info.
Ok, I asked for more feedback. Yes, TDP changes do work, but they are
not correct. Users AB testing with a Decky plugin called
SimpleDeckyTDP report major divergence.
SimpleDeckyTDP uses a combination of RyzenAdj for AMD and RAPL for
Intel. When set to 7W, users report 25W power consumption when using
the kernel driver and 15W when using simpledeckytdp on A8. 15W is the
correct value for a 7W envelope on an X840u APU.
STAPM should not be required to use or deviate from STT. Both of them
should have equivalent behavior and match ryzenadj as the underlying
DPTC hooks are the same, ryzenadj just accesses the mailbox directly.
Using STAPM is a workaround for buggy vendor firmware that does not
control boost values correctly (SPPT/FPPT) and is what the first
generation Lenovo Legion Go did as well before they fixed it.
STAPM as a workaround works because it uses timers to control boost
values, so they decay after 2-3 minutes to the steady state TDP set by
the firmware. STT boosts for as long as the device thermal envelope
allows. STAPM is deprecated by AMD, only STT should be used going
forward. +CC Mario
It's up to the OEM to decide whether they have programmed their firmware to to use STT or STAPM. They should not be used at the same time.
If their BIOS/EC is using one and reverse engineered software is using the other, you will expect problems.
This points to a BIOS firmware issue. You should consider getting in
touch with your MSI contact for feedback and to plan a fix. I do not
have an MSI contact. And yes, I am not happy my users have to rely on
third party unvetted plugins for basic power controls.
But we cannot revisit this driver while basic behavior. does not work.
If MSI does not fix this or there isn't a bug in this driver we can
fix, the only viable way forward from our side is to move power
controls to the DPTC RFC driver/RAPL for affected models and only
retain fan curves on this driver. I do not want to do this, I have
userspace integration for this driver already, but it is not
functional to the standard expected by users.
I checked the ACPI firmware and unlike other manufacturers, there is
no EC independent control of the power envelope. We would still need
to check a low battery hook in firmware but other than that it is a
straight translation.
Antheas