[PATCH 0/10] drivers/media/rc/ati_remote.c tweaks

[PATCH 0/10] drivers/media/rc/ati_remote.c tweaks

[George Spelvin]

Here are a bunch of cleanups to the ati_remote driver that have been
sitting in my tree since 2011, and I'm hoping to push upstream.  They work
fine on my v1 ATI Remote Wonder.

Patch 4/10 is the nicest in terms of code deletion, but the
others shrink the driver, too.

There are some that I'm not sure if they're wanted:

6/10 is the most questionable code rearrangement.
Cleaner or messier?  Feedback welcome.

Patches 8 and 9 are just prettying up the keymap table.

Patch 10 fixes some of the sillier default key assignments.  I'm pretty
sure this would constitute a kernel regression and is not okay, but it's
another local change I made, and I might as well see what people think.
Feel free to forget about this one.

[PATCH 01/10] ati_remote: Check the checksum

[George Spelvin]

An input report is 4 bytes long, but there are only 12 bits
of actual payload.  The 4 bytes are:
data[0] = 0x14
data[1] = data[2] + data[3] + 0xd5 (a checksum byte)
data[2] = the raw scancode (plus toggle bit in msbit)
data[3] = channel << 4 (the low 4 bits must be zero)

Ignore reports with a bad checksum.

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/media/rc/ati_remote.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 2df7c55160..3ddd66a23d 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -507,8 +507,9 @@ static void ati_remote_input_report(struct urb *urb)
 	 */
 
 	/* Deal with strange looking inputs */
-	if ( (urb->actual_length != 4) || (data[0] != 0x14) ||
-		((data[3] & 0x0f) != 0x00) ) {
+	if ( urb->actual_length != 4 || data[0] != 0x14 ||
+	     data[1] != (unsigned char)(data[2] + data[3] + 0xD5) ||
+	     (data[3] & 0x0f) != 0x00) {
 		ati_remote_dump(&urb->dev->dev, data, urb->actual_length);
 		return;
 	}
@@ -524,9 +525,9 @@ static void ati_remote_input_report(struct urb *urb)
 	remote_num = (data[3] >> 4) & 0x0f;
 	if (channel_mask & (1 << (remote_num + 1))) {
 		dbginfo(&ati_remote->interface->dev,
-			"Masked input from channel 0x%02x: data %02x,%02x, "
+			"Masked input from channel 0x%02x: data %02x, "
 			"mask= 0x%02lx\n",
-			remote_num, data[1], data[2], channel_mask);
+			remote_num, data[2], channel_mask);
 		return;
 	}
 
-- 
1.9.2

[PATCH 02/10] ati_remote: Shrink ati_remote_tbl structure

[George Spelvin]

