* [PATCH 01/11] bt8xx: fixup RC5 decoding
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
@ 2014-03-29 16:10 ` David Härdeman
2014-03-29 16:10 ` [PATCH 02/11] rc-core: improve ir-kbd-i2c get_key functions David Härdeman
` (9 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:10 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
The bt8xx driver does RC5 decoding for Nebula digi hardware, but includes
some pointless limitations (both start bits must be one, the
device/address/system must be 0x00). Remove those limitations and update
the keymap to use the full RC5 scancode (fortunately the 0x00 address
means that this is perfectly backwards compatible).
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/pci/bt8xx/bttv-input.c | 62 ++++++++++---------
drivers/media/pci/bt8xx/bttvp.h | 2 -
drivers/media/rc/keymaps/rc-nebula.c | 112 +++++++++++++++++-----------------
3 files changed, 88 insertions(+), 88 deletions(-)
diff --git a/drivers/media/pci/bt8xx/bttv-input.c b/drivers/media/pci/bt8xx/bttv-input.c
index 5930bce..ffc0ee1 100644
--- a/drivers/media/pci/bt8xx/bttv-input.c
+++ b/drivers/media/pci/bt8xx/bttv-input.c
@@ -154,10 +154,10 @@ static void bttv_input_timer(unsigned long data)
* testing.
*/
-#define RC5_START(x) (((x) >> 12) & 3)
-#define RC5_TOGGLE(x) (((x) >> 11) & 1)
-#define RC5_ADDR(x) (((x) >> 6) & 31)
-#define RC5_INSTR(x) ((x) & 63)
+#define RC5_START(x) (((x) >> 12) & 0x03)
+#define RC5_TOGGLE(x) (((x) >> 11) & 0x01)
+#define RC5_ADDR(x) (((x) >> 6) & 0x1f)
+#define RC5_INSTR(x) (((x) >> 0) & 0x3f)
/* decode raw bit pattern to RC5 code */
static u32 bttv_rc5_decode(unsigned int code)
@@ -195,8 +195,8 @@ static void bttv_rc5_timer_end(unsigned long data)
{
struct bttv_ir *ir = (struct bttv_ir *)data;
struct timeval tv;
- u32 gap;
- u32 rc5 = 0;
+ u32 gap, rc5, scancode;
+ u8 toggle, command, system;
/* get time */
do_gettimeofday(&tv);
@@ -221,26 +221,29 @@ static void bttv_rc5_timer_end(unsigned long data)
if (ir->last_bit < 20) {
/* ignore spurious codes (caused by light/other remotes) */
dprintk("short code: %x\n", ir->code);
- } else {
- ir->code = (ir->code << ir->shift_by) | 1;
- rc5 = bttv_rc5_decode(ir->code);
-
- /* two start bits? */
- if (RC5_START(rc5) != ir->start) {
- pr_info(DEVNAME ":"
- " rc5 start bits invalid: %u\n", RC5_START(rc5));
-
- /* right address? */
- } else if (RC5_ADDR(rc5) == ir->addr) {
- u32 toggle = RC5_TOGGLE(rc5);
- u32 instr = RC5_INSTR(rc5);
-
- /* Good code */
- rc_keydown(ir->dev, instr, toggle);
- dprintk("instruction %x, toggle %x\n",
- instr, toggle);
- }
+ return;
}
+
+ ir->code = (ir->code << ir->shift_by) | 1;
+ rc5 = bttv_rc5_decode(ir->code);
+
+ toggle = RC5_TOGGLE(rc5);
+ system = RC5_ADDR(rc5);
+ command = RC5_INSTR(rc5);
+
+ switch (RC5_START(rc5)) {
+ case 0x3:
+ break;
+ case 0x2:
+ command += 0x40;
+ break;
+ default:
+ return;
+ }
+
+ scancode = system << 8 | command;
+ rc_keydown(ir->dev, scancode, toggle);
+ dprintk("scancode %x, toggle %x\n", scancode, toggle);
}
static int bttv_rc5_irq(struct bttv *btv)
@@ -310,8 +313,6 @@ static void bttv_ir_start(struct bttv *btv, struct bttv_ir *ir)
/* set timer_end for code completion */
setup_timer(&ir->timer, bttv_rc5_timer_end, (unsigned long)ir);
ir->shift_by = 1;
- ir->start = 3;
- ir->addr = 0x0;
ir->rc5_remote_gap = ir_rc5_remote_gap;
}
}
@@ -490,8 +491,8 @@ int bttv_input_init(struct bttv *btv)
ir->polling = 50; // ms
break;
case BTTV_BOARD_NEBULA_DIGITV:
- ir_codes = RC_MAP_NEBULA;
- ir->rc5_gpio = true;
+ ir_codes = RC_MAP_NEBULA;
+ ir->rc5_gpio = true;
break;
case BTTV_BOARD_MACHTV_MAGICTV:
ir_codes = RC_MAP_APAC_VIEWCOMP;
@@ -514,7 +515,8 @@ int bttv_input_init(struct bttv *btv)
ir->mask_keycode);
break;
}
- if (NULL == ir_codes) {
+
+ if (!ir_codes) {
dprintk("Ooops: IR config error [card=%d]\n", btv->c.type);
err = -ENODEV;
goto err_out_free;
diff --git a/drivers/media/pci/bt8xx/bttvp.h b/drivers/media/pci/bt8xx/bttvp.h
index 6eefb59..9fe19488 100644
--- a/drivers/media/pci/bt8xx/bttvp.h
+++ b/drivers/media/pci/bt8xx/bttvp.h
@@ -133,8 +133,6 @@ struct bttv_ir {
u32 polling;
u32 last_gpio;
int shift_by;
- int start; // What should RC5_START() be
- int addr; // What RC5_ADDR() should be.
int rc5_remote_gap;
/* RC5 gpio */
diff --git a/drivers/media/rc/keymaps/rc-nebula.c b/drivers/media/rc/keymaps/rc-nebula.c
index 8ec881a..4c50f33 100644
--- a/drivers/media/rc/keymaps/rc-nebula.c
+++ b/drivers/media/rc/keymaps/rc-nebula.c
@@ -14,68 +14,68 @@
#include <linux/module.h>
static struct rc_map_table nebula[] = {
- { 0x00, KEY_0 },
- { 0x01, KEY_1 },
- { 0x02, KEY_2 },
- { 0x03, KEY_3 },
- { 0x04, KEY_4 },
- { 0x05, KEY_5 },
- { 0x06, KEY_6 },
- { 0x07, KEY_7 },
- { 0x08, KEY_8 },
- { 0x09, KEY_9 },
- { 0x0a, KEY_TV },
- { 0x0b, KEY_AUX },
- { 0x0c, KEY_DVD },
- { 0x0d, KEY_POWER },
- { 0x0e, KEY_CAMERA }, /* labelled 'Picture' */
- { 0x0f, KEY_AUDIO },
- { 0x10, KEY_INFO },
- { 0x11, KEY_F13 }, /* 16:9 */
- { 0x12, KEY_F14 }, /* 14:9 */
- { 0x13, KEY_EPG },
- { 0x14, KEY_EXIT },
- { 0x15, KEY_MENU },
- { 0x16, KEY_UP },
- { 0x17, KEY_DOWN },
- { 0x18, KEY_LEFT },
- { 0x19, KEY_RIGHT },
- { 0x1a, KEY_ENTER },
- { 0x1b, KEY_CHANNELUP },
- { 0x1c, KEY_CHANNELDOWN },
- { 0x1d, KEY_VOLUMEUP },
- { 0x1e, KEY_VOLUMEDOWN },
- { 0x1f, KEY_RED },
- { 0x20, KEY_GREEN },
- { 0x21, KEY_YELLOW },
- { 0x22, KEY_BLUE },
- { 0x23, KEY_SUBTITLE },
- { 0x24, KEY_F15 }, /* AD */
- { 0x25, KEY_TEXT },
- { 0x26, KEY_MUTE },
- { 0x27, KEY_REWIND },
- { 0x28, KEY_STOP },
- { 0x29, KEY_PLAY },
- { 0x2a, KEY_FASTFORWARD },
- { 0x2b, KEY_F16 }, /* chapter */
- { 0x2c, KEY_PAUSE },
- { 0x2d, KEY_PLAY },
- { 0x2e, KEY_RECORD },
- { 0x2f, KEY_F17 }, /* picture in picture */
- { 0x30, KEY_KPPLUS }, /* zoom in */
- { 0x31, KEY_KPMINUS }, /* zoom out */
- { 0x32, KEY_F18 }, /* capture */
- { 0x33, KEY_F19 }, /* web */
- { 0x34, KEY_EMAIL },
- { 0x35, KEY_PHONE },
- { 0x36, KEY_PC },
+ { 0x0000, KEY_0 },
+ { 0x0001, KEY_1 },
+ { 0x0002, KEY_2 },
+ { 0x0003, KEY_3 },
+ { 0x0004, KEY_4 },
+ { 0x0005, KEY_5 },
+ { 0x0006, KEY_6 },
+ { 0x0007, KEY_7 },
+ { 0x0008, KEY_8 },
+ { 0x0009, KEY_9 },
+ { 0x000a, KEY_TV },
+ { 0x000b, KEY_AUX },
+ { 0x000c, KEY_DVD },
+ { 0x000d, KEY_POWER },
+ { 0x000e, KEY_CAMERA }, /* labelled 'Picture' */
+ { 0x000f, KEY_AUDIO },
+ { 0x0010, KEY_INFO },
+ { 0x0011, KEY_F13 }, /* 16:9 */
+ { 0x0012, KEY_F14 }, /* 14:9 */
+ { 0x0013, KEY_EPG },
+ { 0x0014, KEY_EXIT },
+ { 0x0015, KEY_MENU },
+ { 0x0016, KEY_UP },
+ { 0x0017, KEY_DOWN },
+ { 0x0018, KEY_LEFT },
+ { 0x0019, KEY_RIGHT },
+ { 0x001a, KEY_ENTER },
+ { 0x001b, KEY_CHANNELUP },
+ { 0x001c, KEY_CHANNELDOWN },
+ { 0x001d, KEY_VOLUMEUP },
+ { 0x001e, KEY_VOLUMEDOWN },
+ { 0x001f, KEY_RED },
+ { 0x0020, KEY_GREEN },
+ { 0x0021, KEY_YELLOW },
+ { 0x0022, KEY_BLUE },
+ { 0x0023, KEY_SUBTITLE },
+ { 0x0024, KEY_F15 }, /* AD */
+ { 0x0025, KEY_TEXT },
+ { 0x0026, KEY_MUTE },
+ { 0x0027, KEY_REWIND },
+ { 0x0028, KEY_STOP },
+ { 0x0029, KEY_PLAY },
+ { 0x002a, KEY_FASTFORWARD },
+ { 0x002b, KEY_F16 }, /* chapter */
+ { 0x002c, KEY_PAUSE },
+ { 0x002d, KEY_PLAY },
+ { 0x002e, KEY_RECORD },
+ { 0x002f, KEY_F17 }, /* picture in picture */
+ { 0x0030, KEY_KPPLUS }, /* zoom in */
+ { 0x0031, KEY_KPMINUS }, /* zoom out */
+ { 0x0032, KEY_F18 }, /* capture */
+ { 0x0033, KEY_F19 }, /* web */
+ { 0x0034, KEY_EMAIL },
+ { 0x0035, KEY_PHONE },
+ { 0x0036, KEY_PC },
};
static struct rc_map_list nebula_map = {
.map = {
.scan = nebula,
.size = ARRAY_SIZE(nebula),
- .rc_type = RC_TYPE_UNKNOWN, /* Legacy IR type */
+ .rc_type = RC_TYPE_RC5,
.name = RC_MAP_NEBULA,
}
};
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 02/11] rc-core: improve ir-kbd-i2c get_key functions
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
2014-03-29 16:10 ` [PATCH 01/11] bt8xx: fixup RC5 decoding David Härdeman
@ 2014-03-29 16:10 ` David Härdeman
2014-03-29 16:11 ` [PATCH 03/11] rc-core: document the protocol type David Härdeman
` (8 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:10 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
The arguments used for ir-kbd-i2c's get_key() functions are not
really suited for rc-core and the ir_raw/ir_key distinction is
just confusing.
Convert all of them to return a protocol/scancode/toggle triple instead.
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/i2c/ir-kbd-i2c.c | 90 +++++++++++++++--------------
drivers/media/pci/bt8xx/bttv-input.c | 8 ++-
drivers/media/pci/cx88/cx88-input.c | 8 ++-
drivers/media/pci/ivtv/ivtv-i2c.c | 9 ++-
drivers/media/pci/saa7134/saa7134-input.c | 76 +++++++++++++++---------
drivers/media/usb/cx231xx/cx231xx-input.c | 20 ++----
include/media/ir-kbd-i2c.h | 6 +-
include/media/rc-map.h | 10 +++
8 files changed, 127 insertions(+), 100 deletions(-)
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index c8fe135..143cb2b 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -62,8 +62,8 @@ module_param(debug, int, 0644); /* debug level (0,1,2) */
/* ----------------------------------------------------------------------- */
-static int get_key_haup_common(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw,
- int size, int offset)
+static int get_key_haup_common(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *ptoggle, int size, int offset)
{
unsigned char buf[6];
int start, range, toggle, dev, code, ircode;
@@ -86,19 +86,10 @@ static int get_key_haup_common(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw,
if (!start)
/* no key pressed */
return 0;
- /*
- * Hauppauge remotes (black/silver) always use
- * specific device ids. If we do not filter the
- * device ids then messages destined for devices
- * such as TVs (id=0) will get through causing
- * mis-fired events.
- *
- * We also filter out invalid key presses which
- * produce annoying debug log entries.
- */
- ircode= (start << 12) | (toggle << 11) | (dev << 6) | code;
- if ((ircode & 0x1fff)==0x1fff)
- /* invalid key press */
+
+ /* filter out invalid key presses */
+ ircode = (start << 12) | (toggle << 11) | (dev << 6) | code;
+ if ((ircode & 0x1fff) == 0x1fff)
return 0;
if (!range)
@@ -107,18 +98,20 @@ static int get_key_haup_common(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw,
dprintk(1,"ir hauppauge (rc5): s%d r%d t%d dev=%d code=%d\n",
start, range, toggle, dev, code);
- /* return key */
- *ir_key = (dev << 8) | code;
- *ir_raw = ircode;
+ *protocol = RC_TYPE_RC5;
+ *scancode = RC_SCANCODE_RC5(dev, code);
+ *ptoggle = toggle;
return 1;
}
-static int get_key_haup(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_haup(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
- return get_key_haup_common (ir, ir_key, ir_raw, 3, 0);
+ return get_key_haup_common (ir, protocol, scancode, toggle, 3, 0);
}
-static int get_key_haup_xvr(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_haup_xvr(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
int ret;
unsigned char buf[1] = { 0 };
@@ -133,10 +126,11 @@ static int get_key_haup_xvr(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
if (ret != 1)
return (ret < 0) ? ret : -EINVAL;
- return get_key_haup_common (ir, ir_key, ir_raw, 6, 3);
+ return get_key_haup_common(ir, protocol, scancode, toggle, 6, 3);
}
-static int get_key_pixelview(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_pixelview(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char b;
@@ -145,12 +139,15 @@ static int get_key_pixelview(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
dprintk(1,"read error\n");
return -EIO;
}
- *ir_key = b;
- *ir_raw = b;
+
+ *protocol = RC_TYPE_OTHER;
+ *scancode = b;
+ *toggle = 0;
return 1;
}
-static int get_key_fusionhdtv(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_fusionhdtv(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char buf[4];
@@ -168,13 +165,14 @@ static int get_key_fusionhdtv(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
if(buf[0] != 0x1 || buf[1] != 0xfe)
return 0;
- *ir_key = buf[2];
- *ir_raw = (buf[2] << 8) | buf[3];
-
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = buf[2];
+ *toggle = 0;
return 1;
}
-static int get_key_knc1(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_knc1(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char b;
@@ -197,13 +195,14 @@ static int get_key_knc1(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
/* keep old data */
return 1;
- *ir_key = b;
- *ir_raw = b;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = b;
+ *toggle = 0;
return 1;
}
-static int get_key_avermedia_cardbus(struct IR_i2c *ir,
- u32 *ir_key, u32 *ir_raw)
+static int get_key_avermedia_cardbus(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char subaddr, key, keygroup;
struct i2c_msg msg[] = { { .addr = ir->c->addr, .flags = 0,
@@ -237,12 +236,11 @@ static int get_key_avermedia_cardbus(struct IR_i2c *ir,
}
key |= (keygroup & 1) << 6;
- *ir_key = key;
- *ir_raw = key;
- if (!strcmp(ir->ir_codes, RC_MAP_AVERMEDIA_M733A_RM_K6)) {
- *ir_key |= keygroup << 8;
- *ir_raw |= keygroup << 8;
- }
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = key;
+ if (ir->c->addr == 0x41) /* AVerMedia EM78P153 */
+ *scancode |= keygroup << 8;
+ *toggle = 0;
return 1;
}
@@ -250,19 +248,21 @@ static int get_key_avermedia_cardbus(struct IR_i2c *ir,
static int ir_key_poll(struct IR_i2c *ir)
{
- static u32 ir_key, ir_raw;
+ enum rc_type protocol;
+ u32 scancode;
+ u8 toggle;
int rc;
dprintk(3, "%s\n", __func__);
- rc = ir->get_key(ir, &ir_key, &ir_raw);
+ rc = ir->get_key(ir, &protocol, &scancode, &toggle);
if (rc < 0) {
dprintk(2,"error\n");
return rc;
}
if (rc) {
- dprintk(1, "%s: keycode = 0x%04x\n", __func__, ir_key);
- rc_keydown(ir->rc, ir_key, 0);
+ dprintk(1, "%s: scancode = 0x%08x\n", __func__, scancode);
+ rc_keydown(ir->rc, scancode, toggle);
}
return 0;
}
@@ -327,7 +327,7 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
case 0x6b:
name = "FusionHDTV";
ir->get_key = get_key_fusionhdtv;
- rc_type = RC_BIT_RC5;
+ rc_type = RC_BIT_UNKNOWN;
ir_codes = RC_MAP_FUSIONHDTV_MCE;
break;
case 0x40:
diff --git a/drivers/media/pci/bt8xx/bttv-input.c b/drivers/media/pci/bt8xx/bttv-input.c
index ffc0ee1..e745f5a 100644
--- a/drivers/media/pci/bt8xx/bttv-input.c
+++ b/drivers/media/pci/bt8xx/bttv-input.c
@@ -336,7 +336,8 @@ static void bttv_ir_stop(struct bttv *btv)
* Get_key functions used by I2C remotes
*/
-static int get_key_pv951(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_pv951(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char b;
@@ -363,8 +364,9 @@ static int get_key_pv951(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
* the device is bound to the vendor-provided RC.
*/
- *ir_key = b;
- *ir_raw = b;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = b;
+ *toggle = 0;
return 1;
}
diff --git a/drivers/media/pci/cx88/cx88-input.c b/drivers/media/pci/cx88/cx88-input.c
index f991696..779fc63 100644
--- a/drivers/media/pci/cx88/cx88-input.c
+++ b/drivers/media/pci/cx88/cx88-input.c
@@ -539,7 +539,8 @@ void cx88_ir_irq(struct cx88_core *core)
ir_raw_event_handle(ir->dev);
}
-static int get_key_pvr2000(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_pvr2000(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
int flags, code;
@@ -563,8 +564,9 @@ static int get_key_pvr2000(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
dprintk("IR Key/Flags: (0x%02x/0x%02x)\n",
code & 0xff, flags & 0xff);
- *ir_key = code & 0xff;
- *ir_raw = code;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = code & 0xff;
+ *toggle = 0;
return 1;
}
diff --git a/drivers/media/pci/ivtv/ivtv-i2c.c b/drivers/media/pci/ivtv/ivtv-i2c.c
index ceed2d8..1a41ba5 100644
--- a/drivers/media/pci/ivtv/ivtv-i2c.c
+++ b/drivers/media/pci/ivtv/ivtv-i2c.c
@@ -148,7 +148,8 @@ static const char * const hw_devicenames[] = {
"ir_video", /* IVTV_HW_I2C_IR_RX_ADAPTEC */
};
-static int get_key_adaptec(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_adaptec(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char keybuf[4];
@@ -167,9 +168,9 @@ static int get_key_adaptec(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
keybuf[2] &= 0x7f;
keybuf[3] |= 0x80;
- *ir_key = keybuf[3] | keybuf[2] << 8 | keybuf[1] << 16 |keybuf[0] << 24;
- *ir_raw = *ir_key;
-
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = keybuf[3] | keybuf[2] << 8 | keybuf[1] << 16 |keybuf[0] << 24;
+ *toggle = 0;
return 1;
}
diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c
index 6f43126..73670ed 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -108,7 +108,8 @@ static int build_key(struct saa7134_dev *dev)
/* --------------------- Chip specific I2C key builders ----------------- */
-static int get_key_flydvb_trio(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_flydvb_trio(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
int gpio;
int attempt = 0;
@@ -158,13 +159,14 @@ static int get_key_flydvb_trio(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
return -EIO;
}
- *ir_key = b;
- *ir_raw = b;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = b;
+ *toggle = 0;
return 1;
}
-static int get_key_msi_tvanywhere_plus(struct IR_i2c *ir, u32 *ir_key,
- u32 *ir_raw)
+static int get_key_msi_tvanywhere_plus(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char b;
int gpio;
@@ -205,14 +207,15 @@ static int get_key_msi_tvanywhere_plus(struct IR_i2c *ir, u32 *ir_key,
/* Button pressed */
dprintk("get_key_msi_tvanywhere_plus: Key = 0x%02X\n", b);
- *ir_key = b;
- *ir_raw = b;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = b;
+ *toggle = 0;
return 1;
}
/* copied and modified from get_key_msi_tvanywhere_plus() */
-static int get_key_kworld_pc150u(struct IR_i2c *ir, u32 *ir_key,
- u32 *ir_raw)
+static int get_key_kworld_pc150u(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char b;
unsigned int gpio;
@@ -253,12 +256,14 @@ static int get_key_kworld_pc150u(struct IR_i2c *ir, u32 *ir_key,
/* Button pressed */
dprintk("get_key_kworld_pc150u: Key = 0x%02X\n", b);
- *ir_key = b;
- *ir_raw = b;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = b;
+ *toggle = 0;
return 1;
}
-static int get_key_purpletv(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_purpletv(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char b;
@@ -276,12 +281,14 @@ static int get_key_purpletv(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
if (b & 0x80)
return 1;
- *ir_key = b;
- *ir_raw = b;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = b;
+ *toggle = 0;
return 1;
}
-static int get_key_hvr1110(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_hvr1110(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char buf[5];
@@ -299,14 +306,20 @@ static int get_key_hvr1110(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
* by preserving it into two separate readings
* buf[4] bits 0 and 1, and buf[1] and buf[2] are always
* zero.
+ *
+ * Note that the keymap which the hvr1110 uses is RC5.
+ *
+ * FIXME: start bits could maybe be used...?
*/
- *ir_key = 0x1fff & ((buf[3] << 8) | (buf[4] >> 2));
- *ir_raw = *ir_key;
+ *protocol = RC_TYPE_RC5;
+ *scancode = RC_SCANCODE_RC5(buf[3] & 0x1f, buf[4] >> 2);
+ *toggle = !!(buf[3] & 0x40);
return 1;
}
-static int get_key_beholdm6xx(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_beholdm6xx(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
unsigned char data[12];
u32 gpio;
@@ -332,17 +345,18 @@ static int get_key_beholdm6xx(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
if (data[9] != (unsigned char)(~data[8]))
return 0;
- *ir_raw = ((data[10] << 16) | (data[11] << 8) | (data[9] << 0));
- *ir_key = *ir_raw;
-
+ *protocol = RC_TYPE_NEC;
+ *scancode = RC_SCANCODE_NECX(((data[10] << 8) | data[11]), data[9]);
+ *toggle = 0;
return 1;
}
/* Common (grey or coloured) pinnacle PCTV remote handling
*
*/
-static int get_key_pinnacle(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw,
- int parity_offset, int marker, int code_modulo)
+static int get_key_pinnacle(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle, int parity_offset,
+ int marker, int code_modulo)
{
unsigned char b[4];
unsigned int start = 0,parity = 0,code = 0;
@@ -377,11 +391,11 @@ static int get_key_pinnacle(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw,
code %= code_modulo;
- *ir_raw = code;
- *ir_key = code;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = code;
+ *toggle = 0;
i2cdprintk("Pinnacle PCTV key %02x\n", code);
-
return 1;
}
@@ -394,10 +408,11 @@ static int get_key_pinnacle(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw,
*
* Sylvain Pasche <sylvain.pasche@gmail.com>
*/
-static int get_key_pinnacle_grey(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_pinnacle_grey(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
- return get_key_pinnacle(ir, ir_key, ir_raw, 1, 0xfe, 0xff);
+ return get_key_pinnacle(ir, protocol, scancode, toggle, 1, 0xfe, 0xff);
}
@@ -405,7 +420,8 @@ static int get_key_pinnacle_grey(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
*
* Ricardo Cerqueira <v4l@cerqueira.org>
*/
-static int get_key_pinnacle_color(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
+static int get_key_pinnacle_color(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle)
{
/* code_modulo parameter (0x88) is used to reduce code value to fit inside IR_KEYTAB_SIZE
*
@@ -413,7 +429,7 @@ static int get_key_pinnacle_color(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
* codes < 128
*/
- return get_key_pinnacle(ir, ir_key, ir_raw, 2, 0x80, 0x88);
+ return get_key_pinnacle(ir, protocol, scancode, toggle, 2, 0x80, 0x88);
}
void saa7134_input_irq(struct saa7134_dev *dev)
diff --git a/drivers/media/usb/cx231xx/cx231xx-input.c b/drivers/media/usb/cx231xx/cx231xx-input.c
index 46d52fa..adcdd92 100644
--- a/drivers/media/usb/cx231xx/cx231xx-input.c
+++ b/drivers/media/usb/cx231xx/cx231xx-input.c
@@ -21,11 +21,12 @@
#include "cx231xx.h"
#include <linux/usb.h>
#include <linux/slab.h>
+#include <linux/bitrev.h>
#define MODULE_NAME "cx231xx-input"
-static int get_key_isdbt(struct IR_i2c *ir, u32 *ir_key,
- u32 *ir_raw)
+static int get_key_isdbt(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *pscancode, u8 *toggle)
{
int rc;
u8 cmd, scancode;
@@ -46,21 +47,14 @@ static int get_key_isdbt(struct IR_i2c *ir, u32 *ir_key,
if (cmd == 0xff)
return 0;
- scancode =
- ((cmd & 0x01) ? 0x80 : 0) |
- ((cmd & 0x02) ? 0x40 : 0) |
- ((cmd & 0x04) ? 0x20 : 0) |
- ((cmd & 0x08) ? 0x10 : 0) |
- ((cmd & 0x10) ? 0x08 : 0) |
- ((cmd & 0x20) ? 0x04 : 0) |
- ((cmd & 0x40) ? 0x02 : 0) |
- ((cmd & 0x80) ? 0x01 : 0);
+ scancode = bitrev8(cmd);
dev_dbg(&ir->rc->input_dev->dev, "cmd %02x, scan = %02x\n",
cmd, scancode);
- *ir_key = scancode;
- *ir_raw = scancode;
+ *protocol = RC_TYPE_OTHER;
+ *pscancode = scancode;
+ *toggle = 0;
return 1;
}
diff --git a/include/media/ir-kbd-i2c.h b/include/media/ir-kbd-i2c.h
index e221bc7..d856435 100644
--- a/include/media/ir-kbd-i2c.h
+++ b/include/media/ir-kbd-i2c.h
@@ -20,7 +20,8 @@ struct IR_i2c {
struct delayed_work work;
char name[32];
char phys[32];
- int (*get_key)(struct IR_i2c*, u32*, u32*);
+ int (*get_key)(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle);
};
enum ir_kbd_get_key_fn {
@@ -44,7 +45,8 @@ struct IR_i2c_init_data {
* Specify either a function pointer or a value indicating one of
* ir_kbd_i2c's internal get_key functions
*/
- int (*get_key)(struct IR_i2c*, u32*, u32*);
+ int (*get_key)(struct IR_i2c *ir, enum rc_type *protocol,
+ u32 *scancode, u8 *toggle);
enum ir_kbd_get_key_fn internal_get_key_func;
struct rc_dev *rc_dev;
diff --git a/include/media/rc-map.h b/include/media/rc-map.h
index e5aa240..894c7e4 100644
--- a/include/media/rc-map.h
+++ b/include/media/rc-map.h
@@ -62,6 +62,16 @@ enum rc_type {
RC_BIT_RC6_0 | RC_BIT_RC6_6A_20 | RC_BIT_RC6_6A_24 | \
RC_BIT_RC6_6A_32 | RC_BIT_RC6_MCE | RC_BIT_SHARP)
+#define RC_SCANCODE_UNKNOWN(x) (x)
+#define RC_SCANCODE_OTHER(x) (x)
+#define RC_SCANCODE_NEC(addr, cmd) (((addr) << 8) | (cmd))
+#define RC_SCANCODE_NECX(addr, cmd) (((addr) << 8) | (cmd))
+#define RC_SCANCODE_NEC32(data) ((data) & 0xffffffff)
+#define RC_SCANCODE_RC5(sys, cmd) (((sys) << 8) | (cmd))
+#define RC_SCANCODE_RC5_SZ(sys, cmd) (((sys) << 8) | (cmd))
+#define RC_SCANCODE_RC6_0(sys, cmd) (((sys) << 8) | (cmd))
+#define RC_SCANCODE_RC6_6A(vendor, sys, cmd) (((vendor) << 16) | ((sys) << 8) | (cmd))
+
struct rc_map_table {
u32 scancode;
u32 keycode;
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 03/11] rc-core: document the protocol type
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
2014-03-29 16:10 ` [PATCH 01/11] bt8xx: fixup RC5 decoding David Härdeman
2014-03-29 16:10 ` [PATCH 02/11] rc-core: improve ir-kbd-i2c get_key functions David Härdeman
@ 2014-03-29 16:11 ` David Härdeman
2014-03-31 9:54 ` James Hogan
2014-03-29 16:11 ` [PATCH 04/11] rc-core: do not change 32bit NEC scancode format for now David Härdeman
` (7 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:11 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
Right now the protocol information is not preserved, rc-core gets handed a
scancode but has no idea which protocol it corresponds to.
This patch (which required reading through the source/keymap for all drivers,
not fun) makes the protocol information explicit which is important
documentation and makes it easier to e.g. support multiple protocols with one
decoder (think rc5 and rc-streamzap). The information isn't used yet so there
should be no functional changes.
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/i2c/ir-kbd-i2c.c | 5 +
drivers/media/pci/bt8xx/bttv-input.c | 12 ++-
drivers/media/pci/cx88/cx88-input.c | 26 ++++++-
drivers/media/pci/dm1105/dm1105.c | 3 +
drivers/media/pci/saa7134/saa7134-input.c | 6 +-
drivers/media/pci/ttpci/budget-ci.c | 8 +-
drivers/media/rc/img-ir/img-ir-hw.c | 8 +-
drivers/media/rc/img-ir/img-ir-hw.h | 3 +
drivers/media/rc/img-ir/img-ir-jvc.c | 4 +
drivers/media/rc/img-ir/img-ir-nec.c | 4 +
drivers/media/rc/img-ir/img-ir-sanyo.c | 4 +
drivers/media/rc/img-ir/img-ir-sharp.c | 4 +
drivers/media/rc/img-ir/img-ir-sony.c | 12 ++-
drivers/media/rc/ir-jvc-decoder.c | 2 -
drivers/media/rc/ir-nec-decoder.c | 2 -
drivers/media/rc/ir-rc5-decoder.c | 5 +
drivers/media/rc/ir-rc5-sz-decoder.c | 2 -
drivers/media/rc/ir-rc6-decoder.c | 37 ++++++++--
drivers/media/rc/ir-sanyo-decoder.c | 2 -
drivers/media/rc/ir-sharp-decoder.c | 2 -
drivers/media/rc/ir-sony-decoder.c | 6 +-
drivers/media/rc/rc-main.c | 32 +++++----
drivers/media/usb/dvb-usb-v2/af9015.c | 18 +++--
drivers/media/usb/dvb-usb-v2/af9035.c | 9 +-
drivers/media/usb/dvb-usb-v2/anysee.c | 3 +
drivers/media/usb/dvb-usb-v2/az6007.c | 25 ++++---
drivers/media/usb/dvb-usb-v2/lmedm04.c | 9 +-
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 12 ++-
drivers/media/usb/dvb-usb/dib0700_core.c | 16 +++-
drivers/media/usb/dvb-usb/dib0700_devices.c | 24 ++++---
drivers/media/usb/dvb-usb/dw2102.c | 7 +-
drivers/media/usb/dvb-usb/m920x.c | 2 -
drivers/media/usb/dvb-usb/pctv452e.c | 8 +-
drivers/media/usb/dvb-usb/ttusb2.c | 6 +-
drivers/media/usb/em28xx/em28xx-input.c | 98 ++++++++++++++++-----------
drivers/media/usb/tm6000/tm6000-input.c | 51 ++++++++++----
include/media/rc-core.h | 6 +-
37 files changed, 301 insertions(+), 182 deletions(-)
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index 143cb2b..f9c4233 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -261,8 +261,9 @@ static int ir_key_poll(struct IR_i2c *ir)
}
if (rc) {
- dprintk(1, "%s: scancode = 0x%08x\n", __func__, scancode);
- rc_keydown(ir->rc, scancode, toggle);
+ dprintk(1, "%s: proto = 0x%04x, scancode = 0x%08x\n",
+ __func__, protocol, scancode);
+ rc_keydown(ir->rc, protocol, scancode, toggle);
}
return 0;
}
diff --git a/drivers/media/pci/bt8xx/bttv-input.c b/drivers/media/pci/bt8xx/bttv-input.c
index e745f5a..67c8d6b 100644
--- a/drivers/media/pci/bt8xx/bttv-input.c
+++ b/drivers/media/pci/bt8xx/bttv-input.c
@@ -73,12 +73,12 @@ static void ir_handle_key(struct bttv *btv)
if ((ir->mask_keydown && (gpio & ir->mask_keydown)) ||
(ir->mask_keyup && !(gpio & ir->mask_keyup))) {
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
} else {
/* HACK: Probably, ir->mask_keydown is missing
for this board */
if (btv->c.type == BTTV_BOARD_WINFAST2000)
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
rc_keyup(ir->dev);
}
@@ -103,7 +103,7 @@ static void ir_enltv_handle_key(struct bttv *btv)
gpio, data,
(gpio & ir->mask_keyup) ? " up" : "up/down");
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
if (keyup)
rc_keyup(ir->dev);
} else {
@@ -117,7 +117,7 @@ static void ir_enltv_handle_key(struct bttv *btv)
if (keyup)
rc_keyup(ir->dev);
else
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
}
ir->last_gpio = data | keyup;
@@ -241,8 +241,8 @@ static void bttv_rc5_timer_end(unsigned long data)
return;
}
- scancode = system << 8 | command;
- rc_keydown(ir->dev, scancode, toggle);
+ scancode = RC_SCANCODE_RC5(system, command);
+ rc_keydown(ir->dev, RC_TYPE_RC5, scancode, toggle);
dprintk("scancode %x, toggle %x\n", scancode, toggle);
}
diff --git a/drivers/media/pci/cx88/cx88-input.c b/drivers/media/pci/cx88/cx88-input.c
index 779fc63..9bf48ca 100644
--- a/drivers/media/pci/cx88/cx88-input.c
+++ b/drivers/media/pci/cx88/cx88-input.c
@@ -130,25 +130,41 @@ static void cx88_ir_handle_key(struct cx88_IR *ir)
data = (data << 4) | ((gpio_key & 0xf0) >> 4);
- rc_keydown(ir->dev, data, 0);
+ rc_keydown(ir->dev, RC_TYPE_UNKNOWN, data, 0);
+
+ } else if (ir->core->boardnr == CX88_BOARD_PROLINK_PLAYTVPVR ||
+ ir->core->boardnr == CX88_BOARD_PIXELVIEW_PLAYTV_ULTRA_PRO) {
+ /* bit cleared on keydown, NEC scancode, 0xAAAACC, A = 0x866b */
+ u16 addr;
+ u8 cmd;
+ u32 scancode;
+
+ addr = (data >> 8) & 0xffff;
+ cmd = (data >> 0) & 0x00ff;
+ scancode = RC_SCANCODE_NECX(addr, cmd);
+
+ if (0 == (gpio & ir->mask_keyup))
+ rc_keydown_notimeout(ir->dev, RC_TYPE_NEC, scancode, 0);
+ else
+ rc_keyup(ir->dev);
} else if (ir->mask_keydown) {
/* bit set on keydown */
if (gpio & ir->mask_keydown)
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
else
rc_keyup(ir->dev);
} else if (ir->mask_keyup) {
/* bit cleared on keydown */
if (0 == (gpio & ir->mask_keyup))
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
else
rc_keyup(ir->dev);
} else {
/* can't distinguish keydown/up :-/ */
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
rc_keyup(ir->dev);
}
}
@@ -329,6 +345,7 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
* 002-T mini RC, provided with newer PV hardware
*/
ir_codes = RC_MAP_PIXELVIEW_MK12;
+ rc_type = RC_BIT_NEC;
ir->gpio_addr = MO_GP1_IO;
ir->mask_keyup = 0x80;
ir->polling = 10; /* ms */
@@ -416,7 +433,6 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
break;
case CX88_BOARD_TWINHAN_VP1027_DVBS:
ir_codes = RC_MAP_TWINHAN_VP1027_DVBS;
- rc_type = RC_BIT_NEC;
ir->sampling = 0xff00; /* address */
break;
}
diff --git a/drivers/media/pci/dm1105/dm1105.c b/drivers/media/pci/dm1105/dm1105.c
index e60ac35..e8826c5 100644
--- a/drivers/media/pci/dm1105/dm1105.c
+++ b/drivers/media/pci/dm1105/dm1105.c
@@ -678,7 +678,8 @@ static void dm1105_emit_key(struct work_struct *work)
data = (ircom >> 8) & 0x7f;
- rc_keydown(ir->dev, data, 0);
+ /* FIXME: UNKNOWN because we don't generate a full NEC scancode (yet?) */
+ rc_keydown(ir->dev, RC_TYPE_UNKNOWN, data, 0);
}
/* work handler */
diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c
index 73670ed..43dd8bd 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -83,14 +83,14 @@ static int build_key(struct saa7134_dev *dev)
if (data == ir->mask_keycode)
rc_keyup(ir->dev);
else
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
return 0;
}
if (ir->polling) {
if ((ir->mask_keydown && (0 != (gpio & ir->mask_keydown))) ||
(ir->mask_keyup && (0 == (gpio & ir->mask_keyup)))) {
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
} else {
rc_keyup(ir->dev);
}
@@ -98,7 +98,7 @@ static int build_key(struct saa7134_dev *dev)
else { /* IRQ driven mode - handle key press and release in one go */
if ((ir->mask_keydown && (0 != (gpio & ir->mask_keydown))) ||
(ir->mask_keyup && (0 == (gpio & ir->mask_keyup)))) {
- rc_keydown_notimeout(ir->dev, data, 0);
+ rc_keydown_notimeout(ir->dev, RC_TYPE_UNKNOWN, data, 0);
rc_keyup(ir->dev);
}
}
diff --git a/drivers/media/pci/ttpci/budget-ci.c b/drivers/media/pci/ttpci/budget-ci.c
index 0acf920..41ce7de 100644
--- a/drivers/media/pci/ttpci/budget-ci.c
+++ b/drivers/media/pci/ttpci/budget-ci.c
@@ -161,14 +161,14 @@ static void msp430_ir_interrupt(unsigned long data)
return;
if (budget_ci->ir.full_rc5) {
- rc_keydown(dev,
- budget_ci->ir.rc5_device <<8 | budget_ci->ir.ir_key,
- (command & 0x20) ? 1 : 0);
+ rc_keydown(dev, RC_TYPE_RC5,
+ RC_SCANCODE_RC5(budget_ci->ir.rc5_device, budget_ci->ir.ir_key),
+ !!(command & 0x20));
return;
}
/* FIXME: We should generate complete scancodes for all devices */
- rc_keydown(dev, budget_ci->ir.ir_key, (command & 0x20) ? 1 : 0);
+ rc_keydown(dev, RC_TYPE_UNKNOWN, budget_ci->ir.ir_key, !!(command & 0x20));
}
static int msp430_ir_init(struct budget_ci *budget_ci)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index 579a52b..aec79f7 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -778,9 +778,11 @@ static void img_ir_handle_data(struct img_ir_priv *priv, u32 len, u64 raw)
struct img_ir_priv_hw *hw = &priv->hw;
const struct img_ir_decoder *dec = hw->decoder;
int ret = IMG_IR_SCANCODE;
- int scancode;
+ u32 scancode;
+ enum rc_type protocol = RC_TYPE_UNKNOWN;
+
if (dec->scancode)
- ret = dec->scancode(len, raw, &scancode, hw->enabled_protocols);
+ ret = dec->scancode(len, raw, &protocol, &scancode, hw->enabled_protocols);
else if (len >= 32)
scancode = (u32)raw;
else if (len < 32)
@@ -789,7 +791,7 @@ static void img_ir_handle_data(struct img_ir_priv *priv, u32 len, u64 raw)
len, (unsigned long long)raw);
if (ret == IMG_IR_SCANCODE) {
dev_dbg(priv->dev, "decoded scan code %#x\n", scancode);
- rc_keydown(hw->rdev, scancode, 0);
+ rc_keydown(hw->rdev, protocol, scancode, 0);
img_ir_end_repeat(priv);
} else if (ret == IMG_IR_REPEATCODE) {
if (hw->mode == IMG_IR_M_REPEATING) {
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h b/drivers/media/rc/img-ir/img-ir-hw.h
index 6c9a94a..e5dc98b 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -157,7 +157,8 @@ struct img_ir_decoder {
struct img_ir_control control;
/* scancode logic */
- int (*scancode)(int len, u64 raw, int *scancode, u64 protocols);
+ int (*scancode)(int len, u64 raw, enum rc_type *protocol,
+ u32 *scancode, u64 enabled_protocols);
int (*filter)(const struct rc_scancode_filter *in,
struct img_ir_filter *out, u64 protocols);
};
diff --git a/drivers/media/rc/img-ir/img-ir-jvc.c b/drivers/media/rc/img-ir/img-ir-jvc.c
index 10209d2..be68a16 100644
--- a/drivers/media/rc/img-ir/img-ir-jvc.c
+++ b/drivers/media/rc/img-ir/img-ir-jvc.c
@@ -7,7 +7,8 @@
#include "img-ir-hw.h"
/* Convert JVC data to a scancode */
-static int img_ir_jvc_scancode(int len, u64 raw, int *scancode, u64 protocols)
+static int img_ir_jvc_scancode(int len, u64 raw, enum rc_type *protocol,
+ u32 *scancode, u64 enabled_protocols)
{
unsigned int cust, data;
@@ -17,6 +18,7 @@ static int img_ir_jvc_scancode(int len, u64 raw, int *scancode, u64 protocols)
cust = (raw >> 0) & 0xff;
data = (raw >> 8) & 0xff;
+ *protocol = RC_TYPE_JVC;
*scancode = cust << 8 | data;
return IMG_IR_SCANCODE;
}
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c
index e7a731b..c0111d6 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -7,7 +7,8 @@
#include "img-ir-hw.h"
/* Convert NEC data to a scancode */
-static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64 protocols)
+static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
+ u32 *scancode, u64 enabled_protocols)
{
unsigned int addr, addr_inv, data, data_inv;
/* a repeat code has no data */
@@ -39,6 +40,7 @@ static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64 protocols)
*scancode = addr << 8 |
data;
}
+ *protocol = RC_TYPE_NEC;
return IMG_IR_SCANCODE;
}
diff --git a/drivers/media/rc/img-ir/img-ir-sanyo.c b/drivers/media/rc/img-ir/img-ir-sanyo.c
index c2c763e..a0a0129 100644
--- a/drivers/media/rc/img-ir/img-ir-sanyo.c
+++ b/drivers/media/rc/img-ir/img-ir-sanyo.c
@@ -18,7 +18,8 @@
#include "img-ir-hw.h"
/* Convert Sanyo data to a scancode */
-static int img_ir_sanyo_scancode(int len, u64 raw, int *scancode, u64 protocols)
+static int img_ir_sanyo_scancode(int len, u64 raw, enum rc_type *protocol,
+ u32 *scancode, u64 enabled_protocols)
{
unsigned int addr, addr_inv, data, data_inv;
/* a repeat code has no data */
@@ -38,6 +39,7 @@ static int img_ir_sanyo_scancode(int len, u64 raw, int *scancode, u64 protocols)
return -EINVAL;
/* Normal Sanyo */
+ *protocol = RC_TYPE_SANYO;
*scancode = addr << 8 | data;
return IMG_IR_SCANCODE;
}
diff --git a/drivers/media/rc/img-ir/img-ir-sharp.c b/drivers/media/rc/img-ir/img-ir-sharp.c
index 3397cc5..8624fd2 100644
--- a/drivers/media/rc/img-ir/img-ir-sharp.c
+++ b/drivers/media/rc/img-ir/img-ir-sharp.c
@@ -7,7 +7,8 @@
#include "img-ir-hw.h"
/* Convert Sharp data to a scancode */
-static int img_ir_sharp_scancode(int len, u64 raw, int *scancode, u64 protocols)
+static int img_ir_sharp_scancode(int len, u64 raw, enum rc_type *protocol,
+ u32 *scancode, u64 enabled_protocols)
{
unsigned int addr, cmd, exp, chk;
@@ -26,6 +27,7 @@ static int img_ir_sharp_scancode(int len, u64 raw, int *scancode, u64 protocols)
/* probably the second half of the message */
return -EINVAL;
+ *protocol = RC_TYPE_SHARP;
*scancode = addr << 8 | cmd;
return IMG_IR_SCANCODE;
}
diff --git a/drivers/media/rc/img-ir/img-ir-sony.c b/drivers/media/rc/img-ir/img-ir-sony.c
index 993409a..eb11ce8 100644
--- a/drivers/media/rc/img-ir/img-ir-sony.c
+++ b/drivers/media/rc/img-ir/img-ir-sony.c
@@ -7,35 +7,39 @@
#include "img-ir-hw.h"
/* Convert Sony data to a scancode */
-static int img_ir_sony_scancode(int len, u64 raw, int *scancode, u64 protocols)
+static int img_ir_sony_scancode(int len, u64 raw, enum rc_type *protocol,
+ u32 *scancode, u64 enabled_protocols)
{
unsigned int dev, subdev, func;
switch (len) {
case 12:
- if (!(protocols & RC_BIT_SONY12))
+ if (!(enabled_protocols & RC_BIT_SONY12))
return -EINVAL;
func = raw & 0x7f; /* first 7 bits */
raw >>= 7;
dev = raw & 0x1f; /* next 5 bits */
subdev = 0;
+ *protocol = RC_TYPE_SONY12;
break;
case 15:
- if (!(protocols & RC_BIT_SONY15))
+ if (!(enabled_protocols & RC_BIT_SONY15))
return -EINVAL;
func = raw & 0x7f; /* first 7 bits */
raw >>= 7;
dev = raw & 0xff; /* next 8 bits */
subdev = 0;
+ *protocol = RC_TYPE_SONY15;
break;
case 20:
- if (!(protocols & RC_BIT_SONY20))
+ if (!(enabled_protocols & RC_BIT_SONY20))
return -EINVAL;
func = raw & 0x7f; /* first 7 bits */
raw >>= 7;
dev = raw & 0x1f; /* next 5 bits */
raw >>= 5;
subdev = raw & 0xff; /* next 8 bits */
+ *protocol = RC_TYPE_SONY20;
break;
default:
return -EINVAL;
diff --git a/drivers/media/rc/ir-jvc-decoder.c b/drivers/media/rc/ir-jvc-decoder.c
index 4ea62a1..7b79eca 100644
--- a/drivers/media/rc/ir-jvc-decoder.c
+++ b/drivers/media/rc/ir-jvc-decoder.c
@@ -140,7 +140,7 @@ again:
scancode = (bitrev8((data->bits >> 8) & 0xff) << 8) |
(bitrev8((data->bits >> 0) & 0xff) << 0);
IR_dprintk(1, "JVC scancode 0x%04x\n", scancode);
- rc_keydown(dev, scancode, data->toggle);
+ rc_keydown(dev, RC_TYPE_JVC, scancode, data->toggle);
data->first = false;
data->old_bits = data->bits;
} else if (data->bits == data->old_bits) {
diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index 9de1791..735a509 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -192,7 +192,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
if (data->is_nec_x)
data->necx_repeat = true;
- rc_keydown(dev, scancode, 0);
+ rc_keydown(dev, RC_TYPE_NEC, scancode, 0);
data->state = STATE_INACTIVE;
return 0;
}
diff --git a/drivers/media/rc/ir-rc5-decoder.c b/drivers/media/rc/ir-rc5-decoder.c
index 4295d9b..3d38cbc 100644
--- a/drivers/media/rc/ir-rc5-decoder.c
+++ b/drivers/media/rc/ir-rc5-decoder.c
@@ -51,6 +51,7 @@ static int ir_rc5_decode(struct rc_dev *dev, struct ir_raw_event ev)
struct rc5_dec *data = &dev->raw->rc5;
u8 toggle;
u32 scancode;
+ enum rc_type protocol;
if (!rc_protocols_enabled(dev, RC_BIT_RC5 | RC_BIT_RC5X))
return 0;
@@ -138,6 +139,7 @@ again:
toggle = (data->bits & 0x20000) ? 1 : 0;
command += (data->bits & 0x01000) ? 0 : 0x40;
scancode = system << 16 | command << 8 | xdata;
+ protocol = RC_TYPE_RC5X;
IR_dprintk(1, "RC5X scancode 0x%06x (toggle: %u)\n",
scancode, toggle);
@@ -154,12 +156,13 @@ again:
toggle = (data->bits & 0x00800) ? 1 : 0;
command += (data->bits & 0x01000) ? 0 : 0x40;
scancode = system << 8 | command;
+ protocol = RC_TYPE_RC5;
IR_dprintk(1, "RC5 scancode 0x%04x (toggle: %u)\n",
scancode, toggle);
}
- rc_keydown(dev, scancode, toggle);
+ rc_keydown(dev, protocol, scancode, toggle);
data->state = STATE_INACTIVE;
return 0;
}
diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c
index dc18b74..85c7711 100644
--- a/drivers/media/rc/ir-rc5-sz-decoder.c
+++ b/drivers/media/rc/ir-rc5-sz-decoder.c
@@ -115,7 +115,7 @@ again:
IR_dprintk(1, "RC5-sz scancode 0x%04x (toggle: %u)\n",
scancode, toggle);
- rc_keydown(dev, scancode, toggle);
+ rc_keydown(dev, RC_TYPE_RC5_SZ, scancode, toggle);
data->state = STATE_INACTIVE;
return 0;
}
diff --git a/drivers/media/rc/ir-rc6-decoder.c b/drivers/media/rc/ir-rc6-decoder.c
index cfbd64e..1dc97a7 100644
--- a/drivers/media/rc/ir-rc6-decoder.c
+++ b/drivers/media/rc/ir-rc6-decoder.c
@@ -88,6 +88,7 @@ static int ir_rc6_decode(struct rc_dev *dev, struct ir_raw_event ev)
struct rc6_dec *data = &dev->raw->rc6;
u32 scancode;
u8 toggle;
+ enum rc_type protocol;
if (!rc_protocols_enabled(dev, RC_BIT_RC6_0 | RC_BIT_RC6_6A_20 |
RC_BIT_RC6_6A_24 | RC_BIT_RC6_6A_32 |
@@ -233,9 +234,11 @@ again:
case RC6_MODE_0:
scancode = data->body;
toggle = data->toggle;
+ protocol = RC_TYPE_RC6_0;
IR_dprintk(1, "RC6(0) scancode 0x%04x (toggle: %u)\n",
scancode, toggle);
break;
+
case RC6_MODE_6A:
if (data->count > CHAR_BIT * sizeof data->body) {
IR_dprintk(1, "RC6 too many (%u) data bits\n",
@@ -244,23 +247,39 @@ again:
}
scancode = data->body;
- if (data->count == RC6_6A_32_NBITS &&
- (scancode & RC6_6A_LCC_MASK) == RC6_6A_MCE_CC) {
- /* MCE RC */
- toggle = (scancode & RC6_6A_MCE_TOGGLE_MASK) ? 1 : 0;
- scancode &= ~RC6_6A_MCE_TOGGLE_MASK;
- } else {
+ switch (data->count) {
+ case 20:
+ protocol = RC_TYPE_RC6_6A_20;
+ toggle = 0;
+ break;
+ case 24:
+ protocol = RC_BIT_RC6_6A_24;
toggle = 0;
+ break;
+ case 32:
+ if ((scancode & RC6_6A_LCC_MASK) == RC6_6A_MCE_CC) {
+ protocol = RC_TYPE_RC6_MCE;
+ scancode &= ~RC6_6A_MCE_TOGGLE_MASK;
+ toggle = !!(scancode & RC6_6A_MCE_TOGGLE_MASK);
+ } else {
+ protocol = RC_BIT_RC6_6A_32;
+ toggle = 0;
+ }
+ break;
+ default:
+ IR_dprintk(1, "RC6(6A) unsupported length\n");
+ goto out;
}
- IR_dprintk(1, "RC6(6A) scancode 0x%08x (toggle: %u)\n",
- scancode, toggle);
+
+ IR_dprintk(1, "RC6(6A) proto 0x%04x, scancode 0x%08x (toggle: %u)\n",
+ protocol, scancode, toggle);
break;
default:
IR_dprintk(1, "RC6 unknown mode\n");
goto out;
}
- rc_keydown(dev, scancode, toggle);
+ rc_keydown(dev, protocol, scancode, toggle);
data->state = STATE_INACTIVE;
return 0;
}
diff --git a/drivers/media/rc/ir-sanyo-decoder.c b/drivers/media/rc/ir-sanyo-decoder.c
index eb715f0..5f77022 100644
--- a/drivers/media/rc/ir-sanyo-decoder.c
+++ b/drivers/media/rc/ir-sanyo-decoder.c
@@ -167,7 +167,7 @@ static int ir_sanyo_decode(struct rc_dev *dev, struct ir_raw_event ev)
scancode = address << 8 | command;
IR_dprintk(1, "SANYO scancode: 0x%06x\n", scancode);
- rc_keydown(dev, scancode, 0);
+ rc_keydown(dev, RC_TYPE_SANYO, scancode, 0);
data->state = STATE_INACTIVE;
return 0;
}
diff --git a/drivers/media/rc/ir-sharp-decoder.c b/drivers/media/rc/ir-sharp-decoder.c
index 66d2039..c8f2519 100644
--- a/drivers/media/rc/ir-sharp-decoder.c
+++ b/drivers/media/rc/ir-sharp-decoder.c
@@ -162,7 +162,7 @@ static int ir_sharp_decode(struct rc_dev *dev, struct ir_raw_event ev)
scancode = address << 8 | command;
IR_dprintk(1, "Sharp scancode 0x%04x\n", scancode);
- rc_keydown(dev, scancode, 0);
+ rc_keydown(dev, RC_TYPE_SHARP, scancode, 0);
data->state = STATE_INACTIVE;
return 0;
}
diff --git a/drivers/media/rc/ir-sony-decoder.c b/drivers/media/rc/ir-sony-decoder.c
index 599c19a..f485f9fe 100644
--- a/drivers/media/rc/ir-sony-decoder.c
+++ b/drivers/media/rc/ir-sony-decoder.c
@@ -42,6 +42,7 @@ enum sony_state {
static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
{
struct sony_dec *data = &dev->raw->sony;
+ enum rc_type protocol;
u32 scancode;
u8 device, subdevice, function;
@@ -131,6 +132,7 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
device = bitrev8((data->bits << 3) & 0xF8);
subdevice = 0;
function = bitrev8((data->bits >> 4) & 0xFE);
+ protocol = RC_TYPE_SONY12;
break;
case 15:
if (!rc_protocols_enabled(dev, RC_BIT_SONY15)) {
@@ -140,6 +142,7 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
device = bitrev8((data->bits >> 0) & 0xFF);
subdevice = 0;
function = bitrev8((data->bits >> 7) & 0xFE);
+ protocol = RC_TYPE_SONY15;
break;
case 20:
if (!rc_protocols_enabled(dev, RC_BIT_SONY20)) {
@@ -149,6 +152,7 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
device = bitrev8((data->bits >> 5) & 0xF8);
subdevice = bitrev8((data->bits >> 0) & 0xFF);
function = bitrev8((data->bits >> 12) & 0xFE);
+ protocol = RC_TYPE_SONY20;
break;
default:
IR_dprintk(1, "Sony invalid bitcount %u\n", data->count);
@@ -157,7 +161,7 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
scancode = device << 16 | subdevice << 8 | function;
IR_dprintk(1, "Sony(%u) scancode 0x%05x\n", data->count, scancode);
- rc_keydown(dev, scancode, 0);
+ rc_keydown(dev, protocol, scancode, 0);
data->state = STATE_INACTIVE;
return 0;
}
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 99697aa..c0bfd50 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -623,6 +623,7 @@ EXPORT_SYMBOL_GPL(rc_repeat);
/**
* ir_do_keydown() - internal function to process a keypress
* @dev: the struct rc_dev descriptor of the device
+ * @protocol: the protocol of the keypress
* @scancode: the scancode of the keypress
* @keycode: the keycode of the keypress
* @toggle: the toggle value of the keypress
@@ -630,13 +631,14 @@ EXPORT_SYMBOL_GPL(rc_repeat);
* This function is used internally to register a keypress, it must be
* called with keylock held.
*/
-static void ir_do_keydown(struct rc_dev *dev, int scancode,
- u32 keycode, u8 toggle)
+static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
+ u32 scancode, u32 keycode, u8 toggle)
{
struct rc_scancode_filter *filter;
- bool new_event = !dev->keypressed ||
- dev->last_scancode != scancode ||
- dev->last_toggle != toggle;
+ bool new_event = (!dev->keypressed ||
+ dev->last_protocol != protocol ||
+ dev->last_scancode != scancode ||
+ dev->last_toggle != toggle);
if (new_event && dev->keypressed)
ir_do_keyup(dev, false);
@@ -651,13 +653,14 @@ static void ir_do_keydown(struct rc_dev *dev, int scancode,
if (new_event && keycode != KEY_RESERVED) {
/* Register a keypress */
dev->keypressed = true;
+ dev->last_protocol = protocol;
dev->last_scancode = scancode;
dev->last_toggle = toggle;
dev->last_keycode = keycode;
IR_dprintk(1, "%s: key down event, "
- "key 0x%04x, scancode 0x%04x\n",
- dev->input_name, keycode, scancode);
+ "key 0x%04x, protocol 0x%04x, scancode 0x%08x\n",
+ dev->input_name, keycode, protocol, scancode);
input_report_key(dev->input_dev, keycode, 1);
led_trigger_event(led_feedback, LED_FULL);
@@ -669,20 +672,21 @@ static void ir_do_keydown(struct rc_dev *dev, int scancode,
/**
* rc_keydown() - generates input event for a key press
* @dev: the struct rc_dev descriptor of the device
- * @scancode: the scancode that we're seeking
+ * @protocol: the protocol for the keypress
+ * @scancode: the scancode for the keypress
* @toggle: the toggle value (protocol dependent, if the protocol doesn't
* support toggle values, this should be set to zero)
*
* This routine is used to signal that a key has been pressed on the
* remote control.
*/
-void rc_keydown(struct rc_dev *dev, int scancode, u8 toggle)
+void rc_keydown(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 toggle)
{
unsigned long flags;
u32 keycode = rc_g_keycode_from_table(dev, scancode);
spin_lock_irqsave(&dev->keylock, flags);
- ir_do_keydown(dev, scancode, keycode, toggle);
+ ir_do_keydown(dev, protocol, scancode, keycode, toggle);
if (dev->keypressed) {
dev->keyup_jiffies = jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT);
@@ -696,20 +700,22 @@ EXPORT_SYMBOL_GPL(rc_keydown);
* rc_keydown_notimeout() - generates input event for a key press without
* an automatic keyup event at a later time
* @dev: the struct rc_dev descriptor of the device
- * @scancode: the scancode that we're seeking
+ * @protocol: the protocol for the keypress
+ * @scancode: the scancode for the keypress
* @toggle: the toggle value (protocol dependent, if the protocol doesn't
* support toggle values, this should be set to zero)
*
* This routine is used to signal that a key has been pressed on the
* remote control. The driver must manually call rc_keyup() at a later stage.
*/
-void rc_keydown_notimeout(struct rc_dev *dev, int scancode, u8 toggle)
+void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol,
+ u32 scancode, u8 toggle)
{
unsigned long flags;
u32 keycode = rc_g_keycode_from_table(dev, scancode);
spin_lock_irqsave(&dev->keylock, flags);
- ir_do_keydown(dev, scancode, keycode, toggle);
+ ir_do_keydown(dev, protocol, scancode, keycode, toggle);
spin_unlock_irqrestore(&dev->keylock, flags);
}
EXPORT_SYMBOL_GPL(rc_keydown_notimeout);
diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c
index da47d23..5ca738a 100644
--- a/drivers/media/usb/dvb-usb-v2/af9015.c
+++ b/drivers/media/usb/dvb-usb-v2/af9015.c
@@ -1213,7 +1213,7 @@ static int af9015_rc_query(struct dvb_usb_device *d)
if ((state->rc_repeat != buf[6] || buf[0]) &&
!memcmp(&buf[12], state->rc_last, 4)) {
dev_dbg(&d->udev->dev, "%s: key repeated\n", __func__);
- rc_keydown(d->rc_dev, state->rc_keycode, 0);
+ rc_repeat(d->rc_dev);
state->rc_repeat = buf[6];
return ret;
}
@@ -1233,18 +1233,22 @@ static int af9015_rc_query(struct dvb_usb_device *d)
if (buf[14] == (u8) ~buf[15]) {
if (buf[12] == (u8) ~buf[13]) {
/* NEC */
- state->rc_keycode = buf[12] << 8 | buf[14];
+ state->rc_keycode = RC_SCANCODE_NEC(buf[12],
+ buf[14]);
} else {
/* NEC extended*/
- state->rc_keycode = buf[12] << 16 |
- buf[13] << 8 | buf[14];
+ state->rc_keycode = RC_SCANCODE_NECX(buf[12] << 8 |
+ buf[13],
+ buf[14]);
}
} else {
/* 32 bit NEC */
- state->rc_keycode = buf[12] << 24 | buf[13] << 16 |
- buf[14] << 8 | buf[15];
+ state->rc_keycode = RC_SCANCODE_NEC32(buf[12] << 24 |
+ buf[13] << 16 |
+ buf[14] << 8 |
+ buf[15]);
}
- rc_keydown(d->rc_dev, state->rc_keycode, 0);
+ rc_keydown(d->rc_dev, RC_TYPE_NEC, state->rc_keycode, 0);
} else {
dev_dbg(&d->udev->dev, "%s: no key press\n", __func__);
/* Invalidate last keypress */
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 021e4d3..3bfba13 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1287,19 +1287,20 @@ static int af9035_rc_query(struct dvb_usb_device *d)
if ((buf[2] + buf[3]) == 0xff) {
if ((buf[0] + buf[1]) == 0xff) {
/* NEC standard 16bit */
- key = buf[0] << 8 | buf[2];
+ key = RC_SCANCODE_NEC(buf[0], buf[2]);
} else {
/* NEC extended 24bit */
- key = buf[0] << 16 | buf[1] << 8 | buf[2];
+ key = RC_SCANCODE_NECX(buf[0] << 8 | buf[1], buf[2]);
}
} else {
/* NEC full code 32bit */
- key = buf[0] << 24 | buf[1] << 16 | buf[2] << 8 | buf[3];
+ key = RC_SCANCODE_NEC32(buf[0] << 24 | buf[1] << 16 |
+ buf[2] << 8 | buf[3]);
}
dev_dbg(&d->udev->dev, "%s: %*ph\n", __func__, 4, buf);
- rc_keydown(d->rc_dev, key, 0);
+ rc_keydown(d->rc_dev, RC_TYPE_NEC, key, 0);
return 0;
diff --git a/drivers/media/usb/dvb-usb-v2/anysee.c b/drivers/media/usb/dvb-usb-v2/anysee.c
index eeab79b..e4a2382 100644
--- a/drivers/media/usb/dvb-usb-v2/anysee.c
+++ b/drivers/media/usb/dvb-usb-v2/anysee.c
@@ -1038,7 +1038,8 @@ static int anysee_rc_query(struct dvb_usb_device *d)
if (ircode[0]) {
dev_dbg(&d->udev->dev, "%s: key pressed %02x\n", __func__,
ircode[1]);
- rc_keydown(d->rc_dev, 0x08 << 8 | ircode[1], 0);
+ rc_keydown(d->rc_dev, RC_TYPE_NEC,
+ RC_SCANCODE_NEC(0x08, ircode[1]), 0);
}
return 0;
diff --git a/drivers/media/usb/dvb-usb-v2/az6007.c b/drivers/media/usb/dvb-usb-v2/az6007.c
index c3c4b98..935dbaa 100644
--- a/drivers/media/usb/dvb-usb-v2/az6007.c
+++ b/drivers/media/usb/dvb-usb-v2/az6007.c
@@ -207,24 +207,27 @@ static int az6007_streaming_ctrl(struct dvb_frontend *fe, int onoff)
static int az6007_rc_query(struct dvb_usb_device *d)
{
struct az6007_device_state *st = d_to_priv(d);
- unsigned code = 0;
+ unsigned code;
az6007_read(d, AZ6007_READ_IR, 0, 0, st->data, 10);
if (st->data[1] == 0x44)
return 0;
- if ((st->data[1] ^ st->data[2]) == 0xff)
- code = st->data[1];
- else
- code = st->data[1] << 8 | st->data[2];
-
- if ((st->data[3] ^ st->data[4]) == 0xff)
- code = code << 8 | st->data[3];
- else
- code = code << 16 | st->data[3] << 8 | st->data[4];
+ if ((st->data[3] ^ st->data[4]) == 0xff) {
+ if ((st->data[1] ^ st->data[2]) == 0xff)
+ code = RC_SCANCODE_NEC(st->data[1], st->data[3]);
+ else
+ code = RC_SCANCODE_NECX(st->data[1] << 8 | st->data[2],
+ st->data[3]);
+ } else {
+ code = RC_SCANCODE_NEC32(st->data[1] << 24 |
+ st->data[2] << 16 |
+ st->data[3] << 8 |
+ st->data[4]);
+ }
- rc_keydown(d->rc_dev, code, st->data[5]);
+ rc_keydown(d->rc_dev, RC_TYPE_NEC, code, st->data[5]);
return 0;
}
diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c
index f674dc0..31f31fc 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -287,14 +287,13 @@ static void lme2510_int_response(struct urb *lme_urb)
case 0xaa:
debug_data_snipet(1, "INT Remote data snipet", ibuf);
if ((ibuf[4] + ibuf[5]) == 0xff) {
- key = ibuf[5];
- key += (ibuf[3] > 0)
- ? (ibuf[3] ^ 0xff) << 8 : 0;
- key += (ibuf[2] ^ 0xff) << 16;
+ key = RC_SCANCODE_NECX((ibuf[2] ^ 0xff) << 8 |
+ (ibuf[3] > 0) ? (ibuf[3] ^ 0xff) : 0,
+ ibuf[5]);
deb_info(1, "INT Key =%08x", key);
if (adap_to_d(adap)->rc_dev != NULL)
rc_keydown(adap_to_d(adap)->rc_dev,
- key, 0);
+ RC_TYPE_NEC, key, 0);
}
break;
case 0xbb:
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index c83c16c..574f4ee 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1249,19 +1249,19 @@ static int rtl2831u_rc_query(struct dvb_usb_device *d)
if (buf[2] == (u8) ~buf[3]) {
if (buf[0] == (u8) ~buf[1]) {
/* NEC standard (16 bit) */
- rc_code = buf[0] << 8 | buf[2];
+ rc_code = RC_SCANCODE_NEC(buf[0], buf[2]);
} else {
/* NEC extended (24 bit) */
- rc_code = buf[0] << 16 |
- buf[1] << 8 | buf[2];
+ rc_code = RC_SCANCODE_NECX(buf[0] << 8 | buf[1],
+ buf[2]);
}
} else {
/* NEC full (32 bit) */
- rc_code = buf[0] << 24 | buf[1] << 16 |
- buf[2] << 8 | buf[3];
+ rc_code = RC_SCANCODE_NEC32(buf[0] << 24 | buf[1] << 16 |
+ buf[2] << 8 | buf[3]);
}
- rc_keydown(d->rc_dev, rc_code, 0);
+ rc_keydown(d->rc_dev, RC_TYPE_NEC, rc_code, 0);
ret = rtl28xx_wr_reg(d, SYS_IRRC_SR, 1);
if (ret)
diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index bf2a908..6afe7ea 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -674,7 +674,8 @@ static void dib0700_rc_urb_completion(struct urb *purb)
{
struct dvb_usb_device *d = purb->context;
struct dib0700_rc_response *poll_reply;
- u32 uninitialized_var(keycode);
+ enum rc_type protocol;
+ u32 uninitialized_var(scancode);
u8 toggle;
deb_info("%s()\n", __func__);
@@ -707,6 +708,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
switch (d->props.rc.core.protocol) {
case RC_BIT_NEC:
+ protocol = RC_TYPE_NEC;
toggle = 0;
/* NEC protocol sends repeat code as 0 0 0 FF */
@@ -719,19 +721,21 @@ static void dib0700_rc_urb_completion(struct urb *purb)
if ((poll_reply->system ^ poll_reply->not_system) != 0xff) {
deb_data("NEC extended protocol\n");
/* NEC extended code - 24 bits */
- keycode = be16_to_cpu(poll_reply->system16) << 8 | poll_reply->data;
+ scancode = RC_SCANCODE_NECX(be16_to_cpu(poll_reply->system16),
+ poll_reply->data);
} else {
deb_data("NEC normal protocol\n");
/* normal NEC code - 16 bits */
- keycode = poll_reply->system << 8 | poll_reply->data;
+ scancode = RC_SCANCODE_NEC(poll_reply->system,
+ poll_reply->data);
}
break;
default:
deb_data("RC5 protocol\n");
- /* RC5 Protocol */
+ protocol = RC_TYPE_RC5;
toggle = poll_reply->report_id;
- keycode = poll_reply->system << 8 | poll_reply->data;
+ scancode = RC_SCANCODE_RC5(poll_reply->system, poll_reply->data);
break;
}
@@ -744,7 +748,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
goto resubmit;
}
- rc_keydown(d->rc_dev, keycode, toggle);
+ rc_keydown(d->rc_dev, protocol, scancode, toggle);
resubmit:
/* Clean the buffer before we requeue */
diff --git a/drivers/media/usb/dvb-usb/dib0700_devices.c b/drivers/media/usb/dvb-usb/dib0700_devices.c
index 829323e..6a70223 100644
--- a/drivers/media/usb/dvb-usb/dib0700_devices.c
+++ b/drivers/media/usb/dvb-usb/dib0700_devices.c
@@ -489,7 +489,8 @@ static u8 rc_request[] = { REQUEST_POLL_RC, 0 };
static int dib0700_rc_query_old_firmware(struct dvb_usb_device *d)
{
u8 key[4];
- u32 keycode;
+ enum rc_type protocol;
+ u32 scancode;
u8 toggle;
int i;
struct dib0700_state *st = d->priv;
@@ -516,28 +517,29 @@ static int dib0700_rc_query_old_firmware(struct dvb_usb_device *d)
dib0700_rc_setup(d); /* reset ir sensor data to prevent false events */
- d->last_event = 0;
switch (d->props.rc.core.protocol) {
case RC_BIT_NEC:
/* NEC protocol sends repeat code as 0 0 0 FF */
if ((key[3-2] == 0x00) && (key[3-3] == 0x00) &&
- (key[3] == 0xff))
- keycode = d->last_event;
- else {
- keycode = key[3-2] << 8 | key[3-3];
- d->last_event = keycode;
+ (key[3] == 0xff)) {
+ rc_repeat(d->rc_dev);
+ return 0;
}
- rc_keydown(d->rc_dev, keycode, 0);
+ protocol = RC_TYPE_NEC;
+ scancode = RC_SCANCODE_NEC(key[3-2], key[3-3]);
+ toggle = 0;
break;
+
default:
/* RC-5 protocol changes toggle bit on new keypress */
- keycode = key[3-2] << 8 | key[3-3];
+ protocol = RC_TYPE_RC5;
+ scancode = RC_SCANCODE_RC5(key[3-2], key[3-3]);
toggle = key[3-1];
- rc_keydown(d->rc_dev, keycode, toggle);
-
break;
}
+
+ rc_keydown(d->rc_dev, protocol, scancode, toggle);
return 0;
}
diff --git a/drivers/media/usb/dvb-usb/dw2102.c b/drivers/media/usb/dvb-usb/dw2102.c
index ae0f56a..8f22d79 100644
--- a/drivers/media/usb/dvb-usb/dw2102.c
+++ b/drivers/media/usb/dvb-usb/dw2102.c
@@ -1482,7 +1482,7 @@ static int dw2102_rc_query(struct dvb_usb_device *d)
if (msg.buf[0] != 0xff) {
deb_rc("%s: rc code: %x, %x\n",
__func__, key[0], key[1]);
- rc_keydown(d->rc_dev, key[0], 1);
+ rc_keydown(d->rc_dev, RC_TYPE_UNKNOWN, key[0], 0);
}
}
@@ -1503,7 +1503,7 @@ static int prof_rc_query(struct dvb_usb_device *d)
if (msg.buf[0] != 0xff) {
deb_rc("%s: rc code: %x, %x\n",
__func__, key[0], key[1]);
- rc_keydown(d->rc_dev, key[0]^0xff, 1);
+ rc_keydown(d->rc_dev, RC_TYPE_UNKNOWN, key[0]^0xff, 0);
}
}
@@ -1524,7 +1524,8 @@ static int su3000_rc_query(struct dvb_usb_device *d)
if (msg.buf[0] != 0xff) {
deb_rc("%s: rc code: %x, %x\n",
__func__, key[0], key[1]);
- rc_keydown(d->rc_dev, key[1] << 8 | key[0], 1);
+ rc_keydown(d->rc_dev, RC_TYPE_RC5,
+ RC_SCANCODE_RC5(key[1], key[0]), 0);
}
}
diff --git a/drivers/media/usb/dvb-usb/m920x.c b/drivers/media/usb/dvb-usb/m920x.c
index 0306cb7..abf8ab2 100644
--- a/drivers/media/usb/dvb-usb/m920x.c
+++ b/drivers/media/usb/dvb-usb/m920x.c
@@ -245,7 +245,7 @@ static int m920x_rc_core_query(struct dvb_usb_device *d)
else if (state == REMOTE_KEY_REPEAT)
rc_repeat(d->rc_dev);
else
- rc_keydown(d->rc_dev, rc_state[1], 0);
+ rc_keydown(d->rc_dev, RC_TYPE_UNKNOWN, rc_state[1], 0);
out:
kfree(rc_state);
diff --git a/drivers/media/usb/dvb-usb/pctv452e.c b/drivers/media/usb/dvb-usb/pctv452e.c
index 449a996..bdfe896 100644
--- a/drivers/media/usb/dvb-usb/pctv452e.c
+++ b/drivers/media/usb/dvb-usb/pctv452e.c
@@ -565,12 +565,12 @@ static int pctv452e_rc_query(struct dvb_usb_device *d)
if ((rx[3] == 9) && (rx[12] & 0x01)) {
/* got a "press" event */
- state->last_rc_key = (rx[7] << 8) | rx[6];
+ state->last_rc_key = RC_SCANCODE_RC5(rx[7], rx[6]);
if (debug > 2)
info("%s: cmd=0x%02x sys=0x%02x\n",
__func__, rx[6], rx[7]);
- rc_keydown(d->rc_dev, state->last_rc_key, 0);
+ rc_keydown(d->rc_dev, RC_TYPE_RC5, state->last_rc_key, 0);
} else if (state->last_rc_key) {
rc_keyup(d->rc_dev);
state->last_rc_key = 0;
@@ -927,7 +927,7 @@ static struct dvb_usb_device_properties pctv452e_properties = {
.rc.core = {
.rc_codes = RC_MAP_DIB0700_RC5_TABLE,
- .allowed_protos = RC_BIT_UNKNOWN,
+ .allowed_protos = RC_BIT_RC5,
.rc_query = pctv452e_rc_query,
.rc_interval = 100,
},
@@ -980,7 +980,7 @@ static struct dvb_usb_device_properties tt_connect_s2_3600_properties = {
.rc.core = {
.rc_codes = RC_MAP_TT_1500,
- .allowed_protos = RC_BIT_UNKNOWN,
+ .allowed_protos = RC_BIT_RC5,
.rc_query = pctv452e_rc_query,
.rc_interval = 100,
},
diff --git a/drivers/media/usb/dvb-usb/ttusb2.c b/drivers/media/usb/dvb-usb/ttusb2.c
index 2ce3d19..f107173 100644
--- a/drivers/media/usb/dvb-usb/ttusb2.c
+++ b/drivers/media/usb/dvb-usb/ttusb2.c
@@ -438,9 +438,9 @@ static int tt3650_rc_query(struct dvb_usb_device *d)
if (rx[8] & 0x01) {
/* got a "press" event */
- st->last_rc_key = (rx[3] << 8) | rx[2];
+ st->last_rc_key = RC_SCANCODE_RC5(rx[3], rx[2]);
deb_info("%s: cmd=0x%02x sys=0x%02x\n", __func__, rx[2], rx[3]);
- rc_keydown(d->rc_dev, st->last_rc_key, rx[1]);
+ rc_keydown(d->rc_dev, RC_TYPE_RC5, st->last_rc_key, rx[1]);
} else if (st->last_rc_key) {
rc_keyup(d->rc_dev);
st->last_rc_key = 0;
@@ -747,7 +747,7 @@ static struct dvb_usb_device_properties ttusb2_properties_ct3650 = {
.rc_interval = 150, /* Less than IR_KEYPRESS_TIMEOUT */
.rc_codes = RC_MAP_TT_1500,
.rc_query = tt3650_rc_query,
- .allowed_protos = RC_BIT_UNKNOWN,
+ .allowed_protos = RC_BIT_RC5,
},
.num_adapters = 1,
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index 56ef49d..014888f 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -27,6 +27,7 @@
#include <linux/interrupt.h>
#include <linux/usb.h>
#include <linux/slab.h>
+#include <linux/bitrev.h>
#include "em28xx.h"
@@ -53,6 +54,7 @@ struct em28xx_ir_poll_result {
unsigned int toggle_bit:1;
unsigned int read_count:7;
+ enum rc_type protocol;
u32 scancode;
};
@@ -72,7 +74,7 @@ struct em28xx_IR {
/* i2c slave address of external device (if used) */
u16 i2c_dev_addr;
- int (*get_key_i2c)(struct i2c_client *, u32 *);
+ int (*get_key_i2c)(struct i2c_client *ir, enum rc_type *protocol, u32 *scancode);
int (*get_key)(struct em28xx_IR *, struct em28xx_ir_poll_result *);
};
@@ -80,7 +82,8 @@ struct em28xx_IR {
I2C IR based get keycodes - should be used with ir-kbd-i2c
**********************************************************/
-static int em28xx_get_key_terratec(struct i2c_client *i2c_dev, u32 *ir_key)
+static int em28xx_get_key_terratec(struct i2c_client *i2c_dev,
+ enum rc_type *protocol, u32 *scancode)
{
unsigned char b;
@@ -98,14 +101,15 @@ static int em28xx_get_key_terratec(struct i2c_client *i2c_dev, u32 *ir_key)
/* keep old data */
return 1;
- *ir_key = b;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = b;
return 1;
}
-static int em28xx_get_key_em_haup(struct i2c_client *i2c_dev, u32 *ir_key)
+static int em28xx_get_key_em_haup(struct i2c_client *i2c_dev,
+ enum rc_type *protocol, u32 *scancode)
{
unsigned char buf[2];
- u16 code;
int size;
/* poll IR chip */
@@ -127,26 +131,13 @@ static int em28xx_get_key_em_haup(struct i2c_client *i2c_dev, u32 *ir_key)
* So, the code translation is not complete. Yet, it is enough to
* work with the provided RC5 IR.
*/
- code =
- ((buf[0] & 0x01) ? 0x0020 : 0) | /* 0010 0000 */
- ((buf[0] & 0x02) ? 0x0010 : 0) | /* 0001 0000 */
- ((buf[0] & 0x04) ? 0x0008 : 0) | /* 0000 1000 */
- ((buf[0] & 0x08) ? 0x0004 : 0) | /* 0000 0100 */
- ((buf[0] & 0x10) ? 0x0002 : 0) | /* 0000 0010 */
- ((buf[0] & 0x20) ? 0x0001 : 0) | /* 0000 0001 */
- ((buf[1] & 0x08) ? 0x1000 : 0) | /* 0001 0000 */
- ((buf[1] & 0x10) ? 0x0800 : 0) | /* 0000 1000 */
- ((buf[1] & 0x20) ? 0x0400 : 0) | /* 0000 0100 */
- ((buf[1] & 0x40) ? 0x0200 : 0) | /* 0000 0010 */
- ((buf[1] & 0x80) ? 0x0100 : 0); /* 0000 0001 */
-
- /* return key */
- *ir_key = code;
+ *protocol = RC_TYPE_RC5;
+ *scancode = (bitrev8(buf[1]) & 0x1f) << 8 | bitrev8(buf[0]) >> 2;
return 1;
}
static int em28xx_get_key_pinnacle_usb_grey(struct i2c_client *i2c_dev,
- u32 *ir_key)
+ enum rc_type *protocol, u32 *scancode)
{
unsigned char buf[3];
@@ -158,13 +149,13 @@ static int em28xx_get_key_pinnacle_usb_grey(struct i2c_client *i2c_dev,
if (buf[0] != 0x00)
return 0;
- *ir_key = buf[2]&0x3f;
-
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = buf[2] & 0x3f;
return 1;
}
static int em28xx_get_key_winfast_usbii_deluxe(struct i2c_client *i2c_dev,
- u32 *ir_key)
+ enum rc_type *protocol, u32 *scancode)
{
unsigned char subaddr, keydetect, key;
@@ -184,7 +175,8 @@ static int em28xx_get_key_winfast_usbii_deluxe(struct i2c_client *i2c_dev,
if (key == 0x00)
return 0;
- *ir_key = key;
+ *protocol = RC_TYPE_UNKNOWN;
+ *scancode = key;
return 1;
}
@@ -215,7 +207,22 @@ static int default_polling_getkey(struct em28xx_IR *ir,
poll_result->read_count = (msg[0] & 0x7f);
/* Remote Control Address/Data (Regs 0x46/0x47) */
- poll_result->scancode = msg[1] << 8 | msg[2];
+ switch (ir->rc_type) {
+ case RC_BIT_RC5:
+ poll_result->protocol = RC_TYPE_RC5;
+ poll_result->scancode = RC_SCANCODE_RC5(msg[1], msg[2]);
+ break;
+
+ case RC_BIT_NEC:
+ poll_result->protocol = RC_TYPE_NEC;
+ poll_result->scancode = RC_SCANCODE_NEC(msg[1], msg[2]);
+ break;
+
+ default:
+ poll_result->protocol = RC_TYPE_UNKNOWN;
+ poll_result->scancode = msg[1] << 8 | msg[2];
+ break;
+ }
return 0;
}
@@ -247,25 +254,32 @@ static int em2874_polling_getkey(struct em28xx_IR *ir,
*/
switch (ir->rc_type) {
case RC_BIT_RC5:
- poll_result->scancode = msg[1] << 8 | msg[2];
+ poll_result->protocol = RC_TYPE_RC5;
+ poll_result->scancode = RC_SCANCODE_RC5(msg[1], msg[2]);
break;
+
case RC_BIT_NEC:
+ poll_result->protocol = RC_TYPE_RC5;
+ poll_result->scancode = msg[1] << 8 | msg[2];
if ((msg[3] ^ msg[4]) != 0xff) /* 32 bits NEC */
- poll_result->scancode = (msg[1] << 24) |
- (msg[2] << 16) |
- (msg[3] << 8) |
- msg[4];
+ poll_result->scancode = RC_SCANCODE_NEC32((msg[1] << 24) |
+ (msg[2] << 16) |
+ (msg[3] << 8) |
+ (msg[4]));
else if ((msg[1] ^ msg[2]) != 0xff) /* 24 bits NEC */
- poll_result->scancode = (msg[1] << 16) |
- (msg[2] << 8) |
- msg[3];
+ poll_result->scancode = RC_SCANCODE_NECX(msg[1] << 8 |
+ msg[2], msg[3]);
else /* Normal NEC */
- poll_result->scancode = msg[1] << 8 | msg[3];
+ poll_result->scancode = RC_SCANCODE_NEC(msg[1], msg[3]);
break;
+
case RC_BIT_RC6_0:
- poll_result->scancode = msg[1] << 8 | msg[2];
+ poll_result->protocol = RC_TYPE_RC6_0;
+ poll_result->scancode = RC_SCANCODE_RC6_0(msg[1], msg[2]);
break;
+
default:
+ poll_result->protocol = RC_TYPE_UNKNOWN;
poll_result->scancode = (msg[1] << 24) | (msg[2] << 16) |
(msg[3] << 8) | msg[4];
break;
@@ -281,22 +295,24 @@ static int em2874_polling_getkey(struct em28xx_IR *ir,
static int em28xx_i2c_ir_handle_key(struct em28xx_IR *ir)
{
struct em28xx *dev = ir->dev;
- static u32 ir_key;
+ static u32 scancode;
+ enum rc_type protocol;
int rc;
struct i2c_client client;
client.adapter = &ir->dev->i2c_adap[dev->def_i2c_bus];
client.addr = ir->i2c_dev_addr;
- rc = ir->get_key_i2c(&client, &ir_key);
+ rc = ir->get_key_i2c(&client, &protocol, &scancode);
if (rc < 0) {
dprintk("ir->get_key_i2c() failed: %d\n", rc);
return rc;
}
if (rc) {
- dprintk("%s: keycode = 0x%04x\n", __func__, ir_key);
- rc_keydown(ir->rc, ir_key, 0);
+ dprintk("%s: proto = 0x%04x, scancode = 0x%04x\n",
+ __func__, protocol, scancode);
+ rc_keydown(ir->rc, protocol, scancode, 0);
}
return 0;
}
@@ -319,10 +335,12 @@ static void em28xx_ir_handle_key(struct em28xx_IR *ir)
poll_result.scancode);
if (ir->full_code)
rc_keydown(ir->rc,
+ poll_result.protocol,
poll_result.scancode,
poll_result.toggle_bit);
else
rc_keydown(ir->rc,
+ RC_TYPE_UNKNOWN,
poll_result.scancode & 0xff,
poll_result.toggle_bit);
diff --git a/drivers/media/usb/tm6000/tm6000-input.c b/drivers/media/usb/tm6000/tm6000-input.c
index d1af543..676c0232 100644
--- a/drivers/media/usb/tm6000/tm6000-input.c
+++ b/drivers/media/usb/tm6000/tm6000-input.c
@@ -162,11 +162,42 @@ static int tm6000_ir_config(struct tm6000_IR *ir)
return 0;
}
+static void tm6000_ir_keydown(struct tm6000_IR *ir,
+ const char *buf, unsigned int len)
+{
+ u8 device, command;
+ u32 scancode;
+ enum rc_type protocol;
+
+ if (len < 1)
+ return;
+
+ command = buf[0];
+ device = (len > 1 ? buf[1] : 0x0);
+ switch (ir->rc_type) {
+ case RC_BIT_RC5:
+ protocol = RC_TYPE_RC5;
+ scancode = RC_SCANCODE_RC5(device, command);
+ break;
+ case RC_BIT_NEC:
+ protocol = RC_TYPE_NEC;
+ scancode = RC_SCANCODE_NEC(device, command);
+ break;
+ default:
+ protocol = RC_TYPE_OTHER;
+ scancode = RC_SCANCODE_OTHER(device << 8 | command);
+ break;
+ }
+
+ dprintk(1, "%s, protocol: 0x%04x, scancode: 0x%08x\n",
+ __func__, protocol, scancode);
+ rc_keydown(ir->rc, protocol, scancode, 0);
+}
+
static void tm6000_ir_urb_received(struct urb *urb)
{
struct tm6000_core *dev = urb->context;
struct tm6000_IR *ir = dev->ir;
- struct tm6000_ir_poll_result poll_result;
char *buf;
dprintk(2, "%s\n",__func__);
@@ -184,12 +215,7 @@ static void tm6000_ir_urb_received(struct urb *urb)
DUMP_PREFIX_OFFSET,16, 1,
buf, urb->actual_length, false);
- poll_result.rc_data = buf[0];
- if (urb->actual_length > 1)
- poll_result.rc_data |= buf[1] << 8;
-
- dprintk(1, "%s, scancode: 0x%04x\n",__func__, poll_result.rc_data);
- rc_keydown(ir->rc, poll_result.rc_data, 0);
+ tm6000_ir_keydown(ir, urb->transfer_buffer, urb->actual_length);
usb_submit_urb(urb, GFP_ATOMIC);
/*
@@ -204,7 +230,6 @@ static void tm6000_ir_handle_key(struct work_struct *work)
{
struct tm6000_IR *ir = container_of(work, struct tm6000_IR, work.work);
struct tm6000_core *dev = ir->dev;
- struct tm6000_ir_poll_result poll_result;
int rc;
u8 buf[2];
@@ -219,13 +244,8 @@ static void tm6000_ir_handle_key(struct work_struct *work)
if (rc < 0)
return;
- if (rc > 1)
- poll_result.rc_data = buf[0] | buf[1] << 8;
- else
- poll_result.rc_data = buf[0];
-
/* Check if something was read */
- if ((poll_result.rc_data & 0xff) == 0xff) {
+ if ((buf[0] & 0xff) == 0xff) {
if (!ir->pwled) {
tm6000_flash_led(dev, 1);
ir->pwled = 1;
@@ -233,8 +253,7 @@ static void tm6000_ir_handle_key(struct work_struct *work)
return;
}
- dprintk(1, "%s, scancode: 0x%04x\n",__func__, poll_result.rc_data);
- rc_keydown(ir->rc, poll_result.rc_data, 0);
+ tm6000_ir_keydown(ir, buf, rc);
tm6000_flash_led(dev, 0);
ir->pwled = 0;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 0b9f890..dbbe63e 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -88,6 +88,7 @@ enum rc_filter_type {
* @keyup_jiffies: time (in jiffies) when the current keypress should be released
* @timer_keyup: timer for releasing a keypress
* @last_keycode: keycode of last keypress
+ * @last_protocol: protocol of last keypress
* @last_scancode: scancode of last keypress
* @last_toggle: toggle value of last command
* @timeout: optional time after which device stops sending data
@@ -138,6 +139,7 @@ struct rc_dev {
unsigned long keyup_jiffies;
struct timer_list timer_keyup;
u32 last_keycode;
+ enum rc_type last_protocol;
u32 last_scancode;
u8 last_toggle;
u32 timeout;
@@ -217,8 +219,8 @@ int rc_open(struct rc_dev *rdev);
void rc_close(struct rc_dev *rdev);
void rc_repeat(struct rc_dev *dev);
-void rc_keydown(struct rc_dev *dev, int scancode, u8 toggle);
-void rc_keydown_notimeout(struct rc_dev *dev, int scancode, u8 toggle);
+void rc_keydown(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 toggle);
+void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 toggle);
void rc_keyup(struct rc_dev *dev);
u32 rc_g_keycode_from_table(struct rc_dev *dev, u32 scancode);
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 03/11] rc-core: document the protocol type
2014-03-29 16:11 ` [PATCH 03/11] rc-core: document the protocol type David Härdeman
@ 2014-03-31 9:54 ` James Hogan
2014-03-31 19:39 ` David Härdeman
0 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2014-03-31 9:54 UTC (permalink / raw)
To: David Härdeman; +Cc: linux-media, m.chehab
[-- Attachment #1: Type: text/plain, Size: 714 bytes --]
On 29/03/14 16:11, David Härdeman wrote:
> Right now the protocol information is not preserved, rc-core gets handed a
> scancode but has no idea which protocol it corresponds to.
>
> This patch (which required reading through the source/keymap for all drivers,
> not fun) makes the protocol information explicit which is important
> documentation and makes it easier to e.g. support multiple protocols with one
> decoder (think rc5 and rc-streamzap). The information isn't used yet so there
> should be no functional changes.
>
> Signed-off-by: David Härdeman <david@hardeman.nu>
Good stuff. I very much approve of the concept, and had considered doing
the same thing myself.
Cheers
James
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 03/11] rc-core: document the protocol type
2014-03-31 9:54 ` James Hogan
@ 2014-03-31 19:39 ` David Härdeman
0 siblings, 0 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-31 19:39 UTC (permalink / raw)
To: James Hogan; +Cc: linux-media, m.chehab
On Mon, Mar 31, 2014 at 10:54:59AM +0100, James Hogan wrote:
>On 29/03/14 16:11, David Härdeman wrote:
>> Right now the protocol information is not preserved, rc-core gets handed a
>> scancode but has no idea which protocol it corresponds to.
>>
>> This patch (which required reading through the source/keymap for all drivers,
>> not fun) makes the protocol information explicit which is important
>> documentation and makes it easier to e.g. support multiple protocols with one
>> decoder (think rc5 and rc-streamzap). The information isn't used yet so there
>> should be no functional changes.
>>
>> Signed-off-by: David Härdeman <david@hardeman.nu>
>
>Good stuff. I very much approve of the concept, and had considered doing
>the same thing myself.
Thanks, and to reiterate, the patch misses two drivers so I'll still
repost a new version...
--
David Härdeman
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 04/11] rc-core: do not change 32bit NEC scancode format for now
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
` (2 preceding siblings ...)
2014-03-29 16:11 ` [PATCH 03/11] rc-core: document the protocol type David Härdeman
@ 2014-03-29 16:11 ` David Härdeman
2014-03-31 9:09 ` James Hogan
2014-03-29 16:11 ` [PATCH 05/11] rc-core: split dev->s_filter David Härdeman
` (6 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:11 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2
The patch ignores the fact that NEC32 scancodes are generated not only in the
NEC raw decoder but also directly in some drivers. Whichever approach is chosen
it should be consistent across drivers and this patch needs more discussion.
Furthermore, I'm convinced that we have to stop playing games trying to
decipher the "meaning" of NEC scancodes (what's the customer/vendor/address,
which byte is the MSB, etc).
This patch is in preparation for the next few patches in this series.
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/rc/img-ir/img-ir-nec.c | 27 ++++++-----
drivers/media/rc/ir-nec-decoder.c | 5 --
drivers/media/rc/keymaps/rc-tivo.c | 86 +++++++++++++++++-----------------
3 files changed, 59 insertions(+), 59 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c
index c0111d6..40ee844 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -5,6 +5,7 @@
*/
#include "img-ir-hw.h"
+#include <linux/bitrev.h>
/* Convert NEC data to a scancode */
static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
@@ -23,11 +24,11 @@ static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
data_inv = (raw >> 24) & 0xff;
if ((data_inv ^ data) != 0xff) {
/* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- *scancode = addr_inv << 24 |
- addr << 16 |
- data_inv << 8 |
- data;
+ /* scan encoding: AAaaDDdd (LSBit first) */
+ *scancode = bitrev8(addr) << 24 |
+ bitrev8(addr_inv) << 16 |
+ bitrev8(data) << 8 |
+ bitrev8(data_inv);
} else if ((addr_inv ^ addr) != 0xff) {
/* Extended NEC */
/* scan encoding: AAaaDD */
@@ -56,13 +57,15 @@ static int img_ir_nec_filter(const struct rc_scancode_filter *in,
if ((in->data | in->mask) & 0xff000000) {
/* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- addr_inv = (in->data >> 24) & 0xff;
- addr_inv_m = (in->mask >> 24) & 0xff;
- addr = (in->data >> 16) & 0xff;
- addr_m = (in->mask >> 16) & 0xff;
- data_inv = (in->data >> 8) & 0xff;
- data_inv_m = (in->mask >> 8) & 0xff;
+ /* scan encoding: AAaaDDdd (LSBit first) */
+ addr = bitrev8((in->data >> 24) & 0xff);
+ addr_m = (in->mask >> 24) & 0xff;
+ addr_inv = bitrev8((in->data >> 16) & 0xff);
+ addr_inv_m = (in->mask >> 16) & 0xff;
+ data = bitrev8((in->data >> 8) & 0xff);
+ data_m = (in->mask >> 8) & 0xff;
+ data_inv = bitrev8((in->data >> 0) & 0xff);
+ data_inv_m = (in->mask >> 0) & 0xff;
} else if ((in->data | in->mask) & 0x00ff0000) {
/* Extended NEC */
/* scan encoding AAaaDD */
diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index 735a509..c4333d5 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -172,10 +172,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
if (send_32bits) {
/* NEC transport, but modified protocol, used by at
* least Apple and TiVo remotes */
- scancode = not_address << 24 |
- address << 16 |
- not_command << 8 |
- command;
+ scancode = data->bits;
IR_dprintk(1, "NEC (modified) scancode 0x%08x\n", scancode);
} else if ((address ^ not_address) != 0xff) {
/* Extended NEC */
diff --git a/drivers/media/rc/keymaps/rc-tivo.c b/drivers/media/rc/keymaps/rc-tivo.c
index 5cc1b45..454e062 100644
--- a/drivers/media/rc/keymaps/rc-tivo.c
+++ b/drivers/media/rc/keymaps/rc-tivo.c
@@ -15,62 +15,62 @@
* Initial mapping is for the TiVo remote included in the Nero LiquidTV bundle,
* which also ships with a TiVo-branded IR transceiver, supported by the mceusb
* driver. Note that the remote uses an NEC-ish protocol, but instead of having
- * a command/not_command pair, it has a vendor ID of 0x3085, but some keys, the
+ * a command/not_command pair, it has a vendor ID of 0xa10c, but some keys, the
* NEC extended checksums do pass, so the table presently has the intended
* values and the checksum-passed versions for those keys.
*/
static struct rc_map_table tivo[] = {
- { 0x3085f009, KEY_MEDIA }, /* TiVo Button */
- { 0x3085e010, KEY_POWER2 }, /* TV Power */
- { 0x3085e011, KEY_TV }, /* Live TV/Swap */
- { 0x3085c034, KEY_VIDEO_NEXT }, /* TV Input */
- { 0x3085e013, KEY_INFO },
- { 0x3085a05f, KEY_CYCLEWINDOWS }, /* Window */
+ { 0xa10c900f, KEY_MEDIA }, /* TiVo Button */
+ { 0xa10c0807, KEY_POWER2 }, /* TV Power */
+ { 0xa10c8807, KEY_TV }, /* Live TV/Swap */
+ { 0xa10c2c03, KEY_VIDEO_NEXT }, /* TV Input */
+ { 0xa10cc807, KEY_INFO },
+ { 0xa10cfa05, KEY_CYCLEWINDOWS }, /* Window */
{ 0x0085305f, KEY_CYCLEWINDOWS },
- { 0x3085c036, KEY_EPG }, /* Guide */
+ { 0xa10c6c03, KEY_EPG }, /* Guide */
- { 0x3085e014, KEY_UP },
- { 0x3085e016, KEY_DOWN },
- { 0x3085e017, KEY_LEFT },
- { 0x3085e015, KEY_RIGHT },
+ { 0xa10c2807, KEY_UP },
+ { 0xa10c6807, KEY_DOWN },
+ { 0xa10ce807, KEY_LEFT },
+ { 0xa10ca807, KEY_RIGHT },
- { 0x3085e018, KEY_SCROLLDOWN }, /* Red Thumbs Down */
- { 0x3085e019, KEY_SELECT },
- { 0x3085e01a, KEY_SCROLLUP }, /* Green Thumbs Up */
+ { 0xa10c1807, KEY_SCROLLDOWN }, /* Red Thumbs Down */
+ { 0xa10c9807, KEY_SELECT },
+ { 0xa10c5807, KEY_SCROLLUP }, /* Green Thumbs Up */
- { 0x3085e01c, KEY_VOLUMEUP },
- { 0x3085e01d, KEY_VOLUMEDOWN },
- { 0x3085e01b, KEY_MUTE },
- { 0x3085d020, KEY_RECORD },
- { 0x3085e01e, KEY_CHANNELUP },
- { 0x3085e01f, KEY_CHANNELDOWN },
+ { 0xa10c3807, KEY_VOLUMEUP },
+ { 0xa10cb807, KEY_VOLUMEDOWN },
+ { 0xa10cd807, KEY_MUTE },
+ { 0xa10c040b, KEY_RECORD },
+ { 0xa10c7807, KEY_CHANNELUP },
+ { 0xa10cf807, KEY_CHANNELDOWN },
{ 0x0085301f, KEY_CHANNELDOWN },
- { 0x3085d021, KEY_PLAY },
- { 0x3085d023, KEY_PAUSE },
- { 0x3085d025, KEY_SLOW },
- { 0x3085d022, KEY_REWIND },
- { 0x3085d024, KEY_FASTFORWARD },
- { 0x3085d026, KEY_PREVIOUS },
- { 0x3085d027, KEY_NEXT }, /* ->| */
+ { 0xa10c840b, KEY_PLAY },
+ { 0xa10cc40b, KEY_PAUSE },
+ { 0xa10ca40b, KEY_SLOW },
+ { 0xa10c440b, KEY_REWIND },
+ { 0xa10c240b, KEY_FASTFORWARD },
+ { 0xa10c640b, KEY_PREVIOUS },
+ { 0xa10ce40b, KEY_NEXT }, /* ->| */
- { 0x3085b044, KEY_ZOOM }, /* Aspect */
- { 0x3085b048, KEY_STOP },
- { 0x3085b04a, KEY_DVD }, /* DVD Menu */
+ { 0xa10c220d, KEY_ZOOM }, /* Aspect */
+ { 0xa10c120d, KEY_STOP },
+ { 0xa10c520d, KEY_DVD }, /* DVD Menu */
- { 0x3085d028, KEY_NUMERIC_1 },
- { 0x3085d029, KEY_NUMERIC_2 },
- { 0x3085d02a, KEY_NUMERIC_3 },
- { 0x3085d02b, KEY_NUMERIC_4 },
- { 0x3085d02c, KEY_NUMERIC_5 },
- { 0x3085d02d, KEY_NUMERIC_6 },
- { 0x3085d02e, KEY_NUMERIC_7 },
- { 0x3085d02f, KEY_NUMERIC_8 },
+ { 0xa10c140b, KEY_NUMERIC_1 },
+ { 0xa10c940b, KEY_NUMERIC_2 },
+ { 0xa10c540b, KEY_NUMERIC_3 },
+ { 0xa10cd40b, KEY_NUMERIC_4 },
+ { 0xa10c340b, KEY_NUMERIC_5 },
+ { 0xa10cb40b, KEY_NUMERIC_6 },
+ { 0xa10c740b, KEY_NUMERIC_7 },
+ { 0xa10cf40b, KEY_NUMERIC_8 },
{ 0x0085302f, KEY_NUMERIC_8 },
- { 0x3085c030, KEY_NUMERIC_9 },
- { 0x3085c031, KEY_NUMERIC_0 },
- { 0x3085c033, KEY_ENTER },
- { 0x3085c032, KEY_CLEAR },
+ { 0xa10c0c03, KEY_NUMERIC_9 },
+ { 0xa10c8c03, KEY_NUMERIC_0 },
+ { 0xa10ccc03, KEY_ENTER },
+ { 0xa10c4c03, KEY_CLEAR },
};
static struct rc_map_list tivo_map = {
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 04/11] rc-core: do not change 32bit NEC scancode format for now
2014-03-29 16:11 ` [PATCH 04/11] rc-core: do not change 32bit NEC scancode format for now David Härdeman
@ 2014-03-31 9:09 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2014-03-31 9:09 UTC (permalink / raw)
To: David Härdeman; +Cc: linux-media, m.chehab
[-- Attachment #1: Type: text/plain, Size: 3142 bytes --]
On 29/03/14 16:11, David Härdeman wrote:
> This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2
>
> The patch ignores the fact that NEC32 scancodes are generated not only in the
> NEC raw decoder but also directly in some drivers. Whichever approach is chosen
> it should be consistent across drivers and this patch needs more discussion.
>
> Furthermore, I'm convinced that we have to stop playing games trying to
> decipher the "meaning" of NEC scancodes (what's the customer/vendor/address,
> which byte is the MSB, etc).
>
> This patch is in preparation for the next few patches in this series.
>
> Signed-off-by: David Härdeman <david@hardeman.nu>
> ---
> drivers/media/rc/img-ir/img-ir-nec.c | 27 ++++++-----
> drivers/media/rc/ir-nec-decoder.c | 5 --
> drivers/media/rc/keymaps/rc-tivo.c | 86 +++++++++++++++++-----------------
> 3 files changed, 59 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c
> index c0111d6..40ee844 100644
> --- a/drivers/media/rc/img-ir/img-ir-nec.c
> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
> @@ -5,6 +5,7 @@
> */
>
> #include "img-ir-hw.h"
> +#include <linux/bitrev.h>
>
> /* Convert NEC data to a scancode */
> static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
> @@ -23,11 +24,11 @@ static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
> data_inv = (raw >> 24) & 0xff;
> if ((data_inv ^ data) != 0xff) {
> /* 32-bit NEC (used by Apple and TiVo remotes) */
> - /* scan encoding: aaAAddDD */
> - *scancode = addr_inv << 24 |
> - addr << 16 |
> - data_inv << 8 |
> - data;
> + /* scan encoding: AAaaDDdd (LSBit first) */
> + *scancode = bitrev8(addr) << 24 |
> + bitrev8(addr_inv) << 16 |
> + bitrev8(data) << 8 |
> + bitrev8(data_inv);
> } else if ((addr_inv ^ addr) != 0xff) {
> /* Extended NEC */
> /* scan encoding: AAaaDD */
> @@ -56,13 +57,15 @@ static int img_ir_nec_filter(const struct rc_scancode_filter *in,
>
> if ((in->data | in->mask) & 0xff000000) {
> /* 32-bit NEC (used by Apple and TiVo remotes) */
> - /* scan encoding: aaAAddDD */
> - addr_inv = (in->data >> 24) & 0xff;
> - addr_inv_m = (in->mask >> 24) & 0xff;
> - addr = (in->data >> 16) & 0xff;
> - addr_m = (in->mask >> 16) & 0xff;
> - data_inv = (in->data >> 8) & 0xff;
> - data_inv_m = (in->mask >> 8) & 0xff;
> + /* scan encoding: AAaaDDdd (LSBit first) */
> + addr = bitrev8((in->data >> 24) & 0xff);
> + addr_m = (in->mask >> 24) & 0xff;
> + addr_inv = bitrev8((in->data >> 16) & 0xff);
> + addr_inv_m = (in->mask >> 16) & 0xff;
> + data = bitrev8((in->data >> 8) & 0xff);
> + data_m = (in->mask >> 8) & 0xff;
> + data_inv = bitrev8((in->data >> 0) & 0xff);
> + data_inv_m = (in->mask >> 0) & 0xff;
I think the masks need bit reversing too, otherwise the mask bits won't
line up with the data as intended.
Otherwise this patch looks okay to me.
Cheers
James
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 05/11] rc-core: split dev->s_filter
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
` (3 preceding siblings ...)
2014-03-29 16:11 ` [PATCH 04/11] rc-core: do not change 32bit NEC scancode format for now David Härdeman
@ 2014-03-29 16:11 ` David Härdeman
2014-04-03 23:27 ` James Hogan
2014-03-29 16:11 ` [PATCH 06/11] rc-core: remove generic scancode filter David Härdeman
` (5 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:11 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
Overloading dev->s_filter to do two different functions (set wakeup filters
and generic hardware filters) makes it impossible to tell what the
hardware actually supports, so create a separate dev->s_wakeup_filter and
make the distinction explicit.
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/rc/img-ir/img-ir-hw.c | 15 ++++++++++++++-
drivers/media/rc/rc-main.c | 31 +++++++++++++++++++------------
include/media/rc-core.h | 6 ++++--
3 files changed, 37 insertions(+), 15 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index aec79f7..871a9b3 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -504,6 +504,18 @@ unlock:
return ret;
}
+static int img_ir_set_normal_filter(struct rc_dev *dev,
+ struct rc_scancode_filter *sc_filter)
+{
+ return img_ir_set_filter(dev, RC_FILTER_NORMAL, sc_filter);
+}
+
+static int img_ir_set_wakeup_filter(struct rc_dev *dev,
+ struct rc_scancode_filter *sc_filter)
+{
+ return img_ir_set_filter(dev, RC_FILTER_WAKEUP, sc_filter);
+}
+
/**
* img_ir_set_decoder() - Set the current decoder.
* @priv: IR private data.
@@ -988,7 +1000,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
rdev->map_name = RC_MAP_EMPTY;
rc_set_allowed_protocols(rdev, img_ir_allowed_protos(priv));
rdev->input_name = "IMG Infrared Decoder";
- rdev->s_filter = img_ir_set_filter;
+ rdev->s_filter = img_ir_set_normal_filter;
+ rdev->s_wakeup_filter = img_ir_set_wakeup_filter;
/* Register hardware decoder */
error = rc_register_device(rdev);
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index c0bfd50..ba955ac 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -929,6 +929,7 @@ static ssize_t store_protocols(struct device *device,
int rc, i, count = 0;
ssize_t ret;
int (*change_protocol)(struct rc_dev *dev, u64 *rc_type);
+ int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter);
struct rc_scancode_filter local_filter, *filter;
/* Device is being removed */
@@ -1013,24 +1014,27 @@ static ssize_t store_protocols(struct device *device,
* Fall back to clearing the filter.
*/
filter = &dev->scancode_filters[fattr->type];
+ set_filter = (fattr->type == RC_FILTER_NORMAL)
+ ? dev->s_filter : dev->s_wakeup_filter;
+
if (old_type != type && filter->mask) {
local_filter = *filter;
if (!type) {
/* no protocol => clear filter */
ret = -1;
- } else if (!dev->s_filter) {
+ } else if (!set_filter) {
/* generic filtering => accept any filter */
ret = 0;
} else {
/* hardware filtering => try setting, otherwise clear */
- ret = dev->s_filter(dev, fattr->type, &local_filter);
+ ret = set_filter(dev, &local_filter);
}
if (ret < 0) {
/* clear the filter */
local_filter.data = 0;
local_filter.mask = 0;
- if (dev->s_filter)
- dev->s_filter(dev, fattr->type, &local_filter);
+ if (set_filter)
+ set_filter(dev, &local_filter);
}
/* commit the new filter */
@@ -1112,6 +1116,7 @@ static ssize_t store_filter(struct device *device,
struct rc_scancode_filter local_filter, *filter;
int ret;
unsigned long val;
+ int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter);
/* Device is being removed */
if (!dev)
@@ -1121,9 +1126,11 @@ static ssize_t store_filter(struct device *device,
if (ret < 0)
return ret;
- /* Scancode filter not supported (but still accept 0) */
- if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL)
- return val ? -EINVAL : count;
+ /* Can the scancode filter be set? */
+ set_filter = (fattr->type == RC_FILTER_NORMAL)
+ ? dev->s_filter : dev->s_wakeup_filter;
+ if (!set_filter)
+ return -EINVAL;
mutex_lock(&dev->lock);
@@ -1134,16 +1141,16 @@ static ssize_t store_filter(struct device *device,
local_filter.mask = val;
else
local_filter.data = val;
+
if (!dev->enabled_protocols[fattr->type] && local_filter.mask) {
/* refuse to set a filter unless a protocol is enabled */
ret = -EINVAL;
goto unlock;
}
- if (dev->s_filter) {
- ret = dev->s_filter(dev, fattr->type, &local_filter);
- if (ret < 0)
- goto unlock;
- }
+
+ ret = set_filter(dev, &local_filter);
+ if (ret < 0)
+ goto unlock;
/* Success, commit the new filter */
*filter = local_filter;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index dbbe63e..8c31e4a 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -113,7 +113,8 @@ enum rc_filter_type {
* device doesn't interrupt host until it sees IR pulses
* @s_learning_mode: enable wide band receiver used for learning
* @s_carrier_report: enable carrier reports
- * @s_filter: set the scancode filter of a given type
+ * @s_filter: set the scancode filter
+ * @s_wakeup_filter: set the wakeup scancode filter
*/
struct rc_dev {
struct device dev;
@@ -161,8 +162,9 @@ struct rc_dev {
int (*s_learning_mode)(struct rc_dev *dev, int enable);
int (*s_carrier_report) (struct rc_dev *dev, int enable);
int (*s_filter)(struct rc_dev *dev,
- enum rc_filter_type type,
struct rc_scancode_filter *filter);
+ int (*s_wakeup_filter)(struct rc_dev *dev,
+ struct rc_scancode_filter *filter);
};
#define to_rc_dev(d) container_of(d, struct rc_dev, dev)
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 05/11] rc-core: split dev->s_filter
2014-03-29 16:11 ` [PATCH 05/11] rc-core: split dev->s_filter David Härdeman
@ 2014-04-03 23:27 ` James Hogan
0 siblings, 0 replies; 30+ messages in thread
From: James Hogan @ 2014-04-03 23:27 UTC (permalink / raw)
To: David Härdeman; +Cc: linux-media, m.chehab
[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]
Hi David,
On Saturday 29 March 2014 17:11:11 David Härdeman wrote:
> Overloading dev->s_filter to do two different functions (set wakeup filters
> and generic hardware filters) makes it impossible to tell what the
> hardware actually supports, so create a separate dev->s_wakeup_filter and
> make the distinction explicit.
>
> Signed-off-by: David Härdeman <david@hardeman.nu>
> ---
> @@ -1121,9 +1126,11 @@ static ssize_t store_filter(struct device *device,
> if (ret < 0)
> return ret;
>
> - /* Scancode filter not supported (but still accept 0) */
> - if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL)
> - return val ? -EINVAL : count;
> + /* Can the scancode filter be set? */
> + set_filter = (fattr->type == RC_FILTER_NORMAL)
> + ? dev->s_filter : dev->s_wakeup_filter;
> + if (!set_filter)
> + return -EINVAL;
Technically the removal of the "fattr->type != RC_FILTER_NORMAL" condition and
returning -EINVAL rather than "val ? -EINVAL : count" should be in patch 6
since it's for generic scancode filter support.
> - if (dev->s_filter) {
> - ret = dev->s_filter(dev, fattr->type, &local_filter);
> - if (ret < 0)
> - goto unlock;
> - }
> +
> + ret = set_filter(dev, &local_filter);
> + if (ret < 0)
> + goto unlock;
same here for removing the if condition.
Otherwise this patch looks okay to me.
Cheers
James
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 06/11] rc-core: remove generic scancode filter
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
` (4 preceding siblings ...)
2014-03-29 16:11 ` [PATCH 05/11] rc-core: split dev->s_filter David Härdeman
@ 2014-03-29 16:11 ` David Härdeman
2014-03-31 9:29 ` James Hogan
2014-03-29 16:11 ` [PATCH 07/11] dib0700: NEC scancode cleanup David Härdeman
` (4 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:11 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
The generic scancode filtering has questionable value and makes it
impossible to determine from userspace if there is an actual
scancode hw filter present or not.
So revert the generic parts.
Based on a patch from James Hogan <james.hogan@imgtec.com>, but this
version also makes sure that only the valid sysfs files are created
in the first place.
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/rc/rc-main.c | 66 +++++++++++++++++++++++++++++---------------
include/media/rc-core.h | 2 +
2 files changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index ba955ac..8675e07 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -634,7 +634,6 @@ EXPORT_SYMBOL_GPL(rc_repeat);
static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
u32 scancode, u32 keycode, u8 toggle)
{
- struct rc_scancode_filter *filter;
bool new_event = (!dev->keypressed ||
dev->last_protocol != protocol ||
dev->last_scancode != scancode ||
@@ -643,11 +642,6 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
if (new_event && dev->keypressed)
ir_do_keyup(dev, false);
- /* Generic scancode filtering */
- filter = &dev->scancode_filters[RC_FILTER_NORMAL];
- if (filter->mask && ((scancode ^ filter->data) & filter->mask))
- return;
-
input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode);
if (new_event && keycode != KEY_RESERVED) {
@@ -1017,14 +1011,11 @@ static ssize_t store_protocols(struct device *device,
set_filter = (fattr->type == RC_FILTER_NORMAL)
? dev->s_filter : dev->s_wakeup_filter;
- if (old_type != type && filter->mask) {
+ if (set_filter && old_type != type && filter->mask) {
local_filter = *filter;
if (!type) {
/* no protocol => clear filter */
ret = -1;
- } else if (!set_filter) {
- /* generic filtering => accept any filter */
- ret = 0;
} else {
/* hardware filtering => try setting, otherwise clear */
ret = set_filter(dev, &local_filter);
@@ -1033,8 +1024,7 @@ static ssize_t store_protocols(struct device *device,
/* clear the filter */
local_filter.data = 0;
local_filter.mask = 0;
- if (set_filter)
- set_filter(dev, &local_filter);
+ set_filter(dev, &local_filter);
}
/* commit the new filter */
@@ -1078,7 +1068,9 @@ static ssize_t show_filter(struct device *device,
return -EINVAL;
mutex_lock(&dev->lock);
- if (fattr->mask)
+ if (!dev->s_filter)
+ val = 0;
+ else if (fattr->mask)
val = dev->scancode_filters[fattr->type].mask;
else
val = dev->scancode_filters[fattr->type].data;
@@ -1202,27 +1194,45 @@ static RC_FILTER_ATTR(wakeup_filter, S_IRUGO|S_IWUSR,
static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR,
show_filter, store_filter, RC_FILTER_WAKEUP, true);
-static struct attribute *rc_dev_attrs[] = {
+static struct attribute *rc_dev_protocol_attrs[] = {
&dev_attr_protocols.attr.attr,
+ NULL,
+};
+
+static struct attribute_group rc_dev_protocol_attr_grp = {
+ .attrs = rc_dev_protocol_attrs,
+};
+
+static struct attribute *rc_dev_wakeup_protocol_attrs[] = {
&dev_attr_wakeup_protocols.attr.attr,
+ NULL,
+};
+
+static struct attribute_group rc_dev_wakeup_protocol_attr_grp = {
+ .attrs = rc_dev_wakeup_protocol_attrs,
+};
+
+static struct attribute *rc_dev_filter_attrs[] = {
&dev_attr_filter.attr.attr,
&dev_attr_filter_mask.attr.attr,
- &dev_attr_wakeup_filter.attr.attr,
- &dev_attr_wakeup_filter_mask.attr.attr,
NULL,
};
-static struct attribute_group rc_dev_attr_grp = {
- .attrs = rc_dev_attrs,
+static struct attribute_group rc_dev_filter_attr_grp = {
+ .attrs = rc_dev_filter_attrs,
+};
+
+static struct attribute *rc_dev_wakeup_filter_attrs[] = {
+ &dev_attr_wakeup_filter.attr.attr,
+ &dev_attr_wakeup_filter_mask.attr.attr,
+ NULL,
};
-static const struct attribute_group *rc_dev_attr_groups[] = {
- &rc_dev_attr_grp,
- NULL
+static struct attribute_group rc_dev_wakeup_filter_attr_grp = {
+ .attrs = rc_dev_wakeup_filter_attrs,
};
static struct device_type rc_dev_type = {
- .groups = rc_dev_attr_groups,
.release = rc_dev_release,
.uevent = rc_dev_uevent,
};
@@ -1279,7 +1289,7 @@ int rc_register_device(struct rc_dev *dev)
static bool raw_init = false; /* raw decoders loaded? */
struct rc_map *rc_map;
const char *path;
- int rc, devno;
+ int rc, devno, attr = 0;
if (!dev || !dev->map_name)
return -EINVAL;
@@ -1307,6 +1317,16 @@ int rc_register_device(struct rc_dev *dev)
return -ENOMEM;
} while (test_and_set_bit(devno, ir_core_dev_number));
+ dev->dev.groups = dev->sysfs_groups;
+ dev->sysfs_groups[attr++] = &rc_dev_protocol_attr_grp;
+ if (dev->s_filter)
+ dev->sysfs_groups[attr++] = &rc_dev_filter_attr_grp;
+ if (dev->s_wakeup_filter)
+ dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp;
+ if (dev->change_wakeup_protocol)
+ dev->sysfs_groups[attr++] = &rc_dev_wakeup_protocol_attr_grp;
+ dev->sysfs_groups[attr++] = NULL;
+
/*
* Take the lock here, as the device sysfs node will appear
* when device_add() is called, which may trigger an ir-keytable udev
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 8c31e4a..2e97b98 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -60,6 +60,7 @@ enum rc_filter_type {
/**
* struct rc_dev - represents a remote control device
* @dev: driver model's view of this device
+ * @sysfs_groups: sysfs attribute groups
* @input_name: name of the input child device
* @input_phys: physical path to the input child device
* @input_id: id of the input child device (struct input_id)
@@ -118,6 +119,7 @@ enum rc_filter_type {
*/
struct rc_dev {
struct device dev;
+ const struct attribute_group *sysfs_groups[5];
const char *input_name;
const char *input_phys;
struct input_id input_id;
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 06/11] rc-core: remove generic scancode filter
2014-03-29 16:11 ` [PATCH 06/11] rc-core: remove generic scancode filter David Härdeman
@ 2014-03-31 9:29 ` James Hogan
2014-03-31 19:38 ` David Härdeman
0 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2014-03-31 9:29 UTC (permalink / raw)
To: David Härdeman; +Cc: linux-media, m.chehab
[-- Attachment #1: Type: text/plain, Size: 6831 bytes --]
On 29/03/14 16:11, David Härdeman wrote:
> The generic scancode filtering has questionable value and makes it
> impossible to determine from userspace if there is an actual
> scancode hw filter present or not.
>
> So revert the generic parts.
>
> Based on a patch from James Hogan <james.hogan@imgtec.com>, but this
> version also makes sure that only the valid sysfs files are created
> in the first place.
>
> Signed-off-by: David Härdeman <david@hardeman.nu>
> ---
> drivers/media/rc/rc-main.c | 66 +++++++++++++++++++++++++++++---------------
> include/media/rc-core.h | 2 +
> 2 files changed, 45 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index ba955ac..8675e07 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -634,7 +634,6 @@ EXPORT_SYMBOL_GPL(rc_repeat);
> static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
> u32 scancode, u32 keycode, u8 toggle)
> {
> - struct rc_scancode_filter *filter;
> bool new_event = (!dev->keypressed ||
> dev->last_protocol != protocol ||
> dev->last_scancode != scancode ||
> @@ -643,11 +642,6 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
> if (new_event && dev->keypressed)
> ir_do_keyup(dev, false);
>
> - /* Generic scancode filtering */
> - filter = &dev->scancode_filters[RC_FILTER_NORMAL];
> - if (filter->mask && ((scancode ^ filter->data) & filter->mask))
> - return;
> -
> input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode);
>
> if (new_event && keycode != KEY_RESERVED) {
> @@ -1017,14 +1011,11 @@ static ssize_t store_protocols(struct device *device,
> set_filter = (fattr->type == RC_FILTER_NORMAL)
> ? dev->s_filter : dev->s_wakeup_filter;
>
> - if (old_type != type && filter->mask) {
> + if (set_filter && old_type != type && filter->mask) {
> local_filter = *filter;
> if (!type) {
> /* no protocol => clear filter */
> ret = -1;
> - } else if (!set_filter) {
> - /* generic filtering => accept any filter */
> - ret = 0;
> } else {
> /* hardware filtering => try setting, otherwise clear */
> ret = set_filter(dev, &local_filter);
> @@ -1033,8 +1024,7 @@ static ssize_t store_protocols(struct device *device,
> /* clear the filter */
> local_filter.data = 0;
> local_filter.mask = 0;
> - if (set_filter)
> - set_filter(dev, &local_filter);
> + set_filter(dev, &local_filter);
> }
>
> /* commit the new filter */
> @@ -1078,7 +1068,9 @@ static ssize_t show_filter(struct device *device,
> return -EINVAL;
>
> mutex_lock(&dev->lock);
> - if (fattr->mask)
> + if (!dev->s_filter)
> + val = 0;
I suspect this should take s_wakeup_filter into account depending on
fattr->type. It's probably quite common to have a wakeup filter but no
normal filter.
The rest looks reasonable, though it could easily have been a separate
patch (at least as long as the show/store callbacks don't assume the
presence of the callbacks they use).
Cheers
James
> + else if (fattr->mask)
> val = dev->scancode_filters[fattr->type].mask;
> else
> val = dev->scancode_filters[fattr->type].data;
> @@ -1202,27 +1194,45 @@ static RC_FILTER_ATTR(wakeup_filter, S_IRUGO|S_IWUSR,
> static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR,
> show_filter, store_filter, RC_FILTER_WAKEUP, true);
>
> -static struct attribute *rc_dev_attrs[] = {
> +static struct attribute *rc_dev_protocol_attrs[] = {
> &dev_attr_protocols.attr.attr,
> + NULL,
> +};
> +
> +static struct attribute_group rc_dev_protocol_attr_grp = {
> + .attrs = rc_dev_protocol_attrs,
> +};
> +
> +static struct attribute *rc_dev_wakeup_protocol_attrs[] = {
> &dev_attr_wakeup_protocols.attr.attr,
> + NULL,
> +};
> +
> +static struct attribute_group rc_dev_wakeup_protocol_attr_grp = {
> + .attrs = rc_dev_wakeup_protocol_attrs,
> +};
> +
> +static struct attribute *rc_dev_filter_attrs[] = {
> &dev_attr_filter.attr.attr,
> &dev_attr_filter_mask.attr.attr,
> - &dev_attr_wakeup_filter.attr.attr,
> - &dev_attr_wakeup_filter_mask.attr.attr,
> NULL,
> };
>
> -static struct attribute_group rc_dev_attr_grp = {
> - .attrs = rc_dev_attrs,
> +static struct attribute_group rc_dev_filter_attr_grp = {
> + .attrs = rc_dev_filter_attrs,
> +};
> +
> +static struct attribute *rc_dev_wakeup_filter_attrs[] = {
> + &dev_attr_wakeup_filter.attr.attr,
> + &dev_attr_wakeup_filter_mask.attr.attr,
> + NULL,
> };
>
> -static const struct attribute_group *rc_dev_attr_groups[] = {
> - &rc_dev_attr_grp,
> - NULL
> +static struct attribute_group rc_dev_wakeup_filter_attr_grp = {
> + .attrs = rc_dev_wakeup_filter_attrs,
> };
>
> static struct device_type rc_dev_type = {
> - .groups = rc_dev_attr_groups,
> .release = rc_dev_release,
> .uevent = rc_dev_uevent,
> };
> @@ -1279,7 +1289,7 @@ int rc_register_device(struct rc_dev *dev)
> static bool raw_init = false; /* raw decoders loaded? */
> struct rc_map *rc_map;
> const char *path;
> - int rc, devno;
> + int rc, devno, attr = 0;
>
> if (!dev || !dev->map_name)
> return -EINVAL;
> @@ -1307,6 +1317,16 @@ int rc_register_device(struct rc_dev *dev)
> return -ENOMEM;
> } while (test_and_set_bit(devno, ir_core_dev_number));
>
> + dev->dev.groups = dev->sysfs_groups;
> + dev->sysfs_groups[attr++] = &rc_dev_protocol_attr_grp;
> + if (dev->s_filter)
> + dev->sysfs_groups[attr++] = &rc_dev_filter_attr_grp;
> + if (dev->s_wakeup_filter)
> + dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp;
> + if (dev->change_wakeup_protocol)
> + dev->sysfs_groups[attr++] = &rc_dev_wakeup_protocol_attr_grp;
> + dev->sysfs_groups[attr++] = NULL;
> +
> /*
> * Take the lock here, as the device sysfs node will appear
> * when device_add() is called, which may trigger an ir-keytable udev
> diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> index 8c31e4a..2e97b98 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -60,6 +60,7 @@ enum rc_filter_type {
> /**
> * struct rc_dev - represents a remote control device
> * @dev: driver model's view of this device
> + * @sysfs_groups: sysfs attribute groups
> * @input_name: name of the input child device
> * @input_phys: physical path to the input child device
> * @input_id: id of the input child device (struct input_id)
> @@ -118,6 +119,7 @@ enum rc_filter_type {
> */
> struct rc_dev {
> struct device dev;
> + const struct attribute_group *sysfs_groups[5];
> const char *input_name;
> const char *input_phys;
> struct input_id input_id;
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 06/11] rc-core: remove generic scancode filter
2014-03-31 9:29 ` James Hogan
@ 2014-03-31 19:38 ` David Härdeman
2014-03-31 22:01 ` James Hogan
0 siblings, 1 reply; 30+ messages in thread
From: David Härdeman @ 2014-03-31 19:38 UTC (permalink / raw)
To: James Hogan; +Cc: linux-media, m.chehab
On Mon, Mar 31, 2014 at 10:29:53AM +0100, James Hogan wrote:
>On 29/03/14 16:11, David Härdeman wrote:
>> The generic scancode filtering has questionable value and makes it
>> impossible to determine from userspace if there is an actual
>> scancode hw filter present or not.
>>
>> So revert the generic parts.
>>
>> Based on a patch from James Hogan <james.hogan@imgtec.com>, but this
>> version also makes sure that only the valid sysfs files are created
>> in the first place.
>>
>> Signed-off-by: David Härdeman <david@hardeman.nu>
>> ---
>> drivers/media/rc/rc-main.c | 66 +++++++++++++++++++++++++++++---------------
>> include/media/rc-core.h | 2 +
>> 2 files changed, 45 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
>> index ba955ac..8675e07 100644
>> --- a/drivers/media/rc/rc-main.c
>> +++ b/drivers/media/rc/rc-main.c
>> @@ -634,7 +634,6 @@ EXPORT_SYMBOL_GPL(rc_repeat);
>> static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
>> u32 scancode, u32 keycode, u8 toggle)
>> {
>> - struct rc_scancode_filter *filter;
>> bool new_event = (!dev->keypressed ||
>> dev->last_protocol != protocol ||
>> dev->last_scancode != scancode ||
>> @@ -643,11 +642,6 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
>> if (new_event && dev->keypressed)
>> ir_do_keyup(dev, false);
>>
>> - /* Generic scancode filtering */
>> - filter = &dev->scancode_filters[RC_FILTER_NORMAL];
>> - if (filter->mask && ((scancode ^ filter->data) & filter->mask))
>> - return;
>> -
>> input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode);
>>
>> if (new_event && keycode != KEY_RESERVED) {
>> @@ -1017,14 +1011,11 @@ static ssize_t store_protocols(struct device *device,
>> set_filter = (fattr->type == RC_FILTER_NORMAL)
>> ? dev->s_filter : dev->s_wakeup_filter;
>>
>> - if (old_type != type && filter->mask) {
>> + if (set_filter && old_type != type && filter->mask) {
>> local_filter = *filter;
>> if (!type) {
>> /* no protocol => clear filter */
>> ret = -1;
>> - } else if (!set_filter) {
>> - /* generic filtering => accept any filter */
>> - ret = 0;
>> } else {
>> /* hardware filtering => try setting, otherwise clear */
>> ret = set_filter(dev, &local_filter);
>> @@ -1033,8 +1024,7 @@ static ssize_t store_protocols(struct device *device,
>> /* clear the filter */
>> local_filter.data = 0;
>> local_filter.mask = 0;
>> - if (set_filter)
>> - set_filter(dev, &local_filter);
>> + set_filter(dev, &local_filter);
>> }
>>
>> /* commit the new filter */
>> @@ -1078,7 +1068,9 @@ static ssize_t show_filter(struct device *device,
>> return -EINVAL;
>>
>> mutex_lock(&dev->lock);
>> - if (fattr->mask)
>> + if (!dev->s_filter)
>> + val = 0;
>
>I suspect this should take s_wakeup_filter into account depending on
>fattr->type. It's probably quite common to have a wakeup filter but no
>normal filter.
Thanks for spotting that.
>The rest looks reasonable, though it could easily have been a separate
>patch (at least as long as the show/store callbacks don't assume the
>presence of the callbacks they use).
Yes, I wanted to avoid there being more intermediary states than
necessary (i.e. first a read/writable sysfs file, then one that can't be
read/written, then the file disappears...).
Can still respin it on top of your patch if you prefer.
--
David Härdeman
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 07/11] dib0700: NEC scancode cleanup
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
` (5 preceding siblings ...)
2014-03-29 16:11 ` [PATCH 06/11] rc-core: remove generic scancode filter David Härdeman
@ 2014-03-29 16:11 ` David Härdeman
2014-03-29 16:11 ` [PATCH 08/11] lmedm04: " David Härdeman
` (3 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:11 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
the RC RX packet is defined as:
struct dib0700_rc_response {
...
u8 not_system;
u8 system;
...
u8 data;
u8 not_data;
The NEC protocol transmits in the order:
system
not_system
data
not_data
Note that the code defines the NEC extended scancode as:
scancode = be16_to_cpu(poll_reply->system16) << 8 | poll_reply->data;
i.e.
scancode = poll_reply->not_system << 16 |
poll_reply->system << 8 |
poll_reply->data;
Which, if the order *is* reversed, would mean that the scancode that
gets defined is in reality:
scancode = poll_reply->system << 16 |
poll_reply->not_system << 8 |
poll_reply->data;
Which is the same as the order used in drivers/media/rc/ir-nec-decoder.c.
This patch changes the code to match my assumption (the generated scancode
should, however, not change).
Signed-off-by: David Härdeman <david@hardeman.nu>
CC: Patrick Boettcher <pboettcher@kernellabs.com>
---
drivers/media/usb/dvb-usb/dib0700_core.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index 6afe7ea..4f5caf5 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -658,13 +658,8 @@ out:
struct dib0700_rc_response {
u8 report_id;
u8 data_state;
- union {
- u16 system16;
- struct {
- u8 not_system;
- u8 system;
- };
- };
+ u8 system;
+ u8 not_system;
u8 data;
u8 not_data;
};
@@ -712,20 +707,27 @@ static void dib0700_rc_urb_completion(struct urb *purb)
toggle = 0;
/* NEC protocol sends repeat code as 0 0 0 FF */
- if ((poll_reply->system == 0x00) && (poll_reply->data == 0x00)
- && (poll_reply->not_data == 0xff)) {
+ if (poll_reply->system == 0x00 &&
+ poll_reply->not_system == 0x00 &&
+ poll_reply->data == 0x00 &&
+ poll_reply->not_data == 0xff) {
poll_reply->data_state = 2;
break;
}
- if ((poll_reply->system ^ poll_reply->not_system) != 0xff) {
+ if (poll_reply->data ^ poll_reply->not_data != 0xff) {
+ deb_data("NEC32 protocol\n");
+ scancode = RC_SCANCODE_NEC32(poll_reply->system << 24 |
+ poll_reply->not_system << 16 |
+ poll_reply->data << 8 |
+ poll_reply->not_data);
+ } else if (poll_reply->system ^ poll_reply->not_system != 0xff) {
deb_data("NEC extended protocol\n");
- /* NEC extended code - 24 bits */
- scancode = RC_SCANCODE_NECX(be16_to_cpu(poll_reply->system16),
+ scancode = RC_SCANCODE_NECX(poll_reply->system << 8 |
+ poll_reply->not_system,
poll_reply->data);
} else {
deb_data("NEC normal protocol\n");
- /* normal NEC code - 16 bits */
scancode = RC_SCANCODE_NEC(poll_reply->system,
poll_reply->data);
}
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 08/11] lmedm04: NEC scancode cleanup
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
` (6 preceding siblings ...)
2014-03-29 16:11 ` [PATCH 07/11] dib0700: NEC scancode cleanup David Härdeman
@ 2014-03-29 16:11 ` David Härdeman
2014-03-29 16:11 ` [PATCH 09/11] saa7134: NEC scancode fix David Härdeman
` (2 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:11 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
I am assuming (given the ^ 0xff) that the hardware sends inverted bytes.
And that the reason ibuf[5] does not need ^ 0xff is that it already is
the inverted command (i.e. ibuf[5] == ~ibuf[4]).
To put it differently:
ibuf[2] = ~addr = not_addr;
ibuf[3] = ~not_addr = addr;
ibuf[4] = ~cmd = not_cmd;
ibuf[5] = ~not_cmd = cmd;
And the scancode can then be understood as:
addr << 16 | not_addr << 8 | cmd
Except for when addr = 0x00 in which case the scancode is simply NEC16:
0x00 << 8 | cmd
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/rc/keymaps/rc-lme2510.c | 80 ++++++++++++++++----------------
drivers/media/usb/dvb-usb-v2/lmedm04.c | 28 ++++++++---
2 files changed, 59 insertions(+), 49 deletions(-)
diff --git a/drivers/media/rc/keymaps/rc-lme2510.c b/drivers/media/rc/keymaps/rc-lme2510.c
index 51f18bb..76e9265 100644
--- a/drivers/media/rc/keymaps/rc-lme2510.c
+++ b/drivers/media/rc/keymaps/rc-lme2510.c
@@ -42,47 +42,47 @@ static struct rc_map_table lme2510_rc[] = {
{ 0x10ed07, KEY_EPG },
{ 0x10ed01, KEY_STOP },
/* Type 2 - 20 buttons */
- { 0xbf15, KEY_0 },
- { 0xbf08, KEY_1 },
- { 0xbf09, KEY_2 },
- { 0xbf0a, KEY_3 },
- { 0xbf0c, KEY_4 },
- { 0xbf0d, KEY_5 },
- { 0xbf0e, KEY_6 },
- { 0xbf10, KEY_7 },
- { 0xbf11, KEY_8 },
- { 0xbf12, KEY_9 },
- { 0xbf00, KEY_POWER },
- { 0xbf04, KEY_MEDIA_REPEAT}, /* Recall */
- { 0xbf1a, KEY_PAUSE }, /* Timeshift */
- { 0xbf02, KEY_VOLUMEUP }, /* 2 x -/+ Keys not marked */
- { 0xbf06, KEY_VOLUMEDOWN }, /* Volume defined as right hand*/
- { 0xbf01, KEY_CHANNELUP },
- { 0xbf05, KEY_CHANNELDOWN },
- { 0xbf14, KEY_ZOOM },
- { 0xbf18, KEY_RECORD },
- { 0xbf16, KEY_STOP },
+ { 0x00bf15, KEY_0 },
+ { 0x00bf08, KEY_1 },
+ { 0x00bf09, KEY_2 },
+ { 0x00bf0a, KEY_3 },
+ { 0x00bf0c, KEY_4 },
+ { 0x00bf0d, KEY_5 },
+ { 0x00bf0e, KEY_6 },
+ { 0x00bf10, KEY_7 },
+ { 0x00bf11, KEY_8 },
+ { 0x00bf12, KEY_9 },
+ { 0x00bf00, KEY_POWER },
+ { 0x00bf04, KEY_MEDIA_REPEAT}, /* Recall */
+ { 0x00bf1a, KEY_PAUSE }, /* Timeshift */
+ { 0x00bf02, KEY_VOLUMEUP }, /* 2 x -/+ Keys not marked */
+ { 0x00bf06, KEY_VOLUMEDOWN }, /* Volume defined as right hand*/
+ { 0x00bf01, KEY_CHANNELUP },
+ { 0x00bf05, KEY_CHANNELDOWN },
+ { 0x00bf14, KEY_ZOOM },
+ { 0x00bf18, KEY_RECORD },
+ { 0x00bf16, KEY_STOP },
/* Type 3 - 20 buttons */
- { 0x1c, KEY_0 },
- { 0x07, KEY_1 },
- { 0x15, KEY_2 },
- { 0x09, KEY_3 },
- { 0x16, KEY_4 },
- { 0x19, KEY_5 },
- { 0x0d, KEY_6 },
- { 0x0c, KEY_7 },
- { 0x18, KEY_8 },
- { 0x5e, KEY_9 },
- { 0x45, KEY_POWER },
- { 0x44, KEY_MEDIA_REPEAT}, /* Recall */
- { 0x4a, KEY_PAUSE }, /* Timeshift */
- { 0x47, KEY_VOLUMEUP }, /* 2 x -/+ Keys not marked */
- { 0x43, KEY_VOLUMEDOWN }, /* Volume defined as right hand*/
- { 0x46, KEY_CHANNELUP },
- { 0x40, KEY_CHANNELDOWN },
- { 0x08, KEY_ZOOM },
- { 0x42, KEY_RECORD },
- { 0x5a, KEY_STOP },
+ { 0x00001c, KEY_0 },
+ { 0x000007, KEY_1 },
+ { 0x000015, KEY_2 },
+ { 0x000009, KEY_3 },
+ { 0x000016, KEY_4 },
+ { 0x000019, KEY_5 },
+ { 0x00000d, KEY_6 },
+ { 0x00000c, KEY_7 },
+ { 0x000018, KEY_8 },
+ { 0x00005e, KEY_9 },
+ { 0x000045, KEY_POWER },
+ { 0x000044, KEY_MEDIA_REPEAT}, /* Recall */
+ { 0x00004a, KEY_PAUSE }, /* Timeshift */
+ { 0x000047, KEY_VOLUMEUP }, /* 2 x -/+ Keys not marked */
+ { 0x000043, KEY_VOLUMEDOWN }, /* Volume defined as right hand*/
+ { 0x000046, KEY_CHANNELUP },
+ { 0x000040, KEY_CHANNELDOWN },
+ { 0x000008, KEY_ZOOM },
+ { 0x000042, KEY_RECORD },
+ { 0x00005a, KEY_STOP },
};
static struct rc_map_list lme2510_map = {
diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c
index 31f31fc..6e3ca72 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -286,15 +286,25 @@ static void lme2510_int_response(struct urb *lme_urb)
switch (ibuf[0]) {
case 0xaa:
debug_data_snipet(1, "INT Remote data snipet", ibuf);
- if ((ibuf[4] + ibuf[5]) == 0xff) {
- key = RC_SCANCODE_NECX((ibuf[2] ^ 0xff) << 8 |
- (ibuf[3] > 0) ? (ibuf[3] ^ 0xff) : 0,
- ibuf[5]);
- deb_info(1, "INT Key =%08x", key);
- if (adap_to_d(adap)->rc_dev != NULL)
- rc_keydown(adap_to_d(adap)->rc_dev,
- RC_TYPE_NEC, key, 0);
- }
+ if (!adap_to_d(adap)->rc_dev)
+ break;
+
+ ibuf[2] ^= 0xff;
+ ibuf[3] ^= 0xff;
+ ibuf[4] ^= 0xff;
+ ibuf[5] ^= 0xff;
+
+ if (ibuf[4] ^ ibuf[5] == 0xff)
+ key = RC_SCANCODE_NECX(ibuf[2] << 8 | ibuf[3],
+ ibuf[4]);
+ else
+ key = RC_SCANCODE_NEC32(ibuf[2] << 24 |
+ ibuf[3] << 16 |
+ ibuf[4] << 8 |
+ ibuf[5]);
+
+ deb_info(1, "INT Key =%08x", key);
+ rc_keydown(adap_to_d(adap)->rc_dev, RC_TYPE_NEC, key, 0);
break;
case 0xbb:
switch (st->tuner_config) {
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 09/11] saa7134: NEC scancode fix
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
` (7 preceding siblings ...)
2014-03-29 16:11 ` [PATCH 08/11] lmedm04: " David Härdeman
@ 2014-03-29 16:11 ` David Härdeman
2014-03-29 16:11 ` [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes David Härdeman
2014-03-29 16:11 ` [PATCH 11/11] [RFC] rc-core: don't throw away protocol information David Härdeman
10 siblings, 0 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:11 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
This driver codes the two address bytes in reverse order when compared to the
other drivers, so make it consistent (and update the keymap, note that the
result is a prefix change from 0x6b86 -> 0x866b, and the latter is pretty
common among the NECX keymaps. While not conclusive, it's still a strong hint
that the change is correct).
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/pci/saa7134/saa7134-input.c | 2 -
drivers/media/rc/keymaps/rc-behold.c | 68 +++++++++++++++--------------
2 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c
index 43dd8bd..887429b 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -346,7 +346,7 @@ static int get_key_beholdm6xx(struct IR_i2c *ir, enum rc_type *protocol,
return 0;
*protocol = RC_TYPE_NEC;
- *scancode = RC_SCANCODE_NECX(((data[10] << 8) | data[11]), data[9]);
+ *scancode = RC_SCANCODE_NECX(data[11] << 8 | data[10], data[9]);
*toggle = 0;
return 1;
}
diff --git a/drivers/media/rc/keymaps/rc-behold.c b/drivers/media/rc/keymaps/rc-behold.c
index d6519f8..520a96f 100644
--- a/drivers/media/rc/keymaps/rc-behold.c
+++ b/drivers/media/rc/keymaps/rc-behold.c
@@ -30,8 +30,8 @@ static struct rc_map_table behold[] = {
/* 0x1c 0x12 *
* TV/FM POWER *
* */
- { 0x6b861c, KEY_TUNER }, /* XXX KEY_TV / KEY_RADIO */
- { 0x6b8612, KEY_POWER },
+ { 0x866b1c, KEY_TUNER }, /* XXX KEY_TV / KEY_RADIO */
+ { 0x866b12, KEY_POWER },
/* 0x01 0x02 0x03 *
* 1 2 3 *
@@ -42,28 +42,28 @@ static struct rc_map_table behold[] = {
* 0x07 0x08 0x09 *
* 7 8 9 *
* */
- { 0x6b8601, KEY_1 },
- { 0x6b8602, KEY_2 },
- { 0x6b8603, KEY_3 },
- { 0x6b8604, KEY_4 },
- { 0x6b8605, KEY_5 },
- { 0x6b8606, KEY_6 },
- { 0x6b8607, KEY_7 },
- { 0x6b8608, KEY_8 },
- { 0x6b8609, KEY_9 },
+ { 0x866b01, KEY_1 },
+ { 0x866b02, KEY_2 },
+ { 0x866b03, KEY_3 },
+ { 0x866b04, KEY_4 },
+ { 0x866b05, KEY_5 },
+ { 0x866b06, KEY_6 },
+ { 0x866b07, KEY_7 },
+ { 0x866b08, KEY_8 },
+ { 0x866b09, KEY_9 },
/* 0x0a 0x00 0x17 *
* RECALL 0 MODE *
* */
- { 0x6b860a, KEY_AGAIN },
- { 0x6b8600, KEY_0 },
- { 0x6b8617, KEY_MODE },
+ { 0x866b0a, KEY_AGAIN },
+ { 0x866b00, KEY_0 },
+ { 0x866b17, KEY_MODE },
/* 0x14 0x10 *
* ASPECT FULLSCREEN *
* */
- { 0x6b8614, KEY_SCREEN },
- { 0x6b8610, KEY_ZOOM },
+ { 0x866b14, KEY_SCREEN },
+ { 0x866b10, KEY_ZOOM },
/* 0x0b *
* Up *
@@ -74,17 +74,17 @@ static struct rc_map_table behold[] = {
* 0x015 *
* Down *
* */
- { 0x6b860b, KEY_CHANNELUP },
- { 0x6b8618, KEY_VOLUMEDOWN },
- { 0x6b8616, KEY_OK }, /* XXX KEY_ENTER */
- { 0x6b860c, KEY_VOLUMEUP },
- { 0x6b8615, KEY_CHANNELDOWN },
+ { 0x866b0b, KEY_CHANNELUP },
+ { 0x866b18, KEY_VOLUMEDOWN },
+ { 0x866b16, KEY_OK }, /* XXX KEY_ENTER */
+ { 0x866b0c, KEY_VOLUMEUP },
+ { 0x866b15, KEY_CHANNELDOWN },
/* 0x11 0x0d *
* MUTE INFO *
* */
- { 0x6b8611, KEY_MUTE },
- { 0x6b860d, KEY_INFO },
+ { 0x866b11, KEY_MUTE },
+ { 0x866b0d, KEY_INFO },
/* 0x0f 0x1b 0x1a *
* RECORD PLAY/PAUSE STOP *
@@ -93,26 +93,26 @@ static struct rc_map_table behold[] = {
*TELETEXT AUDIO SOURCE *
* RED YELLOW *
* */
- { 0x6b860f, KEY_RECORD },
- { 0x6b861b, KEY_PLAYPAUSE },
- { 0x6b861a, KEY_STOP },
- { 0x6b860e, KEY_TEXT },
- { 0x6b861f, KEY_RED }, /*XXX KEY_AUDIO */
- { 0x6b861e, KEY_VIDEO },
+ { 0x866b0f, KEY_RECORD },
+ { 0x866b1b, KEY_PLAYPAUSE },
+ { 0x866b1a, KEY_STOP },
+ { 0x866b0e, KEY_TEXT },
+ { 0x866b1f, KEY_RED }, /*XXX KEY_AUDIO */
+ { 0x866b1e, KEY_VIDEO },
/* 0x1d 0x13 0x19 *
* SLEEP PREVIEW DVB *
* GREEN BLUE *
* */
- { 0x6b861d, KEY_SLEEP },
- { 0x6b8613, KEY_GREEN },
- { 0x6b8619, KEY_BLUE }, /* XXX KEY_SAT */
+ { 0x866b1d, KEY_SLEEP },
+ { 0x866b13, KEY_GREEN },
+ { 0x866b19, KEY_BLUE }, /* XXX KEY_SAT */
/* 0x58 0x5c *
* FREEZE SNAPSHOT *
* */
- { 0x6b8658, KEY_SLOW },
- { 0x6b865c, KEY_CAMERA },
+ { 0x866b58, KEY_SLOW },
+ { 0x866b5c, KEY_CAMERA },
};
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
` (8 preceding siblings ...)
2014-03-29 16:11 ` [PATCH 09/11] saa7134: NEC scancode fix David Härdeman
@ 2014-03-29 16:11 ` David Härdeman
2014-03-31 9:44 ` James Hogan
2014-03-29 16:11 ` [PATCH 11/11] [RFC] rc-core: don't throw away protocol information David Härdeman
10 siblings, 1 reply; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:11 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
Using the full 32 bits for all kinds of NEC scancodes simplifies rc-core
and the nec decoder without any loss of functionality.
In order to maintain backwards compatibility, some heuristics are added
in rc-main.c to convert scancodes to NEC32 as necessary.
I plan to introduce a different ioctl later which makes the protocol
explicit (and which expects all NEC scancodes to be 32 bit, thereby
removing the need for guesswork).
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/pci/saa7134/saa7134-input.c | 6 +-
drivers/media/rc/img-ir/img-ir-nec.c | 79 ++----------------------
drivers/media/rc/ir-nec-decoder.c | 28 ++-------
drivers/media/rc/keymaps/rc-tivo.c | 95 ++++++++++++++---------------
drivers/media/rc/rc-main.c | 53 ++++++++++++++++
drivers/media/usb/dvb-usb-v2/af9015.c | 22 +------
drivers/media/usb/dvb-usb-v2/af9035.c | 15 +----
drivers/media/usb/dvb-usb-v2/az6007.c | 16 +----
drivers/media/usb/dvb-usb-v2/lmedm04.c | 12 +---
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 16 +----
drivers/media/usb/dvb-usb/dib0700_core.c | 21 ++----
drivers/media/usb/em28xx/em28xx-input.c | 14 +---
include/media/rc-map.h | 16 ++++-
13 files changed, 151 insertions(+), 242 deletions(-)
diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c
index 887429b..4ba61ff 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -342,11 +342,9 @@ static int get_key_beholdm6xx(struct IR_i2c *ir, enum rc_type *protocol,
return -EIO;
}
- if (data[9] != (unsigned char)(~data[8]))
- return 0;
-
*protocol = RC_TYPE_NEC;
- *scancode = RC_SCANCODE_NECX(data[11] << 8 | data[10], data[9]);
+ *scancode = RC_SCANCODE_NEC32(data[11] << 24 | data[10] << 16 |
+ data[9] << 8 | data[8]);
*toggle = 0;
return 1;
}
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c
index 40ee844..133ea45 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -5,42 +5,20 @@
*/
#include "img-ir-hw.h"
-#include <linux/bitrev.h>
/* Convert NEC data to a scancode */
static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
u32 *scancode, u64 enabled_protocols)
{
- unsigned int addr, addr_inv, data, data_inv;
/* a repeat code has no data */
if (!len)
return IMG_IR_REPEATCODE;
+
if (len != 32)
return -EINVAL;
- /* raw encoding: ddDDaaAA */
- addr = (raw >> 0) & 0xff;
- addr_inv = (raw >> 8) & 0xff;
- data = (raw >> 16) & 0xff;
- data_inv = (raw >> 24) & 0xff;
- if ((data_inv ^ data) != 0xff) {
- /* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: AAaaDDdd (LSBit first) */
- *scancode = bitrev8(addr) << 24 |
- bitrev8(addr_inv) << 16 |
- bitrev8(data) << 8 |
- bitrev8(data_inv);
- } else if ((addr_inv ^ addr) != 0xff) {
- /* Extended NEC */
- /* scan encoding: AAaaDD */
- *scancode = addr << 16 |
- addr_inv << 8 |
- data;
- } else {
- /* Normal NEC */
- /* scan encoding: AADD */
- *scancode = addr << 8 |
- data;
- }
+
+ /* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
+ *scancode = swab32((u32)raw);
*protocol = RC_TYPE_NEC;
return IMG_IR_SCANCODE;
}
@@ -49,52 +27,9 @@ static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
static int img_ir_nec_filter(const struct rc_scancode_filter *in,
struct img_ir_filter *out, u64 protocols)
{
- unsigned int addr, addr_inv, data, data_inv;
- unsigned int addr_m, addr_inv_m, data_m, data_inv_m;
-
- data = in->data & 0xff;
- data_m = in->mask & 0xff;
-
- if ((in->data | in->mask) & 0xff000000) {
- /* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: AAaaDDdd (LSBit first) */
- addr = bitrev8((in->data >> 24) & 0xff);
- addr_m = (in->mask >> 24) & 0xff;
- addr_inv = bitrev8((in->data >> 16) & 0xff);
- addr_inv_m = (in->mask >> 16) & 0xff;
- data = bitrev8((in->data >> 8) & 0xff);
- data_m = (in->mask >> 8) & 0xff;
- data_inv = bitrev8((in->data >> 0) & 0xff);
- data_inv_m = (in->mask >> 0) & 0xff;
- } else if ((in->data | in->mask) & 0x00ff0000) {
- /* Extended NEC */
- /* scan encoding AAaaDD */
- addr = (in->data >> 16) & 0xff;
- addr_m = (in->mask >> 16) & 0xff;
- addr_inv = (in->data >> 8) & 0xff;
- addr_inv_m = (in->mask >> 8) & 0xff;
- data_inv = data ^ 0xff;
- data_inv_m = data_m;
- } else {
- /* Normal NEC */
- /* scan encoding: AADD */
- addr = (in->data >> 8) & 0xff;
- addr_m = (in->mask >> 8) & 0xff;
- addr_inv = addr ^ 0xff;
- addr_inv_m = addr_m;
- data_inv = data ^ 0xff;
- data_inv_m = data_m;
- }
-
- /* raw encoding: ddDDaaAA */
- out->data = data_inv << 24 |
- data << 16 |
- addr_inv << 8 |
- addr;
- out->mask = data_inv_m << 24 |
- data_m << 16 |
- addr_inv_m << 8 |
- addr_m;
+ /* scan encoding: AAaaDDdd -> raw encoding: ddDDaaAA */
+ out->data = swab32((u32)in->data);
+ out->mask = swab32((u32)in->mask);
return 0;
}
diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index c4333d5..798e32b 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -50,7 +50,6 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
struct nec_dec *data = &dev->raw->nec;
u32 scancode;
u8 address, not_address, command, not_command;
- bool send_32bits = false;
if (!rc_protocols_enabled(dev, RC_BIT_NEC))
return 0;
@@ -163,28 +162,11 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
command = bitrev8((data->bits >> 8) & 0xff);
not_command = bitrev8((data->bits >> 0) & 0xff);
- if ((command ^ not_command) != 0xff) {
- IR_dprintk(1, "NEC checksum error: received 0x%08x\n",
- data->bits);
- send_32bits = true;
- }
-
- if (send_32bits) {
- /* NEC transport, but modified protocol, used by at
- * least Apple and TiVo remotes */
- scancode = data->bits;
- IR_dprintk(1, "NEC (modified) scancode 0x%08x\n", scancode);
- } else if ((address ^ not_address) != 0xff) {
- /* Extended NEC */
- scancode = address << 16 |
- not_address << 8 |
- command;
- IR_dprintk(1, "NEC (Ext) scancode 0x%06x\n", scancode);
- } else {
- /* Normal NEC */
- scancode = address << 8 | command;
- IR_dprintk(1, "NEC scancode 0x%04x\n", scancode);
- }
+ scancode = RC_SCANCODE_NEC32(address << 24 |
+ not_address << 16 |
+ command << 8 |
+ not_command);
+ IR_dprintk(1, "NEC scancode 0x%08x\n", scancode);
if (data->is_nec_x)
data->necx_repeat = true;
diff --git a/drivers/media/rc/keymaps/rc-tivo.c b/drivers/media/rc/keymaps/rc-tivo.c
index 454e062..63ed7ad 100644
--- a/drivers/media/rc/keymaps/rc-tivo.c
+++ b/drivers/media/rc/keymaps/rc-tivo.c
@@ -15,62 +15,61 @@
* Initial mapping is for the TiVo remote included in the Nero LiquidTV bundle,
* which also ships with a TiVo-branded IR transceiver, supported by the mceusb
* driver. Note that the remote uses an NEC-ish protocol, but instead of having
- * a command/not_command pair, it has a vendor ID of 0xa10c, but some keys, the
- * NEC extended checksums do pass, so the table presently has the intended
- * values and the checksum-passed versions for those keys.
+ * a command/not_command pair, it has a vendor ID of 0x8530, but some keys, the
+ * NEC extended checksums do pass, so the table has the full code for all keys.
*/
static struct rc_map_table tivo[] = {
- { 0xa10c900f, KEY_MEDIA }, /* TiVo Button */
- { 0xa10c0807, KEY_POWER2 }, /* TV Power */
- { 0xa10c8807, KEY_TV }, /* Live TV/Swap */
- { 0xa10c2c03, KEY_VIDEO_NEXT }, /* TV Input */
- { 0xa10cc807, KEY_INFO },
- { 0xa10cfa05, KEY_CYCLEWINDOWS }, /* Window */
- { 0x0085305f, KEY_CYCLEWINDOWS },
- { 0xa10c6c03, KEY_EPG }, /* Guide */
+ { 0x853009f0, KEY_MEDIA }, /* TiVo Button */
+ { 0x853010e0, KEY_POWER2 }, /* TV Power */
+ { 0x853011e0, KEY_TV }, /* Live TV/Swap */
+ { 0x853034c0, KEY_VIDEO_NEXT }, /* TV Input */
+ { 0x853013e0, KEY_INFO },
+ { 0x85305fa0, KEY_CYCLEWINDOWS }, /* Window */
+ { 0x85005f30, KEY_CYCLEWINDOWS },
+ { 0x853036c0, KEY_EPG }, /* Guide */
- { 0xa10c2807, KEY_UP },
- { 0xa10c6807, KEY_DOWN },
- { 0xa10ce807, KEY_LEFT },
- { 0xa10ca807, KEY_RIGHT },
+ { 0x853014e0, KEY_UP },
+ { 0x853016e0, KEY_DOWN },
+ { 0x853017e0, KEY_LEFT },
+ { 0x853015e0, KEY_RIGHT },
- { 0xa10c1807, KEY_SCROLLDOWN }, /* Red Thumbs Down */
- { 0xa10c9807, KEY_SELECT },
- { 0xa10c5807, KEY_SCROLLUP }, /* Green Thumbs Up */
+ { 0x853018e0, KEY_SCROLLDOWN }, /* Red Thumbs Down */
+ { 0x853019e0, KEY_SELECT },
+ { 0x85301ae0, KEY_SCROLLUP }, /* Green Thumbs Up */
- { 0xa10c3807, KEY_VOLUMEUP },
- { 0xa10cb807, KEY_VOLUMEDOWN },
- { 0xa10cd807, KEY_MUTE },
- { 0xa10c040b, KEY_RECORD },
- { 0xa10c7807, KEY_CHANNELUP },
- { 0xa10cf807, KEY_CHANNELDOWN },
- { 0x0085301f, KEY_CHANNELDOWN },
+ { 0x85301ce0, KEY_VOLUMEUP },
+ { 0x85301de0, KEY_VOLUMEDOWN },
+ { 0x85301be0, KEY_MUTE },
+ { 0x853020d0, KEY_RECORD },
+ { 0x85301ee0, KEY_CHANNELUP },
+ { 0x85301fe0, KEY_CHANNELDOWN },
+ { 0x85001f30, KEY_CHANNELDOWN },
- { 0xa10c840b, KEY_PLAY },
- { 0xa10cc40b, KEY_PAUSE },
- { 0xa10ca40b, KEY_SLOW },
- { 0xa10c440b, KEY_REWIND },
- { 0xa10c240b, KEY_FASTFORWARD },
- { 0xa10c640b, KEY_PREVIOUS },
- { 0xa10ce40b, KEY_NEXT }, /* ->| */
+ { 0x853021d0, KEY_PLAY },
+ { 0x853023d0, KEY_PAUSE },
+ { 0x853025d0, KEY_SLOW },
+ { 0x853022d0, KEY_REWIND },
+ { 0x853024d0, KEY_FASTFORWARD },
+ { 0x853026d0, KEY_PREVIOUS },
+ { 0x853027d0, KEY_NEXT }, /* ->| */
- { 0xa10c220d, KEY_ZOOM }, /* Aspect */
- { 0xa10c120d, KEY_STOP },
- { 0xa10c520d, KEY_DVD }, /* DVD Menu */
+ { 0x853044b0, KEY_ZOOM }, /* Aspect */
+ { 0x853048b0, KEY_STOP },
+ { 0x85304ab0, KEY_DVD }, /* DVD Menu */
- { 0xa10c140b, KEY_NUMERIC_1 },
- { 0xa10c940b, KEY_NUMERIC_2 },
- { 0xa10c540b, KEY_NUMERIC_3 },
- { 0xa10cd40b, KEY_NUMERIC_4 },
- { 0xa10c340b, KEY_NUMERIC_5 },
- { 0xa10cb40b, KEY_NUMERIC_6 },
- { 0xa10c740b, KEY_NUMERIC_7 },
- { 0xa10cf40b, KEY_NUMERIC_8 },
- { 0x0085302f, KEY_NUMERIC_8 },
- { 0xa10c0c03, KEY_NUMERIC_9 },
- { 0xa10c8c03, KEY_NUMERIC_0 },
- { 0xa10ccc03, KEY_ENTER },
- { 0xa10c4c03, KEY_CLEAR },
+ { 0x853028d0, KEY_NUMERIC_1 },
+ { 0x853029d0, KEY_NUMERIC_2 },
+ { 0x85302ad0, KEY_NUMERIC_3 },
+ { 0x85302bd0, KEY_NUMERIC_4 },
+ { 0x85302cd0, KEY_NUMERIC_5 },
+ { 0x85302dd0, KEY_NUMERIC_6 },
+ { 0x85302ed0, KEY_NUMERIC_7 },
+ { 0x85302fd0, KEY_NUMERIC_8 },
+ { 0x85002f30, KEY_NUMERIC_8 },
+ { 0x853030c0, KEY_NUMERIC_9 },
+ { 0x853031c0, KEY_NUMERIC_0 },
+ { 0x853033c0, KEY_ENTER },
+ { 0x853032c0, KEY_CLEAR },
};
static struct rc_map_list tivo_map = {
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 8675e07..59bc44f 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -316,6 +316,49 @@ static unsigned int ir_establish_scancode(struct rc_dev *dev,
}
/**
+ * guess_protocol() - heuristics to guess the protocol for a scancode
+ * @rdev: the struct rc_dev device descriptor
+ * @return: the guessed RC_TYPE_* protocol
+ *
+ * Internal routine to guess the current IR protocol for legacy ioctls.
+ */
+static inline enum rc_type guess_protocol(struct rc_dev *rdev)
+{
+ struct rc_map *rc_map = &rdev->rc_map;
+
+ if (hweight64(rdev->enabled_protocols[RC_FILTER_NORMAL]) == 1)
+ return rc_bitmap_to_type(rdev->enabled_protocols[RC_FILTER_NORMAL]);
+ else if (hweight64(rdev->allowed_protocols[RC_FILTER_NORMAL]) == 1)
+ return rc_bitmap_to_type(rdev->allowed_protocols[RC_FILTER_NORMAL]);
+ else
+ return rc_map->rc_type;
+}
+
+/**
+ * to_nec32() - helper function to try to convert misc NEC scancodes to NEC32
+ * @orig: original scancode
+ * @return: NEC32 scancode
+ *
+ * This helper routine is used to provide backwards compatibility.
+ */
+static u32 to_nec32(u32 orig)
+{
+ u8 b3 = (u8)(orig >> 16);
+ u8 b2 = (u8)(orig >> 8);
+ u8 b1 = (u8)(orig >> 0);
+
+ if (orig <= 0xffff)
+ /* Plain old NEC */
+ return b2 << 24 | ((u8)~b2) << 16 | b1 << 8 | ((u8)~b1);
+ else if (orig <= 0xffffff)
+ /* NEC extended */
+ return b3 << 24 | b2 << 16 | b1 << 8 | ((u8)~b1);
+ else
+ /* NEC32 */
+ return orig;
+}
+
+/**
* ir_setkeycode() - set a keycode in the scancode->keycode table
* @idev: the struct input_dev device descriptor
* @scancode: the desired scancode
@@ -348,6 +391,9 @@ static int ir_setkeycode(struct input_dev *idev,
if (retval)
goto out;
+ if (guess_protocol(rdev) == RC_TYPE_NEC)
+ scancode = to_nec32(scancode);
+
index = ir_establish_scancode(rdev, rc_map, scancode, true);
if (index >= rc_map->len) {
retval = -ENOMEM;
@@ -388,7 +434,10 @@ static int ir_setkeytable(struct rc_dev *dev,
for (i = 0; i < from->size; i++) {
index = ir_establish_scancode(dev, rc_map,
- from->scan[i].scancode, false);
+ from->rc_type == RC_TYPE_NEC ?
+ to_nec32(from->scan[i].scancode) :
+ from->scan[i].scancode,
+ false);
if (index >= rc_map->len) {
rc = -ENOMEM;
break;
@@ -462,6 +511,8 @@ static int ir_getkeycode(struct input_dev *idev,
if (retval)
goto out;
+ if (guess_protocol(rdev) == RC_TYPE_NEC)
+ scancode = to_nec32(scancode);
index = ir_lookup_by_scancode(rc_map, scancode);
}
diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c
index 5ca738a..8776eaf 100644
--- a/drivers/media/usb/dvb-usb-v2/af9015.c
+++ b/drivers/media/usb/dvb-usb-v2/af9015.c
@@ -1230,24 +1230,10 @@ static int af9015_rc_query(struct dvb_usb_device *d)
/* Remember this key */
memcpy(state->rc_last, &buf[12], 4);
- if (buf[14] == (u8) ~buf[15]) {
- if (buf[12] == (u8) ~buf[13]) {
- /* NEC */
- state->rc_keycode = RC_SCANCODE_NEC(buf[12],
- buf[14]);
- } else {
- /* NEC extended*/
- state->rc_keycode = RC_SCANCODE_NECX(buf[12] << 8 |
- buf[13],
- buf[14]);
- }
- } else {
- /* 32 bit NEC */
- state->rc_keycode = RC_SCANCODE_NEC32(buf[12] << 24 |
- buf[13] << 16 |
- buf[14] << 8 |
- buf[15]);
- }
+ state->rc_keycode = RC_SCANCODE_NEC32(buf[12] << 24 |
+ buf[13] << 16 |
+ buf[14] << 8 |
+ buf[15]);
rc_keydown(d->rc_dev, RC_TYPE_NEC, state->rc_keycode, 0);
} else {
dev_dbg(&d->udev->dev, "%s: no key press\n", __func__);
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3bfba13..6bc9693 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1284,19 +1284,8 @@ static int af9035_rc_query(struct dvb_usb_device *d)
else if (ret < 0)
goto err;
- if ((buf[2] + buf[3]) == 0xff) {
- if ((buf[0] + buf[1]) == 0xff) {
- /* NEC standard 16bit */
- key = RC_SCANCODE_NEC(buf[0], buf[2]);
- } else {
- /* NEC extended 24bit */
- key = RC_SCANCODE_NECX(buf[0] << 8 | buf[1], buf[2]);
- }
- } else {
- /* NEC full code 32bit */
- key = RC_SCANCODE_NEC32(buf[0] << 24 | buf[1] << 16 |
- buf[2] << 8 | buf[3]);
- }
+ key = RC_SCANCODE_NEC32(buf[0] << 24 | buf[1] << 16 |
+ buf[2] << 8 | buf[3]);
dev_dbg(&d->udev->dev, "%s: %*ph\n", __func__, 4, buf);
diff --git a/drivers/media/usb/dvb-usb-v2/az6007.c b/drivers/media/usb/dvb-usb-v2/az6007.c
index 935dbaa..7e38278 100644
--- a/drivers/media/usb/dvb-usb-v2/az6007.c
+++ b/drivers/media/usb/dvb-usb-v2/az6007.c
@@ -214,18 +214,10 @@ static int az6007_rc_query(struct dvb_usb_device *d)
if (st->data[1] == 0x44)
return 0;
- if ((st->data[3] ^ st->data[4]) == 0xff) {
- if ((st->data[1] ^ st->data[2]) == 0xff)
- code = RC_SCANCODE_NEC(st->data[1], st->data[3]);
- else
- code = RC_SCANCODE_NECX(st->data[1] << 8 | st->data[2],
- st->data[3]);
- } else {
- code = RC_SCANCODE_NEC32(st->data[1] << 24 |
- st->data[2] << 16 |
- st->data[3] << 8 |
- st->data[4]);
- }
+ code = RC_SCANCODE_NEC32(st->data[1] << 24 |
+ st->data[2] << 16 |
+ st->data[3] << 8 |
+ st->data[4]);
rc_keydown(d->rc_dev, RC_TYPE_NEC, code, st->data[5]);
diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c
index 6e3ca72..c78833e 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -294,14 +294,10 @@ static void lme2510_int_response(struct urb *lme_urb)
ibuf[4] ^= 0xff;
ibuf[5] ^= 0xff;
- if (ibuf[4] ^ ibuf[5] == 0xff)
- key = RC_SCANCODE_NECX(ibuf[2] << 8 | ibuf[3],
- ibuf[4]);
- else
- key = RC_SCANCODE_NEC32(ibuf[2] << 24 |
- ibuf[3] << 16 |
- ibuf[4] << 8 |
- ibuf[5]);
+ key = RC_SCANCODE_NEC32(ibuf[2] << 24 |
+ ibuf[3] << 16 |
+ ibuf[4] << 8 |
+ ibuf[5]);
deb_info(1, "INT Key =%08x", key);
rc_keydown(adap_to_d(adap)->rc_dev, RC_TYPE_NEC, key, 0);
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index 574f4ee..45c77b1 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1246,20 +1246,8 @@ static int rtl2831u_rc_query(struct dvb_usb_device *d)
goto err;
if (buf[4] & 0x01) {
- if (buf[2] == (u8) ~buf[3]) {
- if (buf[0] == (u8) ~buf[1]) {
- /* NEC standard (16 bit) */
- rc_code = RC_SCANCODE_NEC(buf[0], buf[2]);
- } else {
- /* NEC extended (24 bit) */
- rc_code = RC_SCANCODE_NECX(buf[0] << 8 | buf[1],
- buf[2]);
- }
- } else {
- /* NEC full (32 bit) */
- rc_code = RC_SCANCODE_NEC32(buf[0] << 24 | buf[1] << 16 |
- buf[2] << 8 | buf[3]);
- }
+ rc_code = RC_SCANCODE_NEC32(buf[0] << 24 | buf[1] << 16 |
+ buf[2] << 8 | buf[3]);
rc_keydown(d->rc_dev, RC_TYPE_NEC, rc_code, 0);
diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index 4f5caf5..d7b7932 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -715,22 +715,11 @@ static void dib0700_rc_urb_completion(struct urb *purb)
break;
}
- if (poll_reply->data ^ poll_reply->not_data != 0xff) {
- deb_data("NEC32 protocol\n");
- scancode = RC_SCANCODE_NEC32(poll_reply->system << 24 |
- poll_reply->not_system << 16 |
- poll_reply->data << 8 |
- poll_reply->not_data);
- } else if (poll_reply->system ^ poll_reply->not_system != 0xff) {
- deb_data("NEC extended protocol\n");
- scancode = RC_SCANCODE_NECX(poll_reply->system << 8 |
- poll_reply->not_system,
- poll_reply->data);
- } else {
- deb_data("NEC normal protocol\n");
- scancode = RC_SCANCODE_NEC(poll_reply->system,
- poll_reply->data);
- }
+ deb_data("NEC protocol\n");
+ scancode = RC_SCANCODE_NEC32(poll_reply->system << 24 |
+ poll_reply->not_system << 16 |
+ poll_reply->data << 8 |
+ poll_reply->not_data);
break;
default:
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index 014888f..4bbd8e4 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -261,16 +261,10 @@ static int em2874_polling_getkey(struct em28xx_IR *ir,
case RC_BIT_NEC:
poll_result->protocol = RC_TYPE_RC5;
poll_result->scancode = msg[1] << 8 | msg[2];
- if ((msg[3] ^ msg[4]) != 0xff) /* 32 bits NEC */
- poll_result->scancode = RC_SCANCODE_NEC32((msg[1] << 24) |
- (msg[2] << 16) |
- (msg[3] << 8) |
- (msg[4]));
- else if ((msg[1] ^ msg[2]) != 0xff) /* 24 bits NEC */
- poll_result->scancode = RC_SCANCODE_NECX(msg[1] << 8 |
- msg[2], msg[3]);
- else /* Normal NEC */
- poll_result->scancode = RC_SCANCODE_NEC(msg[1], msg[3]);
+ poll_result->scancode = RC_SCANCODE_NEC32((msg[1] << 24) |
+ (msg[2] << 16) |
+ (msg[3] << 8) |
+ (msg[4]));
break;
case RC_BIT_RC6_0:
diff --git a/include/media/rc-map.h b/include/media/rc-map.h
index 894c7e4..2e6c659 100644
--- a/include/media/rc-map.h
+++ b/include/media/rc-map.h
@@ -33,6 +33,8 @@ enum rc_type {
RC_TYPE_SHARP = 18, /* Sharp protocol */
};
+#define rc_bitmap_to_type(x) fls64(x)
+
#define RC_BIT_NONE 0
#define RC_BIT_UNKNOWN (1 << RC_TYPE_UNKNOWN)
#define RC_BIT_OTHER (1 << RC_TYPE_OTHER)
@@ -64,13 +66,21 @@ enum rc_type {
#define RC_SCANCODE_UNKNOWN(x) (x)
#define RC_SCANCODE_OTHER(x) (x)
-#define RC_SCANCODE_NEC(addr, cmd) (((addr) << 8) | (cmd))
-#define RC_SCANCODE_NECX(addr, cmd) (((addr) << 8) | (cmd))
-#define RC_SCANCODE_NEC32(data) ((data) & 0xffffffff)
#define RC_SCANCODE_RC5(sys, cmd) (((sys) << 8) | (cmd))
#define RC_SCANCODE_RC5_SZ(sys, cmd) (((sys) << 8) | (cmd))
#define RC_SCANCODE_RC6_0(sys, cmd) (((sys) << 8) | (cmd))
#define RC_SCANCODE_RC6_6A(vendor, sys, cmd) (((vendor) << 16) | ((sys) << 8) | (cmd))
+#define RC_SCANCODE_NEC(addr, cmd) \
+ ((( (addr) & 0xff) << 24) | \
+ ((~(addr) & 0xff) << 16) | \
+ (( (cmd) & 0xff) << 8 ) | \
+ ((~(cmd) & 0xff) << 0 ))
+#define RC_SCANCODE_NECX(addr, cmd) \
+ ((( (addr) & 0xffff) << 16) | \
+ (( (cmd) & 0x00ff) << 8) | \
+ ((~(cmd) & 0x00ff) << 0))
+#define RC_SCANCODE_NEC32(data) ((data) & 0xffffffff)
+
struct rc_map_table {
u32 scancode;
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-29 16:11 ` [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes David Härdeman
@ 2014-03-31 9:44 ` James Hogan
2014-03-31 10:19 ` David Härdeman
0 siblings, 1 reply; 30+ messages in thread
From: James Hogan @ 2014-03-31 9:44 UTC (permalink / raw)
To: David Härdeman; +Cc: linux-media, m.chehab
[-- Attachment #1: Type: text/plain, Size: 2323 bytes --]
On 29/03/14 16:11, David Härdeman wrote:
> Using the full 32 bits for all kinds of NEC scancodes simplifies rc-core
> and the nec decoder without any loss of functionality.
>
> In order to maintain backwards compatibility, some heuristics are added
> in rc-main.c to convert scancodes to NEC32 as necessary.
>
> I plan to introduce a different ioctl later which makes the protocol
> explicit (and which expects all NEC scancodes to be 32 bit, thereby
> removing the need for guesswork).
>
> Signed-off-by: David Härdeman <david@hardeman.nu>
> ---
> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c
> index 40ee844..133ea45 100644
> --- a/drivers/media/rc/img-ir/img-ir-nec.c
> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
> @@ -5,42 +5,20 @@
> */
>
> #include "img-ir-hw.h"
> -#include <linux/bitrev.h>
>
> /* Convert NEC data to a scancode */
> static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
> u32 *scancode, u64 enabled_protocols)
> {
> - unsigned int addr, addr_inv, data, data_inv;
> /* a repeat code has no data */
> if (!len)
> return IMG_IR_REPEATCODE;
> +
> if (len != 32)
> return -EINVAL;
> - /* raw encoding: ddDDaaAA */
> - addr = (raw >> 0) & 0xff;
> - addr_inv = (raw >> 8) & 0xff;
> - data = (raw >> 16) & 0xff;
> - data_inv = (raw >> 24) & 0xff;
> - if ((data_inv ^ data) != 0xff) {
> - /* 32-bit NEC (used by Apple and TiVo remotes) */
> - /* scan encoding: AAaaDDdd (LSBit first) */
> - *scancode = bitrev8(addr) << 24 |
> - bitrev8(addr_inv) << 16 |
> - bitrev8(data) << 8 |
> - bitrev8(data_inv);
> - } else if ((addr_inv ^ addr) != 0xff) {
> - /* Extended NEC */
> - /* scan encoding: AAaaDD */
> - *scancode = addr << 16 |
> - addr_inv << 8 |
> - data;
> - } else {
> - /* Normal NEC */
> - /* scan encoding: AADD */
> - *scancode = addr << 8 |
> - data;
> - }
> +
> + /* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
> + *scancode = swab32((u32)raw);
What's the point of the byte swapping?
Surely the most natural NEC encoding would just treat it as a single
32-bit (LSBit first) field rather than 4 8-bit fields that needs swapping.
Cheers
James
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 9:44 ` James Hogan
@ 2014-03-31 10:19 ` David Härdeman
2014-03-31 10:56 ` James Hogan
2014-03-31 12:14 ` Mauro Carvalho Chehab
0 siblings, 2 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-31 10:19 UTC (permalink / raw)
To: James Hogan; +Cc: linux-media, m.chehab
On 2014-03-31 11:44, James Hogan wrote:
> On 29/03/14 16:11, David Härdeman wrote:
>> Using the full 32 bits for all kinds of NEC scancodes simplifies
>> rc-core
>> and the nec decoder without any loss of functionality.
>>
>> In order to maintain backwards compatibility, some heuristics are
>> added
>> in rc-main.c to convert scancodes to NEC32 as necessary.
>>
>> I plan to introduce a different ioctl later which makes the protocol
>> explicit (and which expects all NEC scancodes to be 32 bit, thereby
>> removing the need for guesswork).
>>
>> Signed-off-by: David Härdeman <david@hardeman.nu>
>> ---
>> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
>> b/drivers/media/rc/img-ir/img-ir-nec.c
>> index 40ee844..133ea45 100644
>> --- a/drivers/media/rc/img-ir/img-ir-nec.c
>> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
>> @@ -5,42 +5,20 @@
>> */
>>
>> #include "img-ir-hw.h"
>> -#include <linux/bitrev.h>
>>
>> /* Convert NEC data to a scancode */
>> static int img_ir_nec_scancode(int len, u64 raw, enum rc_type
>> *protocol,
>> u32 *scancode, u64 enabled_protocols)
>> {
>> - unsigned int addr, addr_inv, data, data_inv;
>> /* a repeat code has no data */
>> if (!len)
>> return IMG_IR_REPEATCODE;
>> +
>> if (len != 32)
>> return -EINVAL;
>> - /* raw encoding: ddDDaaAA */
>> - addr = (raw >> 0) & 0xff;
>> - addr_inv = (raw >> 8) & 0xff;
>> - data = (raw >> 16) & 0xff;
>> - data_inv = (raw >> 24) & 0xff;
>> - if ((data_inv ^ data) != 0xff) {
>> - /* 32-bit NEC (used by Apple and TiVo remotes) */
>> - /* scan encoding: AAaaDDdd (LSBit first) */
>> - *scancode = bitrev8(addr) << 24 |
>> - bitrev8(addr_inv) << 16 |
>> - bitrev8(data) << 8 |
>> - bitrev8(data_inv);
>> - } else if ((addr_inv ^ addr) != 0xff) {
>> - /* Extended NEC */
>> - /* scan encoding: AAaaDD */
>> - *scancode = addr << 16 |
>> - addr_inv << 8 |
>> - data;
>> - } else {
>> - /* Normal NEC */
>> - /* scan encoding: AADD */
>> - *scancode = addr << 8 |
>> - data;
>> - }
>> +
>> + /* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
>> + *scancode = swab32((u32)raw);
>
> What's the point of the byte swapping?
>
> Surely the most natural NEC encoding would just treat it as a single
> 32-bit (LSBit first) field rather than 4 8-bit fields that needs
> swapping.
Thanks for having a look at the patches, I agree with your comments on
the other patches (and I have to respin some of them because I missed
two drivers), but the comments to this patch confuses me a bit.
That the NEC data is transmitted as 32 bits encoded with LSB bit order
within each byte is AFAIK just about the only thing that all
sources/documentation of the protocal can agree on (so bitrev:ing the
bits within each byte makes sense, unless the hardware has done it
already).
As for the byte order, AAaaDDdd corresponds to the transmission order
and seems to be what most drivers expect/use for their RX data.
Are you suggesting that rc-core should standardize on ddDDaaAA order?
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 10:19 ` David Härdeman
@ 2014-03-31 10:56 ` James Hogan
2014-03-31 13:22 ` David Härdeman
2014-03-31 12:14 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 30+ messages in thread
From: James Hogan @ 2014-03-31 10:56 UTC (permalink / raw)
To: David Härdeman; +Cc: linux-media, m.chehab
[-- Attachment #1: Type: text/plain, Size: 4453 bytes --]
On 31/03/14 11:19, David Härdeman wrote:
> On 2014-03-31 11:44, James Hogan wrote:
>> On 29/03/14 16:11, David Härdeman wrote:
>>> Using the full 32 bits for all kinds of NEC scancodes simplifies rc-core
>>> and the nec decoder without any loss of functionality.
>>>
>>> In order to maintain backwards compatibility, some heuristics are added
>>> in rc-main.c to convert scancodes to NEC32 as necessary.
>>>
>>> I plan to introduce a different ioctl later which makes the protocol
>>> explicit (and which expects all NEC scancodes to be 32 bit, thereby
>>> removing the need for guesswork).
>>>
>>> Signed-off-by: David Härdeman <david@hardeman.nu>
>>> ---
>>> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
>>> b/drivers/media/rc/img-ir/img-ir-nec.c
>>> index 40ee844..133ea45 100644
>>> --- a/drivers/media/rc/img-ir/img-ir-nec.c
>>> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
>>> @@ -5,42 +5,20 @@
>>> */
>>>
>>> #include "img-ir-hw.h"
>>> -#include <linux/bitrev.h>
>>>
>>> /* Convert NEC data to a scancode */
>>> static int img_ir_nec_scancode(int len, u64 raw, enum rc_type
>>> *protocol,
>>> u32 *scancode, u64 enabled_protocols)
>>> {
>>> - unsigned int addr, addr_inv, data, data_inv;
>>> /* a repeat code has no data */
>>> if (!len)
>>> return IMG_IR_REPEATCODE;
>>> +
>>> if (len != 32)
>>> return -EINVAL;
>>> - /* raw encoding: ddDDaaAA */
>>> - addr = (raw >> 0) & 0xff;
>>> - addr_inv = (raw >> 8) & 0xff;
>>> - data = (raw >> 16) & 0xff;
>>> - data_inv = (raw >> 24) & 0xff;
>>> - if ((data_inv ^ data) != 0xff) {
>>> - /* 32-bit NEC (used by Apple and TiVo remotes) */
>>> - /* scan encoding: AAaaDDdd (LSBit first) */
>>> - *scancode = bitrev8(addr) << 24 |
>>> - bitrev8(addr_inv) << 16 |
>>> - bitrev8(data) << 8 |
>>> - bitrev8(data_inv);
>>> - } else if ((addr_inv ^ addr) != 0xff) {
>>> - /* Extended NEC */
>>> - /* scan encoding: AAaaDD */
>>> - *scancode = addr << 16 |
>>> - addr_inv << 8 |
>>> - data;
>>> - } else {
>>> - /* Normal NEC */
>>> - /* scan encoding: AADD */
>>> - *scancode = addr << 8 |
>>> - data;
>>> - }
>>> +
>>> + /* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
>>> + *scancode = swab32((u32)raw);
>>
>> What's the point of the byte swapping?
>>
>> Surely the most natural NEC encoding would just treat it as a single
>> 32-bit (LSBit first) field rather than 4 8-bit fields that needs
>> swapping.
>
> Thanks for having a look at the patches, I agree with your comments on
> the other patches (and I have to respin some of them because I missed
> two drivers), but the comments to this patch confuses me a bit.
>
> That the NEC data is transmitted as 32 bits encoded with LSB bit order
> within each byte is AFAIK just about the only thing that all
> sources/documentation of the protocal can agree on (so bitrev:ing the
> bits within each byte makes sense, unless the hardware has done it
> already).
Agreed (in the case of img-ir there's a bit orientation setting which
ensures that the u64 raw has the correct bit order, in the case of NEC
the first bit received goes in the lowest order bit of the raw data).
> As for the byte order, AAaaDDdd corresponds to the transmission order
> and seems to be what most drivers expect/use for their RX data.
AAaaDDdd is big endian rendering, no? (like "%08x")
If it should be interpreted as LSBit first, then the first bits received
should go in the low bits of the scancode, and by extension the first
bytes received in the low bytes of the scancode, i.e. at the end of the
inherently big-endian hexadecimal rendering of the scancode.
> Are you suggesting that rc-core should standardize on ddDDaaAA order?
Yes (where ddDDaaAA means something like scancode
"0x(~cmd)(cmd)(~addr)(addr)")
This would mean that if the data is put in the right bit order (first
bit received in BIT(0), last bit received in BIT(31)), then the scancode
= raw, and if the data is received in the reverse bit order (like the
raw decoder, shifting the data left and inserting the last bit in
BIT(0)) then the scancode = bitrev32(raw).
Have I missed something?
Cheers
James
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 10:56 ` James Hogan
@ 2014-03-31 13:22 ` David Härdeman
2014-03-31 14:06 ` James Hogan
2014-03-31 15:26 ` Mauro Carvalho Chehab
0 siblings, 2 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-31 13:22 UTC (permalink / raw)
To: James Hogan; +Cc: linux-media, m.chehab
On 2014-03-31 12:56, James Hogan wrote:
> On 31/03/14 11:19, David Härdeman wrote:
>> On 2014-03-31 11:44, James Hogan wrote:
>>> On 29/03/14 16:11, David Härdeman wrote:
>>>> + /* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
>>>> + *scancode = swab32((u32)raw);
>>>
>>> What's the point of the byte swapping?
>>>
>>> Surely the most natural NEC encoding would just treat it as a single
>>> 32-bit (LSBit first) field rather than 4 8-bit fields that needs
>>> swapping.
>>
>> Thanks for having a look at the patches, I agree with your comments on
>> the other patches (and I have to respin some of them because I missed
>> two drivers), but the comments to this patch confuses me a bit.
>>
>> That the NEC data is transmitted as 32 bits encoded with LSB bit order
>> within each byte is AFAIK just about the only thing that all
>> sources/documentation of the protocal can agree on (so bitrev:ing the
>> bits within each byte makes sense, unless the hardware has done it
>> already).
>
> Agreed (in the case of img-ir there's a bit orientation setting which
> ensures that the u64 raw has the correct bit order, in the case of NEC
> the first bit received goes in the lowest order bit of the raw data).
>
>> As for the byte order, AAaaDDdd corresponds to the transmission order
>> and seems to be what most drivers expect/use for their RX data.
>
> AAaaDDdd is big endian rendering, no? (like "%08x")
Yeah, you could call it that.
> If it should be interpreted as LSBit first, then the first bits
> received
> should go in the low bits of the scancode, and by extension the first
> bytes received in the low bytes of the scancode, i.e. at the end of the
> inherently big-endian hexadecimal rendering of the scancode.
I'm not saying the whole scancode should be interpreted as one 32 bit
LSBit integer, just that the endianness within each byte should be
respected.
>> Are you suggesting that rc-core should standardize on ddDDaaAA order?
>
> Yes (where ddDDaaAA means something like scancode
> "0x(~cmd)(cmd)(~addr)(addr)")
Yes, that's what I meant.
> This would mean that if the data is put in the right bit order (first
> bit received in BIT(0), last bit received in BIT(31)), then the
> scancode
> = raw, and if the data is received in the reverse bit order (like the
> raw decoder, shifting the data left and inserting the last bit in
> BIT(0)) then the scancode = bitrev32(raw).
>
> Have I missed something?
I just think we have to agree to disagree :)
For me, storing/presenting the scancode as 0xAAaaDDdd is "obviously" the
clearest and least confusing interpretation. But I might have spent too
long time using that notation in code and mentally to be able to find
anything else intuitive :)
0xAAaaDDdd means that you read/parse/print it left to right, just as you
would if you drew a pulse-space chart showing the received IR pulse
(time normally progresses to the right...modulo the per-byte bitrev).
It kind of matches the other protocol scancodes as well (the "address"
bits high, cmd bits low, the high bits tend to remain constant for one
given remote, the low bits change, although it's not a hard rule) and it
matches most software I've ever seen (AFAIK, LIRC represents NEC32
scancodes this way, as does e.g. the Pronto software and protocol).
That said...I think we at least agree that we need *a* representation
and that it should be used consistently in all drivers, right?
//David
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 13:22 ` David Härdeman
@ 2014-03-31 14:06 ` James Hogan
2014-03-31 15:26 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 30+ messages in thread
From: James Hogan @ 2014-03-31 14:06 UTC (permalink / raw)
To: David Härdeman; +Cc: linux-media, m.chehab
[-- Attachment #1: Type: text/plain, Size: 2259 bytes --]
On 31/03/14 14:22, David Härdeman wrote:
> On 2014-03-31 12:56, James Hogan wrote:
>> This would mean that if the data is put in the right bit order (first
>> bit received in BIT(0), last bit received in BIT(31)), then the scancode
>> = raw, and if the data is received in the reverse bit order (like the
>> raw decoder, shifting the data left and inserting the last bit in
>> BIT(0)) then the scancode = bitrev32(raw).
>>
>> Have I missed something?
>
> I just think we have to agree to disagree :)
>
> For me, storing/presenting the scancode as 0xAAaaDDdd is "obviously" the
> clearest and least confusing interpretation. But I might have spent too
> long time using that notation in code and mentally to be able to find
> anything else intuitive :)
>
> 0xAAaaDDdd means that you read/parse/print it left to right, just as you
> would if you drew a pulse-space chart showing the received IR pulse
> (time normally progresses to the right...modulo the per-byte bitrev).
Sure, but the NEC bit order is little endian, and the scancode is a
32bit value not an array of 4 bytes, so it's artificial to expect it to
make any sense when read as big endian. E.g. if you extended the
transmission to 48 bits you'd expect the hex printed scancode to extend
to the left not the right.
The bits in the 32-bit word also become discontinuous for no good
reason, especially considering the cases we're trying to take into
account (NEC-32 and NEC-24) both effectively have 16-bit fields.
> It kind of matches the other protocol scancodes as well (the "address"
> bits high, cmd bits low, the high bits tend to remain constant for one
> given remote, the low bits change, although it's not a hard rule) and it
Very true, but you still have the low byte of the command in the 2nd
lowest byte, which is why my original suggestion was:
0xaaAAddDD
I.e. swap 16bit halves, each 16bit field intact.
> matches most software I've ever seen (AFAIK, LIRC represents NEC32
> scancodes this way, as does e.g. the Pronto software and protocol).
>
> That said...I think we at least agree that we need *a* representation
> and that it should be used consistently in all drivers, right?
Yes, that would be nice.
Cheers
James
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 13:22 ` David Härdeman
2014-03-31 14:06 ` James Hogan
@ 2014-03-31 15:26 ` Mauro Carvalho Chehab
2014-03-31 16:47 ` David Härdeman
1 sibling, 1 reply; 30+ messages in thread
From: Mauro Carvalho Chehab @ 2014-03-31 15:26 UTC (permalink / raw)
To: David Härdeman; +Cc: James Hogan, linux-media
Em Mon, 31 Mar 2014 15:22:47 +0200
David Härdeman <david@hardeman.nu> escreveu:
> On 2014-03-31 12:56, James Hogan wrote:
> > On 31/03/14 11:19, David Härdeman wrote:
> >> On 2014-03-31 11:44, James Hogan wrote:
> >>> On 29/03/14 16:11, David Härdeman wrote:
> >>>> + /* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
> >>>> + *scancode = swab32((u32)raw);
> >>>
> >>> What's the point of the byte swapping?
> >>>
> >>> Surely the most natural NEC encoding would just treat it as a single
> >>> 32-bit (LSBit first) field rather than 4 8-bit fields that needs
> >>> swapping.
> >>
> >> Thanks for having a look at the patches, I agree with your comments on
> >> the other patches (and I have to respin some of them because I missed
> >> two drivers), but the comments to this patch confuses me a bit.
> >>
> >> That the NEC data is transmitted as 32 bits encoded with LSB bit order
> >> within each byte is AFAIK just about the only thing that all
> >> sources/documentation of the protocal can agree on (so bitrev:ing the
> >> bits within each byte makes sense, unless the hardware has done it
> >> already).
> >
> > Agreed (in the case of img-ir there's a bit orientation setting which
> > ensures that the u64 raw has the correct bit order, in the case of NEC
> > the first bit received goes in the lowest order bit of the raw data).
> >
> >> As for the byte order, AAaaDDdd corresponds to the transmission order
> >> and seems to be what most drivers expect/use for their RX data.
> >
> > AAaaDDdd is big endian rendering, no? (like "%08x")
>
> Yeah, you could call it that.
>
> > If it should be interpreted as LSBit first, then the first bits
> > received
> > should go in the low bits of the scancode, and by extension the first
> > bytes received in the low bytes of the scancode, i.e. at the end of the
> > inherently big-endian hexadecimal rendering of the scancode.
>
> I'm not saying the whole scancode should be interpreted as one 32 bit
> LSBit integer, just that the endianness within each byte should be
> respected.
>
> >> Are you suggesting that rc-core should standardize on ddDDaaAA order?
> >
> > Yes (where ddDDaaAA means something like scancode
> > "0x(~cmd)(cmd)(~addr)(addr)")
>
> Yes, that's what I meant.
>
> > This would mean that if the data is put in the right bit order (first
> > bit received in BIT(0), last bit received in BIT(31)), then the
> > scancode
> > = raw, and if the data is received in the reverse bit order (like the
> > raw decoder, shifting the data left and inserting the last bit in
> > BIT(0)) then the scancode = bitrev32(raw).
> >
> > Have I missed something?
>
> I just think we have to agree to disagree :)
>
> For me, storing/presenting the scancode as 0xAAaaDDdd is "obviously" the
> clearest and least confusing interpretation. But I might have spent too
> long time using that notation in code and mentally to be able to find
> anything else intuitive :)
Inside the RC core, for all other protocols, the order always
ADDRESS + COMMAND.
Up to NEC-24 bits, this is preserved, as the command is always 0xDD
and the address is either 0xaaAA or 0xAA.
The 32-bits NEC is a little ackward, if we consider the command as
also being 8 bits, and the address having 24 bits.
The Tivo keytable is weird:
{ 0x3085f009, KEY_MEDIA }, /* TiVo Button */
{ 0x3085e010, KEY_POWER2 }, /* TV Power */
{ 0x3085e011, KEY_TV }, /* Live TV/Swap */
{ 0x3085c034, KEY_VIDEO_NEXT }, /* TV Input */
{ 0x3085e013, KEY_INFO },
{ 0x3085a05f, KEY_CYCLEWINDOWS }, /* Window */
{ 0x0085305f, KEY_CYCLEWINDOWS },
{ 0x3085c036, KEY_EPG }, /* Guide */
...
There, the only part of the scancode that doesn't change is 0x85.
It seems that they're using 8 bits for address (0xaa) and 24
bits for command (0xAADDdd).
So, it seems that they're actually sending address/command as:
[command >> 24><Address][(command >>8) & 0xff][command & 0xff]
With seems too awkward.
IMHO, it would make more sense to store those data as:
<address><command>
So, KEY_MEDIA, for example, would be:
+ { 0x8530f009, KEY_MEDIA }, /* TiVo Button */
However, I'm not sure how other 32 bits NEC scancodes might be.
So, I think we should keep the internal representation as-is,
for now, while we're not sure about how other vendors handle
it, as, for now, there's just one IR table with 32 bits nec.
That's said, I don't mind much how this is internally stored at
the Kernel level, as we can always change it, but we should provide
backward compatibility for userspace, when userspace sends
to Kernel a 16 bit or a 24 bit keytable.
So, I think we should first focus on how to properly get/set the
bitsize at the API in a way that this is backward compatible.
Ok, the API actually sends the bit size of each keycode, as the
size length is variable, but I'm not sure if this is reliable enough,
as I think that the current userspace just sets it to 32 bits, even
when passing a 16 bits key.
In any case, it doesn't make any sense to require userspace to
convert a 16 bits normal NEC table (or a 24 bits "extended" NEC
table) into a 32 bits data+checksum bitpack on userspace.
Regards,
Mauro
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 15:26 ` Mauro Carvalho Chehab
@ 2014-03-31 16:47 ` David Härdeman
0 siblings, 0 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-31 16:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: James Hogan, linux-media
On Mon, Mar 31, 2014 at 12:26:56PM -0300, Mauro Carvalho Chehab wrote:
>Inside the RC core, for all other protocols, the order always
>ADDRESS + COMMAND.
>
>Up to NEC-24 bits, this is preserved, as the command is always 0xDD
>and the address is either 0xaaAA or 0xAA.
>
>The 32-bits NEC is a little ackward, if we consider the command as
>also being 8 bits, and the address having 24 bits.
>
>The Tivo keytable is weird:
>
> { 0x3085f009, KEY_MEDIA }, /* TiVo Button */
> { 0x3085e010, KEY_POWER2 }, /* TV Power */
> { 0x3085e011, KEY_TV }, /* Live TV/Swap */
> { 0x3085c034, KEY_VIDEO_NEXT }, /* TV Input */
> { 0x3085e013, KEY_INFO },
> { 0x3085a05f, KEY_CYCLEWINDOWS }, /* Window */
> { 0x0085305f, KEY_CYCLEWINDOWS },
> { 0x3085c036, KEY_EPG }, /* Guide */
> ...
>
>There, the only part of the scancode that doesn't change is 0x85.
>It seems that they're using 8 bits for address (0xaa) and 24
>bits for command (0xAADDdd).
>
>So, it seems that they're actually sending address/command as:
>
> [command >> 24><Address][(command >>8) & 0xff][command & 0xff]
>
>With seems too awkward.
>
>IMHO, it would make more sense to store those data as:
> <address><command>
>
>So, KEY_MEDIA, for example, would be:
>+ { 0x8530f009, KEY_MEDIA }, /* TiVo Button */
>
>However, I'm not sure how other 32 bits NEC scancodes might be.
And it's completely irrelevant. There's little to no value in trying to
determine what's a "command" and what's an "address". We have to
standardize on one in-memory representation of the 32 bits, and then we
should just treat it as that...as a u32 lookup key for the
scancode<->keycode table which lacks any further "meaning".
>So, I think we should keep the internal representation as-is,
>for now, while we're not sure about how other vendors handle
>it, as, for now, there's just one IR table with 32 bits nec.
It doesn't matter how other vendors handle (i.e. interpret) the
different bits, that's what we want to get away from, it's the whole
point of this discussion.
>That's said, I don't mind much how this is internally stored at
>the Kernel level, as we can always change it, but we should provide
>backward compatibility for userspace, when userspace sends
>to Kernel a 16 bit or a 24 bit keytable.
Yes, which is part of what I've proposed.
It's not a coicidence that I've proposed a new ioctl and the NEC32
standardization at the same time. A new ioctl is the perfect time and
place to get this right once and for all.
So with the new ioctl, the "protocol" is made explicit, and the
definition of a scancode follows from the "protocol" (protocol as in
RC_TYPE_*).
For RC_TYPE_NEC, that scancode would be a 32 bit int (exact byte and bit
order to be determined, but not terribly important for this discussion).
That removes *all* ambiguity and makes RC_TYPE_NEC behave *exactly* like
all other protocols. At the same time it removes pointless policy from
the kernel and causes a reduction in code (mostly thinking of the
pointless NEC16/24/32 parsing code that gets duplicated across drivers).
>So, I think we should first focus on how to properly get/set the
>bitsize at the API in a way that this is backward compatible.
No, adding bitsizes adds complexity and additional layers of abstraction
for no good reason. And it is not needed for *any other protocol*. Why?
Because the protocol already defines the bitsize. And so would NEC if we
would just use all 32 bits throughout.
With that change, the bitsize is implicit in *each protocol* and the new
ioctl I proposed makes the protocol explicit (while providing at least a
best-effort guess for NEC scancodes when the legacy ioctl is used).
(and no, please, don't suggest we add RC_TYPE_NEC, RC_TYPE_NEC24,
RC_TYPE_NEC32...)
>Ok, the API actually sends the bit size of each keycode, as the
>size length is variable, but I'm not sure if this is reliable enough,
>as I think that the current userspace just sets it to 32 bits, even
>when passing a 16 bits key.
That won't work as you've noted yourself.
>In any case, it doesn't make any sense to require userspace to
>convert a 16 bits normal NEC table (or a 24 bits "extended" NEC
>table) into a 32 bits data+checksum bitpack on userspace.
I disagree. Strongly.
It makes perfect sense. Policy doesn't belong in the kernel and all
that. Asking userspace to provide a full description of the 32 bits that
are transmitted removes all ambiguity and makes any "bitsize"
irrelevant. For all the other protocols we support, the "bitsize" is
known on a per-protocol basis. The same can be true for RC_TYPE_NEC.
And userspace can still write nice user-friendly 16 bit keymaps if it
likes and convert to kernel scancode notation on the fly. That's
something userspace anyways has to do today.
Consider the 32 bit scancode as simply being the communication protocol
between userspace <-> kernel if you like. There's no reason to
complicate that with bitsizes and/or multiple protocols when a single 32
bit scancode describes exactly everything that the kernel and userspace
needs to know.
--
David Härdeman
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 10:19 ` David Härdeman
2014-03-31 10:56 ` James Hogan
@ 2014-03-31 12:14 ` Mauro Carvalho Chehab
2014-03-31 12:58 ` David Härdeman
1 sibling, 1 reply; 30+ messages in thread
From: Mauro Carvalho Chehab @ 2014-03-31 12:14 UTC (permalink / raw)
To: David Härdeman; +Cc: James Hogan, linux-media
Hi David,
Em Mon, 31 Mar 2014 12:19:10 +0200
David Härdeman <david@hardeman.nu> escreveu:
> On 2014-03-31 11:44, James Hogan wrote:
> > On 29/03/14 16:11, David Härdeman wrote:
> >> Using the full 32 bits for all kinds of NEC scancodes simplifies
> >> rc-core
> >> and the nec decoder without any loss of functionality.
> >>
> >> In order to maintain backwards compatibility, some heuristics are
> >> added
> >> in rc-main.c to convert scancodes to NEC32 as necessary.
> >>
> >> I plan to introduce a different ioctl later which makes the protocol
> >> explicit (and which expects all NEC scancodes to be 32 bit, thereby
> >> removing the need for guesswork).
> >>
> >> Signed-off-by: David Härdeman <david@hardeman.nu>
> >> ---
> >> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
> >> b/drivers/media/rc/img-ir/img-ir-nec.c
> >> index 40ee844..133ea45 100644
> >> --- a/drivers/media/rc/img-ir/img-ir-nec.c
> >> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
> >> @@ -5,42 +5,20 @@
> >> */
> >>
> >> #include "img-ir-hw.h"
> >> -#include <linux/bitrev.h>
> >>
> >> /* Convert NEC data to a scancode */
> >> static int img_ir_nec_scancode(int len, u64 raw, enum rc_type
> >> *protocol,
> >> u32 *scancode, u64 enabled_protocols)
> >> {
> >> - unsigned int addr, addr_inv, data, data_inv;
> >> /* a repeat code has no data */
> >> if (!len)
> >> return IMG_IR_REPEATCODE;
> >> +
> >> if (len != 32)
> >> return -EINVAL;
> >> - /* raw encoding: ddDDaaAA */
> >> - addr = (raw >> 0) & 0xff;
> >> - addr_inv = (raw >> 8) & 0xff;
> >> - data = (raw >> 16) & 0xff;
> >> - data_inv = (raw >> 24) & 0xff;
> >> - if ((data_inv ^ data) != 0xff) {
> >> - /* 32-bit NEC (used by Apple and TiVo remotes) */
> >> - /* scan encoding: AAaaDDdd (LSBit first) */
> >> - *scancode = bitrev8(addr) << 24 |
> >> - bitrev8(addr_inv) << 16 |
> >> - bitrev8(data) << 8 |
> >> - bitrev8(data_inv);
> >> - } else if ((addr_inv ^ addr) != 0xff) {
> >> - /* Extended NEC */
> >> - /* scan encoding: AAaaDD */
> >> - *scancode = addr << 16 |
> >> - addr_inv << 8 |
> >> - data;
> >> - } else {
> >> - /* Normal NEC */
> >> - /* scan encoding: AADD */
> >> - *scancode = addr << 8 |
> >> - data;
> >> - }
> >> +
> >> + /* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
> >> + *scancode = swab32((u32)raw);
> >
> > What's the point of the byte swapping?
> >
> > Surely the most natural NEC encoding would just treat it as a single
> > 32-bit (LSBit first) field rather than 4 8-bit fields that needs
> > swapping.
>
> Thanks for having a look at the patches, I agree with your comments on
> the other patches (and I have to respin some of them because I missed
> two drivers), but the comments to this patch confuses me a bit.
>
> That the NEC data is transmitted as 32 bits encoded with LSB bit order
> within each byte is AFAIK just about the only thing that all
> sources/documentation of the protocal can agree on (so bitrev:ing the
> bits within each byte makes sense, unless the hardware has done it
> already).
>
> As for the byte order, AAaaDDdd corresponds to the transmission order
> and seems to be what most drivers expect/use for their RX data.
>
> Are you suggesting that rc-core should standardize on ddDDaaAA order?
Let's better name this, as AAaaDDdd implies that:
aa = ~AA
dd = ~DD
As described at the NEC protocol.
The 24 or 32 bits variation is actually a violation of the NEC protocol.
What some IRs actually provide is:
xxyyADDdd (24 bits NEC)
where:
Address = yyxx
Data = DD
As described as "Extended NEC protocol" at:
http://www.sbprojects.com/knowledge/ir/nec.php
or:
xxyyADDzz (32 bits NEC)
where:
Address = zzxxyy
Data = DD
Also, currently, there's just one IR table with 32 bits nec:
rc-tivo.c, used by the mceusb driver.
Well, changing the NEC decoders to always send a 32 bits code has
several issues:
1) It makes the normal NEC protocol as an exception, and not as a
rule;
2) It breaks all in-kernel tables for 16 bits and 24 bits NEC.
As already said, currently, there's just one driver using 32
bits NEC, and just for one IR type (RC_MAP_TIVO);
3) It causes regressions to userspace, as userspace tables won't
work anymore;
4) Your to_nec32() macro will break support for 24-bits IRs
shipped with devices that can only provide 16 bits.
In order to explain (4), let's see what happens when a 24-bits
NEC code is received by a in-hardware decoder.
There are a wide range of Chinese IR devices shipped with widely
used media hardware that produce a 24-bit NEC code. One of the
most popular of such manufacturers use the address = 0x866b
(btw, the get_key_beholdm6xx() function at saa7134 driver seems
to be wrong, as the keytables for behold device has the address of
this vendor mapped as 0x6b86).
The way those codes are handled inside each in-hardware NEC
decoder are different. I've seen all those alternatives:
a) the full 24-bits code is received by the driver;
b) some hardware will simply discard the MSB of the address;
c) a few hardware will discard the entire keycode, as the
checksum bytes won't match.
The devices from the 0x866b manufacturer is used by a wide range
of devices that can do either (a) or (b).
Well, as the to_nec32() doesn't know the original keycode, it
would map an address like 0x866b as 0x946b, with is wrong, and
won't match the corresponding NEC table.
Due to (3) (it causes userspace regressions), we can't apply
such changes.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Regards,
Mauro
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 12:14 ` Mauro Carvalho Chehab
@ 2014-03-31 12:58 ` David Härdeman
2014-03-31 13:15 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 30+ messages in thread
From: David Härdeman @ 2014-03-31 12:58 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: James Hogan, linux-media
On 2014-03-31 14:14, Mauro Carvalho Chehab wrote:
> Em Mon, 31 Mar 2014 12:19:10 +0200
> David Härdeman <david@hardeman.nu> escreveu:
>> On 2014-03-31 11:44, James Hogan wrote:
>> > On 29/03/14 16:11, David Härdeman wrote:
>> >> Using the full 32 bits for all kinds of NEC scancodes simplifies
>> >> rc-core
>> >> and the nec decoder without any loss of functionality.
>> >>
>> >> In order to maintain backwards compatibility, some heuristics are
>> >> added
>> >> in rc-main.c to convert scancodes to NEC32 as necessary.
>> >>
>> >> I plan to introduce a different ioctl later which makes the protocol
>> >> explicit (and which expects all NEC scancodes to be 32 bit, thereby
>> >> removing the need for guesswork).
>> >>
>> >> Signed-off-by: David Härdeman <david@hardeman.nu>
>> >> ---
>> >> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
>> >> b/drivers/media/rc/img-ir/img-ir-nec.c
>> >> index 40ee844..133ea45 100644
>> >> --- a/drivers/media/rc/img-ir/img-ir-nec.c
>> >> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
>> >> @@ -5,42 +5,20 @@
>> >> */
>> >>
>> >> #include "img-ir-hw.h"
>> >> -#include <linux/bitrev.h>
>> >>
>> >> /* Convert NEC data to a scancode */
>> >> static int img_ir_nec_scancode(int len, u64 raw, enum rc_type
>> >> *protocol,
>> >> u32 *scancode, u64 enabled_protocols)
>> >> {
>> >> - unsigned int addr, addr_inv, data, data_inv;
>> >> /* a repeat code has no data */
>> >> if (!len)
>> >> return IMG_IR_REPEATCODE;
>> >> +
>> >> if (len != 32)
>> >> return -EINVAL;
>> >> - /* raw encoding: ddDDaaAA */
>> >> - addr = (raw >> 0) & 0xff;
>> >> - addr_inv = (raw >> 8) & 0xff;
>> >> - data = (raw >> 16) & 0xff;
>> >> - data_inv = (raw >> 24) & 0xff;
>> >> - if ((data_inv ^ data) != 0xff) {
>> >> - /* 32-bit NEC (used by Apple and TiVo remotes) */
>> >> - /* scan encoding: AAaaDDdd (LSBit first) */
>> >> - *scancode = bitrev8(addr) << 24 |
>> >> - bitrev8(addr_inv) << 16 |
>> >> - bitrev8(data) << 8 |
>> >> - bitrev8(data_inv);
>> >> - } else if ((addr_inv ^ addr) != 0xff) {
>> >> - /* Extended NEC */
>> >> - /* scan encoding: AAaaDD */
>> >> - *scancode = addr << 16 |
>> >> - addr_inv << 8 |
>> >> - data;
>> >> - } else {
>> >> - /* Normal NEC */
>> >> - /* scan encoding: AADD */
>> >> - *scancode = addr << 8 |
>> >> - data;
>> >> - }
>> >> +
>> >> + /* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
>> >> + *scancode = swab32((u32)raw);
>> >
>> > What's the point of the byte swapping?
>> >
>> > Surely the most natural NEC encoding would just treat it as a single
>> > 32-bit (LSBit first) field rather than 4 8-bit fields that needs
>> > swapping.
>>
>> Thanks for having a look at the patches, I agree with your comments on
>> the other patches (and I have to respin some of them because I missed
>> two drivers), but the comments to this patch confuses me a bit.
>>
>> That the NEC data is transmitted as 32 bits encoded with LSB bit order
>> within each byte is AFAIK just about the only thing that all
>> sources/documentation of the protocal can agree on (so bitrev:ing the
>> bits within each byte makes sense, unless the hardware has done it
>> already).
>>
>> As for the byte order, AAaaDDdd corresponds to the transmission order
>> and seems to be what most drivers expect/use for their RX data.
>>
>> Are you suggesting that rc-core should standardize on ddDDaaAA order?
>
>
> Let's better name this, as AAaaDDdd implies that:
> aa = ~AA
> dd = ~DD
> As described at the NEC protocol.
I really don't think James and I had any trouble understanding each
other :)
> The 24 or 32 bits variation is actually a violation of the NEC
> protocol.
Violation is a misnomer. NEC created the 24 bit version, it's an
extension. Many companies (such as your employer :)) have created
further variations.
> What some IRs actually provide is:
> xxyyADDdd (24 bits NEC)
> where:
> Address = yyxx
> Data = DD
>
> As described as "Extended NEC protocol" at:
> http://www.sbprojects.com/knowledge/ir/nec.php
>
> or:
> xxyyADDzz (32 bits NEC)
> where:
> Address = zzxxyy
> Data = DD
No need to explain the protocol to me.
> Also, currently, there's just one IR table with 32 bits nec:
> rc-tivo.c, used by the mceusb driver.
Yes, I know.
> Well, changing the NEC decoders to always send a 32 bits code has
> several issues:
>
> 1) It makes the normal NEC protocol as an exception, and not as a
> rule;
It's not an exception. I just makes all 32 bits explicit.
And the lack of that explicit information currently makes the scancode
ambiguous. Right now if I give you a NEC scancode of 0xff00 (like we
give to userspace with the EV_SCAN event), you can't tell what it
means...it could, for example, be a 32 bit code of 0x0000ff00...
> 2) It breaks all in-kernel tables for 16 bits and 24 bits NEC.
> As already said, currently, there's just one driver using 32
> bits NEC, and just for one IR type (RC_MAP_TIVO);
No, the proposed patch doesn't break all in-kernel tables. The in-kernel
tables are converted on the fly to NEC32 when loaded.
> 3) It causes regressions to userspace, as userspace tables won't
> work anymore;
I know it may cause troubles for userspace, however:
a) You've already accepted patches that change the scancode format of
the NEC decoder within the last few weeks so you've already set the
stage for the same kind of trouble (even if I agree with James on parts
of that patch)
b) The current code is broken as well...using the same remote will
generate different scancodes depending on the driver (even if the old
and new hardware *can* receive the full scancode), meaning that your
keytable will suddenly stop working if you change HW. That's bad.
> 4) Your to_nec32() macro will break support for 24-bits IRs
> shipped with devices that can only provide 16 bits.
>
> In order to explain (4), let's see what happens when a 24-bits
> NEC code is received by a in-hardware decoder.
>
> There are a wide range of Chinese IR devices shipped with widely
> used media hardware that produce a 24-bit NEC code. One of the
> most popular of such manufacturers use the address = 0x866b
> (btw, the get_key_beholdm6xx() function at saa7134 driver seems
> to be wrong, as the keytables for behold device has the address of
> this vendor mapped as 0x6b86).
I know, I've already identified and fixed that problem in a separate
patch that's posted to the list. And it will also break out-of-kernel
user-defined keymaps. Any inconsistency is a no-win situation. And we
*do* have inconsistencies right now.
> The way those codes are handled inside each in-hardware NEC
> decoder are different. I've seen all those alternatives:
>
> a) the full 24-bits code is received by the driver;
> b) some hardware will simply discard the MSB of the address;
> c) a few hardware will discard the entire keycode, as the
> checksum bytes won't match.
I know there's a lot of variety, another example is drivers that discard
(possibly after matching address) everything but the "command" part of
the scancode. That should not be used as an excuse not to try to make
the behavior as consistent as possible. After all...that's the point of
a common API.
> The devices from the 0x866b manufacturer is used by a wide range
> of devices that can do either (a) or (b).
>
> Well, as the to_nec32() doesn't know the original keycode, it
> would map an address like 0x866b as 0x946b, with is wrong, and
> won't match the corresponding NEC table.
Yes, if the hardware throws away information, rc-core will sometime
generate a scancode which does not match the real one.
As you say:
if the actual remote control transmits: 0x866b01fe
and the hardware truncates it to: 0x..6b01fe
then rc-core would convert back to: 0x946b01fe
And that could be fixed with a scanmask for that driver (0xffffff)?
(We could also expose the scanmask to userspace so it knows which part
of the scancode it can trust...)
> Due to (3) (it causes userspace regressions), we can't apply
> such changes.
I know Linus' policy with regard to userspace regressions, but see
above.
Regards,
David
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 12:58 ` David Härdeman
@ 2014-03-31 13:15 ` Mauro Carvalho Chehab
2014-03-31 13:54 ` David Härdeman
0 siblings, 1 reply; 30+ messages in thread
From: Mauro Carvalho Chehab @ 2014-03-31 13:15 UTC (permalink / raw)
To: David Härdeman; +Cc: James Hogan, linux-media
Em Mon, 31 Mar 2014 14:58:10 +0200
David Härdeman <david@hardeman.nu> escreveu:
> On 2014-03-31 14:14, Mauro Carvalho Chehab wrote:
> > Em Mon, 31 Mar 2014 12:19:10 +0200
> > David Härdeman <david@hardeman.nu> escreveu:
> >> On 2014-03-31 11:44, James Hogan wrote:
> >> > On 29/03/14 16:11, David Härdeman wrote:
> >> >> Using the full 32 bits for all kinds of NEC scancodes simplifies
> >> >> rc-core
> >> >> and the nec decoder without any loss of functionality.
> >> >>
> >> >> In order to maintain backwards compatibility, some heuristics are
> >> >> added
> >> >> in rc-main.c to convert scancodes to NEC32 as necessary.
> >> >>
> >> >> I plan to introduce a different ioctl later which makes the protocol
> >> >> explicit (and which expects all NEC scancodes to be 32 bit, thereby
> >> >> removing the need for guesswork).
> >> >>
> >> >> Signed-off-by: David Härdeman <david@hardeman.nu>
> >> >> ---
> >> >> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
> >> >> b/drivers/media/rc/img-ir/img-ir-nec.c
> >> >> index 40ee844..133ea45 100644
> >> >> --- a/drivers/media/rc/img-ir/img-ir-nec.c
> >> >> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
> >> >> @@ -5,42 +5,20 @@
> >> >> */
> >> >>
> >> >> #include "img-ir-hw.h"
> >> >> -#include <linux/bitrev.h>
> >> >>
> >> >> /* Convert NEC data to a scancode */
> >> >> static int img_ir_nec_scancode(int len, u64 raw, enum rc_type
> >> >> *protocol,
> >> >> u32 *scancode, u64 enabled_protocols)
> >> >> {
> >> >> - unsigned int addr, addr_inv, data, data_inv;
> >> >> /* a repeat code has no data */
> >> >> if (!len)
> >> >> return IMG_IR_REPEATCODE;
> >> >> +
> >> >> if (len != 32)
> >> >> return -EINVAL;
> >> >> - /* raw encoding: ddDDaaAA */
> >> >> - addr = (raw >> 0) & 0xff;
> >> >> - addr_inv = (raw >> 8) & 0xff;
> >> >> - data = (raw >> 16) & 0xff;
> >> >> - data_inv = (raw >> 24) & 0xff;
> >> >> - if ((data_inv ^ data) != 0xff) {
> >> >> - /* 32-bit NEC (used by Apple and TiVo remotes) */
> >> >> - /* scan encoding: AAaaDDdd (LSBit first) */
> >> >> - *scancode = bitrev8(addr) << 24 |
> >> >> - bitrev8(addr_inv) << 16 |
> >> >> - bitrev8(data) << 8 |
> >> >> - bitrev8(data_inv);
> >> >> - } else if ((addr_inv ^ addr) != 0xff) {
> >> >> - /* Extended NEC */
> >> >> - /* scan encoding: AAaaDD */
> >> >> - *scancode = addr << 16 |
> >> >> - addr_inv << 8 |
> >> >> - data;
> >> >> - } else {
> >> >> - /* Normal NEC */
> >> >> - /* scan encoding: AADD */
> >> >> - *scancode = addr << 8 |
> >> >> - data;
> >> >> - }
> >> >> +
> >> >> + /* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
> >> >> + *scancode = swab32((u32)raw);
> >> >
> >> > What's the point of the byte swapping?
> >> >
> >> > Surely the most natural NEC encoding would just treat it as a single
> >> > 32-bit (LSBit first) field rather than 4 8-bit fields that needs
> >> > swapping.
> >>
> >> Thanks for having a look at the patches, I agree with your comments on
> >> the other patches (and I have to respin some of them because I missed
> >> two drivers), but the comments to this patch confuses me a bit.
> >>
> >> That the NEC data is transmitted as 32 bits encoded with LSB bit order
> >> within each byte is AFAIK just about the only thing that all
> >> sources/documentation of the protocal can agree on (so bitrev:ing the
> >> bits within each byte makes sense, unless the hardware has done it
> >> already).
> >>
> >> As for the byte order, AAaaDDdd corresponds to the transmission order
> >> and seems to be what most drivers expect/use for their RX data.
> >>
> >> Are you suggesting that rc-core should standardize on ddDDaaAA order?
> >
> >
> > Let's better name this, as AAaaDDdd implies that:
> > aa = ~AA
> > dd = ~DD
> > As described at the NEC protocol.
>
> I really don't think James and I had any trouble understanding each
> other :)
Ok, but others on reading this thread may misunderstand the meanings.
>
> > The 24 or 32 bits variation is actually a violation of the NEC
> > protocol.
>
> Violation is a misnomer. NEC created the 24 bit version, it's an
> extension. Many companies (such as your employer :)) have created
> further variations.
I'm fine if you call it as an extension, but the original NEC _is_
16 bits, and most drivers are compliant with it.
We should not break what's working.
> > What some IRs actually provide is:
> > xxyyADDdd (24 bits NEC)
> > where:
> > Address = yyxx
> > Data = DD
> >
> > As described as "Extended NEC protocol" at:
> > http://www.sbprojects.com/knowledge/ir/nec.php
> >
> > or:
> > xxyyADDzz (32 bits NEC)
> > where:
> > Address = zzxxyy
> > Data = DD
>
> No need to explain the protocol to me.
>
> > Also, currently, there's just one IR table with 32 bits nec:
> > rc-tivo.c, used by the mceusb driver.
>
> Yes, I know.
>
> > Well, changing the NEC decoders to always send a 32 bits code has
> > several issues:
> >
> > 1) It makes the normal NEC protocol as an exception, and not as a
> > rule;
>
> It's not an exception. I just makes all 32 bits explicit.
Well, if all drivers but one only have 16 or 24 bits tables, this is
an exception.
> And the lack of that explicit information currently makes the scancode
> ambiguous. Right now if I give you a NEC scancode of 0xff00 (like we
> give to userspace with the EV_SCAN event), you can't tell what it
> means...it could, for example, be a 32 bit code of 0x0000ff00...
>
> > 2) It breaks all in-kernel tables for 16 bits and 24 bits NEC.
> > As already said, currently, there's just one driver using 32
> > bits NEC, and just for one IR type (RC_MAP_TIVO);
>
> No, the proposed patch doesn't break all in-kernel tables. The in-kernel
> tables are converted on the fly to NEC32 when loaded.
That's messy. We should either change everything in Kernelspace to
32 bits or keep as is.
If such emulation is needed, it should be only for userspace tables.
> > 3) It causes regressions to userspace, as userspace tables won't
> > work anymore;
>
> I know it may cause troubles for userspace, however:
>
> a) You've already accepted patches that change the scancode format of
> the NEC decoder within the last few weeks so you've already set the
> stage for the same kind of trouble (even if I agree with James on parts
> of that patch)
If I let this pass, we should revert it before it reaches upstream.
What patch caused regressions?
> b) The current code is broken as well...using the same remote will
> generate different scancodes depending on the driver (even if the old
> and new hardware *can* receive the full scancode), meaning that your
> keytable will suddenly stop working if you change HW. That's bad.
On the devices I have here, it is not broken. Let's fix it where this
is broken, and not use it as an excuse to break even more things.
> > 4) Your to_nec32() macro will break support for 24-bits IRs
> > shipped with devices that can only provide 16 bits.
> >
> > In order to explain (4), let's see what happens when a 24-bits
> > NEC code is received by a in-hardware decoder.
> >
> > There are a wide range of Chinese IR devices shipped with widely
> > used media hardware that produce a 24-bit NEC code. One of the
> > most popular of such manufacturers use the address = 0x866b
> > (btw, the get_key_beholdm6xx() function at saa7134 driver seems
> > to be wrong, as the keytables for behold device has the address of
> > this vendor mapped as 0x6b86).
>
> I know, I've already identified and fixed that problem in a separate
> patch that's posted to the list. And it will also break out-of-kernel
> user-defined keymaps. Any inconsistency is a no-win situation. And we
> *do* have inconsistencies right now.
Yes. That's one of the reasons why this was not fixed yet (and the other
one is that I don't have any of such device in hands, in order to be
sure that this is not another vendor that, by coincidence, has address
0x6b86).
> > The way those codes are handled inside each in-hardware NEC
> > decoder are different. I've seen all those alternatives:
> >
> > a) the full 24-bits code is received by the driver;
> > b) some hardware will simply discard the MSB of the address;
> > c) a few hardware will discard the entire keycode, as the
> > checksum bytes won't match.
>
> I know there's a lot of variety, another example is drivers that discard
> (possibly after matching address) everything but the "command" part of
> the scancode. That should not be used as an excuse not to try to make
> the behavior as consistent as possible. After all...that's the point of
> a common API.
It should be consistent, and it should be able to support the existing
hardware.
> > The devices from the 0x866b manufacturer is used by a wide range
> > of devices that can do either (a) or (b).
> >
> > Well, as the to_nec32() doesn't know the original keycode, it
> > would map an address like 0x866b as 0x946b, with is wrong, and
> > won't match the corresponding NEC table.
>
> Yes, if the hardware throws away information, rc-core will sometime
> generate a scancode which does not match the real one.
>
> As you say:
>
> if the actual remote control transmits: 0x866b01fe
> and the hardware truncates it to: 0x..6b01fe
> then rc-core would convert back to: 0x946b01fe
>
> And that could be fixed with a scanmask for that driver (0xffffff)?
I think you're meaning 0x0000ffff, right?
> (We could also expose the scanmask to userspace so it knows which part
> of the scancode it can trust...)
Yes, we could do it, but the current userspace should keep working.
Eventually, that means to add some backward compat code there, in order
to preserve the behavior with current tables.
> > Due to (3) (it causes userspace regressions), we can't apply
> > such changes.
>
> I know Linus' policy with regard to userspace regressions, but see
> above.
>
> Regards,
> David
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Regards,
Mauro
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
2014-03-31 13:15 ` Mauro Carvalho Chehab
@ 2014-03-31 13:54 ` David Härdeman
0 siblings, 0 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-31 13:54 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: James Hogan, linux-media, jarod, sean
On 2014-03-31 15:15, Mauro Carvalho Chehab wrote:
> Em Mon, 31 Mar 2014 14:58:10 +0200
> David Härdeman <david@hardeman.nu> escreveu:
>> On 2014-03-31 14:14, Mauro Carvalho Chehab wrote:
>>> The 24 or 32 bits variation is actually a violation of the NEC
>>> protocol.
>>
>> Violation is a misnomer. NEC created the 24 bit version, it's an
>> extension. Many companies (such as your employer :)) have created
>> further variations.
>
> I'm fine if you call it as an extension, but the original NEC _is_
> 16 bits, and most drivers are compliant with it.
>
> We should not break what's working.
You're misrepresenting the proposed changes now.
I'm trying to fixup the scancode handling in the best way possible, I'm
not willfully breaking anything.
Some things are inconsistent right now between the drivers, in such a
situation when driver A says "first X then Y" and driver B says "first Y
then X" the situation is:
a) already "broken"; and
b) can't be fixed without introducing "breakage" of a different kind to
either A or B
>>> Well, changing the NEC decoders to always send a 32 bits code has
>>> several issues:
>>>
>>> 1) It makes the normal NEC protocol as an exception, and not as a
>>> rule;
>>
>> It's not an exception. I just makes all 32 bits explicit.
>
> Well, if all drivers but one only have 16 or 24 bits tables, this is
> an exception.
Not really. 32 bits are transmitted no matter what you call the
protocol. I'm proposing storing those 32 bits in the scancode<->keycode
table. Not what I'd call an exception (this particular point starts to
feel a bit off-topic though so I think we can drop it).
>> And the lack of that explicit information currently makes the scancode
>> ambiguous. Right now if I give you a NEC scancode of 0xff00 (like we
>> give to userspace with the EV_SCAN event), you can't tell what it
>> means...it could, for example, be a 32 bit code of 0x0000ff00...
You didn't answer this part. It's actually one of the biggest reasons
for introducing the full scancode everywhere.
>> > 2) It breaks all in-kernel tables for 16 bits and 24 bits NEC.
>> > As already said, currently, there's just one driver using 32
>> > bits NEC, and just for one IR type (RC_MAP_TIVO);
>>
>> No, the proposed patch doesn't break all in-kernel tables. The
>> in-kernel
>> tables are converted on the fly to NEC32 when loaded.
>
> That's messy. We should either change everything in Kernelspace to
> 32 bits or keep as is.
No problem, I could respin the patch to also patch the keytables (which
is what I did first), but I'll wait until we've agreed on something).
> If such emulation is needed, it should be only for userspace tables.
>
>> > 3) It causes regressions to userspace, as userspace tables won't
>> > work anymore;
>>
>> I know it may cause troubles for userspace, however:
>>
>> a) You've already accepted patches that change the scancode format of
>> the NEC decoder within the last few weeks so you've already set the
>> stage for the same kind of trouble (even if I agree with James on
>> parts
>> of that patch)
>
> If I let this pass, we should revert it before it reaches upstream.
>
> What patch caused regressions?
18bc17448147e93f31cc9b1a83be49f1224657b2, since it changes the scancode
it'll break userspace keytables, it's mentioned in patch 4/11 in my
patchset.
>> b) The current code is broken as well...using the same remote will
>> generate different scancodes depending on the driver (even if the old
>> and new hardware *can* receive the full scancode), meaning that your
>> keytable will suddenly stop working if you change HW. That's bad.
>
> On the devices I have here, it is not broken. Let's fix it where this
> is broken, and not use it as an excuse to break even more things.
Whether the hardware you happen to have agrees is beside the point?
>>> (btw, the get_key_beholdm6xx() function at saa7134 driver seems
>>> to be wrong, as the keytables for behold device has the address of
>>> this vendor mapped as 0x6b86).
>>
>> I know, I've already identified and fixed that problem in a separate
>> patch that's posted to the list. And it will also break out-of-kernel
>> user-defined keymaps. Any inconsistency is a no-win situation. And we
>> *do* have inconsistencies right now.
>
> Yes. That's one of the reasons why this was not fixed yet (and the
> other
> one is that I don't have any of such device in hands, in order to be
> sure that this is not another vendor that, by coincidence, has address
> 0x6b86).
I know we can't be 100% sure, but the byte order in the driver itself
also supports the notion that the address bytes have been reversed.
>>> The way those codes are handled inside each in-hardware NEC
>>> decoder are different. I've seen all those alternatives:
>>>
>>> a) the full 24-bits code is received by the driver;
>>> b) some hardware will simply discard the MSB of the address;
>>> c) a few hardware will discard the entire keycode, as the
>>> checksum bytes won't match.
>>
>> I know there's a lot of variety, another example is drivers that
>> discard
>> (possibly after matching address) everything but the "command" part of
>> the scancode. That should not be used as an excuse not to try to make
>> the behavior as consistent as possible. After all...that's the point
>> of
>> a common API.
>
> It should be consistent, and it should be able to support the existing
> hardware.
Yes, I agree, but I'm not sure what your point is? Existing hardware
doesn't lose support with my patches?
>>> The devices from the 0x866b manufacturer is used by a wide range
>>> of devices that can do either (a) or (b).
>>>
>>> Well, as the to_nec32() doesn't know the original keycode, it
>>> would map an address like 0x866b as 0x946b, with is wrong, and
>>> won't match the corresponding NEC table.
>>
>> Yes, if the hardware throws away information, rc-core will sometime
>> generate a scancode which does not match the real one.
>>
>> As you say:
>>
>> if the actual remote control transmits: 0x866b01fe
>> and the hardware truncates it to: 0x..6b01fe
>> then rc-core would convert back to: 0x946b01fe
>>
>> And that could be fixed with a scanmask for that driver (0xffffff)?
>
> I think you're meaning 0x0000ffff, right?
Why 0x0000ffff?....the example I gave suggested HW which throws away one
byte, meaning the last three bytes (0x6b01fe) remain valid?
>> (We could also expose the scanmask to userspace so it knows which part
>> of the scancode it can trust...)
>
> Yes, we could do it, but the current userspace should keep working.
> Eventually, that means to add some backward compat code there, in order
> to preserve the behavior with current tables.
Not sure what you're suggesting?
Regards,
David
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 11/11] [RFC] rc-core: don't throw away protocol information
2014-03-29 16:10 [PATCH 00/11] rc-core: My current patch queue David Härdeman
` (9 preceding siblings ...)
2014-03-29 16:11 ` [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes David Härdeman
@ 2014-03-29 16:11 ` David Härdeman
10 siblings, 0 replies; 30+ messages in thread
From: David Härdeman @ 2014-03-29 16:11 UTC (permalink / raw)
To: linux-media; +Cc: james.hogan, m.chehab
Setting and getting keycodes in the input subsystem used to be done via
the EVIOC[GS]KEYCODE ioctl and "unsigned int[2]" (one int for scancode
and one for the keycode).
The interface has now been extended to use the EVIOC[GS]KEYCODE_V2 ioctl
which uses the following struct:
struct input_keymap_entry {
__u8 flags;
__u8 len;
__u16 index;
__u32 keycode;
__u8 scancode[32];
};
(scancode can of course be even bigger, thanks to the len member).
This patch changes how the "input_keymap_entry" struct is interpreted
by rc-core by casting it to "rc_keymap_entry":
struct rc_scancode {
__u16 protocol;
__u16 reserved[3];
__u64 scancode;
}
struct rc_keymap_entry {
__u8 flags;
__u8 len;
__u16 index;
__u32 keycode;
union {
struct rc_scancode rc;
__u8 raw[32];
};
};
The u64 scancode member is large enough for all current protocols and it
would be possible to extend it in the future should it be necessary for
some exotic protocol.
The main advantage with this change is that the protocol is made explicit,
which means that we're not throwing away data (the protocol type) and that
it'll be easier to support multiple protocols with one decoder (think rc5
and rc5-streamzap).
Heuristics are also added to hopefully do the right thing with older
ioctls in order to preserve backwards compatibility.
Further patches will also add the ability to communicate the protocol to
userspace.
Signed-off-by: David Härdeman <david@hardeman.nu>
---
drivers/media/rc/rc-main.c | 184 ++++++++++++++++++++++++++++++++------------
include/media/rc-core.h | 20 +++++
include/media/rc-map.h | 8 +-
3 files changed, 155 insertions(+), 57 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 59bc44f..cd7fd27 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -102,7 +102,7 @@ EXPORT_SYMBOL_GPL(rc_map_unregister);
static struct rc_map_table empty[] = {
- { 0x2a, KEY_COFFEE },
+ { RC_TYPE_OTHER, 0x2a, KEY_COFFEE },
};
static struct rc_map_list empty_map = {
@@ -118,7 +118,6 @@ static struct rc_map_list empty_map = {
* ir_create_table() - initializes a scancode table
* @rc_map: the rc_map to initialize
* @name: name to assign to the table
- * @rc_type: ir type to assign to the new table
* @size: initial size of the table
* @return: zero on success or a negative error code
*
@@ -126,10 +125,9 @@ static struct rc_map_list empty_map = {
* memory to hold at least the specified number of elements.
*/
static int ir_create_table(struct rc_map *rc_map,
- const char *name, u64 rc_type, size_t size)
+ const char *name, size_t size)
{
rc_map->name = name;
- rc_map->rc_type = rc_type;
rc_map->alloc = roundup_pow_of_two(size * sizeof(struct rc_map_table));
rc_map->size = rc_map->alloc / sizeof(struct rc_map_table);
rc_map->scan = kmalloc(rc_map->alloc, GFP_KERNEL);
@@ -224,16 +222,20 @@ static unsigned int ir_update_mapping(struct rc_dev *dev,
/* Did the user wish to remove the mapping? */
if (new_keycode == KEY_RESERVED || new_keycode == KEY_UNKNOWN) {
- IR_dprintk(1, "#%d: Deleting scan 0x%04x\n",
- index, rc_map->scan[index].scancode);
+ IR_dprintk(1, "#%d: Deleting proto 0x%04x, scan 0x%08llx\n",
+ index, rc_map->scan[index].protocol,
+ (unsigned long long)rc_map->scan[index].scancode);
rc_map->len--;
memmove(&rc_map->scan[index], &rc_map->scan[index+ 1],
(rc_map->len - index) * sizeof(struct rc_map_table));
} else {
- IR_dprintk(1, "#%d: %s scan 0x%04x with key 0x%04x\n",
+ IR_dprintk(1, "#%d: %s proto 0x%04x, scan 0x%08llx "
+ "with key 0x%04x\n",
index,
old_keycode == KEY_RESERVED ? "New" : "Replacing",
- rc_map->scan[index].scancode, new_keycode);
+ rc_map->scan[index].protocol,
+ (unsigned long long)rc_map->scan[index].scancode,
+ new_keycode);
rc_map->scan[index].keycode = new_keycode;
__set_bit(new_keycode, dev->input_dev->keybit);
}
@@ -260,9 +262,9 @@ static unsigned int ir_update_mapping(struct rc_dev *dev,
* ir_establish_scancode() - set a keycode in the scancode->keycode table
* @dev: the struct rc_dev device descriptor
* @rc_map: scancode table to be searched
- * @scancode: the desired scancode
- * @resize: controls whether we allowed to resize the table to
- * accommodate not yet present scancodes
+ * @entry: the entry to be added to the table
+ * @resize: controls whether we are allowed to resize the table to
+ * accomodate not yet present scancodes
* @return: index of the mapping containing scancode in question
* or -1U in case of failure.
*
@@ -272,7 +274,7 @@ static unsigned int ir_update_mapping(struct rc_dev *dev,
*/
static unsigned int ir_establish_scancode(struct rc_dev *dev,
struct rc_map *rc_map,
- unsigned int scancode,
+ struct rc_map_table *entry,
bool resize)
{
unsigned int i;
@@ -286,16 +288,27 @@ static unsigned int ir_establish_scancode(struct rc_dev *dev,
* indicate the valid bits of the scancodes.
*/
if (dev->scanmask)
- scancode &= dev->scanmask;
+ entry->scancode &= dev->scanmask;
- /* First check if we already have a mapping for this ir command */
+ /*
+ * First check if we already have a mapping for this command.
+ * Note that the keytable is sorted first on protocol and second
+ * on scancode (lowest to highest).
+ */
for (i = 0; i < rc_map->len; i++) {
- if (rc_map->scan[i].scancode == scancode)
- return i;
+ if (rc_map->scan[i].protocol < entry->protocol)
+ continue;
+
+ if (rc_map->scan[i].protocol > entry->protocol)
+ break;
+
+ if (rc_map->scan[i].scancode < entry->scancode)
+ continue;
- /* Keytable is sorted from lowest to highest scancode */
- if (rc_map->scan[i].scancode >= scancode)
+ if (rc_map->scan[i].scancode > entry->scancode)
break;
+
+ return i;
}
/* No previous mapping found, we might need to grow the table */
@@ -308,7 +321,8 @@ static unsigned int ir_establish_scancode(struct rc_dev *dev,
if (i < rc_map->len)
memmove(&rc_map->scan[i + 1], &rc_map->scan[i],
(rc_map->len - i) * sizeof(struct rc_map_table));
- rc_map->scan[i].scancode = scancode;
+ rc_map->scan[i].scancode = entry->scancode;
+ rc_map->scan[i].protocol = entry->protocol;
rc_map->scan[i].keycode = KEY_RESERVED;
rc_map->len++;
@@ -330,8 +344,10 @@ static inline enum rc_type guess_protocol(struct rc_dev *rdev)
return rc_bitmap_to_type(rdev->enabled_protocols[RC_FILTER_NORMAL]);
else if (hweight64(rdev->allowed_protocols[RC_FILTER_NORMAL]) == 1)
return rc_bitmap_to_type(rdev->allowed_protocols[RC_FILTER_NORMAL]);
+ else if (rc_map->len > 0)
+ return rc_map->scan[0].protocol;
else
- return rc_map->rc_type;
+ return RC_TYPE_OTHER;
}
/**
@@ -374,10 +390,12 @@ static int ir_setkeycode(struct input_dev *idev,
struct rc_dev *rdev = input_get_drvdata(idev);
struct rc_map *rc_map = &rdev->rc_map;
unsigned int index;
- unsigned int scancode;
+ struct rc_map_table entry;
int retval = 0;
unsigned long flags;
+ entry.keycode = ke->keycode;
+
spin_lock_irqsave(&rc_map->lock, flags);
if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
@@ -386,19 +404,42 @@ static int ir_setkeycode(struct input_dev *idev,
retval = -EINVAL;
goto out;
}
- } else {
+ } else if (ke->len == sizeof(int)) {
+ /* Legacy EVIOCSKEYCODE ioctl */
+ u32 scancode;
retval = input_scancode_to_scalar(ke, &scancode);
if (retval)
goto out;
- if (guess_protocol(rdev) == RC_TYPE_NEC)
- scancode = to_nec32(scancode);
+ entry.scancode = scancode;
+ entry.protocol = guess_protocol(rdev);
+ if (entry.protocol == RC_TYPE_NEC)
+ entry.scancode = to_nec32(scancode);
+
+ index = ir_establish_scancode(rdev, rc_map, &entry, true);
+ if (index >= rc_map->len) {
+ retval = -ENOMEM;
+ goto out;
+ }
+ } else if (ke->len == sizeof(struct rc_scancode)) {
+ /* New EVIOCSKEYCODE_V2 ioctl */
+ const struct rc_keymap_entry *rke = (struct rc_keymap_entry *)ke;
+ entry.protocol = rke->rc.protocol;
+ entry.scancode = rke->rc.scancode;
- index = ir_establish_scancode(rdev, rc_map, scancode, true);
+ if (rke->rc.reserved[0] || rke->rc.reserved[1] || rke->rc.reserved[2]) {
+ retval = -EINVAL;
+ goto out;
+ }
+
+ index = ir_establish_scancode(rdev, rc_map, &entry, true);
if (index >= rc_map->len) {
retval = -ENOMEM;
goto out;
}
+ } else {
+ retval = -EINVAL;
+ goto out;
}
*old_keycode = ir_update_mapping(rdev, rc_map, index, ke->keycode);
@@ -421,11 +462,11 @@ static int ir_setkeytable(struct rc_dev *dev,
const struct rc_map *from)
{
struct rc_map *rc_map = &dev->rc_map;
+ struct rc_map_table entry;
unsigned int i, index;
int rc;
- rc = ir_create_table(rc_map, from->name,
- from->rc_type, from->size);
+ rc = ir_create_table(rc_map, from->name, from->size);
if (rc)
return rc;
@@ -433,18 +474,19 @@ static int ir_setkeytable(struct rc_dev *dev,
rc_map->size, rc_map->alloc);
for (i = 0; i < from->size; i++) {
- index = ir_establish_scancode(dev, rc_map,
- from->rc_type == RC_TYPE_NEC ?
- to_nec32(from->scan[i].scancode) :
- from->scan[i].scancode,
- false);
+ if (from->rc_type == RC_TYPE_NEC)
+ entry.scancode = to_nec32(from->scan[i].scancode);
+ else
+ entry.scancode = from->scan[i].scancode;
+
+ entry.protocol = from->rc_type;
+ index = ir_establish_scancode(dev, rc_map, &entry, false);
if (index >= rc_map->len) {
rc = -ENOMEM;
break;
}
- ir_update_mapping(dev, rc_map, index,
- from->scan[i].keycode);
+ ir_update_mapping(dev, rc_map, index, from->scan[i].keycode);
}
if (rc)
@@ -456,6 +498,7 @@ static int ir_setkeytable(struct rc_dev *dev,
/**
* ir_lookup_by_scancode() - locate mapping by scancode
* @rc_map: the struct rc_map to search
+ * @protocol: protocol to look for in the table
* @scancode: scancode to look for in the table
* @return: index in the table, -1U if not found
*
@@ -463,17 +506,24 @@ static int ir_setkeytable(struct rc_dev *dev,
* given scancode.
*/
static unsigned int ir_lookup_by_scancode(const struct rc_map *rc_map,
- unsigned int scancode)
+ u16 protocol, u64 scancode)
{
int start = 0;
int end = rc_map->len - 1;
int mid;
+ struct rc_map_table *m;
while (start <= end) {
mid = (start + end) / 2;
- if (rc_map->scan[mid].scancode < scancode)
+ m = &rc_map->scan[mid];
+
+ if (m->protocol < protocol)
start = mid + 1;
- else if (rc_map->scan[mid].scancode > scancode)
+ else if (m->protocol > protocol)
+ end = mid - 1;
+ else if (m->scancode < scancode)
+ start = mid + 1;
+ else if (m->scancode > scancode)
end = mid - 1;
else
return mid;
@@ -494,35 +544,60 @@ static unsigned int ir_lookup_by_scancode(const struct rc_map *rc_map,
static int ir_getkeycode(struct input_dev *idev,
struct input_keymap_entry *ke)
{
+ struct rc_keymap_entry *rke = (struct rc_keymap_entry *)ke;
struct rc_dev *rdev = input_get_drvdata(idev);
struct rc_map *rc_map = &rdev->rc_map;
struct rc_map_table *entry;
unsigned long flags;
unsigned int index;
- unsigned int scancode;
int retval;
spin_lock_irqsave(&rc_map->lock, flags);
if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
index = ke->index;
- } else {
+ } else if (ke->len == sizeof(int)) {
+ /* Legacy EVIOCGKEYCODE ioctl */
+ u32 scancode;
+ u16 protocol;
+
retval = input_scancode_to_scalar(ke, &scancode);
if (retval)
goto out;
- if (guess_protocol(rdev) == RC_TYPE_NEC)
+ protocol = guess_protocol(rdev);
+ if (protocol == RC_TYPE_NEC)
scancode = to_nec32(scancode);
- index = ir_lookup_by_scancode(rc_map, scancode);
+
+ index = ir_lookup_by_scancode(rc_map, protocol, scancode);
+
+ } else if (ke->len == sizeof(struct rc_scancode)) {
+ /* New EVIOCGKEYCODE_V2 ioctl */
+ if (rke->rc.reserved[0] || rke->rc.reserved[1] || rke->rc.reserved[2]) {
+ retval = -EINVAL;
+ goto out;
+ }
+
+ index = ir_lookup_by_scancode(rc_map,
+ rke->rc.protocol, rke->rc.scancode);
+
+ } else {
+ retval = -EINVAL;
+ goto out;
}
if (index < rc_map->len) {
entry = &rc_map->scan[index];
-
ke->index = index;
ke->keycode = entry->keycode;
- ke->len = sizeof(entry->scancode);
- memcpy(ke->scancode, &entry->scancode, sizeof(entry->scancode));
+ if (ke->len == sizeof(int)) {
+ u32 scancode = entry->scancode;
+ memcpy(ke->scancode, &scancode, sizeof(scancode));
+ } else {
+ ke->len = sizeof(struct rc_scancode);
+ rke->rc.protocol = entry->protocol;
+ rke->rc.scancode = entry->scancode;
+ }
} else if (!(ke->flags & INPUT_KEYMAP_BY_INDEX)) {
/*
@@ -547,6 +622,7 @@ out:
/**
* rc_g_keycode_from_table() - gets the keycode that corresponds to a scancode
* @dev: the struct rc_dev descriptor of the device
+ * @protocol: the protocol to look for
* @scancode: the scancode to look for
* @return: the corresponding keycode, or KEY_RESERVED
*
@@ -554,7 +630,8 @@ out:
* keycode. Normally it should not be used since drivers should have no
* interest in keycodes.
*/
-u32 rc_g_keycode_from_table(struct rc_dev *dev, u32 scancode)
+u32 rc_g_keycode_from_table(struct rc_dev *dev,
+ enum rc_type protocol, u64 scancode)
{
struct rc_map *rc_map = &dev->rc_map;
unsigned int keycode;
@@ -563,15 +640,16 @@ u32 rc_g_keycode_from_table(struct rc_dev *dev, u32 scancode)
spin_lock_irqsave(&rc_map->lock, flags);
- index = ir_lookup_by_scancode(rc_map, scancode);
+ index = ir_lookup_by_scancode(rc_map, protocol, scancode);
keycode = index < rc_map->len ?
rc_map->scan[index].keycode : KEY_RESERVED;
spin_unlock_irqrestore(&rc_map->lock, flags);
if (keycode != KEY_RESERVED)
- IR_dprintk(1, "%s: scancode 0x%04x keycode 0x%02x\n",
- dev->input_name, scancode, keycode);
+ IR_dprintk(1, "%s: protocol 0x%04x scancode 0x%08llx keycode 0x%02x\n",
+ dev->input_name, protocol,
+ (unsigned long long)scancode, keycode);
return keycode;
}
@@ -728,7 +806,7 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
void rc_keydown(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 toggle)
{
unsigned long flags;
- u32 keycode = rc_g_keycode_from_table(dev, scancode);
+ u32 keycode = rc_g_keycode_from_table(dev, protocol, scancode);
spin_lock_irqsave(&dev->keylock, flags);
ir_do_keydown(dev, protocol, scancode, keycode, toggle);
@@ -757,7 +835,7 @@ void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol,
u32 scancode, u8 toggle)
{
unsigned long flags;
- u32 keycode = rc_g_keycode_from_table(dev, scancode);
+ u32 keycode = rc_g_keycode_from_table(dev, protocol, scancode);
spin_lock_irqsave(&dev->keylock, flags);
ir_do_keydown(dev, protocol, scancode, keycode, toggle);
@@ -991,6 +1069,8 @@ static ssize_t store_protocols(struct device *device,
old_type = dev->enabled_protocols[fattr->type];
type = old_type;
+ type = dev->enabled_protocols;
+
while ((tmp = strsep((char **) &data, " \n")) != NULL) {
if (!*tmp)
break;
@@ -1446,8 +1526,8 @@ int rc_register_device(struct rc_dev *dev)
goto out_input;
}
- if (dev->change_protocol) {
- u64 rc_type = (1 << rc_map->rc_type);
+ if (dev->change_protocol && rc_map->len > 0) {
+ u64 rc_type = (1 << rc_map->scan[0].protocol);
rc = dev->change_protocol(dev, &rc_type);
if (rc < 0)
goto out_raw;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 2e97b98..6f66305 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -34,6 +34,24 @@ enum rc_driver_type {
RC_DRIVER_IR_RAW, /* Needs a Infra-Red pulse/space decoder */
};
+/* This is used for the input EVIOC[SG]KEYCODE_V2 ioctls */
+struct rc_scancode {
+ __u16 protocol;
+ __u16 reserved[3];
+ __u64 scancode;
+};
+
+struct rc_keymap_entry {
+ __u8 flags;
+ __u8 len;
+ __u16 index;
+ __u32 keycode;
+ union {
+ struct rc_scancode rc;
+ __u8 raw[32];
+ };
+};
+
/**
* struct rc_scancode_filter - Filter scan codes.
* @data: Scancode data to match.
@@ -226,7 +244,7 @@ void rc_repeat(struct rc_dev *dev);
void rc_keydown(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 toggle);
void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 toggle);
void rc_keyup(struct rc_dev *dev);
-u32 rc_g_keycode_from_table(struct rc_dev *dev, u32 scancode);
+u32 rc_g_keycode_from_table(struct rc_dev *dev, enum rc_type protocol, u64 scancode);
/*
* From rc-raw.c
diff --git a/include/media/rc-map.h b/include/media/rc-map.h
index 2e6c659..bfa27fc 100644
--- a/include/media/rc-map.h
+++ b/include/media/rc-map.h
@@ -81,10 +81,10 @@ enum rc_type {
((~(cmd) & 0x00ff) << 0))
#define RC_SCANCODE_NEC32(data) ((data) & 0xffffffff)
-
struct rc_map_table {
- u32 scancode;
- u32 keycode;
+ u64 scancode;
+ u32 keycode;
+ enum rc_type protocol;
};
struct rc_map {
@@ -92,7 +92,7 @@ struct rc_map {
unsigned int size; /* Max number of entries */
unsigned int len; /* Used number of entries */
unsigned int alloc; /* Size of *scan in bytes */
- enum rc_type rc_type;
+ enum rc_type rc_type; /* For in-kernel keymaps */
const char *name;
spinlock_t lock;
};
^ permalink raw reply related [flat|nested] 30+ messages in thread