All of lore.kernel.org
 help / color / mirror / Atom feed
* 3.9.2 kernel - IR / em28xx_rc broken?
@ 2013-05-18 13:57 Chris Rankin
  2013-05-18 14:36 ` Frank Schäfer
  0 siblings, 1 reply; 25+ messages in thread
From: Chris Rankin @ 2013-05-18 13:57 UTC (permalink / raw)
  To: linux-media



Hi,

I have a PCTV 290e DVB2 adapter (em28xx, em28xx_dvb, em28xx_rc, cxd2820r), and I have just discovered that the IR remote control has stopped working with VDR when using a vanilla 3.9.2 kernel. Downgrading the kernel to 3.8.12 fixes things again. (Switching to my old DVB NOVA-T2 device fixes things too, although it cannot receive HDTV channels, of course).

Has anyone else noticed problems like this, please?

Thanks,
Chris


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-18 13:57 3.9.2 kernel - IR / em28xx_rc broken? Chris Rankin
@ 2013-05-18 14:36 ` Frank Schäfer
  2013-05-18 15:17   ` Chris Rankin
  0 siblings, 1 reply; 25+ messages in thread
From: Frank Schäfer @ 2013-05-18 14:36 UTC (permalink / raw)
  To: Chris Rankin; +Cc: linux-media

Am 18.05.2013 15:57, schrieb Chris Rankin:
> I have a PCTV 290e DVB2 adapter (em28xx, em28xx_dvb, em28xx_rc, cxd2820r), and I have just discovered that the IR remote control has stopped working with VDR when using a vanilla 3.9.2 kernel. Downgrading the kernel to 3.8.12 fixes things again. (Switching to my old DVB NOVA-T2 device fixes things too, although it cannot receive HDTV channels, of course).

Great. :( :( :(
There have been several changes in the em28xx and core RC code between 
3.8 and 3.9...
I can't see anything obvious, the RC device seems to be registered 
correctly.
Could you please bisect ?

Regards,
Frank

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-18 14:36 ` Frank Schäfer
@ 2013-05-18 15:17   ` Chris Rankin
  2013-05-18 16:58     ` Frank Schäfer
  0 siblings, 1 reply; 25+ messages in thread
From: Chris Rankin @ 2013-05-18 15:17 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: linux-media

----- Original Message -----

>> Am 18.05.2013 15:57, schrieb Chris Rankin:
>> I have a PCTV 290e DVB2 adapter (em28xx, em28xx_dvb, em28xx_rc, cxd2820r), and I have just discovered that the IR remote control has stopped working with VDR when using a vanilla 3.9.2 kernel.
>> Downgrading the kernel to 3.8.12 fixes things again. (Switching to my old DVB NOVA-T2 device fixes things too, although it cannot receive HDTV channels, of course).

