All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] Apple remote support
@ 2010-10-29  3:11 Jarod Wilson
  2010-10-29  3:13 ` [RFC PATCH 1/2] ir-nec-decoder: decode Apple's NEC remote variant Jarod Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 31+ messages in thread
From: Jarod Wilson @ 2010-10-29  3:11 UTC (permalink / raw)
  To: linux-media

I've got one of those tiny little 6-button Apple remotes here, now it can
be decoded in-kernel (tested w/an mceusb transceiver).

Jarod Wilson (2):
      ir-nec-decoder: decode Apple's NEC remote variant
      IR: add Apple remote keymap

 drivers/media/IR/ir-nec-decoder.c       |   10 +++++-
 drivers/media/IR/keymaps/Makefile       |    1 +
 drivers/media/IR/keymaps/rc-nec-apple.c |   51 +++++++++++++++++++++++++++++++
 include/media/rc-map.h                  |    1 +
 4 files changed, 62 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/IR/keymaps/rc-nec-apple.c

-- 
Jarod Wilson
jarod@redhat.com


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

* [RFC PATCH 1/2] ir-nec-decoder: decode Apple's NEC remote variant
  2010-10-29  3:11 [RFC PATCH 0/2] Apple remote support Jarod Wilson
@ 2010-10-29  3:13 ` Jarod Wilson
  2010-10-29 22:15   ` Andy Walls
  2010-10-29  3:13 ` [RFC PATCH 2/2] IR: add Apple remote keymap Jarod Wilson
  2010-10-29  3:15 ` [RFC PATCH 0/2] Apple remote support Jarod Wilson
  2 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-10-29  3:13 UTC (permalink / raw)
  To: linux-media

Apple's remotes use an NEC-like protocol, but without checksumming. See
http://en.wikipedia.org/wiki/Apple_Remote for details. Since they always
send a specific vendor code, check for that, and bypass the checksum
check.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/ir-nec-decoder.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index 70993f7..6dcddd2 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -50,6 +50,7 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	struct nec_dec *data = &ir_dev->raw->nec;
 	u32 scancode;
 	u8 address, not_address, command, not_command;
+	bool apple = false;
 
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_NEC))
 		return 0;
@@ -158,7 +159,14 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 		command	    = bitrev8((data->bits >>  8) & 0xff);
 		not_command = bitrev8((data->bits >>  0) & 0xff);
 
-		if ((command ^ not_command) != 0xff) {
+		/* Apple remotes use an NEC-like proto, but w/o a checksum */
+		if ((address == 0xee) && (not_address == 0x87)) {
+			apple = true;
+			IR_dprintk(1, "Apple remote, ID byte 0x%02x\n",
+				   not_command);
+		}
+
+		if (((command ^ not_command) != 0xff) && !apple) {
 			IR_dprintk(1, "NEC checksum error: received 0x%08x\n",
 				   data->bits);
 			break;
-- 
1.7.1


-- 
Jarod Wilson
jarod@redhat.com


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

* [RFC PATCH 2/2] IR: add Apple remote keymap
  2010-10-29  3:11 [RFC PATCH 0/2] Apple remote support Jarod Wilson
  2010-10-29  3:13 ` [RFC PATCH 1/2] ir-nec-decoder: decode Apple's NEC remote variant Jarod Wilson
@ 2010-10-29  3:13 ` Jarod Wilson
  2010-10-29  3:15 ` [RFC PATCH 0/2] Apple remote support Jarod Wilson
  2 siblings, 0 replies; 31+ messages in thread
From: Jarod Wilson @ 2010-10-29  3:13 UTC (permalink / raw)
  To: linux-media

There are at least two variants, and probably more, of the Apple
remotes. http://en.wikipedia.org/wiki/Apple_Remote contains one set of
codes, while the Apple remote I have in my possession sends a slightly
different set of codes. Just add both to the table. Probably needs
extending for the latest Apple remote, but I don't have one of those.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/keymaps/Makefile       |    1 +
 drivers/media/IR/keymaps/rc-nec-apple.c |   51 +++++++++++++++++++++++++++++++
 include/media/rc-map.h                  |    1 +
 3 files changed, 53 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/IR/keymaps/rc-nec-apple.c

diff --git a/drivers/media/IR/keymaps/Makefile b/drivers/media/IR/keymaps/Makefile
index 3194d39..8fa0e53 100644
--- a/drivers/media/IR/keymaps/Makefile
+++ b/drivers/media/IR/keymaps/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \
 			rc-msi-tvanywhere.o \
 			rc-msi-tvanywhere-plus.o \
 			rc-nebula.o \
+			rc-nec-apple.o \
 			rc-nec-terratec-cinergy-xs.o \
 			rc-norwood.o \
 			rc-npgtech.o \
