Hi Rafael,
On 16/12/2020 13:47, Rafael J. Wysocki wrote:
> On Wed, Dec 16, 2020 at 7:42 PM Barnabás Pőcze <pobrn@xxxxxxxxxxxxxx> wrote:
>>
>> 2020. december 16., szerda 19:13 keltezéssel, Rafael J. Wysocki írta:
>>
>>> On Fri, Dec 11, 2020 at 3:15 AM Mark Pearson <markpearson@xxxxxxxxxx> wrote:
>>>>
>>>> This is the initial implementation of the platform-profile feature.
>>>> It provides the details discussed and outlined in the
>>>> sysfs-platform_profile document.
>>>>
>>>> Many modern systems have the ability to modify the operating profile to
>>>> control aspects like fan speed, temperature and power levels. This
>>>> module provides a common sysfs interface that platform modules can register
>>>> against to control their individual profile options.
>>>>
>>>> Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx>
>>> [...]
>>>> +enum platform_profile_option {
>>>> + PLATFORM_PROFILE_LOW,
>>>> + PLATFORM_PROFILE_COOL,
>>>> + PLATFORM_PROFILE_QUIET,
>>>> + PLATFORM_PROFILE_BALANCED,
>>>> + PLATFORM_PROFILE_PERFORM,
>>>> + PLATFORM_PROFILE_LAST, /*must always be last */
>>>> +};
>>>> +
>>>> +struct platform_profile_handler {
>>>> + unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];
>>>> + int (*profile_get)(enum platform_profile_option *profile);
>>>
>>> I'm not sure why this callback is necessary and, provided that there
>>> is a good enough reason, why it cannot return an enum
>>> platform_profile_option value.
>>>
>>> In principle, if ->profile_set() returns 0, the requested profile can
>>> be saved in a static var and then returned by subsequent "read"
>>> operations.
>>>
>>
>> It is possible that the platform profile can be changed with (e.g.) a dedicated
>> button (commonly found on laptops), in which case there needs to be a mechanism
>> to retrieve the new profile, which would not be possible without introducing
>> something else in place of that getter - unless I'm missing something obvious.
>
> Fair enough.
>
> The other question remains, then.
>
My thinking here that I shouldn't make assumptions for future platform
implementations - there may be valid cases in the future where being
able to return an error condition if there was an error would be useful.
Just trying to keep this somewhat future proof. Returning an error
condition seemed a useful thing to have available.
Mark
