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

[Armin Wolf <W_Armin@gmx.de>]

Am 27.12.24 um 05:59 schrieb Kurt Borja:

> 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@gmail.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.

-ENODEV is also OK.

Thanks,
Armin Wolf

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