On Tue, Oct 25, 2022 at 10:00:07AM +0000, Vaittinen, Matti wrote: > On 10/25/22 12:18, Andy Shevchenko wrote: > > On Tue, Oct 25, 2022 at 11:50:59AM +0300, Matti Vaittinen wrote: ... > >> + ret = fwnode_irq_get(fwnode, index); > > > >> + > > > > Redundant blank line and better to use traditional pattern: > > >> + if (!ret) > >> + return -EINVAL; > >> + > >> + return ret; > > > > if (ret) > > return ret; > > > > /* We treat mapping errors as invalid case */ > > return -EINVAL; > > > >> } > > I like the added comment - but in this case I don't prefer the > "traditional pattern" you suggest. We do check for a very special error > case indicated by ret 0. > > if (!ret) > return -EINVAL; > > makes it obvious the zero is special error. I don't think so. It makes ! easily to went through the cracks. If you want an explicit, use ' == 0' and add a comment. > if (ret) > return ret; > > the traditional pattern makes this look like traditional error return - > which it is not. The comment you added is nice though. It could be just > before the check for > > if (!ret). -- With Best Regards, Andy Shevchenko
