Re: [PATCH] iio: adc: max11410: fix read_poll_timeout() usage

From: Jonathan Cameron <jic23@kernel.org>
To: "Tilki, Ibrahim" <Ibrahim.Tilki@analog.com>
Cc: "Sa, Nuno" <Nuno.Sa@analog.com>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>
Subject: Re: [PATCH] iio: adc: max11410: fix read_poll_timeout() usage
Date: Sun, 12 Mar 2023 15:26:36 +0000	[thread overview]
Message-ID: <20230312152636.2baee309@jic23-huawei> (raw)
In-Reply-To: <CY4PR03MB24888E9F4583FA53C7D8076596B59@CY4PR03MB2488.namprd03.prod.outlook.com>

On Thu, 9 Mar 2023 12:37:54 +0000
"Tilki, Ibrahim" <Ibrahim.Tilki@analog.com> wrote:

> > Even though we are passing 'ret' as stop condition for
> > read_poll_timeout(), that return code is still being ignored. The reason
> > is that the poll will stop if the passed condition is true which will
> > happen if the passed op() returns error. However, read_poll_timeout()
> > returns 0 if the *complete* condition evaluates to true. Therefore, the
> > error code returned by op() will be ignored.
> > 
> > To fix this we need to check for both error codes:
> >  * The one returned by read_poll_timeout() which is either 0 or
> > ETIMEDOUT.
> >  * The one returned by the passed op().
> > 
> > Fixes: a44ef7c46097 ("iio: adc: add max11410 adc driver")
> > Signed-off-by: Nuno SÃ¡ <nuno.sa@analog.com>  
> 
> Acked-by: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
> 
Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> 
> > ---
> >  drivers/iio/adc/max11410.c | 22 +++++++++++++++-------
> >  1 file changed, 15 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/max11410.c b/drivers/iio/adc/max11410.c
> > index 237b2ce3f264..6af829349b4e 100644
> > --- a/drivers/iio/adc/max11410.c
> > +++ b/drivers/iio/adc/max11410.c
> > @@ -414,13 +414,17 @@ static int max11410_sample(struct max11410_state *st, int *sample_raw,
> >  		if (!ret)
> >  			return -ETIMEDOUT;
> >  	} else {
> > +		int ret2;
> > +
> >  		/* Wait for status register Conversion Ready flag */
> > -		ret = read_poll_timeout(max11410_read_reg, ret,
> > -					ret || (val & MAX11410_STATUS_CONV_READY_BIT),
> > +		ret = read_poll_timeout(max11410_read_reg, ret2,
> > +					ret2 || (val & MAX11410_STATUS_CONV_READY_BIT),
> >  					5000, MAX11410_CONVERSION_TIMEOUT_MS * 1000,
> >  					true, st, MAX11410_REG_STATUS, &val);
> >  		if (ret)
> >  			return ret;
> > +		if (ret2)
> > +			return ret2;
> >  	}
> >  
> >  	/* Read ADC Data */
> > @@ -851,17 +855,21 @@ static int max11410_init_vref(struct device *dev,
> >  
> >  static int max11410_calibrate(struct max11410_state *st, u32 cal_type)
> >  {
> > -	int ret, val;
> > +	int ret, ret2, val;
> >  
> >  	ret = max11410_write_reg(st, MAX11410_REG_CAL_START, cal_type);
> >  	if (ret)
> >  		return ret;
> >  
> >  	/* Wait for status register Calibration Ready flag */
> > -	return read_poll_timeout(max11410_read_reg, ret,
> > -				 ret || (val & MAX11410_STATUS_CAL_READY_BIT),
> > -				 50000, MAX11410_CALIB_TIMEOUT_MS * 1000, true,
> > -				 st, MAX11410_REG_STATUS, &val);
> > +	ret = read_poll_timeout(max11410_read_reg, ret2,
> > +				ret2 || (val & MAX11410_STATUS_CAL_READY_BIT),
> > +				50000, MAX11410_CALIB_TIMEOUT_MS * 1000, true,
> > +				st, MAX11410_REG_STATUS, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return ret2;
> >  }
> >  
> >  static int max11410_self_calibrate(struct max11410_state *st)