On 12/5/2024 08:22, Ilpo Järvinen wrote:
On Sun, 1 Dec 2024, Mario Limonciello wrote:
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.
Reviewed-by: Armin Wolf <W_Armin@xxxxxx>
Tested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Reviewed-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
drivers/acpi/platform_profile.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
index d5f0679d59d50..16746d9b9aa7c 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -407,25 +407,37 @@ EXPORT_SYMBOL_GPL(platform_profile_notify);
int platform_profile_cycle(void)
{
- enum platform_profile_option profile;
- enum platform_profile_option next;
+ enum platform_profile_option next = PLATFORM_PROFILE_LAST;
+ enum platform_profile_option profile = PLATFORM_PROFILE_LAST;
+ unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];
int err;
+ set_bit(PLATFORM_PROFILE_LAST, choices);
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);
+ if (profile == PLATFORM_PROFILE_CUSTOM ||
+ profile == PLATFORM_PROFILE_LAST)
+ return -EINVAL;
+
+ 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,
+ /* never iterate into a custom if all drivers supported it */
+ clear_bit(PLATFORM_PROFILE_CUSTOM, choices);
I'm confused by the comment. I was under impression the custom "profile"
is just a framework construct when the _framework_ couldn't find a
consistent profile? How can a driver decide to "support it"? It sounds
like a driver overstepping its intended domain of operation.
If the intention really is for the driver to "support" or "not support"
custom profile, then you should adjust the commit message of the patch
which introduced it.
Yes I had envisioned that a driver could potentially set custom as well.
This idea was introduced by my RFC series that precluded doing the
multiple driver handlers.
The basic idea is that some drivers (for example asus-wmi and
asus-armoury) have the ability for the user to change a sysfs file that
represents sPPT or fPPT directly.
If this has been done they're "off the beating path" of a predfined
profile because they're picking and choosing individual knobs.
So if a user touches those a driver could set profile as "custom" and if
a user chooses the platform profile the driver will override all of
those and report a pre-defined profile.
So, yes I had that all in my mind but as you point out I definitely
forgot to mention it in the commit messages.
Do you agree with it? If so, I'll amend the next version where
applicable (probably the patch that introduces custom and the
documentation patch).