diff --git a/drivers/media/IR/keymaps/rc-nec-apple.c b/drivers/media/IR/keymaps/rc-nec-apple.c
new file mode 100644
index 0000000..9f6189c
--- /dev/null
+++ b/drivers/media/IR/keymaps/rc-nec-apple.c
@@ -0,0 +1,51 @@
+/* rc-nec-apple.c - Keytable for Apple Remote Controls
+ *
+ * Copyright (c) 2010 by Jarod Wilson <jarod@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <media/rc-map.h>
+
+static struct ir_scancode nec_apple[] = {
+	{ 0xee8702, KEY_MENU},
+	{ 0xee8703, KEY_MENU},
+	{ 0xee8704, KEY_PLAYPAUSE},
+	{ 0xee8705, KEY_PLAYPAUSE},
+	{ 0xee8706, KEY_FORWARD},
+	{ 0xee8707, KEY_FORWARD},
+	{ 0xee8708, KEY_BACK},
+	{ 0xee8709, KEY_BACK},
+	{ 0xee870a, KEY_VOLUMEUP},
+	{ 0xee870b, KEY_VOLUMEUP},
+	{ 0xee870c, KEY_VOLUMEDOWN},
+	{ 0xee870d, KEY_VOLUMEDOWN},
+};
+
+static struct rc_keymap nec_apple_map = {
+	.map = {
+		.scan    = nec_apple,
+		.size    = ARRAY_SIZE(nec_apple),
+		.ir_type = IR_TYPE_NEC,
+		.name    = RC_MAP_NEC_APPLE,
+	}
+};
+
+static int __init init_rc_map_nec_apple(void)
+{
+	return ir_register_map(&nec_apple_map);
+}
+
+static void __exit exit_rc_map_nec_apple(void)
+{
+	ir_unregister_map(&nec_apple_map);
+}
+
+module_init(init_rc_map_nec_apple)
+module_exit(exit_rc_map_nec_apple)
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jarod Wilson <jarod@redhat.com>");
diff --git a/include/media/rc-map.h b/include/media/rc-map.h
index 25883cf..868f0c3 100644
--- a/include/media/rc-map.h
+++ b/include/media/rc-map.h
@@ -110,6 +110,7 @@ void rc_map_init(void);
 #define RC_MAP_MSI_TVANYWHERE_PLUS       "rc-msi-tvanywhere-plus"
 #define RC_MAP_MSI_TVANYWHERE            "rc-msi-tvanywhere"
 #define RC_MAP_NEBULA                    "rc-nebula"
+#define RC_MAP_NEC_APPLE                 "rc-nec-apple"
 #define RC_MAP_NEC_TERRATEC_CINERGY_XS   "rc-nec-terratec-cinergy-xs"
 #define RC_MAP_NORWOOD                   "rc-norwood"
 #define RC_MAP_NPGTECH                   "rc-npgtech"
-- 
1.7.1


-- 
Jarod Wilson
jarod@redhat.com


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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-29  3:11 [RFC PATCH 0/2] Apple remote support Jarod Wilson
  2010-10-29  3:13 ` [RFC PATCH 1/2] ir-nec-decoder: decode Apple's NEC remote variant Jarod Wilson
  2010-10-29  3:13 ` [RFC PATCH 2/2] IR: add Apple remote keymap Jarod Wilson
@ 2010-10-29  3:15 ` Jarod Wilson
  2010-10-29 13:46   ` Mauro Carvalho Chehab
  2 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-10-29  3:15 UTC (permalink / raw)
  To: linux-media

On Thu, Oct 28, 2010 at 11:11:31PM -0400, Jarod Wilson wrote:
> I've got one of those tiny little 6-button Apple remotes here, now it can
> be decoded in-kernel (tested w/an mceusb transceiver).

Oh yeah, RFC, because I'm not sure if we should have a more generic "skip
the checksum check" support -- I seem to recall discussion about it in the
not so recent past. And a decoder hack for one specific remote is just
kinda ugly...

-- 
Jarod Wilson
jarod@redhat.com


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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-29  3:15 ` [RFC PATCH 0/2] Apple remote support Jarod Wilson
@ 2010-10-29 13:46   ` Mauro Carvalho Chehab
  2010-10-29 15:11     ` Jarod Wilson
  0 siblings, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-29 13:46 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: linux-media

Em 29-10-2010 01:15, Jarod Wilson escreveu:
> On Thu, Oct 28, 2010 at 11:11:31PM -0400, Jarod Wilson wrote:
>> I've got one of those tiny little 6-button Apple remotes here, now it can
>> be decoded in-kernel (tested w/an mceusb transceiver).
> 
> Oh yeah, RFC, because I'm not sure if we should have a more generic "skip
> the checksum check" support -- I seem to recall discussion about it in the
> not so recent past. And a decoder hack for one specific remote is just
> kinda ugly...

Yeah, I have the same doubt. One possibility would be to simply report a 32 bits
code, if the check fails. I don't doubt that we'll find other remotes with
a "NEC relaxed" protocol, with no checksum at all.

Cheers,
Mauro.


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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-29 13:46   ` Mauro Carvalho Chehab
@ 2010-10-29 15:11     ` Jarod Wilson
  2010-10-29 19:17       ` David Härdeman
  0 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-10-29 15:11 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media

On Fri, Oct 29, 2010 at 11:46:02AM -0200, Mauro Carvalho Chehab wrote:
> Em 29-10-2010 01:15, Jarod Wilson escreveu:
> > On Thu, Oct 28, 2010 at 11:11:31PM -0400, Jarod Wilson wrote:
> >> I've got one of those tiny little 6-button Apple remotes here, now it can
> >> be decoded in-kernel (tested w/an mceusb transceiver).
> > 
> > Oh yeah, RFC, because I'm not sure if we should have a more generic "skip
> > the checksum check" support -- I seem to recall discussion about it in the
> > not so recent past. And a decoder hack for one specific remote is just
> > kinda ugly...
> 
> Yeah, I have the same doubt. One possibility would be to simply report a 32 bits
> code, if the check fails. I don't doubt that we'll find other remotes with
> a "NEC relaxed" protocol, with no checksum at all.

So the Apple remotes do something funky... One of the four bytes is a
remote identifier byte, which is used for pairing the remote to a specific
device, and you can change the ID byte by simply holding down buttons on
the remote. We could ignore the ID byte, and just match all Apple remotes,
or we could add some sort of pairing support where we require the right ID
byte in order to do scancode -> keycode mapping... But in the match all
case, I think we need the NEC extended scancode (e.g. 0xee8703 for KEY_MENU
on my remote), while in the match paired case, we need the full
4-byte/32-bit code... Offhand, I'm not quite sure how to cleanly handle
both cases. When using lirc, the full 32-bits are used, and you either
have your config with exact matches (remote ID byte included), or you add
an ignore mask, which tells scancode matching to just ignore the ID byte.

-- 
Jarod Wilson
jarod@redhat.com


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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-29 15:11     ` Jarod Wilson
@ 2010-10-29 19:17       ` David Härdeman
  2010-10-29 19:27         ` Jarod Wilson
  0 siblings, 1 reply; 31+ messages in thread
From: David Härdeman @ 2010-10-29 19:17 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Mauro Carvalho Chehab, linux-media

On Fri, Oct 29, 2010 at 11:11:41AM -0400, Jarod Wilson wrote:
> So the Apple remotes do something funky... One of the four bytes is a
> remote identifier byte, which is used for pairing the remote to a specific
> device, and you can change the ID byte by simply holding down buttons on
> the remote.

How many different ID's are possible to set on the remote?

> We could ignore the ID byte, and just match all Apple remotes,
> or we could add some sort of pairing support where we require the right ID
> byte in order to do scancode -> keycode mapping... But in the match all
> case, I think we need the NEC extended scancode (e.g. 0xee8703 for KEY_MENU
> on my remote), while in the match paired case, we need the full
> 4-byte/32-bit code... Offhand, I'm not quite sure how to cleanly handle
> both cases.

If the number of possible ID values is not obscene, you could report the 
full 32 bit scancode and have a keymap with all the different 
variations.

-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-29 19:17       ` David Härdeman
@ 2010-10-29 19:27         ` Jarod Wilson
  2010-10-29 19:59           ` David Härdeman
  0 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-10-29 19:27 UTC (permalink / raw)
  To: David Härdeman; +Cc: Mauro Carvalho Chehab, linux-media

On Fri, Oct 29, 2010 at 09:17:11PM +0200, David Härdeman wrote:
> On Fri, Oct 29, 2010 at 11:11:41AM -0400, Jarod Wilson wrote:
> > So the Apple remotes do something funky... One of the four bytes is a
> > remote identifier byte, which is used for pairing the remote to a specific
> > device, and you can change the ID byte by simply holding down buttons on
> > the remote.
> 
> How many different ID's are possible to set on the remote?

256, apparently.

> > We could ignore the ID byte, and just match all Apple remotes,
> > or we could add some sort of pairing support where we require the right ID
> > byte in order to do scancode -> keycode mapping... But in the match all
> > case, I think we need the NEC extended scancode (e.g. 0xee8703 for KEY_MENU
> > on my remote), while in the match paired case, we need the full
> > 4-byte/32-bit code... Offhand, I'm not quite sure how to cleanly handle
> > both cases.
> 
> If the number of possible ID values is not obscene, you could report the 
> full 32 bit scancode and have a keymap with all the different 
> variations.

I'm thinking 256 * 6 is probably a bit obscene, but I guess that way, you
could work with all remotes if desired, and to "pair", just remove all but
the 6 for your remote... I'll keep giving it some thought, I feel like we
ought to have a cleaner way to do this.

-- 
Jarod Wilson
jarod@redhat.com


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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-29 19:27         ` Jarod Wilson
@ 2010-10-29 19:59           ` David Härdeman
  2010-10-29 20:09             ` Jarod Wilson
  0 siblings, 1 reply; 31+ messages in thread
From: David Härdeman @ 2010-10-29 19:59 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Mauro Carvalho Chehab, linux-media

On Fri, Oct 29, 2010 at 03:27:33PM -0400, Jarod Wilson wrote:
> On Fri, Oct 29, 2010 at 09:17:11PM +0200, David Härdeman wrote:
> > On Fri, Oct 29, 2010 at 11:11:41AM -0400, Jarod Wilson wrote:
> > > So the Apple remotes do something funky... One of the four bytes is a
> > > remote identifier byte, which is used for pairing the remote to a specific
> > > device, and you can change the ID byte by simply holding down buttons on
> > > the remote.
> > 
> > How many different ID's are possible to set on the remote?
> 
> 256, apparently.

Does the remote pick one for you at random?

-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-29 19:59           ` David Härdeman
@ 2010-10-29 20:09             ` Jarod Wilson
  2010-10-30 23:36               ` David Härdeman
  0 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-10-29 20:09 UTC (permalink / raw)
  To: David Härdeman; +Cc: Mauro Carvalho Chehab, linux-media

On Fri, Oct 29, 2010 at 09:59:18PM +0200, David Härdeman wrote:
> On Fri, Oct 29, 2010 at 03:27:33PM -0400, Jarod Wilson wrote:
> > On Fri, Oct 29, 2010 at 09:17:11PM +0200, David Härdeman wrote:
> > > On Fri, Oct 29, 2010 at 11:11:41AM -0400, Jarod Wilson wrote:
> > > > So the Apple remotes do something funky... One of the four bytes is a
> > > > remote identifier byte, which is used for pairing the remote to a specific
> > > > device, and you can change the ID byte by simply holding down buttons on
> > > > the remote.
> > > 
> > > How many different ID's are possible to set on the remote?
> > 
> > 256, apparently.
> 
> Does the remote pick one for you at random?

Looks like its randomly set at the factory, then holding a particular key
combo on the remote for 5 seconds, you can cycle to another one. Not sure
if "another one" means "increment by one" or "randomly pick another one"
yet though.

-- 
Jarod Wilson
jarod@redhat.com


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

* Re: [RFC PATCH 1/2] ir-nec-decoder: decode Apple's NEC remote variant
  2010-10-29  3:13 ` [RFC PATCH 1/2] ir-nec-decoder: decode Apple's NEC remote variant Jarod Wilson
@ 2010-10-29 22:15   ` Andy Walls
  0 siblings, 0 replies; 31+ messages in thread
From: Andy Walls @ 2010-10-29 22:15 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: linux-media

On Thu, 2010-10-28 at 23:13 -0400, Jarod Wilson wrote:
> Apple's remotes use an NEC-like protocol, but without checksumming. See
> http://en.wikipedia.org/wiki/Apple_Remote for details. Since they always
> send a specific vendor code, check for that, and bypass the checksum
> check.

Jarrod,

This is kind of icky.

According to the Wikipedia article, the Apple remote is use the NEC
protocol's physical (PHY) layer, but it's using a different datalink
(DL) layer.

I say the data link layers are different, because the target device
address bytes are in different places and the command payload differs:

NEC bytes are:   Addr Addr' Command Command'
Apple bytes are: 0xee 0x87  Command ID


In NEC "Addr" is used to address a particular device (TV model, VCR
model, etc.) which in the Apple protocol seems to be the function of ID.

Maybe the Apple remote protocol just needs it's own decoder.  That way
the Apple rc-map can just ignore the ID bytes (or maybe make it a user
configurable option to ignore).

Or, if you really want to work, split up PHY vs DL layer in the ir
protocol decoders. ;) 

Regards,
Andy



> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
>  drivers/media/IR/ir-nec-decoder.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
> index 70993f7..6dcddd2 100644
> --- a/drivers/media/IR/ir-nec-decoder.c
> +++ b/drivers/media/IR/ir-nec-decoder.c
> @@ -50,6 +50,7 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
>  	struct nec_dec *data = &ir_dev->raw->nec;
>  	u32 scancode;
>  	u8 address, not_address, command, not_command;
> +	bool apple = false;
>  
>  	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_NEC))
>  		return 0;
> @@ -158,7 +159,14 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
>  		command	    = bitrev8((data->bits >>  8) & 0xff);
>  		not_command = bitrev8((data->bits >>  0) & 0xff);
>  
> -		if ((command ^ not_command) != 0xff) {
> +		/* Apple remotes use an NEC-like proto, but w/o a checksum */
> +		if ((address == 0xee) && (not_address == 0x87)) {
> +			apple = true;
> +			IR_dprintk(1, "Apple remote, ID byte 0x%02x\n",
> +				   not_command);
> +		}
> +
> +		if (((command ^ not_command) != 0xff) && !apple) {
>  			IR_dprintk(1, "NEC checksum error: received 0x%08x\n",
>  				   data->bits);
>  			break;
> -- 
> 1.7.1
> 
> 



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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-29 20:09             ` Jarod Wilson
@ 2010-10-30 23:36               ` David Härdeman
  2010-10-31  2:32                 ` Jarod Wilson
  0 siblings, 1 reply; 31+ messages in thread
From: David Härdeman @ 2010-10-30 23:36 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Mauro Carvalho Chehab, linux-media

On Fri, Oct 29, 2010 at 04:09:37PM -0400, Jarod Wilson wrote:
> On Fri, Oct 29, 2010 at 09:59:18PM +0200, David Härdeman wrote:
> > On Fri, Oct 29, 2010 at 03:27:33PM -0400, Jarod Wilson wrote:
> > > On Fri, Oct 29, 2010 at 09:17:11PM +0200, David Härdeman wrote:
> > > > On Fri, Oct 29, 2010 at 11:11:41AM -0400, Jarod Wilson wrote:
> > > > > So the Apple remotes do something funky... One of the four bytes is a
> > > > > remote identifier byte, which is used for pairing the remote to a specific
> > > > > device, and you can change the ID byte by simply holding down buttons on
> > > > > the remote.
> > > > 
> > > > How many different ID's are possible to set on the remote?
> > > 
> > > 256, apparently.
> > 
> > Does the remote pick one for you at random?
> 
> Looks like its randomly set at the factory, then holding a particular key
> combo on the remote for 5 seconds, you can cycle to another one. Not sure
> if "another one" means "increment by one" or "randomly pick another one"
> yet though.

In that case, one solution would be:

* using the full 32 bit scancode
* add a module parameter to squash the ID byte to zero
* default the module parameter to true
* create a keymap suitable for ID = 0x00

Users who really want to distinguish remotes can then change the module 
parameter and generate a keymap for their particular ID. Most others 
will be blissfully unaware of this feature.

-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-30 23:36               ` David Härdeman
@ 2010-10-31  2:32                 ` Jarod Wilson
  2010-11-01 21:56                   ` David Härdeman
  0 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-10-31  2:32 UTC (permalink / raw)
  To: David Härdeman; +Cc: Jarod Wilson, Mauro Carvalho Chehab, linux-media

On Sat, Oct 30, 2010 at 7:36 PM, David Härdeman <david@hardeman.nu> wrote:
> On Fri, Oct 29, 2010 at 04:09:37PM -0400, Jarod Wilson wrote:
>> On Fri, Oct 29, 2010 at 09:59:18PM +0200, David Härdeman wrote:
>> > On Fri, Oct 29, 2010 at 03:27:33PM -0400, Jarod Wilson wrote:
>> > > On Fri, Oct 29, 2010 at 09:17:11PM +0200, David Härdeman wrote:
>> > > > On Fri, Oct 29, 2010 at 11:11:41AM -0400, Jarod Wilson wrote:
>> > > > > So the Apple remotes do something funky... One of the four bytes is a
>> > > > > remote identifier byte, which is used for pairing the remote to a specific
>> > > > > device, and you can change the ID byte by simply holding down buttons on
>> > > > > the remote.
>> > > >
>> > > > How many different ID's are possible to set on the remote?
>> > >
>> > > 256, apparently.
>> >
>> > Does the remote pick one for you at random?
>>
>> Looks like its randomly set at the factory, then holding a particular key
>> combo on the remote for 5 seconds, you can cycle to another one. Not sure
>> if "another one" means "increment by one" or "randomly pick another one"
>> yet though.
>
> In that case, one solution would be:
>
> * using the full 32 bit scancode
> * add a module parameter to squash the ID byte to zero
> * default the module parameter to true
> * create a keymap suitable for ID = 0x00
>
> Users who really want to distinguish remotes can then change the module
> parameter and generate a keymap for their particular ID. Most others
> will be blissfully unaware of this feature.

I was thinking something similar but slightly different. I think ID =
0x00 is a valid ID byte, so I was thinking static int pair_id = -1; to
start out. This would be a stand-alone apple-only decoder, so we'd
look for the apple identifier bytes, bail if not found. We'd also look
at the ID byte, and if pair_id is 0-255, we'd bail if the ID byte
didn't match it. The scancode we'd actually use to match the key table
would be just the one command byte. It seems to make sense in my head,
at least.

-- 
Jarod Wilson
jarod@wilsonet.com

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-10-31  2:32                 ` Jarod Wilson
@ 2010-11-01 21:56                   ` David Härdeman
  2010-11-02 20:42                     ` Jarod Wilson
  0 siblings, 1 reply; 31+ messages in thread
From: David Härdeman @ 2010-11-01 21:56 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Jarod Wilson, Mauro Carvalho Chehab, linux-media

On Sat, Oct 30, 2010 at 10:32:14PM -0400, Jarod Wilson wrote:
> On Sat, Oct 30, 2010 at 7:36 PM, David Härdeman <david@hardeman.nu> wrote:
> > In that case, one solution would be:
> >
> > * using the full 32 bit scancode
> > * add a module parameter to squash the ID byte to zero
> > * default the module parameter to true
> > * create a keymap suitable for ID = 0x00
> >
> > Users who really want to distinguish remotes can then change the module
> > parameter and generate a keymap for their particular ID. Most others
> > will be blissfully unaware of this feature.
> 
> I was thinking something similar but slightly different. I think ID =
> 0x00 is a valid ID byte, so I was thinking static int pair_id = -1; to
> start out. This would be a stand-alone apple-only decoder, so we'd
> look for the apple identifier bytes, bail if not found. We'd also look
> at the ID byte, and if pair_id is 0-255, we'd bail if the ID byte
> didn't match it. The scancode we'd actually use to match the key table
> would be just the one command byte. It seems to make sense in my head,
> at least.

But you'd lose the ability to support two different remotes with 
different ID's (if you want different mappings in the keymap).

-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-01 21:56                   ` David Härdeman
@ 2010-11-02 20:42                     ` Jarod Wilson
  2010-11-04 12:16                       ` David Härdeman
  0 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-11-02 20:42 UTC (permalink / raw)
  To: David Härdeman; +Cc: Jarod Wilson, Mauro Carvalho Chehab, linux-media

On Mon, Nov 1, 2010 at 5:56 PM, David Härdeman <david@hardeman.nu> wrote:
> On Sat, Oct 30, 2010 at 10:32:14PM -0400, Jarod Wilson wrote:
>> On Sat, Oct 30, 2010 at 7:36 PM, David Härdeman <david@hardeman.nu> wrote:
>> > In that case, one solution would be:
>> >
>> > * using the full 32 bit scancode
>> > * add a module parameter to squash the ID byte to zero
>> > * default the module parameter to true
>> > * create a keymap suitable for ID = 0x00
>> >
>> > Users who really want to distinguish remotes can then change the module
>> > parameter and generate a keymap for their particular ID. Most others
>> > will be blissfully unaware of this feature.
>>
>> I was thinking something similar but slightly different. I think ID =
>> 0x00 is a valid ID byte, so I was thinking static int pair_id = -1; to
>> start out. This would be a stand-alone apple-only decoder, so we'd
>> look for the apple identifier bytes, bail if not found. We'd also look
>> at the ID byte, and if pair_id is 0-255, we'd bail if the ID byte
>> didn't match it. The scancode we'd actually use to match the key table
>> would be just the one command byte. It seems to make sense in my head,
>> at least.
>
> But you'd lose the ability to support two different remotes with
> different ID's (if you want different mappings in the keymap).

Hm, true. How likely are people to want to do that, I wonder?

So alternatively, rather than a pair_id param, could use a
check_pair_byte param. If 0, then just & 0xff the pair byte, and have
0xff there in the default keymap (using all 32 bits for each code) or
just don't make it part of the scancode and use 24 bits. If
check_pair_byte = 1, pass the pair byte along unmodified, using all 32
bits for the scancode. Would also require the user to add the
remote-specific 32-bit codes though. But we're moving towards keymaps
being uploaded from userspace anyway, so perhaps that's not a big
deal. Would rather avoid the default keymap having 256 entries for
every key for every pair id variant.

-- 
Jarod Wilson
jarod@wilsonet.com

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-02 20:42                     ` Jarod Wilson
@ 2010-11-04 12:16                       ` David Härdeman
  2010-11-04 15:54                         ` Jarod Wilson
  0 siblings, 1 reply; 31+ messages in thread
From: David Härdeman @ 2010-11-04 12:16 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Jarod Wilson, Mauro Carvalho Chehab, linux-media

On Tue, 2 Nov 2010 16:42:05 -0400, Jarod Wilson <jarod@wilsonet.com>
wrote:
> On Mon, Nov 1, 2010 at 5:56 PM, David Härdeman <david@hardeman.nu>
wrote:
>> On Sat, Oct 30, 2010 at 10:32:14PM -0400, Jarod Wilson wrote:
>>> On Sat, Oct 30, 2010 at 7:36 PM, David Härdeman <david@hardeman.nu>
>>> wrote:
>>> > In that case, one solution would be:
>>> >
>>> > * using the full 32 bit scancode
>>> > * add a module parameter to squash the ID byte to zero
>>> > * default the module parameter to true
>>> > * create a keymap suitable for ID = 0x00
>>> >
>>> > Users who really want to distinguish remotes can then change the
>>> > module
>>> > parameter and generate a keymap for their particular ID. Most others
>>> > will be blissfully unaware of this feature.
>>>
>>> I was thinking something similar but slightly different. I think ID =
>>> 0x00 is a valid ID byte, so I was thinking static int pair_id = -1; to
>>> start out. This would be a stand-alone apple-only decoder, so we'd
>>> look for the apple identifier bytes, bail if not found. We'd also look
>>> at the ID byte, and if pair_id is 0-255, we'd bail if the ID byte
>>> didn't match it. The scancode we'd actually use to match the key table
>>> would be just the one command byte. It seems to make sense in my head,
>>> at least.
>>
>> But you'd lose the ability to support two different remotes with
>> different ID's (if you want different mappings in the keymap).
> 
> Hm, true. How likely are people to want to do that, I wonder?
> 
> So alternatively, rather than a pair_id param, could use a
> check_pair_byte param. If 0, then just & 0xff the pair byte, and have
> 0xff there in the default keymap (using all 32 bits for each code)

Yes, that's what I proposed above :) (just & 0x00 the pair byte)

-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-04 12:16                       ` David Härdeman
@ 2010-11-04 15:54                         ` Jarod Wilson
  2010-11-04 19:38                           ` David Härdeman
  0 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-11-04 15:54 UTC (permalink / raw)
  To: David Härdeman; +Cc: Jarod Wilson, Mauro Carvalho Chehab, linux-media

On Thu, Nov 4, 2010 at 8:16 AM, David Härdeman <david@hardeman.nu> wrote:
> On Tue, 2 Nov 2010 16:42:05 -0400, Jarod Wilson <jarod@wilsonet.com>
> wrote:
>> On Mon, Nov 1, 2010 at 5:56 PM, David Härdeman <david@hardeman.nu>
> wrote:
>>> On Sat, Oct 30, 2010 at 10:32:14PM -0400, Jarod Wilson wrote:
>>>> On Sat, Oct 30, 2010 at 7:36 PM, David Härdeman <david@hardeman.nu>
>>>> wrote:
>>>> > In that case, one solution would be:
>>>> >
>>>> > * using the full 32 bit scancode
>>>> > * add a module parameter to squash the ID byte to zero
>>>> > * default the module parameter to true
>>>> > * create a keymap suitable for ID = 0x00
>>>> >
>>>> > Users who really want to distinguish remotes can then change the
>>>> > module
>>>> > parameter and generate a keymap for their particular ID. Most others
>>>> > will be blissfully unaware of this feature.
>>>>
>>>> I was thinking something similar but slightly different. I think ID =
>>>> 0x00 is a valid ID byte, so I was thinking static int pair_id = -1; to
>>>> start out. This would be a stand-alone apple-only decoder, so we'd
>>>> look for the apple identifier bytes, bail if not found. We'd also look
>>>> at the ID byte, and if pair_id is 0-255, we'd bail if the ID byte
>>>> didn't match it. The scancode we'd actually use to match the key table
>>>> would be just the one command byte. It seems to make sense in my head,
>>>> at least.
>>>
>>> But you'd lose the ability to support two different remotes with
>>> different ID's (if you want different mappings in the keymap).
>>
>> Hm, true. How likely are people to want to do that, I wonder?
>>
>> So alternatively, rather than a pair_id param, could use a
>> check_pair_byte param. If 0, then just & 0xff the pair byte, and have
>> 0xff there in the default keymap (using all 32 bits for each code)
>
> Yes, that's what I proposed above :) (just & 0x00 the pair byte)

Gah. I fail at reading. :)

