Chase Douglas writes:
> On Tue, 2010-07-06 at 03:54 +0800, Ping Cheng wrote:
>> On Mon, Jul 5, 2010 at 10:50 PM, Michael Poole <mdpoole@xxxxxxxxxxx> wrote:
>> > The X and Y values have two more significant bits in the same byte
>> > that contains click status. Include these in the reported value.
>> > Thanks to Iain Hibbert of NetBSD for pointing this out.
>> >
>> > Signed-off-by: Michael Poole <mdpoole@xxxxxxxxxxx>
>> > ---
>> > drivers/hid/hid-magicmouse.c | 4 ++--
>> > 1 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
>> > index 0b89c1c..7cdda23 100644
>> > --- a/drivers/hid/hid-magicmouse.c
>> > +++ b/drivers/hid/hid-magicmouse.c
>> > @@ -267,8 +267,8 @@ static int magicmouse_raw_event(struct hid_device *hdev,
>> > * to have the current touch information before
>> > * generating a click event.
>> > */
>> > - x = (signed char)data[1];
>> > - y = (signed char)data[2];
>> > + x = (int)(((data[3] & 0x0c) << 28) | (data[1] << 22)) >> 22;
>> > + y = (int)(((data[3] & 0x30) << 26) | (data[2] << 22)) >> 22;
>>
>> Will the following give us the same result?
>>
>> + x = (int)(((data[3] & 0x0c) << 6) | data[1]);
>> + y = (int)(((data[3] & 0x30) << 4) | data[2]);
>
> I thought about this too, but there's a sign extension issue. If the X
> coordinate is -1 (these are relative coordinates), then you will end up
> with (int)(1023), which is no longer -1.
>
> When you shift to the right, the bits are sign-extended. Thus, shifting
> everything to the most significant bit of an integer, and then shifting
> everything back to the least significant will properly sign extend.
>
> (I actually wrote a test program just to verify this)
>
> That's just a reason for the shifts. There very well could be a more
> elegant solution.
C99 says that the result of right-shifting a negative value is
compiler-defined. gcc documents that it ensures sign extension. Other
parts of hid-magicmouse.c use this idiom already. The corresponding
idiom in hid-core.c (see the snto32() function) would look something
like this:
x = ((data[3] & 0x0c) << 6) | data[1];
x |= (x & (1 << 9)) ? (-1 << 10) : 0;
Which do people find more readable? Is the more portable behavior of
this idiom preferred over requiring sign extension by the compiler?
Michael Poole
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
