Re: [PATCH 08/20] alienware-wmi: Add WMI Drivers

Armin Wolf <W_Armin@xxxxxx> · Sun, 29 Dec 2024 01:26:36 +0100

Am 27.12.24 um 05:44 schrieb Kurt Borja:

On Fri, Dec 27, 2024 at 04:21:14AM +0100, Armin Wolf wrote:
Am 21.12.24 um 06:59 schrieb Kurt Borja:

Add WMI drivers for LEGACY and WMAX devices.

This involves moving platform driver and device registration to a helper
function, which is now called from the driver's preferred WMI device
driver probe. However this is only done if !quirks->thermal because
newer WMAX interface doesn't support any of the features exposed by this
device.

Only one driver is registered on module initialization to prevent
registering duplicate platform driver and device.

Aditionally, create_thermal_profile() now takes wmi_device * instead of
platform_device *.
Since the HDMI, Amplifier and deepslp groups depend on the WMAX interface i would
register them inside wmax_wmi_probe() using device_add_groups() (you might need to
rework alienware_alienfx_setup() for that).

In the end the only code shared between both drivers should be related to the LED handling.
Everything else AFAIK depends on the WMAX interface and should therefore be inside the
WMAX driver.
Unfortunately this breaks old user-space interface, as the path to this
attributes would change :(. This is why I can't even set a parent dev
for the platform_device.

I only know of one project which interefaces with this driver [1], and
it does use the /sys/devices/platform/alienware-wmi path. Maybe I could
send a pull req, but it would still break old unknown projects and
scripts.

[1] https://crates.io/crates/alienware

I meant that you use device_add_groups() on the platform device, not on the WMI device itself.
Since alienware_alienfx_setup() stores a pointer to the platform device using dev_set_drvdata() the WMI driver
can use dev_get_drvdata() to access the platform device.

Thanks,
Armin Wolf

Thanks,
Armin Wolf

Signed-off-by: Kurt Borja <kuurtb@xxxxxxxxx>
---
   drivers/platform/x86/dell/alienware-wmi.c | 186 +++++++++++++++++-----
   1 file changed, 146 insertions(+), 40 deletions(-)

diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index 4b68d64bd742..f2f6842e27e6 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -15,6 +15,7 @@
   #include <linux/platform_profile.h>
   #include <linux/dmi.h>
   #include <linux/leds.h>
+#include <linux/wmi.h>

   #define LEGACY_CONTROL_GUID		"A90597CE-A997-11DA-B012-B622A1EF5492"
   #define LEGACY_POWER_CONTROL_GUID	"A80593CE-A997-11DA-B012-B622A1EF5492"
@@ -39,8 +40,6 @@
   MODULE_AUTHOR("Mario Limonciello <mario.limonciello@xxxxxxxxxxx>");
   MODULE_DESCRIPTION("Alienware special feature control");
   MODULE_LICENSE("GPL");
-MODULE_ALIAS("wmi:" LEGACY_CONTROL_GUID);
-MODULE_ALIAS("wmi:" WMAX_CONTROL_GUID);

   static bool force_platform_profile;
   module_param_unsafe(force_platform_profile, bool, 0);
@@ -412,7 +411,10 @@ struct alienfx_priv {
   	u8 lighting_control_state;
   };

-static struct platform_device *platform_device;
+struct alienfx_platdata {
+	struct wmi_device *wdev;
+};
+
   static struct platform_profile_handler pp_handler;
   static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];

@@ -1048,7 +1050,7 @@ static int thermal_profile_set(struct platform_profile_handler *pprof,
   	return wmax_thermal_control(supported_thermal_profiles[profile]);
   }

-static int create_thermal_profile(struct platform_device *platform_device)
+static int create_thermal_profile(struct wmi_device *wdev)
   {
   	enum platform_profile_option profile;
   	enum wmax_thermal_mode mode;
@@ -1097,7 +1099,7 @@ static int create_thermal_profile(struct platform_device *platform_device)
   	pp_handler.profile_get = thermal_profile_get;
   	pp_handler.profile_set = thermal_profile_set;
   	pp_handler.name = "alienware-wmi";
-	pp_handler.dev = &platform_device->dev;
+	pp_handler.dev = &wdev->dev;

   	return platform_profile_register(&pp_handler);
   }
@@ -1153,19 +1155,138 @@ static struct platform_driver platform_driver = {
   	.probe = alienfx_probe,
   };

