* [PATCH] iio: adc: max11410: fix read_poll_timeout() usage
@ 2023-03-07 9:53 Nuno Sá
2023-03-09 12:37 ` Tilki, Ibrahim
0 siblings, 1 reply; 3+ messages in thread
From: Nuno Sá @ 2023-03-07 9:53 UTC (permalink / raw)
To: linux-iio; +Cc: Jonathan Cameron, Ibrahim Tilki, Lars-Peter Clausen
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>
---
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)
--
2.39.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* RE: [PATCH] iio: adc: max11410: fix read_poll_timeout() usage
2023-03-07 9:53 [PATCH] iio: adc: max11410: fix read_poll_timeout() usage Nuno Sá
@ 2023-03-09 12:37 ` Tilki, Ibrahim
2023-03-12 15:26 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Tilki, Ibrahim @ 2023-03-09 12:37 UTC (permalink / raw)
To: Sa, Nuno, linux-iio@vger.kernel.org; +Cc: Jonathan Cameron, Lars-Peter Clausen
> 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>
> ---
> 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)
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] iio: adc: max11410: fix read_poll_timeout() usage
2023-03-09 12:37 ` Tilki, Ibrahim
@ 2023-03-12 15:26 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2023-03-12 15:26 UTC (permalink / raw)
To: Tilki, Ibrahim; +Cc: Sa, Nuno, linux-iio@vger.kernel.org, Lars-Peter Clausen
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)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-03-12 15:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-07 9:53 [PATCH] iio: adc: max11410: fix read_poll_timeout() usage Nuno Sá
2023-03-09 12:37 ` Tilki, Ibrahim
2023-03-12 15:26 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox