On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote:

> This patch restores remote control input for cx2388x based boards on
> Linux kernels >= 2.6.38.
> 
> After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote
> control input of my Hauppauge Nova-S plus was no longer functioning.  
> I posted a question on this newsgroup and Mauro Carvalho Chehab gave
> some helpful pointers as to the likely cause.
> 
> Turns out that there are 2 problems:
> 
> 1. In the IR interrupt handler of cx88-input.c there's a 32-bit multiply
> overflow which causes IR pulse durations to be incorrectly calculated.
> 
> 2. The RC5 decoder appends the system code to the scancode and passes
> the combination to rc_keydown().  Unfortunately, the combined value is
> then forwarded to input_event() which then fails to recognise a valid
> scancode and hence no input events are generated.
> 
> I note that in commit 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d, which
> introduced these changes, David Härdeman changed the IR sample frequency
> to a supposed 4kHz.  However, the registers dealing with IR input are
> undocumented in the cx2388x datasheets and there's no publicly available
> information on them.  I have to ask the question why this change was
> made as it is of no apparent benefit and could have unanticipated
> consequences.  IMHO that change should also be reverted unless there is
> evidence to substantiate it.
> 
> Signed off by: Lawrence Rust <lvr at softsystem dot co dot uk>

Nacked-by: Jarod Wilson <ja...@redhat.com>

> diff --git a/drivers/media/rc/ir-rc5-decoder.c 
> b/drivers/media/rc/ir-rc5-decoder.c
> index ebdba55..c4052da 100644
> --- a/drivers/media/rc/ir-rc5-decoder.c
> +++ b/drivers/media/rc/ir-rc5-decoder.c
> @@ -144,10 +144,15 @@ again:
>                       system   = (data->bits & 0x007C0) >> 6;
>                       toggle   = (data->bits & 0x00800) ? 1 : 0;
>                       command += (data->bits & 0x01000) ? 0 : 0x40;
> -                     scancode = system << 8 | command;
> -
> -                     IR_dprintk(1, "RC5 scancode 0x%04x (toggle: %u)\n",
> -                                scancode, toggle);
> +            /* Notes
> +             * 1. Should filter unknown systems e.g Hauppauge use 0x1e or 
> 0x1f
> +             * 2. Don't include system in the scancode otherwise 
> input_event()
> +             *    doesn't recognise the scancode
> +             */
> +                     scancode = command;
> +
> +                     IR_dprintk(1, "RC5 scancode 0x%02x (system: 0x%02x 
> toggle: %u)\n",
> +                                scancode, system, toggle);
>               }
> 
>               rc_keydown(dev, scancode, toggle);

This part is so very very wrong. We should NOT filter here. Filtering
can be achieved on the keymap side, and you *do* include the system
here. The fix for your issue is an update to the relevant keymap so
that its matching the system byte as well.

The divide fix looks sane though.

-- 
Jarod Wilson
ja...@wilsonet.com



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

Reply via email to