linux-kernel - Re: [PATCH v5 3/5] iio:temperature:mlx90632: Convert polling while loop to do-while

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Aug 2020 13:13:53 +0200
From:   Crt Mori <cmo@...exis.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        linux-iio <linux-iio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 3/5] iio:temperature:mlx90632: Convert polling while
 loop to do-while

On Thu, 13 Aug 2020 at 13:03, Andy Shevchenko <andy.shevchenko@...il.com> wrote:
>
> On Thu, Aug 13, 2020 at 10:53 AM Crt Mori <cmo@...exis.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