* [PATCH] drivers: iio: proximity: This patch fix the following checkpatch warning.
@ 2019-04-17 18:15 Mohan Kumar
2019-04-18 23:23 ` Matt Ranostay
0 siblings, 1 reply; 5+ messages in thread
From: Mohan Kumar @ 2019-04-17 18:15 UTC (permalink / raw)
To: ak; +Cc: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel
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@gmail.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);
}
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: iio: proximity: This patch fix the following checkpatch warning.
2019-04-17 18:15 [PATCH] drivers: iio: proximity: This patch fix the following checkpatch warning Mohan Kumar
@ 2019-04-18 23:23 ` Matt Ranostay
2019-04-22 10:48 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Matt Ranostay @ 2019-04-18 23:23 UTC (permalink / raw)
To: Mohan Kumar
Cc: Andreas Klinger, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
linux-kernel
See comments inline
On Wed, Apr 17, 2019 at 11:15 AM Mohan Kumar <mohankumar718@gmail.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@gmail.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.
- 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: iio: proximity: This patch fix the following checkpatch warning.
2019-04-18 23:23 ` Matt Ranostay
@ 2019-04-22 10:48 ` Jonathan Cameron
2019-04-24 9:23 ` Matt Ranostay
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2019-04-22 10:48 UTC (permalink / raw)
To: Matt Ranostay
Cc: Mohan Kumar, Andreas Klinger, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio, linux-kernel
On Thu, 18 Apr 2019 16:23:51 -0700
Matt Ranostay <matt.ranostay@konsulko.com> wrote:
> See comments inline
>
> On Wed, Apr 17, 2019 at 11:15 AM Mohan Kumar <mohankumar718@gmail.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@gmail.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?
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
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: iio: proximity: This patch fix the following checkpatch warning.
2019-04-22 10:48 ` Jonathan Cameron
@ 2019-04-24 9:23 ` Matt Ranostay
2019-04-24 12:16 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Matt Ranostay @ 2019-04-24 9:23 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Mohan Kumar, Andreas Klinger, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio, linux-kernel
On Mon, Apr 22, 2019 at 6:48 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 18 Apr 2019 16:23:51 -0700
> Matt Ranostay <matt.ranostay@konsulko.com> wrote:
>
> > See comments inline
> >
> > On Wed, Apr 17, 2019 at 11:15 AM Mohan Kumar <mohankumar718@gmail.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@gmail.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?
- 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
> > >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: iio: proximity: This patch fix the following checkpatch warning.
2019-04-24 9:23 ` Matt Ranostay
@ 2019-04-24 12:16 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2019-04-24 12:16 UTC (permalink / raw)
To: Matt Ranostay
Cc: Jonathan Cameron, Mohan Kumar, Andreas Klinger, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
linux-kernel
On Wed, 24 Apr 2019 17:23:26 +0800
Matt Ranostay <matt.ranostay@konsulko.com> wrote:
> On Mon, Apr 22, 2019 at 6:48 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Thu, 18 Apr 2019 16:23:51 -0700
> > Matt Ranostay <matt.ranostay@konsulko.com> wrote:
> >
> > > See comments inline
> > >
> > > On Wed, Apr 17, 2019 at 11:15 AM Mohan Kumar <mohankumar718@gmail.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@gmail.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
> > > >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-24 12:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-17 18:15 [PATCH] drivers: iio: proximity: This patch fix the following checkpatch warning Mohan Kumar
2019-04-18 23:23 ` Matt Ranostay
2019-04-22 10:48 ` Jonathan Cameron
2019-04-24 9:23 ` Matt Ranostay
2019-04-24 12:16 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).