On Wed, 30 Oct 2024, Mario Limonciello wrote: > If multiple platform profile handlers have been registered then when > setting a profile verify that all profile handlers support the requested > profile and set it to each handler. > > If this fails for any given handler, revert all profile handlers back to > balanced and log an error into the kernel ring buffer. > > Tested-by: Matthew Schwartz <matthew.schwartz@xxxxxxxxx> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > drivers/acpi/platform_profile.c | 25 ++++++++++++++++++++----- > 1 file changed, 20 insertions(+), 5 deletions(-) > > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c > index 90cbc0de4d5bc..c2bb325ba531c 100644 > --- a/drivers/acpi/platform_profile.c > +++ b/drivers/acpi/platform_profile.c > @@ -99,6 +99,8 @@ static ssize_t platform_profile_store(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t count) > { > + struct platform_profile_handler *handler; > + unsigned long choices; > int err, i; > > /* Scan for a matching profile */ > @@ -107,16 +109,29 @@ static ssize_t platform_profile_store(struct device *dev, > return -EINVAL; > > scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { > - if (!cur_profile) > + if (!platform_profile_is_registered()) > return -ENODEV; > > - /* Check that platform supports this profile choice */ > - if (!test_bit(i, cur_profile->choices)) > + /* Check that all handlers support this profile choice */ > + choices = platform_profile_get_choices(); > + if (!test_bit(i, &choices)) > return -EOPNOTSUPP; > > - err = cur_profile->profile_set(cur_profile, i); > - if (err) > + list_for_each_entry(handler, &platform_profile_handler_list, list) { > + err = handler->profile_set(handler, i); > + if (err) { > + pr_err("Failed to set profile for handler %s\n", handler->name); > + break; > + } > + } > + if (err) { > + list_for_each_entry_continue_reverse(handler, &platform_profile_handler_list, list) { Too long line. This looks an error rollback though so instead of break inside the loop you could goto into a label at the end of the function and have much less indentation to begin with. > + if (handler->profile_set(handler, PLATFORM_PROFILE_BALANCED)) > + pr_err("Failed to revert profile for handler %s\n", > + handler->name); > + } > return err; > + } > } > > sysfs_notify(acpi_kobj, NULL, "platform_profile"); > -- i.