Re: PATCH [1/3] drivers/input/xpad.c: Improve Xbox 360 wireless support and add sysfs interface

[Mike Murphy]

On Mon, Mar 2, 2009 at 10:12 PM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> 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
>

Thanks Linus... that solution worked, and it did make the code
shorter. To get a clean compile, I had to cast the actual argument
data pointer to (__le16 *), but that only adds 2 casts. I will send
the revision shortly.

Thanks,
Mike
-- 
Mike Murphy
Ph.D. Candidate and NSF Graduate Research Fellow
Clemson University School of Computing
120 McAdams Hall
Clemson, SC 29634-0974 USA
Tel: +1 864.656.2838   Fax: +1 864.656.0145
http://cirg.cs.clemson.edu/~mamurph
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/