All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Herrmann <dh.herrmann@googlemail.com>
To: linux-input@vger.kernel.org
Cc: padovan@profusion.mobi, jkosina@suse.cz, oliver@neukum.org,
	dh.herrmann@googlemail.com
Subject: [PATCH 07/12 v2] HID: wiimote: Add output queue for wiimote driver
Date: Fri, 17 Jun 2011 16:49:17 +0200	[thread overview]
Message-ID: <1308322162-13953-7-git-send-email-dh.herrmann@googlemail.com> (raw)
In-Reply-To: <1308322162-13953-1-git-send-email-dh.herrmann@googlemail.com>

The raw hid output function that is supported by bluetooth low-level
hid driver does not provide an output queue and also may sleep. The
wiimote driver, though, may need to send data in atomic context so
this patch adds a buffered output queue for the wiimote driver.

We use the shared workqueue to send our buffer to the hid device.
There is always only one active worker which reschedules itself until
the wiimote queue is empty. This prevents the worker from occupying
the shared workqueue for too long. If our queue is full, every further
output is discarded.

Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
V2: Add write memory barrier after atomic_set operation.

 drivers/hid/hid-wiimote.c |  106 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-wiimote.c b/drivers/hid/hid-wiimote.c
index b6ae2a1..c06fdc8 100644
--- a/drivers/hid/hid-wiimote.c
+++ b/drivers/hid/hid-wiimote.c
@@ -15,15 +15,28 @@
 #include <linux/hid.h>
 #include <linux/input.h>
 #include <linux/module.h>
+#include <linux/spinlock.h>
 #include "hid-ids.h"
 
 #define WIIMOTE_VERSION "0.1"
 #define WIIMOTE_NAME "Nintendo Wii Remote"
+#define WIIMOTE_BUFSIZE 32
+
+struct wiimote_buf {
+	__u8 data[HID_MAX_BUFFER_SIZE];
+	size_t size;
+};
 
 struct wiimote_data {
 	atomic_t ready;
 	struct hid_device *hdev;
 	struct input_dev *input;
+
+	spinlock_t qlock;
+	__u8 head;
+	__u8 tail;
+	struct wiimote_buf outq[WIIMOTE_BUFSIZE];
+	struct work_struct worker;
 };
 
 static ssize_t wiimote_hid_send(struct hid_device *hdev, __u8 *buffer,
@@ -45,6 +58,79 @@ static ssize_t wiimote_hid_send(struct hid_device *hdev, __u8 *buffer,
 	return ret;
 }
 
+static void wiimote_worker(struct work_struct *work)
+{
+	struct wiimote_data *wdata = container_of(work, struct wiimote_data,
+									worker);
+	unsigned long flags;
+
+	if (!atomic_read(&wdata->ready))
+		return;
+
+	spin_lock_irqsave(&wdata->qlock, flags);
+
+	if (wdata->head != wdata->tail) {
+		spin_unlock_irqrestore(&wdata->qlock, flags);
+		wiimote_hid_send(wdata->hdev, wdata->outq[wdata->tail].data,
+						wdata->outq[wdata->tail].size);
+		spin_lock_irqsave(&wdata->qlock, flags);
+
+		wdata->tail = (wdata->tail + 1) % WIIMOTE_BUFSIZE;
+
+		/*
+		 * We are using the shared workqueue so send at most one whole
+		 * buffer and then reschedule ourself so we do not occupy the
+		 * shared workqueue for too long.
+		 */
+		if (wdata->head != wdata->tail)
+			schedule_work(&wdata->worker);
+	}
+
+	spin_unlock_irqrestore(&wdata->qlock, flags);
+}
+
+static void wiimote_queue(struct wiimote_data *wdata, const __u8 *buffer,
+								size_t count)
+{
+	unsigned long flags;
+	__u8 newhead;
+
+	if (!atomic_read(&wdata->ready))
+		return;
+
+	if (count > HID_MAX_BUFFER_SIZE) {
+		hid_warn(wdata->hdev, "Sending too large output report\n");
+		return;
+	}
+
+	/*
+	 * Copy new request into our output queue and check whether the
+	 * queue is full. If it is full, discard this request.
+	 * If it is empty we need to start a new worker that will
+	 * send out the buffer to the hid device.
+	 * If the queue is not empty, then there must be a worker
+	 * that is currently sending out our buffer and this worker
+	 * will reschedule itself until the queue is empty.
+	 */
+
+	spin_lock_irqsave(&wdata->qlock, flags);
+
+	memcpy(wdata->outq[wdata->head].data, buffer, count);
+	wdata->outq[wdata->head].size = count;
+	newhead = (wdata->head + 1) % WIIMOTE_BUFSIZE;
+
+	if (wdata->head == wdata->tail) {
+		wdata->head = newhead;
+		schedule_work(&wdata->worker);
+	} else if (newhead != wdata->tail) {
+		wdata->head = newhead;
+	} else {
+		hid_warn(wdata->hdev, "Output queue is full");
+	}
+
+	spin_unlock_irqrestore(&wdata->qlock, flags);
+}
+
 static int wiimote_input_event(struct input_dev *dev, unsigned int type,
 						unsigned int code, int value)
 {
@@ -98,6 +184,9 @@ static struct wiimote_data *wiimote_create(struct hid_device *hdev)
 	wdata->input->id.version = wdata->hdev->version;
 	wdata->input->name = WIIMOTE_NAME;
 
+	spin_lock_init(&wdata->qlock);
+	INIT_WORK(&wdata->worker, wiimote_worker);
+
 	return wdata;
 }
 
@@ -154,8 +243,25 @@ static void wiimote_hid_remove(struct hid_device *hdev)
 	struct wiimote_data *wdata = hid_get_drvdata(hdev);
 
 	hid_info(hdev, "Device removed\n");
+
+	/*
+	 * We handle events from HID, input and sysfs and
+	 * send requests to HID and input. So stop sysfs
+	 * first, then lock HID output and stop HID.
+	 * Now only input is left so stop it and free the
+	 * the device data.
+	 */
+
+	atomic_set(&wdata->ready, 0);
+	smp_wmb();
+	cancel_work_sync(&wdata->worker);
+
 	hid_hw_stop(hdev);
 	input_unregister_device(wdata->input);
+
+	/* the worker may have been rescheduled so wait again for it */
+	cancel_work_sync(&wdata->worker);
+
 	wiimote_destroy(wdata);
 }
 
