Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755843AbcK2GuP (ORCPT ); Tue, 29 Nov 2016 01:50:15 -0500 Received: from jablonecka.jablonka.cz ([91.219.244.36]:48432 "EHLO jablonecka.jablonka.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753566AbcK2GuF (ORCPT ); Tue, 29 Nov 2016 01:50:05 -0500 Date: Tue, 29 Nov 2016 07:49:56 +0100 From: Vojtech Pavlik To: Aniroop Mathur Cc: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, s.samuel@samsung.com, r.mahale@samsung.com, aniroop.mathur@gmail.com Subject: Re: [PATCH] Input: joystick: sidewinder - change msleep to usleep_range for small msecs Message-ID: <20161129064956.GB10549@suse.com> References: <1480361902-4671-1-git-send-email-a.mathur@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1480361902-4671-1-git-send-email-a.mathur@samsung.com> X-Bounce-Cookie: It's a lemon tree, dear Watson! User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5560 Lines: 124 On Tue, Nov 29, 2016 at 01:08:22AM +0530, Aniroop Mathur wrote: > 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) > This is not the desired behaviour for many cases like device resume time, > device suspend time, device enable time, connection time, probe time, > loops, retry logic, etc > msleep is built on jiffies / legacy timers which are not precise whereas > usleep_range is build on top of hrtimers so the wakeups are precise. > Thus, change msleep to usleep_range for precise wakeups. > > For example: > On a machine with tick rate / HZ as 100, msleep(6) will make the process to > sleep for a minimum period of 10 ms whereas usleep_range(6000, 6100) will make > sure that the process does not sleep for more than 6100 us or 6.1ms This patch seems clearly unnecessary. SW_TIMEOUT is 6ms or longer for MS Sidewinder devices. Vojtech > Signed-off-by: Aniroop Mathur > --- > drivers/input/joystick/sidewinder.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/input/joystick/sidewinder.c b/drivers/input/joystick/sidewinder.c > index 4a95b22..e5a1292 100644 > --- a/drivers/input/joystick/sidewinder.c > +++ b/drivers/input/joystick/sidewinder.c > @@ -50,7 +50,7 @@ MODULE_LICENSE("GPL"); > > #define SW_START 600 /* The time we wait for the first bit [600 us] */ > #define SW_STROBE 60 /* Max time per bit [60 us] */ > -#define SW_TIMEOUT 6 /* Wait for everything to settle [6 ms] */ > +#define SW_TIMEOUT 6000 /* Wait for everything to settle [6000 us] */ > #define SW_KICK 45 /* Wait after A0 fall till kick [45 us] */ > #define SW_END 8 /* Number of bits before end of packet to kick */ > #define SW_FAIL 16 /* Number of packet read errors to fail and reinitialize */ > @@ -139,7 +139,7 @@ static int sw_read_packet(struct gameport *gameport, unsigned char *buf, int len > unsigned char pending, u, v; > > i = -id; /* Don't care about data, only want ID */ > - timeout = id ? gameport_time(gameport, SW_TIMEOUT * 1000) : 0; /* Set up global timeout for ID packet */ > + timeout = id ? gameport_time(gameport, SW_TIMEOUT) : 0; /* Set up global timeout for ID packet */ > kick = id ? gameport_time(gameport, SW_KICK) : 0; /* Set up kick timeout for ID packet */ > start = gameport_time(gameport, SW_START); > strobe = gameport_time(gameport, SW_STROBE); > @@ -248,7 +248,7 @@ static void sw_init_digital(struct gameport *gameport) > i = 0; > do { > gameport_trigger(gameport); /* Trigger */ > - t = gameport_time(gameport, SW_TIMEOUT * 1000); > + t = gameport_time(gameport, SW_TIMEOUT); > while ((gameport_read(gameport) & 1) && t) t--; /* Wait for axis to fall back to 0 */ > udelay(seq[i]); /* Delay magic time */ > } while (seq[++i]); > @@ -483,13 +483,13 @@ static int sw_read(struct sw *sw) > " - reinitializing joystick.\n", sw->gameport->phys); > > if (!i && sw->type == SW_ID_3DP) { /* 3D Pro can be in analog mode */ > - mdelay(3 * SW_TIMEOUT); > + mdelay(3 * (SW_TIMEOUT / 1000)); > sw_init_digital(sw->gameport); > } > > - mdelay(SW_TIMEOUT); > + mdelay(SW_TIMEOUT / 1000); > i = sw_read_packet(sw->gameport, buf, SW_LENGTH, 0); /* Read normal data packet */ > - mdelay(SW_TIMEOUT); > + mdelay(SW_TIMEOUT / 1000); > sw_read_packet(sw->gameport, buf, SW_LENGTH, i); /* Read ID packet, this initializes the stick */ > > sw->fail = SW_FAIL; > @@ -616,14 +616,14 @@ static int sw_connect(struct gameport *gameport, struct gameport_driver *drv) > gameport->phys, gameport->io, gameport->speed); > > i = sw_read_packet(gameport, buf, SW_LENGTH, 0); /* Read normal packet */ > - msleep(SW_TIMEOUT); > + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100); > dbg("Init 1: Mode %d. Length %d.", m , i); > > if (!i) { /* No data. 3d Pro analog mode? */ > sw_init_digital(gameport); /* Switch to digital */ > - msleep(SW_TIMEOUT); > + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100); > i = sw_read_packet(gameport, buf, SW_LENGTH, 0); /* Retry reading packet */ > - msleep(SW_TIMEOUT); > + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100); > dbg("Init 1b: Length %d.", i); > if (!i) { /* No data -> FAIL */ > err = -ENODEV; > @@ -636,7 +636,7 @@ static int sw_connect(struct gameport *gameport, struct gameport_driver *drv) > dbg("Init 2: Mode %d. ID Length %d.", m, j); > > if (j <= 0) { /* Read ID failed. Happens in 1-bit mode on PP */ > - msleep(SW_TIMEOUT); > + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100); > i = sw_read_packet(gameport, buf, SW_LENGTH, 0); /* Retry reading packet */ > m |= sw_guess_mode(buf, i); > dbg("Init 2b: Mode %d. Length %d.", m, i); > @@ -644,7 +644,7 @@ static int sw_connect(struct gameport *gameport, struct gameport_driver *drv) > err = -ENODEV; > goto fail2; > } > - msleep(SW_TIMEOUT); > + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100); > j = sw_read_packet(gameport, idbuf, SW_LENGTH, i); /* Retry reading ID */ > dbg("Init 2c: ID Length %d.", j); > } > @@ -655,7 +655,7 @@ static int sw_connect(struct gameport *gameport, struct gameport_driver *drv) > > do { > k--; > - msleep(SW_TIMEOUT); > + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100); > i = sw_read_packet(gameport, buf, SW_LENGTH, 0); /* Read data packet */ > dbg("Init 3: Mode %d. Length %d. Last %d. Tries %d.", m, i, l, k); > > -- > 2.6.2 > -- Vojtech Pavlik