Am 15.12.2012 02:54, schrieb Mauro Carvalho Chehab: > Em Sat, 15 Dec 2012 02:56:25 +0200 > Antti Palosaari <crope@xxxxxx> escreveu: > >> On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote: >>> Em Fri, 14 Dec 2012 17:39:50 -0200 >>> Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> escreveu: >>> >>>>> Anyway, first we have to GET the bytes from the hardware. That's our >>>>> current problem ! >>>>> And the hardware seems to need a different setup for reg 0x50 for the >>>>> different NEC sub protocols. >>>>> Which means that the we need to know the sub protocol BEFORE we get any >>>>> bytes from the device. >>>> No. All em28xx needs is to make sure that the NEC protocol will return >>>> the full 32 bits scancode. >>> It seems a way easier/quicker to just add the proper support there at the >>> driver than keep answering to this thread ;) >>> >>> Tested here with a Terratec HTC stick, and using two different IR's: >>> - a Terratec IR (address code 0x14 - standard NEC); >>> - a Pixelview IR (address code 0x866b - 24 bits NEC). >>> >>> All tests were done with the very latest version of ir-keytable, found at >>> v4l-utils.git tree (http://git.linuxtv.org/v4l-utils.git). >>> >>> See the results, with the Terratec table loaded (default when the >>> driver is loaded): >>> >>> # ir-keytable -t >>> Testing events. Please, press CTRL-C to abort. >>> # Terratec IR >>> 1355529698.198046: event type EV_MSC(0x04): scancode = 0x1402 >>> 1355529698.198046: event type EV_KEY(0x01) key_down: KEY_1(0x0001) >>> 1355529698.198046: event type EV_SYN(0x00). >>> 11355529698.298170: event type EV_MSC(0x04): scancode = 0x1402 >>> 1355529698.298170: event type EV_SYN(0x00). >>> 1355529698.547998: event type EV_KEY(0x01) key_up: KEY_1(0x0001) >>> 1355529698.547998: event type EV_SYN(0x00). >>> # Pixelview IR >>> 1355530261.416415: event type EV_MSC(0x04): scancode = 0x866b01 >>> 1355530261.416415: event type EV_SYN(0x00). >>> 1355530262.216301: event type EV_MSC(0x04): scancode = 0x866b0b >>> 1355530262.216301: event type EV_SYN(0x00). >>> >>> Replacing the keytable to the Pixelview's one: >>> >>> # ir-keytable -w /etc/rc_keymaps/pixelview_002t -c >>> Read pixelview_002t table >>> Old keytable cleared >>> Wrote 26 keycode(s) to driver >>> Protocols changed to NEC >>> >>> # ir-keytable -t >>> Testing events. Please, press CTRL-C to abort. >>> 1355530569.420398: event type EV_MSC(0x04): scancode = 0x1402 >>> 1355530569.420398: event type EV_SYN(0x00). >>> 1355530588.120409: event type EV_MSC(0x04): scancode = 0x866b01 >>> 1355530588.120409: event type EV_KEY(0x01) key_down: KEY_1(0x0001) >>> 1355530591.670077: event type EV_SYN(0x00). >>> >>> And, finally, keeping both keytables active at the same time: >>> >>> # ir-keytable -c -w /etc/rc_keymaps/pixelview_002t -w /etc/rc_keymaps/nec_terratec_cinergy_xs >>> Read pixelview_002t table >>> Read nec_terratec_cinergy_xs table >>> Old keytable cleared >>> Wrote 74 keycode(s) to driver >>> Protocols changed to NEC >>> >>> # sudo ir-keytable -t >>> Testing events. Please, press CTRL-C to abort. >>> 1355530856.325201: event type EV_MSC(0x04): scancode = 0x866b01 >>> 1355530856.325201: event type EV_KEY(0x01) key_down: KEY_1(0x0001) >>> 1355530856.325201: event type EV_SYN(0x00). >>> 11355530856.575070: event type EV_KEY(0x01) key_up: KEY_1(0x0001) >>> 1355530856.575070: event type EV_SYN(0x00). >>> 1355530869.125226: event type EV_MSC(0x04): scancode = 0x1402 >>> 1355530869.125226: event type EV_KEY(0x01) key_down: KEY_1(0x0001) >>> 1355530869.125226: event type EV_SYN(0x00). >>> 11355530869.225216: event type EV_MSC(0x04): scancode = 0x1402 >>> 1355530869.225216: event type EV_SYN(0x00). >>> 1355530869.475075: event type EV_KEY(0x01) key_up: KEY_1(0x0001) >>> 1355530869.475075: event type EV_SYN(0x00). >>> >>> - >>> >>> em28xx: add support for 24bits/32 bits NEC variants on em2874 and upper >>> >>> By disabling the NEC parity check, it is possible to handle all 3 NEC >>> protocol variants (32, 24 or 16 bits). >>> >>> Change the driver in order to handle all of them. >>> >>> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> >> >> >> >> >> NACK. NEC variant selection logic is broken by design. >> >> >> >> >> >>> diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c >>> index 97d36b4..c84e4c8 100644 >>> --- a/drivers/media/usb/em28xx/em28xx-input.c >>> +++ b/drivers/media/usb/em28xx/em28xx-input.c >>> @@ -57,8 +57,8 @@ MODULE_PARM_DESC(ir_debug, "enable debug messages [IR]"); >>> struct em28xx_ir_poll_result { >>> unsigned int toggle_bit:1; >>> unsigned int read_count:7; >>> - u8 rc_address; >>> - u8 rc_data[4]; /* 1 byte on em2860/2880, 4 on em2874 */ >>> + >>> + u32 scancode; >>> }; >>> >>> struct em28xx_IR { >>> @@ -72,6 +72,7 @@ struct em28xx_IR { >>> struct delayed_work work; >>> unsigned int full_code:1; >>> unsigned int last_readcount; >>> + u64 rc_type; >>> >>> int (*get_key)(struct em28xx_IR *, struct em28xx_ir_poll_result *); >>> }; >>> @@ -236,11 +237,8 @@ static int default_polling_getkey(struct em28xx_IR *ir, >>> /* Infrared read count (Reg 0x45[6:0] */ >>> poll_result->read_count = (msg[0] & 0x7f); >>> >>> - /* Remote Control Address (Reg 0x46) */ >>> - poll_result->rc_address = msg[1]; >>> - >>> - /* Remote Control Data (Reg 0x47) */ >>> - poll_result->rc_data[0] = msg[2]; >>> + /* Remote Control Address/Data (Regs 0x46/0x47) */ >>> + poll_result->scancode = msg[1] << 8 | msg[2]; >>> >>> return 0; >>> } >>> @@ -266,13 +264,30 @@ static int em2874_polling_getkey(struct em28xx_IR *ir, >>> /* Infrared read count (Reg 0x51[6:0] */ >>> poll_result->read_count = (msg[0] & 0x7f); >>> >>> - /* Remote Control Address (Reg 0x52) */ >>> - poll_result->rc_address = msg[1]; >>> - >>> - /* Remote Control Data (Reg 0x53-55) */ >>> - poll_result->rc_data[0] = msg[2]; >>> - poll_result->rc_data[1] = msg[3]; >>> - poll_result->rc_data[2] = msg[4]; >>> + /* Remote Control Address (Reg 0x52) */ >>> + /* Remote Control Data (Reg 0x53-0x55) */ >>> + switch (ir->rc_type) { >>> + case RC_TYPE_RC5: >>> + poll_result->scancode = msg[1] << 8 | msg[2]; >>> + break; >>> + case RC_TYPE_NEC: >>> + if ((msg[3] ^ msg[4]) != 0xff) /* 32 bits NEC */ >> See for example KEY_CYCLEWINDOWS from RC_MAP_TIVO. Do you think it >> works..... :-( > { 0xa10cfa05, KEY_CYCLEWINDOWS }, /* Window */ > > You're right: for it to work, this key would be needed to be defined as: > > { 0xa10c05, KEY_CYCLEWINDOWS }, /* Window */ > > I agree, that's weird, but a vendor that uses a key definition like > that doesn't know what he's doing, as a remote control with address = 0xa10c > will very likely produce the same code. > > Btw, the way it is currently declared won't work either with mceusb, as > the IR decoder also does the same thing. > > (c/c Jarod, as he added the Tivo IR). > > ... > >> OK, it is much better and I can even see that in Kernel than keeping >> old, very limited implementation. >> >> My aim was just to probe whole variant selection method is quite broken, >> and it is impossible to get working with 100% reliable. As I have said >> loudly :) , I want 32bit scancodes for all NEC remotes, no variants at >> all. I think you are about the only person who wants to keep current >> multiple NEC variant implementation... > I'm not bound to it, and no, I'm not the only one that voted for this > implementation. This were discussed in the past, when support for "extended" > nec got added (24 bits). When the first 32 bits NEC-yet-another-weird-variant > arrived, the choice was natural. > > The thing is: userspace can't be broken by whatever change we do. The way > it got implemented were the one that wouldn't generate regressions. > It is as simple as that. > > Cheers, > Mauro Sorry.... we completely lost the focus ! Do you remeber the thread title ? ;) We have two separate issues here. 1) Making Matthews hardware / the scancode retrieval code work And _if_ it turns out that we can't make it work without knowing the sub protocol type in advance 2) how to handle this (which doesn't necessarily mean that we have to solve it in the RC core) So lets focus on 1) first: After reading the code again, it boils down to the following code lines in em28xx_ir_handle_key(): if (unlikely(poll_result.read_count != ir->last_readcount)) { dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", __func__, ... rc_keydown(...) With reg 0x50 set to EM2874_IR_NEC=0x00, Matthew doesn't get any debugging messages when he presses the RC buttons. With reg 0x50 set to 0x01, there are only few messages, with the same single scancode: Am 10.12.2012 17:00, schrieb Matthew Gyurgyik: >>> Here is the dmesg output: >>> >>>> [root@tux ~]# dmesg -t | sort | uniq | grep 'em28xx IR' | grep handle >>>> em28xx IR (em28xx #0)/ir: 6em28xx_ir_handle_key: toggle: 0, count: 1, >>>> key 0x61d6 >>>> em28xx IR (em28xx #0)/ir: 6em28xx_ir_handle_key: toggle: 0, count: 2, >>>> key 0x61d6 >>>> em28xx IR (em28xx #0)/ir: 6em28xx_ir_handle_key: toggle: 1, count: 1, >>>> key 0x61d6 >>>> em28xx IR (em28xx #0)/ir: em28xx_ir_handle_key: toggle: 0, count: 1, >>>> key 0x61d6 >>>> em28xx IR (em28xx #0)/ir: em28xx_ir_handle_key: toggle: 0, count: 2, >>>> key 0x61d6 >>>> em28xx IR (em28xx #0)/ir: em28xx_ir_handle_key: toggle: 1, count: 1, >>>> key 0x61d6 >>>> em28xx IR (em28xx #0)/ir: em28xx_ir_handle_key: toggle: 1, count: 2, >>>> key 0x61d6 >>> >>> I pressed all the buttons on the remote (40 buttons). >> >> Did you cut the dmesg output ? Or do you really get these messages for >> key 0x61d6 only ? > > Correct, I only got the messages for key 0x61d6 regardless of which > physical button I press. So if Matthew didn't make any mistakes, the problem seems to be the read count handling... ---------------------------------- Concerning the rc core / keymap stuff: it seems like there are some weak spots. The discussion is focussed on the scan codes and it seems to have a long history. I just want to make clear that I don't have an opinion about this yet (nor do I want to change someone elses opinion !). I actually don't care about it at the moment. If we're going to discuss this further, I suggest to do that in a separate thread with a more meaningful title. Regards, Frank -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
