platform/x86: alienware-wmi: Refactor LED control methods
authorKurt Borja <kuurtb@gmail.com>
Fri, 7 Feb 2025 15:46:00 +0000 (10:46 -0500)
committerIlpo Järvinen <ilpo.jarvinen@linux.intel.com>
Mon, 10 Feb 2025 10:56:17 +0000 (12:56 +0200)
Both WMI devices handled by this module specify a distinct interface for
LED control. Previously this module handled this by dynamically adapting
arguments passed to wmi_evaluate_method() based on the `interface`
global variable.

To avoid the use of global variables, and enable the migration to
non-deprecated WMI methods, let the WMI drivers define upd_led and
upd_brightness operations, which completely replace
alienware_update_led() and wmax_brightness().

Also define alienware_wmi_command(), which serves as a wrapper for
wmidev_evaluate_method(). This new method is very similar to
alienware_wmax_command() but is WMI device agnostic.

Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
Link: https://lore.kernel.org/r/20250207154610.13675-5-kuurtb@gmail.com
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
drivers/platform/x86/dell/alienware-wmi.c

index 9512143..525d25a 100644 (file)
@@ -10,6 +10,7 @@
 #include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/platform_profile.h>
@@ -426,12 +427,43 @@ struct alienfx_priv {
        u8 lighting_control_state;
 };
 
+struct alienfx_ops {
+       int (*upd_led)(struct alienfx_priv *priv, struct wmi_device *wdev,
+                      u8 location);
+       int (*upd_brightness)(struct alienfx_priv *priv, struct wmi_device *wdev,
+                             u8 brightness);
+};
+
 struct alienfx_platdata {
        struct wmi_device *wdev;
+       struct alienfx_ops ops;
 };
 
 static u8 interface;
 
+static int alienware_wmi_command(struct wmi_device *wdev, u32 method_id,
+                                void *in_args, size_t in_size, u32 *out_data)
+{
+       struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL};
+       struct acpi_buffer in = {in_size, in_args};
+       acpi_status ret;
+
+       ret = wmidev_evaluate_method(wdev, 0, method_id, &in, out_data ? &out : NULL);
+       if (ACPI_FAILURE(ret))
+               return -EIO;
+
+       union acpi_object *obj __free(kfree) = out.pointer;
+
+       if (out_data) {
+               if (obj && obj->type == ACPI_TYPE_INTEGER)
+                       *out_data = (u32)obj->integer.value;
+               else
+                       return -ENOMSG;
+       }
+
+       return 0;
+}
+
 /*
  * Helpers used for zone control
  */
@@ -462,46 +494,6 @@ static int parse_rgb(const char *buf, struct color_platform *colors)
 /*
  * Individual RGB zone control
  */