> Great. :( :( :(
> There have been several changes in the em28xx and core RC code between 3.8 and 3.9...
> I can't see anything obvious, the RC device seems to be registered correctly.
> Could you please bisect ?

Unfortunately, no I can't. (No git tree here - just a tarball downloaded via FTP). However, maybe I could out some printk() statements into the code if you could point out where the "hot-spots" might be, please?

Cheers,
Chris


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-18 15:17   ` Chris Rankin
@ 2013-05-18 16:58     ` Frank Schäfer
  2013-05-18 21:02       ` Chris Rankin
  2013-05-18 21:11       ` Chris Rankin
  0 siblings, 2 replies; 25+ messages in thread
From: Frank Schäfer @ 2013-05-18 16:58 UTC (permalink / raw)
  To: Chris Rankin; +Cc: linux-media

Am 18.05.2013 17:17, schrieb Chris Rankin:
> ----- Original Message -----
>
>>> Am 18.05.2013 15:57, schrieb Chris Rankin:
>>> I have a PCTV 290e DVB2 adapter (em28xx, em28xx_dvb, em28xx_rc, cxd2820r), and I have just discovered that the IR remote control has stopped working with VDR when using a vanilla 3.9.2 kernel.
>>> Downgrading the kernel to 3.8.12 fixes things again. (Switching to my old DVB NOVA-T2 device fixes things too, although it cannot receive HDTV channels, of course).
>> Great. :( :( :(
>> There have been several changes in the em28xx and core RC code between 3.8 and 3.9...
>> I can't see anything obvious, the RC device seems to be registered correctly.
>> Could you please bisect ?
> Unfortunately, no I can't. (No git tree here - just a tarball downloaded via FTP). However, maybe I could out some printk() statements into the code if you could point out where the "hot-spots" might be, please?
>

For the em28xx driver: em28xx-input.c:
em28xx_ir_work() is called every 100ms
     calls em28xx_ir_handle_key()
         - calls ir->get_key() which is em2874_polling_getkey() in case 
of your device
         - reports the detected key via rc_keydown() through the RC core

HTH,
Frank


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-18 16:58     ` Frank Schäfer
@ 2013-05-18 21:02       ` Chris Rankin
  2013-05-19 13:40         ` Frank Schäfer
  2013-05-18 21:11       ` Chris Rankin
  1 sibling, 1 reply; 25+ messages in thread
From: Chris Rankin @ 2013-05-18 21:02 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: linux-media

----- Original Message -----

> For the em28xx driver: em28xx-input.c:
> em28xx_ir_work() is called every 100ms
>    calls em28xx_ir_handle_key()
>        - calls ir->get_key() which is em2874_polling_getkey() in case of your device
>        - reports the detected key via rc_keydown() through the RC core

By the looks of things, it's not recognising the protocol: em2874_ir_change_protocol() is setting ir->rc_type to RC_BIT_UNKNOWN. Shouldn't it be using RC5 instead?

Cheers,
Chris

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-18 16:58     ` Frank Schäfer
  2013-05-18 21:02       ` Chris Rankin
@ 2013-05-18 21:11       ` Chris Rankin
  1 sibling, 0 replies; 25+ messages in thread
From: Chris Rankin @ 2013-05-18 21:11 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: linux-media

----- Original Message -----

> For the em28xx driver: em28xx-input.c:
> em28xx_ir_work() is called every 100ms
>     calls em28xx_ir_handle_key()
>         - calls ir->get_key() which is em2874_polling_getkey() in case of your device
>         - reports the detected key via rc_keydown() through the RC core

Confirmed: em28xx is failing to set the protocol to RC5, which means that em2874_polling_getkey() is using the "default" (UNKNOWN) case to interpret the scan codes.

Forcing em2874_polling_getkey() to use RC5 fixes the remote control for this device.

Cheers,
Chris


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-18 21:02       ` Chris Rankin
@ 2013-05-19 13:40         ` Frank Schäfer
  2013-05-19 14:11           ` Chris Rankin
  0 siblings, 1 reply; 25+ messages in thread
From: Frank Schäfer @ 2013-05-19 13:40 UTC (permalink / raw)
  To: Chris Rankin; +Cc: linux-media

Am 18.05.2013 23:02, schrieb Chris Rankin:
> ----- Original Message -----
>
>> For the em28xx driver: em28xx-input.c:
>> em28xx_ir_work() is called every 100ms
>>      calls em28xx_ir_handle_key()
>>          - calls ir->get_key() which is em2874_polling_getkey() in case of your device
>>          - reports the detected key via rc_keydown() through the RC core
> By the looks of things, it's not recognising the protocol: em2874_ir_change_protocol() is setting ir->rc_type to RC_BIT_UNKNOWN. Shouldn't it be using RC5 instead?
em28xx_ir_change_protocol() should be called at least twice:
First from em28xx_ir_init() with RC_BIT_UNKNOWN (initial configuration) 
and later from the RC core with RC_BIT_RC5.
Can you confirm that ?

Regards,
Frank

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-19 13:40         ` Frank Schäfer
@ 2013-05-19 14:11           ` Chris Rankin
  2013-05-19 17:26             ` Frank Schäfer
  0 siblings, 1 reply; 25+ messages in thread
From: Chris Rankin @ 2013-05-19 14:11 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: linux-media

----- Original Message -----

> em28xx_ir_change_protocol() should be called at least twice:
> First from em28xx_ir_init() with RC_BIT_UNKNOWN (initial configuration) 
> and later from the RC core with RC_BIT_RC5.
> Can you confirm that ?

Yes, it does appear to be called twice: first with *rc_type=1 and then later with *rc_type=8. But that still doesn't seem to stop ir->rc_type being RC_BIT_UNKNOWN in em2874_polling_getkey().

Cheers,
Chris


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-19 14:11           ` Chris Rankin
@ 2013-05-19 17:26             ` Frank Schäfer
  2013-05-19 19:59               ` Chris Rankin
  0 siblings, 1 reply; 25+ messages in thread
From: Frank Schäfer @ 2013-05-19 17:26 UTC (permalink / raw)
  To: Chris Rankin; +Cc: linux-media

Am 19.05.2013 16:11, schrieb Chris Rankin:
> ----- Original Message -----
>
>> em28xx_ir_change_protocol() should be called at least twice:
>> First from em28xx_ir_init() with RC_BIT_UNKNOWN (initial configuration) 
>> and later from the RC core with RC_BIT_RC5.
>> Can you confirm that ?
> Yes, it does appear to be called twice: first with *rc_type=1 and then later with *rc_type=8.

Good, that's how it should be.

>  But that still doesn't seem to stop ir->rc_type being RC_BIT_UNKNOWN in em2874_polling_getkey().

Hmm... that's weird. Are you sure about that ? Is this really a 3.9.2
vanilla kernel ?
The code looks good, ir->rc_type is updated by
em2874_ir_change_protocol() when the protocol is changed.
I also tried to reproduce your problem with a em2884 device with a RC5
remote control a few minutes ago, but everything works as expected...

One thing I noticed is that in em28xx_ir_handle_key() ir->last_readcount
is reset to 0 for the em2874 and em2884.
I assume the same should be done for em28174, too...
Anyway, that's a separate issue and older kernels did the same.

Regards,
Frank

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-19 17:26             ` Frank Schäfer
@ 2013-05-19 19:59               ` Chris Rankin
  2013-05-19 21:02                 ` Frank Schäfer
  0 siblings, 1 reply; 25+ messages in thread
From: Chris Rankin @ 2013-05-19 19:59 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: linux-media

----- Original Message -----

> Hmm... that's weird. Are you sure about that ? Is this really a 3.9.2 vanilla kernel ?

Quite sure, although it turns out that there's a bit more to it. Here is the dmesg output with my debugging messages in:

[ 6263.496794] em28174 #0: Calling em28xx_ir_change_protocol...
[ 6263.533332] em28174 #0: Calling em2874_ir_change_protocol...(type=1)
[ 6263.576099] Registered IR keymap rc-pinnacle-pctv-hd
[ 6263.607181] input: em28xx IR (em28174 #0) as /devices/pci0000:00/0000:00:1d.7/usb10/10-4/rc/rc6/input22
[ 6263.608329] ir-keytable[30882]: segfault at 0 ip 0000000000401cc0 sp 00007fff7ca22dd0 error 4 in ir-keytable[400000+8000]
[ 6263.756019] rc6: em28xx IR (em28174 #0) as /devices/pci0000:00/0000:00:1d.7/usb10/10-4/rc/rc6
[ 6263.816551] em28174 #0: Calling em28xx_ir_change_protocol...
[ 6263.853024] em28174 #0: Calling em2874_ir_change_protocol...(type=8)
[ 6263.895796] Em28xx: Initialized (Em28xx Input Extension) extension

This is the state after I have loaded my em28xx_rc modue. But then I need to call ir-keytable:

# ir-keytable -a /etc/rc_maps.cfg -s rc6

[ 6284.491992] em28174 #0: Calling em28xx_ir_change_protocol...
[ 6284.528492] em28174 #0: Calling em2874_ir_change_protocol...(type=1)

And this seems to reset the protocol back to "unknown". (But I need to use this other remote control to use VDR - the PCTV one just doesn't have enough buttons).

Cheers,
Chris


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-19 19:59               ` Chris Rankin
@ 2013-05-19 21:02                 ` Frank Schäfer
  2013-05-19 22:36                   ` Chris Rankin
                                     ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Frank Schäfer @ 2013-05-19 21:02 UTC (permalink / raw)
  To: Chris Rankin; +Cc: Mauro Carvalho Chehab, linux-media

Am 19.05.2013 21:59, schrieb Chris Rankin:
> ----- Original Message -----
>
>> Hmm... that's weird. Are you sure about that ? Is this really a 3.9.2 vanilla kernel ?
> Quite sure, although it turns out that there's a bit more to it. Here is the dmesg output with my debugging messages in:
>
> [ 6263.496794] em28174 #0: Calling em28xx_ir_change_protocol...
> [ 6263.533332] em28174 #0: Calling em2874_ir_change_protocol...(type=1)
> [ 6263.576099] Registered IR keymap rc-pinnacle-pctv-hd
> [ 6263.607181] input: em28xx IR (em28174 #0) as /devices/pci0000:00/0000:00:1d.7/usb10/10-4/rc/rc6/input22
> [ 6263.608329] ir-keytable[30882]: segfault at 0 ip 0000000000401cc0 sp 00007fff7ca22dd0 error 4 in ir-keytable[400000+8000]
> [ 6263.756019] rc6: em28xx IR (em28174 #0) as /devices/pci0000:00/0000:00:1d.7/usb10/10-4/rc/rc6
> [ 6263.816551] em28174 #0: Calling em28xx_ir_change_protocol...
> [ 6263.853024] em28174 #0: Calling em2874_ir_change_protocol...(type=8)
> [ 6263.895796] Em28xx: Initialized (Em28xx Input Extension) extension
>
> This is the state after I have loaded my em28xx_rc modue. But then I need to call ir-keytable:
>
> # ir-keytable -a /etc/rc_maps.cfg -s rc6
>
> [ 6284.491992] em28174 #0: Calling em28xx_ir_change_protocol...
> [ 6284.528492] em28174 #0: Calling em2874_ir_change_protocol...(type=1)
>
> And this seems to reset the protocol back to "unknown". (But I need to use this other remote control to use VDR - the PCTV one just doesn't have enough buttons).
Ok, then it seems to be no em28xx issue.
What happens with kernel 3.8 ? Does ir-keytable trigger an
em28xx_ir_change_protocol() call there, too, but with type=8 ? Or is
this call missing ?

I'm not familar with ir-keytable and the RC core.
Mauro ? Can you take over ? ;)

Regards,
Frank

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-19 21:02                 ` Frank Schäfer
@ 2013-05-19 22:36                   ` Chris Rankin
  2013-05-19 23:04                   ` Chris Rankin
  2013-05-20  0:45                   ` Chris Rankin
  2 siblings, 0 replies; 25+ messages in thread
From: Chris Rankin @ 2013-05-19 22:36 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Mauro Carvalho Chehab, linux-media

----- Original Message -----

>> And this seems to reset the protocol back to "unknown". (But I need to use this other remote control to use VDR - the PCTV one just doesn't have enough buttons).

> Ok, then it seems to be no em28xx issue.
> What happens with kernel 3.8 ? Does ir-keytable trigger an
> em28xx_ir_change_protocol() call there, too, but with type=8 ? Or is this call missing ?

Possibly the significant difference between 3.8 and 3.9 is that the em2874_polling_getkey() function in 3.8 can only do one thing, whereas in 3.9 its behaviour switches on ir->rc_type.

The 3.9 version of em28xx_ir_change_protocol() also sets ir->rc_type to *rc_type before it exits, which means that the RC framework will "unconfigure" the em28xx remote control if it were to send RC_BIT_UNKNOWN for any reason.

And "yes", the 3.8 kernel does seem to call em28xx_ir_change_protocol() with *rc_type = RC_BIT_UNKNOWN occasionally too. It's just that under 3.8, the em28xx neither noticed nor cared.

Cheers,
Chris

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-19 21:02                 ` Frank Schäfer
  2013-05-19 22:36                   ` Chris Rankin
@ 2013-05-19 23:04                   ` Chris Rankin
  2013-05-20 12:38                     ` Frank Schäfer
  2013-05-20  0:45                   ` Chris Rankin
  2 siblings, 1 reply; 25+ messages in thread
From: Chris Rankin @ 2013-05-19 23:04 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Mauro Carvalho Chehab, linux-media

----- Original Message -----

> What happens with kernel 3.8 ? Does ir-keytable trigger an
> em28xx_ir_change_protocol() call there, too, but with type=8 ? Or is this call missing ?

This is the dmesg output from 3.8, with an extra ex28xx_info() call at the start of em28xx_ir_change_protocol():

[ 2149.668729] Em28xx: Initialized (Em28xx dvb Extension) extension
[ 2149.674447] em28xx #0: Changing protocol: rc_type=1
[ 2149.700087] Registered IR keymap rc-pinnacle-pctv-hd
[ 2149.700444] input: em28xx IR (em28xx #0) as /devices/pci0000:00/0000:00:1d.7/usb5/5-1/rc/rc0/input15
[ 2149.700655] rc0: em28xx IR (em28xx #0) as /devices/pci0000:00/0000:00:1d.7/usb5/5-1/rc/rc0
[ 2149.700660] em28xx #0: Changing protocol: rc_type=8
[ 2149.702337] Em28xx: Initialized (Em28xx Input Extension) extension
[ 2149.704204] em28xx #0: Changing protocol: rc_type=1

And this is me calling ir-keytable:

[ 2183.812407] em28xx #0: Changing protocol: rc_type=1

The point is that 3.8 ignores rc_type=1, whereas 3.9 uses it to update a new ir->rc_type field - which in turn controls how em2874_polling_getkey() encodes its scancode.

Cheers,
Chris


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-19 21:02                 ` Frank Schäfer
  2013-05-19 22:36                   ` Chris Rankin
  2013-05-19 23:04                   ` Chris Rankin
@ 2013-05-20  0:45                   ` Chris Rankin
  2013-05-20 12:40                     ` Frank Schäfer
  2 siblings, 1 reply; 25+ messages in thread
From: Chris Rankin @ 2013-05-20  0:45 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Mauro Carvalho Chehab, linux-media

----- Original Message -----

> I'm not familar with ir-keytable and the RC core.
> Mauro ? Can you take over ? ;)

This patch seems to "do the right thing"... I doubt it will apply cleanly because of TAB/space issues, but you should get the idea :-).

--- linux-3.9/drivers/media/usb/em28xx/em28xx-input.c.orig    2013-05-19 21:18:39.000000000 +0100
+++ linux-3.9/drivers/media/usb/em28xx/em28xx-input.c    2013-05-20 01:36:51.000000000 +0100
@@ -417,6 +417,7 @@
         *rc_type = RC_BIT_RC6_0;
     } else if (*rc_type & RC_BIT_UNKNOWN) {
         *rc_type = RC_BIT_UNKNOWN;
+                return 0;
     } else {
         *rc_type = ir->rc_type;
         return -EINVAL;

This is against 3.9.3.

Signed-off-by: Chris Rankin <rankincj@yahoo.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-19 23:04                   ` Chris Rankin
@ 2013-05-20 12:38                     ` Frank Schäfer
  2013-05-20 12:47                       ` Frank Schäfer
  2013-05-20 13:01                       ` Chris Rankin
  0 siblings, 2 replies; 25+ messages in thread
From: Frank Schäfer @ 2013-05-20 12:38 UTC (permalink / raw)
  To: Chris Rankin; +Cc: Mauro Carvalho Chehab, linux-media

Am 20.05.2013 01:04, schrieb Chris Rankin:
> ----- Original Message -----
>
>> What happens with kernel 3.8 ? Does ir-keytable trigger an
>> em28xx_ir_change_protocol() call there, too, but with type=8 ? Or is this call missing ?
> This is the dmesg output from 3.8, with an extra ex28xx_info() call at the start of em28xx_ir_change_protocol():
>
> [ 2149.668729] Em28xx: Initialized (Em28xx dvb Extension) extension
> [ 2149.674447] em28xx #0: Changing protocol: rc_type=1
> [ 2149.700087] Registered IR keymap rc-pinnacle-pctv-hd
> [ 2149.700444] input: em28xx IR (em28xx #0) as /devices/pci0000:00/0000:00:1d.7/usb5/5-1/rc/rc0/input15
> [ 2149.700655] rc0: em28xx IR (em28xx #0) as /devices/pci0000:00/0000:00:1d.7/usb5/5-1/rc/rc0
> [ 2149.700660] em28xx #0: Changing protocol: rc_type=8
> [ 2149.702337] Em28xx: Initialized (Em28xx Input Extension) extension
> [ 2149.704204] em28xx #0: Changing protocol: rc_type=1
>
> And this is me calling ir-keytable:
>
> [ 2183.812407] em28xx #0: Changing protocol: rc_type=1

So with 3.8 the same happens as with 3.9.

Well, if ir-keycode / the RC core requests RC_BIT_UNKNOWN, they get
RC_BIT_UNKNOWN. ;)
If you expect the device to be configured for another protocol (RC5 ?),
you need to find out what's going wrong in the RC core and/or ir-keycode.

> The point is that 3.8 ignores rc_type=1, whereas 3.9 uses it to update a new ir->rc_type field - which in turn controls how em2874_polling_getkey() encodes its scancode.

Indeed, since 3.9
1.) em2874_polling_getkey() cares about the rc_type
2.) the new rc_type is saved back to ir->rc_type

AFAICS both changes are correct.

But there was a third change:
3.) the scancode passed to the RC core with rc_keypress() in case of
RC_BIT_UNKNOWN changed from a 16 bit value to 32 bit value (e.g.: old:
00 00 ab cd => new: ab cd xx xx).

Hmm... isn't this an ABI break !?

Regards,
Frank


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20  0:45                   ` Chris Rankin
@ 2013-05-20 12:40                     ` Frank Schäfer
  2013-05-20 12:48                       ` Chris Rankin
  0 siblings, 1 reply; 25+ messages in thread
From: Frank Schäfer @ 2013-05-20 12:40 UTC (permalink / raw)
  To: Chris Rankin; +Cc: Mauro Carvalho Chehab, linux-media

Am 20.05.2013 02:45, schrieb Chris Rankin:
> ----- Original Message -----
>
>> I'm not familar with ir-keytable and the RC core.
>> Mauro ? Can you take over ? ;)
> This patch seems to "do the right thing"... I doubt it will apply cleanly because of TAB/space issues, but you should get the idea :-).
>
> --- linux-3.9/drivers/media/usb/em28xx/em28xx-input.c.orig    2013-05-19 21:18:39.000000000 +0100
> +++ linux-3.9/drivers/media/usb/em28xx/em28xx-input.c    2013-05-20 01:36:51.000000000 +0100
> @@ -417,6 +417,7 @@
>          *rc_type = RC_BIT_RC6_0;
>      } else if (*rc_type & RC_BIT_UNKNOWN) {
>          *rc_type = RC_BIT_UNKNOWN;
> +                return 0;
>      } else {
>          *rc_type = ir->rc_type;
>          return -EINVAL;
>
> This is against 3.9.3.
>
> Signed-off-by: Chris Rankin <rankincj@yahoo.com>
No, this patch is wrong.
Updating ir->rc_type with the new value of *rc_type is correct.

Regards,
Frank

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20 12:38                     ` Frank Schäfer
@ 2013-05-20 12:47                       ` Frank Schäfer
  2013-05-20 13:01                       ` Chris Rankin
  1 sibling, 0 replies; 25+ messages in thread
From: Frank Schäfer @ 2013-05-20 12:47 UTC (permalink / raw)
  To: Chris Rankin; +Cc: Mauro Carvalho Chehab, linux-media

Am 20.05.2013 14:38, schrieb Frank Schäfer:
> ...
> But there was a third change:
> 3.) the scancode passed to the RC core with rc_keypress() in case of
> RC_BIT_UNKNOWN changed from a 16 bit value to 32 bit value (e.g.: old:
> 00 00 ab cd => new: ab cd xx xx).

See

commit 105e3687ada4ebe6dfbda7abc3b16106f86a787d
Author: Mauro Carvalho Chehab <mchehab@redhat.com>
Date:   Sat Dec 15 08:29:11 2012 -0300

    [media] em28xx: add support for NEC proto variants on em2874 and upper


default_polling_getkey() (em2860/em2880) still returns the 16 bit scancode.
Whatever scancode is correct, it should be the same in both cases...

Mauro, can you take a look at this ?

Regards,
Frank

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20 12:40                     ` Frank Schäfer
@ 2013-05-20 12:48                       ` Chris Rankin
  0 siblings, 0 replies; 25+ messages in thread
From: Chris Rankin @ 2013-05-20 12:48 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Mauro Carvalho Chehab, linux-media

----- Original Message -----

> This patch seems to "do the right thing"... I doubt it will apply cleanly because of TAB/space issues, but you should get the idea :-).
>
> --- linux-3.9/drivers/media/usb/em28xx/em28xx-input.c.orig    2013-05-19 21:18:39.000000000 +0100
> +++ linux-3.9/drivers/media/usb/em28xx/em28xx-input.c    2013-05-20 01:36:51.000000000 +0100
> @@ -417,6 +417,7 @@
>          *rc_type = RC_BIT_RC6_0;
>      } else if (*rc_type & RC_BIT_UNKNOWN) {
>          *rc_type = RC_BIT_UNKNOWN;
> +                return 0;
>      } else {
>          *rc_type = ir->rc_type;
>          return -EINVAL;
>
> This is against 3.9.3.
>
> Signed-off-by: Chris Rankin <rankincj@yahoo.com>

> No, this patch is wrong.
> Updating ir->rc_type with the new value of *rc_type is correct.

Well, it restores 3.8 behaviour, i.e. em28xx not clobbering its "RC5" configuration when RC core subsequently calls ir_change_protocol() with *rc_type=RC_BIT_UNKNOWN. The ir->rc_type parameter is new to 3.9, by the looks of things.

Cheers,
Chris


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20 12:38                     ` Frank Schäfer
  2013-05-20 12:47                       ` Frank Schäfer
@ 2013-05-20 13:01                       ` Chris Rankin
  2013-05-20 13:43                         ` Frank Schäfer
  2013-06-13 10:50                         ` Mauro Carvalho Chehab
  1 sibling, 2 replies; 25+ messages in thread
From: Chris Rankin @ 2013-05-20 13:01 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Mauro Carvalho Chehab, linux-media

----- Original Message -----

>> And this is me calling ir-keytable:
>>
>> [ 2183.812407] em28xx #0: Changing protocol: rc_type=1

> So with 3.8 the same happens as with 3.9.

Yes, that does appear to be part of the RC core ABI.

> Well, if ir-keycode / the RC core requests RC_BIT_UNKNOWN, they get RC_BIT_UNKNOWN. ;)
> If you expect the device to be configured for another protocol (RC5 ?),
> you need to find out what's going wrong in the RC core and/or ir-keycode.

Are other RCs affected by this? I have difficulty blaming RC core when its behaviour hasn't changed.

> The point is that 3.8 ignores rc_type=1, whereas 3.9 uses it to update a new ir->rc_type field - which in turn controls how em2874_polling_getkey() encodes its scancode.

> Indeed, since 3.9
> 1.) em2874_polling_getkey() cares about the rc_type
> 2.) the new rc_type is saved back to ir->rc_type
> 
> AFAICS both changes are correct.

Except that given the current ABI, these changes break my RC.

> But there was a third change:
> 3.) the scancode passed to the RC core with rc_keypress() in case of
> RC_BIT_UNKNOWN changed from a 16 bit value to 32 bit value (e.g.: old: 00 00 ab cd => new: ab cd xx xx).
>
> Hmm... isn't this an ABI break !?

em28xx only used to support RC5 in 3.8, by the looks of things. The behaviour when configured for "RC_BIT_UNKNOWN" would therefore have been "undefined"... ;-).

Cheers,
Chris

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20 13:01                       ` Chris Rankin
@ 2013-05-20 13:43                         ` Frank Schäfer
  2013-05-20 14:51                           ` Chris Rankin
  2013-06-13 10:50                         ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 25+ messages in thread
From: Frank Schäfer @ 2013-05-20 13:43 UTC (permalink / raw)
  To: Chris Rankin; +Cc: Mauro Carvalho Chehab, linux-media

Am 20.05.2013 15:01, schrieb Chris Rankin:
> ----- Original Message -----
>
>>> And this is me calling ir-keytable:
>>>
>>> [ 2183.812407] em28xx #0: Changing protocol: rc_type=1
>> So with 3.8 the same happens as with 3.9.
> Yes, that does appear to be part of the RC core ABI.
>
>> Well, if ir-keycode / the RC core requests RC_BIT_UNKNOWN, they get RC_BIT_UNKNOWN. ;)
>> If you expect the device to be configured for another protocol (RC5 ?),
>> you need to find out what's going wrong in the RC core and/or ir-keycode.
> Are other RCs affected by this? I have difficulty blaming RC core when its behaviour hasn't changed.

If I had to guess, I would say you should check your rc_maps.cfg /
keytable. ;)

>
>> The point is that 3.8 ignores rc_type=1, whereas 3.9 uses it to update a new ir->rc_type field - which in turn controls how em2874_polling_getkey() encodes its scancode.
>> Indeed, since 3.9
>> 1.) em2874_polling_getkey() cares about the rc_type
>> 2.) the new rc_type is saved back to ir->rc_type
>>
>> AFAICS both changes are correct.
> Except that given the current ABI, these changes break my RC.