Okay, so we seem to be in agreement for an approach to handling this.
I'll toss something together implementing that RSN... Though I talked
with Mauro about this a bit yesterday here at LPC, and we're thinking
maybe we slide this support back over into the nec decoder and make it
a slightly more generic "use full 32 bits" NEC variant we look for
and/or enable/disable somehow. I've got another remote here, for a
Motorola cable box, which is NEC-ish, but always fails decode w/a
checksum error ("got 0x00000000", iirc), which may also need to use
the full 32 bits somehow... Probably a very important protocol variant
to support, particularly once we have native transmit support, as its
used by plenty of cable boxes on the major carriers here in the US.

-- 
Jarod Wilson
jarod@wilsonet.com

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-04 15:54                         ` Jarod Wilson
@ 2010-11-04 19:38                           ` David Härdeman
  2010-11-04 19:43                             ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 31+ messages in thread
From: David Härdeman @ 2010-11-04 19:38 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Jarod Wilson, Mauro Carvalho Chehab, linux-media

On Thu, Nov 04, 2010 at 11:54:25AM -0400, Jarod Wilson wrote:
>Okay, so we seem to be in agreement for an approach to handling this.
>I'll toss something together implementing that RSN... Though I talked
>with Mauro about this a bit yesterday here at LPC, and we're thinking
>maybe we slide this support back over into the nec decoder and make it
>a slightly more generic "use full 32 bits" NEC variant we look for
>and/or enable/disable somehow. I've got another remote here, for a
>Motorola cable box, which is NEC-ish, but always fails decode w/a
>checksum error ("got 0x00000000", iirc), which may also need to use
>the full 32 bits somehow... Probably a very important protocol variant
>to support, particularly once we have native transmit support, as its
>used by plenty of cable boxes on the major carriers here in the US.

