| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <hc7l4cs7odvzjvty7mthbdwpt5ddd3ztghtomi5xhjuvpeulsp@q6kehnenpbda>
Date: Thu, 26 Dec 2024 23:48:21 -0500
From: Kurt Borja <kuurtb@...il.com>
To: Armin Wolf <W_Armin@....de>
Cc: platform-driver-x86@...r.kernel.org, ilpo.jarvinen@...ux.intel.com,
mario.limonciello@....com, hdegoede@...hat.com, linux-kernel@...r.kernel.org,
Dell.Client.Kernel@...l.com
Subject: Re: [PATCH 08/20] alienware-wmi: Add WMI Drivers
On Thu, Dec 26, 2024 at 11:44:52PM -0500, Kurt Borja wrote:
> 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
Nevermind this, I thought you said .dev_groups xd. I will do this on v2.
>
> >
> > Thanks,
> > Armin Wolf
> >
> > >
> > > Signed-off-by: Kurt Borja <kuurtb@...il.com>
> > > ---
> > > 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@...look.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);
> > > @@ -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);
Powered by blists - more mailing lists