Add WMI drivers for LEGACY and WMAX devices.

This involves moving the platform device registration to a helper
function that is now called from the driver's preferred WMI device
driver probe. In the case of the WMAX this is done only if
`!quirks->thermal` because the newer WMAX interface doesn't support any
of the LED features of this driver. This also eliminates the need to
check for `quirks->num_zones > 0` inside alienfx_probe().

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

Additionally, create_thermal_profile() now takes wmi_device * instead of
platform_device *.

Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/alienware-wmi.c | 177 ++++++++++++++++------
 1 file changed, 134 insertions(+), 43 deletions(-)

diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index ab86deb1adb9..506c096553e8 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@outlook.com>");
 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);
@@ -421,7 +420,10 @@ struct alienfx_priv {
 	u8 lighting_control_state;
 };
 
-static struct platform_device *platform_device;
+struct alienfx_platdata {
+	struct wmi_device *wdev;
+};
+
 static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
 
 static u8 interface;
@@ -801,7 +803,7 @@ static DEVICE_ATTR_RW(source);
 
 static bool hdmi_group_visible(struct kobject *kobj)
 {
-	return quirks->hdmi_mux;
+	return interface == WMAX && quirks->hdmi_mux;
 }
 DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(hdmi);
 
@@ -848,7 +850,7 @@ static DEVICE_ATTR_RO(status);
 
 static bool amplifier_group_visible(struct kobject *kobj)
 {
-	return quirks->amplifier;
+	return interface == WMAX && quirks->amplifier;
 }
 DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(amplifier);
 
@@ -917,7 +919,7 @@ static DEVICE_ATTR_RW(deepsleep);
 
 static bool deepsleep_group_visible(struct kobject *kobj)
 {
-	return quirks->deepslp;
+	return interface == WMAX && quirks->deepslp;
 }
 DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(deepsleep);
 
@@ -1136,11 +1138,11 @@ static const struct platform_profile_ops awcc_platform_profile_ops = {
 	.profile_set = thermal_profile_set,
 };
 
