diff mbox

[07/16] HID: wiimote: Parse accelerometer data

Message ID 1311869316-17128-8-git-send-email-dh.herrmann@googlemail.com (mailing list archive)
State New, archived
Delegated to: Jiri Kosina
Headers show

Commit Message

David Herrmann July 28, 2011, 4:08 p.m. UTC 
Add parser functions for accelerometer data reported by the wiimote. The data is
almost always reported in the same format, so we can use a single handler.
However, an own handler function is created for each DRM-mode because when IR
and extension support is added, each of them is parsed differently.

Also set the appropriate DRM including accelerometer data on DRM requests to
actually retrieve the accelerometer data.

Data is reported to userspace as ABS_X/Y/Z values. The values are between -500
and 500 and 0 means no acceleration. See also userspace xwiimote library for
data parsing.

Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
 drivers/hid/hid-wiimote.c |  102 ++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 101 insertions(+), 1 deletions(-)

Comments

Oliver Neukum July 28, 2011, 6:12 p.m. UTC | #1 
Am Donnerstag, 28. Juli 2011, 18:08:27 schrieb David Herrmann:
> +static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
> +{
> +       __u16 x, y, z;
> +
> +       if (!(wdata->state.flags & WIIPROTO_FLAG_ACCEL))
> +               return;
> +
> +       /*
> +        * payload is: BB BB XX YY ZZ
> +        * Buttons data contains LSBs
> +        */
> +
> +       x = payload[2] << 2;
> +       y = payload[3] << 2;
> +       z = payload[4] << 2;
> +
> +       x |= (payload[0] >> 5) & 0x3;
> +       y |= (payload[1] >> 4) & 0x2;
> +       z |= (payload[1] >> 5) & 0x2;

Could you make the comments a bit clearer. Those last lines are impossible
to understand.

	Regards
		Oliver
David Herrmann July 28, 2011, 6:51 p.m. UTC | #2 
On Thu, Jul 28, 2011 at 8:12 PM, Oliver Neukum <oneukum@suse.de> wrote:
> Am Donnerstag, 28. Juli 2011, 18:08:27 schrieb David Herrmann:
>> +static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
>> +{
>> +       __u16 x, y, z;
>> +
>> +       if (!(wdata->state.flags & WIIPROTO_FLAG_ACCEL))
>> +               return;
>> +
>> +       /*
>> +        * payload is: BB BB XX YY ZZ
>> +        * Buttons data contains LSBs
>> +        */
>> +
>> +       x = payload[2] << 2;
>> +       y = payload[3] << 2;
>> +       z = payload[4] << 2;
>> +
>> +       x |= (payload[0] >> 5) & 0x3;
>> +       y |= (payload[1] >> 4) & 0x2;
>> +       z |= (payload[1] >> 5) & 0x2;
>
> Could you make the comments a bit clearer. Those last lines are impossible
> to understand.

The LSBs are encoded in the "BB BB" data and I am extracting them. I
have documented the whole protocol in a separated document but if it
is common practice to add those comments to the code, I will add it in
the next version.

X, Y and Z data of accelerometer have 10 bits of precision. The data
is reported as 5 bytes:
BB BB XX YY ZZ
The first two bytes also contain the button data but there are 5
additional bits in the BB BB data that contain the LSBs of the
accelerometer.
XX YY ZZ are the upper 8 bits of the three accelerometer values.

BB BB have 8 bits. Bit 6 and 7 of first byte are LSBs of X value.
Bit 6 of second byte is LSB of Y value. Bit 7 of second byte is LSB of
Z value. The 10th bit of Y and Z are no available and set to 0.
As a table:

1st byte BB
Bit: 1 2 3 4 5 6 7 8
     ---------------
     B B B B B X X B

2nd byte BB
Bit: 1 2 3 4 5 6 7 8
     ---------------
     B B B B B Y Z B

B is button data and ignored here. handler_keys() takes care of them.

>        Regards
>                Oliver

Thanks for your review.
Regards
David
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Kosina Aug. 10, 2011, 11:55 a.m. UTC | #3 
On Thu, 28 Jul 2011, David Herrmann wrote:

> >> +static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
> >> +{
> >> +       __u16 x, y, z;
> >> +
> >> +       if (!(wdata->state.flags & WIIPROTO_FLAG_ACCEL))
> >> +               return;
> >> +
> >> +       /*
> >> +        * payload is: BB BB XX YY ZZ
> >> +        * Buttons data contains LSBs
> >> +        */
> >> +
> >> +       x = payload[2] << 2;
> >> +       y = payload[3] << 2;
> >> +       z = payload[4] << 2;
> >> +
> >> +       x |= (payload[0] >> 5) & 0x3;
> >> +       y |= (payload[1] >> 4) & 0x2;
> >> +       z |= (payload[1] >> 5) & 0x2;
> >
> > Could you make the comments a bit clearer. Those last lines are impossible
> > to understand.
> 
> The LSBs are encoded in the "BB BB" data and I am extracting them. I
> have documented the whole protocol in a separated document but if it
> is common practice to add those comments to the code, I will add it in
> the next version.

In some cases, the usual way is to put a brief description of the 
protocol/data structures mandated by hardware in the comments at the very 
beginning of the driver file.

The description below seems like a perfect fit for such purpose. If you 
have much more elaborate description, it should probably go into 
Documentation/.

Thanks,
diff mbox

Patch

diff --git a/drivers/hid/hid-wiimote.c b/drivers/hid/hid-wiimote.c
index 13c27b7..6947c2d 100644
--- a/drivers/hid/hid-wiimote.c
+++ b/drivers/hid/hid-wiimote.c
@@ -30,6 +30,7 @@  struct wiimote_buf {
 struct wiimote_state {
 	spinlock_t lock;
 	__u8 flags;
+	__u8 accel_split[2];
 };
 
 struct wiimote_data {
@@ -63,6 +64,12 @@  enum wiiproto_reqs {
 	WIIPROTO_REQ_STATUS = 0x20,
 	WIIPROTO_REQ_RETURN = 0x22,
 	WIIPROTO_REQ_DRM_K = 0x30,
+	WIIPROTO_REQ_DRM_KA = 0x31,
+	WIIPROTO_REQ_DRM_KAI = 0x33,
+	WIIPROTO_REQ_DRM_KAE = 0x35,
+	WIIPROTO_REQ_DRM_KAIE = 0x37,
+	WIIPROTO_REQ_DRM_SKAI1 = 0x3e,
+	WIIPROTO_REQ_DRM_SKAI2 = 0x3f,
 };
 
 enum wiiproto_keys {
@@ -240,7 +247,10 @@  static void wiiproto_req_leds(struct wiimote_data *wdata, int leds)
  */
 static __u8 select_drm(struct wiimote_data *wdata)
 {
-	return WIIPROTO_REQ_DRM_K;
+	if (wdata->state.flags & WIIPROTO_FLAG_ACCEL)
+		return WIIPROTO_REQ_DRM_KA;
+	else
+		return WIIPROTO_REQ_DRM_K;
 }
 
 static void wiiproto_req_drm(struct wiimote_data *wdata, __u8 drm)
@@ -434,6 +444,32 @@  static void handler_keys(struct wiimote_data *wdata, const __u8 *payload)
 							!!(payload[1] & 0x80));
 }
 
+static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
+{
+	__u16 x, y, z;
+
+	if (!(wdata->state.flags & WIIPROTO_FLAG_ACCEL))
+		return;
+
+	/*
+	 * payload is: BB BB XX YY ZZ
+	 * Buttons data contains LSBs
+	 */
+
+	x = payload[2] << 2;
+	y = payload[3] << 2;
+	z = payload[4] << 2;
+
+	x |= (payload[0] >> 5) & 0x3;
+	y |= (payload[1] >> 4) & 0x2;
+	z |= (payload[1] >> 5) & 0x2;
+
+	input_event(wdata->input, EV_ABS, ABS_X, x - 0x200);
+	input_event(wdata->input, EV_ABS, ABS_Y, y - 0x200);
+	input_event(wdata->input, EV_ABS, ABS_Z, z - 0x200);
+}
+
+
 static void handler_status(struct wiimote_data *wdata, const __u8 *payload)
 {
 	handler_keys(wdata, payload);
@@ -454,6 +490,56 @@  static void handler_return(struct wiimote_data *wdata, const __u8 *payload)
 									cmd);
 }
 
+static void handler_drm_KA(struct wiimote_data *wdata, const __u8 *payload)
+{
+	handler_keys(wdata, payload);
+	handler_accel(wdata, payload);
+}
+
+static void handler_drm_KAI(struct wiimote_data *wdata, const __u8 *payload)
+{
+	handler_keys(wdata, payload);
+	handler_accel(wdata, payload);
+}
+
+static void handler_drm_KAE(struct wiimote_data *wdata, const __u8 *payload)
+{
+	handler_keys(wdata, payload);
+	handler_accel(wdata, payload);
+}
+
+static void handler_drm_KAIE(struct wiimote_data *wdata, const __u8 *payload)
+{
+	handler_keys(wdata, payload);
+	handler_accel(wdata, payload);
+}
+
+static void handler_drm_SKAI1(struct wiimote_data *wdata, const __u8 *payload)
+{
+	handler_keys(wdata, payload);
+
+	wdata->state.accel_split[0] = payload[2];
+	wdata->state.accel_split[1] = (payload[0] >> 1) & (0x10 | 0x20);
+	wdata->state.accel_split[1] |= (payload[1] << 1) & (0x40 | 0x80);
+}
+
+static void handler_drm_SKAI2(struct wiimote_data *wdata, const __u8 *payload)
+{
+	__u8 buf[5];
+
+	handler_keys(wdata, payload);
+
+	wdata->state.accel_split[1] |= (payload[0] >> 5) & (0x01 | 0x02);
+	wdata->state.accel_split[1] |= (payload[1] >> 3) & (0x04 | 0x08);
+
+	buf[0] = 0;
+	buf[1] = 0;
+	buf[2] = wdata->state.accel_split[0];
+	buf[3] = payload[2];
+	buf[4] = wdata->state.accel_split[1];
+	handler_accel(wdata, buf);
+}
+
 struct wiiproto_handler {
 	__u8 id;
 	size_t size;
@@ -464,6 +550,12 @@  static struct wiiproto_handler handlers[] = {
 	{ .id = WIIPROTO_REQ_STATUS, .size = 6, .func = handler_status },
 	{ .id = WIIPROTO_REQ_RETURN, .size = 4, .func = handler_return },
 	{ .id = WIIPROTO_REQ_DRM_K, .size = 2, .func = handler_keys },
+	{ .id = WIIPROTO_REQ_DRM_KA, .size = 5, .func = handler_drm_KA },
+	{ .id = WIIPROTO_REQ_DRM_KAI, .size = 17, .func = handler_drm_KAI },
+	{ .id = WIIPROTO_REQ_DRM_KAE, .size = 21, .func = handler_drm_KAE },
+	{ .id = WIIPROTO_REQ_DRM_KAIE, .size = 21, .func = handler_drm_KAIE },
+	{ .id = WIIPROTO_REQ_DRM_SKAI1, .size = 21, .func = handler_drm_SKAI1 },
+	{ .id = WIIPROTO_REQ_DRM_SKAI2, .size = 21, .func = handler_drm_SKAI2 },
 	{ .id = 0 }
 };
 
@@ -528,6 +620,14 @@  static struct wiimote_data *wiimote_create(struct hid_device *hdev)
 	for (i = 0; i < WIIPROTO_KEY_COUNT; ++i)
 		set_bit(wiiproto_keymap[i], wdata->input->keybit);
 
+	set_bit(EV_ABS, wdata->input->evbit);
+	set_bit(ABS_X, wdata->input->absbit);
+	set_bit(ABS_Y, wdata->input->absbit);
+	set_bit(ABS_Z, wdata->input->absbit);
+	input_set_abs_params(wdata->input, ABS_X, -500, 500, 2, 4);
+	input_set_abs_params(wdata->input, ABS_Y, -500, 500, 2, 4);
+	input_set_abs_params(wdata->input, ABS_Z, -500, 500, 2, 4);
+
 	spin_lock_init(&wdata->qlock);
 	INIT_WORK(&wdata->worker, wiimote_worker);