linux-kernel - Re: [PATCH] drivers: iio: proximity: This patch fix the following checkpatch warning.

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] [day] [month] [year] [list]

Message-ID: <20190424131655.000013bb@huawei.com>
Date:   Wed, 24 Apr 2019 13:16:55 +0100
From:   Jonathan Cameron <jonathan.cameron@...wei.com>
To:     Matt Ranostay <matt.ranostay@...sulko.com>
CC:     Jonathan Cameron <jic23@...nel.org>,
        Mohan Kumar <mohankumar718@...il.com>,
        Andreas Klinger <ak@...klinger.de>,
        "Hartmut Knaack" <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        "Peter Meerwald-Stadler" <pmeerw@...erw.net>,
        <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drivers: iio: proximity: This patch fix the following
 checkpatch warning.

On Wed, 24 Apr 2019 17:23:26 +0800
Matt Ranostay <matt.ranostay@...sulko.com> wrote:

> On Mon, Apr 22, 2019 at 6:48 PM Jonathan Cameron <jic23@...nel.org> wrote:
> >
> > On Thu, 18 Apr 2019 16:23:51 -0700
> > Matt Ranostay <matt.ranostay@...sulko.com> wrote:
> >  
> > > See comments inline
> > >
> > > On Wed, Apr 17, 2019 at 11:15 AM Mohan Kumar <mohankumar718@...il.com> wrote:  
> > > >
> > > > As per Documentation/timers/timers-howto.txt Msleep < 20ms can sleep for
> > > > up to 20ms. so use usleep_range.
> > > >
> > > > Signed-off-by: Mohan Kumar <mohankumar718@...il.com>
> > > > ---
> > > >  drivers/iio/proximity/mb1232.c | 2 +-
> > > >  drivers/iio/proximity/srf08.c  | 4 ++--
> > > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/proximity/mb1232.c b/drivers/iio/proximity/mb1232.c
> > > > index 166b3e6..74f7eae 100644
> > > > --- a/drivers/iio/proximity/mb1232.c
> > > > +++ b/drivers/iio/proximity/mb1232.c
> > > > @@ -81,7 +81,7 @@ static s16 mb1232_read_distance(struct mb1232_data *data)
> > > >                 }
> > > >         } else {
> > > >                 /* use simple sleep if announce irq is not connected */
> > > > -               msleep(15);
> > > > +               usleep_range(15000, 20000);  
> > >
> > > This is actually less than ideal.. because usleep_range uses the
> > > hrtimers which forces an interrupt.
> > >
> > > Unless you actually need to read between 15ms and 20ms it is best just
> > > to leave the msleep() as it is, and let it take a bit longer
> > > if required.  
> > Good response.  Perhaps Mohan could follow up with a patch to add
> > a comment to this effect to save anyone else trying to 'fix' this
> > issue in future?  
> 
> Change of the warning message in the checkpatch.pl script you mean?
No. That's basically telling people to 'think about it'.  I meant a comment
in the source for this driver to say 'We don't care here because...'.

That way anyone preparing a patch for this in future will see the comment
and not bother.

Jonathan

> 
> - Matt
> 
> >
> > Thanks,
> >
> > Jonathan  
> > >
> > > - Matt
> > >  
> > > >         }
> > > >
> > > >         ret = i2c_master_recv(client, (char *)&buf, sizeof(buf));
> > > > diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
> > > > index f2bf783..605a582 100644
> > > > --- a/drivers/iio/proximity/srf08.c
> > > > +++ b/drivers/iio/proximity/srf08.c
> > > > @@ -150,7 +150,7 @@ static int srf08_read_ranging(struct srf08_data *data)
> > > >          * polling for not more than 20 ms should be enough
> > > >          */
> > > >         waittime = 1 + data->range_mm / 172;
> > > > -       msleep(waittime);
> > > > +       usleep_range(waittime * 1000, (waittime * 1000) + 2000);
> > > >         for (i = 0; i < 4; i++) {
> > > >                 ret = i2c_smbus_read_byte_data(data->client,
> > > >                                                 SRF08_READ_SW_REVISION);
> > > > @@ -158,7 +158,7 @@ static int srf08_read_ranging(struct srf08_data *data)
> > > >                 /* check if a valid version number is read */
> > > >                 if (ret < 255 && ret > 0)
> > > >                         break;
> > > > -               msleep(5);
> > > > +               usleep_range(5000, 15000);
> > > >         }
> > > >
> > > >         if (ret >= 255 || ret <= 0) {
> > > > --
> > > > 2.7.4
> > > >  
> >