Message ID | 20121214222631.1f191d6e@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Em Fri, 14 Dec 2012 22:26:31 -0200 Mauro Carvalho Chehab <mchehab@redhat.com> escreveu: > Em Fri, 14 Dec 2012 17:39:50 -0200 > Mauro Carvalho Chehab <mchehab@redhat.com> 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). Just in case, I tested also that RC5 keeps working, by using a Hauppauge grey control: # ir-keytable -c -w /etc/rc_keymaps/hauppauge Read hauppauge table Old keytable cleared Wrote 136 keycode(s) to driver Protocols changed to RC-5 # sudo ir-keytable -t Testing events. Please, press CTRL-C to abort. 1355531445.443208: event type EV_MSC(0x04): scancode = 0x1e02 1355531445.443208: event type EV_KEY(0x01) key_down: KEY_2(0x0001) 1355531445.443208: event type EV_SYN(0x00). 21355531445.543207: event type EV_MSC(0x04): scancode = 0x1e02 1355531445.543207: event type EV_SYN(0x00). 1355531445.793072: event type EV_KEY(0x01) key_up: KEY_2(0x0001) 1355531445.793072: event type EV_SYN(0x00). 1355531446.643224: event type EV_MSC(0x04): scancode = 0x1e02 1355531446.643224: event type EV_KEY(0x01) key_down: KEY_2(0x0001) 1355531446.643224: event type EV_SYN(0x00). 21355531446.743205: event type EV_MSC(0x04): scancode = 0x1e02 1355531446.743205: event type EV_SYN(0x00). Cheers, Mauro -- 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
On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote: > Em Fri, 14 Dec 2012 17:39:50 -0200 > Mauro Carvalho Chehab <mchehab@redhat.com> 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@redhat.com> 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..... :-( > + poll_result->scancode = (msg[1] << 24) | > + (msg[2] << 16) | > + (msg[3] << 8) | > + msg[4]; > + else if ((msg[1] ^ msg[2]) != 0xff) /* 24 bits NEC */ > + poll_result->scancode = (msg[1] << 16) | > + (msg[3] << 8) | > + msg[4]; > + else /* Normal NEC */ > + poll_result->scancode = msg[1] << 8 | msg[3]; > + break; > + default: > + poll_result->scancode = (msg[1] << 24) | (msg[2] << 16) | > + (msg[3] << 8) | msg[4]; > + break; > + } > > return 0; > } > @@ -294,17 +309,16 @@ static void em28xx_ir_handle_key(struct em28xx_IR *ir) > } > > if (unlikely(poll_result.read_count != ir->last_readcount)) { > - dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", __func__, > + dprintk("%s: toggle: %d, count: %d, key 0x%04x\n", __func__, > poll_result.toggle_bit, poll_result.read_count, > - poll_result.rc_address, poll_result.rc_data[0]); > + poll_result.scancode); > if (ir->full_code) > rc_keydown(ir->rc, > - poll_result.rc_address << 8 | > - poll_result.rc_data[0], > + poll_result.scancode, > poll_result.toggle_bit); > else > rc_keydown(ir->rc, > - poll_result.rc_data[0], > + poll_result.scancode & 0xff, > poll_result.toggle_bit); > > if (ir->dev->chip_id == CHIP_ID_EM2874 || > @@ -359,11 +373,13 @@ static int em28xx_ir_change_protocol(struct rc_dev *rc_dev, u64 rc_type) > ir->full_code = 1; > } else if (rc_type == RC_TYPE_NEC) { > dev->board.xclk &= ~EM28XX_XCLK_IR_RC5_MODE; > - ir_config = EM2874_IR_NEC; > + ir_config = EM2874_IR_NEC | EM2874_IR_NEC_NO_PARITY; > ir->full_code = 1; > } else if (rc_type != RC_TYPE_UNKNOWN) > rc = -EINVAL; > > + ir->rc_type = rc_type; > + > em28xx_write_reg_bits(dev, EM28XX_R0F_XCLK, dev->board.xclk, > EM28XX_XCLK_IR_RC5_MODE); > > diff --git a/drivers/media/usb/em28xx/em28xx-reg.h b/drivers/media/usb/em28xx/em28xx-reg.h > index 6ff3682..2ad3573 100644 > --- a/drivers/media/usb/em28xx/em28xx-reg.h > +++ b/drivers/media/usb/em28xx/em28xx-reg.h > @@ -177,6 +177,7 @@ > > /* em2874 IR config register (0x50) */ > #define EM2874_IR_NEC 0x00 > +#define EM2874_IR_NEC_NO_PARITY 0x01 > #define EM2874_IR_RC5 0x04 > #define EM2874_IR_RC6_MODE_0 0x08 > #define EM2874_IR_RC6_MODE_6A 0x0b > 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... regards Antti
Em Sat, 15 Dec 2012 02:56:25 +0200
Antti Palosaari <crope@iki.fi> escreveu:
> NACK. NEC variant selection logic is broken by design.
If you think so, then feel free to fix it without causing regressions to
the existing userspace.
While you don't do it, I don't see anything wrong on this patch, as it
will behave just like any other NEC decoder.
Cheers,
Mauro
--
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
On 12/15/2012 03:03 AM, Mauro Carvalho Chehab wrote: > Em Sat, 15 Dec 2012 02:56:25 +0200 > Antti Palosaari <crope@iki.fi> escreveu: > >> NACK. NEC variant selection logic is broken by design. > > If you think so, then feel free to fix it without causing regressions to > the existing userspace. > > While you don't do it, I don't see anything wrong on this patch, as it > will behave just like any other NEC decoder. yes, so true as I mentioned end of the mail. But it is very high probability there is some non/wrong working keys when 32bit NEC variant remote is used with that implementation. And what happened those patches David sends sometime ago. I remember there was a patch for the af9015 which removes that kind of logic from the driver. If not change NEC to 32bit at least heuristic could be moved to single point - rc-core. regards Antti
Em Sat, 15 Dec 2012 03:12:58 +0200 Antti Palosaari <crope@iki.fi> escreveu: > On 12/15/2012 03:03 AM, Mauro Carvalho Chehab wrote: > > Em Sat, 15 Dec 2012 02:56:25 +0200 > > Antti Palosaari <crope@iki.fi> escreveu: > > > >> NACK. NEC variant selection logic is broken by design. > > > > If you think so, then feel free to fix it without causing regressions to > > the existing userspace. > > > > While you don't do it, I don't see anything wrong on this patch, as it > > will behave just like any other NEC decoder. > > yes, so true as I mentioned end of the mail. Oh, I didn't saw your comments. Sorry. Please, next time, drop the part of the code that you're not commenting. On long emails like that, it is sometimes hard to see what's out there. I'll reply to your comments there again. > But it is very high > probability there is some non/wrong working keys when 32bit NEC variant > remote is used with that implementation. > > And what happened those patches David sends sometime ago. I remember > there was a patch for the af9015 which removes that kind of logic from > the driver. If not change NEC to 32bit at least heuristic could be moved > to single point - rc-core. There were some problems on his series, and it was breaking the userspace API. David made a new series, with a smaller set of patches that got applied, without those changes. The big issue there is to not break the current NEC userspace tables. Unfortunately, when the NEC software decoder was written, it were taking care only of the real NEC standard (the 24-bits and 32-bits variants aren't part of any spec I'm aware of). When we latter added support for those weird variants (RC-5 variants; Sony variants; NEC variants), it was decided, after long debates at the mailing list, to not create a new protocol for each, but, instead, to add support for them into the existing code. This is OK on most cases, as the variants are real variants, and the decoder can properly distinguish them. Unfortunately, NEC protocol variants don't fill on such case, as the only difference is that they doesn't honour to the checksum bytes. So, again after long discussions, it was decided to implement it the way it is. Changing it right now is not trivial, as it is easy to break existing userspace. Regards, Mauro -- 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
Em Sat, 15 Dec 2012 02:56:25 +0200 Antti Palosaari <crope@iki.fi> 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@redhat.com> 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@redhat.com> > > > > > > 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 -- 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
Am 15.12.2012 02:54, schrieb Mauro Carvalho Chehab: > Em Sat, 15 Dec 2012 02:56:25 +0200 > Antti Palosaari <crope@iki.fi> 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@redhat.com> 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@redhat.com> >> >> >> >> >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Sat, 15 Dec 2012 14:11:24 +0100 Frank Schäfer <fschaefer.oss@googlemail.com> escreveu: > Sorry.... we completely lost the focus ! > Do you remeber the thread title ? ;) > > We have two separate issues here. > 1) Making Matthews hardware Didn't read the entire thread, but it seems that this were already solved. > / 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 The patches for IR to work with NEC-24/32 bits and RC6 mode 0 were posted. All it lacks after applying the patches is likely to: 1) put any dummy IR table on the board card logic, to enable IR code; 2) run ir-keycode -t on his hardware to get the remote controller scancode; 3) find the table that matches his IR at drivers/media/rc/keymaps/ or add a new one with the discovered scancodes; 4) put the right IR table ad em28xx-cards.c; 5) send the final patches upstream. > 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 The above is obviously not using the patches I sent yesterday/today, otherwise, it would be seeing, instead, the full 32 bits code there. Also, the better is to use ir-keytable program on test mode ("ir-keytable -t"), as it allows to see if the 3 protocol messages received by each keystroke are being received (EV_MSC, EV_KEY and EV_SYN). > >>> > >>> 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. Agreed. This is a separate question, and should be handled on a different thread. Regards, Mauro -- 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
On 12/15/2012 03:11 PM, Frank Schäfer wrote: > Am 15.12.2012 02:54, schrieb Mauro Carvalho Chehab: >> Em Sat, 15 Dec 2012 02:56:25 +0200 >> Antti Palosaari <crope@iki.fi> 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@redhat.com> 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@redhat.com> >>> >>> >>> >>> >>> 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... That is what happened and it is correct behavior as Matthews remote is 24 bit NEC (and driver does not know how to handle it). If you look again those raw dumps which I send previous mail you could see the issue. 61d6 seen here is remote controller address, two first bits. It outputs that because debug does not output rest 2 remaining bytes where is actual key code. If you set em28xx to 32bit NEC mode then key code for that remote set 61d6 by driver mistakenly as it does not know there is 2 bytes more to handle. I copy & pasted here relevant lines from the em28xx driver. Maybe you could now see why code is wrong - why the extended address byte is set as to scancode mistakenly. Look especially care following variables: msg[1], msg[2], poll_result->rc_address and poll_result.rc_data[0] static int em2874_polling_getkey() { rc = dev->em28xx_read_reg_req_len(dev, 0, EM2874_R51_IR, msg, sizeof(msg)); /* 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]; } static void em28xx_ir_handle_key() { dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", __func__, poll_result.toggle_bit, poll_result.read_count, poll_result.rc_address, poll_result.rc_data[0]); } I am about 99% sure Mauro's patch works correctly for Matthews device. Maybe you missed point hardware returns different format in case of 32bit and 16bit values. If it is 16bit mode it return only 2 bytes and if 32bit mode it returns 4 bytes? > > > ---------------------------------- > > > 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 regards Antti
Am 15.12.2012 14:38, schrieb Antti Palosaari: > On 12/15/2012 03:11 PM, Frank Schäfer wrote: >> Am 15.12.2012 02:54, schrieb Mauro Carvalho Chehab: >>> Em Sat, 15 Dec 2012 02:56:25 +0200 >>> Antti Palosaari <crope@iki.fi> 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@redhat.com> 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@redhat.com> >>>> >>>> >>>> >>>> >>>> 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... > > That is what happened and it is correct behavior as Matthews remote is > 24 bit NEC (and driver does not know how to handle it). If you look > again those raw dumps which I send previous mail you could see the > issue. 61d6 seen here is remote controller address, two first bits. It > outputs that because debug does not output rest 2 remaining bytes > where is actual key code. If you set em28xx to 32bit NEC mode then key > code for that remote set 61d6 by driver mistakenly as it does not know > there is 2 bytes more to handle. Antti, the problem with Matthews RC isn't the number of bytes printed to the log. The problems is, that NO messages appear in the log (with one exception). > > I copy & pasted here relevant lines from the em28xx driver. Maybe you > could now see why code is wrong - why the extended address byte is set > as to scancode mistakenly. Look especially care following variables: > msg[1], msg[2], poll_result->rc_address and poll_result.rc_data[0] > > static int em2874_polling_getkey() > { > rc = dev->em28xx_read_reg_req_len(dev, 0, EM2874_R51_IR, msg, > sizeof(msg)); > > /* 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]; > } > You missed the relevant part of the code: static int em2874_polling_getkey() { ... /* Infrared read count (Reg 0x51[6:0] */ poll_result->read_count = (msg[0] & 0x7f); ... } > > > static void em28xx_ir_handle_key() > { > dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", __func__, > poll_result.toggle_bit, poll_result.read_count, > poll_result.rc_address, poll_result.rc_data[0]); > } > Same here, you missed the if () statement: static void em28xx_ir_handle_key(struct em28xx_IR *ir) { ... if (unlikely(poll_result.read_count != ir->last_readcount)) { dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", __func__, poll_result.toggle_bit, poll_result.read_count, poll_result.rc_address, poll_result.rc_data[0]); ... } It doesn't matter which format the scancode (4 bytes) has, a message should be printed in any case. But according to Matthew, that doesn't happen. Hence, the poll_result.read_count != ir->last_readcount must be the problem. > > I am about 99% sure Mauro's patch works correctly for Matthews device. > If Matthew didn't make any mistakes, I can't see how these patches can make it work. Which doesn't mean that they don't make sense. Matthew, could you please validate your test results and try Mauros patches ? If it doesn't work, please create another USB-log. > Maybe you missed point hardware returns different format in case of > 32bit and 16bit values. If it is 16bit mode it return only 2 bytes and > if 32bit mode it returns 4 bytes? > No. Regards, Frank -- 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
On 12/15/2012 06:21 PM, Frank Schäfer wrote: > Am 15.12.2012 14:38, schrieb Antti Palosaari: >> On 12/15/2012 03:11 PM, Frank Schäfer wrote: >>> Am 15.12.2012 02:54, schrieb Mauro Carvalho Chehab: >>>> Em Sat, 15 Dec 2012 02:56:25 +0200 >>>> Antti Palosaari <crope@iki.fi> 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@redhat.com> escreveu: >>> 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... >> >> That is what happened and it is correct behavior as Matthews remote is >> 24 bit NEC (and driver does not know how to handle it). If you look >> again those raw dumps which I send previous mail you could see the >> issue. 61d6 seen here is remote controller address, two first bits. It >> outputs that because debug does not output rest 2 remaining bytes >> where is actual key code. If you set em28xx to 32bit NEC mode then key >> code for that remote set 61d6 by driver mistakenly as it does not know >> there is 2 bytes more to handle. > > Antti, the problem with Matthews RC isn't the number of bytes printed to > the log. > The problems is, that NO messages appear in the log (with one exception). What is that exception? In that case there should be around 40 similar debug lines - as address byte is same for all buttons and debug prints only address byte, toggle and count. As toggle and count still outputs some numbers we will see that many line. Without toggle and count there is likely only one debug line seen as output is piped through uniq which drops similar lines. >> I copy & pasted here relevant lines from the em28xx driver. Maybe you >> could now see why code is wrong - why the extended address byte is set >> as to scancode mistakenly. Look especially care following variables: >> msg[1], msg[2], poll_result->rc_address and poll_result.rc_data[0] >> >> static int em2874_polling_getkey() >> { >> rc = dev->em28xx_read_reg_req_len(dev, 0, EM2874_R51_IR, msg, >> sizeof(msg)); >> >> /* 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]; >> } >> > > You missed the relevant part of the code: > > static int em2874_polling_getkey() > { > ... > /* Infrared read count (Reg 0x51[6:0] */ > poll_result->read_count = (msg[0] & 0x7f); > ... > } > > >> >> >> static void em28xx_ir_handle_key() >> { >> dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", __func__, >> poll_result.toggle_bit, poll_result.read_count, >> poll_result.rc_address, poll_result.rc_data[0]); >> } >> > > Same here, you missed the if () statement: > > static void em28xx_ir_handle_key(struct em28xx_IR *ir) > { > ... > if (unlikely(poll_result.read_count != ir->last_readcount)) { > dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", __func__, > poll_result.toggle_bit, poll_result.read_count, > poll_result.rc_address, poll_result.rc_data[0]); > ... > } > > > It doesn't matter which format the scancode (4 bytes) has, a message > should be printed in any case. > But according to Matthew, that doesn't happen. Hence, the > poll_result.read_count != ir->last_readcount must be the problem. I don't see which messages are missing. I think read_count is not incremented in case NEC 16bit checksum fails - hw just discards whole code => read_count not increase => no debug. For my tests there was always 81/01 when key was pressed and 00 when no key pressed (as seen also logs I posted yesterday). >> I am about 99% sure Mauro's patch works correctly for Matthews device. >> > > If Matthew didn't make any mistakes, I can't see how these patches can > make it work. Which doesn't mean that they don't make sense. > > > Matthew, could you please validate your test results and try Mauros > patches ? > If it doesn't work, please create another USB-log. > > >> Maybe you missed point hardware returns different format in case of >> 32bit and 16bit values. If it is 16bit mode it return only 2 bytes and >> if 32bit mode it returns 4 bytes? >> > > No. > > > Regards, > Frank regards Antti
Am 15.12.2012 17:51, schrieb Antti Palosaari: > On 12/15/2012 06:21 PM, Frank Schäfer wrote: >> Am 15.12.2012 14:38, schrieb Antti Palosaari: >>> On 12/15/2012 03:11 PM, Frank Schäfer wrote: >>>> Am 15.12.2012 02:54, schrieb Mauro Carvalho Chehab: >>>>> Em Sat, 15 Dec 2012 02:56:25 +0200 >>>>> Antti Palosaari <crope@iki.fi> 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@redhat.com> escreveu: > >>>> 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... >>> >>> That is what happened and it is correct behavior as Matthews remote is >>> 24 bit NEC (and driver does not know how to handle it). If you look >>> again those raw dumps which I send previous mail you could see the >>> issue. 61d6 seen here is remote controller address, two first bits. It >>> outputs that because debug does not output rest 2 remaining bytes >>> where is actual key code. If you set em28xx to 32bit NEC mode then key >>> code for that remote set 61d6 by driver mistakenly as it does not know >>> there is 2 bytes more to handle. >> >> Antti, the problem with Matthews RC isn't the number of bytes printed to >> the log. >> The problems is, that NO messages appear in the log (with one >> exception). > > What is that exception? > > In that case there should be around 40 similar debug lines - as > address byte is same for all buttons and debug prints only address > byte, toggle and count. That's what I mean. He said he didn't cut dmesg. > As toggle and count still outputs some numbers we will see that many > line. Without toggle and count there is likely only one debug line > seen as output is piped through uniq which drops similar lines. > >>> I copy & pasted here relevant lines from the em28xx driver. Maybe you >>> could now see why code is wrong - why the extended address byte is set >>> as to scancode mistakenly. Look especially care following variables: >>> msg[1], msg[2], poll_result->rc_address and poll_result.rc_data[0] >>> >>> static int em2874_polling_getkey() >>> { >>> rc = dev->em28xx_read_reg_req_len(dev, 0, EM2874_R51_IR, msg, >>> sizeof(msg)); >>> >>> /* 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]; >>> } >>> >> >> You missed the relevant part of the code: >> >> static int em2874_polling_getkey() >> { >> ... >> /* Infrared read count (Reg 0x51[6:0] */ >> poll_result->read_count = (msg[0] & 0x7f); >> ... >> } >> >> >>> >>> >>> static void em28xx_ir_handle_key() >>> { >>> dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", __func__, >>> poll_result.toggle_bit, poll_result.read_count, >>> poll_result.rc_address, poll_result.rc_data[0]); >>> } >>> >> >> Same here, you missed the if () statement: >> >> static void em28xx_ir_handle_key(struct em28xx_IR *ir) >> { >> ... >> if (unlikely(poll_result.read_count != ir->last_readcount)) { >> dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", >> __func__, >> poll_result.toggle_bit, poll_result.read_count, >> poll_result.rc_address, poll_result.rc_data[0]); >> ... >> } >> >> >> It doesn't matter which format the scancode (4 bytes) has, a message >> should be printed in any case. >> But according to Matthew, that doesn't happen. Hence, the >> poll_result.read_count != ir->last_readcount must be the problem. > > I don't see which messages are missing. > > I think read_count is not incremented in case NEC 16bit checksum fails > - hw just discards whole code => read_count not increase => no debug. > For my tests there was always 81/01 when key was pressed and 00 when > no key pressed (as seen also logs I posted yesterday). I don't know, I don't have the data sheet, I don't have the hardware and I didn't make the test myself. If you are sure you can declare it working, fine. Care to fix the tuner issues ? Regards, Frank > >>> I am about 99% sure Mauro's patch works correctly for Matthews device. >>> >> >> If Matthew didn't make any mistakes, I can't see how these patches can >> make it work. Which doesn't mean that they don't make sense. >> >> >> Matthew, could you please validate your test results and try Mauros >> patches ? >> If it doesn't work, please create another USB-log. >> >> >>> Maybe you missed point hardware returns different format in case of >>> 32bit and 16bit values. If it is 16bit mode it return only 2 bytes and >>> if 32bit mode it returns 4 bytes? >>> >> >> No. >> >> >> Regards, >> Frank > > regards > Antti > -- 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
On 12/15/2012 06:21 PM, Frank Schäfer wrote: >> Matthew, could you please validate your test results and try Mauros >> patches ? If it doesn't work, please create another USB-log. >> Sorry it took me so long to test the patch, but the results look promising, I actually got various keycodes! dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt evtest was also generating output Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), value 61d618e7 Event: time 1355705906.950551, -------------- SYN_REPORT ------------ This is the current patch I'm using: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt What needs to be done to generate a keymap file? Is there anything I can collect or try to do, to get channel scanning working? Just let me know what you need me to do. I really appreciate all the help! Thanks, Matthew -- 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
On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: > On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>> Matthew, could you please validate your test results and try Mauros >>> patches ? If it doesn't work, please create another USB-log. >>> > > Sorry it took me so long to test the patch, but the results look > promising, I actually got various keycodes! > > dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt > > evtest was also generating output > > Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), value > 61d618e7 > Event: time 1355705906.950551, -------------- SYN_REPORT ------------ > > This is the current patch I'm using: > http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt > > What needs to be done to generate a keymap file? > > Is there anything I can collect or try to do, to get channel scanning > working? > > Just let me know what you need me to do. I really appreciate all the help! You don't need to do nothing as that remote is already there. Just ensure buttons are same and we are happy. http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 RC_MAP_MSI_DIGIVOX_III should be added to your device profile in order to load correct keytable by default. You could test it easily, just add following definition .ir_codes = RC_MAP_MSI_DIGIVOX_III, to em28xx-cards.c board config and it is all. regards Antti
On 12/16/2012 08:26 PM, Antti Palosaari wrote: > On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: >> On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>>> Matthew, could you please validate your test results and try Mauros >>>> patches ? If it doesn't work, please create another USB-log. >>>> >> >> Sorry it took me so long to test the patch, but the results look >> promising, I actually got various keycodes! >> >> dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >> >> evtest was also generating output >> >> Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), value >> 61d618e7 >> Event: time 1355705906.950551, -------------- SYN_REPORT ------------ >> >> This is the current patch I'm using: >> http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >> >> What needs to be done to generate a keymap file? >> >> Is there anything I can collect or try to do, to get channel scanning >> working? >> >> Just let me know what you need me to do. I really appreciate all the >> help! > > You don't need to do nothing as that remote is already there. Just > ensure buttons are same and we are happy. > http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 > > > RC_MAP_MSI_DIGIVOX_III should be added to your device profile in order > to load correct keytable by default. You could test it easily, just add > following definition > > .ir_codes = RC_MAP_MSI_DIGIVOX_III, > > to em28xx-cards.c board config and it is all. > > regards > Antti > Maybe I'm missing something but these are different key codes and lengths. tux:~ $ echo 0x61d643bc | wc -c # my dmesg dump 11 tux:~ $ echo 0x61d601 | wc -c # DIGIVOX mini III 9 As I understand it, this was the whole reason for the patches that Mauros wrote. Regards, Matthew -- 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
On 12/17/2012 03:37 AM, Matthew Gyurgyik wrote: > On 12/16/2012 08:26 PM, Antti Palosaari wrote: >> On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: >>> On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>>>> Matthew, could you please validate your test results and try Mauros >>>>> patches ? If it doesn't work, please create another USB-log. >>>>> >>> >>> Sorry it took me so long to test the patch, but the results look >>> promising, I actually got various keycodes! >>> >>> dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>> >>> evtest was also generating output >>> >>> Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), value >>> 61d618e7 >>> Event: time 1355705906.950551, -------------- SYN_REPORT ------------ >>> >>> This is the current patch I'm using: >>> http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>> >>> What needs to be done to generate a keymap file? >>> >>> Is there anything I can collect or try to do, to get channel scanning >>> working? >>> >>> Just let me know what you need me to do. I really appreciate all the >>> help! >> >> You don't need to do nothing as that remote is already there. Just >> ensure buttons are same and we are happy. >> http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 >> >> >> >> RC_MAP_MSI_DIGIVOX_III should be added to your device profile in order >> to load correct keytable by default. You could test it easily, just add >> following definition >> >> .ir_codes = RC_MAP_MSI_DIGIVOX_III, >> >> to em28xx-cards.c board config and it is all. >> >> regards >> Antti >> > Maybe I'm missing something but these are different key codes and lengths. > > tux:~ $ echo 0x61d643bc | wc -c # my dmesg dump > 11 > tux:~ $ echo 0x61d601 | wc -c # DIGIVOX mini III > 9 0x61d643bc == 0x61d643 0x61d601fe == 0x61d601 Those are same codes, other (debug) is just 32bit full format. Last byte in that case is dropped out as it is used for parity check - formula: DD == ~DD > As I understand it, this was the whole reason for the patches that > Mauros wrote. Nope, the reason was it didn't support 32bit at all. regards Antti
On 12/17/2012 11:33 AM, Antti Palosaari wrote: > On 12/17/2012 03:37 AM, Matthew Gyurgyik wrote: >> On 12/16/2012 08:26 PM, Antti Palosaari wrote: >>> On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: >>>> On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>>>>> Matthew, could you please validate your test results and try Mauros >>>>>> patches ? If it doesn't work, please create another USB-log. >>>>>> >>>> >>>> Sorry it took me so long to test the patch, but the results look >>>> promising, I actually got various keycodes! >>>> >>>> dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>> >>>> evtest was also generating output >>>> >>>> Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), >>>> value >>>> 61d618e7 >>>> Event: time 1355705906.950551, -------------- SYN_REPORT ------------ >>>> >>>> This is the current patch I'm using: >>>> http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>> >>>> What needs to be done to generate a keymap file? >>>> >>>> Is there anything I can collect or try to do, to get channel scanning >>>> working? >>>> >>>> Just let me know what you need me to do. I really appreciate all the >>>> help! >>> >>> You don't need to do nothing as that remote is already there. Just >>> ensure buttons are same and we are happy. >>> http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 >>> >>> >>> >>> >>> RC_MAP_MSI_DIGIVOX_III should be added to your device profile in order >>> to load correct keytable by default. You could test it easily, just add >>> following definition >>> >>> .ir_codes = RC_MAP_MSI_DIGIVOX_III, >>> >>> to em28xx-cards.c board config and it is all. >>> >>> regards >>> Antti >>> >> Maybe I'm missing something but these are different key codes and >> lengths. >> >> tux:~ $ echo 0x61d643bc | wc -c # my dmesg dump >> 11 >> tux:~ $ echo 0x61d601 | wc -c # DIGIVOX mini III >> 9 > > 0x61d643bc == 0x61d643 > 0x61d601fe == 0x61d601 > > Those are same codes, other (debug) is just 32bit full format. Last byte > in that case is dropped out as it is used for parity check - formula: DD > == ~DD > >> As I understand it, this was the whole reason for the patches that >> Mauros wrote. > > Nope, the reason was it didn't support 32bit at all. I looked the patch and it seems like it should store and print 24bit scancode for your remote. Maybe you didn't set default remote end it fall back to unknown remote protocol which stores all bytes. Or some other bug. Test it with default keytable (RC_MAP_MSI_DIGIVOX_III) and if it does not output numbers there must be a bug. I am too lazy to test it currently. regards Antti
On 12/17/2012 06:08 AM, Antti Palosaari wrote: > On 12/17/2012 11:33 AM, Antti Palosaari wrote: >> On 12/17/2012 03:37 AM, Matthew Gyurgyik wrote: >>> On 12/16/2012 08:26 PM, Antti Palosaari wrote: >>>> On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: >>>>> On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>>>>>> Matthew, could you please validate your test results and try Mauros >>>>>>> patches ? If it doesn't work, please create another USB-log. >>>>>>> >>>>> >>>>> Sorry it took me so long to test the patch, but the results look >>>>> promising, I actually got various keycodes! >>>>> >>>>> dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>> >>>>> evtest was also generating output >>>>> >>>>> Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), >>>>> value >>>>> 61d618e7 >>>>> Event: time 1355705906.950551, -------------- SYN_REPORT ------------ >>>>> >>>>> This is the current patch I'm using: >>>>> http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>> >>>>> What needs to be done to generate a keymap file? >>>>> >>>>> Is there anything I can collect or try to do, to get channel scanning >>>>> working? >>>>> >>>>> Just let me know what you need me to do. I really appreciate all the >>>>> help! >>>> >>>> You don't need to do nothing as that remote is already there. Just >>>> ensure buttons are same and we are happy. >>>> http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 >>>> >>>> >>>> >>>> >>>> >>>> RC_MAP_MSI_DIGIVOX_III should be added to your device profile in order >>>> to load correct keytable by default. You could test it easily, just add >>>> following definition >>>> >>>> .ir_codes = RC_MAP_MSI_DIGIVOX_III, >>>> >>>> to em28xx-cards.c board config and it is all. >>>> >>>> regards >>>> Antti >>>> >>> Maybe I'm missing something but these are different key codes and >>> lengths. >>> >>> tux:~ $ echo 0x61d643bc | wc -c # my dmesg dump >>> 11 >>> tux:~ $ echo 0x61d601 | wc -c # DIGIVOX mini III >>> 9 >> >> 0x61d643bc == 0x61d643 >> 0x61d601fe == 0x61d601 >> >> Those are same codes, other (debug) is just 32bit full format. Last byte >> in that case is dropped out as it is used for parity check - formula: DD >> == ~DD >> >>> As I understand it, this was the whole reason for the patches that >>> Mauros wrote. >> >> Nope, the reason was it didn't support 32bit at all. > > I looked the patch and it seems like it should store and print 24bit > scancode for your remote. Maybe you didn't set default remote end it > fall back to unknown remote protocol which stores all bytes. Or some > other bug. Test it with default keytable (RC_MAP_MSI_DIGIVOX_III) and if > it does not output numbers there must be a bug. I am too lazy to test it > currently. > > regards > Antti > > I am using the RC_MAP_MSI_DIGIVOX_III mapping + .ir_codes = RC_MAP_MSI_DIGIVOX_III, http://pyther.net/a/digivox_atsc/dec16/msi_digivox_atsc.patch Matthew -- 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
On 12/17/2012 01:17 PM, Matthew Gyurgyik wrote: > On 12/17/2012 06:08 AM, Antti Palosaari wrote: >> On 12/17/2012 11:33 AM, Antti Palosaari wrote: >>> On 12/17/2012 03:37 AM, Matthew Gyurgyik wrote: >>>> On 12/16/2012 08:26 PM, Antti Palosaari wrote: >>>>> On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: >>>>>> On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>>>>>>> Matthew, could you please validate your test results and try Mauros >>>>>>>> patches ? If it doesn't work, please create another USB-log. >>>>>>>> >>>>>> >>>>>> Sorry it took me so long to test the patch, but the results look >>>>>> promising, I actually got various keycodes! >>>>>> >>>>>> dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>> >>>>>> evtest was also generating output >>>>>> >>>>>> Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), >>>>>> value >>>>>> 61d618e7 >>>>>> Event: time 1355705906.950551, -------------- SYN_REPORT ------------ >>>>>> >>>>>> This is the current patch I'm using: >>>>>> http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>> >>>>>> What needs to be done to generate a keymap file? >>>>>> >>>>>> Is there anything I can collect or try to do, to get channel scanning >>>>>> working? >>>>>> >>>>>> Just let me know what you need me to do. I really appreciate all the >>>>>> help! >>>>> >>>>> You don't need to do nothing as that remote is already there. Just >>>>> ensure buttons are same and we are happy. >>>>> http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> RC_MAP_MSI_DIGIVOX_III should be added to your device profile in order >>>>> to load correct keytable by default. You could test it easily, just >>>>> add >>>>> following definition >>>>> >>>>> .ir_codes = RC_MAP_MSI_DIGIVOX_III, >>>>> >>>>> to em28xx-cards.c board config and it is all. >>>>> >>>>> regards >>>>> Antti >>>>> >>>> Maybe I'm missing something but these are different key codes and >>>> lengths. >>>> >>>> tux:~ $ echo 0x61d643bc | wc -c # my dmesg dump >>>> 11 >>>> tux:~ $ echo 0x61d601 | wc -c # DIGIVOX mini III >>>> 9 >>> >>> 0x61d643bc == 0x61d643 >>> 0x61d601fe == 0x61d601 >>> >>> Those are same codes, other (debug) is just 32bit full format. Last byte >>> in that case is dropped out as it is used for parity check - formula: DD >>> == ~DD >>> >>>> As I understand it, this was the whole reason for the patches that >>>> Mauros wrote. >>> >>> Nope, the reason was it didn't support 32bit at all. >> >> I looked the patch and it seems like it should store and print 24bit >> scancode for your remote. Maybe you didn't set default remote end it >> fall back to unknown remote protocol which stores all bytes. Or some >> other bug. Test it with default keytable (RC_MAP_MSI_DIGIVOX_III) and if >> it does not output numbers there must be a bug. I am too lazy to test it >> currently. >> >> regards >> Antti >> >> > I am using the RC_MAP_MSI_DIGIVOX_III mapping > > + .ir_codes = RC_MAP_MSI_DIGIVOX_III, > http://pyther.net/a/digivox_atsc/dec16/msi_digivox_atsc.patch I tested Mauros patch with nanoStick T2 290e, using 24bit NEC remote - worked fine. Your patch is hard to read as it contains that remote patch too. But what I looked one difference which look suspicious - it is that: .xclk = EM28XX_XCLK_FREQUENCY_12MHZ, could you remove and test? If it is really that one, then there is a bug in Mauros patches and it breaks all devices having NEC remote mapped currently. regards Antti
Em 17-12-2012 10:30, Antti Palosaari escreveu: > On 12/17/2012 01:17 PM, Matthew Gyurgyik wrote: >> On 12/17/2012 06:08 AM, Antti Palosaari wrote: >>> On 12/17/2012 11:33 AM, Antti Palosaari wrote: >>>> On 12/17/2012 03:37 AM, Matthew Gyurgyik wrote: >>>>> On 12/16/2012 08:26 PM, Antti Palosaari wrote: >>>>>> On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: >>>>>>> On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>>>>>>>> Matthew, could you please validate your test results and try Mauros >>>>>>>>> patches ? If it doesn't work, please create another USB-log. >>>>>>>>> >>>>>>> >>>>>>> Sorry it took me so long to test the patch, but the results look >>>>>>> promising, I actually got various keycodes! >>>>>>> >>>>>>> dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>>> >>>>>>> evtest was also generating output >>>>>>> >>>>>>> Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), >>>>>>> value >>>>>>> 61d618e7 >>>>>>> Event: time 1355705906.950551, -------------- SYN_REPORT ------------ >>>>>>> >>>>>>> This is the current patch I'm using: >>>>>>> http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>>> >>>>>>> What needs to be done to generate a keymap file? >>>>>>> >>>>>>> Is there anything I can collect or try to do, to get channel scanning >>>>>>> working? >>>>>>> >>>>>>> Just let me know what you need me to do. I really appreciate all the >>>>>>> help! >>>>>> >>>>>> You don't need to do nothing as that remote is already there. Just >>>>>> ensure buttons are same and we are happy. >>>>>> http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> RC_MAP_MSI_DIGIVOX_III should be added to your device profile in order >>>>>> to load correct keytable by default. You could test it easily, just >>>>>> add >>>>>> following definition >>>>>> >>>>>> .ir_codes = RC_MAP_MSI_DIGIVOX_III, >>>>>> >>>>>> to em28xx-cards.c board config and it is all. >>>>>> >>>>>> regards >>>>>> Antti >>>>>> >>>>> Maybe I'm missing something but these are different key codes and >>>>> lengths. >>>>> >>>>> tux:~ $ echo 0x61d643bc | wc -c # my dmesg dump >>>>> 11 >>>>> tux:~ $ echo 0x61d601 | wc -c # DIGIVOX mini III >>>>> 9 >>>> >>>> 0x61d643bc == 0x61d643 >>>> 0x61d601fe == 0x61d601 >>>> >>>> Those are same codes, other (debug) is just 32bit full format. Last byte >>>> in that case is dropped out as it is used for parity check - formula: DD >>>> == ~DD >>>> >>>>> As I understand it, this was the whole reason for the patches that >>>>> Mauros wrote. >>>> >>>> Nope, the reason was it didn't support 32bit at all. >>> >>> I looked the patch and it seems like it should store and print 24bit >>> scancode for your remote. Maybe you didn't set default remote end it >>> fall back to unknown remote protocol which stores all bytes. Or some >>> other bug. Test it with default keytable (RC_MAP_MSI_DIGIVOX_III) and if >>> it does not output numbers there must be a bug. I am too lazy to test it >>> currently. >>> >>> regards >>> Antti >>> >>> >> I am using the RC_MAP_MSI_DIGIVOX_III mapping >> >> + .ir_codes = RC_MAP_MSI_DIGIVOX_III, >> http://pyther.net/a/digivox_atsc/dec16/msi_digivox_atsc.patch > > I tested Mauros patch with nanoStick T2 290e, using 24bit NEC remote - worked fine. Your patch is hard to read as it contains that remote patch too. But what I looked one difference which look suspicious - it is that: > .xclk = EM28XX_XCLK_FREQUENCY_12MHZ, > > could you remove and test? > > If it is really that one, then there is a bug in Mauros patches and it breaks all devices having NEC remote mapped currently. The em28xx-input should not be touching on xclk frequency changes. Some devices require specific settings there in order to work, and mangling it is a very bad idea. Btw, I don't think that are there any bugs with regards to that, as we use em28xx_write_reg_bits(): /* * em28xx_write_reg_bits() * sets only some bits (specified by bitmask) of a register, by first reading * the actual value */ int em28xx_write_reg_bits(struct em28xx *dev, u16 reg, u8 val, u8 bitmask) { ... newval = (((u8) oldval) & ~bitmask) | (val & bitmask); return em28xx_write_regs(dev, reg, &newval, 1); } From patch 2/2: + 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; + } else { + *rc_type = ir->rc_type; + return -EINVAL; + } + ir->get_key = default_polling_getkey; + em28xx_write_reg_bits(dev, EM28XX_R0F_XCLK, dev->board.xclk, + EM28XX_XCLK_IR_RC5_MODE); (this is for em2860 code, but em2874_ir_change_protocol() has a similar logic - the only exception is the support for RC6_0 as well) This line: em28xx_write_reg_bits(dev, EM28XX_R0F_XCLK, dev->board.xclk, EM28XX_XCLK_IR_RC5_MODE); Warrants that only the EM28XX_XCLK_IR_RC5_MODE bit is affected. So, except if I'm missing something, the implementation looks correct on my eyes. Regards, Mauro -- 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
Hi Matthew, Em 17-12-2012 09:17, Matthew Gyurgyik escreveu: > On 12/17/2012 06:08 AM, Antti Palosaari wrote: >> On 12/17/2012 11:33 AM, Antti Palosaari wrote: >>> On 12/17/2012 03:37 AM, Matthew Gyurgyik wrote: >>>> On 12/16/2012 08:26 PM, Antti Palosaari wrote: >>>>> On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: >>>>>> On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>>>>>>> Matthew, could you please validate your test results and try Mauros >>>>>>>> patches ? If it doesn't work, please create another USB-log. >>>>>>>> >>>>>> >>>>>> Sorry it took me so long to test the patch, but the results look >>>>>> promising, I actually got various keycodes! >>>>>> >>>>>> dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>> >>>>>> evtest was also generating output >>>>>> >>>>>> Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), >>>>>> value >>>>>> 61d618e7 >>>>>> Event: time 1355705906.950551, -------------- SYN_REPORT ------------ >>>>>> >>>>>> This is the current patch I'm using: >>>>>> http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>> >>>>>> What needs to be done to generate a keymap file? >>>>>> >>>>>> Is there anything I can collect or try to do, to get channel scanning >>>>>> working? >>>>>> >>>>>> Just let me know what you need me to do. I really appreciate all the >>>>>> help! >>>>> >>>>> You don't need to do nothing as that remote is already there. Just >>>>> ensure buttons are same and we are happy. >>>>> http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> RC_MAP_MSI_DIGIVOX_III should be added to your device profile in order >>>>> to load correct keytable by default. You could test it easily, just add >>>>> following definition >>>>> >>>>> .ir_codes = RC_MAP_MSI_DIGIVOX_III, >>>>> >>>>> to em28xx-cards.c board config and it is all. >>>>> >>>>> regards >>>>> Antti >>>>> >>>> Maybe I'm missing something but these are different key codes and >>>> lengths. >>>> >>>> tux:~ $ echo 0x61d643bc | wc -c # my dmesg dump >>>> 11 >>>> tux:~ $ echo 0x61d601 | wc -c # DIGIVOX mini III >>>> 9 >>> >>> 0x61d643bc == 0x61d643 >>> 0x61d601fe == 0x61d601 >>> >>> Those are same codes, other (debug) is just 32bit full format. Last byte >>> in that case is dropped out as it is used for parity check - formula: DD >>> == ~DD >>> >>>> As I understand it, this was the whole reason for the patches that >>>> Mauros wrote. >>> >>> Nope, the reason was it didn't support 32bit at all. >> >> I looked the patch and it seems like it should store and print 24bit >> scancode for your remote. Maybe you didn't set default remote end it >> fall back to unknown remote protocol which stores all bytes. Or some >> other bug. Test it with default keytable (RC_MAP_MSI_DIGIVOX_III) and if >> it does not output numbers there must be a bug. I am too lazy to test it >> currently. >> >> regards >> Antti >> >> > I am using the RC_MAP_MSI_DIGIVOX_III mapping > > + .ir_codes = RC_MAP_MSI_DIGIVOX_III, > http://pyther.net/a/digivox_atsc/dec16/msi_digivox_atsc.patch From this log: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt You clearly have a "standard" NEC-extended IR - e. g. 24 bits per scancode, 16 bits for address, 8 bits for command. Instead of using evtest, I really recomend you to use ir-keytable (part of v4l-utils package). You can compile it directly from our git tree: http://git.linuxtv.org/v4l-utils.git with: $ autoreconf -vfi $ ./configure $ make # make install The version there has a few updates to provide a more complete report. In order to use it in test mode, you can just do: # ir-keycode -t If your IR is at rc0 (otherwise, you'll need to use the "-s rc1" parameter). It should print all events produced by an input device, including the scancodes (EV_MSC) and keystrokes (EV_KEY). One question: are you compiling a 32 bits or a 64 bits kernel? The is/were a bug with gcc and switch() when a 64 bits int is used on switch. Maybe we'll need to change the switch at the nec handling by a series of IFs due to such bug. Regards, Mauro > > Matthew -- 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
On 12/17/2012 11:14 AM, Mauro Carvalho Chehab wrote: > Hi Matthew, > > Em 17-12-2012 09:17, Matthew Gyurgyik escreveu: >> On 12/17/2012 06:08 AM, Antti Palosaari wrote: >>> On 12/17/2012 11:33 AM, Antti Palosaari wrote: >>>> On 12/17/2012 03:37 AM, Matthew Gyurgyik wrote: >>>>> On 12/16/2012 08:26 PM, Antti Palosaari wrote: >>>>>> On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: >>>>>>> On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>>>>>>>> Matthew, could you please validate your test results and try >>>>>>>>> Mauros >>>>>>>>> patches ? If it doesn't work, please create another USB-log. >>>>>>>>> >>>>>>> >>>>>>> Sorry it took me so long to test the patch, but the results look >>>>>>> promising, I actually got various keycodes! >>>>>>> >>>>>>> dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>>> >>>>>>> evtest was also generating output >>>>>>> >>>>>>> Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), >>>>>>> value >>>>>>> 61d618e7 >>>>>>> Event: time 1355705906.950551, -------------- SYN_REPORT >>>>>>> ------------ >>>>>>> >>>>>>> This is the current patch I'm using: >>>>>>> http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>>> >>>>>>> What needs to be done to generate a keymap file? >>>>>>> >>>>>>> Is there anything I can collect or try to do, to get channel >>>>>>> scanning >>>>>>> working? >>>>>>> >>>>>>> Just let me know what you need me to do. I really appreciate all the >>>>>>> help! >>>>>> >>>>>> You don't need to do nothing as that remote is already there. Just >>>>>> ensure buttons are same and we are happy. >>>>>> http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> RC_MAP_MSI_DIGIVOX_III should be added to your device profile in >>>>>> order >>>>>> to load correct keytable by default. You could test it easily, >>>>>> just add >>>>>> following definition >>>>>> >>>>>> .ir_codes = RC_MAP_MSI_DIGIVOX_III, >>>>>> >>>>>> to em28xx-cards.c board config and it is all. >>>>>> >>>>>> regards >>>>>> Antti >>>>>> >>>>> Maybe I'm missing something but these are different key codes and >>>>> lengths. >>>>> >>>>> tux:~ $ echo 0x61d643bc | wc -c # my dmesg dump >>>>> 11 >>>>> tux:~ $ echo 0x61d601 | wc -c # DIGIVOX mini III >>>>> 9 >>>> >>>> 0x61d643bc == 0x61d643 >>>> 0x61d601fe == 0x61d601 >>>> >>>> Those are same codes, other (debug) is just 32bit full format. Last >>>> byte >>>> in that case is dropped out as it is used for parity check - >>>> formula: DD >>>> == ~DD >>>> >>>>> As I understand it, this was the whole reason for the patches that >>>>> Mauros wrote. >>>> >>>> Nope, the reason was it didn't support 32bit at all. >>> >>> I looked the patch and it seems like it should store and print 24bit >>> scancode for your remote. Maybe you didn't set default remote end it >>> fall back to unknown remote protocol which stores all bytes. Or some >>> other bug. Test it with default keytable (RC_MAP_MSI_DIGIVOX_III) and if >>> it does not output numbers there must be a bug. I am too lazy to test it >>> currently. >>> >>> regards >>> Antti >>> >>> >> I am using the RC_MAP_MSI_DIGIVOX_III mapping >> >> + .ir_codes = RC_MAP_MSI_DIGIVOX_III, >> http://pyther.net/a/digivox_atsc/dec16/msi_digivox_atsc.patch > > From this log: > http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt > > You clearly have a "standard" NEC-extended IR - e. g. 24 bits per scancode, > 16 bits for address, 8 bits for command. > > Instead of using evtest, I really recomend you to use ir-keytable > (part of v4l-utils package). You can compile it directly from our > git tree: http://git.linuxtv.org/v4l-utils.git with: > $ autoreconf -vfi > $ ./configure > $ make > # make install > > The version there has a few updates to provide a more complete report. > > In order to use it in test mode, you can just do: > # ir-keycode -t > > If your IR is at rc0 (otherwise, you'll need to use the "-s rc1" > parameter). > > It should print all events produced by an input device, including > the scancodes (EV_MSC) and keystrokes (EV_KEY). > > One question: are you compiling a 32 bits or a 64 bits kernel? The is/were > a bug with gcc and switch() when a 64 bits int is used on switch. Maybe > we'll need to change the switch at the nec handling by a series of IFs > due to such bug. > I am compiling a 64 bit kernel. I attempted to build a new kernel, however I am running into some difficulties. With the upcoming holiday I probably won't be able to get test results until the beginning of January, due to not having access to my PC. I hope this is ok. > Regards, > Mauro > > >> >> Matthew > -- 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
On 12/17/2012 11:14 AM, Mauro Carvalho Chehab wrote: > Hi Matthew, > > Em 17-12-2012 09:17, Matthew Gyurgyik escreveu: >> On 12/17/2012 06:08 AM, Antti Palosaari wrote: >>> On 12/17/2012 11:33 AM, Antti Palosaari wrote: >>>> On 12/17/2012 03:37 AM, Matthew Gyurgyik wrote: >>>>> On 12/16/2012 08:26 PM, Antti Palosaari wrote: >>>>>> On 12/17/2012 03:09 AM, Matthew Gyurgyik wrote: >>>>>>> On 12/15/2012 06:21 PM, Frank Schäfer wrote: >>>>>>>>> Matthew, could you please validate your test results and try >>>>>>>>> Mauros >>>>>>>>> patches ? If it doesn't work, please create another USB-log. >>>>>>>>> >>>>>>> >>>>>>> Sorry it took me so long to test the patch, but the results look >>>>>>> promising, I actually got various keycodes! >>>>>>> >>>>>>> dmesg: http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>>> >>>>>>> evtest was also generating output >>>>>>> >>>>>>> Event: time 1355705906.950551, type 4 (EV_MSC), code 4 (MSC_SCAN), >>>>>>> value >>>>>>> 61d618e7 >>>>>>> Event: time 1355705906.950551, -------------- SYN_REPORT >>>>>>> ------------ >>>>>>> >>>>>>> This is the current patch I'm using: >>>>>>> http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt >>>>>>> >>>>>>> What needs to be done to generate a keymap file? >>>>>>> >>>>>>> Is there anything I can collect or try to do, to get channel >>>>>>> scanning >>>>>>> working? >>>>>>> >>>>>>> Just let me know what you need me to do. I really appreciate all the >>>>>>> help! >>>>>> >>>>>> You don't need to do nothing as that remote is already there. Just >>>>>> ensure buttons are same and we are happy. >>>>>> http://lxr.free-electrons.com/source/drivers/media/IR/keymaps/rc-msi-digivox-iii.c?v=2.6.37 >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> RC_MAP_MSI_DIGIVOX_III should be added to your device profile in >>>>>> order >>>>>> to load correct keytable by default. You could test it easily, >>>>>> just add >>>>>> following definition >>>>>> >>>>>> .ir_codes = RC_MAP_MSI_DIGIVOX_III, >>>>>> >>>>>> to em28xx-cards.c board config and it is all. >>>>>> >>>>>> regards >>>>>> Antti >>>>>> >>>>> Maybe I'm missing something but these are different key codes and >>>>> lengths. >>>>> >>>>> tux:~ $ echo 0x61d643bc | wc -c # my dmesg dump >>>>> 11 >>>>> tux:~ $ echo 0x61d601 | wc -c # DIGIVOX mini III >>>>> 9 >>>> >>>> 0x61d643bc == 0x61d643 >>>> 0x61d601fe == 0x61d601 >>>> >>>> Those are same codes, other (debug) is just 32bit full format. Last >>>> byte >>>> in that case is dropped out as it is used for parity check - >>>> formula: DD >>>> == ~DD >>>> >>>>> As I understand it, this was the whole reason for the patches that >>>>> Mauros wrote. >>>> >>>> Nope, the reason was it didn't support 32bit at all. >>> >>> I looked the patch and it seems like it should store and print 24bit >>> scancode for your remote. Maybe you didn't set default remote end it >>> fall back to unknown remote protocol which stores all bytes. Or some >>> other bug. Test it with default keytable (RC_MAP_MSI_DIGIVOX_III) and if >>> it does not output numbers there must be a bug. I am too lazy to test it >>> currently. >>> >>> regards >>> Antti >>> >>> >> I am using the RC_MAP_MSI_DIGIVOX_III mapping >> >> + .ir_codes = RC_MAP_MSI_DIGIVOX_III, >> http://pyther.net/a/digivox_atsc/dec16/msi_digivox_atsc.patch > > From this log: > http://pyther.net/a/digivox_atsc/dec16/dmesg_remote.txt > > You clearly have a "standard" NEC-extended IR - e. g. 24 bits per scancode, > 16 bits for address, 8 bits for command. > > Instead of using evtest, I really recomend you to use ir-keytable > (part of v4l-utils package). You can compile it directly from our > git tree: http://git.linuxtv.org/v4l-utils.git with: > $ autoreconf -vfi > $ ./configure > $ make > # make install > > The version there has a few updates to provide a more complete report. > > In order to use it in test mode, you can just do: > # ir-keycode -t > > If your IR is at rc0 (otherwise, you'll need to use the "-s rc1" > parameter). Here is the report (I pressed every key on the remote): http://pyther.net/a/digivox_atsc/dec17/ir-keytables.txt > > It should print all events produced by an input device, including > the scancodes (EV_MSC) and keystrokes (EV_KEY). > > One question: are you compiling a 32 bits or a 64 bits kernel? The is/were > a bug with gcc and switch() when a 64 bits int is used on switch. Maybe > we'll need to change the switch at the nec handling by a series of IFs > due to such bug. I am compiling a 64 bit kernel. > > Regards, > Mauro > > I can test patches Tue and Wed this week. Afterwards, I probably won't be able to test anything until Dec 28th/29th as I will be away from my workstation. In regards to my issue compiling my kernel, it helps if I include devtmpfs. :) Thanks, Matthew -- 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
On 12/18/2012 05:08 AM, Matthew Gyurgyik wrote: > I can test patches Tue and Wed this week. Afterwards, I probably won't > be able to test anything until Dec 28th/29th as I will be away from my > workstation. > > In regards to my issue compiling my kernel, it helps if I include > devtmpfs. :) Matthew, test? Both remote and television. http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/HU345-Q regards Antti
On 01/02/2013 03:59 PM, Antti Palosaari wrote: > On 12/18/2012 05:08 AM, Matthew Gyurgyik wrote: >> I can test patches Tue and Wed this week. Afterwards, I probably won't >> be able to test anything until Dec 28th/29th as I will be away from my >> workstation. >> >> In regards to my issue compiling my kernel, it helps if I include >> devtmpfs. :) > > Matthew, test? Both remote and television. > > http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/HU345-Q > > regards > Antti So using the HU345-Q branch I get the following results Remote: Using evtest it looks like all the key codes register correctly. (KEY_1, KEY_YELLOW, KEY_VOLUMEUP, etc...) However, ir_keytable fails [root@tux bin]# ./ir-keytable -t Not found device rc0 Tunning: I did a basic test with mplayer and tunning worked. I'll have to do more testing. Scanning: Running a scan resulted in a kernel panic. Scan command: scan -A 2 -t 1 /usr/share/dvb/atsc/us-Cable-Standard-center-frequencies-QAM256 > ~/channels_msidigivox.conf Kernel Messages: http://pyther.net/a/digivox_atsc/jan02/kernel_log.txt Let me know what additional info I can provide. As always, I appreciate the help! Thanks, Matthew -- 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
On 01/02/2013 09:53 PM, Matthew Gyurgyik wrote: > On 01/02/2013 03:59 PM, Antti Palosaari wrote: >> On 12/18/2012 05:08 AM, Matthew Gyurgyik wrote: >>> I can test patches Tue and Wed this week. Afterwards, I probably won't >>> be able to test anything until Dec 28th/29th as I will be away from my >>> workstation. >>> >>> In regards to my issue compiling my kernel, it helps if I include >>> devtmpfs. :) >> >> Matthew, test? Both remote and television. >> >> http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/HU345-Q >> >> regards >> Antti > > > So using the HU345-Q branch I get the following results > > Remote: > > Using evtest it looks like all the key codes register correctly. (KEY_1, > KEY_YELLOW, KEY_VOLUMEUP, etc...) > > However, ir_keytable fails > > [root@tux bin]# ./ir-keytable -t > Not found device rc0 > > Tunning: > > I did a basic test with mplayer and tunning worked. I'll have to do more > testing. > > Scanning: > > Running a scan resulted in a kernel panic. > > Scan command: scan -A 2 -t 1 > /usr/share/dvb/atsc/us-Cable-Standard-center-frequencies-QAM256 > > ~/channels_msidigivox.conf > > Kernel Messages: http://pyther.net/a/digivox_atsc/jan02/kernel_log.txt > > Let me know what additional info I can provide. As always, I appreciate > the help! > > Thanks, > Matthew > Antti, Is there any follow up testing I could do? Is there any additional information you need from me. Thanks, Matthew -- 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
On 01/20/2013 04:40 PM, Matthew Gyurgyik wrote: > On 01/02/2013 09:53 PM, Matthew Gyurgyik wrote: >> On 01/02/2013 03:59 PM, Antti Palosaari wrote: >>> On 12/18/2012 05:08 AM, Matthew Gyurgyik wrote: >>>> I can test patches Tue and Wed this week. Afterwards, I probably won't >>>> be able to test anything until Dec 28th/29th as I will be away from my >>>> workstation. >>>> >>>> In regards to my issue compiling my kernel, it helps if I include >>>> devtmpfs. :) >>> >>> Matthew, test? Both remote and television. >>> >>> http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/HU345-Q >>> >>> regards >>> Antti >> >> >> So using the HU345-Q branch I get the following results >> >> Remote: >> >> Using evtest it looks like all the key codes register correctly. (KEY_1, >> KEY_YELLOW, KEY_VOLUMEUP, etc...) >> >> However, ir_keytable fails >> >> [root@tux bin]# ./ir-keytable -t >> Not found device rc0 >> >> Tunning: >> >> I did a basic test with mplayer and tunning worked. I'll have to do more >> testing. >> >> Scanning: >> >> Running a scan resulted in a kernel panic. >> >> Scan command: scan -A 2 -t 1 >> /usr/share/dvb/atsc/us-Cable-Standard-center-frequencies-QAM256 > >> ~/channels_msidigivox.conf >> >> Kernel Messages: http://pyther.net/a/digivox_atsc/jan02/kernel_log.txt >> >> Let me know what additional info I can provide. As always, I appreciate >> the help! >> >> Thanks, >> Matthew >> > > > Antti, > > Is there any follow up testing I could do? Is there any additional > information you need from me. > > Thanks, > Matthew Matthew, Thank you for testing continuously! I looked it and for my eyes it works as it should (both television and remote controller as you reported). All those bugs you mention has no relations to that certain device. I think all are general em28xx driver bugs. There has been recently quite much changes done for the em28xx driver and probably some of those findings are already fixed. I am not em28xx driver expert, due to that it is hard to say what is wrong. I will try to make final patch soon and after your test it could be sent to the mainline. regards Antti
On 01/20/2013 12:46 PM, Antti Palosaari wrote: > On 01/20/2013 04:40 PM, Matthew Gyurgyik wrote: >> On 01/02/2013 09:53 PM, Matthew Gyurgyik wrote: >>> On 01/02/2013 03:59 PM, Antti Palosaari wrote: >>>> On 12/18/2012 05:08 AM, Matthew Gyurgyik wrote: >>>>> I can test patches Tue and Wed this week. Afterwards, I probably won't >>>>> be able to test anything until Dec 28th/29th as I will be away from my >>>>> workstation. >>>>> >>>>> In regards to my issue compiling my kernel, it helps if I include >>>>> devtmpfs. :) >>>> >>>> Matthew, test? Both remote and television. >>>> >>>> http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/HU345-Q >>>> >>>> >>>> regards >>>> Antti >>> >>> >>> So using the HU345-Q branch I get the following results >>> >>> Remote: >>> >>> Using evtest it looks like all the key codes register correctly. (KEY_1, >>> KEY_YELLOW, KEY_VOLUMEUP, etc...) >>> >>> However, ir_keytable fails >>> >>> [root@tux bin]# ./ir-keytable -t >>> Not found device rc0 >>> >>> Tunning: >>> >>> I did a basic test with mplayer and tunning worked. I'll have to do more >>> testing. >>> >>> Scanning: >>> >>> Running a scan resulted in a kernel panic. >>> >>> Scan command: scan -A 2 -t 1 >>> /usr/share/dvb/atsc/us-Cable-Standard-center-frequencies-QAM256 > >>> ~/channels_msidigivox.conf >>> >>> Kernel Messages: http://pyther.net/a/digivox_atsc/jan02/kernel_log.txt >>> >>> Let me know what additional info I can provide. As always, I appreciate >>> the help! >>> >>> Thanks, >>> Matthew >>> >> >> >> Antti, >> >> Is there any follow up testing I could do? Is there any additional >> information you need from me. >> >> Thanks, >> Matthew > > Matthew, > Thank you for testing continuously! I looked it and for my eyes it works > as it should (both television and remote controller as you reported). > All those bugs you mention has no relations to that certain device. I > think all are general em28xx driver bugs. There has been recently quite > much changes done for the em28xx driver and probably some of those > findings are already fixed. I am not em28xx driver expert, due to that > it is hard to say what is wrong. I will try to make final patch soon and > after your test it could be sent to the mainline. > > regards > Antti > Any update on this? Thanks, Matthew -- 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
On 02/17/2013 01:38 AM, Matthew Gyurgyik wrote: > On 01/20/2013 12:46 PM, Antti Palosaari wrote: >> On 01/20/2013 04:40 PM, Matthew Gyurgyik wrote: >>> On 01/02/2013 09:53 PM, Matthew Gyurgyik wrote: >>>> On 01/02/2013 03:59 PM, Antti Palosaari wrote: >>>>> On 12/18/2012 05:08 AM, Matthew Gyurgyik wrote: >>>>>> I can test patches Tue and Wed this week. Afterwards, I probably >>>>>> won't >>>>>> be able to test anything until Dec 28th/29th as I will be away >>>>>> from my >>>>>> workstation. >>>>>> >>>>>> In regards to my issue compiling my kernel, it helps if I include >>>>>> devtmpfs. :) >>>>> >>>>> Matthew, test? Both remote and television. >>>>> >>>>> http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/HU345-Q >>>>> >>>>> >>>>> >>>>> regards >>>>> Antti >>>> >>>> >>>> So using the HU345-Q branch I get the following results >>>> >>>> Remote: >>>> >>>> Using evtest it looks like all the key codes register correctly. >>>> (KEY_1, >>>> KEY_YELLOW, KEY_VOLUMEUP, etc...) >>>> >>>> However, ir_keytable fails >>>> >>>> [root@tux bin]# ./ir-keytable -t >>>> Not found device rc0 >>>> >>>> Tunning: >>>> >>>> I did a basic test with mplayer and tunning worked. I'll have to do >>>> more >>>> testing. >>>> >>>> Scanning: >>>> >>>> Running a scan resulted in a kernel panic. >>>> >>>> Scan command: scan -A 2 -t 1 >>>> /usr/share/dvb/atsc/us-Cable-Standard-center-frequencies-QAM256 > >>>> ~/channels_msidigivox.conf >>>> >>>> Kernel Messages: http://pyther.net/a/digivox_atsc/jan02/kernel_log.txt >>>> >>>> Let me know what additional info I can provide. As always, I appreciate >>>> the help! >>>> >>>> Thanks, >>>> Matthew >>>> >>> >>> >>> Antti, >>> >>> Is there any follow up testing I could do? Is there any additional >>> information you need from me. >>> >>> Thanks, >>> Matthew >> >> Matthew, >> Thank you for testing continuously! I looked it and for my eyes it works >> as it should (both television and remote controller as you reported). >> All those bugs you mention has no relations to that certain device. I >> think all are general em28xx driver bugs. There has been recently quite >> much changes done for the em28xx driver and probably some of those >> findings are already fixed. I am not em28xx driver expert, due to that >> it is hard to say what is wrong. I will try to make final patch soon and >> after your test it could be sent to the mainline. >> >> regards >> Antti >> > Any update on this? > > Thanks, > Matthew I rebased it to the latest LinuxTV 3.9. There is quite a lot of changes done for em28xx driver so it could work. Please test. http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/HU345-Q regards Antti
On 02/24/2013 05:23 PM, Antti Palosaari wrote: > > I rebased it to the latest LinuxTV 3.9. There is quite a lot of changes > done for em28xx driver so it could work. Please test. > > http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/HU345-Q > > regards > Antti > I checked out the branch and these are my results from some brief testing. Channel scan: results in INPUT/output error Tuning and watching a channel. With mplayer when tuning to a specific channel it seemed to play without issue. Occasionally when starting mplayer, the audio and video was way out of sync. It was if the video was only displaying a few frame per second. Remote: the number keys worked as the inputed their respective value into my terminal. I will test the others tomorrow. http://pyther.net/a/digivox_atsc/feb24/scan.txt http://pyther.net/a/digivox_atsc/feb24/dmesg.txt Thanks, Matthew -- 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
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 */ + poll_result->scancode = (msg[1] << 24) | + (msg[2] << 16) | + (msg[3] << 8) | + msg[4]; + else if ((msg[1] ^ msg[2]) != 0xff) /* 24 bits NEC */ + poll_result->scancode = (msg[1] << 16) | + (msg[3] << 8) | + msg[4]; + else /* Normal NEC */ + poll_result->scancode = msg[1] << 8 | msg[3]; + break; + default: + poll_result->scancode = (msg[1] << 24) | (msg[2] << 16) | + (msg[3] << 8) | msg[4]; + break; + } return 0; } @@ -294,17 +309,16 @@ static void em28xx_ir_handle_key(struct em28xx_IR *ir) } if (unlikely(poll_result.read_count != ir->last_readcount)) { - dprintk("%s: toggle: %d, count: %d, key 0x%02x%02x\n", __func__, + dprintk("%s: toggle: %d, count: %d, key 0x%04x\n", __func__, poll_result.toggle_bit, poll_result.read_count, - poll_result.rc_address, poll_result.rc_data[0]); + poll_result.scancode); if (ir->full_code) rc_keydown(ir->rc, - poll_result.rc_address << 8 | - poll_result.rc_data[0], + poll_result.scancode, poll_result.toggle_bit); else rc_keydown(ir->rc, - poll_result.rc_data[0], + poll_result.scancode & 0xff, poll_result.toggle_bit); if (ir->dev->chip_id == CHIP_ID_EM2874 || @@ -359,11 +373,13 @@ static int em28xx_ir_change_protocol(struct rc_dev *rc_dev, u64 rc_type) ir->full_code = 1; } else if (rc_type == RC_TYPE_NEC) { dev->board.xclk &= ~EM28XX_XCLK_IR_RC5_MODE; - ir_config = EM2874_IR_NEC; + ir_config = EM2874_IR_NEC | EM2874_IR_NEC_NO_PARITY; ir->full_code = 1; } else if (rc_type != RC_TYPE_UNKNOWN) rc = -EINVAL; + ir->rc_type = rc_type; + em28xx_write_reg_bits(dev, EM28XX_R0F_XCLK, dev->board.xclk, EM28XX_XCLK_IR_RC5_MODE); diff --git a/drivers/media/usb/em28xx/em28xx-reg.h b/drivers/media/usb/em28xx/em28xx-reg.h index 6ff3682..2ad3573 100644 --- a/drivers/media/usb/em28xx/em28xx-reg.h +++ b/drivers/media/usb/em28xx/em28xx-reg.h @@ -177,6 +177,7 @@ /* em2874 IR config register (0x50) */ #define EM2874_IR_NEC 0x00 +#define EM2874_IR_NEC_NO_PARITY 0x01 #define EM2874_IR_RC5 0x04 #define EM2874_IR_RC6_MODE_0 0x08 #define EM2874_IR_RC6_MODE_6A 0x0b