On Wed, Dec 16, 2020 at 8:19 PM Mark Pearson <markpearson@xxxxxxxxxx> wrote:
>
> 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.
You can still return an error while returning a platform_profile_option value.
Just add a special value representing an error to that set.
