Re: [PATCH 1/6] platform/x86: msi-wmi-platform: Move guard out of switch in platform_write()

[Derek John Clark]

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.

Thanks,
Derek
> Antheas
>
> > Best,
> > Antheas
> >
> >
> > > Guenter
> > >
> > > >>
> > > >>                      buffer[0] = MSI_PLATFORM_AP_SUBFEATURE_FAN_MODE;
> > > >>                      ret = msi_wmi_platform_query_unlocked(
> > > >>
> > > >
> > >
> > >
>