linux-kernel - Re: [PATCH 19/20] platform/x86: Update alienware-wmi config entries

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <znrmeluldzbh6gnczl3najtdmw3kydv7omtio5xufxa234pqaf@v5qwv6r2esiy>
Date: Thu, 26 Dec 2024 23:59:25 -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 19/20] platform/x86: Update alienware-wmi config entries

On Fri, Dec 27, 2024 at 05:08:03AM +0100, Armin Wolf wrote:
> Am 21.12.24 um 06:59 schrieb Kurt Borja:
> 
> > Add config entries for each WMI driver managed by the alienware-wmi
> > module to be able to conditionally compile them.
> > 
> > Signed-off-by: Kurt Borja <kuurtb@...il.com>
> > ---
> >   drivers/platform/x86/dell/Kconfig         | 30 +++++++++++++++++++----
> >   drivers/platform/x86/dell/Makefile        |  4 +--
> >   drivers/platform/x86/dell/alienware-wmi.h | 23 +++++++++++++++++
> >   3 files changed, 50 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> > index d09060aedd3f..f8a0dffcaab7 100644
> > --- a/drivers/platform/x86/dell/Kconfig
> > +++ b/drivers/platform/x86/dell/Kconfig
> > @@ -18,15 +18,35 @@ config ALIENWARE_WMI
> >   	tristate "Alienware Special feature control"
> >   	default m
> >   	depends on ACPI
> > +	depends on ACPI_WMI
> > +	depends on DMI
> >   	depends on LEDS_CLASS
> >   	depends on NEW_LEDS
> > -	depends on ACPI_WMI
> > +	help
> > +	 This is a driver for controlling Alienware WMI driven features.
> > +
> > +	 On legacy devices, it exposes an interface for controlling the AlienFX
> > +	 zones on Alienware machines that don't contain a dedicated
> > +	 AlienFX USB MCU such as the X51 and X51-R2.
> > +
> > +	 On newer devices, it exposes the AWCC thermal control interface through
> > +	 known Kernel APIs.
> > +
> > +config ALIENWARE_WMI_LEGACY
> > +	bool "Alienware Legacy WMI device driver"
> > +	default y
> > +	depends on ALIENWARE_WMI
> > +	help
> > +	 Legacy Alienware WMI driver with AlienFX LED control capabilities.
> > +
> > +config ALIENWARE_WMI_WMAX
> > +	bool "Alienware WMAX WMI device driver"
> > +	default y
> > +	depends on ALIENWARE_WMI
> >   	select ACPI_PLATFORM_PROFILE
> >   	help
> > -	 This is a driver for controlling Alienware BIOS driven
> > -	 features.  It exposes an interface for controlling the AlienFX
> > -	 zones on Alienware machines that don't contain a dedicated AlienFX
> > -	 USB MCU such as the X51 and X51-R2.
> > +	 Alienware WMI driver with AlienFX LED, HDMI, amplifier, deep sleep and
> > +	 AWCC thermal control capabilities.
> > 
> >   config DCDBAS
> >   	tristate "Dell Systems Management Base Driver"
> > diff --git a/drivers/platform/x86/dell/Makefile b/drivers/platform/x86/dell/Makefile
> > index d5718ef34c48..8ac9a933c770 100644
> > --- a/drivers/platform/x86/dell/Makefile
> > +++ b/drivers/platform/x86/dell/Makefile
> > @@ -6,8 +6,8 @@
> > 
> >   obj-$(CONFIG_ALIENWARE_WMI)			+= alienware-wmi.o
> >   alienware-wmi-objs				:= alienware-wmi-base.o
> > -alienware-wmi-y					+= alienware-wmi-legacy.o
> > -alienware-wmi-y					+= alienware-wmi-wmax.o
> > +alienware-wmi-$(CONFIG_ALIENWARE_WMI_LEGACY)	+= alienware-wmi-legacy.o
> > +alienware-wmi-$(CONFIG_ALIENWARE_WMI_WMAX)	+= alienware-wmi-wmax.o
> >   obj-$(CONFIG_DCDBAS)				+= dcdbas.o
> >   obj-$(CONFIG_DELL_LAPTOP)			+= dell-laptop.o
> >   obj-$(CONFIG_DELL_RBTN)				+= dell-rbtn.o
> > diff --git a/drivers/platform/x86/dell/alienware-wmi.h b/drivers/platform/x86/dell/alienware-wmi.h
> > index 78ac10122155..97b52e51cd7d 100644
> > --- a/drivers/platform/x86/dell/alienware-wmi.h
> > +++ b/drivers/platform/x86/dell/alienware-wmi.h
> > @@ -94,10 +94,33 @@ acpi_status alienware_wmi_command(struct wmi_device *wdev, u32 method_id,
> >   int alienware_alienfx_setup(struct alienfx_platdata *pdata);
> >   void alienware_alienfx_exit(struct wmi_device *wdev);
> > 
> > +#if IS_ENABLED(CONFIG_ALIENWARE_WMI_LEGACY)
> >   int __init alienware_legacy_wmi_init(void);
> >   void __exit alienware_legacy_wmi_exit(void);
> > +#else
> > +int __init alienware_legacy_wmi_init(void)
> > +{
> > +	return 0;
> 
> Please return -EOPNOTSUPP here.

Maybe -ENODEV is better in this case?

Also this should be static inline, I'll fix it.

> 
> > +}
> > +
> > +void __exit alienware_legacy_wmi_exit(void)
> > +{
> > +}
> > +#endif
> > 
> > +#if IS_ENABLED(CONFIG_ALIENWARE_WMI_WMAX)
> >   int __init alienware_wmax_wmi_init(void);
> >   void __exit alienware_wmax_wmi_exit(void);
> > +#else
> > +int __init alienware_wmax_wmi_init(void)
> > +{
> > +	return 0;
> 
> Same as above.
> 
> Thanks,
> Armin Wolf
> 
> > +}
> > +
> > +
> > +void __exit alienware_wmax_wmi_exit(void)
> > +{
> > +}
> > +#endif
> > 
> >   #endif