Re: [PATCH v4 14/20] ACPI: platform_profile: Only show profiles common for all handlers

[Mario Limonciello <mario.limonciello@amd.com>]

On 11/6/2024 13:41, Armin Wolf wrote:
> Am 05.11.24 um 16:33 schrieb Mario Limonciello:
> 
>> If multiple platform profile handlers have been registered, don't allow
>> switching to profiles unique to only one handler.
>>
>> Tested-by: Matthew Schwartz <matthew.schwartz@linux.dev>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/acpi/platform_profile.c | 55 +++++++++++++++++++++++----------
>>   1 file changed, 38 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/ 
>> platform_profile.c
>> index c9917daf683cb..8de7c8352673e 100644
>> --- a/drivers/acpi/platform_profile.c
>> +++ b/drivers/acpi/platform_profile.c
>> @@ -235,27 +235,48 @@ static const struct class platform_profile_class 
>> = {
>>       .dev_groups = profile_groups,
>>   };
>>
>> -static ssize_t platform_profile_choices_show(struct device *dev,
>> -                    struct device_attribute *attr,
>> -                    char *buf)
>> +/**
>> + * _aggregate_choices - Aggregate the available profile choices
>> + * @dev: The device
>> + * @data: The available profile choices
>> + * Return: 0 on success, -errno on failure
>> + */
>> +static int _aggregate_choices(struct device *dev, void *data)
>>   {
>> -    int len = 0;
>> -    int i;
>> +    unsigned long *aggregate = data;
>> +    unsigned long choices = 0;
>> +    int err;
>>
>> -    scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) {
>> -        if (!cur_profile)
>> -            return -ENODEV;
>> +    err = _get_class_choices(dev, &choices);
>> +    if (err)
>> +        return err;
>>
>> -        for_each_set_bit(i, cur_profile->choices, 
>> PLATFORM_PROFILE_LAST) {
>> -            if (len == 0)
>> -                len += sysfs_emit_at(buf, len, "%s", profile_names[i]);
>> -            else
>> -                len += sysfs_emit_at(buf, len, " %s", profile_names[i]);
>> -        }
>> -    }
>> -    len += sysfs_emit_at(buf, len, "\n");
>> +    if (!*aggregate)
>> +        *aggregate = choices;
>> +    else
>> +        *aggregate &= choices;
>>
>> -    return len;
>> +    return 0;
>> +}
>> +
>> +/**
>> + * platform_profile_choices_show - Show the available profile choices 
>> for legacy sysfs interface
>> + * @dev: The device
>> + * @attr: The attribute
>> + * @buf: The buffer to write to
>> + * Return: The number of bytes written
>> + */
>> +static ssize_t platform_profile_choices_show(struct device *dev,
>> +                         struct device_attribute *attr,
>> +                         char *buf)
>> +{
>> +    unsigned long aggregate = 0;
>> +    int err;
>> +
>> +    err = class_for_each_device(&platform_profile_class, NULL,
>> +                    &aggregate, _aggregate_choices);
> 
> Error handling seems to be missing.
> 
Yup, noticed that when unloading things exploded.
Fixing in the next version.

> Thanks,
> Armin Wolf
> 
>> +
>> +    return _commmon_choices_show(aggregate, buf);
>>   }
>>
>>   static ssize_t platform_profile_show(struct device *dev,