The variable types are simply larger than they need to be.
Shrink to signed and unsigned chars.

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/media/rc/ati_remote.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 3ddd66a23d..b92da56e9a 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -289,11 +289,11 @@ struct ati_remote {
 
 /* Translation table from hardware messages to input events. */
 static const struct {
-	short kind;
+	unsigned char kind;
 	unsigned char data;
-	int type;
-	unsigned int code;
-	int value;
+	unsigned char type;
+	unsigned short code;
+	signed char value;
 }  ati_remote_tbl[] = {
 	/* Directional control pad axes */
 	{KIND_ACCEL,   0x70, EV_REL, REL_X, -1},   /* left */
-- 
1.9.2

[PATCH 03/10] ati_remote: Delete superfluous input_sync()

[George Spelvin]

It's not necessary, and since both events happen "at the same time"
in response to a single input event, the input device framework prefers
not to have it there.

(It's not a big deal one way or the other, but deleting cruft
is generally a good thing.)

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/media/rc/ati_remote.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index b92da56e9a..933d614475 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -632,7 +632,6 @@ static void ati_remote_input_report(struct urb *urb)
 
 		input_event(dev, ati_remote_tbl[index].type,
 			ati_remote_tbl[index].code, 1);
-		input_sync(dev);
 		input_event(dev, ati_remote_tbl[index].type,
 			ati_remote_tbl[index].code, 0);
 		input_sync(dev);
-- 
1.9.2

[PATCH 04/10] ati_remote: Generalize KIND_ACCEL to accept diagonals

[George Spelvin]

Rather than having special code cases for diagonal mouse
movements, extend the general purpose code used for the
cardinal directions to handle arbitrary (x,y) deltas.

The deltas themselves are stored in translation table's "code"
field; this is also progress toward the goal of eliminating
the "value" element entirely.

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/media/rc/ati_remote.c | 71 ++++++++++++++-----------------------------
 1 file changed, 23 insertions(+), 48 deletions(-)

diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 933d614475..ba5c1bba53 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -281,11 +281,7 @@ struct ati_remote {
 #define KIND_END        0
 #define KIND_LITERAL    1   /* Simply pass to input system */
 #define KIND_FILTERED   2   /* Add artificial key-up events, drop keyrepeats */
-#define KIND_LU         3   /* Directional keypad diagonals - left up, */
-#define KIND_RU         4   /*   right up,  */
-#define KIND_LD         5   /*   left down, */
-#define KIND_RD         6   /*   right down */
-#define KIND_ACCEL      7   /* Directional keypad - left, right, up, down.*/
+#define KIND_ACCEL      3   /* Directional keypad - left, right, up, down.*/
 
 /* Translation table from hardware messages to input events. */
 static const struct {
@@ -295,16 +291,17 @@ static const struct {
 	unsigned short code;
 	signed char value;
 }  ati_remote_tbl[] = {
-	/* Directional control pad axes */
-	{KIND_ACCEL,   0x70, EV_REL, REL_X, -1},   /* left */
-	{KIND_ACCEL,   0x71, EV_REL, REL_X, 1},    /* right */
-	{KIND_ACCEL,   0x72, EV_REL, REL_Y, -1},   /* up */
-	{KIND_ACCEL,   0x73, EV_REL, REL_Y, 1},    /* down */
+	/* Directional control pad axes.  Code is xxyy */
+	{KIND_ACCEL,   0x70, EV_REL, 0xff00, 0},	/* left */
+	{KIND_ACCEL,   0x71, EV_REL, 0x0100, 0},	/* right */
+	{KIND_ACCEL,   0x72, EV_REL, 0x00ff, 0},	/* up */
+	{KIND_ACCEL,   0x73, EV_REL, 0x0001, 0},	/* down */
+
 	/* Directional control pad diagonals */
-	{KIND_LU,      0x74, EV_REL, 0, 0},        /* left up */
-	{KIND_RU,      0x75, EV_REL, 0, 0},        /* right up */
-	{KIND_LD,      0x77, EV_REL, 0, 0},        /* left down */
-	{KIND_RD,      0x76, EV_REL, 0, 0},        /* right down */
+	{KIND_ACCEL,   0x74, EV_REL, 0xffff, 0},	/* left up */
+	{KIND_ACCEL,   0x75, EV_REL, 0x01ff, 0},	/* right up */
+	{KIND_ACCEL,   0x77, EV_REL, 0xff01, 0},	/* left down */
+	{KIND_ACCEL,   0x76, EV_REL, 0x0101, 0},	/* right down */
 
 	/* "Mouse button" buttons */
 	{KIND_LITERAL, 0x78, EV_KEY, BTN_LEFT, 1}, /* left btn down */
@@ -493,7 +490,6 @@ static void ati_remote_input_report(struct urb *urb)
 	unsigned char *data= ati_remote->inbuf;
 	struct input_dev *dev = ati_remote->idev;
 	int index = -1;
-	int acc;
 	int remote_num;
 	unsigned char scancode;
 	u32 wheel_keycode = KEY_RESERVED;
@@ -573,10 +569,8 @@ static void ati_remote_input_report(struct urb *urb)
 		input_sync(dev);
 
 		ati_remote->old_jiffies = jiffies;
-		return;
-	}
 
-	if (index < 0 || ati_remote_tbl[index].kind == KIND_FILTERED) {
+	} else if (index < 0 || ati_remote_tbl[index].kind == KIND_FILTERED) {
 		unsigned long now = jiffies;
 
 		/* Filter duplicate events which happen "too close" together. */
@@ -636,46 +630,27 @@ static void ati_remote_input_report(struct urb *urb)
 			ati_remote_tbl[index].code, 0);
 		input_sync(dev);
 
-	} else {
+	} else if (ati_remote_tbl[index].kind == KIND_ACCEL) {
+		signed char dx = ati_remote_tbl[index].code >> 8;
+		signed char dy = ati_remote_tbl[index].code & 255;
 
 		/*
 		 * Other event kinds are from the directional control pad, and
 		 * have an acceleration factor applied to them.  Without this
 		 * acceleration, the control pad is mostly unusable.
 		 */
-		acc = ati_remote_compute_accel(ati_remote);
-
-		switch (ati_remote_tbl[index].kind) {
-		case KIND_ACCEL:
-			input_event(dev, ati_remote_tbl[index].type,
-				ati_remote_tbl[index].code,
-				ati_remote_tbl[index].value * acc);
-			break;
-		case KIND_LU:
-			input_report_rel(dev, REL_X, -acc);
-			input_report_rel(dev, REL_Y, -acc);
-			break;
-		case KIND_RU:
-			input_report_rel(dev, REL_X, acc);
-			input_report_rel(dev, REL_Y, -acc);
-			break;
-		case KIND_LD:
-			input_report_rel(dev, REL_X, -acc);
-			input_report_rel(dev, REL_Y, acc);
-			break;
-		case KIND_RD:
-			input_report_rel(dev, REL_X, acc);
-			input_report_rel(dev, REL_Y, acc);
-			break;
-		default:
-			dev_dbg(&ati_remote->interface->dev,
-				"ati_remote kind=%d\n",
-				ati_remote_tbl[index].kind);
-		}
+		int acc = ati_remote_compute_accel(ati_remote);
+		if (dx)
+			input_report_rel(dev, REL_X, dx * acc);
+		if (dy)
+			input_report_rel(dev, REL_Y, dy * acc);
 		input_sync(dev);
 
 		ati_remote->old_jiffies = jiffies;
 		ati_remote->old_data = data[2];
+	} else {
+		dev_dbg(&ati_remote->interface->dev, "ati_remote kind=%d\n",
+			ati_remote_tbl[index].kind);
 	}
 }
 
-- 
1.9.2

[PATCH 05/10] ati_remote: Shrink the ati_remote_tbl even more

[George Spelvin]

Get rid of the unnecessary "type" and "value" fields.

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/media/rc/ati_remote.c | 69 ++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index ba5c1bba53..8d9937fd5d 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -279,43 +279,42 @@ struct ati_remote {
 
 /* "Kinds" of messages sent from the hardware to the driver. */
 #define KIND_END        0
-#define KIND_LITERAL    1   /* Simply pass to input system */
+#define KIND_LITERAL    1   /* Simply pass to input system as EV_KEY */
 #define KIND_FILTERED   2   /* Add artificial key-up events, drop keyrepeats */
-#define KIND_ACCEL      3   /* Directional keypad - left, right, up, down.*/
+#define KIND_ACCEL      3   /* Translate to EV_REL mouse-move events */
 
 /* Translation table from hardware messages to input events. */
 static const struct {
 	unsigned char kind;
-	unsigned char data;
-	unsigned char type;
-	unsigned short code;
-	signed char value;
+	unsigned char data;	/* Raw key code from remote */
+	unsigned short code;	/* Input layer translation */
 }  ati_remote_tbl[] = {
 	/* Directional control pad axes.  Code is xxyy */
-	{KIND_ACCEL,   0x70, EV_REL, 0xff00, 0},	/* left */
-	{KIND_ACCEL,   0x71, EV_REL, 0x0100, 0},	/* right */
-	{KIND_ACCEL,   0x72, EV_REL, 0x00ff, 0},	/* up */
-	{KIND_ACCEL,   0x73, EV_REL, 0x0001, 0},	/* down */
+	{KIND_ACCEL,    0x70, 0xff00},	/* left */
+	{KIND_ACCEL,    0x71, 0x0100},	/* right */
+	{KIND_ACCEL,    0x72, 0x00ff},	/* up */
+	{KIND_ACCEL,    0x73, 0x0001},	/* down */
 
 	/* Directional control pad diagonals */
-	{KIND_ACCEL,   0x74, EV_REL, 0xffff, 0},	/* left up */
-	{KIND_ACCEL,   0x75, EV_REL, 0x01ff, 0},	/* right up */
-	{KIND_ACCEL,   0x77, EV_REL, 0xff01, 0},	/* left down */
-	{KIND_ACCEL,   0x76, EV_REL, 0x0101, 0},	/* right down */
-
-	/* "Mouse button" buttons */
-	{KIND_LITERAL, 0x78, EV_KEY, BTN_LEFT, 1}, /* left btn down */
-	{KIND_LITERAL, 0x79, EV_KEY, BTN_LEFT, 0}, /* left btn up */
-	{KIND_LITERAL, 0x7c, EV_KEY, BTN_RIGHT, 1},/* right btn down */
-	{KIND_LITERAL, 0x7d, EV_KEY, BTN_RIGHT, 0},/* right btn up */
+	{KIND_ACCEL,    0x74, 0xffff},	/* left up */
+	{KIND_ACCEL,    0x75, 0x01ff},	/* right up */
+	{KIND_ACCEL,    0x77, 0xff01},	/* left down */
+	{KIND_ACCEL,    0x76, 0x0101},	/* right down */
+
+	/* "Mouse button" buttons.  The code below uses the fact that the
+	 * lsbit of the raw code is a down/up indicator. */
+	{KIND_LITERAL,  0x78, BTN_LEFT}, /* left btn down */
+	{KIND_LITERAL,  0x79, BTN_LEFT}, /* left btn up */
+	{KIND_LITERAL,  0x7c, BTN_RIGHT},/* right btn down */
+	{KIND_LITERAL,  0x7d, BTN_RIGHT},/* right btn up */
 
 	/* Artificial "doubleclick" events are generated by the hardware.
 	 * They are mapped to the "side" and "extra" mouse buttons here. */
-	{KIND_FILTERED, 0x7a, EV_KEY, BTN_SIDE, 1}, /* left dblclick */
-	{KIND_FILTERED, 0x7e, EV_KEY, BTN_EXTRA, 1},/* right dblclick */
+	{KIND_FILTERED, 0x7a, BTN_SIDE}, /* left dblclick */
+	{KIND_FILTERED, 0x7e, BTN_EXTRA},/* right dblclick */
 
 	/* Non-mouse events are handled by rc-core */
-	{KIND_END, 0x00, EV_MAX + 1, 0, 0}
+	{KIND_END, 0x00, 0}
 };
 
 /*
@@ -563,9 +562,12 @@ static void ati_remote_input_report(struct urb *urb)
 	}
 
 	if (index >= 0 && ati_remote_tbl[index].kind == KIND_LITERAL) {
-		input_event(dev, ati_remote_tbl[index].type,
-			ati_remote_tbl[index].code,
-			ati_remote_tbl[index].value);
+		/*
+		 * The lsbit of the raw key code is a down/up flag.
+		 * Invert it to match the input layer's conventions.
+		 */
+		input_event(dev, EV_KEY, ati_remote_tbl[index].code,
+			!(data[2] & 1));
 		input_sync(dev);
 
 		ati_remote->old_jiffies = jiffies;
@@ -586,9 +588,9 @@ static void ati_remote_input_report(struct urb *urb)
 		ati_remote->old_data = data[2];
 		ati_remote->old_jiffies = now;
 
-		/* Ensure we skip at least the 4 first duplicate events (generated
-		 * by a single keypress), and continue skipping until repeat_delay
-		 * msecs have passed
+		/* Ensure we skip at least the 4 first duplicate events
+		 * (generated by a single keypress), and continue skipping
+		 * until repeat_delay msecs have passed.
 		 */
 		if (ati_remote->repeat_count > 0 &&
 		    (ati_remote->repeat_count < 5 ||
@@ -624,10 +626,8 @@ static void ati_remote_input_report(struct urb *urb)
 			return;
 		}
 
-		input_event(dev, ati_remote_tbl[index].type,
-			ati_remote_tbl[index].code, 1);
-		input_event(dev, ati_remote_tbl[index].type,
-			ati_remote_tbl[index].code, 0);
+		input_event(dev, EV_KEY, ati_remote_tbl[index].code, 1);
+		input_event(dev, EV_KEY, ati_remote_tbl[index].code, 0);
 		input_sync(dev);
 
 	} else if (ati_remote_tbl[index].kind == KIND_ACCEL) {
@@ -738,7 +738,8 @@ static void ati_remote_input_init(struct ati_remote *ati_remote)
 		BIT_MASK(BTN_RIGHT) | BIT_MASK(BTN_SIDE) | BIT_MASK(BTN_EXTRA);
 	idev->relbit[0] = BIT_MASK(REL_X) | BIT_MASK(REL_Y);
 	for (i = 0; ati_remote_tbl[i].kind != KIND_END; i++)
-		if (ati_remote_tbl[i].type == EV_KEY)
+		if (ati_remote_tbl[i].kind == KIND_LITERAL ||
+		    ati_remote_tbl[i].kind == KIND_FILTERED)
 			set_bit(ati_remote_tbl[i].code, idev->keybit);
 
 	input_set_drvdata(idev, ati_remote);
-- 
1.9.2

[PATCH 06/10] ati_remote: Merge some duplicate code

[George Spelvin]

The KIND_FILTERED assignment of old_jiffies can't be merged, because
it must precede repeat handling.

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/media/rc/ati_remote.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 8d9937fd5d..69d7912e03 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -568,7 +568,6 @@ static void ati_remote_input_report(struct urb *urb)
 		 */
 		input_event(dev, EV_KEY, ati_remote_tbl[index].code,
 			!(data[2] & 1));
-		input_sync(dev);
 
 		ati_remote->old_jiffies = jiffies;
 
@@ -585,7 +584,6 @@ static void ati_remote_input_report(struct urb *urb)
 			ati_remote->first_jiffies = now;
 		}
 
-		ati_remote->old_data = data[2];
 		ati_remote->old_jiffies = now;
 
 		/* Ensure we skip at least the 4 first duplicate events
@@ -598,7 +596,10 @@ static void ati_remote_input_report(struct urb *urb)
 				      msecs_to_jiffies(repeat_delay))))
 			return;
 
-		if (index < 0) {
+		if (index >= 0) {
+			input_event(dev, EV_KEY, ati_remote_tbl[index].code, 1);
+			input_event(dev, EV_KEY, ati_remote_tbl[index].code, 0);
+		} else {
 			/* Not a mouse event, hand it to rc-core. */
 			int count = 1;
 
@@ -623,13 +624,9 @@ static void ati_remote_input_report(struct urb *urb)
 						     data[2]);
 				rc_keyup(ati_remote->rdev);
 			}
-			return;
+			goto nosync;
 		}
 
