Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 24 May 2021 15:47:47 +0300 | Subject | Re: [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
| |