[bug report] iio: adc: max1027: Use the EOC IRQ when populated for single reads

[bug report] iio: adc: max1027: Use the EOC IRQ when populated for single reads

[Dan Carpenter]

Hello Miquel Raynal,

The patch 54f14be01e17: "iio: adc: max1027: Use the EOC IRQ when
populated for single reads" from Sep 21, 2021, leads to the following
Smatch static checker warning:

	drivers/iio/adc/max1027.c:289 max1027_wait_eoc()
	info: return a literal instead of 'ret'

drivers/iio/adc/max1027.c
    278 static int max1027_wait_eoc(struct iio_dev *indio_dev)
    279 {
    280         struct max1027_state *st = iio_priv(indio_dev);
    281         unsigned int conversion_time = MAX1027_CONVERSION_UDELAY;
    282         int ret;
    283 
    284         if (st->spi->irq) {
    285                 ret = wait_for_completion_timeout(&st->complete,
    286                                                   msecs_to_jiffies(1000));
    287                 reinit_completion(&st->complete);
    288                 if (!ret)
--> 289                         return ret;

wait_for_completion_timeout() returns zero if it times out.  Should this
be return an error code?  Otherwise we could just delete the "ret"
variable and return 0 below.

    290         } else {
    291                 if (indio_dev->active_scan_mask)
    292                         conversion_time *= hweight32(*indio_dev->active_scan_mask);
    293 
    294                 usleep_range(conversion_time, conversion_time * 2);
    295         }
    296 
    297         return 0;
    298 }

regards,
dan carpenter

Re: [bug report] iio: adc: max1027: Use the EOC IRQ when populated for single reads

[Miquel Raynal]

Hi Jonathan,

dan.carpenter@oracle.com wrote on Mon, 4 Oct 2021 13:17:01 +0300:

> Hello Miquel Raynal,
> 
> The patch 54f14be01e17: "iio: adc: max1027: Use the EOC IRQ when
> populated for single reads" from Sep 21, 2021, leads to the following
> Smatch static checker warning:
> 
> 	drivers/iio/adc/max1027.c:289 max1027_wait_eoc()
> 	info: return a literal instead of 'ret'
> 
> drivers/iio/adc/max1027.c
>     278 static int max1027_wait_eoc(struct iio_dev *indio_dev)
>     279 {
>     280         struct max1027_state *st = iio_priv(indio_dev);
>     281         unsigned int conversion_time = MAX1027_CONVERSION_UDELAY;
>     282         int ret;
>     283 
>     284         if (st->spi->irq) {
>     285                 ret = wait_for_completion_timeout(&st->complete,
>     286                                                   msecs_to_jiffies(1000));
>     287                 reinit_completion(&st->complete);
>     288                 if (!ret)
> --> 289                         return ret;  
> 
> wait_for_completion_timeout() returns zero if it times out.  Should this
> be return an error code?  Otherwise we could just delete the "ret"
> variable and return 0 below.

I received this bug report, indeed this should use

			if (!ret)
				return -ETIMEDOUT;

Can you fix it directly? Do you want a fixup! patch to apply and
squash? Or a perhaps a regular patch?

> 
>     290         } else {
>     291                 if (indio_dev->active_scan_mask)
>     292                         conversion_time *= hweight32(*indio_dev->active_scan_mask);
>     293 
>     294                 usleep_range(conversion_time, conversion_time * 2);
>     295         }
>     296 
>     297         return 0;
>     298 }
> 
> regards,
> dan carpenter


Thanks,
Miquèl