2019-04-17 18:17:13

by Mohan Kumar

Subject: [PATCH] drivers: iio: proximity: This patch fix the following checkpatch warning.

As per Documentation/timers/timers-howto.txt Msleep < 20ms can sleep for
up to 20ms. so use usleep_range.

Signed-off-by: Mohan Kumar <[email protected]>
---
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);
}

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

2019-04-18 23:25:36

by Matt Ranostay

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

See comments inline

On Wed, Apr 17, 2019 at 11:15 AM Mohan Kumar <[email protected]> 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 <[email protected]>
> ---
> 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.

- 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
>

2019-04-22 10:49:32

by Jonathan Cameron

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

On Thu, 18 Apr 2019 16:23:51 -0700
Matt Ranostay <[email protected]> wrote:

> See comments inline
>
> On Wed, Apr 17, 2019 at 11:15 AM Mohan Kumar <[email protected]> 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 <[email protected]>
> > ---
> > 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?

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
> >

2019-04-24 09:26:30

by Matt Ranostay

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

On Mon, Apr 22, 2019 at 6:48 PM Jonathan Cameron <[email protected]> wrote:
>
> On Thu, 18 Apr 2019 16:23:51 -0700
> Matt Ranostay <[email protected]> wrote:
>
> > See comments inline
> >
> > On Wed, Apr 17, 2019 at 11:15 AM Mohan Kumar <[email protected]> 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 <[email protected]>
> > > ---
> > > 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?

- 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
> > >
>

2019-04-24 13:49:52

by Jonathan Cameron

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

On Wed, 24 Apr 2019 17:23:26 +0800
Matt Ranostay <[email protected]> wrote:

> On Mon, Apr 22, 2019 at 6:48 PM Jonathan Cameron <[email protected]> wrote:
> >
> > On Thu, 18 Apr 2019 16:23:51 -0700
> > Matt Ranostay <[email protected]> wrote:
> >
> > > See comments inline
> > >
> > > On Wed, Apr 17, 2019 at 11:15 AM Mohan Kumar <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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
> > > >
> >