[PATCH] Re: [PATCH] hiddev code and docs cleanup

I took some time to clean up the code a little, and to add the new
calls to the documentation.  See patch below.

--
Paul

Documentation/usb/hiddev.txt

@@ -18,7 +18,7 @@ normalised event interface - see Documentation/input/input.txt
 The data flow for a HID event produced by a device is something like
 the following :
 
- usb.c ---> hid.c  ----> input.c ----> [keyboard/mouse/joystick/event]
+ usb.c ---> hid-core.c  ----> input.c ----> [keyboard/mouse/joystick/event]
                          |
                          |
                           --> hiddev.c ----> POWER / MONITOR CONTROL 
@@ -63,12 +63,18 @@ conveniently linked of http://www.linux-usb.org.
 
 The hiddev API uses a read() interface, and a set of ioctl() calls.
 
+HID devices exchange data with the host computer using data
+bundles called "reports".  Each report is divided into "fields",
+each of which can have one or more "usages".  In the hid-core,
+each one of these usages has a single signed 32 bit value.
 
 read():
-This is the event interface. When the HID device performs an
-interrupt transfer, indicating a change of state, data will be made
-available at the associated hiddev device with the content of a struct
-hiddev_event: 
+This is the event interface.  When the HID device's state changes,
+it performs an interrupt transfer containing a report which contains
+the changed value.  The hid-core.c module parses the report, and
+returns to hiddev.c the individual usages that have changed within
+the report.  In its basic mode, the hiddev will make these individual
+usage changes available to the reader using a struct hiddev_event:
 
        struct hiddev_event {
            unsigned hid;
@@ -78,7 +84,10 @@ hiddev_event:
 containing the HID usage identifier for the status that changed, and
 the value that it was changed to. Note that the structure is defined
 within <linux/hiddev.h>, along with some other useful #defines and
-structures. 
+structures.  The HID usage identifier is a composite of the HID usage
+page shifted to the 16 high order bits ORed with the usage code.  The
+behavior of the read() function can be modified using the HIDIOCSFLAG
+ioctl() described below.
 
 
 ioctl(): 
@@ -108,7 +117,9 @@ HIDIOCINITREPORT - (none)
 Instructs the kernel to retrieve all input and feature report values
 from the device. At this point, all the usage structures will contain
 current values for the device, and will maintain it as the device
-changes.
+changes.  Note that the use of this ioctl is unnecessary in general,
+since later kernels automatically initialize the reports from the
+device at attach time.
 
 HIDIOCGNAME - string (variable length)
 Gets the device name
@@ -121,7 +132,7 @@ INITREPORT).
 HIDIOCSREPORT - struct hiddev_report_info (write)
 Instructs the kernel to send a report to the device. This report can
 be filled in by the user through HIDIOCSUSAGE calls (below) to fill in
-individual usage values in the report beforesending the report in full
+individual usage values in the report before sending the report in full
 to the device. 
 
 HIDIOCGREPORTINFO - struct hiddev_report_info (read/write)
@@ -157,6 +168,25 @@ filled in with the report and field infomation associated with this
 usage if it is found. 
 
 HIDIOCSUSAGE - struct hiddev_usage_ref (write)
-Sets the value of a usage in an output report.
-
-
+Sets the value of a usage in an output report.  The user fills in
+the hiddev_usage_ref structure as above, but additionally fills in
+the value field.
+
+HIDIOCGFLAG - int (read)
+HIDIOCSFLAG - int (write)
+These operations respectively inspect and replace the mode flags
+that influence the read() call above.  The flags are as follows:
+
+    HIDDEV_FLAG_UREF - read() calls will now return 
+        struct hiddev_usage_ref instead of struct hiddev_event.
+        This is a larger structure, but in situations where the
+        device has more than one usage in its reports with the
+        same usage code, this mode serves to resolve such
+        ambiguity.
+
+    HIDDEV_FLAG_REPORT - This flag can only be used in conjunction
+        with HIDDEV_FLAG_UREF.  With this flag set, when the device
+        sends a report, a struct hiddev_usage_ref will be returned
+        to read() filled in with the report_type and report_id, but 
+        with field_index set to FIELD_INDEX_NONE.  This serves as
+        additional notification when the device has sent a report.

drivers/usb/hiddev.c

@@ -341,9 +341,6 @@ static unsigned int hiddev_poll(struct file *file, poll_table *wait)
 	return 0;
 }
 
-#define GET_TIMEOUT 3
-#define SET_TIMEOUT 3
-
 /*
  * "ioctl" file op
  */
@@ -529,36 +526,12 @@ static int hiddev_ioctl(struct inode *inode, struct file *file,
 		return copy_to_user((void *) arg, &uref, sizeof(uref));
 
 	case HIDIOCGUSAGE:
-		if (copy_from_user(&uref, (void *) arg, sizeof(uref)))
-			return -EFAULT;
-
-		if (uref.report_id == HID_REPORT_ID_UNKNOWN) {
-			field = hiddev_lookup_usage(hid, &uref);
-			if (field == NULL)
-				return -EINVAL;
-		} else {
-			rinfo.report_type = uref.report_type;
-			rinfo.report_id = uref.report_id;
-			if ((report = hiddev_lookup_report(hid, &rinfo)) == NULL)
-				return -EINVAL;
-
-			if (uref.field_index >= report->maxfield)
-				return -EINVAL;
-
-			field = report->field[uref.field_index];
-			if (uref.usage_index >= field->maxusage)
-				return -EINVAL;
-		}
-
-		uref.value = field->value[uref.usage_index];
-
-		return copy_to_user((void *) arg, &uref, sizeof(uref));
-
 	case HIDIOCSUSAGE:
 		if (copy_from_user(&uref, (void *) arg, sizeof(uref)))
 			return -EFAULT;
 
-		if (uref.report_type == HID_REPORT_TYPE_INPUT)
+		if (cmd == HIDIOCSUSAGE &&
+		    uref.report_type != HID_REPORT_TYPE_OUTPUT)
 			return -EINVAL;
 
 		if (uref.report_id == HID_REPORT_ID_UNKNOWN) {
@@ -579,7 +552,12 @@ static int hiddev_ioctl(struct inode *inode, struct file *file,
 				return -EINVAL;
 		}
 
+		if (cmd == HIDIOCGUSAGE) {
+			uref.value = field->value[uref.usage_index];
+			return copy_to_user((void *) arg, &uref, sizeof(uref));
+		} else {
 			field->value[uref.usage_index] = uref.value;
+		}
 
 		return 0;
 

include/linux/hiddev.h

@@ -119,6 +119,10 @@ struct hiddev_usage_ref {
 	__s32 value;
 };
 
+/* FIELD_INDEX_NONE is returned in read() data from the kernel when flags
+ * is set to (HIDDEV_FLAG_UREF | HIDDEV_FLAG_REPORT) and a new report has
+ * been sent by the device 
+ */
 #define HID_FIELD_INDEX_NONE 0xffffffff
 
 /*