Re: [PATCH 2/2 v1] iio: light: Add a driver for Sharp GP2AP002x00F

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2 Jan 2020 14:15:33 -0800
Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:

> 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.

That's a perfectly valid method as long as ret is only ever an error
(or good).  We've tended to go with ret in IIO, so better to carry on with
that. 

I'm not that fussed about dropping the return ret; out, but definitely
prefer explicit if (ret) to make it clear ret is never positive in the
good path though.

Thanks,

Jonathan

> 
> Thanks.
> 




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux