Re: The return value of hid_input_report and raw_event
From: Jiri Kosina <hidden>
Date: 2014-12-16 09:27:27
On Mon, 15 Dec 2014, Peter Wu wrote:
Hi Jiri and list, The HID core has a hid_input_report function which returns an integer, but all its callers are not really changing their behavior based on the return value. The few^Wonly exception that does not completely ignore the return value is the hid-logitech-dj driver which prints a debugging message. Should this method be changed to return void? The kerneldoc does not document its return value anyway.
Hi Peter, yes, that definitely would be an acceptable cleanup.
quoted hunk ↗ jump to hunk
Confusion 2: the raw_event callback of the hid_driver structure returns an int, but over time non-negative values are ignored by hid_input_report. This happened in the following change: -------------------------------------------------- commit b1a1442a23776756b254b69786848a94d92445ba Author: Jiri Kosina [off-list ref] Date: Mon Jun 3 11:27:48 2013 +0200 HID: core: fix reporting of raw events hdrw->raw event can return three different return value types: - ret < 0 indicates that the hdrv driver found an error while parsing - ret == 0 indicates no error has been encountered, and the driver has processed the report - ret > 0 indicates that there was no parsing error, and the driver hasn't processed the event. Calling hid_report_raw_event() has to be called appropriately so that it reflects what has been done by ->raw_event() callback, otherwise we might updates of the in-kernel structure are lost upon arrival of the report, which is wrong. Reported-and-tested-by: Srinivas Pandruvada [off-list ref] Reported-and-tested-by: Daniel Leung [off-list ref] Signed-off-by: Jiri Kosina [off-list ref]diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index c272078..8f616bd 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c@@ -1293,7 +1293,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i if (hdrv && hdrv->raw_event && hid_match_report(hid, report)) { ret = hdrv->raw_event(hid, report, data, size); - if (ret != 0) { + if (ret < 0) { ret = ret < 0 ? ret : 0; goto unlock; } --------------------------------------------------So now it does not matter whether the raw_event method returns 0 or 1. The documentation says: raw_event and event should return 0 on no action performed, 1 when no further processing should be done and negative on error What does "no further processing" mean? I guess that the intention was to signal from a HID driver to ignore the packet, but that the above change was done to update the state and allow hidraw to pick up the report.
This change was done so that we don't call hid_report_raw_event() when ->raw_event() callback encountered error during parsing (and therefore returned negative value). See this thread https://lkml.org/lkml/2013/4/8/540 for an example. -- Jiri Kosina SUSE Labs