| Message ID | 20241109044151.29804-19-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add support for binding ACPI platform profile to multiple drivers | expand |
Am 09.11.24 um 05:41 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. Reviewed-by: Armin Wolf <W_Armin@gmx.de> > Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > v6: > * Handle cases of inconsistent profiles or all profile handlers > supporting custom. > v5: > * Adjust mutex use > --- > 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 2676f4a13689e..c574483be4fd1 100644 > --- a/drivers/acpi/platform_profile.c > +++ b/drivers/acpi/platform_profile.c > @@ -423,28 +423,40 @@ 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; > > if (!class_is_registered(&platform_profile_class)) > return -ENODEV; > > + 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); > + > + next = find_next_bit_wrap(choices, > + PLATFORM_PROFILE_LAST, > profile + 1); > > - 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; > }
Am 18.11.24 um 20:53 schrieb Armin Wolf: > Am 09.11.24 um 05:41 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. > > Reviewed-by: Armin Wolf <W_Armin@gmx.de> > >> Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> v6: >> * Handle cases of inconsistent profiles or all profile handlers >> supporting custom. >> v5: >> * Adjust mutex use >> --- >> 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 2676f4a13689e..c574483be4fd1 100644 >> --- a/drivers/acpi/platform_profile.c >> +++ b/drivers/acpi/platform_profile.c >> @@ -423,28 +423,40 @@ 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; >> >> if (!class_is_registered(&platform_profile_class)) >> return -ENODEV; >> >> + 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); >> + >> + next = find_next_bit_wrap(choices, >> + PLATFORM_PROFILE_LAST, >> profile + 1); >> >> - if (WARN_ON(next == PLATFORM_PROFILE_LAST)) >> - return -EINVAL; >> + err = class_for_each_device(&platform_profile_class, NULL, >> &next, >> + _store_class_profile); I just noticed that the class device is not notified here. Please use _store_and_notify() insted of _store_class_profile() here. Thanks, Armin Wolf >> >> - err = cur_profile->profile_set(cur_profile, next); >> if (err) >> return err; >> } >
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index 2676f4a13689e..c574483be4fd1 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -423,28 +423,40 @@ 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; if (!class_is_registered(&platform_profile_class)) return -ENODEV; + 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); + + next = find_next_bit_wrap(choices, + PLATFORM_PROFILE_LAST, profile + 1); - 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; }