lkml.org 
[lkml]   [2021]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3 6/6] leds: Add support for RTL8231 LED scan matrix
    On Mon, May 24, 2021 at 3:04 PM Sander Vanheule <sander@svanheule.net> wrote:
    > On Mon, 2021-05-24 at 13:24 +0300, Andy Shevchenko wrote:
    > > On Mon, May 24, 2021 at 1:34 AM Sander Vanheule <sander@svanheule.net> wrote:

    ...

    > > > + if (ret != 2)
    > > > + return -ENODEV;
    > >
    > > I would say -EINVAL, but -ENODEV is similarly okay.
    >
    > Any specific reason you think EINVAL is more appropriate than ENODEV?

    My logic is that the initial values (from resource provider) are incorrect.
    But as I said, I'm fine with either.

    ...

    > > > + int err;
    > >
    > > ret or err? Be consistent across a single driver.
    >
    > I had first used 'err' for both fwnode_property_count_u32() and
    > fwnode_property_read_u32_array(). The former returns "actual count or error
    > code", while the latter is only "error code". And I found it weird to read the
    > code as "does error code equal 2", if I used 'err' as variable name.
    >
    > I've split this up:
    > * addr_count for fwnode_property_count_u32's result
    > * err for fwnode_property_read_u32_array's result
    >
    > Since addr_count is only used before err is touched, I guess the compiler will
    > optimize this out anyway?

    Usually we do this pattern (and it seems you missed the point, name of
    variable is ret in some functions and err in the rest):

    err /* ret */ = foo();
    if (err < 0)
    return err;
    count = err;

    --
    With Best Regards,
    Andy Shevchenko

    \
     
     \ /
      Last update: 2021-05-24 14:49    [W:4.416 / U:0.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site