I've always found the "checksum" tests in the NEC decoder to be 
unnecessary so I'm all for using a 32 bit scancode in all cases (and 
still using a module param to squash the ID byte of apple remotes, 
defaulting to "yes").

-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-04 19:38                           ` David Härdeman
@ 2010-11-04 19:43                             ` Mauro Carvalho Chehab
  2010-11-05 13:27                               ` David Härdeman
  0 siblings, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2010-11-04 19:43 UTC (permalink / raw)
  To: David Härdeman; +Cc: Jarod Wilson, Jarod Wilson, linux-media

Em 04-11-2010 15:38, David Härdeman escreveu:
> On Thu, Nov 04, 2010 at 11:54:25AM -0400, Jarod Wilson wrote:
>> Okay, so we seem to be in agreement for an approach to handling this.
>> I'll toss something together implementing that RSN... Though I talked
>> with Mauro about this a bit yesterday here at LPC, and we're thinking
>> maybe we slide this support back over into the nec decoder and make it
>> a slightly more generic "use full 32 bits" NEC variant we look for
>> and/or enable/disable somehow. I've got another remote here, for a
>> Motorola cable box, which is NEC-ish, but always fails decode w/a
>> checksum error ("got 0x00000000", iirc), which may also need to use
>> the full 32 bits somehow... Probably a very important protocol variant
>> to support, particularly once we have native transmit support, as its
>> used by plenty of cable boxes on the major carriers here in the US.
> 
> I've always found the "checksum" tests in the NEC decoder to be unnecessary so I'm all for using a 32 bit scancode in all cases (and still using a module param to squash the ID byte of apple remotes, defaulting to "yes").
> 
This means changing all existing NEC tables to have 32 bits, and add
the "redundant" information on all of them. It doesn't seem a good idea
to me to add a penalty for those NEC tables that follow the standard.

Mauro

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-04 19:43                             ` Mauro Carvalho Chehab
@ 2010-11-05 13:27                               ` David Härdeman
  2010-11-05 14:04                                 ` Christopher Harrington
  2010-11-15  4:11                                 ` Jarod Wilson
  0 siblings, 2 replies; 31+ messages in thread
From: David Härdeman @ 2010-11-05 13:27 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Jarod Wilson, Jarod Wilson, linux-media

On Thu, 04 Nov 2010 15:43:33 -0400, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 04-11-2010 15:38, David Härdeman escreveu:
>> On Thu, Nov 04, 2010 at 11:54:25AM -0400, Jarod Wilson wrote:
>>> Okay, so we seem to be in agreement for an approach to handling this.
>>> I'll toss something together implementing that RSN... Though I talked
>>> with Mauro about this a bit yesterday here at LPC, and we're thinking
>>> maybe we slide this support back over into the nec decoder and make it
>>> a slightly more generic "use full 32 bits" NEC variant we look for
>>> and/or enable/disable somehow. I've got another remote here, for a
>>> Motorola cable box, which is NEC-ish, but always fails decode w/a
>>> checksum error ("got 0x00000000", iirc), which may also need to use
>>> the full 32 bits somehow... Probably a very important protocol variant
>>> to support, particularly once we have native transmit support, as its
>>> used by plenty of cable boxes on the major carriers here in the US.
>> 
>> I've always found the "checksum" tests in the NEC decoder to be
>> unnecessary so I'm all for using a 32 bit scancode in all cases (and
>> still using a module param to squash the ID byte of apple remotes,
>> defaulting to "yes").
>> 
> This means changing all existing NEC tables to have 32 bits, and add
> the "redundant" information on all of them.

Yep (though we should use macros to generate scancodes)

> It doesn't seem a good idea
> to me to add a penalty for those NEC tables that follow the standard.

Which penalty?

Using a 32 bit scancode won't affect keytable size or lookup speed.

In some corner cases, additional keytable lookups will be performed for
decoded scancodes which would otherwise be deemed "invalid", but at the
time that decision can be made, most of the processing (reading timing
events from hardware, handing them to decoders, decoding them) has already
been done.

If you're referring to the pain caused by changing existing keytables
(thereby breaking custom keytables), I think it's inevitable. Throwing away
information is not a good solution.

As this subsystem progresses, there's going to be more and more reports of
remotes which, intentionally or unintentionally, do not follow the NEC
"standard" (I use that word in the most liberal sense). Using the full 32
bits allows us to support them without any module parameters or code
changes.

Which solution do you suggest?

-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-05 13:27                               ` David Härdeman
@ 2010-11-05 14:04                                 ` Christopher Harrington
  2010-11-07 19:01                                   ` Jarod Wilson
  2010-11-15  4:11                                 ` Jarod Wilson
  1 sibling, 1 reply; 31+ messages in thread
From: Christopher Harrington @ 2010-11-05 14:04 UTC (permalink / raw)
  To: David Härdeman; +Cc: linux-media

On Fri, Nov 5, 2010 at 08:27, David Härdeman <david@hardeman.nu> wrote:
> If you're referring to the pain caused by changing existing keytables
> (thereby breaking custom keytables), I think it's inevitable. Throwing away
> information is not a good solution.
>
> As this subsystem progresses, there's going to be more and more reports of
> remotes which, intentionally or unintentionally, do not follow the NEC
> "standard" (I use that word in the most liberal sense). Using the full 32
> bits allows us to support them without any module parameters or code
> changes.
>
> Which solution do you suggest?

What about reporting two key events: One with the Apple ID masked, and
one with the Apple ID retained? The "default" keymap could then match
the masked IDs, with the option of the user changing their keymaps to
ignore the masked one and only "pair" with the specific IDs they're
looking for?

As a bonus, they will be able to watch incoming scancodes to see the
ID they might want to pair with, even though those scancodes would be
ignored.

-- 
-Chris Harrington
Phone: 612.598.3650

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-05 14:04                                 ` Christopher Harrington
@ 2010-11-07 19:01                                   ` Jarod Wilson
  0 siblings, 0 replies; 31+ messages in thread
From: Jarod Wilson @ 2010-11-07 19:01 UTC (permalink / raw)
  To: Christopher Harrington; +Cc: David Härdeman, linux-media

On Nov 5, 2010, at 10:04 AM, Christopher Harrington wrote:

> On Fri, Nov 5, 2010 at 08:27, David Härdeman <david@hardeman.nu> wrote:
>> If you're referring to the pain caused by changing existing keytables
>> (thereby breaking custom keytables), I think it's inevitable. Throwing away
>> information is not a good solution.
>> 
>> As this subsystem progresses, there's going to be more and more reports of
>> remotes which, intentionally or unintentionally, do not follow the NEC
>> "standard" (I use that word in the most liberal sense). Using the full 32
>> bits allows us to support them without any module parameters or code
>> changes.
>> 
>> Which solution do you suggest?
> 
> What about reporting two key events: One with the Apple ID masked, and
> one with the Apple ID retained? The "default" keymap could then match
> the masked IDs, with the option of the user changing their keymaps to
> ignore the masked one and only "pair" with the specific IDs they're
> looking for?
> 
> As a bonus, they will be able to watch incoming scancodes to see the
> ID they might want to pair with, even though those scancodes would be
> ignored.

Interesting idea. But I think that 99% of the time, sending two scancodes is unnecessary overhead. I've got some reworked code I still need to test out which simply spits out the Apple remote's pair ID into dmesg w/core ir debugging enabled. If we're not pairing, its masked out in the full 32-bit scancode sent along, but still available via dmesg. If we are pairing, the pair byte is included in the full 32-bit scancode sent along.

-- 
Jarod Wilson
jarod@wilsonet.com




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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-05 13:27                               ` David Härdeman
  2010-11-05 14:04                                 ` Christopher Harrington
@ 2010-11-15  4:11                                 ` Jarod Wilson
  2010-11-15 18:39                                   ` David Härdeman
  2010-11-16 12:08                                   ` Mauro Carvalho Chehab
  1 sibling, 2 replies; 31+ messages in thread