No, the change that actually broke your RC is the third change.

>
>> But there was a third change:
>> 3.) the scancode passed to the RC core with rc_keypress() in case of
>> RC_BIT_UNKNOWN changed from a 16 bit value to 32 bit value (e.g.: old: 00 00 ab cd => new: ab cd xx xx).
>>
>> Hmm... isn't this an ABI break !?
> em28xx only used to support RC5 in 3.8, by the looks of things.

... and NEC.

>  The behaviour when configured for "RC_BIT_UNKNOWN" would therefore have been "undefined"... ;-).

With both kernels (3.8 and 3.9), the hardware is configured for RC5 when
RC_BIT_UNKNOWN is selected.
The only thing that changed in the configuration part (apart from the
new ir->rc_type field and RC& support) is,
that em28xx_ir_change_protocol() used to return -EINVAL for
RC_NIT_UNKNOWN and 3.9 now returns 0.
But that seems to be no issue here.

Regards,
Frank

>
> Cheers,
> Chris


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20 13:43                         ` Frank Schäfer
@ 2013-05-20 14:51                           ` Chris Rankin
  2013-05-20 15:36                             ` Frank Schäfer
  0 siblings, 1 reply; 25+ messages in thread
From: Chris Rankin @ 2013-05-20 14:51 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Mauro Carvalho Chehab, linux-media

----- Original Message -----

> If I had to guess, I would say you should check your rc_maps.cfg / keytable. ;)

This is unchanged between 3.8.x and 3.9.x, and so is correct by definition.

Kernel Upgrades Do Not Break Userspace.

Cheers,
Chris


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20 14:51                           ` Chris Rankin
@ 2013-05-20 15:36                             ` Frank Schäfer
  2013-05-20 16:02                               ` Chris Rankin
  2013-06-13 10:39                               ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 25+ messages in thread
