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

[Antheas Kapenekakis]

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

Antheas

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