On 12/17/20 4:09 PM, Rafael J. Wysocki wrote:
On Thu, Dec 17, 2020 at 4:02 PM Barnabás Pőcze <pobrn@xxxxxxxxxxxxxx> wrote:
2020. december 17., csütörtök 14:36 keltezéssel, Rafael J. Wysocki <rafael@xxxxxxxxxx> írta:
[...]
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.
I'd like to understand what is better about that approach than having an
error and a returnable parameter?
I'm probably missing something obvious but if I add an extra
platform_profile option (e.g PLATFORM_PROFILE_ERROR) to be used in an
error case (and return just an enum platform_profile_option) it seems I
lose the granularity of being able to return a specific error condition.
It doesn't feel like an improvement.
And what's the user expected to do about the different error codes
that can be returned?
Even assuming the users of the API cannot or will not handle different errors
differently, who would benefit from getting rid of this information which may be
an aid in debugging for those who know what they're looking at?
And if a new enum value is introduced to signal an error condition, how is that
going to be communicated to the users? A magic string like "error" or "-1"?
Or under a single errno like -EIO?
Yes.
Personally, I believe neither of these two
solutions are better than returning the actual errno which is deemed most
appropriate by the platform profile handler. Or am I missing a third way?
Can we please defer this until we actually have a driver needing to
return different error values from ->get_profile() (and I find it hard
to believe that there will be any drivers like that)?
I can maybe give an example. On Microsoft Surface devices, the
performance mode / platform profile is set via a request to the embedded
controller and can be queried the same way. This request can fail (most
notably with ETIMEDOUT and EREMOTEIO). I think at least being able to
show users an error in this case would be helpful.
A driver for that is currently at [1], but I haven't adapted that yet to
the platform profile patchset.
On the other hand, I can also query and store the profile when loading
the driver and then update it when it's changed. That'd require that no
one else changes the profile, but I think we can safely assume that.
[1]: https://github.com/linux-surface/surface-aggregator-module/blob/master/module/src/clients/surface_perfmode.c,
Regards,
Max
Let's do the simpler thing until we have a real need to do the more
complicated one.
Otherwise we're preparing for things that may never happen.