From: Frank Schäfer @ 2013-05-20 15:36 UTC (permalink / raw)
  To: Chris Rankin; +Cc: Mauro Carvalho Chehab, Linux Media Mailing List

Am 20.05.2013 16:51, schrieb Chris Rankin:
> ----- Original Message -----
>
>> If I had to guess, I would say you should check your rc_maps.cfg / keytable. ;)
> This is unchanged between 3.8.x and 3.9.x, and so is correct by definition.

No, just because it didn't change it isn't automatically correct. ;)
Which protocol type does you keytable specify/select ?
It should be RC5. If it's none or unknown, it's just dump luck that
things are working (because the driver fortunately configures the device
for RC5 in case of  RC_BIT_UNKNOWN).

> Kernel Upgrades Do Not Break Userspace.

Right.
That's why I would say the third (scancode) change is problematic.
Let's see what Mauro thinks about this.

Regards,
Frank

> Cheers,
> Chris
>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20 15:36                             ` Frank Schäfer
@ 2013-05-20 16:02                               ` Chris Rankin
  2013-06-13 10:39                               ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 25+ messages in thread
From: Chris Rankin @ 2013-05-20 16:02 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Mauro Carvalho Chehab, Linux Media Mailing List

----- Original Message -----

> No, just because it didn't change it isn't automatically correct. ;)