-static int create_thermal_profile(struct platform_device *platform_device)
+static int create_thermal_profile(struct wmi_device *wdev)
 {
 	struct device *ppdev;
 
-	ppdev = devm_platform_profile_register(&platform_device->dev, "alienware-wmi",
+	ppdev = devm_platform_profile_register(&wdev->dev, "alienware-wmi",
 					       NULL, &awcc_platform_profile_ops);
 
 	return PTR_ERR_OR_ZERO(ppdev);
@@ -1153,9 +1155,6 @@ static int alienfx_probe(struct platform_device *pdev)
 {
 	struct alienfx_priv *priv;
 
-	if (!quirks->num_zones)
-		return -ENODEV;
-
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -1192,18 +1191,118 @@ static struct platform_driver platform_driver = {
 	.probe = alienfx_probe,
 };
 
-static int __init alienware_wmi_init(void)
+static void alienware_alienfx_remove(void *data)
 {
+	struct platform_device *pdev = data;
+
+	platform_device_unregister(pdev);
+}
+
+static int alienware_alienfx_setup(struct alienfx_platdata *pdata)
+{
+	struct device *dev = &pdata->wdev->dev;
+	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;
-	}
+	pdev = platform_device_register_data(NULL, "alienware-wmi",
+					     PLATFORM_DEVID_NONE, pdata,
+					     sizeof(*pdata));
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	dev_set_drvdata(dev, pdev);
+	ret = devm_add_action_or_reset(dev, alienware_alienfx_remove, pdev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/*
+ * 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 const 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,
+	.no_singleton = true,
+};
+
+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;
+
+	if (quirks->thermal)
+		ret = create_thermal_profile(wdev);
+	else
+		ret = alienware_alienfx_setup(&pdata);
+
+	return ret;
+}
+
+static const 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,
+	.no_singleton = true,
+};
+
+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)
@@ -1220,32 +1319,20 @@ static int __init alienware_wmi_init(void)
 	}
 
 	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;
-	}
-	ret = platform_device_add(platform_device);
-	if (ret)
-		goto fail_platform_device2;
+	if (ret < 0)
+		return ret;
 
-	if (quirks->thermal) {
-		ret = create_thermal_profile(platform_device);
-		if (ret)
-			goto fail_prep_thermal_profile;
+	if (wmi_has_guid(WMAX_CONTROL_GUID)) {
+		interface = WMAX;
+		ret = alienware_wmax_wmi_init();
+	} else {
+		interface = LEGACY;
+		ret = alienware_legacy_wmi_init();
 	}
 
-	return 0;
+	if (ret < 0)
+		platform_driver_unregister(&platform_driver);
 
-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;
 }
 
@@ -1253,7 +1340,11 @@ module_init(alienware_wmi_init);
 
 static void __exit alienware_wmi_exit(void)
 {
-	platform_device_unregister(platform_device);
+	if (interface == WMAX)
+		alienware_wmax_wmi_exit();
+	else
+		alienware_legacy_wmi_exit();
+
 	platform_driver_unregister(&platform_driver);
 }
 
-- 
2.48.1

On Fri, Feb 07, 2025 at 10:45:58AM -0500, Kurt Borja wrote:
> Add WMI drivers for LEGACY and WMAX devices.
> 
> This involves moving the platform device registration to a helper
> function that is now called from the driver's preferred WMI device
> driver probe. In the case of the WMAX this is done only if
> `!quirks->thermal` because the newer WMAX interface doesn't support any
> of the LED features of this driver. This also eliminates the need to
> check for `quirks->num_zones > 0` inside alienfx_probe().
> 
> Only one WMI driver is registered on module initialization to prevent
> registering a duplicate platform device.
> 
> Additionally, create_thermal_profile() now takes wmi_device * instead of
> platform_device *.

...

> +static int alienware_alienfx_setup(struct alienfx_platdata *pdata)
> +{
> +	struct device *dev = &pdata->wdev->dev;
> +	struct platform_device *pdev;
>  	int ret;
>  
> +	pdev = platform_device_register_data(NULL, "alienware-wmi",
> +					     PLATFORM_DEVID_NONE, pdata,
> +					     sizeof(*pdata));
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
> +
> +	dev_set_drvdata(dev, pdev);
> +	ret = devm_add_action_or_reset(dev, alienware_alienfx_remove, pdev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

	return devm_add_action_or_reset(...);

> +}

...

> +static const struct wmi_device_id alienware_legacy_device_id_table[] = {
> +	{ LEGACY_CONTROL_GUID, NULL },

Drop that ' , NULL' part, it makes an additional burden if the type of the
driver_data is ever changed.

> +	{ },

No comma in the terminator entries.

> +};

...

> +static const struct wmi_device_id alienware_wmax_device_id_table[] = {
> +	{ WMAX_CONTROL_GUID, NULL },
> +	{ },
> +};

Ditto.

...

> +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);
> +}

I believe we have module_wmi_driver() which can be used after the split
(haven't checked those patches yet).

-- 
With Best Regards,
Andy Shevchenko

On Tue Feb 11, 2025 at 11:30 AM -05, Andy Shevchenko wrote:
> On Fri, Feb 07, 2025 at 10:45:58AM -0500, Kurt Borja wrote:
>> Add WMI drivers for LEGACY and WMAX devices.
>> 
>> This involves moving the platform device registration to a helper
>> function that is now called from the driver's preferred WMI device
>> driver probe. In the case of the WMAX this is done only if
>> `!quirks->thermal` because the newer WMAX interface doesn't support any
>> of the LED features of this driver. This also eliminates the need to
>> check for `quirks->num_zones > 0` inside alienfx_probe().
>> 
>> Only one WMI driver is registered on module initialization to prevent
>> registering a duplicate platform device.
>> 
>> Additionally, create_thermal_profile() now takes wmi_device * instead of
>> platform_device *.
>
> ...
>
>> +static int alienware_alienfx_setup(struct alienfx_platdata *pdata)
>> +{
>> +	struct device *dev = &pdata->wdev->dev;
>> +	struct platform_device *pdev;
>>  	int ret;
>>  
>> +	pdev = platform_device_register_data(NULL, "alienware-wmi",
>> +					     PLATFORM_DEVID_NONE, pdata,
>> +					     sizeof(*pdata));
>> +	if (IS_ERR(pdev))
>> +		return PTR_ERR(pdev);
>> +
>> +	dev_set_drvdata(dev, pdev);
>> +	ret = devm_add_action_or_reset(dev, alienware_alienfx_remove, pdev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>
> 	return devm_add_action_or_reset(...);

Right!

>
>> +}
>
> ...
>
>> +static const struct wmi_device_id alienware_legacy_device_id_table[] = {
>> +	{ LEGACY_CONTROL_GUID, NULL },
>
> Drop that ' , NULL' part, it makes an additional burden if the type of the
> driver_data is ever changed.

Makes sense.

>
>> +	{ },
>
> No comma in the terminator entries.

Makes sense.

>
>> +};
>
> ...
>
>> +static const struct wmi_device_id alienware_wmax_device_id_table[] = {
>> +	{ WMAX_CONTROL_GUID, NULL },
>> +	{ },
>> +};
>
> Ditto.
>
> ...
>
>> +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);
>> +}
>
> I believe we have module_wmi_driver() which can be used after the split
> (haven't checked those patches yet).

This module has two WMI drivers, so I believe this can't be used.

-- 
 ~ Kurt

On Tue, Feb 11, 2025 at 12:46:40PM -0500, Kurt Borja wrote:
> On Tue Feb 11, 2025 at 11:30 AM -05, Andy Shevchenko wrote:
> > On Fri, Feb 07, 2025 at 10:45:58AM -0500, Kurt Borja wrote:

...

> >> +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);
> >> +}
> >
> > I believe we have module_wmi_driver() which can be used after the split
> > (haven't checked those patches yet).
> 
> This module has two WMI drivers, so I believe this can't be used.

Even after the split?

-- 
With Best Regards,
Andy Shevchenko