All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Input/HID: Guitar/Drums support
@ 2013-08-26 17:14 David Herrmann
  2013-08-26 17:14 ` [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars David Herrmann
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: David Herrmann @ 2013-08-26 17:14 UTC (permalink / raw)
  To: linux-input
  Cc: Jiri Kosina, Nicolas Adenis-Lamarre, Dmitry Torokhov, David Herrmann

Hi

This series adds wiimote extension support for guitars and drums. The patches
are fairly simple and work the same as the other 5 wiimote extensions. But as we
currently don't have IDs to report drums or guitar events to user-space, I had
to add them.

Patch #1 adds basic IDs to <linux/input.h>. I tried to find as many devices as I
could and looked what IDs would be required. Turned out, the device I was
implementing already supported almost all (except 2 instead of 4 cymbals and 3
instead of 4 toms).

Patch #2 and #3 add the wiimote extensions.

This has been mostly worked out and tested by Nicolas (thanks!) and we started
this work almost 3 month ago. Due to holidays it took a bit longer to get
pressure reports working. But now everything should be fine.

The xwiimote user-space tools already support this and provide testing utilities
if someone is interested: http://github.com/dvdhrm/xwiimote

Jiri, Dmitry, can we try merging this for 3.13? I know it's late, but it did see
a lot of testing from Nicolas.

Regards
David

Btw., if someone has similar PS3 or WorldTour devices, I'd be glad to add
support for them!

David Herrmann (2):
  Input: introduce BTN/ABS bits for drums and guitars
  HID: wiimote: add support for Guitar-Hero drums

Nicolas Adenis-Lamarre (1):
  HID: wiimote: add support for Guitar-Hero guitars

 drivers/hid/hid-wiimote-core.c    |  14 ++
 drivers/hid/hid-wiimote-modules.c | 392 ++++++++++++++++++++++++++++++++++++++
 drivers/hid/hid-wiimote.h         |   3 +
 include/linux/mod_devicetable.h   |   2 +-
 include/uapi/linux/input.h        |  25 ++-
 5 files changed, 433 insertions(+), 3 deletions(-)

-- 
1.8.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars
  2013-08-26 17:14 [PATCH 0/3] Input/HID: Guitar/Drums support David Herrmann
@ 2013-08-26 17:14 ` David Herrmann
  2013-09-02 11:41   ` Jiri Kosina
  2013-08-26 17:14 ` [PATCH 2/3] HID: wiimote: add support for Guitar-Hero drums David Herrmann
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: David Herrmann @ 2013-08-26 17:14 UTC (permalink / raw)
  To: linux-input
  Cc: Jiri Kosina, Nicolas Adenis-Lamarre, Dmitry Torokhov, David Herrmann

There are a bunch of guitar and drums devices out there that all report
similar data. To avoid reporting this as BTN_MISC or ABS_MISC, we
allocate some proper namespace for them. Note that most of these devices
are toys and we cannot report any sophisticated physics via this API.

I did some google-images research and tried to provide definitions that
work with all common devices. That's why I went with 4 toms, 4 cymbals,
one bass, one hi-hat. I haven't seen other drums and I doubt that we need
any additions to that. Anyway, the naming-scheme is intentionally done in
an extensible way.

For guitars, we support 5 frets (normally aligned vertically, compared to
the real horizontal layouts), a single strum-bar with up/down directions,
an optional fret-board and a whammy-bar.

Most of the devices provide pressure values so I went with ABS_* bits. If
we ever support devices which only provide digital input, we have to
decide whether to emulate pressure data or add additional BTN_* bits.

