Re: [PATCH v3] iio: light: opt3001: split opt3001_get_processed() logic

[Joshua Crofts]

On Sun, 12 Jul 2026 16:58:55 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
> > +	ret = wait_event_timeout(opt->result_ready_queue,
> > +				 opt->result_ready,
> > +				 msecs_to_jiffies(OPT3001_RESULT_READY_LONG));
> > +	if (ret == 0)
> > +		ret = -ETIMEDOUT;  
> 
> ret is int and for holding an error code, with the above ret is rewritten
> with positive number and might lead to subtle issues. The recommended way
> to handle is
> 
> 	if (wait_event_timeout(opt->result_ready_queue, opt->result_ready,
> 			       msecs_to_jiffies(OPT3001_RESULT_READY_LONG)))
> 		ret = 0;
> 	else
> 		ret = -ETIMEDOUT;
> 
> or something like that.

Sure, I can do that.

...

> > +	if (ret < 0) {
> > +		dev_err(dev, "failed to write register %02x\n",
> > +			OPT3001_LOW_LIMIT);
> > +		return ret;
> >  	}
> >  
> > +	return 0;  
> 
> Also can be simply
> 
> 	return ret;
> 
> in both branches.

Yeah, I agonized over this since I did the change you describe in a
different patchset, and I was advised to do it the way as I did in
this patch so that it's more readable (i.e. we know which path is
the successful one). I'm really indifferent to either change and it
doesn't cut that many lines.

-- 
Kind regards

CJD