No response yet; time to escalate...

On Tue, Jun 28, 2005 at 07:46:46PM -0400, Adam Kropelin wrote:
> Here is a somewhat more direct way of solving the same problem. Please
> consider this for merging to mainline ASAP; the feedback loop described
> below makes apcupsd and possibly other hiddev clients unusable on 
> 2.6.12 kernels today.

--

Currently hid-core follows the same code path for input reports
regardless of whether they are a result of interrupt transfers or
control transfers. That leads to interrupt events erroneously being
reported to hiddev for regular control transfers.

Prior to 2.6.12 the problem was mitigated by the fact that
reporting to hiddev is supressed if the field value has not changed,
which is often the case. Said filtering was removed in 2.6.12-rc1 which
means any input reports fetched via control transfers result in hiddev
interrupt events. This behavior can quickly lead to a feedback loop
where a userspace app, in response to interrupt events, issues control
transfers which in turn create more interrupt events.

This patch prevents input reports that arrive via control transfers from
being reported to hiddev as interrupt events.

Signed-off-by: Adam Kropelin <akropel1@rochester.rr.com>

Index: dmitry/drivers/usb/input/hid-core.c
===================================================================
--- dmitry.orig/drivers/usb/input/hid-core.c	2005-07-05 16:31:05.000000000 +0200
+++ dmitry/drivers/usb/input/hid-core.c	2005-07-05 16:31:15.000000000 +0200
@@ -789,12 +789,12 @@
 	return -1;
 }
 
-static void hid_process_event(struct hid_device *hid, struct hid_field *field, struct hid_usage *usage, __s32 value, struct pt_regs *regs)
+static void hid_process_event(struct hid_device *hid, struct hid_field *field, struct hid_usage *usage, __s32 value, int interrupt, struct pt_regs *regs)
 {
 	hid_dump_input(usage, value);
 	if (hid->claimed & HID_CLAIMED_INPUT)
 		hidinput_hid_event(hid, field, usage, value, regs);
-	if (hid->claimed & HID_CLAIMED_HIDDEV)
+	if (hid->claimed & HID_CLAIMED_HIDDEV && interrupt)
 		hiddev_hid_event(hid, field, usage, value, regs);
 }
 
@@ -804,7 +804,7 @@
  * reporting to the layer).
  */
 
-static void hid_input_field(struct hid_device *hid, struct hid_field *field, __u8 *data, struct pt_regs *regs)
+static void hid_input_field(struct hid_device *hid, struct hid_field *field, __u8 *data, int interrupt, struct pt_regs *regs)
 {
 	unsigned n;
 	unsigned count = field->report_count;
@@ -831,19 +831,19 @@
 	for (n = 0; n < count; n++) {
 
 		if (HID_MAIN_ITEM_VARIABLE & field->flags) {
-			hid_process_event(hid, field, &field->usage[n], value[n], regs);
+			hid_process_event(hid, field, &field->usage[n], value[n], interrupt, regs);
 			continue;
 		}
 
 		if (field->value[n] >= min && field->value[n] <= max
 			&& field->usage[field->value[n] - min].hid
 			&& search(value, field->value[n], count))
-				hid_process_event(hid, field, &field->usage[field->value[n] - min], 0, regs);
+				hid_process_event(hid, field, &field->usage[field->value[n] - min], 0, interrupt, regs);
 
 		if (value[n] >= min && value[n] <= max
 			&& field->usage[value[n] - min].hid
 			&& search(field->value, value[n], count))
-				hid_process_event(hid, field, &field->usage[value[n] - min], 1, regs);
+				hid_process_event(hid, field, &field->usage[value[n] - min], 1, interrupt, regs);
 	}
 
 	memcpy(field->value, value, count * sizeof(__s32));
@@ -851,7 +851,7 @@
 	kfree(value);
 }
 
-static int hid_input_report(int type, struct urb *urb, struct pt_regs *regs)
+static int hid_input_report(int type, struct urb *urb, int interrupt, struct pt_regs *regs)
 {
 	struct hid_device *hid = urb->context;
 	struct hid_report_enum *report_enum = hid->report_enum + type;
@@ -899,7 +899,7 @@
 		hiddev_report_event(hid, report);
 
 	for (n = 0; n < report->maxfield; n++)
-		hid_input_field(hid, report->field[n], data, regs);
+		hid_input_field(hid, report->field[n], data, interrupt, regs);
 
 	if (hid->claimed & HID_CLAIMED_INPUT)
 		hidinput_report_event(hid, report);
@@ -918,7 +918,7 @@
 
 	switch (urb->status) {
 		case 0:			/* success */
-			hid_input_report(HID_INPUT_REPORT, urb, regs);
+			hid_input_report(HID_INPUT_REPORT, urb, 1, regs);
 			break;
 		case -ECONNRESET:	/* unlink */
 		case -ENOENT:
@@ -1142,7 +1142,7 @@
 	switch (urb->status) {
 		case 0:			/* success */
 			if (hid->ctrl[hid->ctrltail].dir == USB_DIR_IN)
-				hid_input_report(hid->ctrl[hid->ctrltail].report->type, urb, regs);
+				hid_input_report(hid->ctrl[hid->ctrltail].report->type, urb, 0, regs);
 		case -ESHUTDOWN:	/* unplug */
 		case -EILSEQ:		/* unplug timectrl on uhci */
 			unplug = 1;