If someone is not familiar with these devices, here are two pictures which
provide almost all introduced interfaces (or try the given keywords
with a google-image search):
  Guitar: ("guitar hero world tour guitar")
    http://images1.wikia.nocookie.net/__cb20120911023442/applezone/es/images/f/f9/Wii_Guitar.jpg
  Drums: ("guitar hero drums")
    http://oyster.ignimgs.com/franchises/images/03/55/35526_band-hero-drum-set-hands-on-20090929040735768.jpg

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 include/linux/mod_devicetable.h |  2 +-
 include/uapi/linux/input.h      | 25 +++++++++++++++++++++++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 45e9214..329aa30 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -277,7 +277,7 @@ struct pcmcia_device_id {
 #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING	0x71
 #define INPUT_DEVICE_ID_KEY_MAX		0x2ff
 #define INPUT_DEVICE_ID_REL_MAX		0x0f
-#define INPUT_DEVICE_ID_ABS_MAX		0x3f
+#define INPUT_DEVICE_ID_ABS_MAX		0x4f
 #define INPUT_DEVICE_ID_MSC_MAX		0x07
 #define INPUT_DEVICE_ID_LED_MAX		0x0f
 #define INPUT_DEVICE_ID_SND_MAX		0x07
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index d584047..76457ee 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -716,6 +716,14 @@ struct input_keymap_entry {
 #define BTN_DPAD_LEFT		0x222
 #define BTN_DPAD_RIGHT		0x223
 
+#define BTN_FRET_FAR_UP		0x224
+#define BTN_FRET_UP		0x225
+#define BTN_FRET_MID		0x226
+#define BTN_FRET_LOW		0x227
+#define BTN_FRET_FAR_LOW	0x228
+#define BTN_STRUM_BAR_UP	0x229
+#define BTN_STRUM_BAR_DOWN	0x22a
+
 #define BTN_TRIGGER_HAPPY		0x2c0
 #define BTN_TRIGGER_HAPPY1		0x2c0
 #define BTN_TRIGGER_HAPPY2		0x2c1
@@ -829,8 +837,21 @@ struct input_keymap_entry {
 #define ABS_MT_TOOL_X		0x3c	/* Center X tool position */
 #define ABS_MT_TOOL_Y		0x3d	/* Center Y tool position */
 
-
-#define ABS_MAX			0x3f
+/* Drums and guitars (mostly toys) */
+#define ABS_TOM_FAR_LEFT	0x40
+#define ABS_TOM_LEFT		0x41
+#define ABS_TOM_RIGHT		0x42
+#define ABS_TOM_FAR_RIGHT	0x43
+#define ABS_CYMBAL_FAR_LEFT	0x44
+#define ABS_CYMBAL_LEFT		0x45
+#define ABS_CYMBAL_RIGHT	0x46
+#define ABS_CYMBAL_FAR_RIGHT	0x47
+#define ABS_BASS		0x48
+#define ABS_HI_HAT		0x49
+#define ABS_FRET_BOARD		0x4a	/* Guitar fret board, vertical pos */
+#define ABS_WHAMMY_BAR		0x4b	/* Guitar whammy bar (or vibrato) */
+
+#define ABS_MAX			0x4f
 #define ABS_CNT			(ABS_MAX+1)
 
 /*
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] HID: wiimote: add support for Guitar-Hero drums
  2013-08-26 17:14 [PATCH 0/3] Input/HID: Guitar/Drums support David Herrmann
  2013-08-26 17:14 ` [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars David Herrmann
@ 2013-08-26 17:14 ` David Herrmann
  2013-08-26 17:14 ` [PATCH 3/3] HID: wiimote: add support for Guitar-Hero guitars David Herrmann
  2013-09-04  8:46 ` [PATCH 0/3] Input/HID: Guitar/Drums support Jiri Kosina
  3 siblings, 0 replies; 8+ messages in thread
From: David Herrmann @ 2013-08-26 17:14 UTC (permalink / raw)
  To: linux-input
  Cc: Jiri Kosina, Nicolas Adenis-Lamarre, Dmitry Torokhov, David Herrmann

Guitar-Hero comes with a drums extension. Use the newly introduced input
drums-bits to report this back to user-space. This is a usual extension
like any other device. Nothing special to take care of.

We report this to user-space as "Nintendo Wii Remote Drums". There are
other drums (like "RockBand" drums) which we currently do not support and
maybe will at some point. However, it is quite likely that we can report
these via the same interface. This allows user-space to work with them
without knowing the exact branding.
I couldn't find anyone who owns a "RockBand" device, though.

Initial-work-by: Nicolas Adenis-Lamarre <nicolas.adenis.lamarre@gmail.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/hid/hid-wiimote-core.c    |   7 ++
 drivers/hid/hid-wiimote-modules.c | 218 ++++++++++++++++++++++++++++++++++++++
 drivers/hid/hid-wiimote.h         |   2 +
 3 files changed, 227 insertions(+)

diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index 6602098..946cdef 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -456,6 +456,9 @@ static __u8 wiimote_cmd_read_ext(struct wiimote_data *wdata, __u8 *rmem)
 		return WIIMOTE_EXT_BALANCE_BOARD;
 	if (rmem[4] == 0x01 && rmem[5] == 0x20)
 		return WIIMOTE_EXT_PRO_CONTROLLER;
+	if (rmem[0] == 0x01 && rmem[1] == 0x00 &&
+	    rmem[4] == 0x01 && rmem[5] == 0x03)
+		return WIIMOTE_EXT_GUITAR_HERO_DRUMS;
 
 	return WIIMOTE_EXT_UNKNOWN;
 }
@@ -489,6 +492,7 @@ static bool wiimote_cmd_map_mp(struct wiimote_data *wdata, __u8 exttype)
 	/* map MP with correct pass-through mode */
 	switch (exttype) {
 	case WIIMOTE_EXT_CLASSIC_CONTROLLER:
+	case WIIMOTE_EXT_GUITAR_HERO_DRUMS:
 		wmem = 0x07;
 		break;
 	case WIIMOTE_EXT_NUNCHUK:
@@ -1079,6 +1083,7 @@ static const char *wiimote_exttype_names[WIIMOTE_EXT_NUM] = {
 	[WIIMOTE_EXT_CLASSIC_CONTROLLER] = "Nintendo Wii Classic Controller",
 	[WIIMOTE_EXT_BALANCE_BOARD] = "Nintendo Wii Balance Board",
 	[WIIMOTE_EXT_PRO_CONTROLLER] = "Nintendo Wii U Pro Controller",
+	[WIIMOTE_EXT_GUITAR_HERO_DRUMS] = "Nintendo Wii Guitar Hero Drums",
 };
 
 /*
@@ -1665,6 +1670,8 @@ static ssize_t wiimote_ext_show(struct device *dev,
 		return sprintf(buf, "balanceboard\n");
 	case WIIMOTE_EXT_PRO_CONTROLLER:
 		return sprintf(buf, "procontroller\n");
+	case WIIMOTE_EXT_GUITAR_HERO_DRUMS:
+		return sprintf(buf, "drums\n");
 	case WIIMOTE_EXT_UNKNOWN:
 		/* fallthrough */
 	default:
diff --git a/drivers/hid/hid-wiimote-modules.c b/drivers/hid/hid-wiimote-modules.c
index 2e7d644..08a44a7 100644
--- a/drivers/hid/hid-wiimote-modules.c
+++ b/drivers/hid/hid-wiimote-modules.c
@@ -1834,6 +1834,223 @@ static const struct wiimod_ops wiimod_pro = {
 };
 
 /*
+ * Drums
+ * Guitar-Hero, Rock-Band and other games came bundled with drums which can
+ * be plugged as extension to a Wiimote. Drum-reports are still not entirely
+ * figured out, but the most important information is known.
+ * We create a separate device for drums and report all information via this
+ * input device.
+ */
+
+static inline void wiimod_drums_report_pressure(struct wiimote_data *wdata,
+						__u8 none, __u8 which,
+						__u8 pressure, __u8 onoff,
+						__u8 *store, __u16 code,
+						__u8 which_code)
+{
+	static const __u8 default_pressure = 3;
+
+	if (!none && which == which_code) {
+		*store = pressure;
+		input_report_abs(wdata->extension.input, code, *store);
+	} else if (onoff != !!*store) {
+		*store = onoff ? default_pressure : 0;
+		input_report_abs(wdata->extension.input, code, *store);
+	}
+}
+
+static void wiimod_drums_in_ext(struct wiimote_data *wdata, const __u8 *ext)
+{
+	__u8 pressure, which, none, hhp, sx, sy;
+	__u8 o, r, y, g, b, bass, bm, bp;
+
+	/*   Byte |  8  |  7  |  6  |  5  |  4  |  3  |  2  |  1  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    1   |  0  |  0  |              SX <5:0>             |
+	 *    2   |  0  |  0  |              SY <5:0>             |
+	 *   -----+-----+-----+-----------------------------+-----+
+	 *    3   | HPP | NON |         WHICH <5:1>         |  ?  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    4   |   SOFT <7:5>    |  0  |  1  |  1  |  0  |  ?  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    5   |  ?  |  1  |  1  | B-  |  1  | B+  |  1  |  ?  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    6   |  O  |  R  |  Y  |  G  |  B  | BSS |  1  |  1  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 * All buttons are 0 if pressed
+	 *
+	 * With Motion+ enabled, the following bits will get invalid:
+	 *   Byte |  8  |  7  |  6  |  5  |  4  |  3  |  2  |  1  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    1   |  0  |  0  |              SX <5:1>       |XXXXX|
+	 *    2   |  0  |  0  |              SY <5:1>       |XXXXX|
+	 *   -----+-----+-----+-----------------------------+-----+
+	 *    3   | HPP | NON |         WHICH <5:1>         |  ?  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    4   |   SOFT <7:5>    |  0  |  1  |  1  |  0  |  ?  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    5   |  ?  |  1  |  1  | B-  |  1  | B+  |  1  |XXXXX|
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    6   |  O  |  R  |  Y  |  G  |  B  | BSS |XXXXX|XXXXX|
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 */
+
+	pressure = 7 - (ext[3] >> 5);
+	which = (ext[2] >> 1) & 0x1f;
+	none = !!(ext[2] & 0x40);
+	hhp = !(ext[2] & 0x80);
+	sx = ext[0] & 0x3f;
+	sy = ext[1] & 0x3f;
+	o = !(ext[5] & 0x80);
+	r = !(ext[5] & 0x40);
+	y = !(ext[5] & 0x20);
+	g = !(ext[5] & 0x10);
+	b = !(ext[5] & 0x08);
+	bass = !(ext[5] & 0x04);
+	bm = !(ext[4] & 0x10);
+	bp = !(ext[4] & 0x04);
+
+	wiimod_drums_report_pressure(wdata, none, which, pressure,
+				     o, &wdata->state.pressure_drums[0],
+				     ABS_CYMBAL_RIGHT, 0x0e);
+	wiimod_drums_report_pressure(wdata, none, which, pressure,
+				     r, &wdata->state.pressure_drums[1],
+				     ABS_TOM_LEFT, 0x19);
+	wiimod_drums_report_pressure(wdata, none, which, pressure,
+				     y, &wdata->state.pressure_drums[2],
+				     ABS_CYMBAL_LEFT, 0x11);
+	wiimod_drums_report_pressure(wdata, none, which, pressure,
+				     g, &wdata->state.pressure_drums[3],
+				     ABS_TOM_FAR_RIGHT, 0x12);
+	wiimod_drums_report_pressure(wdata, none, which, pressure,
+				     b, &wdata->state.pressure_drums[4],
+				     ABS_TOM_RIGHT, 0x0f);
+
+	/* Bass shares pressure with hi-hat (set via hhp) */
+	wiimod_drums_report_pressure(wdata, none, hhp ? 0xff : which, pressure,
+				     bass, &wdata->state.pressure_drums[5],
+				     ABS_BASS, 0x1b);
+	/* Hi-hat has no on/off values, just pressure. Force to off/0. */
+	wiimod_drums_report_pressure(wdata, none, hhp ? which : 0xff, pressure,
+				     0, &wdata->state.pressure_drums[6],
+				     ABS_HI_HAT, 0x0e);
+
+	input_report_abs(wdata->extension.input, ABS_X, sx - 0x20);
+	input_report_abs(wdata->extension.input, ABS_Y, sy - 0x20);
+
+	input_report_key(wdata->extension.input, BTN_START, bp);
+	input_report_key(wdata->extension.input, BTN_SELECT, bm);
+
+	input_sync(wdata->extension.input);
+}
+
+static int wiimod_drums_open(struct input_dev *dev)
+{
+	struct wiimote_data *wdata = input_get_drvdata(dev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&wdata->state.lock, flags);
+	wdata->state.flags |= WIIPROTO_FLAG_EXT_USED;
+	wiiproto_req_drm(wdata, WIIPROTO_REQ_NULL);
+	spin_unlock_irqrestore(&wdata->state.lock, flags);
+
+	return 0;
+}
+
+static void wiimod_drums_close(struct input_dev *dev)
+{
+	struct wiimote_data *wdata = input_get_drvdata(dev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&wdata->state.lock, flags);
+	wdata->state.flags &= ~WIIPROTO_FLAG_EXT_USED;
+	wiiproto_req_drm(wdata, WIIPROTO_REQ_NULL);
+	spin_unlock_irqrestore(&wdata->state.lock, flags);
+}
+
+static int wiimod_drums_probe(const struct wiimod_ops *ops,
+			      struct wiimote_data *wdata)
+{
+	int ret;
+
+	wdata->extension.input = input_allocate_device();
+	if (!wdata->extension.input)
+		return -ENOMEM;
+
+	input_set_drvdata(wdata->extension.input, wdata);
+	wdata->extension.input->open = wiimod_drums_open;
+	wdata->extension.input->close = wiimod_drums_close;
+	wdata->extension.input->dev.parent = &wdata->hdev->dev;
+	wdata->extension.input->id.bustype = wdata->hdev->bus;
+	wdata->extension.input->id.vendor = wdata->hdev->vendor;
+	wdata->extension.input->id.product = wdata->hdev->product;
+	wdata->extension.input->id.version = wdata->hdev->version;
+	wdata->extension.input->name = WIIMOTE_NAME " Drums";
+
+	set_bit(EV_KEY, wdata->extension.input->evbit);
+	set_bit(BTN_START, wdata->extension.input->keybit);
+	set_bit(BTN_SELECT, wdata->extension.input->keybit);
+
+	set_bit(EV_ABS, wdata->extension.input->evbit);
+	set_bit(ABS_X, wdata->extension.input->absbit);
+	set_bit(ABS_Y, wdata->extension.input->absbit);
+	set_bit(ABS_TOM_LEFT, wdata->extension.input->absbit);
+	set_bit(ABS_TOM_RIGHT, wdata->extension.input->absbit);
+	set_bit(ABS_TOM_FAR_RIGHT, wdata->extension.input->absbit);
+	set_bit(ABS_CYMBAL_LEFT, wdata->extension.input->absbit);
+	set_bit(ABS_CYMBAL_RIGHT, wdata->extension.input->absbit);
+	set_bit(ABS_BASS, wdata->extension.input->absbit);
+	set_bit(ABS_HI_HAT, wdata->extension.input->absbit);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_X, -32, 31, 1, 1);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_Y, -32, 31, 1, 1);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_TOM_LEFT, 0, 7, 0, 0);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_TOM_RIGHT, 0, 7, 0, 0);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_TOM_FAR_RIGHT, 0, 7, 0, 0);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_CYMBAL_LEFT, 0, 7, 0, 0);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_CYMBAL_RIGHT, 0, 7, 0, 0);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_BASS, 0, 7, 0, 0);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_HI_HAT, 0, 7, 0, 0);
+
+	ret = input_register_device(wdata->extension.input);
+	if (ret)
+		goto err_free;
+
+	return 0;
+
+err_free:
+	input_free_device(wdata->extension.input);
+	wdata->extension.input = NULL;
+	return ret;
+}
+
+static void wiimod_drums_remove(const struct wiimod_ops *ops,
+				struct wiimote_data *wdata)
+{
+	if (!wdata->extension.input)
+		return;
+
+	input_unregister_device(wdata->extension.input);
+	wdata->extension.input = NULL;
+}
+
+static const struct wiimod_ops wiimod_drums = {
+	.flags = 0,
+	.arg = 0,
+	.probe = wiimod_drums_probe,
+	.remove = wiimod_drums_remove,
+	.in_ext = wiimod_drums_in_ext,
+};
+
+/*
  * Builtin Motion Plus
  * This module simply sets the WIIPROTO_FLAG_BUILTIN_MP protocol flag which
  * disables polling for Motion-Plus. This should be set only for devices which
@@ -2083,4 +2300,5 @@ const struct wiimod_ops *wiimod_ext_table[WIIMOTE_EXT_NUM] = {
 	[WIIMOTE_EXT_CLASSIC_CONTROLLER] = &wiimod_classic,
 	[WIIMOTE_EXT_BALANCE_BOARD] = &wiimod_bboard,
 	[WIIMOTE_EXT_PRO_CONTROLLER] = &wiimod_pro,
+	[WIIMOTE_EXT_GUITAR_HERO_DRUMS] = &wiimod_drums,
 };
diff --git a/drivers/hid/hid-wiimote.h b/drivers/hid/hid-wiimote.h
index f1474f3..6e18b55 100644
--- a/drivers/hid/hid-wiimote.h
+++ b/drivers/hid/hid-wiimote.h
@@ -88,6 +88,7 @@ enum wiimote_exttype {
 	WIIMOTE_EXT_CLASSIC_CONTROLLER,
 	WIIMOTE_EXT_BALANCE_BOARD,
 	WIIMOTE_EXT_PRO_CONTROLLER,
+	WIIMOTE_EXT_GUITAR_HERO_DRUMS,
 	WIIMOTE_EXT_NUM,
 };
 
@@ -135,6 +136,7 @@ struct wiimote_state {
 
 	/* calibration data */
 	__u16 calib_bboard[4][3];
