From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A339DC433FE for ; Wed, 12 Jan 2022 09:43:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1352159AbiALJny (ORCPT ); Wed, 12 Jan 2022 04:43:54 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:53940 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346466AbiALJnx (ORCPT ); Wed, 12 Jan 2022 04:43:53 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1641980632; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2QGd/oMFcr6Ir+/XaEdR55weZ768VSZZbW/LYHrL9+M=; b=Xg6C2URg2AEYJt2fIam9GCduQuQRkwv3aA+NTTi2EL7PxSlr5Kvpv/gDigdzHYNJs7nR8H NLEbBzR5+9604BuJOCZ6YhhBJnCZAy2BbgHBNtv+nQARhnrtzq5l68CJCVxYZvqLxr5dOK Tkhpsaf2CYCwvueVHFV2B8XnDiwjczc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-435-ZWtc7wjgPQaliFxNXStJvA-1; Wed, 12 Jan 2022 04:43:51 -0500 X-MC-Unique: ZWtc7wjgPQaliFxNXStJvA-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9209564083; Wed, 12 Jan 2022 09:43:50 +0000 (UTC) Received: from [10.39.192.199] (unknown [10.39.192.199]) by smtp.corp.redhat.com (Postfix) with ESMTP id A26A754558; Wed, 12 Jan 2022 09:43:49 +0000 (UTC) Message-ID: <92b7b0ec-6764-64d2-2ded-01bff7cc2193@redhat.com> Date: Wed, 12 Jan 2022 10:43:48 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 Subject: Re: [PATCH 05/18] HID: introduce hid_get_feature Content-Language: en-US To: Angela Czubak , Dmitry Torokhov Cc: linux-input@vger.kernel.org, upstream@semihalf.com References: <20211221191743.1893185-1-acz@semihalf.com> <20211221191743.1893185-6-acz@semihalf.com> From: Benjamin Tissoires In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org On 1/10/22 20:43, Angela Czubak wrote: > On Fri, Jan 7, 2022 at 11:01 PM Dmitry Torokhov > wrote: >> >> On Tue, Dec 21, 2021 at 07:17:30PM +0000, Angela Czubak wrote: >>> Move mt_get_feature from hid-multitouch to hid-core as it is a generic >>> function that can be used by other drivers as well. >>> >>> Signed-off-by: Angela Czubak >>> --- >>> drivers/hid/hid-core.c | 39 ++++++++++++++++++++++++++++++++++++ >>> drivers/hid/hid-multitouch.c | 38 +++-------------------------------- >>> include/linux/hid.h | 1 + >>> 3 files changed, 43 insertions(+), 35 deletions(-) >>> >>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c >>> index dbed2524fd47..c11cb7324157 100644 >>> --- a/drivers/hid/hid-core.c >>> +++ b/drivers/hid/hid-core.c >>> @@ -1796,6 +1796,45 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, >>> } >>> EXPORT_SYMBOL_GPL(hid_report_raw_event); >>> >>> +/** >>> + * hid_get_feature - retrieve feature report from device >>> + * >>> + * @hdev: hid device >>> + * @report: hid report to retrieve >>> + */ >>> +void hid_get_feature(struct hid_device *hdev, struct hid_report *report) >> >> If this is a generic API I believe it should return success/error code >> so that users can decide what to do. >> > Does it mean I should also modify hid-multitouch.c so that the return > value is actually checked? Currently it seems to ignore any failures. >> Thanks. Honestly that function is a hack in hid-multitouch. You can replace it by: ``` hid_device_io_start(hid); hid_hw_request(hid, report, HID_REQ_GET_REPORT); hid_hw_wait(hid); hid_device_io_stop(hid); ``` The hack allows to not have to use hid_device_io_{start|stop}(), which is probably not clean. As for the return value, hid_hw_request() can be used as asynchronous, which is why it returns void. However, returning an actual int would definitively be better because some cases are failing silently (like if the device is not io started). Cheers, Benjamin > >> >> -- >> Dmitry