It's Fedora 18's "haupp" keytable - so it must be correct :-).


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20 15:36                             ` Frank Schäfer
  2013-05-20 16:02                               ` Chris Rankin
@ 2013-06-13 10:39                               ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 25+ messages in thread
From: Mauro Carvalho Chehab @ 2013-06-13 10:39 UTC (permalink / raw)
  To: Chris Rankin; +Cc: Frank Schäfer, Linux Media Mailing List

Hi,

Sorry for not getting into it earlier... too much work those days.

Em Mon, 20 May 2013 17:36:46 +0200
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:

> Am 20.05.2013 16:51, schrieb Chris Rankin:
> > ----- Original Message -----
> >
> >> If I had to guess, I would say you should check your rc_maps.cfg / keytable. ;)
> > This is unchanged between 3.8.x and 3.9.x, and so is correct by definition.
> 
> No, just because it didn't change it isn't automatically correct. ;)
> Which protocol type does you keytable specify/select ?
> It should be RC5. If it's none or unknown, it's just dump luck that
> things are working (because the driver fortunately configures the device
> for RC5 in case of  RC_BIT_UNKNOWN).

The RC_BIT_UNKNOWN has 3 usages:
	1) when the protocol is unknown;
	2) on legacy tables, when the full scancode is unknown;
	3) on broken devices where just the command part of the scancode
	   (7 or 8 bits) is known.

