* [PATCH 0/2] NEC scancodes and protocols in keymaps - v2
@ 2015-04-06 11:23 David Härdeman
2015-04-06 11:23 ` [PATCH 1/2] rc-core: use the full 32 bits for NEC scancodes David Härdeman
2015-04-06 11:23 ` [PATCH 2/2] rc-core: don't throw away protocol information David Härdeman
0 siblings, 2 replies; 8+ messages in thread
From: David Härdeman @ 2015-04-06 11:23 UTC (permalink / raw)
To: linux-media; +Cc: m.chehab
The following two patches should show more clearly what I mean by
adding protocols to the keytables (and letting userspace add
keytable entries with explicit protocol information). Consider
it a basis for discussion.
Each patch has a separate description, please refer to those for
more information.
v2: fix some bugs found while implementing support in ir-keytables.
---
David Härdeman (2):
rc-core: use the full 32 bits for NEC scancodes
rc-core: don't throw away protocol information
drivers/media/rc/ati_remote.c | 1
drivers/media/rc/imon.c | 7 +
drivers/media/rc/ir-nec-decoder.c | 26 ---
drivers/media/rc/rc-main.c | 234 ++++++++++++++++++++++++------
drivers/media/usb/dvb-usb-v2/af9015.c | 22 +--
drivers/media/usb/dvb-usb-v2/af9035.c | 25 +--
drivers/media/usb/dvb-usb-v2/az6007.c | 16 +-
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 20 +--
drivers/media/usb/dvb-usb/dib0700_core.c | 24 +--
drivers/media/usb/em28xx/em28xx-input.c | 37 +----
include/media/rc-core.h | 26 +++
include/media/rc-map.h | 23 ++-
12 files changed, 266 insertions(+), 195 deletions(-)
--
David Härdeman
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] rc-core: use the full 32 bits for NEC scancodes 2015-04-06 11:23 [PATCH 0/2] NEC scancodes and protocols in keymaps - v2 David Härdeman @ 2015-04-06 11:23 ` David Härdeman 2015-05-14 20:57 ` Mauro Carvalho Chehab 2015-04-06 11:23 ` [PATCH 2/2] rc-core: don't throw away protocol information David Härdeman 1 sibling, 1 reply; 8+ messages in thread From: David Härdeman @ 2015-04-06 11:23 UTC (permalink / raw) To: linux-media; +Cc: 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. At the same time it ensures that scancodes for NEC16/NEC24/NEC32 do not overlap and removes any ambiguity. For example, before this patch, consider these two NEC messages: NEC16 message to address 0x05, command 0x03 NEC24 message to address 0x0005, command 0x03 They'll both have scancode 0x00000503, and there's no way to tell which message was received. In order to maintain backwards compatibility, some heuristics are added in rc-main.c to convert scancodes to NEC32 as necessary when userspace adds entries to the keytable using the regular input ioctls. No conversion will be made for the newer "rc_keymap_entry" based ioctls (see the next patch). Signed-off-by: David Härdeman <david@hardeman.nu> --- drivers/media/rc/ir-nec-decoder.c | 26 ++------------ drivers/media/rc/rc-main.c | 54 +++++++++++++++++++++++++++++- drivers/media/usb/dvb-usb-v2/af9015.c | 22 ++---------- drivers/media/usb/dvb-usb-v2/af9035.c | 25 +++----------- drivers/media/usb/dvb-usb-v2/az6007.c | 16 ++------- drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 20 +++-------- drivers/media/usb/dvb-usb/dib0700_core.c | 24 +++---------- drivers/media/usb/em28xx/em28xx-input.c | 37 +++++---------------- include/media/rc-map.h | 16 +++++++-- 9 files changed, 102 insertions(+), 138 deletions(-) diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c index 7b81fec..16907c1 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 (!(dev->enabled_protocols & RC_BIT_NEC)) return 0; @@ -163,28 +162,9 @@ 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/rc-main.c b/drivers/media/rc/rc-main.c index d068c4e..3379379 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -317,6 +317,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) == 1) + return rc_bitmap_to_type(rdev->enabled_protocols); + else if (hweight64(rdev->allowed_protocols) == 1) + return rc_bitmap_to_type(rdev->allowed_protocols); + 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 u64 to_nec32(u64 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 @@ -349,6 +392,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; @@ -389,7 +435,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; @@ -463,6 +512,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); } @@ -660,7 +711,6 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol, led_trigger_event(led_feedback, LED_FULL); } - input_sync(dev->input_dev); } diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c index 16c0b7d..4cc1463 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 80a29f5..6c00233 100644 --- a/drivers/media/usb/dvb-usb-v2/af9035.c +++ b/drivers/media/usb/dvb-usb-v2/af9035.c @@ -1763,9 +1763,8 @@ err: static int af9035_rc_query(struct dvb_usb_device *d) { int ret; - u32 key; - u8 buf[4]; - struct usb_req req = { CMD_IR_GET, 0, 0, NULL, 4, buf }; + u8 b[4]; + struct usb_req req = { CMD_IR_GET, 0, 0, NULL, 4, b }; ret = af9035_ctrl_msg(d, &req); if (ret == 1) @@ -1773,23 +1772,11 @@ 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]); - } - - dev_dbg(&d->udev->dev, "%s: %*ph\n", __func__, 4, buf); + dev_dbg(&d->udev->dev, "%s: %*ph\n", __func__, 4, b); - rc_keydown(d->rc_dev, RC_TYPE_NEC, key, 0); + rc_keydown(d->rc_dev, RC_TYPE_NEC, + RC_SCANCODE_NEC32(b[0] << 24 | b[1] << 16 | b[2] << 8 | b[3]), + 0); return 0; 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/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c index 77dcfdf..bef7b2c 100644 --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c @@ -1432,7 +1432,7 @@ static int rtl2831u_rc_query(struct dvb_usb_device *d) int ret, i; struct rtl28xxu_dev *dev = d->priv; u8 buf[5]; - u32 rc_code; + u64 rc_code; struct rtl28xxu_reg_val rc_nec_tab[] = { { 0x3033, 0x80 }, { 0x3020, 0x43 }, @@ -1466,20 +1466,10 @@ 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 605b090..2b059ca 100644 --- a/drivers/media/usb/dvb-usb/dib0700_core.c +++ b/drivers/media/usb/dvb-usb/dib0700_core.c @@ -722,26 +722,14 @@ static void dib0700_rc_urb_completion(struct urb *purb) poll_reply->nec.data == 0x00 && poll_reply->nec.not_data == 0xff) { poll_reply->data_state = 2; - break; + rc_repeat(d->rc_dev); + goto resubmit; } - if ((poll_reply->nec.data ^ poll_reply->nec.not_data) != 0xff) { - deb_data("NEC32 protocol\n"); - keycode = RC_SCANCODE_NEC32(poll_reply->nec.system << 24 | - poll_reply->nec.not_system << 16 | - poll_reply->nec.data << 8 | - poll_reply->nec.not_data); - } else if ((poll_reply->nec.system ^ poll_reply->nec.not_system) != 0xff) { - deb_data("NEC extended protocol\n"); - keycode = RC_SCANCODE_NECX(poll_reply->nec.system << 8 | - poll_reply->nec.not_system, - poll_reply->nec.data); - - } else { - deb_data("NEC normal protocol\n"); - keycode = RC_SCANCODE_NEC(poll_reply->nec.system, - poll_reply->nec.data); - } + keycode = RC_SCANCODE_NEC32(poll_reply->nec.system << 24 | + poll_reply->nec.not_system << 16 | + poll_reply->nec.data << 8 | + poll_reply->nec.not_data); break; default: diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c index 4007356..2b5c6a1 100644 --- a/drivers/media/usb/em28xx/em28xx-input.c +++ b/drivers/media/usb/em28xx/em28xx-input.c @@ -67,7 +67,6 @@ struct em28xx_IR { /* poll decoder */ int polling; struct delayed_work work; - unsigned int full_code:1; unsigned int last_readcount; u64 rc_type; @@ -258,18 +257,10 @@ static int em2874_polling_getkey(struct em28xx_IR *ir, 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 = 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] << 0)); break; case RC_BIT_RC6_0: @@ -327,16 +318,11 @@ static void em28xx_ir_handle_key(struct em28xx_IR *ir) dprintk("%s: toggle: %d, count: %d, key 0x%04x\n", __func__, poll_result.toggle_bit, poll_result.read_count, 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); + + rc_keydown(ir->rc, + poll_result.protocol, + poll_result.scancode, + poll_result.toggle_bit); if (ir->dev->chip_id == CHIP_ID_EM2874 || ir->dev->chip_id == CHIP_ID_EM2884) @@ -387,11 +373,9 @@ static int em2860_ir_change_protocol(struct rc_dev *rc_dev, u64 *rc_type) /* Adjust xclk based on IR table for RC5/NEC tables */ if (*rc_type & RC_BIT_RC5) { dev->board.xclk |= EM28XX_XCLK_IR_RC5_MODE; - ir->full_code = 1; *rc_type = RC_BIT_RC5; } else if (*rc_type & RC_BIT_NEC) { dev->board.xclk &= ~EM28XX_XCLK_IR_RC5_MODE; - ir->full_code = 1; *rc_type = RC_BIT_NEC; } else if (*rc_type & RC_BIT_UNKNOWN) { *rc_type = RC_BIT_UNKNOWN; @@ -416,17 +400,14 @@ static int em2874_ir_change_protocol(struct rc_dev *rc_dev, u64 *rc_type) /* Adjust xclk and set type based on IR table for RC5/NEC/RC6 tables */ if (*rc_type & RC_BIT_RC5) { dev->board.xclk |= EM28XX_XCLK_IR_RC5_MODE; - ir->full_code = 1; *rc_type = RC_BIT_RC5; } else if (*rc_type & RC_BIT_NEC) { dev->board.xclk &= ~EM28XX_XCLK_IR_RC5_MODE; ir_config = EM2874_IR_NEC | EM2874_IR_NEC_NO_PARITY; - ir->full_code = 1; *rc_type = RC_BIT_NEC; } else if (*rc_type & RC_BIT_RC6_0) { dev->board.xclk |= EM28XX_XCLK_IR_RC5_MODE; ir_config = EM2874_IR_RC6_MODE_0; - ir->full_code = 1; *rc_type = RC_BIT_RC6_0; } else if (*rc_type & RC_BIT_UNKNOWN) { *rc_type = RC_BIT_UNKNOWN; diff --git a/include/media/rc-map.h b/include/media/rc-map.h index e7a1514..d0bbfc1 100644 --- a/include/media/rc-map.h +++ b/include/media/rc-map.h @@ -34,6 +34,8 @@ enum rc_type { RC_TYPE_XMP = 19, /* XMP protocol */ }; +#define rc_bitmap_to_type(x) (fls64(x) - 1) + #define RC_BIT_NONE 0 #define RC_BIT_UNKNOWN (1 << RC_TYPE_UNKNOWN) #define RC_BIT_OTHER (1 << RC_TYPE_OTHER) @@ -68,14 +70,22 @@ 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; u32 keycode; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] rc-core: use the full 32 bits for NEC scancodes 2015-04-06 11:23 ` [PATCH 1/2] rc-core: use the full 32 bits for NEC scancodes David Härdeman @ 2015-05-14 20:57 ` Mauro Carvalho Chehab 2015-05-20 20:24 ` David Härdeman 0 siblings, 1 reply; 8+ messages in thread From: Mauro Carvalho Chehab @ 2015-05-14 20:57 UTC (permalink / raw) To: David Härdeman; +Cc: linux-media Em Mon, 06 Apr 2015 13:23:08 +0200 David Härdeman <david@hardeman.nu> escreveu: > Using the full 32 bits for all kinds of NEC scancodes simplifies rc-core > and the nec decoder without any loss of functionality. At the same time > it ensures that scancodes for NEC16/NEC24/NEC32 do not overlap and > removes any ambiguity. > > For example, before this patch, consider these two NEC messages: > NEC16 message to address 0x05, command 0x03 > NEC24 message to address 0x0005, command 0x03 > > They'll both have scancode 0x00000503, and there's no way to tell which > message was received. > > In order to maintain backwards compatibility, some heuristics are added > in rc-main.c to convert scancodes to NEC32 as necessary when userspace > adds entries to the keytable using the regular input ioctls. > > No conversion will be made for the newer "rc_keymap_entry" based ioctls > (see the next patch). > > Signed-off-by: David Härdeman <david@hardeman.nu> Checkpatch has something to say about this patch: WARNING: else is not generally useful after a break or return #140: FILE: drivers/media/rc/rc-main.c:357: + return b3 << 24 | b2 << 16 | b1 << 8 | ((u8)~b1); + else WARNING: line over 80 characters #259: FILE: drivers/media/usb/dvb-usb-v2/af9035.c:1778: + RC_SCANCODE_NEC32(b[0] << 24 | b[1] << 16 | b[2] << 8 | b[3]), ERROR: space prohibited after that open parenthesis '(' #479: FILE: include/media/rc-map.h:79: + ((( (addr) & 0xff) << 24) | \ ERROR: space prohibited after that open parenthesis '(' #481: FILE: include/media/rc-map.h:81: + (( (cmd) & 0xff) << 8 ) | \ ERROR: space prohibited before that close parenthesis ')' #481: FILE: include/media/rc-map.h:81: + (( (cmd) & 0xff) << 8 ) | \ ERROR: space prohibited before that close parenthesis ')' #482: FILE: include/media/rc-map.h:82: + ((~(cmd) & 0xff) << 0 )) ERROR: space prohibited after that open parenthesis '(' #484: FILE: include/media/rc-map.h:84: + ((( (addr) & 0xffff) << 16) | \ ERROR: space prohibited after that open parenthesis '(' #485: FILE: include/media/rc-map.h:85: + (( (cmd) & 0x00ff) << 8) | \ > --- > drivers/media/rc/ir-nec-decoder.c | 26 ++------------ > drivers/media/rc/rc-main.c | 54 +++++++++++++++++++++++++++++- > drivers/media/usb/dvb-usb-v2/af9015.c | 22 ++---------- > drivers/media/usb/dvb-usb-v2/af9035.c | 25 +++----------- > drivers/media/usb/dvb-usb-v2/az6007.c | 16 ++------- > drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 20 +++-------- > drivers/media/usb/dvb-usb/dib0700_core.c | 24 +++---------- > drivers/media/usb/em28xx/em28xx-input.c | 37 +++++---------------- > include/media/rc-map.h | 16 +++++++-- > 9 files changed, 102 insertions(+), 138 deletions(-) > > diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c > index 7b81fec..16907c1 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 (!(dev->enabled_protocols & RC_BIT_NEC)) > return 0; > @@ -163,28 +162,9 @@ 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/rc-main.c b/drivers/media/rc/rc-main.c > index d068c4e..3379379 100644 > --- a/drivers/media/rc/rc-main.c > +++ b/drivers/media/rc/rc-main.c > @@ -317,6 +317,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) == 1) > + return rc_bitmap_to_type(rdev->enabled_protocols); > + else if (hweight64(rdev->allowed_protocols) == 1) > + return rc_bitmap_to_type(rdev->allowed_protocols); > + 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 u64 to_nec32(u64 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 > @@ -349,6 +392,9 @@ static int ir_setkeycode(struct input_dev *idev, > if (retval) > goto out; > > + if (guess_protocol(rdev) == 0 > + scancode = to_nec32(scancode); This function can be called from userspace. I can't see how this would do the right thing if more than one protocol is enabled. > + > index = ir_establish_scancode(rdev, rc_map, scancode, true); > if (index >= rc_map->len) { > retval = -ENOMEM; > @@ -389,7 +435,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; > @@ -463,6 +512,8 @@ static int ir_getkeycode(struct input_dev *idev, > if (retval) > goto out; > > + if (guess_protocol(rdev) == RC_TYPE_NEC) > + scancode = to_nec32(scancode); This also can be called from userspace. It should not return different scancodes for the same mapping if just NEC is enabled or if more protocols are enabled. > index = ir_lookup_by_scancode(rc_map, scancode); > } > > @@ -660,7 +711,6 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol, > > led_trigger_event(led_feedback, LED_FULL); > } > - > input_sync(dev->input_dev); > } > > diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c > index 16c0b7d..4cc1463 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 80a29f5..6c00233 100644 > --- a/drivers/media/usb/dvb-usb-v2/af9035.c > +++ b/drivers/media/usb/dvb-usb-v2/af9035.c > @@ -1763,9 +1763,8 @@ err: > static int af9035_rc_query(struct dvb_usb_device *d) > { > int ret; > - u32 key; > - u8 buf[4]; > - struct usb_req req = { CMD_IR_GET, 0, 0, NULL, 4, buf }; > + u8 b[4]; > + struct usb_req req = { CMD_IR_GET, 0, 0, NULL, 4, b }; > > ret = af9035_ctrl_msg(d, &req); > if (ret == 1) > @@ -1773,23 +1772,11 @@ 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]); > - } > - > - dev_dbg(&d->udev->dev, "%s: %*ph\n", __func__, 4, buf); > + dev_dbg(&d->udev->dev, "%s: %*ph\n", __func__, 4, b); > > - rc_keydown(d->rc_dev, RC_TYPE_NEC, key, 0); > + rc_keydown(d->rc_dev, RC_TYPE_NEC, > + RC_SCANCODE_NEC32(b[0] << 24 | b[1] << 16 | b[2] << 8 | b[3]), > + 0); > > return 0; > > 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/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > index 77dcfdf..bef7b2c 100644 > --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > @@ -1432,7 +1432,7 @@ static int rtl2831u_rc_query(struct dvb_usb_device *d) > int ret, i; > struct rtl28xxu_dev *dev = d->priv; > u8 buf[5]; > - u32 rc_code; > + u64 rc_code; > struct rtl28xxu_reg_val rc_nec_tab[] = { > { 0x3033, 0x80 }, > { 0x3020, 0x43 }, > @@ -1466,20 +1466,10 @@ 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 605b090..2b059ca 100644 > --- a/drivers/media/usb/dvb-usb/dib0700_core.c > +++ b/drivers/media/usb/dvb-usb/dib0700_core.c > @@ -722,26 +722,14 @@ static void dib0700_rc_urb_completion(struct urb *purb) > poll_reply->nec.data == 0x00 && > poll_reply->nec.not_data == 0xff) { > poll_reply->data_state = 2; > - break; > + rc_repeat(d->rc_dev); > + goto resubmit; > } > > - if ((poll_reply->nec.data ^ poll_reply->nec.not_data) != 0xff) { > - deb_data("NEC32 protocol\n"); > - keycode = RC_SCANCODE_NEC32(poll_reply->nec.system << 24 | > - poll_reply->nec.not_system << 16 | > - poll_reply->nec.data << 8 | > - poll_reply->nec.not_data); > - } else if ((poll_reply->nec.system ^ poll_reply->nec.not_system) != 0xff) { > - deb_data("NEC extended protocol\n"); > - keycode = RC_SCANCODE_NECX(poll_reply->nec.system << 8 | > - poll_reply->nec.not_system, > - poll_reply->nec.data); > - > - } else { > - deb_data("NEC normal protocol\n"); > - keycode = RC_SCANCODE_NEC(poll_reply->nec.system, > - poll_reply->nec.data); > - } > + keycode = RC_SCANCODE_NEC32(poll_reply->nec.system << 24 | > + poll_reply->nec.not_system << 16 | > + poll_reply->nec.data << 8 | > + poll_reply->nec.not_data); > > break; > default: > diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c > index 4007356..2b5c6a1 100644 > --- a/drivers/media/usb/em28xx/em28xx-input.c > +++ b/drivers/media/usb/em28xx/em28xx-input.c > @@ -67,7 +67,6 @@ struct em28xx_IR { > /* poll decoder */ > int polling; > struct delayed_work work; > - unsigned int full_code:1; > unsigned int last_readcount; > u64 rc_type; > > @@ -258,18 +257,10 @@ static int em2874_polling_getkey(struct em28xx_IR *ir, > 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 = 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] << 0)); > break; > > case RC_BIT_RC6_0: > @@ -327,16 +318,11 @@ static void em28xx_ir_handle_key(struct em28xx_IR *ir) > dprintk("%s: toggle: %d, count: %d, key 0x%04x\n", __func__, > poll_result.toggle_bit, poll_result.read_count, > 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); > + > + rc_keydown(ir->rc, > + poll_result.protocol, > + poll_result.scancode, > + poll_result.toggle_bit); This hunk will break drivers. if !full_code, there are just 8 bits for the protocol, and not 16. The table, however, may have scancodes for 16-bits NEC. The devices that use !full_code generally has a very crappy micro-controller that returns only 8 bits of either RC5 or NEC. > > if (ir->dev->chip_id == CHIP_ID_EM2874 || > ir->dev->chip_id == CHIP_ID_EM2884) > @@ -387,11 +373,9 @@ static int em2860_ir_change_protocol(struct rc_dev *rc_dev, u64 *rc_type) > /* Adjust xclk based on IR table for RC5/NEC tables */ > if (*rc_type & RC_BIT_RC5) { > dev->board.xclk |= EM28XX_XCLK_IR_RC5_MODE; > - ir->full_code = 1; > *rc_type = RC_BIT_RC5; > } else if (*rc_type & RC_BIT_NEC) { > dev->board.xclk &= ~EM28XX_XCLK_IR_RC5_MODE; > - ir->full_code = 1; > *rc_type = RC_BIT_NEC; > } else if (*rc_type & RC_BIT_UNKNOWN) { > *rc_type = RC_BIT_UNKNOWN; > @@ -416,17 +400,14 @@ static int em2874_ir_change_protocol(struct rc_dev *rc_dev, u64 *rc_type) > /* Adjust xclk and set type based on IR table for RC5/NEC/RC6 tables */ > if (*rc_type & RC_BIT_RC5) { > dev->board.xclk |= EM28XX_XCLK_IR_RC5_MODE; > - ir->full_code = 1; > *rc_type = RC_BIT_RC5; > } else if (*rc_type & RC_BIT_NEC) { > dev->board.xclk &= ~EM28XX_XCLK_IR_RC5_MODE; > ir_config = EM2874_IR_NEC | EM2874_IR_NEC_NO_PARITY; > - ir->full_code = 1; > *rc_type = RC_BIT_NEC; > } else if (*rc_type & RC_BIT_RC6_0) { > dev->board.xclk |= EM28XX_XCLK_IR_RC5_MODE; > ir_config = EM2874_IR_RC6_MODE_0; > - ir->full_code = 1; > *rc_type = RC_BIT_RC6_0; > } else if (*rc_type & RC_BIT_UNKNOWN) { > *rc_type = RC_BIT_UNKNOWN; > diff --git a/include/media/rc-map.h b/include/media/rc-map.h > index e7a1514..d0bbfc1 100644 > --- a/include/media/rc-map.h > +++ b/include/media/rc-map.h > @@ -34,6 +34,8 @@ enum rc_type { > RC_TYPE_XMP = 19, /* XMP protocol */ > }; > > +#define rc_bitmap_to_type(x) (fls64(x) - 1) > + > #define RC_BIT_NONE 0 > #define RC_BIT_UNKNOWN (1 << RC_TYPE_UNKNOWN) > #define RC_BIT_OTHER (1 << RC_TYPE_OTHER) > @@ -68,14 +70,22 @@ 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; > u32 keycode; > > -- > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] rc-core: use the full 32 bits for NEC scancodes 2015-05-14 20:57 ` Mauro Carvalho Chehab @ 2015-05-20 20:24 ` David Härdeman 2015-06-13 23:49 ` David Härdeman 0 siblings, 1 reply; 8+ messages in thread From: David Härdeman @ 2015-05-20 20:24 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media On Thu, May 14, 2015 at 05:57:39PM -0300, Mauro Carvalho Chehab wrote: >Em Mon, 06 Apr 2015 13:23:08 +0200 >David Härdeman <david@hardeman.nu> escreveu: > >> Using the full 32 bits for all kinds of NEC scancodes simplifies rc-core >> and the nec decoder without any loss of functionality. At the same time >> it ensures that scancodes for NEC16/NEC24/NEC32 do not overlap and >> removes any ambiguity. >> >> For example, before this patch, consider these two NEC messages: >> NEC16 message to address 0x05, command 0x03 >> NEC24 message to address 0x0005, command 0x03 >> >> They'll both have scancode 0x00000503, and there's no way to tell which >> message was received. >> >> In order to maintain backwards compatibility, some heuristics are added >> in rc-main.c to convert scancodes to NEC32 as necessary when userspace >> adds entries to the keytable using the regular input ioctls. >> >> No conversion will be made for the newer "rc_keymap_entry" based ioctls >> (see the next patch). >> >> Signed-off-by: David Härdeman <david@hardeman.nu> > >Checkpatch has something to say about this patch: > >WARNING: else is not generally useful after a break or return >#140: FILE: drivers/media/rc/rc-main.c:357: >+ return b3 << 24 | b2 << 16 | b1 << 8 | ((u8)~b1); >+ else That seems like a bogus warning?...the return line is after an else if... >WARNING: line over 80 characters >#259: FILE: drivers/media/usb/dvb-usb-v2/af9035.c:1778: >+ RC_SCANCODE_NEC32(b[0] << 24 | b[1] << 16 | b[2] << 8 | b[3]), That file isn't 80 chars per line "clean" anyway (and 80 char lines do not seem to be a "hard" rule?). Anyway, I've changed it in my local version... >ERROR: space prohibited after that open parenthesis '(' >#479: FILE: include/media/rc-map.h:79: >+ ((( (addr) & 0xff) << 24) | \ > >ERROR: space prohibited after that open parenthesis '(' >#481: FILE: include/media/rc-map.h:81: >+ (( (cmd) & 0xff) << 8 ) | \ > >ERROR: space prohibited before that close parenthesis ')' >#481: FILE: include/media/rc-map.h:81: >+ (( (cmd) & 0xff) << 8 ) | \ > >ERROR: space prohibited before that close parenthesis ')' >#482: FILE: include/media/rc-map.h:82: >+ ((~(cmd) & 0xff) << 0 )) > >ERROR: space prohibited after that open parenthesis '(' >#484: FILE: include/media/rc-map.h:84: >+ ((( (addr) & 0xffff) << 16) | \ > >ERROR: space prohibited after that open parenthesis '(' >#485: FILE: include/media/rc-map.h:85: >+ (( (cmd) & 0x00ff) << 8) | \ I don't think "fixing" any of these help readability, but I've changed them in my local version. >> --- >> drivers/media/rc/ir-nec-decoder.c | 26 ++------------ >> drivers/media/rc/rc-main.c | 54 +++++++++++++++++++++++++++++- >> drivers/media/usb/dvb-usb-v2/af9015.c | 22 ++---------- >> drivers/media/usb/dvb-usb-v2/af9035.c | 25 +++----------- >> drivers/media/usb/dvb-usb-v2/az6007.c | 16 ++------- >> drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 20 +++-------- >> drivers/media/usb/dvb-usb/dib0700_core.c | 24 +++---------- >> drivers/media/usb/em28xx/em28xx-input.c | 37 +++++---------------- >> include/media/rc-map.h | 16 +++++++-- >> 9 files changed, 102 insertions(+), 138 deletions(-) >> >> diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c >> index 7b81fec..16907c1 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 (!(dev->enabled_protocols & RC_BIT_NEC)) >> return 0; >> @@ -163,28 +162,9 @@ 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/rc-main.c b/drivers/media/rc/rc-main.c >> index d068c4e..3379379 100644 >> --- a/drivers/media/rc/rc-main.c >> +++ b/drivers/media/rc/rc-main.c >> @@ -317,6 +317,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) == 1) >> + return rc_bitmap_to_type(rdev->enabled_protocols); >> + else if (hweight64(rdev->allowed_protocols) == 1) >> + return rc_bitmap_to_type(rdev->allowed_protocols); >> + else >> + return rc_map->rc_type; >> +} ^^^^ This function is the most important one to understand in order to understand how the heuristics work... >> + >> +/** >> + * 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 u64 to_nec32(u64 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 >> @@ -349,6 +392,9 @@ static int ir_setkeycode(struct input_dev *idev, >> if (retval) >> goto out; >> >> + if (guess_protocol(rdev) == 0 >> + scancode = to_nec32(scancode); > >This function can be called from userspace. I can't see how this would do >the right thing if more than one protocol is enabled. > >> + >> index = ir_establish_scancode(rdev, rc_map, scancode, true); >> if (index >= rc_map->len) { >> retval = -ENOMEM; >> @@ -389,7 +435,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; >> @@ -463,6 +512,8 @@ static int ir_getkeycode(struct input_dev *idev, >> if (retval) >> goto out; >> >> + if (guess_protocol(rdev) == RC_TYPE_NEC) >> + scancode = to_nec32(scancode); > >This also can be called from userspace. It should not return different >scancodes for the same mapping if just NEC is enabled or if more protocols >are enabled. There is no way to do this in a 100% backwards compatible way, that's why the patch description uses the word "heuristics". I've tried different approaches (such as introducing and using a kernel-internal RC_TYPE_ANY protocol for legacy ioctl() calls) but none of them solve the problem 100%. The current API is also broken, but in a different way. If you set a scancode <-> keycode mapping right now using the current ioctl()s, you can get different results with the exact same mapping but with different RX hardware (which defeats the whole idea of having a kernel API for remote controls...) even though there is enough information to do the right thing (one example is already given in the patch comments)...that is BAD. The heuristics can also get it wrong...in slightly different situations (e.g. if you load a hardware driver that supports nec and rc-5, but has a rc-5 default keymap, then enable both nec and rc-5 from userspace, and finally set keymap entries using nec scancodes...in which case they'll be interpreted as rc-5 keymap scancodes). So, we trade one kind of breakage for another...but the alternative kind that I'm proposing here at least paves the way for the updated ioctls which solve the ambiguity. And distributions can make sure to ship updated userspace tools together with an updated kernel (configuration tool updates together with kernel updates is one of those "special" cases that e.g. LWN covers every now and then). I'm not saying the situation is ideal. But at least I'm trying to fix it once and for all. Do you have a better solution in mind other than to simply keep throwing away all protocol information and ignoring scancode overlaps and inconsistencies? >> index = ir_lookup_by_scancode(rc_map, scancode); >> } >> >> @@ -660,7 +711,6 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol, >> >> led_trigger_event(led_feedback, LED_FULL); >> } >> - >> input_sync(dev->input_dev); >> } >> >> diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c >> index 16c0b7d..4cc1463 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 80a29f5..6c00233 100644 >> --- a/drivers/media/usb/dvb-usb-v2/af9035.c >> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c >> @@ -1763,9 +1763,8 @@ err: >> static int af9035_rc_query(struct dvb_usb_device *d) >> { >> int ret; >> - u32 key; >> - u8 buf[4]; >> - struct usb_req req = { CMD_IR_GET, 0, 0, NULL, 4, buf }; >> + u8 b[4]; >> + struct usb_req req = { CMD_IR_GET, 0, 0, NULL, 4, b }; >> >> ret = af9035_ctrl_msg(d, &req); >> if (ret == 1) >> @@ -1773,23 +1772,11 @@ 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]); >> - } >> - >> - dev_dbg(&d->udev->dev, "%s: %*ph\n", __func__, 4, buf); >> + dev_dbg(&d->udev->dev, "%s: %*ph\n", __func__, 4, b); >> >> - rc_keydown(d->rc_dev, RC_TYPE_NEC, key, 0); >> + rc_keydown(d->rc_dev, RC_TYPE_NEC, >> + RC_SCANCODE_NEC32(b[0] << 24 | b[1] << 16 | b[2] << 8 | b[3]), >> + 0); >> >> return 0; >> >> 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/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c >> index 77dcfdf..bef7b2c 100644 >> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c >> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c >> @@ -1432,7 +1432,7 @@ static int rtl2831u_rc_query(struct dvb_usb_device *d) >> int ret, i; >> struct rtl28xxu_dev *dev = d->priv; >> u8 buf[5]; >> - u32 rc_code; >> + u64 rc_code; >> struct rtl28xxu_reg_val rc_nec_tab[] = { >> { 0x3033, 0x80 }, >> { 0x3020, 0x43 }, >> @@ -1466,20 +1466,10 @@ 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 605b090..2b059ca 100644 >> --- a/drivers/media/usb/dvb-usb/dib0700_core.c >> +++ b/drivers/media/usb/dvb-usb/dib0700_core.c >> @@ -722,26 +722,14 @@ static void dib0700_rc_urb_completion(struct urb *purb) >> poll_reply->nec.data == 0x00 && >> poll_reply->nec.not_data == 0xff) { >> poll_reply->data_state = 2; >> - break; >> + rc_repeat(d->rc_dev); >> + goto resubmit; >> } >> >> - if ((poll_reply->nec.data ^ poll_reply->nec.not_data) != 0xff) { >> - deb_data("NEC32 protocol\n"); >> - keycode = RC_SCANCODE_NEC32(poll_reply->nec.system << 24 | >> - poll_reply->nec.not_system << 16 | >> - poll_reply->nec.data << 8 | >> - poll_reply->nec.not_data); >> - } else if ((poll_reply->nec.system ^ poll_reply->nec.not_system) != 0xff) { >> - deb_data("NEC extended protocol\n"); >> - keycode = RC_SCANCODE_NECX(poll_reply->nec.system << 8 | >> - poll_reply->nec.not_system, >> - poll_reply->nec.data); >> - >> - } else { >> - deb_data("NEC normal protocol\n"); >> - keycode = RC_SCANCODE_NEC(poll_reply->nec.system, >> - poll_reply->nec.data); >> - } >> + keycode = RC_SCANCODE_NEC32(poll_reply->nec.system << 24 | >> + poll_reply->nec.not_system << 16 | >> + poll_reply->nec.data << 8 | >> + poll_reply->nec.not_data); >> >> break; >> default: >> diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c >> index 4007356..2b5c6a1 100644 >> --- a/drivers/media/usb/em28xx/em28xx-input.c >> +++ b/drivers/media/usb/em28xx/em28xx-input.c >> @@ -67,7 +67,6 @@ struct em28xx_IR { >> /* poll decoder */ >> int polling; >> struct delayed_work work; >> - unsigned int full_code:1; >> unsigned int last_readcount; >> u64 rc_type; >> >> @@ -258,18 +257,10 @@ static int em2874_polling_getkey(struct em28xx_IR *ir, >> 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 = 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] << 0)); >> break; >> >> case RC_BIT_RC6_0: >> @@ -327,16 +318,11 @@ static void em28xx_ir_handle_key(struct em28xx_IR *ir) >> dprintk("%s: toggle: %d, count: %d, key 0x%04x\n", __func__, >> poll_result.toggle_bit, poll_result.read_count, >> 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); >> + >> + rc_keydown(ir->rc, >> + poll_result.protocol, >> + poll_result.scancode, >> + poll_result.toggle_bit); > >This hunk will break drivers. if !full_code, there are just 8 bits >for the protocol, and not 16. The table, however, may have scancodes >for 16-bits NEC. > >The devices that use !full_code generally has a very crappy >micro-controller that returns only 8 bits of either RC5 or NEC. > >> >> if (ir->dev->chip_id == CHIP_ID_EM2874 || >> ir->dev->chip_id == CHIP_ID_EM2884) >> @@ -387,11 +373,9 @@ static int em2860_ir_change_protocol(struct rc_dev *rc_dev, u64 *rc_type) >> /* Adjust xclk based on IR table for RC5/NEC tables */ >> if (*rc_type & RC_BIT_RC5) { >> dev->board.xclk |= EM28XX_XCLK_IR_RC5_MODE; >> - ir->full_code = 1; >> *rc_type = RC_BIT_RC5; >> } else if (*rc_type & RC_BIT_NEC) { >> dev->board.xclk &= ~EM28XX_XCLK_IR_RC5_MODE; >> - ir->full_code = 1; >> *rc_type = RC_BIT_NEC; >> } else if (*rc_type & RC_BIT_UNKNOWN) { >> *rc_type = RC_BIT_UNKNOWN; >> @@ -416,17 +400,14 @@ static int em2874_ir_change_protocol(struct rc_dev *rc_dev, u64 *rc_type) >> /* Adjust xclk and set type based on IR table for RC5/NEC/RC6 tables */ >> if (*rc_type & RC_BIT_RC5) { >> dev->board.xclk |= EM28XX_XCLK_IR_RC5_MODE; >> - ir->full_code = 1; >> *rc_type = RC_BIT_RC5; >> } else if (*rc_type & RC_BIT_NEC) { >> dev->board.xclk &= ~EM28XX_XCLK_IR_RC5_MODE; >> ir_config = EM2874_IR_NEC | EM2874_IR_NEC_NO_PARITY; >> - ir->full_code = 1; >> *rc_type = RC_BIT_NEC; >> } else if (*rc_type & RC_BIT_RC6_0) { >> dev->board.xclk |= EM28XX_XCLK_IR_RC5_MODE; >> ir_config = EM2874_IR_RC6_MODE_0; >> - ir->full_code = 1; >> *rc_type = RC_BIT_RC6_0; >> } else if (*rc_type & RC_BIT_UNKNOWN) { >> *rc_type = RC_BIT_UNKNOWN; >> diff --git a/include/media/rc-map.h b/include/media/rc-map.h >> index e7a1514..d0bbfc1 100644 >> --- a/include/media/rc-map.h >> +++ b/include/media/rc-map.h >> @@ -34,6 +34,8 @@ enum rc_type { >> RC_TYPE_XMP = 19, /* XMP protocol */ >> }; >> >> +#define rc_bitmap_to_type(x) (fls64(x) - 1) >> + >> #define RC_BIT_NONE 0 >> #define RC_BIT_UNKNOWN (1 << RC_TYPE_UNKNOWN) >> #define RC_BIT_OTHER (1 << RC_TYPE_OTHER) >> @@ -68,14 +70,22 @@ 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; >> u32 keycode; >> >> -- >> 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 > -- David Härdeman ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] rc-core: use the full 32 bits for NEC scancodes 2015-05-20 20:24 ` David Härdeman @ 2015-06-13 23:49 ` David Härdeman 0 siblings, 0 replies; 8+ messages in thread From: David Härdeman @ 2015-06-13 23:49 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media On 2015-05-20 22:24, David Härdeman wrote: > On Thu, May 14, 2015 at 05:57:39PM -0300, Mauro Carvalho Chehab wrote: >> Em Mon, 06 Apr 2015 13:23:08 +0200 >> David Härdeman <david@hardeman.nu> escreveu: .... >>> +static inline enum rc_type guess_protocol(struct rc_dev *rdev) >>> +{ >>> + struct rc_map *rc_map = &rdev->rc_map; >>> + >>> + if (hweight64(rdev->enabled_protocols) == 1) >>> + return rc_bitmap_to_type(rdev->enabled_protocols); >>> + else if (hweight64(rdev->allowed_protocols) == 1) >>> + return rc_bitmap_to_type(rdev->allowed_protocols); >>> + else >>> + return rc_map->rc_type; >>> +} > > ^^^^ > This function is the most important one to understand in order to > understand how the heuristics work... > >>> + >>> +/** >>> + * 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 u64 to_nec32(u64 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 >>> @@ -349,6 +392,9 @@ static int ir_setkeycode(struct input_dev *idev, >>> if (retval) >>> goto out; >>> >>> + if (guess_protocol(rdev) == 0 >>> + scancode = to_nec32(scancode); >> >> This function can be called from userspace. I can't see how this would >> do >> the right thing if more than one protocol is enabled. >> >>> + >>> index = ir_establish_scancode(rdev, rc_map, scancode, true); >>> if (index >= rc_map->len) { >>> retval = -ENOMEM; >>> @@ -389,7 +435,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; >>> @@ -463,6 +512,8 @@ static int ir_getkeycode(struct input_dev *idev, >>> if (retval) >>> goto out; >>> >>> + if (guess_protocol(rdev) == RC_TYPE_NEC) >>> + scancode = to_nec32(scancode); >> >> This also can be called from userspace. It should not return different >> scancodes for the same mapping if just NEC is enabled or if more >> protocols >> are enabled. > > There is no way to do this in a 100% backwards compatible way, that's > why the patch description uses the word "heuristics". > > I've tried different approaches (such as introducing and using a > kernel-internal RC_TYPE_ANY protocol for legacy ioctl() calls) but none > of them solve the problem 100%. > > The current API is also broken, but in a different way. If you set a > scancode <-> keycode mapping right now using the current ioctl()s, you > can get different results with the exact same mapping but with > different > RX hardware (which defeats the whole idea of having a kernel API for > remote controls...) even though there is enough information to do the > right thing (one example is already given in the patch comments)...that > is BAD. > > The heuristics can also get it wrong...in slightly different situations > (e.g. if you load a hardware driver that supports nec and rc-5, but has > a rc-5 default keymap, then enable both nec and rc-5 from userspace, > and > finally set keymap entries using nec scancodes...in which case they'll > be interpreted as rc-5 keymap scancodes). > > So, we trade one kind of breakage for another...but the alternative > kind > that I'm proposing here at least paves the way for the updated ioctls > which solve the ambiguity. > > And distributions can make sure to ship updated userspace tools > together > with an updated kernel (configuration tool updates together with kernel > updates is one of those "special" cases that e.g. LWN covers every now > and then). > > I'm not saying the situation is ideal. But at least I'm trying to fix > it > once and for all. > > Do you have a better solution in mind other than to simply keep > throwing > away all protocol information and ignoring scancode overlaps and > inconsistencies? It wasn't a rhetorical question...this is an issue that needs to be fixed one way or another...do you have a better solution in mind? ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] rc-core: don't throw away protocol information 2015-04-06 11:23 [PATCH 0/2] NEC scancodes and protocols in keymaps - v2 David Härdeman 2015-04-06 11:23 ` [PATCH 1/2] rc-core: use the full 32 bits for NEC scancodes David Härdeman @ 2015-04-06 11:23 ` David Härdeman 2015-05-14 21:05 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 8+ messages in thread From: David Härdeman @ 2015-04-06 11:23 UTC (permalink / raw) To: linux-media; +Cc: m.chehab It is currently impossible to distinguish between scancodes which have been generated using different protocols (and scancodes can, and will, overlap). For example: RC5 message to address 0x00, command 0x03 has scancode 0x00000503 JVC message to address 0x00, command 0x03 has scancode 0x00000503 It is only possible to distinguish (and parse) scancodes by known the scancode *and* the protocol. 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). This also means that struct rc_map no longer hardcodes the protocol, meaning that keytables with mixed entries are possible. Heuristics are also added to hopefully do the right thing with older ioctls in order to preserve backwards compatibility. Signed-off-by: David Härdeman <david@hardeman.nu> --- drivers/media/rc/ati_remote.c | 1 drivers/media/rc/imon.c | 7 + drivers/media/rc/rc-main.c | 196 +++++++++++++++++++++++++++++------------ include/media/rc-core.h | 26 +++++ include/media/rc-map.h | 7 + 5 files changed, 172 insertions(+), 65 deletions(-) diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c index a356318..a1df608 100644 --- a/drivers/media/rc/ati_remote.c +++ b/drivers/media/rc/ati_remote.c @@ -544,6 +544,7 @@ static void ati_remote_input_report(struct urb *urb) * set, assume this is a scrollwheel up/down event. */ wheel_keycode = rc_g_keycode_from_table(ati_remote->rdev, + RC_TYPE_OTHER, scancode & 0x78); if (wheel_keycode == KEY_RESERVED) { diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c index 65f80b8..ec4414a 100644 --- a/drivers/media/rc/imon.c +++ b/drivers/media/rc/imon.c @@ -1263,14 +1263,15 @@ static u32 imon_remote_key_lookup(struct imon_context *ictx, u32 scancode) bool is_release_code = false; /* Look for the initial press of a button */ - keycode = rc_g_keycode_from_table(ictx->rdev, scancode); + keycode = rc_g_keycode_from_table(ictx->rdev, ictx->rc_type, scancode); ictx->rc_toggle = 0x0; ictx->rc_scancode = scancode; /* Look for the release of a button */ if (keycode == KEY_RESERVED) { release = scancode & ~0x4000; - keycode = rc_g_keycode_from_table(ictx->rdev, release); + keycode = rc_g_keycode_from_table(ictx->rdev, ictx->rc_type, + release); if (keycode != KEY_RESERVED) is_release_code = true; } @@ -1299,7 +1300,7 @@ static u32 imon_mce_key_lookup(struct imon_context *ictx, u32 scancode) scancode = scancode | MCE_KEY_MASK | MCE_TOGGLE_BIT; ictx->rc_scancode = scancode; - keycode = rc_g_keycode_from_table(ictx->rdev, scancode); + keycode = rc_g_keycode_from_table(ictx->rdev, ictx->rc_type, scancode); /* not used in mce mode, but make sure we know its false */ ictx->release_code = false; diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index 3379379..52b792d 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -103,7 +103,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 = { @@ -119,7 +119,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 * @@ -127,10 +126,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); @@ -225,16 +223,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); } @@ -261,9 +263,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. * @@ -273,7 +275,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; @@ -287,16 +289,27 @@ static unsigned int ir_establish_scancode(struct rc_dev *dev, * indicate the valid bits of the scancodes. */ if (dev->scancode_mask) - scancode &= dev->scancode_mask; + entry->scancode &= dev->scancode_mask; - /* 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 */ @@ -309,7 +322,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++; @@ -331,8 +345,10 @@ static inline enum rc_type guess_protocol(struct rc_dev *rdev) return rc_bitmap_to_type(rdev->enabled_protocols); else if (hweight64(rdev->allowed_protocols) == 1) return rc_bitmap_to_type(rdev->allowed_protocols); + else if (rc_map->len > 0) + return rc_map->scan[0].protocol; else - return rc_map->rc_type; + return RC_TYPE_OTHER; } /** @@ -375,10 +391,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) { @@ -387,19 +405,42 @@ static int ir_setkeycode(struct input_dev *idev, retval = -EINVAL; goto out; } - } else { + } else if (ke->len == sizeof(int)) { + /* Old EVIOCSKEYCODE[_V2] 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, scancode, true); + 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; + + 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); @@ -422,11 +463,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; @@ -434,18 +475,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) @@ -457,6 +499,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 * @@ -464,17 +507,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 (m->protocol > protocol) + end = mid - 1; + else if (m->scancode < scancode) start = mid + 1; - else if (rc_map->scan[mid].scancode > scancode) + else if (m->scancode > scancode) end = mid - 1; else return mid; @@ -495,35 +545,61 @@ 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; + printk(KERN_ERR "ir_getkeycode: ke->len is %i\n", ke->len); 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)) { /* @@ -548,6 +624,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 * @@ -555,7 +632,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; @@ -564,15 +642,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; } @@ -705,8 +784,9 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol, dev->last_keycode = keycode; IR_dprintk(1, "%s: key down event, " - "key 0x%04x, protocol 0x%04x, scancode 0x%08x\n", - dev->input_name, keycode, protocol, scancode); + "key 0x%04x, protocol 0x%04x, scancode 0x%08llx\n", + dev->input_name, keycode, protocol, + (long long unsigned)scancode); input_report_key(dev->input_dev, keycode, 1); led_trigger_event(led_feedback, LED_FULL); @@ -725,10 +805,11 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol, * This routine is used to signal that a key has been pressed on the * remote control. */ -void rc_keydown(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 toggle) +void rc_keydown(struct rc_dev *dev, enum rc_type protocol, + u64 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); @@ -754,10 +835,10 @@ EXPORT_SYMBOL_GPL(rc_keydown); * remote control. The driver must manually call rc_keyup() at a later stage. */ void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol, - u32 scancode, u8 toggle) + u64 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); @@ -1472,9 +1553,14 @@ int rc_register_device(struct rc_dev *dev) } if (dev->change_protocol) { - u64 rc_type = (1ll << rc_map->rc_type); + u64 rc_type = 0; + + if (rc_map->len > 0) + rc_type = (1ll << rc_map->scan[0].protocol); + if (dev->driver_type == RC_DRIVER_IR_RAW) rc_type |= RC_BIT_LIRC; + 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 6b4400c..c414ece 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. @@ -149,7 +167,7 @@ struct rc_dev { struct timer_list timer_keyup; u32 last_keycode; enum rc_type last_protocol; - u32 last_scancode; + u64 last_scancode; u8 last_toggle; u32 timeout; u32 min_timeout; @@ -192,10 +210,10 @@ 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, 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_keydown(struct rc_dev *dev, enum rc_type protocol, u64 scancode, u8 toggle); +void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol, u64 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 d0bbfc1..4f36869 100644 --- a/include/media/rc-map.h +++ b/include/media/rc-map.h @@ -87,8 +87,9 @@ enum rc_type { #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 { @@ -96,7 +97,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] 8+ messages in thread
* Re: [PATCH 2/2] rc-core: don't throw away protocol information 2015-04-06 11:23 ` [PATCH 2/2] rc-core: don't throw away protocol information David Härdeman @ 2015-05-14 21:05 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 8+ messages in thread From: Mauro Carvalho Chehab @ 2015-05-14 21:05 UTC (permalink / raw) To: David Härdeman; +Cc: linux-media Em Mon, 06 Apr 2015 13:23:13 +0200 David Härdeman <david@hardeman.nu> escreveu: > It is currently impossible to distinguish between scancodes which have > been generated using different protocols (and scancodes can, and will, > overlap). > > For example: > RC5 message to address 0x00, command 0x03 has scancode 0x00000503 > JVC message to address 0x00, command 0x03 has scancode 0x00000503 > > It is only possible to distinguish (and parse) scancodes by known the > scancode *and* the protocol. > > 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). > > This also means that struct rc_map no longer hardcodes the protocol, meaning > that keytables with mixed entries are possible. > > Heuristics are also added to hopefully do the right thing with older > ioctls in order to preserve backwards compatibility. > > Signed-off-by: David Härdeman <david@hardeman.nu> > --- > drivers/media/rc/ati_remote.c | 1 > drivers/media/rc/imon.c | 7 + > drivers/media/rc/rc-main.c | 196 +++++++++++++++++++++++++++++------------ > include/media/rc-core.h | 26 +++++ > include/media/rc-map.h | 7 + > 5 files changed, 172 insertions(+), 65 deletions(-) > > diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c > index a356318..a1df608 100644 > --- a/drivers/media/rc/ati_remote.c > +++ b/drivers/media/rc/ati_remote.c > @@ -544,6 +544,7 @@ static void ati_remote_input_report(struct urb *urb) > * set, assume this is a scrollwheel up/down event. > */ > wheel_keycode = rc_g_keycode_from_table(ati_remote->rdev, > + RC_TYPE_OTHER, > scancode & 0x78); > > if (wheel_keycode == KEY_RESERVED) { > diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c > index 65f80b8..ec4414a 100644 > --- a/drivers/media/rc/imon.c > +++ b/drivers/media/rc/imon.c > @@ -1263,14 +1263,15 @@ static u32 imon_remote_key_lookup(struct imon_context *ictx, u32 scancode) > bool is_release_code = false; > > /* Look for the initial press of a button */ > - keycode = rc_g_keycode_from_table(ictx->rdev, scancode); > + keycode = rc_g_keycode_from_table(ictx->rdev, ictx->rc_type, scancode); > ictx->rc_toggle = 0x0; > ictx->rc_scancode = scancode; > > /* Look for the release of a button */ > if (keycode == KEY_RESERVED) { > release = scancode & ~0x4000; > - keycode = rc_g_keycode_from_table(ictx->rdev, release); > + keycode = rc_g_keycode_from_table(ictx->rdev, ictx->rc_type, > + release); > if (keycode != KEY_RESERVED) > is_release_code = true; > } > @@ -1299,7 +1300,7 @@ static u32 imon_mce_key_lookup(struct imon_context *ictx, u32 scancode) > scancode = scancode | MCE_KEY_MASK | MCE_TOGGLE_BIT; > > ictx->rc_scancode = scancode; > - keycode = rc_g_keycode_from_table(ictx->rdev, scancode); > + keycode = rc_g_keycode_from_table(ictx->rdev, ictx->rc_type, scancode); > > /* not used in mce mode, but make sure we know its false */ > ictx->release_code = false; > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c > index 3379379..52b792d 100644 > --- a/drivers/media/rc/rc-main.c > +++ b/drivers/media/rc/rc-main.c > @@ -103,7 +103,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 = { > @@ -119,7 +119,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 > * > @@ -127,10 +126,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); > @@ -225,16 +223,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); > } > @@ -261,9 +263,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. > * > @@ -273,7 +275,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; > @@ -287,16 +289,27 @@ static unsigned int ir_establish_scancode(struct rc_dev *dev, > * indicate the valid bits of the scancodes. > */ > if (dev->scancode_mask) > - scancode &= dev->scancode_mask; > + entry->scancode &= dev->scancode_mask; > > - /* 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; Perhaps because I'm too tired after spending the entire day reviewing patches, but I didn't understand well the algo you're trying to do here. The most important thing: would this be backward-compatible when multiple protocols are enabled and the user is using the legacy struct to set a keycode? I guess not, because I'm not seeing any special check for such case. So, could you better explain (using some comments at this code), how do you expect it to work, especially with legacy calls and multiple enabled protocols whose tables are set from userspace? > > - /* 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 */ > @@ -309,7 +322,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++; > > @@ -331,8 +345,10 @@ static inline enum rc_type guess_protocol(struct rc_dev *rdev) > return rc_bitmap_to_type(rdev->enabled_protocols); > else if (hweight64(rdev->allowed_protocols) == 1) > return rc_bitmap_to_type(rdev->allowed_protocols); > + else if (rc_map->len > 0) > + return rc_map->scan[0].protocol; > else > - return rc_map->rc_type; > + return RC_TYPE_OTHER; > } > > /** > @@ -375,10 +391,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) { > @@ -387,19 +405,42 @@ static int ir_setkeycode(struct input_dev *idev, > retval = -EINVAL; > goto out; > } > - } else { > + } else if (ke->len == sizeof(int)) { > + /* Old EVIOCSKEYCODE[_V2] 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, scancode, true); > + 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; > + > + 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); > @@ -422,11 +463,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; > > @@ -434,18 +475,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) > @@ -457,6 +499,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 > * > @@ -464,17 +507,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 (m->protocol > protocol) > + end = mid - 1; > + else if (m->scancode < scancode) > start = mid + 1; > - else if (rc_map->scan[mid].scancode > scancode) > + else if (m->scancode > scancode) > end = mid - 1; > else > return mid; > @@ -495,35 +545,61 @@ 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; > > + printk(KERN_ERR "ir_getkeycode: ke->len is %i\n", ke->len); > 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)) { > /* > @@ -548,6 +624,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 > * > @@ -555,7 +632,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; > @@ -564,15 +642,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; > } > @@ -705,8 +784,9 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol, > dev->last_keycode = keycode; > > IR_dprintk(1, "%s: key down event, " > - "key 0x%04x, protocol 0x%04x, scancode 0x%08x\n", > - dev->input_name, keycode, protocol, scancode); > + "key 0x%04x, protocol 0x%04x, scancode 0x%08llx\n", > + dev->input_name, keycode, protocol, > + (long long unsigned)scancode); > input_report_key(dev->input_dev, keycode, 1); > > led_trigger_event(led_feedback, LED_FULL); > @@ -725,10 +805,11 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol, > * This routine is used to signal that a key has been pressed on the > * remote control. > */ > -void rc_keydown(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 toggle) > +void rc_keydown(struct rc_dev *dev, enum rc_type protocol, > + u64 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); > @@ -754,10 +835,10 @@ EXPORT_SYMBOL_GPL(rc_keydown); > * remote control. The driver must manually call rc_keyup() at a later stage. > */ > void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol, > - u32 scancode, u8 toggle) > + u64 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); > @@ -1472,9 +1553,14 @@ int rc_register_device(struct rc_dev *dev) > } > > if (dev->change_protocol) { > - u64 rc_type = (1ll << rc_map->rc_type); > + u64 rc_type = 0; > + > + if (rc_map->len > 0) > + rc_type = (1ll << rc_map->scan[0].protocol); > + > if (dev->driver_type == RC_DRIVER_IR_RAW) > rc_type |= RC_BIT_LIRC; > + > 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 6b4400c..c414ece 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. > @@ -149,7 +167,7 @@ struct rc_dev { > struct timer_list timer_keyup; > u32 last_keycode; > enum rc_type last_protocol; > - u32 last_scancode; > + u64 last_scancode; > u8 last_toggle; > u32 timeout; > u32 min_timeout; > @@ -192,10 +210,10 @@ 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, 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_keydown(struct rc_dev *dev, enum rc_type protocol, u64 scancode, u8 toggle); > +void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol, u64 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 d0bbfc1..4f36869 100644 > --- a/include/media/rc-map.h > +++ b/include/media/rc-map.h > @@ -87,8 +87,9 @@ enum rc_type { > #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 { > @@ -96,7 +97,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; > }; > > -- > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/2] NEC scancodes and protocols in keymaps
@ 2015-04-02 12:02 David Härdeman
2015-04-02 12:03 ` [PATCH 2/2] rc-core: don't throw away protocol information David Härdeman
0 siblings, 1 reply; 8+ messages in thread
From: David Härdeman @ 2015-04-02 12:02 UTC (permalink / raw)
To: linux-media; +Cc: sean, mchehab
The following two patches should show more clearly what I mean by
adding protocols to the keytables (and letting userspace add
keytable entries with explicit protocol information). Consider
it a basis for discussion.
Each patch has a separate description, please refer to those for
more information.
---
David Härdeman (2):
rc-core: use the full 32 bits for NEC scancodes
rc-core: don't throw away protocol information
drivers/media/rc/ati_remote.c | 1
drivers/media/rc/imon.c | 7 +
drivers/media/rc/ir-nec-decoder.c | 26 ---
drivers/media/rc/rc-main.c | 233 ++++++++++++++++++++++++------
drivers/media/usb/dvb-usb-v2/af9015.c | 22 +--
drivers/media/usb/dvb-usb-v2/af9035.c | 23 +--
drivers/media/usb/dvb-usb-v2/az6007.c | 16 +-
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 20 +--
drivers/media/usb/dvb-usb/dib0700_core.c | 24 +--
drivers/media/usb/em28xx/em28xx-input.c | 37 +----
include/media/rc-core.h | 26 +++
include/media/rc-map.h | 23 ++-
12 files changed, 264 insertions(+), 194 deletions(-)
--
David Härdeman
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 2/2] rc-core: don't throw away protocol information 2015-04-02 12:02 [PATCH 0/2] NEC scancodes and protocols in keymaps David Härdeman @ 2015-04-02 12:03 ` David Härdeman 0 siblings, 0 replies; 8+ messages in thread From: David Härdeman @ 2015-04-02 12:03 UTC (permalink / raw) To: linux-media; +Cc: sean, mchehab It is currently impossible to distinguish between scancodes which have been generated using different protocols (and scancodes can, and will, overlap). For example: RC5 message to address 0x00, command 0x03 has scancode 0x00000503 JVC message to address 0x00, command 0x03 has scancode 0x00000503 It is only possible to distinguish (and parse) scancodes by known the scancode *and* the protocol. 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). This also means that struct rc_map no longer hardcodes the protocol, meaning that keytables with mixed entries are possible. Heuristics are also added to hopefully do the right thing with older ioctls in order to preserve backwards compatibility. Signed-off-by: David Härdeman <david@hardeman.nu> --- drivers/media/rc/ati_remote.c | 1 drivers/media/rc/imon.c | 7 + drivers/media/rc/rc-main.c | 195 +++++++++++++++++++++++++++++------------ include/media/rc-core.h | 26 +++++ include/media/rc-map.h | 7 + 5 files changed, 171 insertions(+), 65 deletions(-) diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c index a356318..a1df608 100644 --- a/drivers/media/rc/ati_remote.c +++ b/drivers/media/rc/ati_remote.c @@ -544,6 +544,7 @@ static void ati_remote_input_report(struct urb *urb) * set, assume this is a scrollwheel up/down event. */ wheel_keycode = rc_g_keycode_from_table(ati_remote->rdev, + RC_TYPE_OTHER, scancode & 0x78); if (wheel_keycode == KEY_RESERVED) { diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c index 65f80b8..ec4414a 100644 --- a/drivers/media/rc/imon.c +++ b/drivers/media/rc/imon.c @@ -1263,14 +1263,15 @@ static u32 imon_remote_key_lookup(struct imon_context *ictx, u32 scancode) bool is_release_code = false; /* Look for the initial press of a button */ - keycode = rc_g_keycode_from_table(ictx->rdev, scancode); + keycode = rc_g_keycode_from_table(ictx->rdev, ictx->rc_type, scancode); ictx->rc_toggle = 0x0; ictx->rc_scancode = scancode; /* Look for the release of a button */ if (keycode == KEY_RESERVED) { release = scancode & ~0x4000; - keycode = rc_g_keycode_from_table(ictx->rdev, release); + keycode = rc_g_keycode_from_table(ictx->rdev, ictx->rc_type, + release); if (keycode != KEY_RESERVED) is_release_code = true; } @@ -1299,7 +1300,7 @@ static u32 imon_mce_key_lookup(struct imon_context *ictx, u32 scancode) scancode = scancode | MCE_KEY_MASK | MCE_TOGGLE_BIT; ictx->rc_scancode = scancode; - keycode = rc_g_keycode_from_table(ictx->rdev, scancode); + keycode = rc_g_keycode_from_table(ictx->rdev, ictx->rc_type, scancode); /* not used in mce mode, but make sure we know its false */ ictx->release_code = false; diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index 40ce504..01c9d33 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -103,7 +103,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 = { @@ -119,7 +119,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 * @@ -127,10 +126,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); @@ -225,16 +223,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); } @@ -261,9 +263,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. * @@ -273,7 +275,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; @@ -287,16 +289,27 @@ static unsigned int ir_establish_scancode(struct rc_dev *dev, * indicate the valid bits of the scancodes. */ if (dev->scancode_mask) - scancode &= dev->scancode_mask; + entry->scancode &= dev->scancode_mask; - /* 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 */ @@ -309,7 +322,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++; @@ -331,8 +345,10 @@ static inline enum rc_type guess_protocol(struct rc_dev *rdev) return rc_bitmap_to_type(rdev->enabled_protocols); else if (hweight64(rdev->allowed_protos) == 1) return rc_bitmap_to_type(rdev->allowed_protos); + else if (rc_map->len > 0) + return rc_map->scan[0].protocol; else - return rc_map->rc_type; + return RC_TYPE_OTHER; } /** @@ -375,10 +391,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) { @@ -387,19 +405,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, scancode, true); + 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; + + 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); @@ -422,11 +463,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; @@ -434,18 +475,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) @@ -457,6 +499,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 * @@ -464,17 +507,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 (m->protocol > protocol) + end = mid - 1; + else if (m->scancode < scancode) start = mid + 1; - else if (rc_map->scan[mid].scancode > scancode) + else if (m->scancode > scancode) end = mid - 1; else return mid; @@ -495,35 +545,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)) { /* @@ -548,6 +623,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 * @@ -555,7 +631,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; @@ -564,15 +641,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; } @@ -705,8 +783,9 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol, dev->last_keycode = keycode; IR_dprintk(1, "%s: key down event, " - "key 0x%04x, protocol 0x%04x, scancode 0x%08x\n", - dev->input_name, keycode, protocol, scancode); + "key 0x%04x, protocol 0x%04x, scancode 0x%08llx\n", + dev->input_name, keycode, protocol, + (long long unsigned)scancode); input_report_key(dev->input_dev, keycode, 1); led_trigger_event(led_feedback, LED_FULL); @@ -725,10 +804,11 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol, * This routine is used to signal that a key has been pressed on the * remote control. */ -void rc_keydown(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 toggle) +void rc_keydown(struct rc_dev *dev, enum rc_type protocol, + u64 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); @@ -754,10 +834,10 @@ EXPORT_SYMBOL_GPL(rc_keydown); * remote control. The driver must manually call rc_keyup() at a later stage. */ void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol, - u32 scancode, u8 toggle) + u64 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); @@ -1472,9 +1552,14 @@ int rc_register_device(struct rc_dev *dev) } if (dev->change_protocol) { - u64 rc_type = (1ll << rc_map->rc_type); + u64 rc_type = 0; + + if (rc_map->len > 0) + rc_type = (1ll << rc_map->scan[0].protocol); + if (dev->driver_type == RC_DRIVER_IR_RAW) rc_type |= RC_BIT_LIRC; + 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 6b4400c..c414ece 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. @@ -149,7 +167,7 @@ struct rc_dev { struct timer_list timer_keyup; u32 last_keycode; enum rc_type last_protocol; - u32 last_scancode; + u64 last_scancode; u8 last_toggle; u32 timeout; u32 min_timeout; @@ -192,10 +210,10 @@ 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, 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_keydown(struct rc_dev *dev, enum rc_type protocol, u64 scancode, u8 toggle); +void rc_keydown_notimeout(struct rc_dev *dev, enum rc_type protocol, u64 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 33997d7..71cff75 100644 --- a/include/media/rc-map.h +++ b/include/media/rc-map.h @@ -87,8 +87,9 @@ enum rc_type { #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 { @@ -96,7 +97,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] 8+ messages in thread
end of thread, other threads:[~2015-06-13 23:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-06 11:23 [PATCH 0/2] NEC scancodes and protocols in keymaps - v2 David Härdeman 2015-04-06 11:23 ` [PATCH 1/2] rc-core: use the full 32 bits for NEC scancodes David Härdeman 2015-05-14 20:57 ` Mauro Carvalho Chehab 2015-05-20 20:24 ` David Härdeman 2015-06-13 23:49 ` David Härdeman 2015-04-06 11:23 ` [PATCH 2/2] rc-core: don't throw away protocol information David Härdeman 2015-05-14 21:05 ` Mauro Carvalho Chehab -- strict thread matches above, loose matches on Subject: below -- 2015-04-02 12:02 [PATCH 0/2] NEC scancodes and protocols in keymaps David Härdeman 2015-04-02 12:03 ` [PATCH 2/2] rc-core: don't throw away protocol information David Härdeman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.