diff mbox series

[v5,3/5] iio:temperature:mlx90632: Convert polling while loop to do-while

Message ID 20200813075125.4949-4-cmo@melexis.com
State New
Headers show
Series iio: temperature: mlx90632: Add extended calibration calculations | expand

Commit Message

Crt Mori Aug. 13, 2020, 7:51 a.m. UTC
Reduce number of lines and improve readability to convert polling while
loops to do-while. The iopoll.h interface was not used, because we
require more than 20ms timeout, because time for sensor to perform a
measurement is around 10ms and it needs to perform measurements for each
channel (which currently is 3).

Signed-off-by: Crt Mori <cmo@melexis.com>
---
 drivers/iio/temperature/mlx90632.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Andy Shevchenko Aug. 13, 2020, 11:03 a.m. UTC | #1
On Thu, Aug 13, 2020 at 10:53 AM Crt Mori <cmo@melexis.com> wrote:
>
> Reduce number of lines and improve readability to convert polling while
> loops to do-while. The iopoll.h interface was not used, because we
> require more than 20ms timeout, because time for sensor to perform a
> measurement is around 10ms and it needs to perform measurements for each
> channel (which currently is 3).

I don't see how it prevents using iopoll.h. It uses usleep_range()
under the hood in the same way you did here, but open coded.

...

