Re: [PATCH v3] platform/x86: Add driver for ACPI WMAA EC-based backlight control

[Daniel Dadap <ddadap@nvidia.com>]

On 8/26/21 8:35 AM, Andy Shevchenko wrote:
> On Wed, Aug 25, 2021 at 7:48 PM Daniel Dadap <ddadap@nvidia.com> wrote:
>> On 8/25/21 4:05 AM, Andy Shevchenko wrote:
>>> On Wed, Aug 25, 2021 at 1:09 AM Daniel Dadap <ddadap@nvidia.com> wrote:
> ...
>
>>>> +struct wmaa_args {
>>>> +       u32 set;
>>>> +       u32 val;
>>>> +       u32 ret;
>>>> +       u32 ignored[3];
>>>> +};
>>> I guess this structure deserves a kernel doc.
>> Do you have a recommended location? From a quick skim I didn't see any
>> document in Documentation/ that seemed most appropriate to add this to.
> It's in a form of the comment on top of the data structure
>
> /**
>   * struct wmaa_args ....
>   * ...
>   */
>
> ...


Ah, of course. Sorry for misunderstanding.


>>>> +       WARN_ON(ret != 0);
>>> Why?
>> To differentiate a 0 because the level is actually 0 versus a 0 because
>> there was an error. The backlight device API doesn't seem to have a way
>> to report errors.
> I meant why do you need WARN_ON() here? This kind of stuff must be justified.


Thanks, I see what you mean now. I'll change it to a dev_warn() or maybe 
even a dev_err(), since if the ACPI call does fail there probably is 
something quite amiss.