+	__u8 pressure_drums[7];
 };
 
 struct wiimote_data {
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] HID: wiimote: add support for Guitar-Hero guitars
  2013-08-26 17:14 [PATCH 0/3] Input/HID: Guitar/Drums support David Herrmann
  2013-08-26 17:14 ` [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars David Herrmann
  2013-08-26 17:14 ` [PATCH 2/3] HID: wiimote: add support for Guitar-Hero drums David Herrmann
@ 2013-08-26 17:14 ` David Herrmann
  2013-09-04  8:46 ` [PATCH 0/3] Input/HID: Guitar/Drums support Jiri Kosina
  3 siblings, 0 replies; 8+ messages in thread
From: David Herrmann @ 2013-08-26 17:14 UTC (permalink / raw)
  To: linux-input
  Cc: Jiri Kosina, Nicolas Adenis-Lamarre, Dmitry Torokhov, David Herrmann

From: Nicolas Adenis-Lamarre <nicolas.adenis.lamarre@gmail.com>

Apart from drums, Guitar-Hero also ships with guitars. Use the recently
introduced input ABS/BTN-bits to report this to user-space.

Devices are reported as "Nintendo Wii Remote Guitar". If I ever get my
hands on "RockBand" guitars, I will try to report them via the same
interface so user-space does not have to bother which device it deals
with.

