On Wed, Jun 14, 2023 at 03:50:36PM +0200, Alexandre Belloni wrote: > On 14/06/2023 15:16:14+0300, Andy Shevchenko wrote: > > On Tue, Jun 13, 2023 at 11:26:51PM +0200, Alexandre Belloni wrote: > > > On 13/06/2023 18:20:56+0300, Andy Shevchenko wrote: > > > > On Tue, Jun 13, 2023 at 03:00:07PM +0200, Rasmus Villemoes wrote: ... > > > > > + ret = regmap_read(regmap, ISL12022_REG_SR, &val); > > > > > + if (ret < 0) > > > > > > > > I always feel uneasy with ' < 0' — Does positive error makes sense? > > > > Is it even possible? OTOH if the entire driver uses this idiom... > > > > oh well, let's make it consistent. > > > > > > /** > > > * regmap_read() - Read a value from a single register > > > * > > > * @map: Register map to read from > > > * @reg: Register to be read from > > > * @val: Pointer to store read value > > > * > > > * A value of zero will be returned on success, a negative errno will > > > * be returned in error cases. > > > */ > > > > I'm not sure what you meant by this. Yes, I know that there is no > > possibility that regmap API returns positive value. Do you mean that > > regmap API documentation is unclear? > > No, I mean that you'd have to be clearer as to why you are uneasy with a > test for a negative value when the function returns 0 for success and a > negative value for an error. Else, this is pure bullying. >From the perspective of the code reader, a person, who might have not known all the implementation details of the calls this kind of check will always puzzle about positive value. When reading such a code the following questions are arisen: 1) Can the positive return value be the case? 2) If so, what is the meaning of a such? 3) Why do we not care about it? All this can simply gone if we use ret = foo(...); if (ret) return ret; As it's clear that whatever is non-zero we accept as something to be promoted to the upper layer. I hope this explains my position. > > > > > + return ret; -- With Best Regards, Andy Shevchenko
