Re: [PATCH] HID: lenovo: Unbreak USB/BT keyboards on non-ACPI platforms
From: Armin Wolf
Date: Sun May 18 2025 - 13:46:41 EST
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@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.
[...]
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@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.
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@xxxxxxxxxx>
Tested-by: Janne Grunau <j@xxxxxxxxxx>
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