From: Jarod Wilson @ 2010-11-15  4:11 UTC (permalink / raw)
  To: David Härdeman; +Cc: Mauro Carvalho Chehab, Jarod Wilson, linux-media

On Fri, Nov 5, 2010 at 9:27 AM, David Härdeman <david@hardeman.nu> wrote:
> On Thu, 04 Nov 2010 15:43:33 -0400, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Em 04-11-2010 15:38, David Härdeman escreveu:
>>> On Thu, Nov 04, 2010 at 11:54:25AM -0400, Jarod Wilson wrote:
>>>> Okay, so we seem to be in agreement for an approach to handling this.
>>>> I'll toss something together implementing that RSN... Though I talked
>>>> with Mauro about this a bit yesterday here at LPC, and we're thinking
>>>> maybe we slide this support back over into the nec decoder and make it
>>>> a slightly more generic "use full 32 bits" NEC variant we look for
>>>> and/or enable/disable somehow. I've got another remote here, for a
>>>> Motorola cable box, which is NEC-ish, but always fails decode w/a
>>>> checksum error ("got 0x00000000", iirc), which may also need to use
>>>> the full 32 bits somehow... Probably a very important protocol variant
>>>> to support, particularly once we have native transmit support, as its
>>>> used by plenty of cable boxes on the major carriers here in the US.
>>>
>>> I've always found the "checksum" tests in the NEC decoder to be
>>> unnecessary so I'm all for using a 32 bit scancode in all cases (and
>>> still using a module param to squash the ID byte of apple remotes,
>>> defaulting to "yes").
>>>
>> This means changing all existing NEC tables to have 32 bits, and add
>> the "redundant" information on all of them.
>
> Yep (though we should use macros to generate scancodes)
>
>> It doesn't seem a good idea
>> to me to add a penalty for those NEC tables that follow the standard.
>
> Which penalty?
>
> Using a 32 bit scancode won't affect keytable size or lookup speed.
>
> In some corner cases, additional keytable lookups will be performed for
> decoded scancodes which would otherwise be deemed "invalid", but at the
> time that decision can be made, most of the processing (reading timing
> events from hardware, handing them to decoders, decoding them) has already
> been done.
>
> If you're referring to the pain caused by changing existing keytables
> (thereby breaking custom keytables), I think it's inevitable. Throwing away
> information is not a good solution.
>
> As this subsystem progresses, there's going to be more and more reports of
> remotes which, intentionally or unintentionally, do not follow the NEC
> "standard" (I use that word in the most liberal sense). Using the full 32
> bits allows us to support them without any module parameters or code
> changes.
>
> Which solution do you suggest?

