Re: [PATCH] HID: lenovo: Unbreak USB/BT keyboards on non-ACPI platforms
From: Armin Wolf
Date: Sat May 17 2025 - 11:59:13 EST
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@xxxxxxxxxx>
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@xxxxxxxxxxxxxxx
Signed-off-by: Janne Grunau <j@xxxxxxxxxx>
------>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
From 738911c9b2c671afdd73747eee02b5e0fa1db428 Mon Sep 17 00:00:00 2001
From: Armin Wolf <W_Armin@xxxxxx>
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@xxxxxx>
---
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.
+ */
+ 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);
--
2.39.5