diff mbox

[media] nec decoder: wrong bit order for nec32 protocol

Message ID 1478708015-1164-5-git-send-email-sean@mess.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sean Young Nov. 9, 2016, 4:13 p.m. UTC
The bits are sent in lsb first. Hardware decoders also send nec32
in this order (e.g. dib0700). This should be consistent, however
I have no way of knowing which order the LME2510 and Tivo keymaps
are (the only two kernel keymaps with NEC32).

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/ir-nec-decoder.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Mauro Carvalho Chehab Nov. 22, 2016, 1:35 p.m. UTC | #1
Em Wed,  9 Nov 2016 16:13:35 +0000
Sean Young <sean@mess.org> escreveu:

> The bits are sent in lsb first. Hardware decoders also send nec32
> in this order (e.g. dib0700). This should be consistent, however
> I have no way of knowing which order the LME2510 and Tivo keymaps
> are (the only two kernel keymaps with NEC32).

Hmm.. the lme2510 receives the scancode directly. So, this
patch shouldn't affect it. So, we're stuck with the Tivo IR.

On Tivo, only a few keys (with duplicated scancodes) don't start with
0xa10c. So, it *seems* that this is an address.

The best here would be to try to get a Tivo remote controller[1], and
do some tests with a driver that has a hardware decoder capable of
output NEC32 data, and some driver that receives raw IR data in
order to be sure.

In any case, we need to patch both the NEC32 decoder and the table
at the same time, to be 100% sure.

[1] or some universal remote controller that could emulate
the Tivo's scan codes. I suspect that the IR in question is
this one, but maybe Jarod could shed some light here:
	https://www.amazon.com/TiVo-Remote-Control-Universal-Replacement/dp/B00DYYKA04


