| Message ID | 20250512-hid_lenovo_unbreak_non_acpi-v1-1-e9e37ecbfbfe@jannau.net |
|---|---|
| State | New |
| Headers | show |
| Series | HID: lenovo: Unbreak USB/BT keyboards on non-ACPI platforms | expand |
Am 12.05.25 um 23:55 schrieb Janne Grunau via B4 Relay: > From: Janne Grunau <j@jannau.net> > > Commit 84c9d2a968c8 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd > Fn keys") added a dependency on ACPI_PLATFORM_PROFILE to cycle through > power profiles. This breaks USB and Bluetooth keyboards on non-ACPI > platforms since platform_profile_init() fails. See the warning below for > the visible symptom but cause is the dependency on the platform_profile > module. > > [ 266.225052] kernel: usb 1-1.3.2: new full-speed USB device number 9 using xhci_hcd > [ 266.316032] kernel: usb 1-1.3.2: New USB device found, idVendor=17ef, idProduct=6047, bcdDevice= 3.30 > [ 266.327129] kernel: usb 1-1.3.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0 > [ 266.327623] kernel: usb 1-1.3.2: Product: ThinkPad Compact USB Keyboard with TrackPoint > [ 266.328096] kernel: usb 1-1.3.2: Manufacturer: Lenovo > [ 266.337488] kernel: ------------[ cut here ]------------ > [ 266.337551] kernel: WARNING: CPU: 4 PID: 2619 at fs/sysfs/group.c:131 internal_create_group+0xc0/0x358 > [ 266.337584] kernel: Modules linked in: platform_profile(+) nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft> > [ 266.337685] kernel: apple_sio spi_apple apple_dart soundcore spmi_apple_controller pinctrl_apple_gpio i2c_pasemi_platform apple_admac i2c_pasemi_core clk_apple_nco xhci_pla> > [ 266.337717] kernel: CPU: 4 UID: 0 PID: 2619 Comm: (udev-worker) Tainted: G S W 6.14.4-400.asahi.fc41.aarch64+16k #1 > [ 266.337750] kernel: Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN > [ 266.337776] kernel: Hardware name: Apple Mac mini (M1, 2020) (DT) > [ 266.337808] kernel: pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) > [ 266.337834] kernel: pc : internal_create_group+0xc0/0x358 > [ 266.337860] kernel: lr : sysfs_create_group+0x20/0x40 > [ 266.337886] kernel: sp : ffff800086f877a0 > [ 266.337914] kernel: x29: ffff800086f877b0 x28: 0000000000000000 x27: ffffb66d0b338348 > [ 266.337939] kernel: x26: ffffb66d0b338358 x25: ffffb66d528c7c50 x24: ffffb66d507e37b0 > [ 266.337965] kernel: x23: 0000fffebf6708d8 x22: 0000000000000000 x21: ffffb66d0b370340 > [ 266.337991] kernel: x20: ffffb66d0b370308 x19: 0000000000000000 x18: 0000000000000000 > [ 266.338029] kernel: x17: 554e514553007373 x16: ffffb66d4f8c2268 x15: 595342555300656c > [ 266.338051] kernel: x14: 69666f72702d6d72 x13: 00353236353d4d55 x12: 4e51455300737361 > [ 266.338075] kernel: x11: ffff6adf91b80100 x10: 0000000000000139 x9 : ffffb66d4f8c2288 > [ 266.338097] kernel: x8 : ffff800086f87620 x7 : 0000000000000000 x6 : 0000000000000000 > [ 266.338116] kernel: x5 : ffff6adfc896e100 x4 : 0000000000000000 x3 : ffff6adfc896e100 > [ 266.338139] kernel: x2 : ffffb66d0b3703a0 x1 : 0000000000000000 x0 : 0000000000000000 > [ 266.338155] kernel: Call trace: > [ 266.338173] kernel: internal_create_group+0xc0/0x358 (P) > [ 266.338193] kernel: sysfs_create_group+0x20/0x40 > [ 266.338206] kernel: platform_profile_init+0x48/0x3ff8 [platform_profile] > [ 266.338224] kernel: do_one_initcall+0x60/0x358 > [ 266.338239] kernel: do_init_module+0x94/0x260 > [ 266.338257] kernel: load_module+0x5e0/0x708 > [ 266.338271] kernel: init_module_from_file+0x94/0x100 > [ 266.338290] kernel: __arm64_sys_finit_module+0x268/0x360 > [ 266.338309] kernel: invoke_syscall+0x6c/0x100 > [ 266.338327] kernel: el0_svc_common.constprop.0+0xc8/0xf0 > [ 266.338346] kernel: do_el0_svc+0x24/0x38 > [ 266.338365] kernel: el0_svc+0x3c/0x170 > [ 266.338385] kernel: el0t_64_sync_handler+0x10c/0x138 > [ 266.338404] kernel: el0t_64_sync+0x1b0/0x1b8 > [ 266.338419] kernel: ---[ end trace 0000000000000000 ]--- > > Fixes: 84c9d2a968c8 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd Fn keys") > Cc: stable@vger.kernel.org > Signed-off-by: Janne Grunau <j@jannau.net> > > ------>8--------- > I don't see an easy solution to keep the functionality in generic HID > code which is used on non-ACPI platforms. Solution for this are not > trivial so remove the functionality for now. > Cc-ing the ACPI maintainers in the case they can think of a solution for > this issue. Hi, i think we can fix that. We just have to skip the compat stuff if acpi_kobj is NULL (means that ACPI is not used). The modern platform profile interface is generic enough to also work on non-ACPI systems. Can you test a patch? Thanks, Armin Wolf > > --- > drivers/hid/Kconfig | 1 - > drivers/hid/hid-lenovo.c | 5 +---- > 2 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index a503252702b7b43c332a12b14bc8b23b83e9f028..6b4445f54b2f2818d451ff28b3f7bcc2b1c7e99f 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -596,7 +596,6 @@ config HID_LED > config HID_LENOVO > tristate "Lenovo / Thinkpad devices" > depends on ACPI > - select ACPI_PLATFORM_PROFILE > select NEW_LEDS > select LEDS_CLASS > help > diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c > index af29ba840522f99bc2f426d4753f70d442cef3af..cff4b5ddd9aa9bad0516f8c9beb58927c24477fc 100644 > --- a/drivers/hid/hid-lenovo.c > +++ b/drivers/hid/hid-lenovo.c > @@ -32,8 +32,6 @@ > #include <linux/leds.h> > #include <linux/workqueue.h> > > -#include <linux/platform_profile.h> > - > #include "hid-ids.h" > > /* Userspace expects F20 for mic-mute KEY_MICMUTE does not work */ > @@ -729,8 +727,7 @@ static int lenovo_raw_event_TP_X12_tab(struct hid_device *hdev, u32 raw_data) > report_key_event(input, KEY_RFKILL); > 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: 20250512-hid_lenovo_unbreak_non_acpi-c041f3cc13b6 > > Best regards,
On Fri, May 16, 2025 at 12:05:11AM +0200, Armin Wolf wrote: > Am 12.05.25 um 23:55 schrieb Janne Grunau via B4 Relay: > > > From: Janne Grunau <j@jannau.net> > > > > Commit 84c9d2a968c8 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd > > Fn keys") added a dependency on ACPI_PLATFORM_PROFILE to cycle through > > power profiles. This breaks USB and Bluetooth keyboards on non-ACPI > > platforms since platform_profile_init() fails. See the warning below for > > the visible symptom but cause is the dependency on the platform_profile > > module. > > > > [ 266.225052] kernel: usb 1-1.3.2: new full-speed USB device number 9 using xhci_hcd > > [ 266.316032] kernel: usb 1-1.3.2: New USB device found, idVendor=17ef, idProduct=6047, bcdDevice= 3.30 > > [ 266.327129] kernel: usb 1-1.3.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0 > > [ 266.327623] kernel: usb 1-1.3.2: Product: ThinkPad Compact USB Keyboard with TrackPoint > > [ 266.328096] kernel: usb 1-1.3.2: Manufacturer: Lenovo > > [ 266.337488] kernel: ------------[ cut here ]------------ > > [ 266.337551] kernel: WARNING: CPU: 4 PID: 2619 at fs/sysfs/group.c:131 internal_create_group+0xc0/0x358 > > [ 266.337584] kernel: Modules linked in: platform_profile(+) nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft> > > [ 266.337685] kernel: apple_sio spi_apple apple_dart soundcore spmi_apple_controller pinctrl_apple_gpio i2c_pasemi_platform apple_admac i2c_pasemi_core clk_apple_nco xhci_pla> > > [ 266.337717] kernel: CPU: 4 UID: 0 PID: 2619 Comm: (udev-worker) Tainted: G S W 6.14.4-400.asahi.fc41.aarch64+16k #1 > > [ 266.337750] kernel: Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN > > [ 266.337776] kernel: Hardware name: Apple Mac mini (M1, 2020) (DT) > > [ 266.337808] kernel: pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) > > [ 266.337834] kernel: pc : internal_create_group+0xc0/0x358 > > [ 266.337860] kernel: lr : sysfs_create_group+0x20/0x40 > > [ 266.337886] kernel: sp : ffff800086f877a0 > > [ 266.337914] kernel: x29: ffff800086f877b0 x28: 0000000000000000 x27: ffffb66d0b338348 > > [ 266.337939] kernel: x26: ffffb66d0b338358 x25: ffffb66d528c7c50 x24: ffffb66d507e37b0 > > [ 266.337965] kernel: x23: 0000fffebf6708d8 x22: 0000000000000000 x21: ffffb66d0b370340 > > [ 266.337991] kernel: x20: ffffb66d0b370308 x19: 0000000000000000 x18: 0000000000000000 > > [ 266.338029] kernel: x17: 554e514553007373 x16: ffffb66d4f8c2268 x15: 595342555300656c > > [ 266.338051] kernel: x14: 69666f72702d6d72 x13: 00353236353d4d55 x12: 4e51455300737361 > > [ 266.338075] kernel: x11: ffff6adf91b80100 x10: 0000000000000139 x9 : ffffb66d4f8c2288 > > [ 266.338097] kernel: x8 : ffff800086f87620 x7 : 0000000000000000 x6 : 0000000000000000 > > [ 266.338116] kernel: x5 : ffff6adfc896e100 x4 : 0000000000000000 x3 : ffff6adfc896e100 > > [ 266.338139] kernel: x2 : ffffb66d0b3703a0 x1 : 0000000000000000 x0 : 0000000000000000 > > [ 266.338155] kernel: Call trace: > > [ 266.338173] kernel: internal_create_group+0xc0/0x358 (P) > > [ 266.338193] kernel: sysfs_create_group+0x20/0x40 > > [ 266.338206] kernel: platform_profile_init+0x48/0x3ff8 [platform_profile] > > [ 266.338224] kernel: do_one_initcall+0x60/0x358 > > [ 266.338239] kernel: do_init_module+0x94/0x260 > > [ 266.338257] kernel: load_module+0x5e0/0x708 > > [ 266.338271] kernel: init_module_from_file+0x94/0x100 > > [ 266.338290] kernel: __arm64_sys_finit_module+0x268/0x360 > > [ 266.338309] kernel: invoke_syscall+0x6c/0x100 > > [ 266.338327] kernel: el0_svc_common.constprop.0+0xc8/0xf0 > > [ 266.338346] kernel: do_el0_svc+0x24/0x38 > > [ 266.338365] kernel: el0_svc+0x3c/0x170 > > [ 266.338385] kernel: el0t_64_sync_handler+0x10c/0x138 > > [ 266.338404] kernel: el0t_64_sync+0x1b0/0x1b8 > > [ 266.338419] kernel: ---[ end trace 0000000000000000 ]--- > > > > Fixes: 84c9d2a968c8 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd Fn keys") > > Cc: stable@vger.kernel.org > > Signed-off-by: Janne Grunau <j@jannau.net> > > > > ------>8--------- > > I don't see an easy solution to keep the functionality in generic HID > > code which is used on non-ACPI platforms. Solution for this are not > > trivial so remove the functionality for now. > > Cc-ing the ACPI maintainers in the case they can think of a solution for > > this issue. > > Hi, > > i think we can fix that. We just have to skip the compat stuff if acpi_kobj is NULL (means that ACPI is not used). > The modern platform profile interface is generic enough to also work on non-ACPI systems. > > Can you test a patch? I can easily test patches Janne
Am 16.05.25 um 01:05 schrieb Janne Grunau: > On Fri, May 16, 2025 at 12:05:11AM +0200, Armin Wolf wrote: >> Am 12.05.25 um 23:55 schrieb Janne Grunau via B4 Relay: >> >>> From: Janne Grunau <j@jannau.net> >>> >>> Commit 84c9d2a968c8 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd >>> Fn keys") added a dependency on ACPI_PLATFORM_PROFILE to cycle through >>> power profiles. This breaks USB and Bluetooth keyboards on non-ACPI >>> platforms since platform_profile_init() fails. See the warning below for >>> the visible symptom but cause is the dependency on the platform_profile >>> module. >>> >>> [ 266.225052] kernel: usb 1-1.3.2: new full-speed USB device number 9 using xhci_hcd >>> [ 266.316032] kernel: usb 1-1.3.2: New USB device found, idVendor=17ef, idProduct=6047, bcdDevice= 3.30 >>> [ 266.327129] kernel: usb 1-1.3.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0 >>> [ 266.327623] kernel: usb 1-1.3.2: Product: ThinkPad Compact USB Keyboard with TrackPoint >>> [ 266.328096] kernel: usb 1-1.3.2: Manufacturer: Lenovo >>> [ 266.337488] kernel: ------------[ cut here ]------------ >>> [ 266.337551] kernel: WARNING: CPU: 4 PID: 2619 at fs/sysfs/group.c:131 internal_create_group+0xc0/0x358 >>> [ 266.337584] kernel: Modules linked in: platform_profile(+) nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft> >>> [ 266.337685] kernel: apple_sio spi_apple apple_dart soundcore spmi_apple_controller pinctrl_apple_gpio i2c_pasemi_platform apple_admac i2c_pasemi_core clk_apple_nco xhci_pla> >>> [ 266.337717] kernel: CPU: 4 UID: 0 PID: 2619 Comm: (udev-worker) Tainted: G S W 6.14.4-400.asahi.fc41.aarch64+16k #1 >>> [ 266.337750] kernel: Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN >>> [ 266.337776] kernel: Hardware name: Apple Mac mini (M1, 2020) (DT) >>> [ 266.337808] kernel: pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) >>> [ 266.337834] kernel: pc : internal_create_group+0xc0/0x358 >>> [ 266.337860] kernel: lr : sysfs_create_group+0x20/0x40 >>> [ 266.337886] kernel: sp : ffff800086f877a0 >>> [ 266.337914] kernel: x29: ffff800086f877b0 x28: 0000000000000000 x27: ffffb66d0b338348 >>> [ 266.337939] kernel: x26: ffffb66d0b338358 x25: ffffb66d528c7c50 x24: ffffb66d507e37b0 >>> [ 266.337965] kernel: x23: 0000fffebf6708d8 x22: 0000000000000000 x21: ffffb66d0b370340 >>> [ 266.337991] kernel: x20: ffffb66d0b370308 x19: 0000000000000000 x18: 0000000000000000 >>> [ 266.338029] kernel: x17: 554e514553007373 x16: ffffb66d4f8c2268 x15: 595342555300656c >>> [ 266.338051] kernel: x14: 69666f72702d6d72 x13: 00353236353d4d55 x12: 4e51455300737361 >>> [ 266.338075] kernel: x11: ffff6adf91b80100 x10: 0000000000000139 x9 : ffffb66d4f8c2288 >>> [ 266.338097] kernel: x8 : ffff800086f87620 x7 : 0000000000000000 x6 : 0000000000000000 >>> [ 266.338116] kernel: x5 : ffff6adfc896e100 x4 : 0000000000000000 x3 : ffff6adfc896e100 >>> [ 266.338139] kernel: x2 : ffffb66d0b3703a0 x1 : 0000000000000000 x0 : 0000000000000000 >>> [ 266.338155] kernel: Call trace: >>> [ 266.338173] kernel: internal_create_group+0xc0/0x358 (P) >>> [ 266.338193] kernel: sysfs_create_group+0x20/0x40 >>> [ 266.338206] kernel: platform_profile_init+0x48/0x3ff8 [platform_profile] >>> [ 266.338224] kernel: do_one_initcall+0x60/0x358 >>> [ 266.338239] kernel: do_init_module+0x94/0x260 >>> [ 266.338257] kernel: load_module+0x5e0/0x708 >>> [ 266.338271] kernel: init_module_from_file+0x94/0x100 >>> [ 266.338290] kernel: __arm64_sys_finit_module+0x268/0x360 >>> [ 266.338309] kernel: invoke_syscall+0x6c/0x100 >>> [ 266.338327] kernel: el0_svc_common.constprop.0+0xc8/0xf0 >>> [ 266.338346] kernel: do_el0_svc+0x24/0x38 >>> [ 266.338365] kernel: el0_svc+0x3c/0x170 >>> [ 266.338385] kernel: el0t_64_sync_handler+0x10c/0x138 >>> [ 266.338404] kernel: el0t_64_sync+0x1b0/0x1b8 >>> [ 266.338419] kernel: ---[ end trace 0000000000000000 ]--- >>> >>> Fixes: 84c9d2a968c8 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd Fn keys") >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Janne Grunau <j@jannau.net> >>> >>> ------>8--------- >>> I don't see an easy solution to keep the functionality in generic HID >>> code which is used on non-ACPI platforms. Solution for this are not >>> trivial so remove the functionality for now. >>> Cc-ing the ACPI maintainers in the case they can think of a solution for >>> this issue. >> Hi, >> >> i think we can fix that. We just have to skip the compat stuff if acpi_kobj is NULL (means that ACPI is not used). >> The modern platform profile interface is generic enough to also work on non-ACPI systems. >> >> Can you test a patch? > I can easily test patches > > Janne Nice, i attached the necessary patch. Please keep in mind that this patch is compile-tested only. Thanks, Armin Wolf
Hej, On Sat, May 17, 2025 at 05:58:24PM +0200, Armin Wolf wrote: > Am 16.05.25 um 01:05 schrieb Janne Grunau: > > > On Fri, May 16, 2025 at 12:05:11AM +0200, Armin Wolf wrote: > >> Am 12.05.25 um 23:55 schrieb Janne Grunau via B4 Relay: > >> > >>> From: Janne Grunau <j@jannau.net> > >>> > >>> Commit 84c9d2a968c8 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd > >>> Fn keys") added a dependency on ACPI_PLATFORM_PROFILE to cycle through > >>> power profiles. This breaks USB and Bluetooth keyboards on non-ACPI > >>> platforms since platform_profile_init() fails. See the warning below for > >>> the visible symptom but cause is the dependency on the platform_profile > >>> module. [...] > >> i think we can fix that. We just have to skip the compat stuff if acpi_kobj is NULL (means that ACPI is not used). > >> The modern platform profile interface is generic enough to also work on non-ACPI systems. > >> > >> Can you test a patch? > > I can easily test patches > > Nice, i attached the necessary patch. Please keep in mind that this patch is compile-tested only. > > From: Armin Wolf <W_Armin@gmx.de> > Date: Sat, 17 May 2025 17:45:09 +0200 > Subject: [PATCH] ACPI: platform_profile: Add support for non-ACPI platforms > > Currently the platform profile subsystem assumes that all supported > (i.e. ACPI-capable) platforms always run with ACPI being enabled. > However some ARM64 notebooks do not support ACPI and are instead > using devicetree for booting. > > Do not register the legacy sysfs interface on such devices as it > depends on the acpi_kobj (/sys/firmware/acpi/) being present. Users > are encouraged to use the new platform-profile class interface > instead. > > Signed-off-by: Armin Wolf <W_Armin@gmx.de> > --- > drivers/acpi/platform_profile.c | 68 ++++++++++++++++++++++++++------- > 1 file changed, 55 insertions(+), 13 deletions(-) > > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c > index ffbfd32f4cf1..c5a5da7d03f1 100644 > --- a/drivers/acpi/platform_profile.c > +++ b/drivers/acpi/platform_profile.c > @@ -190,6 +190,20 @@ static ssize_t profile_show(struct device *dev, > return sysfs_emit(buf, "%s\n", profile_names[profile]); > } > > +/** > + * profile_notify_legacy - Notify the legacy sysfs interface > + * > + * This wrapper takes care of only notifying the legacy sysfs interface > + * if it was registered during module initialization. > + */ > +static void profile_notify_legacy(void) > +{ > + if (!acpi_kobj) > + return; > + > + sysfs_notify(acpi_kobj, NULL, "platform_profile"); > +} > + > /** > * profile_store - Set the profile for a class device > * @dev: The device > @@ -215,7 +229,7 @@ static ssize_t profile_store(struct device *dev, > return ret; > } > > - sysfs_notify(acpi_kobj, NULL, "platform_profile"); > + profile_notify_legacy(); > > return count; > } > @@ -435,7 +449,7 @@ static ssize_t platform_profile_store(struct kobject *kobj, > return ret; > } > > - sysfs_notify(acpi_kobj, NULL, "platform_profile"); > + profile_notify_legacy(); > > return count; > } > @@ -472,6 +486,22 @@ static const struct attribute_group platform_profile_group = { > .is_visible = profile_class_is_visible, > }; > > +/** > + * profile_update_legacy - Update the legacy sysfs interface > + * > + * This wrapper takes care of only updating the legacy sysfs interface > + * if it was registered during module initialization. > + * > + * Return: 0 on success or error code on failure. > + */ > +static int profile_update_legacy(void) > +{ > + if (!acpi_kobj) > + return 0; > + > + return sysfs_update_group(acpi_kobj, &platform_profile_group); > +} > + > /** > * platform_profile_notify - Notify class device and legacy sysfs interface > * @dev: The class device > @@ -481,7 +511,7 @@ void platform_profile_notify(struct device *dev) > scoped_cond_guard(mutex_intr, return, &profile_lock) { > _notify_class_profile(dev, NULL); > } > - sysfs_notify(acpi_kobj, NULL, "platform_profile"); > + profile_notify_legacy(); > } > EXPORT_SYMBOL_GPL(platform_profile_notify); > > @@ -529,7 +559,7 @@ int platform_profile_cycle(void) > return err; > } > > - sysfs_notify(acpi_kobj, NULL, "platform_profile"); > + profile_notify_legacy(); > > return 0; > } > @@ -603,9 +633,9 @@ struct device *platform_profile_register(struct device *dev, const char *name, > goto cleanup_ida; > } > > - sysfs_notify(acpi_kobj, NULL, "platform_profile"); > + profile_notify_legacy(); > > - err = sysfs_update_group(acpi_kobj, &platform_profile_group); > + err = profile_update_legacy(); > if (err) > goto cleanup_cur; > > @@ -639,8 +669,8 @@ void platform_profile_remove(struct device *dev) > ida_free(&platform_profile_ida, pprof->minor); > device_unregister(&pprof->dev); > > - sysfs_notify(acpi_kobj, NULL, "platform_profile"); > - sysfs_update_group(acpi_kobj, &platform_profile_group); > + profile_notify_legacy(); > + profile_update_legacy(); > } > EXPORT_SYMBOL_GPL(platform_profile_remove); > > @@ -692,16 +722,28 @@ static int __init platform_profile_init(void) > if (err) > return err; > > - err = sysfs_create_group(acpi_kobj, &platform_profile_group); > - if (err) > - class_unregister(&platform_profile_class); > + /* > + * The ACPI kobject can be missing if ACPI was disabled during booting. > + * We thus skip the initialization of the legacy sysfs interface in such > + * cases to allow the platform profile class to work on ARM64 notebooks > + * without ACPI support. I wouldn't say work as I'd expect that there are 0 registered platform_profile class devices. > + */ > + if (acpi_kobj) { > + err = sysfs_create_group(acpi_kobj, &platform_profile_group); > + if (err < 0) { > + class_unregister(&platform_profile_class); > + return err; > + } > + } > > - return err; > + return 0; > } > > static void __exit platform_profile_exit(void) > { > - sysfs_remove_group(acpi_kobj, &platform_profile_group); > + if (acpi_kobj) > + sysfs_remove_group(acpi_kobj, &platform_profile_group); > + > class_unregister(&platform_profile_class); > } > module_init(platform_profile_init); thanks, patch works on the affected system and the HID device for the Lenovo keyboard probes successfully. We still need to stub platform_profile_cycle() to get rid of the ACPI Kconfig dependency. I'll send that out separately. Reviewed-by: Janne Grunau <j@jannau.net> Tested-by: Janne Grunau <j@jannau.net>
Am 18.05.25 um 11:43 schrieb Janne Grunau: > Hej, > > On Sat, May 17, 2025 at 05:58:24PM +0200, Armin Wolf wrote: >> Am 16.05.25 um 01:05 schrieb Janne Grunau: >> >>> On Fri, May 16, 2025 at 12:05:11AM +0200, Armin Wolf wrote: >>>> Am 12.05.25 um 23:55 schrieb Janne Grunau via B4 Relay: >>>> >>>>> From: Janne Grunau <j@jannau.net> >>>>> >>>>> Commit 84c9d2a968c8 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd >>>>> Fn keys") added a dependency on ACPI_PLATFORM_PROFILE to cycle through >>>>> power profiles. This breaks USB and Bluetooth keyboards on non-ACPI >>>>> platforms since platform_profile_init() fails. See the warning below for >>>>> the visible symptom but cause is the dependency on the platform_profile >>>>> module. > [...] > >>>> i think we can fix that. We just have to skip the compat stuff if acpi_kobj is NULL (means that ACPI is not used). >>>> The modern platform profile interface is generic enough to also work on non-ACPI systems. >>>> >>>> Can you test a patch? >>> I can easily test patches >> Nice, i attached the necessary patch. Please keep in mind that this patch is compile-tested only. >> >> From: Armin Wolf <W_Armin@gmx.de> >> Date: Sat, 17 May 2025 17:45:09 +0200 >> Subject: [PATCH] ACPI: platform_profile: Add support for non-ACPI platforms >> >> Currently the platform profile subsystem assumes that all supported >> (i.e. ACPI-capable) platforms always run with ACPI being enabled. >> However some ARM64 notebooks do not support ACPI and are instead >> using devicetree for booting. >> >> Do not register the legacy sysfs interface on such devices as it >> depends on the acpi_kobj (/sys/firmware/acpi/) being present. Users >> are encouraged to use the new platform-profile class interface >> instead. >> >> Signed-off-by: Armin Wolf <W_Armin@gmx.de> >> --- >> drivers/acpi/platform_profile.c | 68 ++++++++++++++++++++++++++------- >> 1 file changed, 55 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c >> index ffbfd32f4cf1..c5a5da7d03f1 100644 >> --- a/drivers/acpi/platform_profile.c >> +++ b/drivers/acpi/platform_profile.c >> @@ -190,6 +190,20 @@ static ssize_t profile_show(struct device *dev, >> return sysfs_emit(buf, "%s\n", profile_names[profile]); >> } >> >> +/** >> + * profile_notify_legacy - Notify the legacy sysfs interface >> + * >> + * This wrapper takes care of only notifying the legacy sysfs interface >> + * if it was registered during module initialization. >> + */ >> +static void profile_notify_legacy(void) >> +{ >> + if (!acpi_kobj) >> + return; >> + >> + sysfs_notify(acpi_kobj, NULL, "platform_profile"); >> +} >> + >> /** >> * profile_store - Set the profile for a class device >> * @dev: The device >> @@ -215,7 +229,7 @@ static ssize_t profile_store(struct device *dev, >> return ret; >> } >> >> - sysfs_notify(acpi_kobj, NULL, "platform_profile"); >> + profile_notify_legacy(); >> >> return count; >> } >> @@ -435,7 +449,7 @@ static ssize_t platform_profile_store(struct kobject *kobj, >> return ret; >> } >> >> - sysfs_notify(acpi_kobj, NULL, "platform_profile"); >> + profile_notify_legacy(); >> >> return count; >> } >> @@ -472,6 +486,22 @@ static const struct attribute_group platform_profile_group = { >> .is_visible = profile_class_is_visible, >> }; >> >> +/** >> + * profile_update_legacy - Update the legacy sysfs interface >> + * >> + * This wrapper takes care of only updating the legacy sysfs interface >> + * if it was registered during module initialization. >> + * >> + * Return: 0 on success or error code on failure. >> + */ >> +static int profile_update_legacy(void) >> +{ >> + if (!acpi_kobj) >> + return 0; >> + >> + return sysfs_update_group(acpi_kobj, &platform_profile_group); >> +} >> + >> /** >> * platform_profile_notify - Notify class device and legacy sysfs interface >> * @dev: The class device >> @@ -481,7 +511,7 @@ void platform_profile_notify(struct device *dev) >> scoped_cond_guard(mutex_intr, return, &profile_lock) { >> _notify_class_profile(dev, NULL); >> } >> - sysfs_notify(acpi_kobj, NULL, "platform_profile"); >> + profile_notify_legacy(); >> } >> EXPORT_SYMBOL_GPL(platform_profile_notify); >> >> @@ -529,7 +559,7 @@ int platform_profile_cycle(void) >> return err; >> } >> >> - sysfs_notify(acpi_kobj, NULL, "platform_profile"); >> + profile_notify_legacy(); >> >> return 0; >> } >> @@ -603,9 +633,9 @@ struct device *platform_profile_register(struct device *dev, const char *name, >> goto cleanup_ida; >> } >> >> - sysfs_notify(acpi_kobj, NULL, "platform_profile"); >> + profile_notify_legacy(); >> >> - err = sysfs_update_group(acpi_kobj, &platform_profile_group); >> + err = profile_update_legacy(); >> if (err) >> goto cleanup_cur; >> >> @@ -639,8 +669,8 @@ void platform_profile_remove(struct device *dev) >> ida_free(&platform_profile_ida, pprof->minor); >> device_unregister(&pprof->dev); >> >> - sysfs_notify(acpi_kobj, NULL, "platform_profile"); >> - sysfs_update_group(acpi_kobj, &platform_profile_group); >> + profile_notify_legacy(); >> + profile_update_legacy(); >> } >> EXPORT_SYMBOL_GPL(platform_profile_remove); >> >> @@ -692,16 +722,28 @@ static int __init platform_profile_init(void) >> if (err) >> return err; >> >> - err = sysfs_create_group(acpi_kobj, &platform_profile_group); >> - if (err) >> - class_unregister(&platform_profile_class); >> + /* >> + * The ACPI kobject can be missing if ACPI was disabled during booting. >> + * We thus skip the initialization of the legacy sysfs interface in such >> + * cases to allow the platform profile class to work on ARM64 notebooks >> + * without ACPI support. > I wouldn't say work as I'd expect that there are 0 registered > platform_profile class devices. Just as expected, currently all drivers registering with the platform profile subsystem are depending on ACPI. In the future this might change. >> + */ >> + if (acpi_kobj) { >> + err = sysfs_create_group(acpi_kobj, &platform_profile_group); >> + if (err < 0) { >> + class_unregister(&platform_profile_class); >> + return err; >> + } >> + } >> >> - return err; >> + return 0; >> } >> >> static void __exit platform_profile_exit(void) >> { >> - sysfs_remove_group(acpi_kobj, &platform_profile_group); >> + if (acpi_kobj) >> + sysfs_remove_group(acpi_kobj, &platform_profile_group); >> + >> class_unregister(&platform_profile_class); >> } >> module_init(platform_profile_init); > thanks, patch works on the affected system and the HID device for the > Lenovo keyboard probes successfully. We still need to stub > platform_profile_cycle() to get rid of the ACPI Kconfig dependency. I'll > send that out separately. > > Reviewed-by: Janne Grunau <j@jannau.net> > Tested-by: Janne Grunau <j@jannau.net> > Alright, i will send this patch to the ACPI mailing list ASAP. Please keep in mind that merely stubbing out the affected functions is not enough, as the platform profile code needs to be moved out of drivers/acpi/ as well. Thanks, Armin Wolf
Hej, On Sun, May 18, 2025 at 07:46:12PM +0200, Armin Wolf wrote: > Am 18.05.25 um 11:43 schrieb Janne Grunau: > > >> > >> static void __exit platform_profile_exit(void) > >> { > >> - sysfs_remove_group(acpi_kobj, &platform_profile_group); > >> + if (acpi_kobj) > >> + sysfs_remove_group(acpi_kobj, &platform_profile_group); > >> + > >> class_unregister(&platform_profile_class); > >> } > >> module_init(platform_profile_init); > > thanks, patch works on the affected system and the HID device for the > > Lenovo keyboard probes successfully. We still need to stub > > platform_profile_cycle() to get rid of the ACPI Kconfig dependency. I'll > > send that out separately. > > > > Reviewed-by: Janne Grunau <j@jannau.net> > > Tested-by: Janne Grunau <j@jannau.net> > > > Alright, i will send this patch to the ACPI mailing list ASAP. Please keep in mind > that merely stubbing out the affected functions is not enough, as the platform profile code > needs to be moved out of drivers/acpi/ as well. Thanks. I will go with patch which #ifdefs the platform_profile_cycle() call out. It is only used for a special detachable tablet keyboard which is ACPI enabled device anyway. So it will be hid-lenovo local change. Janne
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index a503252702b7b43c332a12b14bc8b23b83e9f028..6b4445f54b2f2818d451ff28b3f7bcc2b1c7e99f 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -596,7 +596,6 @@ config HID_LED config HID_LENOVO tristate "Lenovo / Thinkpad devices" depends on ACPI - select ACPI_PLATFORM_PROFILE select NEW_LEDS select LEDS_CLASS help diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c index af29ba840522f99bc2f426d4753f70d442cef3af..cff4b5ddd9aa9bad0516f8c9beb58927c24477fc 100644 --- a/drivers/hid/hid-lenovo.c +++ b/drivers/hid/hid-lenovo.c @@ -32,8 +32,6 @@ #include <linux/leds.h> #include <linux/workqueue.h> -#include <linux/platform_profile.h> - #include "hid-ids.h" /* Userspace expects F20 for mic-mute KEY_MICMUTE does not work */ @@ -729,8 +727,7 @@ static int lenovo_raw_event_TP_X12_tab(struct hid_device *hdev, u32 raw_data) report_key_event(input, KEY_RFKILL); 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) ?