linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Herrmann <dh.herrmann@gmail.com>
To: linux-input@vger.kernel.org
Cc: Jiri Kosina <jkosina@suse.cz>,
	Benjamin Tissoires <benjamin.tissoires@gmail.com>,
	Henrik Rydberg <rydberg@euromail.se>,
	Oliver Neukum <oliver@neukum.org>,
	David Herrmann <dh.herrmann@gmail.com>
Subject: [RFC 3/8] HID: input: generic hidinput_input_event handler
Date: Mon, 15 Jul 2013 19:10:12 +0200	[thread overview]
Message-ID: <1373908217-16748-4-git-send-email-dh.herrmann@gmail.com> (raw)
In-Reply-To: <1373908217-16748-1-git-send-email-dh.herrmann@gmail.com>

The hidinput_input_event() callback converts input events written from
userspace into HID reports and sends them to the device. We currently
implement this in every HID transport driver, even though most of them do
the same.

This provides a generic hidinput_input_event() implementation which is
mostly copied from usbhid. It uses a delayed worker to allow multiple LED
events to be collected into a single output event.
We use the custom ->request() transport driver callback to allow drivers
to adjust the outgoing report and handle the request asynchronously. If no
custom ->request() callback is available, we fall back to the generic raw
output report handler (which is synchronous).

Drivers can still provide custom hidinput_input_event() handlers (see
logitech-dj) if the generic implementation doesn't fit their needs.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/hid/hid-input.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/hid.h     |  1 +
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 7480799..308eee8 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1137,6 +1137,74 @@ unsigned int hidinput_count_leds(struct hid_device *hid)
 }
 EXPORT_SYMBOL_GPL(hidinput_count_leds);
 
+static void hidinput_led_worker(struct work_struct *work)
+{
+	struct hid_device *hid = container_of(work, struct hid_device,
+					      led_work);
+	struct hid_field *field;
+	struct hid_report *report;
+	int len;
+	__u8 *buf;
+
+	field = hidinput_get_led_field(hid);
+	if (!field)
+		return;
+
+	/*
+	 * field->report is accessed unlocked regarding HID core. So there might
+	 * be another incoming SET-LED request from user-space, which changes
+	 * the LED state while we assemble our outgoing buffer. However, this
+	 * doesn't matter as hid_output_report() correctly converts it into a
+	 * boolean value no matter what information is currently set on the LED
+	 * field (even garbage). So the remote device will always get a valid
+	 * request.
+	 * And in case we send a wrong value, a next led worker is spawned
+	 * for every SET-LED request so the following worker will send the
+	 * correct value, guaranteed!
+	 */
+
+	report = field->report;
+
+	/* use custom SET_REPORT request if possible (asynchronous) */
+	if (hid->ll_driver->request)
+		return hid->ll_driver->request(hid, report, HID_REQ_SET_REPORT);
+
+	/* fall back to generic raw-output-report */
+	len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
+	buf = kmalloc(len, GFP_KERNEL);
+	if (!buf)
+		return;
+
+	hid_output_report(report, buf);
+	/* synchronous output report */
+	hid->hid_output_raw_report(hid, buf, len, HID_OUTPUT_REPORT);
+	kfree(buf);
+}
+
+static int hidinput_input_event(struct input_dev *dev, unsigned int type,
+				unsigned int code, int value)
+{
+	struct hid_device *hid = input_get_drvdata(dev);
+	struct hid_field *field;
+	int offset;
+
+	if (type == EV_FF)
+		return input_ff_event(dev, type, code, value);
+
+	if (type != EV_LED)
+		return -1;
+
+	if ((offset = hidinput_find_field(hid, type, code, &field)) == -1) {
+		hid_warn(dev, "event field not found\n");
+		return -1;
+	}
+
+	hid_set_field(field, offset, value);
+
+	schedule_work(&hid->led_work);
+	return 0;
+}
+
 static int hidinput_open(struct input_dev *dev)
 {
 	struct hid_device *hid = input_get_drvdata(dev);
@@ -1183,7 +1251,10 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid)
 	}
 
 	input_set_drvdata(input_dev, hid);
