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 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.

Best,
Antheas


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