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.
> + */
> + 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>