Signed-off-by: Nicolas.Adenis-Lamarre <nicolas.adenis.lamarre@gmail.com>
(add commit-msg and adjust to new BTN_* IDs)
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/hid/hid-wiimote-core.c    |   7 ++
 drivers/hid/hid-wiimote-modules.c | 174 ++++++++++++++++++++++++++++++++++++++
 drivers/hid/hid-wiimote.h         |   1 +
 3 files changed, 182 insertions(+)

diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index 946cdef..d3055d1 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -459,6 +459,9 @@ static __u8 wiimote_cmd_read_ext(struct wiimote_data *wdata, __u8 *rmem)
 	if (rmem[0] == 0x01 && rmem[1] == 0x00 &&
 	    rmem[4] == 0x01 && rmem[5] == 0x03)
 		return WIIMOTE_EXT_GUITAR_HERO_DRUMS;
+	if (rmem[0] == 0x00 && rmem[1] == 0x00 &&
+	    rmem[4] == 0x01 && rmem[5] == 0x03)
+		return WIIMOTE_EXT_GUITAR_HERO_GUITAR;
 
 	return WIIMOTE_EXT_UNKNOWN;
 }
@@ -493,6 +496,7 @@ static bool wiimote_cmd_map_mp(struct wiimote_data *wdata, __u8 exttype)
 	switch (exttype) {
 	case WIIMOTE_EXT_CLASSIC_CONTROLLER:
 	case WIIMOTE_EXT_GUITAR_HERO_DRUMS:
+	case WIIMOTE_EXT_GUITAR_HERO_GUITAR:
 		wmem = 0x07;
 		break;
 	case WIIMOTE_EXT_NUNCHUK:
@@ -1084,6 +1088,7 @@ static const char *wiimote_exttype_names[WIIMOTE_EXT_NUM] = {
 	[WIIMOTE_EXT_BALANCE_BOARD] = "Nintendo Wii Balance Board",
 	[WIIMOTE_EXT_PRO_CONTROLLER] = "Nintendo Wii U Pro Controller",
 	[WIIMOTE_EXT_GUITAR_HERO_DRUMS] = "Nintendo Wii Guitar Hero Drums",
+	[WIIMOTE_EXT_GUITAR_HERO_GUITAR] = "Nintendo Wii Guitar Hero Guitar",
 };
 
 /*
@@ -1672,6 +1677,8 @@ static ssize_t wiimote_ext_show(struct device *dev,
 		return sprintf(buf, "procontroller\n");
 	case WIIMOTE_EXT_GUITAR_HERO_DRUMS:
 		return sprintf(buf, "drums\n");
+	case WIIMOTE_EXT_GUITAR_HERO_GUITAR:
+		return sprintf(buf, "guitar\n");
 	case WIIMOTE_EXT_UNKNOWN:
 		/* fallthrough */
 	default:
diff --git a/drivers/hid/hid-wiimote-modules.c b/drivers/hid/hid-wiimote-modules.c
index 08a44a7..7e124c3 100644
--- a/drivers/hid/hid-wiimote-modules.c
+++ b/drivers/hid/hid-wiimote-modules.c
@@ -2051,6 +2051,179 @@ static const struct wiimod_ops wiimod_drums = {
 };
 
 /*
+ * Guitar
+ * Guitar-Hero, Rock-Band and other games came bundled with guitars which can
+ * be plugged as extension to a Wiimote.
+ * We create a separate device for guitars and report all information via this
+ * input device.
+ */
+
+static void wiimod_guitar_in_ext(struct wiimote_data *wdata, const __u8 *ext)
+{
+	__u8 sx, sy, tb, wb, bd, bm, bp, bo, br, bb, bg, by, bu;
+
+	/*   Byte |  8  |  7  |  6  |  5  |  4  |  3  |  2  |  1  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    1   |  0  |  0  |              SX <5:0>             |
+	 *    2   |  0  |  0  |              SY <5:0>             |
+	 *   -----+-----+-----+-----+-----------------------------+
+	 *    3   |  0  |  0  |  0  |      TB <4:0>               |
+	 *   -----+-----+-----+-----+-----------------------------+
+	 *    4   |  0  |  0  |  0  |      WB <4:0>               |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    5   |  1  | BD  |  1  | B-  |  1  | B+  |  1  |  1  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    6   | BO  | BR  | BB  | BG  | BY  |  1  |  1  | BU  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 * All buttons are 0 if pressed
+	 *
+	 * With Motion+ enabled, the following bits will get invalid:
+	 *   Byte |  8  |  7  |  6  |  5  |  4  |  3  |  2  |  1  |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    1   |  0  |  0  |              SX <5:1>       |XXXXX|
+	 *    2   |  0  |  0  |              SY <5:1>       |XXXXX|
+	 *   -----+-----+-----+-----+-----------------------+-----+
+	 *    3   |  0  |  0  |  0  |      TB <4:0>               |
+	 *   -----+-----+-----+-----+-----------------------------+
+	 *    4   |  0  |  0  |  0  |      WB <4:0>               |
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    5   |  1  | BD  |  1  | B-  |  1  | B+  |  1  |XXXXX|
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 *    6   | BO  | BR  | BB  | BG  | BY  |  1  |XXXXX|XXXXX|
+	 *   -----+-----+-----+-----+-----+-----+-----+-----+-----+
+	 */
+
+	sx = ext[0] & 0x3f;
+	sy = ext[1] & 0x3f;
+	tb = ext[2] & 0x1f;
+	wb = ext[3] & 0x1f;
+	bd = !(ext[4] & 0x40);
+	bm = !(ext[4] & 0x10);
+	bp = !(ext[4] & 0x04);
+	bo = !(ext[5] & 0x80);
+	br = !(ext[5] & 0x40);
+	bb = !(ext[5] & 0x20);
+	bg = !(ext[5] & 0x10);
+	by = !(ext[5] & 0x08);
+	bu = !(ext[5] & 0x01);
+
+	input_report_abs(wdata->extension.input, ABS_X, sx - 0x20);
+	input_report_abs(wdata->extension.input, ABS_Y, sy - 0x20);
+	input_report_abs(wdata->extension.input, ABS_FRET_BOARD, tb);
+	input_report_abs(wdata->extension.input, ABS_WHAMMY_BAR, wb - 0x10);
+
+	input_report_key(wdata->extension.input, BTN_MODE, bm);
+	input_report_key(wdata->extension.input, BTN_START, bp);
+	input_report_key(wdata->extension.input, BTN_STRUM_BAR_UP, bu);
+	input_report_key(wdata->extension.input, BTN_STRUM_BAR_DOWN, bd);
+	input_report_key(wdata->extension.input, BTN_FRET_FAR_UP, bg);
+	input_report_key(wdata->extension.input, BTN_FRET_UP, br);
+	input_report_key(wdata->extension.input, BTN_FRET_MID, by);
+	input_report_key(wdata->extension.input, BTN_FRET_LOW, bb);
+	input_report_key(wdata->extension.input, BTN_FRET_FAR_LOW, bo);
+
+	input_sync(wdata->extension.input);
+}
+
+static int wiimod_guitar_open(struct input_dev *dev)
+{
+	struct wiimote_data *wdata = input_get_drvdata(dev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&wdata->state.lock, flags);
+	wdata->state.flags |= WIIPROTO_FLAG_EXT_USED;
+	wiiproto_req_drm(wdata, WIIPROTO_REQ_NULL);
+	spin_unlock_irqrestore(&wdata->state.lock, flags);
+
+	return 0;
+}
+
+static void wiimod_guitar_close(struct input_dev *dev)
+{
+	struct wiimote_data *wdata = input_get_drvdata(dev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&wdata->state.lock, flags);
+	wdata->state.flags &= ~WIIPROTO_FLAG_EXT_USED;
+	wiiproto_req_drm(wdata, WIIPROTO_REQ_NULL);
+	spin_unlock_irqrestore(&wdata->state.lock, flags);
+}
+
+static int wiimod_guitar_probe(const struct wiimod_ops *ops,
+			       struct wiimote_data *wdata)
+{
+	int ret;
+
+	wdata->extension.input = input_allocate_device();
+	if (!wdata->extension.input)
+		return -ENOMEM;
+
+	input_set_drvdata(wdata->extension.input, wdata);
+	wdata->extension.input->open = wiimod_guitar_open;
+	wdata->extension.input->close = wiimod_guitar_close;
+	wdata->extension.input->dev.parent = &wdata->hdev->dev;
+	wdata->extension.input->id.bustype = wdata->hdev->bus;
+	wdata->extension.input->id.vendor = wdata->hdev->vendor;
+	wdata->extension.input->id.product = wdata->hdev->product;
+	wdata->extension.input->id.version = wdata->hdev->version;
+	wdata->extension.input->name = WIIMOTE_NAME " Guitar";
+
+	set_bit(EV_KEY, wdata->extension.input->evbit);
+	set_bit(BTN_MODE, wdata->extension.input->keybit);
+	set_bit(BTN_START, wdata->extension.input->keybit);
+	set_bit(BTN_FRET_FAR_UP, wdata->extension.input->keybit);
+	set_bit(BTN_FRET_UP, wdata->extension.input->keybit);
+	set_bit(BTN_FRET_MID, wdata->extension.input->keybit);
+	set_bit(BTN_FRET_LOW, wdata->extension.input->keybit);
+	set_bit(BTN_FRET_FAR_LOW, wdata->extension.input->keybit);
+	set_bit(BTN_STRUM_BAR_UP, wdata->extension.input->keybit);
+	set_bit(BTN_STRUM_BAR_DOWN, wdata->extension.input->keybit);
+
+	set_bit(EV_ABS, wdata->extension.input->evbit);
+	set_bit(ABS_X, wdata->extension.input->absbit);
+	set_bit(ABS_Y, wdata->extension.input->absbit);
+	set_bit(ABS_FRET_BOARD, wdata->extension.input->absbit);
+	set_bit(ABS_WHAMMY_BAR, wdata->extension.input->absbit);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_X, -32, 31, 1, 1);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_Y, -32, 31, 1, 1);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_FRET_BOARD, 0, 0x1f, 1, 1);
+	input_set_abs_params(wdata->extension.input,
+			     ABS_WHAMMY_BAR, 0, 0x0f, 1, 1);
+
+	ret = input_register_device(wdata->extension.input);
+	if (ret)
+		goto err_free;
+
+	return 0;
+
+err_free:
+	input_free_device(wdata->extension.input);
+	wdata->extension.input = NULL;
+	return ret;
+}
+
+static void wiimod_guitar_remove(const struct wiimod_ops *ops,
+				 struct wiimote_data *wdata)
+{
+	if (!wdata->extension.input)
+		return;
+
+	input_unregister_device(wdata->extension.input);
+	wdata->extension.input = NULL;
+}
+
+static const struct wiimod_ops wiimod_guitar = {
+	.flags = 0,
+	.arg = 0,
+	.probe = wiimod_guitar_probe,
+	.remove = wiimod_guitar_remove,
+	.in_ext = wiimod_guitar_in_ext,
+};
+
+/*
  * Builtin Motion Plus
  * This module simply sets the WIIPROTO_FLAG_BUILTIN_MP protocol flag which
  * disables polling for Motion-Plus. This should be set only for devices which
@@ -2301,4 +2474,5 @@ const struct wiimod_ops *wiimod_ext_table[WIIMOTE_EXT_NUM] = {
 	[WIIMOTE_EXT_BALANCE_BOARD] = &wiimod_bboard,
 	[WIIMOTE_EXT_PRO_CONTROLLER] = &wiimod_pro,
 	[WIIMOTE_EXT_GUITAR_HERO_DRUMS] = &wiimod_drums,
+	[WIIMOTE_EXT_GUITAR_HERO_GUITAR] = &wiimod_guitar,
 };
diff --git a/drivers/hid/hid-wiimote.h b/drivers/hid/hid-wiimote.h
index 6e18b55..379cdfb 100644
--- a/drivers/hid/hid-wiimote.h
+++ b/drivers/hid/hid-wiimote.h
@@ -89,6 +89,7 @@ enum wiimote_exttype {
 	WIIMOTE_EXT_BALANCE_BOARD,
 	WIIMOTE_EXT_PRO_CONTROLLER,
 	WIIMOTE_EXT_GUITAR_HERO_DRUMS,
+	WIIMOTE_EXT_GUITAR_HERO_GUITAR,
 	WIIMOTE_EXT_NUM,
 };
 
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars
  2013-08-26 17:14 ` [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars David Herrmann
@ 2013-09-02 11:41   ` Jiri Kosina
  2013-09-03 18:05     ` Dmitry Torokhov
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Kosina @ 2013-09-02 11:41 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input, Nicolas Adenis-Lamarre, Dmitry Torokhov

On Mon, 26 Aug 2013, David Herrmann wrote:

> There are a bunch of guitar and drums devices out there that all report
> similar data. To avoid reporting this as BTN_MISC or ABS_MISC, we
> allocate some proper namespace for them. Note that most of these devices
> are toys and we cannot report any sophisticated physics via this API.
> 
> I did some google-images research and tried to provide definitions that
> work with all common devices. That's why I went with 4 toms, 4 cymbals,
> one bass, one hi-hat. I haven't seen other drums and I doubt that we need
> any additions to that. Anyway, the naming-scheme is intentionally done in
> an extensible way.
> 
> For guitars, we support 5 frets (normally aligned vertically, compared to
> the real horizontal layouts), a single strum-bar with up/down directions,
> an optional fret-board and a whammy-bar.
> 
> Most of the devices provide pressure values so I went with ABS_* bits. If
> we ever support devices which only provide digital input, we have to
> decide whether to emulate pressure data or add additional BTN_* bits.
> 
> If someone is not familiar with these devices, here are two pictures which
> provide almost all introduced interfaces (or try the given keywords
> with a google-image search):
>   Guitar: ("guitar hero world tour guitar")
>     http://images1.wikia.nocookie.net/__cb20120911023442/applezone/es/images/f/f9/Wii_Guitar.jpg
>   Drums: ("guitar hero drums")
>     http://oyster.ignimgs.com/franchises/images/03/55/35526_band-hero-drum-set-hands-on-20090929040735768.jpg
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Hi,

I have reviewed and like the implementation of 2/3 and 3/3, but I of 
course would like to have Ack from Dmitry for this, so that I could take 
it through my tree together with the rest of the patchset.

Dmitry, pretty please?

Thanks.

> ---
>  include/linux/mod_devicetable.h |  2 +-
>  include/uapi/linux/input.h      | 25 +++++++++++++++++++++++--
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 45e9214..329aa30 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -277,7 +277,7 @@ struct pcmcia_device_id {
>  #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING	0x71
>  #define INPUT_DEVICE_ID_KEY_MAX		0x2ff
>  #define INPUT_DEVICE_ID_REL_MAX		0x0f
> -#define INPUT_DEVICE_ID_ABS_MAX		0x3f
> +#define INPUT_DEVICE_ID_ABS_MAX		0x4f
>  #define INPUT_DEVICE_ID_MSC_MAX		0x07
>  #define INPUT_DEVICE_ID_LED_MAX		0x0f
>  #define INPUT_DEVICE_ID_SND_MAX		0x07
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index d584047..76457ee 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -716,6 +716,14 @@ struct input_keymap_entry {
>  #define BTN_DPAD_LEFT		0x222
>  #define BTN_DPAD_RIGHT		0x223
>  
> +#define BTN_FRET_FAR_UP		0x224
> +#define BTN_FRET_UP		0x225
> +#define BTN_FRET_MID		0x226
> +#define BTN_FRET_LOW		0x227
> +#define BTN_FRET_FAR_LOW	0x228
> +#define BTN_STRUM_BAR_UP	0x229
> +#define BTN_STRUM_BAR_DOWN	0x22a
> +
>  #define BTN_TRIGGER_HAPPY		0x2c0
>  #define BTN_TRIGGER_HAPPY1		0x2c0
>  #define BTN_TRIGGER_HAPPY2		0x2c1
> @@ -829,8 +837,21 @@ struct input_keymap_entry {
>  #define ABS_MT_TOOL_X		0x3c	/* Center X tool position */
>  #define ABS_MT_TOOL_Y		0x3d	/* Center Y tool position */
>  
> -
> -#define ABS_MAX			0x3f
> +/* Drums and guitars (mostly toys) */
> +#define ABS_TOM_FAR_LEFT	0x40
> +#define ABS_TOM_LEFT		0x41
> +#define ABS_TOM_RIGHT		0x42
> +#define ABS_TOM_FAR_RIGHT	0x43
> +#define ABS_CYMBAL_FAR_LEFT	0x44
> +#define ABS_CYMBAL_LEFT		0x45
> +#define ABS_CYMBAL_RIGHT	0x46
> +#define ABS_CYMBAL_FAR_RIGHT	0x47
> +#define ABS_BASS		0x48
> +#define ABS_HI_HAT		0x49
> +#define ABS_FRET_BOARD		0x4a	/* Guitar fret board, vertical pos */
> +#define ABS_WHAMMY_BAR		0x4b	/* Guitar whammy bar (or vibrato) */
> +
> +#define ABS_MAX			0x4f
>  #define ABS_CNT			(ABS_MAX+1)
>  
>  /*
> -- 
> 1.8.4
> 

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars
  2013-09-02 11:41   ` Jiri Kosina
@ 2013-09-03 18:05     ` Dmitry Torokhov
  2013-09-03 21:14       ` David Herrmann
  0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Torokhov @ 2013-09-03 18:05 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: David Herrmann, linux-input, Nicolas Adenis-Lamarre

On Mon, Sep 02, 2013 at 01:41:57PM +0200, Jiri Kosina wrote:
> On Mon, 26 Aug 2013, David Herrmann wrote:
> 
> > There are a bunch of guitar and drums devices out there that all report
> > similar data. To avoid reporting this as BTN_MISC or ABS_MISC, we
> > allocate some proper namespace for them. Note that most of these devices
> > are toys and we cannot report any sophisticated physics via this API.
> > 
> > I did some google-images research and tried to provide definitions that
> > work with all common devices. That's why I went with 4 toms, 4 cymbals,
> > one bass, one hi-hat. I haven't seen other drums and I doubt that we need
> > any additions to that. Anyway, the naming-scheme is intentionally done in
> > an extensible way.
> > 
> > For guitars, we support 5 frets (normally aligned vertically, compared to
> > the real horizontal layouts), a single strum-bar with up/down directions,
> > an optional fret-board and a whammy-bar.
> > 
> > Most of the devices provide pressure values so I went with ABS_* bits. If
> > we ever support devices which only provide digital input, we have to
> > decide whether to emulate pressure data or add additional BTN_* bits.
> > 
> > If someone is not familiar with these devices, here are two pictures which
> > provide almost all introduced interfaces (or try the given keywords
> > with a google-image search):
> >   Guitar: ("guitar hero world tour guitar")
> >     http://images1.wikia.nocookie.net/__cb20120911023442/applezone/es/images/f/f9/Wii_Guitar.jpg
> >   Drums: ("guitar hero drums")
> >     http://oyster.ignimgs.com/franchises/images/03/55/35526_band-hero-drum-set-hands-on-20090929040735768.jpg
> > 
> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> 
> Hi,
> 
> I have reviewed and like the implementation of 2/3 and 3/3, but I of 
> course would like to have Ack from Dmitry for this, so that I could take 
> it through my tree together with the rest of the patchset.
> 
> Dmitry, pretty please?

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

We might want to see how much memory is taken now by ABS structures and
think if we could reduce it somehow...

Thanks.

 
Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars
  2013-09-03 18:05     ` Dmitry Torokhov
@ 2013-09-03 21:14       ` David Herrmann
  0 siblings, 0 replies; 8+ messages in thread
From: David Herrmann @ 2013-09-03 21:14 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Jiri Kosina, open list:HID CORE LAYER

Hi

On Tue, Sep 3, 2013 at 8:05 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Sep 02, 2013 at 01:41:57PM +0200, Jiri Kosina wrote:
>> On Mon, 26 Aug 2013, David Herrmann wrote:
>>
>> > There are a bunch of guitar and drums devices out there that all report
>> > similar data. To avoid reporting this as BTN_MISC or ABS_MISC, we
>> > allocate some proper namespace for them. Note that most of these devices
>> > are toys and we cannot report any sophisticated physics via this API.
>> >
>> > I did some google-images research and tried to provide definitions that
>> > work with all common devices. That's why I went with 4 toms, 4 cymbals,
>> > one bass, one hi-hat. I haven't seen other drums and I doubt that we need
>> > any additions to that. Anyway, the naming-scheme is intentionally done in
>> > an extensible way.
>> >
>> > For guitars, we support 5 frets (normally aligned vertically, compared to
>> > the real horizontal layouts), a single strum-bar with up/down directions,
>> > an optional fret-board and a whammy-bar.
>> >
>> > Most of the devices provide pressure values so I went with ABS_* bits. If
>> > we ever support devices which only provide digital input, we have to
>> > decide whether to emulate pressure data or add additional BTN_* bits.
>> >
>> > If someone is not familiar with these devices, here are two pictures which
>> > provide almost all introduced interfaces (or try the given keywords
>> > with a google-image search):
>> >   Guitar: ("guitar hero world tour guitar")
>> >     http://images1.wikia.nocookie.net/__cb20120911023442/applezone/es/images/f/f9/Wii_Guitar.jpg
>> >   Drums: ("guitar hero drums")
>> >     http://oyster.ignimgs.com/franchises/images/03/55/35526_band-hero-drum-set-hands-on-20090929040735768.jpg
>> >
>> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>>
>> Hi,
>>
>> I have reviewed and like the implementation of 2/3 and 3/3, but I of
>> course would like to have Ack from Dmitry for this, so that I could take
>> it through my tree together with the rest of the patchset.
>>
>> Dmitry, pretty please?
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> We might want to see how much memory is taken now by ABS structures and
> think if we could reduce it somehow...

Thanks Dmitry! And I thought about the "absinfo"-size, too. Some rough numbers:

sizeof(struct input_absinfo) = 24 bytes
ABS_CNT * sizeof(struct input_absinfo) = 0x50 * 24 bytes = 1920 bytes

sizeof(struct input_dev) = 1824 bytes
name, phys, uniq + misc ~= 100 bytes
dev->vals ~= 10 * sizeof(struct input_value) = 10 * 8 bytes = 80 bytes

So we get 1824 + 100 + 80 = 2008 bytes for an average trivial input device.
The abs-values add 1920 bytes to that. So I don't think it's that bad.

Anyway, some trivial ideas off the top of my head:

1)
Changing "absinfo" to double-indirection:
    struct input_absinfo *absinfo[ABS_CNT];
    struct input_absinfo *real_absinfo;
We allocate "real_absinfo" as array for as many ABS entries as we need
and set the pointers in "absinfo" accordingly (setting everything else
to NULL).
This, however, still adds ABS_CNT * 8 = 0x50 * 8 = 640 bytes only for
the pointer array. So not really much better. And we get the
additional TLB lookup for the double-indirection. I don't know whether
it matters for the quite expensive input-handling, though.

2)
Using idr. But considering that most input-devices only have very few
(<10?) ABS bits set, it probably doesn't scale.

3)
An rbtree for all absinfo objects. Only adds 24 bytes per absinfo and
lookup is only needed once in input_handle_abs_event(). It takes the
least space of all (except for linear search) and it's still
reasonably fast for all setups.
All in all, if we assume <10 input devices on a machine, we would save
around 20kb with 3). I am not sure what impact it has on performance.
But looking at the deep call-chains in input-interrupts, I doubt it
would be noticeable. But that's only a rough guess..

4)
We let drivers manage absinfo objects and pass it together with
input_event() (for ABS only). Would probably be the nicest solution
but require huge codebase changes.

But maybe someone else has some way better ideas.. It's getting late here.

Cheers
David

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] Input/HID: Guitar/Drums support
  2013-08-26 17:14 [PATCH 0/3] Input/HID: Guitar/Drums support David Herrmann
                   ` (2 preceding siblings ...)
  2013-08-26 17:14 ` [PATCH 3/3] HID: wiimote: add support for Guitar-Hero guitars David Herrmann
@ 2013-09-04  8:46 ` Jiri Kosina
  3 siblings, 0 replies; 8+ messages in thread
From: Jiri Kosina @ 2013-09-04  8:46 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input, Nicolas Adenis-Lamarre, Dmitry Torokhov

On Mon, 26 Aug 2013, David Herrmann wrote:

> This series adds wiimote extension support for guitars and drums. 

Applied, thanks!

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-09-04  8:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-26 17:14 [PATCH 0/3] Input/HID: Guitar/Drums support David Herrmann
2013-08-26 17:14 ` [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars David Herrmann
2013-09-02 11:41   ` Jiri Kosina
2013-09-03 18:05     ` Dmitry Torokhov
2013-09-03 21:14       ` David Herrmann
2013-08-26 17:14 ` [PATCH 2/3] HID: wiimote: add support for Guitar-Hero drums David Herrmann
2013-08-26 17:14 ` [PATCH 3/3] HID: wiimote: add support for Guitar-Hero guitars David Herrmann
2013-09-04  8:46 ` [PATCH 0/3] Input/HID: Guitar/Drums support Jiri Kosina

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.