-- 
1.7.5.2


  parent reply	other threads:[~2011-06-17 14:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-17 14:49 [PATCH 01/12 v2] HID: wiimote: Add Nintendo Wii Remote driver stub David Herrmann
2011-06-17 14:49 ` [PATCH 02/12 v2] HID: wiimote: Register wiimote hid " David Herrmann
2011-06-17 14:49 ` [PATCH 03/12 v2] HID: wiimote: Add wiimote device structure David Herrmann
2011-06-17 14:49 ` [PATCH 04/12 v2] HID: wiimote: Register input device in wiimote hid driver David Herrmann
2011-06-17 14:49 ` [PATCH 05/12 v2] HID: wiimote: Synchronize wiimote input and hid event handling David Herrmann
2011-06-17 14:49 ` [PATCH 06/12 v2] HID: wiimote: Add wiimote send function David Herrmann
2011-06-17 14:49 ` David Herrmann [this message]
2011-06-17 14:49 ` [PATCH 08/12 v2] HID: wiimote: Add wiimote event handler David Herrmann
2011-06-17 14:49 ` [PATCH 09/12 v2] HID: wiimote: Add wiimote input button parser David Herrmann
2011-06-17 14:49 ` [PATCH 10/12 v2] HID: wiimote: Add wiimote led request David Herrmann
2011-06-17 14:49 ` [PATCH 11/12 v2] HID: wiimote: Cache wiimote led state David Herrmann
2011-06-17 14:49 ` [PATCH 12/12 v2] HID: wiimote: Add sysfs support to wiimote driver David Herrmann
2011-06-24 12:14 ` [PATCH 01/12 v2] HID: wiimote: Add Nintendo Wii Remote driver stub Jiri Kosina
2011-06-27 14:29   ` David Herrmann

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=1308322162-13953-7-git-send-email-dh.herrmann@googlemail.com \
    --to=dh.herrmann@googlemail.com \
    --cc=jkosina@suse.cz \
    --cc=linux-input@vger.kernel.org \
    --cc=oliver@neukum.org \
    --cc=padovan@profusion.mobi \
    /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.