> 
> Signed-off-by: Sean Young <sean@mess.org>
> ---
>  drivers/media/rc/ir-nec-decoder.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
> index 2a9d155..ba02d05 100644
> --- a/drivers/media/rc/ir-nec-decoder.c
> +++ b/drivers/media/rc/ir-nec-decoder.c
> @@ -170,7 +170,10 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
>  		if (send_32bits) {
>  			/* NEC transport, but modified protocol, used by at
>  			 * least Apple and TiVo remotes */
> -			scancode = data->bits;
> +			scancode = address     << 24 |
> +				   not_address << 16 |
> +				   command     << 8 |
> +				   not_command;
>  			IR_dprintk(1, "NEC (modified) scancode 0x%08x\n", scancode);
>  			rc_type = RC_TYPE_NEC32;
>  		} else if ((address ^ not_address) != 0xff) {


Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarod Wilson Nov. 22, 2016, 2:19 p.m. UTC | #2
On Tue, Nov 22, 2016 at 11:35:06AM -0200, Mauro Carvalho Chehab wrote:
> Em Wed,  9 Nov 2016 16:13:35 +0000
> Sean Young <sean@mess.org> escreveu:
> 
> > The bits are sent in lsb first. Hardware decoders also send nec32
> > in this order (e.g. dib0700). This should be consistent, however
> > I have no way of knowing which order the LME2510 and Tivo keymaps
> > are (the only two kernel keymaps with NEC32).
> 
> Hmm.. the lme2510 receives the scancode directly. So, this
> patch shouldn't affect it. So, we're stuck with the Tivo IR.
> 
> On Tivo, only a few keys (with duplicated scancodes) don't start with
> 0xa10c. So, it *seems* that this is an address.
> 
> The best here would be to try to get a Tivo remote controller[1], and
> do some tests with a driver that has a hardware decoder capable of
> output NEC32 data, and some driver that receives raw IR data in
> order to be sure.
> 
> In any case, we need to patch both the NEC32 decoder and the table
> at the same time, to be 100% sure.
> 
> [1] or some universal remote controller that could emulate
> the Tivo's scan codes. I suspect that the IR in question is
> this one, but maybe Jarod could shed some light here:
> 	https://www.amazon.com/TiVo-Remote-Control-Universal-Replacement/dp/B00DYYKA04

Been away from the game for a few years now, so there are a good number of
cobwebs in this section of my brain... I'm pretty sure I do have both a
remote and receiver on hand that would fit the bill here though. Is the
question primarily about what actually gets emitted by the TiVo remote?
Mauro Carvalho Chehab Nov. 22, 2016, 3:08 p.m. UTC | #3
Em Tue, 22 Nov 2016 09:19:19 -0500
Jarod Wilson <jarod@redhat.com> escreveu:

> On Tue, Nov 22, 2016 at 11:35:06AM -0200, Mauro Carvalho Chehab wrote:
> > Em Wed,  9 Nov 2016 16:13:35 +0000
> > Sean Young <sean@mess.org> escreveu:
> >   
> > > The bits are sent in lsb first. Hardware decoders also send nec32
> > > in this order (e.g. dib0700). This should be consistent, however
> > > I have no way of knowing which order the LME2510 and Tivo keymaps
> > > are (the only two kernel keymaps with NEC32).  
> > 
> > Hmm.. the lme2510 receives the scancode directly. So, this
> > patch shouldn't affect it. So, we're stuck with the Tivo IR.
> > 
> > On Tivo, only a few keys (with duplicated scancodes) don't start with
> > 0xa10c. So, it *seems* that this is an address.
> > 
> > The best here would be to try to get a Tivo remote controller[1], and
> > do some tests with a driver that has a hardware decoder capable of
> > output NEC32 data, and some driver that receives raw IR data in
> > order to be sure.
> > 
> > In any case, we need to patch both the NEC32 decoder and the table
> > at the same time, to be 100% sure.
> > 
> > [1] or some universal remote controller that could emulate
> > the Tivo's scan codes. I suspect that the IR in question is
> > this one, but maybe Jarod could shed some light here:
> > 	https://www.amazon.com/TiVo-Remote-Control-Universal-Replacement/dp/B00DYYKA04  
> 
> Been away from the game for a few years now, so there are a good number of
> cobwebs in this section of my brain... I'm pretty sure I do have both a
> remote and receiver on hand that would fit the bill here though. Is the
> question primarily about what actually gets emitted by the TiVo remote?

Yes. Sean suspects that we're decoding the NEC 32 bits in the
wrong order at the in-kernel NEC decoder on x86[1].

The idea here is to double-check how the bits are encapsulated, in order
to do the right thing at the IR decoder.

[1] the current code actually sucks, as it produces different
scancodes on big endian or little endian, so it needs to be
fixed anyway, as it simply does:
	scancode = data->bits;

instead of using be32_to_cpu().


Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sean Young Nov. 22, 2016, 3:49 p.m. UTC | #4
On Tue, Nov 22, 2016 at 09:19:19AM -0500, Jarod Wilson wrote:
> On Tue, Nov 22, 2016 at 11:35:06AM -0200, Mauro Carvalho Chehab wrote:
> > Em Wed,  9 Nov 2016 16:13:35 +0000
> > Sean Young <sean@mess.org> escreveu:
> > 
> > > The bits are sent in lsb first. Hardware decoders also send nec32
> > > in this order (e.g. dib0700). This should be consistent, however
> > > I have no way of knowing which order the LME2510 and Tivo keymaps
> > > are (the only two kernel keymaps with NEC32).
> > 
> > Hmm.. the lme2510 receives the scancode directly. So, this
> > patch shouldn't affect it. So, we're stuck with the Tivo IR.
> > 
> > On Tivo, only a few keys (with duplicated scancodes) don't start with
> > 0xa10c. So, it *seems* that this is an address.

The problem here is that each *byte* is sent lsb first, so only the 
ordering within each byte would change. 

> > The best here would be to try to get a Tivo remote controller[1], and
> > do some tests with a driver that has a hardware decoder capable of
> > output NEC32 data, and some driver that receives raw IR data in
> > order to be sure.
> > 
> > In any case, we need to patch both the NEC32 decoder and the table
> > at the same time, to be 100% sure.
> > 
> > [1] or some universal remote controller that could emulate
> > the Tivo's scan codes. I suspect that the IR in question is
> > this one, but maybe Jarod could shed some light here:
> > 	https://www.amazon.com/TiVo-Remote-Control-Universal-Replacement/dp/B00DYYKA04
> 
> Been away from the game for a few years now, so there are a good number of
> cobwebs in this section of my brain... I'm pretty sure I do have both a
> remote and receiver on hand that would fit the bill here though. Is the
> question primarily about what actually gets emitted by the TiVo remote?

Yes, it is. Hardware nec decoders send each byte lsb first, but 
ir-nec-decoder.c decoder does msb first for nec32 (not for extended nec or
plain nec).

If we have a mode2 recording of the remote (and if we know what button it
is), then we can replay it against a hardware nec decoder and the software
decoder. Rather than mode2, this can also be done with ir-ctl which is in
v4l-utils git.

Alternatively simply checking if the tivo keymap works with the
software decoder (mceusb hardware for example) should be sufficient.

That would be really helpful, thanks.


Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sean Young Nov. 22, 2016, 3:56 p.m. UTC | #5
On Tue, Nov 22, 2016 at 01:08:52PM -0200, Mauro Carvalho Chehab wrote:
> Em Tue, 22 Nov 2016 09:19:19 -0500
> Jarod Wilson <jarod@redhat.com> escreveu:
> > Been away from the game for a few years now, so there are a good number of
> > cobwebs in this section of my brain... I'm pretty sure I do have both a
> > remote and receiver on hand that would fit the bill here though. Is the
> > question primarily about what actually gets emitted by the TiVo remote?
> 
> Yes. Sean suspects that we're decoding the NEC 32 bits in the
> wrong order at the in-kernel NEC decoder on x86[1].
> 
> The idea here is to double-check how the bits are encapsulated, in order
> to do the right thing at the IR decoder.
> 
> [1] the current code actually sucks, as it produces different
> scancodes on big endian or little endian, so it needs to be
> fixed anyway, as it simply does:
> 	scancode = data->bits;
> 
> instead of using be32_to_cpu().

Bits are shifted in one at at time in the nec decoder so it is endian safe.

The problem is that the same NEC32 IR will produce different scancodes
with ir-nec-decoder.c and other nec decoders, e.g. dib0700_core.c


Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index 2a9d155..ba02d05 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -170,7 +170,10 @@  static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
 		if (send_32bits) {
 			/* NEC transport, but modified protocol, used by at
 			 * least Apple and TiVo remotes */
-			scancode = data->bits;
+			scancode = address     << 24 |
+				   not_address << 16 |
+				   command     << 8 |
+				   not_command;
 			IR_dprintk(1, "NEC (modified) scancode 0x%08x\n", scancode);
 			rc_type = RC_TYPE_NEC32;
 		} else if ((address ^ not_address) != 0xff) {