Well, here's what I sent along on Friday:

https://patchwork.kernel.org/patch/321592/

Gives us support for using the full 32-bit codes right now w/o having
to change any tables yet, but does require a modparam to skip the
checksum checks, unless its an apple remote which we already know the
vendor bytes for. I do think I'm ultimately leaning towards just doing
the full 32 bits for all nec extended though -- optionally, we might
include a modparam to *enable* the checksum check for those that want
strict compliance (but I'd still say use the full 32 bits). The only
issue I see with going to the full 32 right now is that we have all
these nec tables with only 24 bits, and we don't know ... oh, wait,
no, nevermind... We *do* know the missing 8 bits, since they have to
fulfill the checksum check for command ^ not_command. So yeah, I'd say
32-bit scancodes for all nec extended, don't enforce the checksum by
default with a module option (or sysfs knob) to enable checksum
compliance.

-- 
Jarod Wilson
jarod@wilsonet.com

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-15  4:11                                 ` Jarod Wilson
@ 2010-11-15 18:39                                   ` David Härdeman
  2010-11-16 12:08                                   ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 31+ messages in thread
From: David Härdeman @ 2010-11-15 18:39 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Mauro Carvalho Chehab, Jarod Wilson, linux-media

On Sun, Nov 14, 2010 at 11:11:47PM -0500, Jarod Wilson wrote:
>Well, here's what I sent along on Friday:
>
>https://patchwork.kernel.org/patch/321592/
>
>Gives us support for using the full 32-bit codes right now w/o having
>to change any tables yet, but does require a modparam to skip the
>checksum checks, unless its an apple remote which we already know the
>vendor bytes for. I do think I'm ultimately leaning towards just doing
>the full 32 bits for all nec extended though -- optionally, we might
>include a modparam to *enable* the checksum check for those that want
>strict compliance (but I'd still say use the full 32 bits). The only
>issue I see with going to the full 32 right now is that we have all
>these nec tables with only 24 bits, and we don't know ... oh, wait,
>no, nevermind... We *do* know the missing 8 bits, since they have to
>fulfill the checksum check for command ^ not_command. So yeah, I'd say
>32-bit scancodes for all nec extended, don't enforce the checksum by
>default with a module option (or sysfs knob) to enable checksum
>compliance.

32 bit scancodes for *all* NEC, not only NEC extended. The "strict"
check will still be provided, just that it's done as part of the
keytable lookup.

-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-15  4:11                                 ` Jarod Wilson
  2010-11-15 18:39                                   ` David Härdeman
@ 2010-11-16 12:08                                   ` Mauro Carvalho Chehab
  2010-11-16 23:26                                     ` David Härdeman
  1 sibling, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2010-11-16 12:08 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: David Härdeman, Jarod Wilson, linux-media

Em 15-11-2010 02:11, Jarod Wilson escreveu:
> On Fri, Nov 5, 2010 at 9:27 AM, David Härdeman <david@hardeman.nu> wrote:
>> On Thu, 04 Nov 2010 15:43:33 -0400, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>> Em 04-11-2010 15:38, David Härdeman escreveu:
>>>> On Thu, Nov 04, 2010 at 11:54:25AM -0400, Jarod Wilson wrote:
>>>>> Okay, so we seem to be in agreement for an approach to handling this.
>>>>> I'll toss something together implementing that RSN... Though I talked
>>>>> with Mauro about this a bit yesterday here at LPC, and we're thinking
>>>>> maybe we slide this support back over into the nec decoder and make it
>>>>> a slightly more generic "use full 32 bits" NEC variant we look for
>>>>> and/or enable/disable somehow. I've got another remote here, for a
>>>>> Motorola cable box, which is NEC-ish, but always fails decode w/a
>>>>> checksum error ("got 0x00000000", iirc), which may also need to use
>>>>> the full 32 bits somehow... Probably a very important protocol variant
>>>>> to support, particularly once we have native transmit support, as its
>>>>> used by plenty of cable boxes on the major carriers here in the US.
>>>>
>>>> I've always found the "checksum" tests in the NEC decoder to be
>>>> unnecessary so I'm all for using a 32 bit scancode in all cases (and
>>>> still using a module param to squash the ID byte of apple remotes,
>>>> defaulting to "yes").
>>>>
>>> This means changing all existing NEC tables to have 32 bits, and add
>>> the "redundant" information on all of them.
>>
>> Yep (though we should use macros to generate scancodes)
>>
>>> It doesn't seem a good idea
>>> to me to add a penalty for those NEC tables that follow the standard.
>>
>> Which penalty?
>>
>> Using a 32 bit scancode won't affect keytable size or lookup speed.
>>
>> In some corner cases, additional keytable lookups will be performed for
>> decoded scancodes which would otherwise be deemed "invalid", but at the
>> time that decision can be made, most of the processing (reading timing
>> events from hardware, handing them to decoders, decoding them) has already
>> been done.
>>
>> If you're referring to the pain caused by changing existing keytables
>> (thereby breaking custom keytables), I think it's inevitable. Throwing away
>> information is not a good solution.
>>
>> As this subsystem progresses, there's going to be more and more reports of
>> remotes which, intentionally or unintentionally, do not follow the NEC
>> "standard" (I use that word in the most liberal sense). Using the full 32
>> bits allows us to support them without any module parameters or code
>> changes.
>>
>> Which solution do you suggest?
> 
> Well, here's what I sent along on Friday:
> 
> https://patchwork.kernel.org/patch/321592/
> 
> Gives us support for using the full 32-bit codes right now w/o having
> to change any tables yet, but does require a modparam to skip the
> checksum checks, unless its an apple remote which we already know the
> vendor bytes for. I do think I'm ultimately leaning towards just doing
> the full 32 bits for all nec extended though -- optionally, we might
> include a modparam to *enable* the checksum check for those that want
> strict compliance (but I'd still say use the full 32 bits). The only
> issue I see with going to the full 32 right now is that we have all
> these nec tables with only 24 bits, and we don't know ... oh, wait,
> no, nevermind... We *do* know the missing 8 bits, since they have to
> fulfill the checksum check for command ^ not_command. So yeah, I'd say
> 32-bit scancodes for all nec extended, don't enforce the checksum by
> default with a module option (or sysfs knob) to enable checksum
> compliance.

A modprobe parameter for it doesn't seem right. Users should not need to
do any manual hack for ther RC to work, if the keycode table is ok.

Also, changing the current tables to 32 bits will break userspace API, as all
userspace keytables for NEC will stop working, all due to a few vendors that 
decided to abuse on the NEC protocol. This doesn't sound fair.

Considering that the new setkeycode/getkeycode ioctls support a variable
size for scancodes, it seems to me that the proper solution is properly
add support for variable-size scancode tables. By doing this, one of the
properties for a scancode table is the size of the scancode. The NEC decoding
logic can take the scancode size into account, when deciding to check checksum
or not.

Cheers,
Mauro

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-16 12:08                                   ` Mauro Carvalho Chehab
@ 2010-11-16 23:26                                     ` David Härdeman
  2010-11-18 16:33                                       ` Jarod Wilson
  0 siblings, 1 reply; 31+ messages in thread
From: David Härdeman @ 2010-11-16 23:26 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Jarod Wilson, Jarod Wilson, linux-media

On Tue, Nov 16, 2010 at 10:08:29AM -0200, Mauro Carvalho Chehab wrote:
>Em 15-11-2010 02:11, Jarod Wilson escreveu:
>> Gives us support for using the full 32-bit codes right now w/o having
>> to change any tables yet, but does require a modparam to skip the
>> checksum checks, unless its an apple remote which we already know the
>> vendor bytes for. I do think I'm ultimately leaning towards just
>> doing the full 32 bits for all nec extended though -- optionally, we
>> might include a modparam to *enable* the checksum check for those
>> that want strict compliance (but I'd still say use the full 32 bits).
>> The only issue I see with going to the full 32 right now is that we
>> have all these nec tables with only 24 bits, and we don't know ...
>> oh, wait, no, nevermind... We *do* know the missing 8 bits, since
>> they have to fulfill the checksum check for command ^ not_command. So
>> yeah, I'd say 32-bit scancodes for all nec extended, don't enforce
>> the checksum by default with a module option (or sysfs knob) to
>> enable checksum compliance.
>
>A modprobe parameter for it doesn't seem right. Users should not need to
>do any manual hack for ther RC to work, if the keycode table is ok.

Agreed. There are already too many weird driver-specific modparams in
use as is.

>Also, changing the current tables to 32 bits will break userspace API, as all
>userspace keytables for NEC will stop working, all due to a few vendors that 
>decided to abuse on the NEC protocol. This doesn't sound fair.

The NEC protocol is hardly a standard. There's lot's of variations out
there. And intentionally throwing away information inside the kernel
doesn't sound fair either.

>Considering that the new setkeycode/getkeycode ioctls support a variable
>size for scancodes, it seems to me that the proper solution is properly
>add support for variable-size scancode tables. By doing this, one of the
>properties for a scancode table is the size of the scancode. The NEC decoding
>logic can take the scancode size into account, when deciding to check checksum
>or not.

With that approach you'd have to have the same scancode mangling code in
each driver which generates NEC scancodes as well as in the nec raw
decoder.

One suggestion would be to use a full 32-bit scancode table, but use the
size from the ioctl to determine how to generate the scancode to be
inserted into the table. So if the ioctl was called with a 2 byte
scancode, assume NEC with addr(8 bits) + cmd(8 bits); 3 byte -> NEC
Extended with addr(16 bits) + cmd(8 bits); 4 byte -> 32 bit scancode.

That way the nec decoder and other scancode drivers can be kept simple,
the scancode table has a full 32 bit scancode for all keys and userspace
won't see the difference (though I still think we should use the new
large scancode API to let userspace properly indicate the protocol along
with the scancode in the future).


-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-16 23:26                                     ` David Härdeman
@ 2010-11-18 16:33                                       ` Jarod Wilson
  2010-11-18 20:43                                         ` David Härdeman
  0 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-11-18 16:33 UTC (permalink / raw)
  To: David Härdeman; +Cc: Mauro Carvalho Chehab, Jarod Wilson, linux-media

On Wed, Nov 17, 2010 at 12:26:36AM +0100, David Härdeman wrote:
> On Tue, Nov 16, 2010 at 10:08:29AM -0200, Mauro Carvalho Chehab wrote:
...
> >Also, changing the current tables to 32 bits will break userspace API, as all
> >userspace keytables for NEC will stop working, all due to a few vendors that 
> >decided to abuse on the NEC protocol. This doesn't sound fair.
> 
> The NEC protocol is hardly a standard. There's lot's of variations out
> there. And intentionally throwing away information inside the kernel
> doesn't sound fair either.
> 
> >Considering that the new setkeycode/getkeycode ioctls support a variable
> >size for scancodes, it seems to me that the proper solution is properly
> >add support for variable-size scancode tables. By doing this, one of the
> >properties for a scancode table is the size of the scancode. The NEC decoding
> >logic can take the scancode size into account, when deciding to check checksum
> >or not.
> 
> With that approach you'd have to have the same scancode mangling code in
> each driver which generates NEC scancodes as well as in the nec raw
> decoder.
> 
> One suggestion would be to use a full 32-bit scancode table, but use the
> size from the ioctl to determine how to generate the scancode to be
> inserted into the table. So if the ioctl was called with a 2 byte
> scancode, assume NEC with addr(8 bits) + cmd(8 bits); 3 byte -> NEC
> Extended with addr(16 bits) + cmd(8 bits); 4 byte -> 32 bit scancode.
> 
> That way the nec decoder and other scancode drivers can be kept simple,
> the scancode table has a full 32 bit scancode for all keys and userspace
> won't see the difference (though I still think we should use the new
> large scancode API to let userspace properly indicate the protocol along
> with the scancode in the future).

I hacked together a semi-nasty variant of this, which I already know Mauro
isn't too keen on, but its perhaps a step in the right direction. At
least, its hopefully better than a modparam approach... ;)

