On Wed, Oct 18, 2006 at 12:44:30AM -0600, Grant Grundler wrote:
> And I've fixed the algorithm to handle 32-bit extracts in the test
> below and will submit the same code in the next patch.

Greg, 

Previous patch broke extract/implement for 32-bit report fields.

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.

Signed-off-by: Grant Grundler <[EMAIL PROTECTED]>

diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
index 45f44fe..1aada46 100644
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -751,30 +751,55 @@ static __inline__ __u32 s32ton(__s32 val
 
 /*
  * 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);
 }
 
 /*

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to