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

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Wed, 15 Jul 2026 10:09:58 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Tue, 14 Jul 2026 20:04:10 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > > -	if (ret < 0) {
> > > +	if (ret < 0)    
> > 
> > At some point consider dropping ' < 0' pieces for the i2c_smbus_write_*() cases.
> > Read cases are different, of course.


Excellent point Andy, and this case is a bit special..

> >   
> > >  		dev_err(dev, "failed to write register %02x\n",
> > >  			OPT3001_CONFIGURATION);
> > > +
> > > +	return ret;  
  
If ret could be positive then this would be letting an unexpected
positive return value escape the function.  So I've tweaked the two
cases of this pattern to if (ret) to avoid that implication.

Applied with 
diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
index 12cfb0e58870..a948353014ac 100644
--- a/drivers/iio/light/opt3001.c
+++ b/drivers/iio/light/opt3001.c
@@ -337,7 +337,7 @@ static int opt3001_start_conversion(struct opt3001 *opt)
        opt3001_set_mode(opt, &reg, OPT3001_CONFIGURATION_M_SINGLE);
 
        ret = i2c_smbus_write_word_swapped(client, OPT3001_CONFIGURATION, reg);
-       if (ret < 0)
+       if (ret)
                dev_err(dev, "failed to write register %02x\n",
                        OPT3001_CONFIGURATION);
 
@@ -392,7 +392,7 @@ static int opt3001_get_processed_irq(struct opt3001 *opt)
         */
        value = (opt->low_thresh_exp << 12) | opt->low_thresh_mantissa;
        ret = i2c_smbus_write_word_swapped(client, OPT3001_LOW_LIMIT, value);
-       if (ret < 0)
+       if (ret)
                dev_err(dev, "failed to write register %02x\n",
                        OPT3001_LOW_LIMIT);
 
> >   
> 
> Even after an 8-part patch set, this driver still needs cleaning up!
> 
:)  You'd be amazed how many patches it takes to clean up some drivers
that are in 'reasonable' condition!

Jonathan