| Message ID | 20250224195059.10185-4-lkml@antheas.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI: platform_profile: fix legacy sysfs with multiple handlers | expand |
Hi Antheas,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.14-rc4 next-20250227]
[cannot apply to rafael-pm/linux-next rafael-pm/bleeding-edge]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Antheas-Kapenekakis/ACPI-platform_profile-Add-support-for-secondary-handlers/20250225-035455
base: linus/master
patch link: https://lore.kernel.org/r/20250224195059.10185-4-lkml%40antheas.dev
patch subject: [PATCH 3/3] ACPI: platform_profile: Do not hide options missing in secondary handlers
config: i386-buildonly-randconfig-003-20250227 (https://download.01.org/0day-ci/archive/20250228/202502280150.DkqQsO8C-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250228/202502280150.DkqQsO8C-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502280150.DkqQsO8C-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/acpi/platform_profile.c:248: warning: Function parameter or struct member 'secondary' not described in '_aggregate_choices'
vim +248 drivers/acpi/platform_profile.c
77be5cacb2c2d8 Mario Limonciello 2024-12-05 239
06ec24388f1de6 Mario Limonciello 2024-12-05 240 /**
06ec24388f1de6 Mario Limonciello 2024-12-05 241 * _aggregate_choices - Aggregate the available profile choices
06ec24388f1de6 Mario Limonciello 2024-12-05 242 * @dev: The device
06ec24388f1de6 Mario Limonciello 2024-12-05 243 * @data: The available profile choices
06ec24388f1de6 Mario Limonciello 2024-12-05 244 *
06ec24388f1de6 Mario Limonciello 2024-12-05 245 * Return: 0 on success, -errno on failure
06ec24388f1de6 Mario Limonciello 2024-12-05 246 */
724b0dfe3faddb Antheas Kapenekakis 2025-02-24 247 static int _aggregate_choices(struct device *dev, void *data, bool secondary)
06ec24388f1de6 Mario Limonciello 2024-12-05 @248 {
06ec24388f1de6 Mario Limonciello 2024-12-05 249 struct platform_profile_handler *handler;
06ec24388f1de6 Mario Limonciello 2024-12-05 250 unsigned long *aggregate = data;
06ec24388f1de6 Mario Limonciello 2024-12-05 251
06ec24388f1de6 Mario Limonciello 2024-12-05 252 lockdep_assert_held(&profile_lock);
d960f14800b581 Kurt Borja 2025-01-15 253 handler = to_pprof_handler(dev);
724b0dfe3faddb Antheas Kapenekakis 2025-02-24 254
724b0dfe3faddb Antheas Kapenekakis 2025-02-24 255 if (handler->ops->secondary != secondary)
724b0dfe3faddb Antheas Kapenekakis 2025-02-24 256 return 0;
724b0dfe3faddb Antheas Kapenekakis 2025-02-24 257
06ec24388f1de6 Mario Limonciello 2024-12-05 258 if (test_bit(PLATFORM_PROFILE_LAST, aggregate))
06ec24388f1de6 Mario Limonciello 2024-12-05 259 bitmap_copy(aggregate, handler->choices, PLATFORM_PROFILE_LAST);
724b0dfe3faddb Antheas Kapenekakis 2025-02-24 260 else if (handler->ops->secondary)
724b0dfe3faddb Antheas Kapenekakis 2025-02-24 261 bitmap_or(aggregate, handler->choices, aggregate, PLATFORM_PROFILE_LAST);
06ec24388f1de6 Mario Limonciello 2024-12-05 262 else
06ec24388f1de6 Mario Limonciello 2024-12-05 263 bitmap_and(aggregate, handler->choices, aggregate, PLATFORM_PROFILE_LAST);
06ec24388f1de6 Mario Limonciello 2024-12-05 264
06ec24388f1de6 Mario Limonciello 2024-12-05 265 return 0;
06ec24388f1de6 Mario Limonciello 2024-12-05 266 }
06ec24388f1de6 Mario Limonciello 2024-12-05 267
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index 2ad53cc6aae5..55e8bb6adf8e 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -63,17 +63,18 @@ static ssize_t _commmon_choices_show(unsigned long *choices, char *buf) * _store_class_profile - Set the profile for a class device * @dev: The class device * @data: The profile to set + * @enforce_valid: For secondary handlers, enforce that the profile is valid * * Return: 0 on success, -errno on failure */ -static int _store_class_profile(struct device *dev, void *data) +static int _store_class_profile(struct device *dev, void *data, bool enforce_valid) { struct platform_profile_handler *handler; int *bit = (int *)data; lockdep_assert_held(&profile_lock); handler = to_pprof_handler(dev); - if (!test_bit(*bit, handler->choices)) + if ((enforce_valid || !handler->ops->secondary) && !test_bit(*bit, handler->choices)) return -EOPNOTSUPP; return handler->ops->profile_set(dev, *bit); @@ -204,7 +205,7 @@ static ssize_t profile_store(struct device *dev, return -EINVAL; scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { - ret = _store_class_profile(dev, &index); + ret = _store_class_profile(dev, &index, true); if (ret) return ret; } @@ -243,21 +244,37 @@ static const struct class platform_profile_class = { * * Return: 0 on success, -errno on failure */ -static int _aggregate_choices(struct device *dev, void *data) +static int _aggregate_choices(struct device *dev, void *data, bool secondary) { struct platform_profile_handler *handler; unsigned long *aggregate = data; lockdep_assert_held(&profile_lock); handler = to_pprof_handler(dev); + + if (handler->ops->secondary != secondary) + return 0; + if (test_bit(PLATFORM_PROFILE_LAST, aggregate)) bitmap_copy(aggregate, handler->choices, PLATFORM_PROFILE_LAST); + else if (handler->ops->secondary) + bitmap_or(aggregate, handler->choices, aggregate, PLATFORM_PROFILE_LAST); else bitmap_and(aggregate, handler->choices, aggregate, PLATFORM_PROFILE_LAST); return 0; } +static int _aggregate_choices_primary(struct device *dev, void *data) +{ + return _aggregate_choices(dev, data, false); +} + +static int _aggregate_choices_secondary(struct device *dev, void *data) +{ + return _aggregate_choices(dev, data, true); +} + /** * platform_profile_choices_show - Show the available profile choices for legacy sysfs interface * @dev: The device @@ -276,9 +293,16 @@ static ssize_t platform_profile_choices_show(struct device *dev, set_bit(PLATFORM_PROFILE_LAST, aggregate); scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { err = class_for_each_device(&platform_profile_class, NULL, - aggregate, _aggregate_choices); + aggregate, _aggregate_choices_primary); if (err) return err; + + if (test_bit(PLATFORM_PROFILE_LAST, aggregate)) { + err = class_for_each_device(&platform_profile_class, NULL, + aggregate, _aggregate_choices_secondary); + if (err) + return err; + } } /* no profile handler registered any more */ @@ -325,7 +349,7 @@ static int _store_and_notify(struct device *dev, void *data) enum platform_profile_option *profile = data; int err; - err = _store_class_profile(dev, profile); + err = _store_class_profile(dev, profile, false); if (err) return err; return _notify_class_profile(dev, NULL); @@ -384,9 +408,18 @@ static ssize_t platform_profile_store(struct device *dev, set_bit(PLATFORM_PROFILE_LAST, choices); scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { ret = class_for_each_device(&platform_profile_class, NULL, - choices, _aggregate_choices); + choices, _aggregate_choices_primary); if (ret) return ret; + + if (test_bit(PLATFORM_PROFILE_LAST, choices)) { + ret = class_for_each_device( + &platform_profile_class, NULL, choices, + _aggregate_choices_secondary); + if (ret) + return ret; + } + if (!test_bit(i, choices)) return -EOPNOTSUPP; @@ -470,10 +503,18 @@ int platform_profile_cycle(void) return -EINVAL; err = class_for_each_device(&platform_profile_class, NULL, - choices, _aggregate_choices); + choices, _aggregate_choices_primary); if (err) return err; + if (test_bit(PLATFORM_PROFILE_LAST, choices)) { + err = class_for_each_device( + &platform_profile_class, NULL, choices, + _aggregate_choices_secondary); + if (err) + return err; + } + /* never iterate into a custom if all drivers supported it */ clear_bit(PLATFORM_PROFILE_CUSTOM, choices);
In the legacy endpoint, previously, only the options common to all handlers were exposed. This causes an issue when the primary handler of the device has its options hidden, as it results in a performance degradation. Therefore, this commit introduces the concept of secondary handlers. These are handlers that are able to accept all options and do not partake in the process of selecting which profiles are visible. These handlers still have a probe function which is used for their endpoint and their endpoint works normally and will block other options. However, when called from the legacy endpoint, all options will be sent to them. It is the expectation that secondary handlers will pick the closest profile they have to what was sent. In the absence of a primary handler, the options shown in the legacy endpoint will be the union of all options of all secondary handlers. Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev> --- drivers/acpi/platform_profile.c | 57 ++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 8 deletions(-)