Re: [PATCH v2] HID: lenovo: Remove CONFIG_ACPI dependency

[Armin Wolf <W_Armin@gmx.de>]

Am 19.05.25 um 22:49 schrieb Janne Grunau via B4 Relay:

> From: Janne Grunau <j@jannau.net>
>
> The hid-lenovo driver supports external Bluetooth and USB devices which
> can be used with non-ACPI systems/kernels. Call platform_profile_cycle()
> only if CONFIG_ACPI_PLATFORM_PROFILE is enabled and select
> CONFIG_ACPI_PLATFORM_PROFILE only if ACPI is enabled.
> This should not affect functionality since only the detachable keyboard
> of a x86 tablet with a custom connector has an hotkey for cycling
> through power profiles.
>
> Fixes: 52572cde8b4a4 ("HID: lenovo: select CONFIG_ACPI_PLATFORM_PROFILE")
> Signed-off-by: Janne Grunau <j@jannau.net>
> ---
> hid-lenovo supports external generic USB and Bluetooth devices and
> should be buildable and usable on non-ACPI kernels and systems. Commit
> 84c9d2a968c82 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd Fn
> keys") added a hot key to cycle through power profiles using ACPI's
> platform_profile. This resulted in adding a dependency on ACPI and
> selecting CONFIG_ACPI_PLATFORM_PROFILE to fix build an link errors in
> commit 52572cde8b4a ("HID: lenovo: select
> CONFIG_ACPI_PLATFORM_PROFILE"). This is undesirable for HID drivers
> supporting generic USB and Bluetooth devices. So instead call
> platform_profile_cycle() only CONFIG_ACPI_PLATFORM_PROFILE is enabled
> and select the latter only if ACPI is enabled.
>
> Supercedes with Armin Wolf's "ACPI: platform_profile: Add support for
> non-ACPI platforms" [1] the earlier removel in "HID: lenovo: Unbreak
> USB/BT keyboards on non-ACPI platforms" [2].
>
> [1]: https://lore.kernel.org/linux-acpi/20250518185111.3560-1-W_Armin@gmx.de/
> [2]: https://lore.kernel.org/linux-input/20250512-hid_lenovo_unbreak_non_acpi-v1-1-e9e37ecbfbfe@jannau.net/
> ---
> Changes in v2:
> - drop stub platform_profile_cycle()
> - call platform_profile_cycle() conditioanlly
> - drop 'depends on ACPI || !ACPI'
> - Link to v1: https://lore.kernel.org/r/20250518-hid_lenovo_acpi_dependency-v1-0-afdb93b5d1a6@jannau.net
> ---
>   drivers/hid/Kconfig      | 3 +--
>   drivers/hid/hid-lenovo.c | 6 ++++--
>   2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index a503252702b7b43c332a12b14bc8b23b83e9f028..1656bb1504f750d73011d3f008e27b4436a58678 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -595,8 +595,7 @@ config HID_LED
>   
>   config HID_LENOVO
>   	tristate "Lenovo / Thinkpad devices"
> -	depends on ACPI
> -	select ACPI_PLATFORM_PROFILE
> +	select ACPI_PLATFORM_PROFILE if ACPI
>   	select NEW_LEDS
>   	select LEDS_CLASS
>   	help
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index af29ba840522f99bc2f426d4753f70d442cef3af..73c6a26638a22ad1c8368112e8ab185232a9df12 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -728,9 +728,11 @@ static int lenovo_raw_event_TP_X12_tab(struct hid_device *hdev, u32 raw_data)
>   			if (hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB) {
>   				report_key_event(input, KEY_RFKILL);
>   				return 1;
> +			} else if (IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)) {
> +				platform_profile_cycle();
> +				return 1;
>   			}

Please turn the "if else" into a single "if":

	if (IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)) {
		platform_profile_cycle();
		return 1;
	}

	return 0;

  With this being done:

Reviewed-by: Armin Wolf <W_Armin@gmx.de>

Thanks,
Armin Wolf

> -			platform_profile_cycle();
> -			return 1;
> +			return 0;
>   		case TP_X12_RAW_HOTKEY_FN_F10:
>   			/* TAB1 has PICKUP Phone and TAB2 use Snipping tool*/
>   			(hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB) ?
>
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250518-hid_lenovo_acpi_dependency-0ea78a6bd740
>
> Best regards,