In the case of em28xx, only (2) applies.

So, the behavior on em28xx is to support legacy IR tables written using
the legacy input layer, that use to support only 8 bits for keycodes.
So, we don't know the address bits of the keycodes. 

The expected behavior on em28xx when RC_BIT_UNKNOWN is used, is that
the keycode tables would be 8-bits masked, and the same mask should
also be applied to the received keycodes. This is done by the RC core.

That is used only by a very few set of em28xx cards:

$ for i in $(git grep RC_MAP_ $(git grep -l  _UNKNOWN drivers/media/rc/keymaps/)|perl -ne 'print "$1\n" if (m/(RC_MAP_[\w\d\_]+)/)');do git grep $i drivers/media/usb/em28xx/; done
drivers/media/usb/em28xx/em28xx-cards.c:          .ir_codes       = RC_MAP_ATI_TV_WONDER_HD_600,
drivers/media/usb/em28xx/em28xx-input.c:                  rc->map_name = RC_MAP_EM_TERRATEC;
drivers/media/usb/em28xx/em28xx-cards.c:          .ir_codes     = RC_MAP_EVGA_INDTUBE,
drivers/media/usb/em28xx/em28xx-cards.c:          .ir_codes     = RC_MAP_GADMEI_RM008Z,
drivers/media/usb/em28xx/em28xx-cards.c:          .ir_codes     = RC_MAP_KAIOMY,
drivers/media/usb/em28xx/em28xx-input.c:                  rc->map_name = RC_MAP_PINNACLE_GREY;
drivers/media/usb/em28xx/em28xx-cards.c:          .ir_codes       = RC_MAP_TERRATEC_CINERGY_XS,
drivers/media/usb/em28xx/em28xx-cards.c:          .ir_codes     = RC_MAP_TERRATEC_CINERGY_XS,
drivers/media/usb/em28xx/em28xx-input.c:                  rc->map_name = RC_MAP_WINFAST_USBII_DELUXE;
drivers/media/usb/em28xx/em28xx-input.c:                  rc->map_name = RC_MAP_WINFAST_USBII_DELUXE;