http://git.kernel.org/?p=linux/kernel/git/jarod/linux-2.6-ir.git;a=commitdiff;h=a2eabcb44fa72e98a912c05a23659d0c946a17e5
http://git.kernel.org/?p=linux/kernel/git/jarod/linux-2.6-ir.git;a=commitdiff;h=164dc9cf5dec582bda5f7a059957ac2da2b0c1aa

Mauro's suggestion, iirc, was that max scancode size should be a property
of the keytable uploaded, and something set at load time (and probably
exposed as a sysfs node, similar to protocols). However, the one issue I
see there is that if someone loads a 16-bit keytable, then does a single
scancode replacement with something much larger, how are we going to
handle that?

-- 
Jarod Wilson
jarod@redhat.com


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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-18 16:33                                       ` Jarod Wilson
@ 2010-11-18 20:43                                         ` David Härdeman
  2010-11-18 20:49                                           ` Jarod Wilson
  0 siblings, 1 reply; 31+ messages in thread
From: David Härdeman @ 2010-11-18 20:43 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Mauro Carvalho Chehab, Jarod Wilson, linux-media

On Thu, Nov 18, 2010 at 11:33:04AM -0500, Jarod Wilson wrote:
>Mauro's suggestion, iirc, was that max scancode size should be a
>property of the keytable uploaded, and something set at load time (and
>probably exposed as a sysfs node, similar to protocols).

I think that would be a step in the wrong direction. It would make the
keytables less flexible while providing no real advantages.

-- 
David Härdeman

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-18 20:43                                         ` David Härdeman
@ 2010-11-18 20:49                                           ` Jarod Wilson
  2010-11-18 20:59                                             ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 31+ messages in thread
From: Jarod Wilson @ 2010-11-18 20:49 UTC (permalink / raw)
  To: David Härdeman; +Cc: Mauro Carvalho Chehab, Jarod Wilson, linux-media

On Thu, Nov 18, 2010 at 09:43:19PM +0100, David Härdeman wrote:
> On Thu, Nov 18, 2010 at 11:33:04AM -0500, Jarod Wilson wrote:
> >Mauro's suggestion, iirc, was that max scancode size should be a
> >property of the keytable uploaded, and something set at load time (and
> >probably exposed as a sysfs node, similar to protocols).
> 
> I think that would be a step in the wrong direction. It would make the
> keytables less flexible while providing no real advantages.

I think it was supposed to be something you could update on the fly when
uploading new keys, so its not entirely inflexible. Default keymap might
be 24-bit NEC, then you upload 32-bit NEC codes, and the max scancode size
would get updated at the same time. Of course, it probably wouldn't work
terribly well to have a mix of 24-bit and 32-bit NEC codes in the same
table.

-- 
Jarod Wilson
jarod@redhat.com


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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-18 20:49                                           ` Jarod Wilson
@ 2010-11-18 20:59                                             ` Mauro Carvalho Chehab
  2010-11-19 23:55                                               ` David Härdeman
  0 siblings, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2010-11-18 20:59 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: David Härdeman, Jarod Wilson, linux-media

