Skip to contentSkip to conversation

Recent change to hid-core.c

DISCUSSION Mark Lord 2 participants 4 messages Dec 15, 2013 – Dec 16, 2013 HID

lore
Share & tools
lore

Mark Lord

author
#0 lore raw

Jiri,

The recent update 08ec2dcc3527a20c619aca2fb36f800908256bac
"Merge branches 'for-3.11/multitouch', 'for-3.11/sony' and 'for-3.11/upstream' into for-linus"
included an unexpected change to the return code handing for ->raw_event() calls.

A HID driver's raw_event() method previously could return these values:
0 --> keep processing.
1 --> no further processing required.
<0 --> error.

Now, "1" and "0" are both treated as "keep processing", so a lower level HID driver has to return a negative error code to achieve the "no further processing required" state.

Was this intentional? Doesn't that have side-effects for some drivers?

Thanks

Signature · 3 lines
Mark Lord
Real-Time Remedies Inc.
[email protected]
#1 lore raw
Quoted 9 lines · Mark Lord

The recent update 08ec2dcc3527a20c619aca2fb36f800908256bac
"Merge branches 'for-3.11/multitouch', 'for-3.11/sony' and 'for-3.11/upstream' into for-linus"
included an unexpected change to the return code handing for ->raw_event() calls.

A HID driver's raw_event() method previously could return these values:

   0 --> keep processing.
   1 --> no further processing required.
   <0 --> error.
Quoted 5 lines

Now, "1" and "0" are both treated as "keep processing", so a lower level HID driver has to return a negative error code to achieve the "no further processing required" state.

Was this intentional? Doesn't that have side-effects for some drivers?

Hi Mark,

this was intentional -- please see commit b1a1442a23 and discussion here:
https://lkml.org/lkml/2013/3/21/591

If you have any other concerns, please let me know.

Signature · 2 lines
Jiri Kosina
SUSE Labs
author
#2 lore raw
Quoted 5 lines · Jiri Kosina
Quoted 3 lines · Mark Lord

The recent update 08ec2dcc3527a20c619aca2fb36f800908256bac
"Merge branches 'for-3.11/multitouch', 'for-3.11/sony' and 'for-3.11/upstream' into for-linus"
included an unexpected change to the return code handing for ->raw_event() calls.

,,

Quoted 1 line
Quoted 1 line

Was this intentional? Doesn't that have side-effects for some drivers?

..

Quoted 2 lines

this was intentional -- please see commit b1a1442a23 and discussion here:
https://lkml.org/lkml/2013/3/21/591

..

Thanks Jiri.
In that case you'll be wanting this patch (unmangled copy attached) as well to clean up a tiny bit of leftover logic in there.

* * * * * * SNIP * * * * * *

Recent updates to raw_event handling resulted in some leftover "dead" logic in hid-core.c::hid_input_report(). Nuke it.

Signed-off-by Mark Lord <[email protected]>
drivers/hid/hid-core.c +1 -3
@@ -1418,10 +1418,8 @@
 
 	if (hdrv && hdrv->raw_event && hid_match_report(hid, report)) {
 		ret = hdrv->raw_event(hid, report, data, size);
-		if (ret < 0) {
-			ret = ret < 0 ? ret : 0;
+		if (ret < 0)
 			goto unlock;
-		}
 	}
 
 	ret = hid_report_raw_event(hid, type, data, size, interrupt);

Recent updates to raw_event handling resulted in some leftover "dead" logic in hid-core.c::hid_input_report(). Nuke it.

Signed-off-by Mark Lord <[email protected]>
drivers/hid/hid-core.c +1 -3
@@ -1418,10 +1418,8 @@
 
 	if (hdrv && hdrv->raw_event && hid_match_report(hid, report)) {
 		ret = hdrv->raw_event(hid, report, data, size);
-		if (ret < 0) {
-			ret = ret < 0 ? ret : 0;
+		if (ret < 0)
 			goto unlock;
-		}
 	}
 
 	ret = hid_report_raw_event(hid, type, data, size, interrupt);
#3 lore raw
Quoted 24 lines · Mark Lord
Quoted 3 lines
Quoted 3 lines

The recent update 08ec2dcc3527a20c619aca2fb36f800908256bac
"Merge branches 'for-3.11/multitouch', 'for-3.11/sony' and 'for-3.11/upstream' into for-linus"
included an unexpected change to the return code handing for ->raw_event() calls.

,,

Quoted 1 line
Quoted 1 line

Was this intentional? Doesn't that have side-effects for some drivers?

..

Quoted 2 lines

this was intentional -- please see commit b1a1442a23 and discussion here:
https://lkml.org/lkml/2013/3/21/591

..

Thanks Jiri.
In that case you'll be wanting this patch (unmangled copy attached) as well to clean up a tiny bit of leftover logic in there.

* * * * * * SNIP * * * * * *

Recent updates to raw_event handling resulted in some leftover "dead" logic in hid-core.c::hid_input_report(). Nuke it.

Signed-off-by Mark Lord <[email protected]>

drivers/hid/hid-core.c line 1424

Quoted 8 lines
drivers/hid/hid-core.c +1 -3
@@ -1418,10 +1418,8 @@
 
 	if (hdrv && hdrv->raw_event && hid_match_report(hid, report)) {
 		ret = hdrv->raw_event(hid, report, data, size);
-		if (ret < 0) {
-			ret = ret < 0 ? ret : 0;
+		if (ret < 0)
 			goto unlock;
-		}

Please see commit 556483e2a ("HID: remove self-assignment from hid_input_report") in Linus' tree.

Signature · 2 lines
Jiri Kosina
SUSE Labs