> -       while (tries-- > 0) {
> +       do {
>                 ret = regmap_read(data->regmap, MLX90632_REG_STATUS,
>                                   &reg_status);
>                 if (ret < 0)
>                         return ret;
> -               if (reg_status & MLX90632_STAT_DATA_RDY)
> -                       break;
>                 usleep_range(10000, 11000);
> -       }
> +       } while (!(reg_status & MLX90632_STAT_DATA_RDY) && tries--);
>
>         if (tries < 0) {
>                 dev_err(&data->client->dev, "data not ready");
Crt Mori Aug. 13, 2020, 11:13 a.m. UTC | #2
On Thu, 13 Aug 2020 at 13:03, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> On Thu, Aug 13, 2020 at 10:53 AM Crt Mori <cmo@melexis.com> wrote:
> >
> > Reduce number of lines and improve readability to convert polling while
> > loops to do-while. The iopoll.h interface was not used, because we
> > require more than 20ms timeout, because time for sensor to perform a
> > measurement is around 10ms and it needs to perform measurements for each
> > channel (which currently is 3).
>
> I don't see how it prevents using iopoll.h. It uses usleep_range()
> under the hood in the same way you did here, but open coded.
>

One loop is indeed 10ms and that is not the problem, the problem is
that timeout is at least 3 calls of this data ready (3 channels), so
that is at minimum 30ms of timeout, or it could even be 4 in worse
case scenario and that is outside of the range for usleep to measure.
So in case of the other loop, where we wait 200ms for channel refresh
it is also out of scope. Timeout should be in number of tries or in
msleep range if you ask me.

> ...
>
> > -       while (tries-- > 0) {
> > +       do {
> >                 ret = regmap_read(data->regmap, MLX90632_REG_STATUS,
> >                                   &reg_status);
> >                 if (ret < 0)
> >                         return ret;
> > -               if (reg_status & MLX90632_STAT_DATA_RDY)
> > -                       break;
> >                 usleep_range(10000, 11000);
> > -       }
> > +       } while (!(reg_status & MLX90632_STAT_DATA_RDY) && tries--);
> >
> >         if (tries < 0) {
> >                 dev_err(&data->client->dev, "data not ready");
>
> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko Aug. 13, 2020, 11:24 a.m. UTC | #3
On Thu, Aug 13, 2020 at 2:14 PM Crt Mori <cmo@melexis.com> wrote:
>
> On Thu, 13 Aug 2020 at 13:03, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >
> > On Thu, Aug 13, 2020 at 10:53 AM Crt Mori <cmo@melexis.com> wrote:
> > >
> > > Reduce number of lines and improve readability to convert polling while
> > > loops to do-while. The iopoll.h interface was not used, because we
> > > require more than 20ms timeout, because time for sensor to perform a
> > > measurement is around 10ms and it needs to perform measurements for each
> > > channel (which currently is 3).
> >
> > I don't see how it prevents using iopoll.h. It uses usleep_range()
> > under the hood in the same way you did here, but open coded.
> >
>
> One loop is indeed 10ms and that is not the problem, the problem is
> that timeout is at least 3 calls of this data ready (3 channels), so
> that is at minimum 30ms of timeout, or it could even be 4 in worse
> case scenario and that is outside of the range for usleep to measure.
> So in case of the other loop, where we wait 200ms for channel refresh
> it is also out of scope. Timeout should be in number of tries or in
> msleep range if you ask me.

I still didn't buy it. You have in both cases usleep_range(). Why in
your case it's okay and in regmap_read_poll_timeout() is not?

> > ...
> >
> > > -       while (tries-- > 0) {
> > > +       do {
> > >                 ret = regmap_read(data->regmap, MLX90632_REG_STATUS,
> > >                                   &reg_status);
> > >                 if (ret < 0)
> > >                         return ret;
> > > -               if (reg_status & MLX90632_STAT_DATA_RDY)
> > > -                       break;
> > >                 usleep_range(10000, 11000);
> > > -       }
> > > +       } while (!(reg_status & MLX90632_STAT_DATA_RDY) && tries--);
> > >
> > >         if (tries < 0) {
> > >                 dev_err(&data->client->dev, "data not ready");
Crt Mori Aug. 13, 2020, 1:03 p.m. UTC | #4
On Thu, 13 Aug 2020 at 13:24, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> On Thu, Aug 13, 2020 at 2:14 PM Crt Mori <cmo@melexis.com> wrote:
> >
> > On Thu, 13 Aug 2020 at 13:03, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Thu, Aug 13, 2020 at 10:53 AM Crt Mori <cmo@melexis.com> wrote:
> > > >
> > > > Reduce number of lines and improve readability to convert polling while
> > > > loops to do-while. The iopoll.h interface was not used, because we
> > > > require more than 20ms timeout, because time for sensor to perform a
> > > > measurement is around 10ms and it needs to perform measurements for each
> > > > channel (which currently is 3).
> > >
> > > I don't see how it prevents using iopoll.h. It uses usleep_range()
> > > under the hood in the same way you did here, but open coded.
> > >
> >
> > One loop is indeed 10ms and that is not the problem, the problem is
> > that timeout is at least 3 calls of this data ready (3 channels), so
> > that is at minimum 30ms of timeout, or it could even be 4 in worse
> > case scenario and that is outside of the range for usleep to measure.
> > So in case of the other loop, where we wait 200ms for channel refresh
> > it is also out of scope. Timeout should be in number of tries or in
> > msleep range if you ask me.
>
> I still didn't buy it. You have in both cases usleep_range(). Why in
> your case it's okay and in regmap_read_poll_timeout() is not?
>

I tried and it did not work, so then I read the manual. Looking into

* regmap_read_poll_timeout_atomic - Poll until a condition is met or a
timeout occurs
...
 * @delay_us: Time to udelay between reads in us (0 tight-loops).
 *            Should be less than ~10us since udelay is used
 *            (see Documentation/timers/timers-howto.rst).
 * @timeout_us: Timeout in us, 0 means never timeout


So I went to read Documentation/timers/timers-howto.rst

SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Use usleep_range

- Why not msleep for (1ms - 20ms)?
Explained originally here:
http://lkml.org/lkml/2007/8/3/250

msleep(1~20) may not do what the caller intends, and
will often sleep longer (~20 ms actual sleep for any
value given in the 1~20ms range). In many cases this
is not the desired behavior.

Since I am above the 20ms range, it is too much for usleep_range and
that proved to be a case as I got -ETIMEOUT and the desired channels
were not read.
> > > ...
> > >
> > > > -       while (tries-- > 0) {
> > > > +       do {
> > > >                 ret = regmap_read(data->regmap, MLX90632_REG_STATUS,
> > > >                                   &reg_status);
> > > >                 if (ret < 0)
> > > >                         return ret;
> > > > -               if (reg_status & MLX90632_STAT_DATA_RDY)
> > > > -                       break;
> > > >                 usleep_range(10000, 11000);
> > > > -       }
> > > > +       } while (!(reg_status & MLX90632_STAT_DATA_RDY) && tries--);
> > > >
> > > >         if (tries < 0) {
> > > >                 dev_err(&data->client->dev, "data not ready");
>
> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko Aug. 13, 2020, 7:40 p.m. UTC | #5
On Thu, Aug 13, 2020 at 4:04 PM Crt Mori <cmo@melexis.com> wrote:
> On Thu, 13 Aug 2020 at 13:24, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Thu, Aug 13, 2020 at 2:14 PM Crt Mori <cmo@melexis.com> wrote:
> > > On Thu, 13 Aug 2020 at 13:03, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > On Thu, Aug 13, 2020 at 10:53 AM Crt Mori <cmo@melexis.com> wrote:

...

> > > > I don't see how it prevents using iopoll.h. It uses usleep_range()
> > > > under the hood in the same way you did here, but open coded.
> > > >
> > >
> > > One loop is indeed 10ms and that is not the problem, the problem is
> > > that timeout is at least 3 calls of this data ready (3 channels), so
> > > that is at minimum 30ms of timeout, or it could even be 4 in worse
> > > case scenario and that is outside of the range for usleep to measure.
> > > So in case of the other loop, where we wait 200ms for channel refresh
> > > it is also out of scope. Timeout should be in number of tries or in
> > > msleep range if you ask me.
> >
> > I still didn't buy it. You have in both cases usleep_range(). Why in
> > your case it's okay and in regmap_read_poll_timeout() is not?
> >
>
> I tried and it did not work, so then I read the manual. Looking into
>
> * regmap_read_poll_timeout_atomic - Poll until a condition is met or a
> timeout occurs

Why _atomic?!

> ...
>  * @delay_us: Time to udelay between reads in us (0 tight-loops).
>  *            Should be less than ~10us since udelay is used
>  *            (see Documentation/timers/timers-howto.rst).
>  * @timeout_us: Timeout in us, 0 means never timeout
>
>
> So I went to read Documentation/timers/timers-howto.rst
>
> SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
> * Use usleep_range
>
> - Why not msleep for (1ms - 20ms)?
> Explained originally here:
> http://lkml.org/lkml/2007/8/3/250
>
> msleep(1~20) may not do what the caller intends, and
> will often sleep longer (~20 ms actual sleep for any
> value given in the 1~20ms range). In many cases this
> is not the desired behavior.
>
> Since I am above the 20ms range, it is too much for usleep_range and
> that proved to be a case as I got -ETIMEOUT and the desired channels
> were not read.
> > > > ...
> > > >
> > > > > -       while (tries-- > 0) {
> > > > > +       do {
> > > > >                 ret = regmap_read(data->regmap, MLX90632_REG_STATUS,
> > > > >                                   &reg_status);
> > > > >                 if (ret < 0)
> > > > >                         return ret;
> > > > > -               if (reg_status & MLX90632_STAT_DATA_RDY)
> > > > > -                       break;
> > > > >                 usleep_range(10000, 11000);

You use here usleep_range(). The same is used for
regmap_read_poll_timeout(). What's the difference?

Since it uses 1/4 of the range you probably need to update tries and
timeout_us to make it work.

> > > > > -       }
> > > > > +       } while (!(reg_status & MLX90632_STAT_DATA_RDY) && tries--);
> > > > >
> > > > >         if (tries < 0) {
> > > > >                 dev_err(&data->client->dev, "data not ready");
Crt Mori Aug. 14, 2020, 7:32 a.m. UTC | #6
On Thu, 13 Aug 2020 at 21:41, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> On Thu, Aug 13, 2020 at 4:04 PM Crt Mori <cmo@melexis.com> wrote:
> > On Thu, 13 Aug 2020 at 13:24, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > On Thu, Aug 13, 2020 at 2:14 PM Crt Mori <cmo@melexis.com> wrote:
> > > > On Thu, 13 Aug 2020 at 13:03, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > On Thu, Aug 13, 2020 at 10:53 AM Crt Mori <cmo@melexis.com> wrote:
>
> ...
>
> > > > > I don't see how it prevents using iopoll.h. It uses usleep_range()
> > > > > under the hood in the same way you did here, but open coded.
> > > > >
> > > >
> > > > One loop is indeed 10ms and that is not the problem, the problem is
> > > > that timeout is at least 3 calls of this data ready (3 channels), so
> > > > that is at minimum 30ms of timeout, or it could even be 4 in worse
> > > > case scenario and that is outside of the range for usleep to measure.
> > > > So in case of the other loop, where we wait 200ms for channel refresh
> > > > it is also out of scope. Timeout should be in number of tries or in
> > > > msleep range if you ask me.
> > >
> > > I still didn't buy it. You have in both cases usleep_range(). Why in
> > > your case it's okay and in regmap_read_poll_timeout() is not?
> > >
> >
> > I tried and it did not work, so then I read the manual. Looking into
> >
> > * regmap_read_poll_timeout_atomic - Poll until a condition is met or a
> > timeout occurs
>
> Why _atomic?!

I just pasted something, it is the same as for non _atomic
>
> > ...
> >  * @delay_us: Time to udelay between reads in us (0 tight-loops).
> >  *            Should be less than ~10us since udelay is used
> >  *            (see Documentation/timers/timers-howto.rst).
> >  * @timeout_us: Timeout in us, 0 means never timeout
> >
> >
> > So I went to read Documentation/timers/timers-howto.rst
> >
> > SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
> > * Use usleep_range
> >
> > - Why not msleep for (1ms - 20ms)?
> > Explained originally here:
> > http://lkml.org/lkml/2007/8/3/250
> >
> > msleep(1~20) may not do what the caller intends, and
> > will often sleep longer (~20 ms actual sleep for any
> > value given in the 1~20ms range). In many cases this
> > is not the desired behavior.
> >
> > Since I am above the 20ms range, it is too much for usleep_range and
> > that proved to be a case as I got -ETIMEOUT and the desired channels
> > were not read.
> > > > > ...
> > > > >
> > > > > > -       while (tries-- > 0) {
> > > > > > +       do {
> > > > > >                 ret = regmap_read(data->regmap, MLX90632_REG_STATUS,
> > > > > >                                   &reg_status);
> > > > > >                 if (ret < 0)
> > > > > >                         return ret;
> > > > > > -               if (reg_status & MLX90632_STAT_DATA_RDY)
> > > > > > -                       break;
> > > > > >                 usleep_range(10000, 11000);
>
> You use here usleep_range(). The same is used for
> regmap_read_poll_timeout(). What's the difference?
>
> Since it uses 1/4 of the range you probably need to update tries and
> timeout_us to make it work.
>

Timeout_us here needs to be in one case 100 * 10ms (maybe not
realistic as we could live with number of around 40 * 10ms), but this
is a lot more than proposed range of usleep which Is up to 20ms. Even
in best case this timeout should be 40 ms to give enough time to
measure 2 channels for sure. So with the current timeout_us
requirement we are outside of the range of the udelay timer and that
is why I would need a macro with number of tries, not with the timeout
value (or timeout value of ms).


> > > > > > -       }
> > > > > > +       } while (!(reg_status & MLX90632_STAT_DATA_RDY) && tries--);
> > > > > >
> > > > > >         if (tries < 0) {
> > > > > >                 dev_err(&data->client->dev, "data not ready");
>
> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko Aug. 14, 2020, 9:31 a.m. UTC | #7
On Fri, Aug 14, 2020 at 10:33 AM Crt Mori <cmo@melexis.com> wrote:
> On Thu, 13 Aug 2020 at 21:41, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Thu, Aug 13, 2020 at 4:04 PM Crt Mori <cmo@melexis.com> wrote:
> > > On Thu, 13 Aug 2020 at 13:24, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > On Thu, Aug 13, 2020 at 2:14 PM Crt Mori <cmo@melexis.com> wrote:
> > > > > On Thu, 13 Aug 2020 at 13:03, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > On Thu, Aug 13, 2020 at 10:53 AM Crt Mori <cmo@melexis.com> wrote:
> >
> > ...
> >
> > > > > > I don't see how it prevents using iopoll.h. It uses usleep_range()
> > > > > > under the hood in the same way you did here, but open coded.
> > > > > >
> > > > >
> > > > > One loop is indeed 10ms and that is not the problem, the problem is
> > > > > that timeout is at least 3 calls of this data ready (3 channels), so
> > > > > that is at minimum 30ms of timeout, or it could even be 4 in worse
> > > > > case scenario and that is outside of the range for usleep to measure.
> > > > > So in case of the other loop, where we wait 200ms for channel refresh
> > > > > it is also out of scope. Timeout should be in number of tries or in
> > > > > msleep range if you ask me.
> > > >
> > > > I still didn't buy it. You have in both cases usleep_range(). Why in
> > > > your case it's okay and in regmap_read_poll_timeout() is not?
> > > >
> > >
> > > I tried and it did not work, so then I read the manual. Looking into
> > >
> > > * regmap_read_poll_timeout_atomic - Poll until a condition is met or a
> > > timeout occurs
> >
> > Why _atomic?!
>
> I just pasted something, it is the same as for non _atomic

OK.

...

> > >  * @delay_us: Time to udelay between reads in us (0 tight-loops).
> > >  *            Should be less than ~10us since udelay is used
> > >  *            (see Documentation/timers/timers-howto.rst).
> > >  * @timeout_us: Timeout in us, 0 means never timeout

...

> > > > > > >                 usleep_range(10000, 11000);
> >
> > You use here usleep_range(). The same is used for
> > regmap_read_poll_timeout(). What's the difference?
> >
> > Since it uses 1/4 of the range you probably need to update tries and
> > timeout_us to make it work.
> >
>
> Timeout_us here needs to be in one case 100 * 10ms (maybe not
> realistic as we could live with number of around 40 * 10ms), but this
> is a lot more than proposed range of usleep which Is up to 20ms. Even
> in best case this timeout should be 40 ms to give enough time to
> measure 2 channels for sure. So with the current timeout_us
> requirement we are outside of the range of the udelay timer and that
> is why I would need a macro with number of tries, not with the timeout
> value (or timeout value of ms).

I do not understand. The regmap_read_poll_timeout() is a macro which
unrolls in the very similar loop you have now in the code.
What prevents it from using it?

I think there is a big misunderstanding about the parameters of that macro.
delay_us (must be small enough), timeout_us can be any long.
Crt Mori Aug. 14, 2020, 9:42 a.m. UTC | #8
On Fri, 14 Aug 2020 at 11:32, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> On Fri, Aug 14, 2020 at 10:33 AM Crt Mori <cmo@melexis.com> wrote:
> > On Thu, 13 Aug 2020 at 21:41, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > On Thu, Aug 13, 2020 at 4:04 PM Crt Mori <cmo@melexis.com> wrote:
> > > > On Thu, 13 Aug 2020 at 13:24, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > On Thu, Aug 13, 2020 at 2:14 PM Crt Mori <cmo@melexis.com> wrote:
> > > > > > On Thu, 13 Aug 2020 at 13:03, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > On Thu, Aug 13, 2020 at 10:53 AM Crt Mori <cmo@melexis.com> wrote:
> > >
> > > ...
> > >
> > > > > > > I don't see how it prevents using iopoll.h. It uses usleep_range()
> > > > > > > under the hood in the same way you did here, but open coded.
> > > > > > >
> > > > > >
> > > > > > One loop is indeed 10ms and that is not the problem, the problem is
> > > > > > that timeout is at least 3 calls of this data ready (3 channels), so
> > > > > > that is at minimum 30ms of timeout, or it could even be 4 in worse
> > > > > > case scenario and that is outside of the range for usleep to measure.
> > > > > > So in case of the other loop, where we wait 200ms for channel refresh
> > > > > > it is also out of scope. Timeout should be in number of tries or in
> > > > > > msleep range if you ask me.
> > > > >
> > > > > I still didn't buy it. You have in both cases usleep_range(). Why in
> > > > > your case it's okay and in regmap_read_poll_timeout() is not?
> > > > >
> > > >
> > > > I tried and it did not work, so then I read the manual. Looking into
> > > >
> > > > * regmap_read_poll_timeout_atomic - Poll until a condition is met or a
> > > > timeout occurs
> > >
> > > Why _atomic?!
> >
> > I just pasted something, it is the same as for non _atomic
>
> OK.
>
> ...
>
> > > >  * @delay_us: Time to udelay between reads in us (0 tight-loops).
> > > >  *            Should be less than ~10us since udelay is used
> > > >  *            (see Documentation/timers/timers-howto.rst).
> > > >  * @timeout_us: Timeout in us, 0 means never timeout
>
> ...
>
> > > > > > > >                 usleep_range(10000, 11000);
> > >
> > > You use here usleep_range(). The same is used for
> > > regmap_read_poll_timeout(). What's the difference?
> > >
> > > Since it uses 1/4 of the range you probably need to update tries and
> > > timeout_us to make it work.
> > >
> >
> > Timeout_us here needs to be in one case 100 * 10ms (maybe not
> > realistic as we could live with number of around 40 * 10ms), but this
> > is a lot more than proposed range of usleep which Is up to 20ms. Even
> > in best case this timeout should be 40 ms to give enough time to
> > measure 2 channels for sure. So with the current timeout_us
> > requirement we are outside of the range of the udelay timer and that
> > is why I would need a macro with number of tries, not with the timeout
> > value (or timeout value of ms).
>
> I do not understand. The regmap_read_poll_timeout() is a macro which
> unrolls in the very similar loop you have now in the code.
> What prevents it from using it?
>
> I think there is a big misunderstanding about the parameters of that macro.
> delay_us (must be small enough), timeout_us can be any long.
>
I tested on Beaglebone with the 100 * 10000 as timeout_us and I always
got the -ETIMEDOUT error. I also tested in the other case where
delay_us is 250000 and then timeout_us would be 4*250000 and I have
also received -ETIMEDOUT as a response.

I can prepare a patch with the iopoll.h API and maybe you will spot
the mistake, as after rechecking timeout_us is indeed 64bit and is
only used in the time comparison operations and not with timers.

> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko Aug. 14, 2020, 12:11 p.m. UTC | #9
On Fri, Aug 14, 2020 at 12:42 PM Crt Mori <cmo@melexis.com> wrote:
> On Fri, 14 Aug 2020 at 11:32, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Fri, Aug 14, 2020 at 10:33 AM Crt Mori <cmo@melexis.com> wrote:
> > > On Thu, 13 Aug 2020 at 21:41, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > On Thu, Aug 13, 2020 at 4:04 PM Crt Mori <cmo@melexis.com> wrote:
> > > > > On Thu, 13 Aug 2020 at 13:24, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

...

> > > > >                 usleep_range(10000, 11000);

Above for 4 attempts is up to 44000us.

With iopoll.h case we may set
 delay_us = 18000 (the result range will be 4500, 18000)
 timeout_us = 72000 (this will give up to 16 attempts to read)

> I tested on Beaglebone with the 100 * 10000 as timeout_us and I always
> got the -ETIMEDOUT error. I also tested in the other case where
> delay_us is 250000 and then timeout_us would be 4*250000 and I have
> also received -ETIMEDOUT as a response.

I don't see how delay_us should be bigger than 20ms (20000).

> I can prepare a patch with the iopoll.h API and maybe you will spot
> the mistake, as after rechecking timeout_us is indeed 64bit and is
> only used in the time comparison operations and not with timers.

Perhaps above will clarify.
diff mbox series

Patch

diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c
index ce75f5a3486b..d87c792b7e72 100644
--- a/drivers/iio/temperature/mlx90632.c
+++ b/drivers/iio/temperature/mlx90632.c
@@ -188,15 +188,13 @@  static int mlx90632_perform_measurement(struct mlx90632_data *data)
 	if (ret < 0)
 		return ret;
 
-	while (tries-- > 0) {
+	do {
 		ret = regmap_read(data->regmap, MLX90632_REG_STATUS,
 				  &reg_status);
 		if (ret < 0)
 			return ret;
-		if (reg_status & MLX90632_STAT_DATA_RDY)
-			break;
 		usleep_range(10000, 11000);
-	}
+	} while (!(reg_status & MLX90632_STAT_DATA_RDY) && tries--);
 
 	if (tries < 0) {
 		dev_err(&data->client->dev, "data not ready");