Hi Jonathan, On Mon, Dec 30, 2019 at 05:39:19PM +0000, Jonathan Cameron wrote: > On Sat, 28 Dec 2019 21:11:09 +0100 > Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > > + /* Enable chip and IRQ, disable analog sleep */ > > + ret = regmap_write(gp2ap002->map, GP2AP002_OPMOD, > > + OPMOD_SSD_OPERATING | OPMOD_VCON_IRQ); > > + if (ret < 0) { > > + dev_err(gp2ap002->dev, "error setting up operation mode\n"); > > + return ret; > > + } > > + > > + /* Interrupt on VOUT enabled */ > > + ret = regmap_write(gp2ap002->map, GP2AP002_CON, CON_OCON_ENABLE); > > + if (ret < 0) { > > + dev_err(gp2ap002->dev, "error setting up VOUT control\n"); > > + return ret; > > drop this return ret out of the brackets as it's either 0 or negative anyway. Not my subsystem, but $0.02 anyways: I like calling the temp as "error" and explicitly return 0 in the success path even if it could be collapsed, as you can easily add more initialization without needing to go and alter previous blocks. Thanks. -- Dmitry