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

[Derek John Clark]

On August 18, 2026 7:42:40 AM PDT, 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() ?
>
>Guenter
>
Hi Ilpo, Guenter,

Antheas has indicated he's already tracking adding {} to the guard in
his response to my cover letter. I agree with Guenter that
scoped_guard would be a reasonable adjustment to this change.

For context, here is the full switch case, derived from [PATCH v1
05/10] platform/x86: msi-wmi-platform: Add platform profile through
shift mode in the parent series:

+ switch (type) {
+ case hwmon_fan:
+ switch (attr) {
+ case hwmon_fan_input:
+ buffer[0] = MSI_PLATFORM_FAN_SUBFEATURE_FAN_SPEED;
+ ret = msi_wmi_platform_query(data, MSI_PLATFORM_GET_FAN, buffer,
+     sizeof(buffer));
+ if (ret < 0)
+ return ret;
+
+ value = get_unaligned_be16(&buffer[channel * 2 + 1]);
+ if (!value)
+ *val = 0;
+ else
+ *val = 480000 / value;
+
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+ case hwmon_pwm:
+ switch (attr) {
+ case hwmon_pwm_enable:
+ buffer[0] = MSI_PLATFORM_AP_SUBFEATURE_FAN_MODE;
+ ret = msi_wmi_platform_query(data, MSI_PLATFORM_GET_AP, buffer,
+     sizeof(buffer));
+ if (ret < 0)
+ return ret;
+
+ flags = buffer[MSI_PLATFORM_AP_FAN_FLAGS_OFFSET];
+ if (flags & MSI_PLATFORM_AP_ENABLE_FAN_TABLES)
+ *val = 1;
+ else
+ *val = 2;
+
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+ default:
+ return -EOPNOTSUPP;
+ }
+}

Thanks,
Derek



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