em28xx: msi Digivox ATSC board id [0db0:8810]

Commit Message

Mauro Carvalho Chehab Dec. 15, 2012, 12:26 a.m. UTC

Em Fri, 14 Dec 2012 17:39:50 -0200
Mauro Carvalho Chehab <[e-mail]> 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 <[e-mail]>


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mauro Carvalho Chehab Dec. 15, 2012, 12:34 a.m. UTC | #1

Em Fri, 14 Dec 2012 22:26:31 -0200
Mauro Carvalho Chehab <[e-mail]> escreveu:

> Em Fri, 14 Dec 2012 17:39:50 -0200
> Mauro Carvalho Chehab <[e-mail]> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Dec. 15, 2012, 12:56 a.m. UTC | #2

On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote:
> Em Fri, 14 Dec 2012 17:39:50 -0200
> Mauro Carvalho Chehab <[e-mail]> 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 <[e-mail]>





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

Mauro Carvalho Chehab Dec. 15, 2012, 1:03 a.m. UTC | #3

Em Sat, 15 Dec 2012 02:56:25 +0200
Antti Palosaari <[e-mail]> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Dec. 15, 2012, 1:12 a.m. UTC | #4

On 12/15/2012 03:03 AM, Mauro Carvalho Chehab wrote:
> Em Sat, 15 Dec 2012 02:56:25 +0200
> Antti Palosaari <[e-mail]> 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

Mauro Carvalho Chehab Dec. 15, 2012, 1:39 a.m. UTC | #5

Em Sat, 15 Dec 2012 03:12:58 +0200
Antti Palosaari <[e-mail]> escreveu:

> On 12/15/2012 03:03 AM, Mauro Carvalho Chehab wrote:
> > Em Sat, 15 Dec 2012 02:56:25 +0200
> > Antti Palosaari <[e-mail]> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mauro Carvalho Chehab Dec. 15, 2012, 1:54 a.m. UTC | #6

Em Sat, 15 Dec 2012 02:56:25 +0200
Antti Palosaari <[e-mail]> escreveu:

> On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote:
> > Em Fri, 14 Dec 2012 17:39:50 -0200
> > Mauro Carvalho Chehab <[e-mail]> 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 <[e-mail]>
> 
> 
> 
> 
> 
> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Frank Schäfer Dec. 15, 2012, 1:11 p.m. UTC | #7

Am 15.12.2012 02:54, schrieb Mauro Carvalho Chehab:
> Em Sat, 15 Dec 2012 02:56:25 +0200
> Antti Palosaari <[e-mail]> escreveu:
>
>> On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote:
>>> Em Fri, 14 Dec 2012 17:39:50 -0200
>>> Mauro Carvalho Chehab <[e-mail]> 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 <[e-mail]>
>>
>>
>>
>>
>> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mauro Carvalho Chehab Dec. 15, 2012, 1:34 p.m. UTC | #8

Em Sat, 15 Dec 2012 14:11:24 +0100
Frank Schäfer <[e-mail]> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Dec. 15, 2012, 1:38 p.m. UTC | #9

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 <[e-mail]> escreveu:
>>
>>> On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote:
>>>> Em Fri, 14 Dec 2012 17:39:50 -0200
>>>> Mauro Carvalho Chehab <[e-mail]> 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 <[e-mail]>
>>>
>>>
>>>
>>>
>>> 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

Frank Schäfer Dec. 15, 2012, 4:21 p.m. UTC | #10

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 <[e-mail]> escreveu:
>>>
>>>> On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote:
>>>>> Em Fri, 14 Dec 2012 17:39:50 -0200
>>>>> Mauro Carvalho Chehab <[e-mail]> 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 <[e-mail]>
>>>>
>>>>
>>>>
>>>>
>>>> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Dec. 15, 2012, 4:51 p.m. UTC | #11

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 <[e-mail]> escreveu:
>>>>
>>>>> On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote:
>>>>>> Em Fri, 14 Dec 2012 17:39:50 -0200
>>>>>> Mauro Carvalho Chehab <[e-mail]> 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

Frank Schäfer Dec. 16, 2012, 6:15 p.m. UTC | #12

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 <[e-mail]> escreveu:
>>>>>
>>>>>> On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote:
>>>>>>> Em Fri, 14 Dec 2012 17:39:50 -0200
>>>>>>> Mauro Carvalho Chehab <[e-mail]> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Matthew Gyurgyik Dec. 17, 2012, 1:09 a.m. UTC | #13

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Dec. 17, 2012, 1:26 a.m. UTC | #14

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

Matthew Gyurgyik Dec. 17, 2012, 1:37 a.m. UTC | #15

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Dec. 17, 2012, 9:33 a.m. UTC | #16

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

Antti Palosaari Dec. 17, 2012, 11:08 a.m. UTC | #17

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

Matthew Gyurgyik Dec. 17, 2012, 11:17 a.m. UTC | #18

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Dec. 17, 2012, 12:30 p.m. UTC | #19

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

Mauro Carvalho Chehab Dec. 17, 2012, 3:53 p.m. UTC | #20

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mauro Carvalho Chehab Dec. 17, 2012, 4:14 p.m. UTC | #21

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Matthew Gyurgyik Dec. 18, 2012, 2:27 a.m. UTC | #22

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Matthew Gyurgyik Dec. 18, 2012, 3:08 a.m. UTC | #23

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Jan. 2, 2013, 8:59 p.m. UTC | #24

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

Matthew Gyurgyik Jan. 3, 2013, 2:53 a.m. UTC | #25

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Matthew Gyurgyik Jan. 20, 2013, 2:40 p.m. UTC | #26

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Jan. 20, 2013, 5:46 p.m. UTC | #27

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

Matthew Gyurgyik Feb. 16, 2013, 11:38 p.m. UTC | #28

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antti Palosaari Feb. 24, 2013, 10:23 p.m. UTC | #29

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

Matthew Gyurgyik Feb. 25, 2013, 1:58 a.m. UTC | #30

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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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