From patchwork Mon Dec 15 22:31:40 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Wu X-Patchwork-Id: 5498051 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 53B64BEEA8 for ; Mon, 15 Dec 2014 22:31:50 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A85AD20A14 for ; Mon, 15 Dec 2014 22:31:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CEF23209F5 for ; Mon, 15 Dec 2014 22:31:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750814AbaLOWbp (ORCPT ); Mon, 15 Dec 2014 17:31:45 -0500 Received: from lekensteyn.nl ([178.21.112.251]:35961 "EHLO lekensteyn.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750796AbaLOWbp (ORCPT ); Mon, 15 Dec 2014 17:31:45 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lekensteyn.nl; s=s2048-2014-q3; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Date:Subject:To:From; bh=uWSAD2Ecd1JhJD0dRpe+oYKZiFHl+8iTHx/7OeUzMrA=; b=C7mGEzk6svMy3qHiRJgzIRudfVHvLo6v1Z4O49diP/9QfkuYo4SPLQ4UG6mJll6Ute2QCBeiDqyBeXL3NZfHAYgqL9RwoYNSV3dAvBY4bOUTs8vGKFoVZnt8EKpVmQOr8Ipa9G2BFHrBeppt6DR0woWe2T98Lw5U58v0/cd1ZZxAueA7DTGalxYx7A1CuT16dzYZlDPs5QZ80iEQuLRbfsKM9gPFgWIpcPjy4LpVangcj6RXt5LQCxGX8CmE9ZChuyaZmGBhFXmW3Em8u4YtnrusrppguSgxzBCaz+bkvVJAWOToGvj+btfcYCDOtqBzcGFNf46QNyU7gNb9i5mVmA==; Received: by lekensteyn.nl with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1Y0eB3-00069g-UA; Mon, 15 Dec 2014 23:31:42 +0100 From: Peter Wu To: Jiri Kosina , linux-input@vger.kernel.org Subject: The return value of hid_input_report and raw_event Date: Mon, 15 Dec 2014 23:31:40 +0100 Message-ID: <7774789.jK4HDCWEI4@al> User-Agent: KMail/4.14.3 (Linux/3.17.0-rc4-custom-00168-g7ec62d4; KDE/4.14.3; x86_64; ; ) MIME-Version: 1.0 X-Spam-Score: 0.0 (/) X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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. 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 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 Reported-and-tested-by: Daniel Leung Signed-off-by: Jiri Kosina -------------------------------------------------- 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. 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; }