Patches are welcome to replace the above scancode tables by one with
the full code, as we want to use RC_BIT_UNKNOWN only on broken hardware
where the scancode is only partially filled.

Btw, the bits that handles the masks are at rc-main.c, at
ir_establish_scancode():

	if (dev->scanmask)
		scancode &= dev->scanmask;

A quick look at em28xx shows that it is not using it anymore, so some
patch likely broke support for RC_BIT_UNKNOWN there.

So, a fix is needed there to fill dev->scanmask if a RC_BIT_UNKNOWN
table is used.

It should be noticed that tables with RC_BIT_UNKNOWN are generally 
not portable among devices, as some of those tables have the command
bits inverted.

> 
> > Kernel Upgrades Do Not Break Userspace.
> 
> Right.
> That's why I would say the third (scancode) change is problematic.
> Let's see what Mauro thinks about this.

For legacy tables, only 16 bits are needed (as all those tables are
for either RC5 or NEC), in the form of <addr><cmd> and dev->scanmask
should be 0xff.

The rationale to not mask the scancode to contain just the <cmd> part
is that the produced debug messages could be used by someone to
identify the address for those broken IR keytables and write a patch
for us fixing it.

Regards,
Mauro

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: 3.9.2 kernel - IR / em28xx_rc broken?
  2013-05-20 13:01                       ` Chris Rankin
  2013-05-20 13:43                         ` Frank Schäfer
