Re: [PATCH 1/6] platform/x86: msi-wmi-platform: Move guard out of switch in platform_write()
Antheas Kapenekakis <[email protected]>
| Newsgroups | org.kernel.vger.linux-kernel,org.kernel.vger.linux-doc,org.kernel.vger.linux-hwmon,org.kernel.vger.platform-driver-x86 |
|---|---|
| Message-ID | <CAGwozwGdwhE6Sm6oa2uxx_sHfFNQ6fHYnAhYHkEPQcp_Yb=Z6w@mail.gmail.com> |
On Tue, 18 Aug 2026 at 16:42, Guenter Roeck <[email protected]> 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 <[email protected]> > >> --- > >> 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( > >> > > > >