-static int alienware_update_led(struct alienfx_priv *priv, u8 location)
-{
-       int method_id;
-       acpi_status status;
-       char *guid;
-       struct acpi_buffer input;
-       struct legacy_led_args legacy_args;
-       struct wmax_led_args wmax_basic_args;
-       if (interface == WMAX) {
-               wmax_basic_args.led_mask = 1 << location;
-               wmax_basic_args.colors = priv->colors[location];
-               wmax_basic_args.state = priv->lighting_control_state;
-               guid = WMAX_CONTROL_GUID;
-               method_id = WMAX_METHOD_ZONE_CONTROL;
-
-               input.length = sizeof(wmax_basic_args);
-               input.pointer = &wmax_basic_args;
-       } else {
-               legacy_args.colors = priv->colors[location];
-               legacy_args.brightness = priv->global_brightness;
-               legacy_args.state = 0;
-               if (priv->lighting_control_state == LEGACY_BOOTING ||
-                   priv->lighting_control_state == LEGACY_SUSPEND) {
-                       guid = LEGACY_POWER_CONTROL_GUID;
-                       legacy_args.state = priv->lighting_control_state;
-               } else
-                       guid = LEGACY_CONTROL_GUID;
-               method_id = location + 1;
-
-               input.length = sizeof(legacy_args);
-               input.pointer = &legacy_args;
-       }
-       pr_debug("alienware-wmi: guid %s method %d\n", guid, method_id);
-
-       status = wmi_evaluate_method(guid, 0, method_id, &input, NULL);
-       if (ACPI_FAILURE(status))
-               pr_err("alienware-wmi: zone set failure: %u\n", status);
-       return ACPI_FAILURE(status);
-}
-
 static ssize_t zone_show(struct device *dev, struct device_attribute *attr,
                         char *buf, u8 location)
 {
@@ -518,13 +510,14 @@ static ssize_t zone_store(struct device *dev, struct device_attribute *attr,
 {
        struct alienfx_priv *priv = dev_get_drvdata(dev);
        struct color_platform *colors = &priv->colors[location];
+       struct alienfx_platdata *pdata = dev_get_platdata(dev);
        int ret;
 
        ret = parse_rgb(buf, colors);
        if (ret)
                return ret;
 
-       ret = alienware_update_led(priv, location);
+       ret = pdata->ops.upd_led(priv, pdata->wdev, location);
 
        return ret ? ret : count;
 }
@@ -660,36 +653,17 @@ static struct attribute_group zone_attribute_group = {
 /*
  * LED Brightness (Global)
  */
-static int wmax_brightness(int brightness)
-{
-       acpi_status status;
-       struct acpi_buffer input;
-       struct wmax_brightness_args args = {
-               .led_mask = 0xFF,
-               .percentage = brightness,
-       };
-       input.length = sizeof(args);
-       input.pointer = &args;
-       status = wmi_evaluate_method(WMAX_CONTROL_GUID, 0,
-                                    WMAX_METHOD_BRIGHTNESS, &input, NULL);
-       if (ACPI_FAILURE(status))
-               pr_err("alienware-wmi: brightness set failure: %u\n", status);
-       return ACPI_FAILURE(status);
-}
-
 static void global_led_set(struct led_classdev *led_cdev,
                           enum led_brightness brightness)
 {
        struct alienfx_priv *priv = container_of(led_cdev, struct alienfx_priv,
                                                 global_led);
+       struct alienfx_platdata *pdata = dev_get_platdata(&priv->pdev->dev);
        int ret;
 
        priv->global_brightness = brightness;
 
-       if (interface == WMAX)
-               ret = wmax_brightness(brightness);
-       else
-               ret = alienware_update_led(priv, 0);
+       ret = pdata->ops.upd_brightness(priv, pdata->wdev, brightness);
        if (ret)
                pr_err("LED brightness update failed\n");
 }
@@ -1249,10 +1223,49 @@ static int alienware_alienfx_setup(struct alienfx_platdata *pdata)
 /*
  * Legacy WMI driver
  */
+static int legacy_wmi_update_led(struct alienfx_priv *priv,
+                                struct wmi_device *wdev, u8 location)
+{
+       struct legacy_led_args legacy_args = {
+               .colors = priv->colors[location],
+               .brightness = priv->global_brightness,
+               .state = 0,
+       };
+       struct acpi_buffer input;
+       acpi_status status;
+
+       if (legacy_args.state != LEGACY_RUNNING) {
+               legacy_args.state = priv->lighting_control_state;
+
+               input.length = sizeof(legacy_args);
+               input.pointer = &legacy_args;
+
+               status = wmi_evaluate_method(LEGACY_POWER_CONTROL_GUID, 0,
+                                            location + 1, &input, NULL);
+               if (ACPI_FAILURE(status))
+                       return -EIO;
+
+               return 0;
+       }
+
+       return alienware_wmi_command(wdev, location + 1, &legacy_args,
+                                    sizeof(legacy_args), NULL);
+}
+
+static int legacy_wmi_update_brightness(struct alienfx_priv *priv,
+                                       struct wmi_device *wdev, u8 brightness)
+{
+       return legacy_wmi_update_led(priv, wdev, 0);
+}
+
 static int legacy_wmi_probe(struct wmi_device *wdev, const void *context)
 {
        struct alienfx_platdata pdata = {
                .wdev = wdev,
+               .ops = {
+                       .upd_led = legacy_wmi_update_led,
+                       .upd_brightness = legacy_wmi_update_brightness,
+               },
        };
 
        return alienware_alienfx_setup(&pdata);
@@ -1287,10 +1300,39 @@ static void __exit alienware_legacy_wmi_exit(void)
 /*
  * WMAX WMI driver
  */
+static int wmax_wmi_update_led(struct alienfx_priv *priv,
+                              struct wmi_device *wdev, u8 location)
+{
+       struct wmax_led_args in_args = {
+               .led_mask = 1 << location,
+               .colors = priv->colors[location],
+               .state = priv->lighting_control_state,
+       };
+
+       return alienware_wmi_command(wdev, WMAX_METHOD_ZONE_CONTROL, &in_args,
+                                    sizeof(in_args), NULL);
+}
+
+static int wmax_wmi_update_brightness(struct alienfx_priv *priv,
+                                     struct wmi_device *wdev, u8 brightness)
+{
+       struct wmax_brightness_args in_args = {
+               .led_mask = 0xFF,
+               .percentage = brightness,
+       };
+
+       return alienware_wmi_command(wdev, WMAX_METHOD_BRIGHTNESS, &in_args,
+                                    sizeof(in_args), NULL);
+}
+
 static int wmax_wmi_probe(struct wmi_device *wdev, const void *context)
 {
        struct alienfx_platdata pdata = {
                .wdev = wdev,
+               .ops = {
+                       .upd_led = wmax_wmi_update_led,
+                       .upd_brightness = wmax_wmi_update_brightness,
+               },
        };
        int ret;