Re: [PATCH 2/2] media: rtl28xxu: improve IR receiver

["Marko Mäkelä" <marko.makela@iki.fi>]

Hi Sean,

I finally took the time to get a deeper understanding of the infrared 
remote control subsystem. I think that I now understand the translation 
into key-down, key-up, and key-repeat events. For the RC5 protocol, 
rc_repeat() will not be called by ir-rc5-decoder.c but instead, 
ir_do_keydown() will handle the repeat. For lirc_scancode_event() it 
will never set the LIRC_SCANCODE_FLAG_REPEAT bit, even if !new_event and 
the protocol does support the toggle bit. That might qualify as a bug.

Sat, Feb 12, 2022 at 04:32:19PM +0000, Sean Young wrote:
>This device presents an IR buffer, which can be read and cleared.
>Clearing the buffer is racey with receiving IR, so wait until the IR
>message is finished before clearing it.
>
>This should minimize the chance of the buffer clear happening while
>IR is being received, although we cannot avoid this completely.

I just realized that this limitation of the interface may be causing 
exactly what I was observing when I was testing this. If a constant 
stream of data is being received because a button is being held down, a 
buffer overflow or wrap-around glitch is inevitable, maybe expect if the 
wrap-around occurs exactly at the 128-byte boundary.

How about the following improvement? If IR_RX_BC is a simple cursor to 
the 128-byte IR_RX_BUF, then rtl2832u_rc_query() could avoid sending 
refresh_tab[] but simply remember where the previous call left off. We 
could always read the 128 bytes at IR_RX_BUF, and process everything 
between the previous position reported by IR_RX_BC and the current 
position reported by IR_RX_BC, and treat buf[] as a ring buffer.

Last time I tested it, the patch was a significant improvement. I think 
that "perfect" is the enemy of "good enough", and the patch should be 
included in the kernel.

Best regards,

	Marko