Em 18-11-2010 18:49, Jarod Wilson escreveu:
> On Thu, Nov 18, 2010 at 09:43:19PM +0100, David Härdeman wrote:
>> On Thu, Nov 18, 2010 at 11:33:04AM -0500, Jarod Wilson wrote:
>>> Mauro's suggestion, iirc, was that max scancode size should be a
>>> property of the keytable uploaded, and something set at load time (and
>>> probably exposed as a sysfs node, similar to protocols).
>>
>> I think that would be a step in the wrong direction. It would make the
>> keytables less flexible while providing no real advantages.

We can't simply just change NEC to 32 bits, as we'll break userspace ABI 
(as current NEC keycode tables use only 16 bits). So, an old table will not
worky anymore, if we do such change.

> I think it was supposed to be something you could update on the fly when
> uploading new keys, so its not entirely inflexible. Default keymap might
> be 24-bit NEC, then you upload 32-bit NEC codes, and the max scancode size
> would get updated at the same time. Of course, it probably wouldn't work
> terribly well to have a mix of 24-bit and 32-bit NEC codes in the same
> table.

There's another reason why it may be interesting to have the scancode size
stored somewhere. With the new flexible scancode size, some devices may have
bigger scancodes (I remember people mentioned some cases with 128 bits when 
we've discussed the getkeycodbig patches in the past). So, we'll need to
address some cases where the scancodes don't have 32 bits. I think that the
current maximum limit is 31 bits (as the search algorithm uses the signal
bit for some reason).

Cheers,
Mauro

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

* Re: [RFC PATCH 0/2] Apple remote support
  2010-11-18 20:59                                             ` Mauro Carvalho Chehab
@ 2010-11-19 23:55                                               ` David Härdeman
  0 siblings, 0 replies; 31+ messages in thread
From: David Härdeman @ 2010-11-19 23:55 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Jarod Wilson, Jarod Wilson, linux-media

On Thu, Nov 18, 2010 at 06:59:43PM -0200, Mauro Carvalho Chehab wrote:
>Em 18-11-2010 18:49, Jarod Wilson escreveu:
>> On Thu, Nov 18, 2010 at 09:43:19PM +0100, David Härdeman wrote:
>>> On Thu, Nov 18, 2010 at 11:33:04AM -0500, Jarod Wilson wrote:
>>>> Mauro's suggestion, iirc, was that max scancode size should be a
>>>> property of the keytable uploaded, and something set at load time (and
>>>> probably exposed as a sysfs node, similar to protocols).
>>>
>>> I think that would be a step in the wrong direction. It would make the
>>> keytables less flexible while providing no real advantages.
>
>We can't simply just change NEC to 32 bits, as we'll break userspace ABI 
>(as current NEC keycode tables use only 16 bits). So, an old table will not
>worky anymore, if we do such change.

The idea was to do the conversion from <whatever> to 32 bits in
get/setkeycode.


-- 
David Härdeman

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

end of thread, other threads:[~2010-11-19 23:55 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-29  3:11 [RFC PATCH 0/2] Apple remote support Jarod Wilson
2010-10-29  3:13 ` [RFC PATCH 1/2] ir-nec-decoder: decode Apple's NEC remote variant Jarod Wilson
2010-10-29 22:15   ` Andy Walls
2010-10-29  3:13 ` [RFC PATCH 2/2] IR: add Apple remote keymap Jarod Wilson
2010-10-29  3:15 ` [RFC PATCH 0/2] Apple remote support Jarod Wilson
2010-10-29 13:46   ` Mauro Carvalho Chehab
2010-10-29 15:11     ` Jarod Wilson
2010-10-29 19:17       ` David Härdeman
2010-10-29 19:27         ` Jarod Wilson
2010-10-29 19:59           ` David Härdeman
2010-10-29 20:09             ` Jarod Wilson
2010-10-30 23:36               ` David Härdeman
2010-10-31  2:32                 ` Jarod Wilson
2010-11-01 21:56                   ` David Härdeman
2010-11-02 20:42                     ` Jarod Wilson
2010-11-04 12:16                       ` David Härdeman
2010-11-04 15:54                         ` Jarod Wilson
2010-11-04 19:38                           ` David Härdeman
2010-11-04 19:43                             ` Mauro Carvalho Chehab
2010-11-05 13:27                               ` David Härdeman
2010-11-05 14:04                                 ` Christopher Harrington
2010-11-07 19:01                                   ` Jarod Wilson
2010-11-15  4:11                                 ` Jarod Wilson
2010-11-15 18:39                                   ` David Härdeman
2010-11-16 12:08                                   ` Mauro Carvalho Chehab
2010-11-16 23:26                                     ` David Härdeman
2010-11-18 16:33                                       ` Jarod Wilson
2010-11-18 20:43                                         ` David Härdeman
2010-11-18 20:49                                           ` Jarod Wilson
2010-11-18 20:59                                             ` Mauro Carvalho Chehab
2010-11-19 23:55                                               ` David Härdeman

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