From: Grant Grundler Date: Thu, 19 Oct 2006 22:09:51 +0000 (-0700) Subject: hid-core: big-endian fix fix X-Git-Tag: v2.6.19-rc5~27^2~10 X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=68717950e11eab8ff754b2721d23e9cb3a47b56f;p=linux-2.6 hid-core: big-endian fix fix Adam Kropelin had posted 32-bit fix in June 2005 about two weeks after I originally had posted my fixes for big endian support. Adam has a UPS device which reports LINEV using 32-bits. Added comments to describe the limitations of the code. extract() is the same version I posted earlier and tested in user space. Made similar changes to implement() routine. I've written (and will shortly post) a test for implement(). Code tested on C3600 (parisc) with USB keyboard/mouse attached. I've dropped test_implement.c and a few other user space test programs on http://iou.parisc-linux.org/~grundler/tests/ -rw-r--r-- 1 grundler grundler 1750 Oct 18 09:13 test_extract.c -rw-r--r-- 1 grundler grundler 561 Jan 25 2006 test_ffs.c -rw-r--r-- 1 grundler users 7175 Apr 8 2005 test_fls.c -rw-r--r-- 1 grundler grundler 206 Sep 1 15:52 test_gettimeofday.c -rw-r--r-- 1 grundler grundler 1886 Oct 19 09:20 test_implement.c -rw-r--r-- 1 grundler users 2707 Jun 4 2005 test_unaligned.c I would appreciate if someone else would look at the output of test_implement.c to make it does The Right Thing. Signed-off-by: Grant Grundler Cc: Matthew Wilcox Cc: Dmitry Torokhov Acked-By: Adam Kropelin Signed-off-by: Andrew Morton Signed-off-by: Greg Kroah-Hartman --- diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c index 45f44fe33b..6d42036c90 100644 --- a/drivers/usb/input/hid-core.c +++ b/drivers/usb/input/hid-core.c @@ -270,7 +270,7 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign * Read data value from item. */ -static __inline__ __u32 item_udata(struct hid_item *item) +static u32 item_udata(struct hid_item *item) { switch (item->size) { case 1: return item->data.u8; @@ -280,7 +280,7 @@ static __inline__ __u32 item_udata(struct hid_item *item) return 0; } -static __inline__ __s32 item_sdata(struct hid_item *item) +static s32 item_sdata(struct hid_item *item) { switch (item->size) { case 1: return item->data.s8; @@ -727,7 +727,7 @@ static struct hid_device *hid_parse_report(__u8 *start, unsigned size) * done by hand. */ -static __inline__ __s32 snto32(__u32 value, unsigned n) +static s32 snto32(__u32 value, unsigned n) { switch (n) { case 8: return ((__s8)value); @@ -741,9 +741,9 @@ static __inline__ __s32 snto32(__u32 value, unsigned n) * Convert a signed 32-bit integer to a signed n-bit integer. */ -static __inline__ __u32 s32ton(__s32 value, unsigned n) +static u32 s32ton(__s32 value, unsigned n) { - __s32 a = value >> (n - 1); + s32 a = value >> (n - 1); if (a && a != -1) return value < 0 ? 1 << (n - 1) : (1 << (n - 1)) - 1; return value & ((1 << n) - 1); @@ -751,30 +751,55 @@ static __inline__ __u32 s32ton(__s32 value, unsigned n) /* * Extract/implement a data field from/to a little endian report (bit array). + * + * Code sort-of follows HID spec: + * http://www.usb.org/developers/devclass_docs/HID1_11.pdf + * + * While the USB HID spec allows unlimited length bit fields in "report + * descriptors", most devices never use more than 16 bits. + * One model of UPS is claimed to report "LINEV" as a 32-bit field. + * Search linux-kernel and linux-usb-devel archives for "hid-core extract". */ static __inline__ __u32 extract(__u8 *report, unsigned offset, unsigned n) { - u32 x; + u64 x; + + WARN_ON(n > 32); report += offset >> 3; /* adjust byte index */ - offset &= 8 - 1; - x = get_unaligned((u32 *) report); - x = le32_to_cpu(x); - x = (x >> offset) & ((1 << n) - 1); - return x; + offset &= 7; /* now only need bit offset into one byte */ + x = get_unaligned((u64 *) report); + x = le64_to_cpu(x); + x = (x >> offset) & ((1ULL << n) - 1); /* extract bit field */ + return (u32) x; } +/* + * "implement" : set bits in a little endian bit stream. + * Same concepts as "extract" (see comments above). + * The data mangled in the bit stream remains in little endian + * order the whole time. It make more sense to talk about + * endianness of register values by considering a register + * a "cached" copy of the little endiad bit stream. + */ static __inline__ void implement(__u8 *report, unsigned offset, unsigned n, __u32 value) { - u32 x; + u64 x; + u64 m = (1ULL << n) - 1; + + WARN_ON(n > 32); + + WARN_ON(value > m); + value &= m; report += offset >> 3; - offset &= 8 - 1; - x = get_unaligned((u32 *)report); - x &= cpu_to_le32(~((((__u32) 1 << n) - 1) << offset)); - x |= cpu_to_le32(value << offset); - put_unaligned(x,(u32 *)report); + offset &= 7; + + x = get_unaligned((u64 *)report); + x &= cpu_to_le64(~(m << offset)); + x |= cpu_to_le64(((u64) value) << offset); + put_unaligned(x, (u64 *) report); } /*