-		input_event(dev, EV_KEY, ati_remote_tbl[index].code, 1);
-		input_event(dev, EV_KEY, ati_remote_tbl[index].code, 0);
-		input_sync(dev);
-
 	} else if (ati_remote_tbl[index].kind == KIND_ACCEL) {
 		signed char dx = ati_remote_tbl[index].code >> 8;
 		signed char dy = ati_remote_tbl[index].code & 255;
@@ -644,14 +641,16 @@ static void ati_remote_input_report(struct urb *urb)
 			input_report_rel(dev, REL_X, dx * acc);
 		if (dy)
 			input_report_rel(dev, REL_Y, dy * acc);
-		input_sync(dev);
-
 		ati_remote->old_jiffies = jiffies;
-		ati_remote->old_data = data[2];
+
 	} else {
 		dev_dbg(&ati_remote->interface->dev, "ati_remote kind=%d\n",
 			ati_remote_tbl[index].kind);
+		return;
 	}
+	input_sync(dev);
+nosync:
+	ati_remote->old_data = data[2];
 }
 
 /*
-- 
1.9.2

[PATCH 07/10] ati_remote: Use non-alomic __set_bit

[George Spelvin]

There's no reason to use a LOCK prefix here.

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/media/rc/ati_remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 69d7912e03..8536eef918 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -739,7 +739,7 @@ static void ati_remote_input_init(struct ati_remote *ati_remote)
 	for (i = 0; ati_remote_tbl[i].kind != KIND_END; i++)
 		if (ati_remote_tbl[i].kind == KIND_LITERAL ||
 		    ati_remote_tbl[i].kind == KIND_FILTERED)
-			set_bit(ati_remote_tbl[i].code, idev->keybit);
+			__set_bit(ati_remote_tbl[i].code, idev->keybit);
 
 	input_set_drvdata(idev, ati_remote);
 
-- 
1.9.2

[PATCH 08/10] ati_remote: Sort buttons in top-to-bottom order

[George Spelvin]

Since numerical order corresponds to top-left-to-bottom-right
order on the remote, this makes the table easier to read.

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/media/rc/keymaps/rc-ati-x10.c | 57 +++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/media/rc/keymaps/rc-ati-x10.c b/drivers/media/rc/keymaps/rc-ati-x10.c
index 81506440ed..4e2cbbafe9 100644
--- a/drivers/media/rc/keymaps/rc-ati-x10.c
+++ b/drivers/media/rc/keymaps/rc-ati-x10.c
@@ -27,6 +27,23 @@
 #include <media/rc-map.h>
 
 static struct rc_map_table ati_x10[] = {
+	/* keyboard - Above the cursor pad */
+	{ 0x00, KEY_A },
+	{ 0x01, KEY_B },
+	{ 0x02, KEY_POWER },      /* Power */
+
+	{ 0x03, KEY_TV },         /* TV */
+	{ 0x04, KEY_DVD },        /* DVD */
+	{ 0x05, KEY_WWW },        /* WEB */
+	{ 0x06, KEY_BOOKMARKS },  /* "book" */
+	{ 0x07, KEY_EDIT },       /* "hand" */
+	/* Below the cursor pad */
+	{ 0x09, KEY_VOLUMEDOWN }, /* VOL + */
+	{ 0x08, KEY_VOLUMEUP },   /* VOL - */
+	{ 0x0a, KEY_MUTE },       /* MUTE  */
+	{ 0x0b, KEY_CHANNELUP },  /* CH + */
+	{ 0x0c, KEY_CHANNELDOWN },/* CH - */
+
 	{ 0x0d, KEY_1 },
 	{ 0x0e, KEY_2 },
 	{ 0x0f, KEY_3 },