-static int __init alienware_wmi_init(void)
+static int alienware_alienfx_setup(struct alienfx_platdata *pdata)
   {
+	struct platform_device *pdev;
   	int ret;

-	if (wmi_has_guid(LEGACY_CONTROL_GUID))
-		interface = LEGACY;
-	else if (wmi_has_guid(WMAX_CONTROL_GUID))
-		interface = WMAX;
-	else {
-		pr_warn("alienware-wmi: No known WMI GUID found\n");
-		return -ENODEV;
+	ret = platform_driver_register(&platform_driver);
+	if (ret < 0)
+		return ret;
+
+	pdev = platform_device_register_data(NULL, "alienware-wmi",
+					     PLATFORM_DEVID_NONE, pdata,
+					     sizeof(*pdata));
+
+	if (IS_ERR(pdev)) {
+		platform_driver_unregister(&platform_driver);
+		return PTR_ERR(pdev);
   	}

+	dev_set_drvdata(&pdata->wdev->dev, pdev);
+
+	return 0;
+}
+
+static void alienware_alienfx_exit(struct wmi_device *wdev)
+{
+	struct platform_device *pdev;
+
+	pdev = dev_get_drvdata(&wdev->dev);
+
+	platform_device_unregister(pdev);
+	platform_driver_unregister(&platform_driver);
+}
+
+/*
+ * Legacy WMI driver
+ */
+static int legacy_wmi_probe(struct wmi_device *wdev, const void *context)
+{
+	struct alienfx_platdata pdata = {
+		.wdev = wdev,
+	};
+
+	return alienware_alienfx_setup(&pdata);
+}
+
+static void legacy_wmi_remove(struct wmi_device *wdev)
+{
+	alienware_alienfx_exit(wdev);
+}
+
+static struct wmi_device_id alienware_legacy_device_id_table[] = {
+	{ LEGACY_CONTROL_GUID, NULL },
+	{ },
+};
+MODULE_DEVICE_TABLE(wmi, alienware_legacy_device_id_table);
+
+static struct wmi_driver alienware_legacy_wmi_driver = {
+	.driver = {
+		.name = "alienware-wmi-alienfx",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+	.id_table = alienware_legacy_device_id_table,
+	.probe = legacy_wmi_probe,
+	.remove = legacy_wmi_remove,
+};
+
+static int __init alienware_legacy_wmi_init(void)
+{
+	return wmi_driver_register(&alienware_legacy_wmi_driver);
+}
+
+static void __exit alienware_legacy_wmi_exit(void)
+{
+	wmi_driver_unregister(&alienware_legacy_wmi_driver);
+}
+
+/*
+ * WMAX WMI driver
+ */
+static int wmax_wmi_probe(struct wmi_device *wdev, const void *context)
+{
+	struct alienfx_platdata pdata = {
+		.wdev = wdev,
+	};
+	int ret = 0;
+
+	if (quirks->thermal)
+		ret = create_thermal_profile(wdev);
+	else
+		ret = alienware_alienfx_setup(&pdata);
+
+	return ret;
+}
+
+static void wmax_wmi_remove(struct wmi_device *wdev)
+{
+	if (quirks->thermal)
+		remove_thermal_profile();
+	else
+		alienware_alienfx_exit(wdev);
+}
+
+static struct wmi_device_id alienware_wmax_device_id_table[] = {
+	{ WMAX_CONTROL_GUID, NULL },
+	{ },
+};
+MODULE_DEVICE_TABLE(wmi, alienware_wmax_device_id_table);
+
+static struct wmi_driver alienware_wmax_wmi_driver = {
+	.driver = {
+		.name = "alienware-wmi-wmax",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+	.id_table = alienware_wmax_device_id_table,
+	.probe = wmax_wmi_probe,
+	.remove = wmax_wmi_remove,
+};
+
+static int __init alienware_wmax_wmi_init(void)
+{
+	return wmi_driver_register(&alienware_wmax_wmi_driver);
+}
+
+static void __exit alienware_wmax_wmi_exit(void)
+{
+	wmi_driver_unregister(&alienware_wmax_wmi_driver);
+}
+
+static int __init alienware_wmi_init(void)
+{
+	int ret;
+
   	dmi_check_system(alienware_quirks);
   	if (quirks == NULL)
   		quirks = &quirk_unknown;
@@ -1180,43 +1301,28 @@ static int __init alienware_wmi_init(void)
   			pr_warn("force_gmode requires platform profile support\n");
   	}

-	ret = platform_driver_register(&platform_driver);
-	if (ret)
-		goto fail_platform_driver;
-	platform_device = platform_device_alloc("alienware-wmi", PLATFORM_DEVID_NONE);
-	if (!platform_device) {
-		ret = -ENOMEM;
-		goto fail_platform_device1;
+	if (wmi_has_guid(WMAX_CONTROL_GUID)) {
+		interface = WMAX;
+		ret = alienware_wmax_wmi_init();
+	} else {
+		interface = LEGACY;
+		ret = alienware_legacy_wmi_init();
   	}
-	ret = platform_device_add(platform_device);
-	if (ret)
-		goto fail_platform_device2;

-	if (quirks->thermal) {
-		ret = create_thermal_profile(platform_device);
-		if (ret)
-			goto fail_prep_thermal_profile;
-	}
+	if (ret < 0)
+		return ret;

   	return 0;
-
-fail_prep_thermal_profile:
-	platform_device_del(platform_device);
-fail_platform_device2:
-	platform_device_put(platform_device);
-fail_platform_device1:
-	platform_driver_unregister(&platform_driver);
-fail_platform_driver:
-	return ret;
   }

   module_init(alienware_wmi_init);

   static void __exit alienware_wmi_exit(void)
   {
-	remove_thermal_profile();
-	platform_device_unregister(platform_device);
-	platform_driver_unregister(&platform_driver);
+	if (interface == WMAX)
+		alienware_wmax_wmi_exit();
+	else
+		alienware_legacy_wmi_exit();
   }

   module_exit(alienware_wmi_exit);







