HID: lenovo: Remove CONFIG_ACPI dependency

[PATCH v2] HID: lenovo: Remove CONFIG_ACPI dependency

Posted by Janne Grunau via B4 Relay 7 months ago

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;
 			}
-			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,
-- 
Janne Grunau <j@jannau.net>

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

Posted by Armin Wolf 7 months ago

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,

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

Posted by Jiri Kosina 6 months, 1 week ago

On Tue, 20 May 2025, Armin Wolf wrote:

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

Makes sense; Janne, are you planning to send updated patch, please?

Thanks,

-- 
Jiri Kosina
SUSE Labs