Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes

[James Hogan <james.hogan@imgtec.com>]

[-- Attachment #1: Type: text/plain, Size: 2323 bytes --]

On 29/03/14 16:11, David Härdeman wrote:
> Using the full 32 bits for all kinds of NEC scancodes simplifies rc-core
> and the nec decoder without any loss of functionality.
> 
> In order to maintain backwards compatibility, some heuristics are added
> in rc-main.c to convert scancodes to NEC32 as necessary.
> 
> I plan to introduce a different ioctl later which makes the protocol
> explicit (and which expects all NEC scancodes to be 32 bit, thereby
> removing the need for guesswork).
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>
> ---
> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c
> index 40ee844..133ea45 100644
> --- a/drivers/media/rc/img-ir/img-ir-nec.c
> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
> @@ -5,42 +5,20 @@
>   */
>  
>  #include "img-ir-hw.h"
> -#include <linux/bitrev.h>
>  
>  /* Convert NEC data to a scancode */
>  static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
>  			       u32 *scancode, u64 enabled_protocols)
>  {
> -	unsigned int addr, addr_inv, data, data_inv;
>  	/* a repeat code has no data */
>  	if (!len)
>  		return IMG_IR_REPEATCODE;
> +
>  	if (len != 32)
>  		return -EINVAL;
> -	/* raw encoding: ddDDaaAA */
> -	addr     = (raw >>  0) & 0xff;
> -	addr_inv = (raw >>  8) & 0xff;
> -	data     = (raw >> 16) & 0xff;
> -	data_inv = (raw >> 24) & 0xff;
> -	if ((data_inv ^ data) != 0xff) {
> -		/* 32-bit NEC (used by Apple and TiVo remotes) */
> -		/* scan encoding: AAaaDDdd (LSBit first) */
> -		*scancode = bitrev8(addr)     << 24 |
> -			    bitrev8(addr_inv) << 16 |
> -			    bitrev8(data)     <<  8 |
> -			    bitrev8(data_inv);
> -	} else if ((addr_inv ^ addr) != 0xff) {
> -		/* Extended NEC */
> -		/* scan encoding: AAaaDD */
> -		*scancode = addr     << 16 |
> -			    addr_inv <<  8 |
> -			    data;
> -	} else {
> -		/* Normal NEC */
> -		/* scan encoding: AADD */
> -		*scancode = addr << 8 |
> -			    data;
> -	}
> +
> +	/* raw encoding : ddDDaaAA -> scan encoding: AAaaDDdd */
> +	*scancode = swab32((u32)raw);

What's the point of the byte swapping?

Surely the most natural NEC encoding would just treat it as a single
32-bit (LSBit first) field rather than 4 8-bit fields that needs swapping.

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]