[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 2 Mar 2009 21:47:02 -0500
From: Mike Murphy <mamurph@...clemson.edu>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
linux-usb@...r.kernel.org, greg@...ah.com, oliver@...kum.org,
fweisbec@...il.com, torvalds@...ux-foundation.org
Subject: Re: PATCH [1/3] drivers/input/xpad.c: Improve Xbox 360 wireless
support and add sysfs interface
On Mon, Mar 2, 2009 at 4:04 PM, Andrew Morton <akpm@...ux-foundation.org> wrote:
>> +static void xpad_process_sticks(struct usb_xpad *xpad, unsigned char *data)
>> +{
>> + struct input_dev *dev = xpad->dev;
>> + int coords[4]; /* x, y, rx, ry */
>> + int x_offset, y_offset, rx_offset, ry_offset;
>> + int c;
>> + int range;
>> + int abs_magnitude, adjusted_magnitude, difference, scale_fraction;
>> + int dead_zone[2], stick_limit[2];
>> +
>> + dead_zone[0] = xpad->left_dead_zone;
>> + dead_zone[1] = xpad->right_dead_zone;
>> + stick_limit[0] = xpad->left_stick_limit;
>> + stick_limit[1] = xpad->right_stick_limit;
>> +
>> + if (xpad->xtype == XTYPE_XBOX) {
>> + x_offset = 12;
>> + y_offset = 14;
>> + rx_offset = 16;
>> + ry_offset = 18;
>> + } else {
>> + x_offset = 6;
>> + y_offset = 8;
>> + rx_offset = 10;
>> + ry_offset = 12;
>> + }
>> +
>> + coords[0] = (__s16) le16_to_cpup((__le16 *)(data + x_offset));
>> + coords[1] = ~(__s16) le16_to_cpup((__le16 *)(data + y_offset));
>> + coords[2] = (__s16) le16_to_cpup((__le16 *)(data + rx_offset));
>> + coords[3] = ~(__s16) le16_to_cpup((__le16 *)(data + ry_offset));
>
> We don't need the first typecast here and if `data' were to have type
> `void *', we could do away with the second cast as well.
>
gcc 4.3.3 refused to compile with data set to type void *'. I also
tried removing the casts and changing to le16_to_cpu, but the result
was that stick inputs were lost. After further testing, I reverted to
the original code, which was taken from the driver in the stable tree.
The typecasting follows this logic:
1. The stick axis inputs are 16-bit *unsigned* little endian (0 -
65535), which need to map onto the *signed* axis (-32767 to +32767)
inside the input subsystem.
2. The innermost typecast (__le16 *) tells gcc to treat the (unsigned
char *) address as a pointer to an unsigned little-endian value, which
is converted to a pointer of host endiannes -- still unsigned -- by
le16_to_cpup.
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.
Is there a cleaner way to accomplish the transition from 16-bit
unsigned little endian to 16-bit signed host endian? If so, I can
change it... if not, I can comment this code to explain why it looks
like it does. I don't have enough experience with the Linux internal
types system to have a better solution.
I've fixed the other problems (and maybe created new ones :)...
revision of this part of the patch to follow.
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists