On Mon, May 24, 2021 at 3:04 PM Sander Vanheule <sander@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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