@@ -36,42 +53,36 @@ static struct rc_map_table ati_x10[] = {
 	{ 0x13, KEY_7 },
 	{ 0x14, KEY_8 },
 	{ 0x15, KEY_9 },
+	{ 0x16, KEY_MENU },       /* "menu" */
 	{ 0x17, KEY_0 },
-	{ 0x00, KEY_A },
-	{ 0x01, KEY_B },
+	{ 0x18, KEY_KPENTER },    /* "check" */
+
+	/* DVD navigation buttons */
 	{ 0x19, KEY_C },
+	{ 0x1a, KEY_UP },         /* up */
 	{ 0x1b, KEY_D },
-	{ 0x21, KEY_E },
-	{ 0x23, KEY_F },
 
-	{ 0x18, KEY_KPENTER },    /* "check" */
-	{ 0x16, KEY_MENU },       /* "menu" */
-	{ 0x02, KEY_POWER },      /* Power */
-	{ 0x03, KEY_TV },         /* TV */
-	{ 0x04, KEY_DVD },        /* DVD */
-	{ 0x05, KEY_WWW },        /* WEB */
-	{ 0x06, KEY_BOOKMARKS },  /* "book" */
-	{ 0x07, KEY_EDIT },       /* "hand" */
 	{ 0x1c, KEY_COFFEE },     /* "timer" */
-	{ 0x20, KEY_FRONT },      /* "max" */
 	{ 0x1d, KEY_LEFT },       /* left */
+	{ 0x1e, KEY_OK },         /* "OK" */
 	{ 0x1f, KEY_RIGHT },      /* right */
+	{ 0x20, KEY_FRONT },      /* "max" */
+
+	{ 0x21, KEY_E },
 	{ 0x22, KEY_DOWN },       /* down */
-	{ 0x1a, KEY_UP },         /* up */
-	{ 0x1e, KEY_OK },         /* "OK" */
-	{ 0x09, KEY_VOLUMEDOWN }, /* VOL + */
-	{ 0x08, KEY_VOLUMEUP },   /* VOL - */
-	{ 0x0a, KEY_MUTE },       /* MUTE  */
-	{ 0x0b, KEY_CHANNELUP },  /* CH + */
-	{ 0x0c, KEY_CHANNELDOWN },/* CH - */
-	{ 0x27, KEY_RECORD },     /* ( o) red */
-	{ 0x25, KEY_PLAY },       /* ( >) */
+	{ 0x23, KEY_F },
+	/* Play/stop/pause buttons */
 	{ 0x24, KEY_REWIND },     /* (<<) */
+	{ 0x25, KEY_PLAY },       /* ( >) */
 	{ 0x26, KEY_FORWARD },    /* (>>) */
+
+	{ 0x27, KEY_RECORD },     /* ( o) red */
 	{ 0x28, KEY_STOP },       /* ([]) */
 	{ 0x29, KEY_PAUSE },      /* ('') */
-	{ 0x2b, KEY_PREVIOUS },   /* (<-) */
+
+	/* Extra keys, not on the original ATI remote */
 	{ 0x2a, KEY_NEXT },       /* (>+) */
+	{ 0x2b, KEY_PREVIOUS },   /* (<-) */
 	{ 0x2d, KEY_INFO },       /* PLAYING */
 	{ 0x2e, KEY_HOME },       /* TOP */
 	{ 0x2f, KEY_END },        /* END */
-- 
1.9.2

[PATCH 09/10] ati_remote: Add comments to keycode table

[George Spelvin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2817 bytes --]

A more detailed description of what the buttons look like and
their intended function makes it easier for people to maintain
this code without access to the hardware.

Signed-off-by: George Spelvin <linux@horizon.com>
---
 drivers/media/rc/keymaps/rc-ati-x10.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/media/rc/keymaps/rc-ati-x10.c b/drivers/media/rc/keymaps/rc-ati-x10.c
index 4e2cbbafe9..df8968eb1f 100644
--- a/drivers/media/rc/keymaps/rc-ati-x10.c
+++ b/drivers/media/rc/keymaps/rc-ati-x10.c
@@ -26,6 +26,17 @@
 #include <linux/module.h>
 #include <media/rc-map.h>
 
+/*
+ * Intended usage comments below are from vendor-supplied
+ * Source: ATI REMOTE WONDER™ Installation Guide
+ * http://www2.ati.com/manuals/remctrl.pdf
+ *
+ * Scancodes were in strict left-right, top-bottom order on the
+ * original ATI Remote Wonder, but were moved on later models.
+ *
+ * Keys A-F are intended to be user-programmable.
+ */
+
 static struct rc_map_table ati_x10[] = {
 	/* keyboard - Above the cursor pad */
 	{ 0x00, KEY_A },
@@ -35,9 +46,11 @@ static struct rc_map_table ati_x10[] = {
 	{ 0x03, KEY_TV },         /* TV */
 	{ 0x04, KEY_DVD },        /* DVD */
 	{ 0x05, KEY_WWW },        /* WEB */
-	{ 0x06, KEY_BOOKMARKS },  /* "book" */
-	{ 0x07, KEY_EDIT },       /* "hand" */
-	/* Below the cursor pad */
+	{ 0x06, KEY_BOOKMARKS },  /* "book": Open Mdeia Library */
+	{ 0x07, KEY_EDIT },       /* "hand": Toggle left mouse button (grab) */
+
+	/* Mouse emulation pad goes here, handled by driver separately */
+
 	{ 0x09, KEY_VOLUMEDOWN }, /* VOL + */
 	{ 0x08, KEY_VOLUMEUP },   /* VOL - */
 	{ 0x0a, KEY_MUTE },       /* MUTE  */
@@ -53,9 +66,9 @@ static struct rc_map_table ati_x10[] = {
 	{ 0x13, KEY_7 },
 	{ 0x14, KEY_8 },
 	{ 0x15, KEY_9 },
-	{ 0x16, KEY_MENU },       /* "menu" */
+	{ 0x16, KEY_MENU },       /* "menu": DVD root menu */
 	{ 0x17, KEY_0 },
-	{ 0x18, KEY_KPENTER },    /* "check" */
+	{ 0x18, KEY_KPENTER },    /* "check": DVD setup menu */
 
 	/* DVD navigation buttons */
 	{ 0x19, KEY_C },
@@ -72,13 +85,13 @@ static struct rc_map_table ati_x10[] = {
 	{ 0x22, KEY_DOWN },       /* down */
 	{ 0x23, KEY_F },
 	/* Play/stop/pause buttons */
-	{ 0x24, KEY_REWIND },     /* (<<) */
-	{ 0x25, KEY_PLAY },       /* ( >) */
-	{ 0x26, KEY_FORWARD },    /* (>>) */
+	{ 0x24, KEY_REWIND },     /* (<<) Rewind */
+	{ 0x25, KEY_PLAY },       /* ( >) Play */
+	{ 0x26, KEY_FORWARD },    /* (>>) Fast forward */
 
 	{ 0x27, KEY_RECORD },     /* ( o) red */
-	{ 0x28, KEY_STOP },       /* ([]) */
-	{ 0x29, KEY_PAUSE },      /* ('') */
+	{ 0x28, KEY_STOP },       /* ([]) Stop */
+	{ 0x29, KEY_PAUSE },      /* ('') Pause */
 
 	/* Extra keys, not on the original ATI remote */
 	{ 0x2a, KEY_NEXT },       /* (>+) */
-- 
1.9.2

[PATCH 10/10] ati_remote: Better default keycodes

[George Spelvin]

This tries to make them more like other remotes, and/or
the button labels.

Notably, the (>>) button is made KEY_FASTFORWARD, which is the
correct opposite of (<<)'s KEY_REVERSE.  (It was KEY_FORWARD,
something else entirely.)

Likewise, KEY_STOP is the Sun keyboard "interrupt program" key;
the media key is KEY_STOPCD.

A restriction is that I try to avoid keycodes above 255, as the X11
client/server protocol is limited to 8-bit key codes.  If not for
this, I would have used the KEY_NUMERIC_x codes for the numbers.

Signed-off-by: George Spelvin <linux@horizon.com>
---
As mentioned earlier, this constitutes a user-visible kernel change and
thus possibly a regression, so it's probably a non-starter, but what the heck.

 drivers/media/rc/keymaps/rc-ati-x10.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/media/rc/keymaps/rc-ati-x10.c b/drivers/media/rc/keymaps/rc-ati-x10.c
index df8968eb1f..b924265b32 100644
--- a/drivers/media/rc/keymaps/rc-ati-x10.c
+++ b/drivers/media/rc/keymaps/rc-ati-x10.c
@@ -57,6 +57,11 @@ static struct rc_map_table ati_x10[] = {
 	{ 0x0b, KEY_CHANNELUP },  /* CH + */
 	{ 0x0c, KEY_CHANNELDOWN },/* CH - */
 
+	/*
+	 * We could use KEY_NUMERIC_x for these, but the X11 protocol
+	 * has problems with keycodes greater than 255, so avoid those high
+	 * keycodes in default maps.
+	 */
 	{ 0x0d, KEY_1 },
 	{ 0x0e, KEY_2 },
 	{ 0x0f, KEY_3 },
@@ -67,39 +72,44 @@ static struct rc_map_table ati_x10[] = {
 	{ 0x14, KEY_8 },
 	{ 0x15, KEY_9 },
 	{ 0x16, KEY_MENU },       /* "menu": DVD root menu */
+				  /* KEY_NUMERIC_STAR? */
 	{ 0x17, KEY_0 },
-	{ 0x18, KEY_KPENTER },    /* "check": DVD setup menu */
+	{ 0x18, KEY_SETUP },      /* "check": DVD setup menu */
+				  /* KEY_NUMERIC_POUND? */
 
 	/* DVD navigation buttons */
 	{ 0x19, KEY_C },
 	{ 0x1a, KEY_UP },         /* up */
 	{ 0x1b, KEY_D },
 
-	{ 0x1c, KEY_COFFEE },     /* "timer" */
+	{ 0x1c, KEY_PROPS },      /* "timer" Should be Data On Screen */
+				  /* Symbol is "circle nailed to box" */
 	{ 0x1d, KEY_LEFT },       /* left */
 	{ 0x1e, KEY_OK },         /* "OK" */
 	{ 0x1f, KEY_RIGHT },      /* right */
-	{ 0x20, KEY_FRONT },      /* "max" */
-
+	{ 0x20, KEY_SCREEN },     /* "max" (X11 warning: 0x177) */
+				  /* Should be AC View Toggle, but
+				     that's not in <input/input.h>.
+				     KEY_ZOOM (0x174)? */
 	{ 0x21, KEY_E },
 	{ 0x22, KEY_DOWN },       /* down */
 	{ 0x23, KEY_F },
 	/* Play/stop/pause buttons */
 	{ 0x24, KEY_REWIND },     /* (<<) Rewind */
-	{ 0x25, KEY_PLAY },       /* ( >) Play */
-	{ 0x26, KEY_FORWARD },    /* (>>) Fast forward */
+	{ 0x25, KEY_PLAY },       /* ( >) Play (KEY_PLAYCD?) */
+	{ 0x26, KEY_FASTFORWARD }, /* (>>) Fast forward */
 
 	{ 0x27, KEY_RECORD },     /* ( o) red */
-	{ 0x28, KEY_STOP },       /* ([]) Stop */
-	{ 0x29, KEY_PAUSE },      /* ('') Pause */
+	{ 0x28, KEY_STOPCD },     /* ([]) Stop  (KEY_STOP is something else!) */
+	{ 0x29, KEY_PAUSE },      /* ('') Pause (KEY_PAUSECD?) */
 
 	/* Extra keys, not on the original ATI remote */
 	{ 0x2a, KEY_NEXT },       /* (>+) */
 	{ 0x2b, KEY_PREVIOUS },   /* (<-) */
-	{ 0x2d, KEY_INFO },       /* PLAYING */
+	{ 0x2d, KEY_INFO },       /* PLAYING  (X11 warning: 0x166) */
 	{ 0x2e, KEY_HOME },       /* TOP */
 	{ 0x2f, KEY_END },        /* END */
-	{ 0x30, KEY_SELECT },     /* SELECT */
+	{ 0x30, KEY_SELECT },     /* SELECT  (X11 warning: 0x161) */
 };
 
 static struct rc_map_list ati_x10_map = {
-- 
1.9.2

Ping: [PATCH 0/10] drivers/media/rc/ati_remote.c tweaks

[George Spelvin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

Just a ping... has anyone looked at this?
(David Härdeman added to recipients list.)

The series can be found in the linux-media archives stating at

mid:20140511111113.14427.qmail@ns.horizon.com
http://www.spinics.net/lists/linux-media/msg76435.html
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/77678

or I'm happy to re-mail it.

Re: Ping: [PATCH 0/10] drivers/media/rc/ati_remote.c tweaks

[Mauro Carvalho Chehab]

Em 6 Jun 2014 23:26:22 -0400
"George Spelvin" <linux@horizon.com> escreveu:

> Just a ping... has anyone looked at this?
> (David Härdeman added to recipients list.)
> 
> The series can be found in the linux-media archives stating at
> 
> mid:20140511111113.14427.qmail@ns.horizon.com
> http://www.spinics.net/lists/linux-media/msg76435.html
> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/77678
> 
> or I'm happy to re-mail it.

As nobody replied, I'm assuming that people are ok with those
changes, so I'll apply.

Regards,
Mauro