All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Mc Guire <hofrat@osadl.org>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Henrik Rydberg <rydberg@bitmath.org>,
	Dudley Du <dudl@cypress.com>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nicholas Mc Guire <hofrat@osadl.org>
Subject: [PATCH RFC V2] Input: cyapa: use time based retry loop
Date: Sun, 15 Jan 2017 12:12:38 +0100	[thread overview]
Message-ID: <1484478758-13133-1-git-send-email-hofrat@osadl.org> (raw)

Using counter based retry loops for peripherals results in the delay
being significantly overrun during high-load situations where delay
functions tend to be vary imprecise and overrun there timeouts. So
condition the termination on the actual condition of 2s for the 
re-calibration to have been successful.

As this is a very long delay there is no advantage in using 
high-resolution timers thus switching this to msleep().

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

V2: The maximum recalibration timeout should have been 2 * HZ as
    Dmitry Torokhov <dmitry.torokhov@gmail.com> noted - V2 sets
    the timeout boundary to 2s now.

Problem detected with coccinelle script (usleep_range for long delay)

The basic model for the fix up here is to use

  timeout = jiffies + (retry_time)
  do {
      check()
      mleep()
  } while (time_is_after_jiffies(timeout));

rather than using a counter and have very long retry loops under load
conditions. In the specific case it is sleep()/check() though to wait
on the previous write to complete. The initial intent was to move the
usleep_range() to msleep() as the delays are very long - if this
refactoring of the timeout loop is desirable or not is not clear (both
time and count based models seem to be common).

Patch was compile tested with: x86_64_defconfig + CONFIG_MOUSE_CYAPA=m

Patch is against 4.10-rc3 (localversion-next is next-20170113)

 drivers/input/mouse/cyapa_gen3.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/input/mouse/cyapa_gen3.c b/drivers/input/mouse/cyapa_gen3.c
index f9600753..0c544de 100644
--- a/drivers/input/mouse/cyapa_gen3.c
+++ b/drivers/input/mouse/cyapa_gen3.c
@@ -789,7 +789,7 @@ static ssize_t cyapa_gen3_do_calibrate(struct device *dev,
 				     const char *buf, size_t count)
 {
 	struct cyapa *cyapa = dev_get_drvdata(dev);
-	int tries;
+	unsigned long timeout;
 	int ret;
 
 	ret = cyapa_read_byte(cyapa, CYAPA_CMD_DEV_STATUS);
@@ -812,31 +812,28 @@ static ssize_t cyapa_gen3_do_calibrate(struct device *dev,
 		goto out;
 	}
 
-	tries = 20;  /* max recalibration timeout 2s. */
+	/* max recalibration timeout 2s. */
+	timeout = jiffies + 2 * HZ;
 	do {
 		/*
 		 * For this recalibration, the max time will not exceed 2s.
 		 * The average time is approximately 500 - 700 ms, and we
 		 * will check the status every 100 - 200ms.
 		 */
-		usleep_range(100000, 200000);
-
+		msleep(100);
 		ret = cyapa_read_byte(cyapa, CYAPA_CMD_DEV_STATUS);
 		if (ret < 0) {
-			dev_err(dev, "Error reading dev status: %d\n",
-				ret);
+			dev_err(dev, "Error reading dev status: %d\n", ret);
 			goto out;
 		}
-		if ((ret & CYAPA_DEV_NORMAL) == CYAPA_DEV_NORMAL)
-			break;
-	} while (--tries);
+		if ((ret & CYAPA_DEV_NORMAL) == CYAPA_DEV_NORMAL) {
+			dev_dbg(dev, "Calibration successful.\n");
+			goto out;
+		}
+	} while (time_is_after_jiffies(timeout));
 
-	if (tries == 0) {
-		dev_err(dev, "Failed to calibrate. Timeout.\n");
-		ret = -ETIMEDOUT;
-		goto out;
-	}
-	dev_dbg(dev, "Calibration successful.\n");
+	dev_err(dev, "Failed to calibrate. Timeout.\n");
+	ret = -ETIMEDOUT;
 
 out:
 	return ret < 0 ? ret : count;
-- 
2.1.4

             reply	other threads:[~2017-01-15 11:11 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-15 11:12 Nicholas Mc Guire [this message]
2017-01-15 23:19 ` [PATCH RFC V2] Input: cyapa: use time based retry loop Dmitry Torokhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1484478758-13133-1-git-send-email-hofrat@osadl.org \
    --to=hofrat@osadl.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=dudl@cypress.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rydberg@bitmath.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.