@ 2013-06-13 10:50                         ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 25+ messages in thread
From: Mauro Carvalho Chehab @ 2013-06-13 10:50 UTC (permalink / raw)
  To: Chris Rankin; +Cc: Frank Schäfer, linux-media

Em Mon, 20 May 2013 06:01:09 -0700 (PDT)
Chris Rankin <rankincj@yahoo.com> escreveu:

> ----- Original Message -----
> 
> >> And this is me calling ir-keytable:
> >>
> >> [ 2183.812407] em28xx #0: Changing protocol: rc_type=1
> 
> > So with 3.8 the same happens as with 3.9.
> 
> Yes, that does appear to be part of the RC core ABI.
> 
> > Well, if ir-keycode / the RC core requests RC_BIT_UNKNOWN, they get RC_BIT_UNKNOWN. ;)
> > If you expect the device to be configured for another protocol (RC5 ?),
> > you need to find out what's going wrong in the RC core and/or ir-keycode.
> 
> Are other RCs affected by this? I have difficulty blaming RC core when its behaviour hasn't changed.
> 
> > The point is that 3.8 ignores rc_type=1, whereas 3.9 uses it to update a new ir->rc_type field - which in turn controls how em2874_polling_getkey() encodes its scancode.

There's something broken at either the tool or at the table, as
the it shouldn't be passing rc_type=1.

Ah, I know what's happening... thare are actually two hauppauge tables:

$ grep haupp rc_maps.cfg 
# cx8800	*				./keycodes/rc5_hauppauge_new
# *		*				./keycodes/rc5_hauppauge_new
*	rc-hauppauge             hauppauge
# *	*			 haupp                # found in nova-t-usb2.c

The proper one is the "hauppauge" one. The "haupp" table is an
alternative (broken) one for a driver that was not properly
converted to the RC core yet. On that driver, the table is coded
directly inside the driver. It shouldn't be hard to fix it there,
but that requires someone with that hardware, in order to test it,
and convert it to use dvb-usb-v2.

If you're using that table, it will try to set the protocol to unknown,
with is wrong, and have an unpredictable result, as, on some boards,
the em28xx will be working with NEC protocol, while, on others, with
RC5.

You should, instead, use this table:

$ more keytable/rc_keymaps/hauppauge
# table hauppauge, type: RC5
0x1e3b KEY_SELECT
0x1e3d KEY_POWER2
0x1e1c KEY_TV
0x1e18 KEY_VIDEO
0x1e19 KEY_AUDIO
0x1e1a KEY_CAMERA
0x1e1b KEY_EPG
0x1e0c KEY_RADIO
0x1e14 KEY_UP
0x1e15 KEY_DOWN
0x1e16 KEY_LEFT
0x1e17 KEY_RIGHT
0x1e25 KEY_OK
...

There, the protocol there seems to be properly identified as RC5, and
the ir-keytable will use RC5 type when setting it.

Regards,
Mauro

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2013-06-13 10:50 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-18 13:57 3.9.2 kernel - IR / em28xx_rc broken? Chris Rankin
2013-05-18 14:36 ` Frank Schäfer
2013-05-18 15:17   ` Chris Rankin
2013-05-18 16:58     ` Frank Schäfer
2013-05-18 21:02       ` Chris Rankin
2013-05-19 13:40         ` Frank Schäfer
2013-05-19 14:11           ` Chris Rankin
2013-05-19 17:26             ` Frank Schäfer
2013-05-19 19:59               ` Chris Rankin
2013-05-19 21:02                 ` Frank Schäfer
2013-05-19 22:36                   ` Chris Rankin
2013-05-19 23:04                   ` Chris Rankin
2013-05-20 12:38                     ` Frank Schäfer
2013-05-20 12:47                       ` Frank Schäfer
2013-05-20 13:01                       ` Chris Rankin
2013-05-20 13:43                         ` Frank Schäfer
2013-05-20 14:51                           ` Chris Rankin
2013-05-20 15:36                             ` Frank Schäfer
2013-05-20 16:02                               ` Chris Rankin
2013-06-13 10:39                               ` Mauro Carvalho Chehab
2013-06-13 10:50                         ` Mauro Carvalho Chehab
2013-05-20  0:45                   ` Chris Rankin
2013-05-20 12:40                     ` Frank Schäfer
2013-05-20 12:48                       ` Chris Rankin
2013-05-18 21:11       ` Chris Rankin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.