Am 07.11.24 um 23:05 schrieb Mario Limonciello:
On 11/7/2024 02:58, Armin Wolf wrote:
Am 07.11.24 um 07:02 schrieb Mario Limonciello:
As multiple platform profile handlers might not all support the same
profile, cycling to the next profile could have a different result
depending on what handler are registered.
Check what is active and supported by all handlers to decide what
to do.
Tested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
v5:
* Adjust mutex use
---
drivers/acpi/platform_profile.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/
platform_profile.c
index 7f302ac4d3779..2c466f2d16b42 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -411,34 +411,39 @@ EXPORT_SYMBOL_GPL(platform_profile_notify);
int platform_profile_cycle(void)
{
+ enum platform_profile_option next = PLATFORM_PROFILE_LAST;
enum platform_profile_option profile;
- enum platform_profile_option next;
+ unsigned long choices;
int err;
if (!class_is_registered(&platform_profile_class))
return -ENODEV;
scoped_cond_guard(mutex_intr, return -ERESTARTSYS,
&profile_lock) {
- if (!cur_profile)
- return -ENODEV;
+ err = class_for_each_device(&platform_profile_class, NULL,
+ &profile, _aggregate_profiles);
+ if (err)
+ return err;
- err = cur_profile->profile_get(cur_profile, &profile);
+ err = class_for_each_device(&platform_profile_class, NULL,
+ &choices, _aggregate_choices);
if (err)
return err;
- next = find_next_bit_wrap(cur_profile->choices,
PLATFORM_PROFILE_LAST,
+ next = find_next_bit_wrap(&choices,
+ PLATFORM_PROFILE_LAST,
profile + 1);
Could it be that this would lead to be "custom" profile being
selected under some conditions?
Yeah, you're right. If all drivers supported custom then this could
happen. I'll clear custom like this:
choices &= ~BIT(PLATFORM_PROFILE_CUSTOM);
Sound good to me.
Also _aggregate_profiles() expects profile to be initialized with
PLATFORM_PROFILE_LAST.
Will correct initialization in platform_profile_cycle() to this.
enum platform_profile_option profile = PLATFORM_PROFILE_LAST;
But this also raises a good point. If _aggregate_profiles() returns
custom then this should be an error because next profile is undefined.
So I'll catch that like this.
err = class_for_each_device()
if (err)
return err;
if (profile == PLATFORM_PROFILE_CUSTOM)
return -EINVAL;
Good point, please also check if profile == PLATFORM_PROFILE_LAST in case no platform profile handlers are currently installed.
Thanks,
Armin Wol
Thanks,
Armin Wolf
- if (WARN_ON(next == PLATFORM_PROFILE_LAST))
- return -EINVAL;
+ err = class_for_each_device(&platform_profile_class, NULL,
&next,
+ _store_class_profile);
- err = cur_profile->profile_set(cur_profile, next);
if (err)
return err;
}
sysfs_notify(acpi_kobj, NULL, "platform_profile");
- return 0;
+
+ return err;
}
EXPORT_SYMBOL_GPL(platform_profile_cycle);
