Apple remote support

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 <[email protected]>

---
 drivers/media/IR/ir-nec-decoder.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

drivers/media/IR/ir-nec-decoder.c +9 -1

index 70993f7..6dcddd2 100644
@@ -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;

Signature · 1 line

1.7.1

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).

Quoted 2 lines

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...

Em 29-10-2010 01:15, Jarod Wilson escreveu:

Quoted 4 lines · Jarod Wilson

Quoted 2 lines

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).

Quoted 4 lines

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.

Quoted 4 lines · Jarod Wilson

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?

Quoted 7 lines

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.

Quoted 7 lines · David Härdeman

Quoted 4 lines · Jarod Wilson

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.

Quoted 7 lines · Jarod Wilson

Quoted 6 lines · David Härdeman

Quoted 4 lines · Jarod Wilson

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.

Quoted 3 lines

Quoted 1 line

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

256, apparently.

Does the remote pick one for you at random?

Quoted 10 lines · David Härdeman

Quoted 9 lines · Jarod Wilson

Quoted 7 lines · David Härdeman

Quoted 4 lines · Jarod Wilson

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?

Quoted 3 lines

Quoted 1 line

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.

Quoted 11 lines · Jarod Wilson

Quoted 10 lines · David Härdeman

Quoted 9 lines · Jarod Wilson

Quoted 7 lines · David Härdeman

Quoted 4 lines · Jarod Wilson

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?

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

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.

Quoted 10 lines · David Härdeman

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.

Quoted 12 lines · Jarod Wilson

Quoted 10 lines · David Härdeman

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.

Quoted 8 lines

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).

On Mon, Nov 1, 2010 at 5:56 PM, David Härdeman <[email protected]>

Quoted 24 lines · Jarod Wilson

Quoted 22 lines

Quoted 11 lines · David Härdeman

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.

Quoted 2 lines

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)

Quoted 1 line · Jarod Wilson

On Mon, Nov 1, 2010 at 5:56 PM, David Härdeman <[email protected]>

Quoted 27 lines · wrote:

Quoted 26 lines · Jarod Wilson

Quoted 14 lines

Quoted 11 lines · David Härdeman

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.

Quoted 8 lines

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).

Quoted 5 lines

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)

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.

Quoted 11 lines

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").

Em 04-11-2010 15:38, David Härdeman escreveu:

Quoted 13 lines · Jarod Wilson

Quoted 11 lines

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.

Quoted 5 lines

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.

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?

Quoted 11 lines · David Härdeman

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.

Quoted 14 lines · Mauro Carvalho Chehab

Em 04-11-2010 15:38, David Härdeman escreveu:

Quoted 13 lines · Jarod Wilson

Quoted 11 lines

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.

Quoted 7 lines

Quoted 5 lines

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)

Quoted 2 lines

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.

Quoted 43 lines

Quoted 14 lines · Mauro Carvalho Chehab

Em 04-11-2010 15:38, David Härdeman escreveu:

Quoted 13 lines · Jarod Wilson

Quoted 11 lines

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.

Quoted 7 lines

Quoted 5 lines

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)

Quoted 2 lines

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.

Quoted 7 lines

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.

Em 15-11-2010 02:11, Jarod Wilson escreveu:

Quoted 6 lines

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

Quoted 8 lines

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.

On Tue, Nov 16, 2010 at 10:08:29AM -0200, Mauro Carvalho Chehab wrote:

Quoted 3 lines

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.

Quoted 6 lines

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).

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).

Quoted 3 lines · Jarod Wilson

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.

Quoted 7 lines · Jarod Wilson

Quoted 3 lines

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.

Em 18-11-2010 18:49, Jarod Wilson escreveu:

Quoted 6 lines · David Härdeman

Quoted 5 lines · Jarod Wilson

Quoted 3 lines

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).

Quoted 2 lines

Quoted 2 lines

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.