lkml.org 
[lkml]   [2009]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: PATCH [1/3] drivers/input/xpad.c: Improve Xbox 360 wireless support and add sysfs interface


On Mon, 2 Mar 2009, Mike Murphy wrote:
>
> 3. The outer cast (__s16) converts the unsigned values to signed
> values, while the "~" inverts the y axes to make them function like a
> joystick instead of a flight simulator control.

You should do the ~ before the cast, or use - if you just want to reverse
things. It probably doesn't much matter (the difference between ~ and - i
just one), but still..

Also, quite frankly, it looks like your 'coords[]' array should just be of
type 's16' (rather than 'int') to begin with. You seem to really never use
it as an int anyway. That would get rid of the cast.

> Is there a cleaner way to accomplish the transition from 16-bit
> unsigned little endian to 16-bit signed host endian?

I think the code is fine, but I think you'd be better off if the "data"
pointer was perhaps of type "le16 *" to begin with.

That obviously means that your "offset" addition should now be in 16-bit
words rather than in bytes, so you'd need to divide the offsets by two to
do that, but those are just numbers anyway. And quite frankly, it looks
like the actual data is just offset differently - but with the same fixed
offset between values - for the two cases, so you could just have _one_
offset (and even just add that into the 'data' pointer).

That would get rid of the second cast. You'd end up with just

s16 coords[4];

/* In words - so this is 12 vs 6 bytes into the data */
data += (xpad->xtype == XTYPE_XBOX) ? 6 : 3;

coords[0] = le16_to_cpup(data);
coords[1] = ~le16_to_cpup(data + 1);
coords[2] = le16_to_cpup(data + 2);
coords[3] = ~le16_to_cpup(data + 3);
..

which looks a bit shorter and avoids those casts. I dunno.

Linus


\
 
 \ /
  Last update: 2009-03-03 04:17    [W:0.063 / U:0.512 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site