linux-kernel - Re: [PATCH v2] HID: lenovo: Remove CONFIG

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] [day] [month] [year] [list]

Message-ID: <p3ps345p-6173-040r-4298-rn34697932p0@xreary.bet>
Date: Tue, 10 Jun 2025 21:58:16 +0200 (CEST)
From: Jiri Kosina <jikos@...nel.org>
To: Armin Wolf <W_Armin@....de>
cc: j@...nau.net, Arnd Bergmann <arnd@...db.de>, 
    Benjamin Tissoires <bentiss@...nel.org>, 
    "Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>, 
    linux-kernel@...r.kernel.org, linux-input@...r.kernel.org, 
    linux-acpi@...r.kernel.org
Subject: Re: [PATCH v2] HID: lenovo: Remove CONFIG_ACPI dependency

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@...nau.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@....de>

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

Thanks,

-- 
Jiri Kosina
SUSE Labs