-	input_dev->event = hid->ll_driver->hidinput_input_event;
+	if (hid->ll_driver->hidinput_input_event)
+		input_dev->event = hid->ll_driver->hidinput_input_event;
+	else if (hid->ll_driver->request || hid->hid_output_raw_report)
+		input_dev->event = hidinput_input_event;
 	input_dev->open = hidinput_open;
 	input_dev->close = hidinput_close;
 	input_dev->setkeycode = hidinput_setkeycode;
@@ -1278,6 +1349,7 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
 	int i, j, k;
 
 	INIT_LIST_HEAD(&hid->inputs);
+	INIT_WORK(&hid->led_work, hidinput_led_worker);
 
 	if (!force) {
 		for (i = 0; i < hid->maxcollection; i++) {
@@ -1379,6 +1451,12 @@ void hidinput_disconnect(struct hid_device *hid)
 		input_unregister_device(hidinput->input);
 		kfree(hidinput);
 	}
+
+	/* led_work is spawned by input_dev callbacks, but doesn't access the
+	 * parent input_dev at all. Once all input devices are removed, we
+	 * know that led_work will never get restarted, so we can cancel it
+	 * synchronously and are safe. */
+	cancel_work_sync(&hid->led_work);
 }
 EXPORT_SYMBOL_GPL(hidinput_disconnect);
 
diff --git a/include/linux/hid.h b/include/linux/hid.h
index b8058c5..ea4b828 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -456,6 +456,7 @@ struct hid_device {							/* device report descriptor */
 	enum hid_type type;						/* device type (mouse, kbd, ...) */
 	unsigned country;						/* HID country */
 	struct hid_report_enum report_enum[HID_REPORT_TYPES];
+	struct work_struct led_work;					/* delayed LED worker */
 
 	struct semaphore driver_lock;					/* protects the current driver, except during input */
 	struct semaphore driver_input_lock;				/* protects the current driver */
-- 
1.8.3.2


  parent reply	other threads:[~2013-07-15 17:10 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-15 17:10 [RFC 0/8] HID: Transport Driver Cleanup David Herrmann
2013-07-15 17:10 ` [RFC 1/8] HID: usbhid: make usbhid_set_leds() static David Herrmann
2013-07-16  7:41   ` Benjamin Tissoires
2013-07-15 17:10 ` [RFC 2/8] HID: usbhid: update LED fields unlocked David Herrmann
2013-07-16  7:46   ` Benjamin Tissoires
2013-07-31  8:28     ` Jiri Kosina
2013-07-15 17:10 ` David Herrmann [this message]
2013-07-16  8:04   ` [RFC 3/8] HID: input: generic hidinput_input_event handler Benjamin Tissoires
2013-07-17 13:58     ` David Herrmann
2013-07-31  8:30       ` Jiri Kosina
2013-07-15 17:10 ` [RFC 4/8] HID: usbhid: use generic hidinput_input_event() David Herrmann
2013-07-16  8:06   ` Benjamin Tissoires
2013-07-15 17:10 ` [RFC 5/8] HID: i2c: " David Herrmann
2013-07-16  8:08   ` Benjamin Tissoires
2013-07-15 17:10 ` [RFC 6/8] HID: uhid: " David Herrmann
2013-07-16  8:10   ` Benjamin Tissoires
2013-07-18 19:53   ` rydberg
2013-07-18 20:49     ` David Herrmann
2013-07-15 17:10 ` [RFC 7/8] HID: add transport driver documentation David Herrmann
2013-07-16 10:32   ` Benjamin Tissoires
2013-07-17 15:05     ` David Herrmann
2013-07-18  8:16       ` Benjamin Tissoires
2013-07-15 17:10 ` [RFC 8/8] HID: implement new transport-driver callbacks David Herrmann
2013-07-15 18:55 ` [RFC 0/8] HID: Transport Driver Cleanup Benjamin Tissoires
2013-07-31  8:38 ` Jiri Kosina
2013-07-31  8:57   ` David Herrmann
2013-07-31  9:03     ` Jiri Kosina

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=1373908217-16748-4-git-send-email-dh.herrmann@gmail.com \
    --to=dh.herrmann@gmail.com \
    --cc=benjamin.tissoires@gmail.com \
    --cc=jkosina@suse.cz \
    --cc=linux-input@vger.kernel.org \
    --cc=oliver@neukum.org \
    --cc=rydberg@euromail.se \
    /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 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).