From patchwork Thu Mar 9 08:16:06 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Tissoires X-Patchwork-Id: 9612733 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id DDE64604D9 for ; Thu, 9 Mar 2017 08:42:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B9BF62851A for ; Thu, 9 Mar 2017 08:42:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AE535285B5; Thu, 9 Mar 2017 08:42:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2ACB22851A for ; Thu, 9 Mar 2017 08:42:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752794AbdCIImT (ORCPT ); Thu, 9 Mar 2017 03:42:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41178 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752157AbdCIImS (ORCPT ); Thu, 9 Mar 2017 03:42:18 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 61215C04B93D; Thu, 9 Mar 2017 08:16:19 +0000 (UTC) Received: from mail.corp.redhat.com (ovpn-117-32.ams2.redhat.com [10.36.117.32]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v298GFG3017679 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 9 Mar 2017 03:16:18 -0500 Date: Thu, 9 Mar 2017 09:16:06 +0100 From: Benjamin Tissoires To: Tomasz Kramkowski Cc: Jiri Kosina , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/1] HID: clamp input to logical range if no null state Message-ID: <20170309081606.GA28052@mail.corp.redhat.com> References: <20170308215211.24263-1-tk@the-tk.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170308215211.24263-1-tk@the-tk.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 09 Mar 2017 08:16:19 +0000 (UTC) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mar 08 2017 or thereabouts, Tomasz Kramkowski wrote: > This patch fixes an issue in drivers/hid/hid-input.c where values > outside of the logical range are not clamped when "null state" bit of > the input control is not set. > > This was discussed on the lists [1] and this change stems from the fact > due to the ambiguity of the HID specification it might be appropriate to > follow Microsoft's own interpretation of the specification. As noted in > Microsoft's documentation [2] in the section titled "Required HID usages > for digitizers" it is noted that values reported outside the logical > range "will be considered as invalid data and the value will be changed > to the nearest boundary value (logical min/max)." > > This patch fixes an issue where the (1292:4745) Innomedia INNEX > GENESIS/ATARI reports out of range values for its X and Y axis of the > DPad which, due to the null state bit being unset, are forwarded to > userspace as is. Now these values will get clamped to the logical range > before being forwarded to userspace. This device was also used to test > this patch. > > This patch expands on commit 3f3752705dbd ("HID: reject input outside > logical range only if null state is set"). > > [1]: http://lkml.kernel.org/r/20170307131036.GA853@gaia.local > [2]: https://msdn.microsoft.com/en-us/library/windows/hardware/dn672278(v=vs.85).asp > > Signed-off-by: Tomasz Kramkowski > --- > drivers/hid/hid-input.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > index cf8256aac2bd..cf38ff79cfe9 100644 > --- a/drivers/hid/hid-input.c > +++ b/drivers/hid/hid-input.c > @@ -1157,12 +1157,15 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct > * don't specify logical min and max. > */ > if ((field->flags & HID_MAIN_ITEM_VARIABLE) && > - (field->flags & HID_MAIN_ITEM_NULL_STATE) && > (field->logical_minimum < field->logical_maximum) && > (value < field->logical_minimum || > value > field->logical_maximum)) { > - dbg_hid("Ignoring out-of-range value %x\n", value); > - return; > + if (field->flags & HID_MAIN_ITEM_NULL_STATE) { > + dbg_hid("Ignoring out-of-range value %x\n", value); > + return; > + } > + value = value < field->logical_minimum ? > + field->logical_minimum : field->logical_maximum; We have a "clamp()" function in the kernel that does the job directly and which is more readable. Also, this makes testing the out of range values twice. How about: --- Cheers, Benjamin > } > > /* > -- > 2.12.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index cf8256a..781f400 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1150,19 +1150,26 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct /* * Ignore out-of-range values as per HID specification, - * section 5.10 and 6.2.25. + * section 5.10 and 6.2.25, when NULL state bit is present. + * When it's not, clamp the value to match Microsoft's input + * driver as mentioned in "Required HID usages for digitizers": + * https://msdn.microsoft.com/en-us/library/windows/hardware/dn672278(v=vs.85).asp * * The logical_minimum < logical_maximum check is done so that we * don't unintentionally discard values sent by devices which * don't specify logical min and max. */ if ((field->flags & HID_MAIN_ITEM_VARIABLE) && - (field->flags & HID_MAIN_ITEM_NULL_STATE) && - (field->logical_minimum < field->logical_maximum) && - (value < field->logical_minimum || - value > field->logical_maximum)) { - dbg_hid("Ignoring out-of-range value %x\n", value); - return; + (field->logical_minimum < field->logical_maximum)) { + if (!(field->flags & HID_MAIN_ITEM_NULL_STATE)) { + value = clamp(value, + field->logical_minimum, + field->logical_maximum); + } else if (value < field->logical_minimum || + value > field->logical_maximum) { + dbg_hid("Ignoring out-of-range value %x\n", value); + return; + } } /*