From patchwork Wed Apr 10 14:54:30 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 10894087 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2845013B5 for ; Wed, 10 Apr 2019 14:55:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 101AE28686 for ; Wed, 10 Apr 2019 14:55:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0472D286A7; Wed, 10 Apr 2019 14:55:27 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham 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 390B628686 for ; Wed, 10 Apr 2019 14:55:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732834AbfDJOzZ (ORCPT ); Wed, 10 Apr 2019 10:55:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54866 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732766AbfDJOzZ (ORCPT ); Wed, 10 Apr 2019 10:55:25 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1D59A3082E4D; Wed, 10 Apr 2019 14:55:25 +0000 (UTC) Received: from shalem.localdomain.com (ovpn-116-249.ams2.redhat.com [10.36.116.249]) by smtp.corp.redhat.com (Postfix) with ESMTP id EA52E16934; Wed, 10 Apr 2019 14:55:23 +0000 (UTC) From: Hans de Goede To: Jiri Kosina , Benjamin Tissoires Cc: Hans de Goede , Nestor Lopez Casado , linux-input@vger.kernel.org Subject: [PATCH v2 08/37] HID: logitech-dj: do not schedule the dj report itself Date: Wed, 10 Apr 2019 16:54:30 +0200 Message-Id: <20190410145459.11430-9-hdegoede@redhat.com> In-Reply-To: <20190410145459.11430-1-hdegoede@redhat.com> References: <20190410145459.11430-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.46]); Wed, 10 Apr 2019 14:55:25 +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 From: Benjamin Tissoires This is a preparatory patch for handling non DJ (HID++ only) receivers, through this module. We can not use the dj_report in the delayed work callback as the HID++ notifications are different both in size and meaning. There should be no functional change. Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-logitech-dj.c | 134 +++++++++++++++++++++------------- 1 file changed, 83 insertions(+), 51 deletions(-) diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index 5a72be1b8d83..74911a0caeb6 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -31,7 +31,7 @@ #include "hid-ids.h" #define DJ_MAX_PAIRED_DEVICES 6 -#define DJ_MAX_NUMBER_NOTIFICATIONS 8 +#define DJ_MAX_NUMBER_NOTIFS 8 #define DJ_RECEIVER_INDEX 0 #define DJ_DEVICE_INDEX_MIN 1 #define DJ_DEVICE_INDEX_MAX 6 @@ -135,6 +135,19 @@ struct dj_device { u8 device_index; }; +#define WORKITEM_TYPE_EMPTY 0 +#define WORKITEM_TYPE_PAIRED 1 +#define WORKITEM_TYPE_UNPAIRED 2 +#define WORKITEM_TYPE_UNKNOWN 255 + +struct dj_workitem { + u8 type; /* WORKITEM_TYPE_* */ + u8 device_index; + u8 quad_id_msb; + u8 quad_id_lsb; + u32 reports_supported; +}; + /* Keyboard descriptor (1) */ static const char kbd_descriptor[] = { 0x05, 0x01, /* USAGE_PAGE (generic Desktop) */ @@ -353,15 +366,15 @@ static struct hid_ll_driver logi_dj_ll_driver; static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev *djrcv_dev); static void logi_dj_recv_destroy_djhid_device(struct dj_receiver_dev *djrcv_dev, - struct dj_report *dj_report) + struct dj_workitem *workitem) { /* Called in delayed work context */ struct dj_device *dj_dev; unsigned long flags; spin_lock_irqsave(&djrcv_dev->lock, flags); - dj_dev = djrcv_dev->paired_dj_devices[dj_report->device_index]; - djrcv_dev->paired_dj_devices[dj_report->device_index] = NULL; + dj_dev = djrcv_dev->paired_dj_devices[workitem->device_index]; + djrcv_dev->paired_dj_devices[workitem->device_index] = NULL; spin_unlock_irqrestore(&djrcv_dev->lock, flags); if (dj_dev != NULL) { @@ -374,26 +387,20 @@ static void logi_dj_recv_destroy_djhid_device(struct dj_receiver_dev *djrcv_dev, } static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, - struct dj_report *dj_report) + struct dj_workitem *workitem) { /* Called in delayed work context */ struct hid_device *djrcv_hdev = djrcv_dev->hdev; struct hid_device *dj_hiddev; struct dj_device *dj_dev; + u8 device_index = workitem->device_index; /* Device index goes from 1 to 6, we need 3 bytes to store the * semicolon, the index, and a null terminator */ unsigned char tmpstr[3]; - if (dj_report->report_params[DEVICE_PAIRED_PARAM_SPFUNCTION] & - SPFUNCTION_DEVICE_LIST_EMPTY) { - dbg_hid("%s: device list is empty\n", __func__); - djrcv_dev->querying_devices = false; - return; - } - - if (djrcv_dev->paired_dj_devices[dj_report->device_index]) { + if (djrcv_dev->paired_dj_devices[device_index]) { /* The device is already known. No need to reallocate it. */ dbg_hid("%s: device is already known\n", __func__); return; @@ -411,10 +418,8 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, dj_hiddev->dev.parent = &djrcv_hdev->dev; dj_hiddev->bus = BUS_USB; dj_hiddev->vendor = djrcv_hdev->vendor; - dj_hiddev->product = - (dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_MSB] - << 8) | - dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_LSB]; + dj_hiddev->product = (workitem->quad_id_msb << 8) | + workitem->quad_id_lsb; snprintf(dj_hiddev->name, sizeof(dj_hiddev->name), "Logitech Unifying Device. Wireless PID:%04x", dj_hiddev->product); @@ -422,7 +427,7 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE; memcpy(dj_hiddev->phys, djrcv_hdev->phys, sizeof(djrcv_hdev->phys)); - snprintf(tmpstr, sizeof(tmpstr), ":%d", dj_report->device_index); + snprintf(tmpstr, sizeof(tmpstr), ":%d", device_index); strlcat(dj_hiddev->phys, tmpstr, sizeof(dj_hiddev->phys)); dj_dev = kzalloc(sizeof(struct dj_device), GFP_KERNEL); @@ -433,14 +438,13 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, goto dj_device_allocate_fail; } - dj_dev->reports_supported = get_unaligned_le32( - dj_report->report_params + DEVICE_PAIRED_RF_REPORT_TYPE); + dj_dev->reports_supported = workitem->reports_supported; dj_dev->hdev = dj_hiddev; dj_dev->dj_receiver_dev = djrcv_dev; - dj_dev->device_index = dj_report->device_index; + dj_dev->device_index = device_index; dj_hiddev->driver_data = dj_dev; - djrcv_dev->paired_dj_devices[dj_report->device_index] = dj_dev; + djrcv_dev->paired_dj_devices[device_index] = dj_dev; if (hid_add_device(dj_hiddev)) { dev_err(&djrcv_hdev->dev, "%s: failed adding dj_device\n", @@ -451,7 +455,7 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, return; hid_add_device_fail: - djrcv_dev->paired_dj_devices[dj_report->device_index] = NULL; + djrcv_dev->paired_dj_devices[device_index] = NULL; kfree(dj_dev); dj_device_allocate_fail: hid_destroy_device(dj_hiddev); @@ -462,7 +466,7 @@ static void delayedwork_callback(struct work_struct *work) struct dj_receiver_dev *djrcv_dev = container_of(work, struct dj_receiver_dev, work); - struct dj_report dj_report; + struct dj_workitem workitem; unsigned long flags; int count; int retval; @@ -471,10 +475,9 @@ static void delayedwork_callback(struct work_struct *work) spin_lock_irqsave(&djrcv_dev->lock, flags); - count = kfifo_out(&djrcv_dev->notif_fifo, &dj_report, - sizeof(struct dj_report)); + count = kfifo_out(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem)); - if (count != sizeof(struct dj_report)) { + if (count != sizeof(workitem)) { dev_err(&djrcv_dev->hdev->dev, "%s: workitem triggered without " "notifications available\n", __func__); spin_unlock_irqrestore(&djrcv_dev->lock, flags); @@ -490,12 +493,54 @@ static void delayedwork_callback(struct work_struct *work) spin_unlock_irqrestore(&djrcv_dev->lock, flags); - switch (dj_report.report_type) { - case REPORT_TYPE_NOTIF_DEVICE_PAIRED: - logi_dj_recv_add_djhid_device(djrcv_dev, &dj_report); + switch (workitem.type) { + case WORKITEM_TYPE_PAIRED: + logi_dj_recv_add_djhid_device(djrcv_dev, &workitem); break; + case WORKITEM_TYPE_UNPAIRED: + logi_dj_recv_destroy_djhid_device(djrcv_dev, &workitem); + break; + case WORKITEM_TYPE_UNKNOWN: + retval = logi_dj_recv_query_paired_devices(djrcv_dev); + if (retval) { + dev_err(&djrcv_dev->hdev->dev, + "%s:logi_dj_recv_query_paired_devices " + "error:%d\n", __func__, retval); + } + break; + case WORKITEM_TYPE_EMPTY: + dbg_hid("%s: device list is empty\n", __func__); + break; + } +} + +static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev, + struct dj_report *dj_report) +{ + /* We are called from atomic context (tasklet && djrcv->lock held) */ + struct dj_workitem workitem = { + .device_index = dj_report->device_index, + }; + + switch (dj_report->report_type) { + case REPORT_TYPE_NOTIF_DEVICE_PAIRED: + workitem.type = WORKITEM_TYPE_PAIRED; + if (dj_report->report_params[DEVICE_PAIRED_PARAM_SPFUNCTION] & + SPFUNCTION_DEVICE_LIST_EMPTY) { + workitem.type = WORKITEM_TYPE_EMPTY; + break; + } + /* fall-through */ case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED: - logi_dj_recv_destroy_djhid_device(djrcv_dev, &dj_report); + workitem.quad_id_msb = + dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_MSB]; + workitem.quad_id_lsb = + dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_LSB]; + workitem.reports_supported = get_unaligned_le32( + dj_report->report_params + + DEVICE_PAIRED_RF_REPORT_TYPE); + if (dj_report->report_type == REPORT_TYPE_NOTIF_DEVICE_UNPAIRED) + workitem.type = WORKITEM_TYPE_UNPAIRED; break; default: /* A normal report (i. e. not belonging to a pair/unpair notification) @@ -505,28 +550,15 @@ static void delayedwork_callback(struct work_struct *work) * to this dj_device never arrived to this driver. The reason is that * hid-core discards all packets coming from a device while probe() is * executing. */ - if (!djrcv_dev->paired_dj_devices[dj_report.device_index]) { - /* ok, we don't know the device, just re-ask the - * receiver for the list of connected devices. */ - retval = logi_dj_recv_query_paired_devices(djrcv_dev); - if (!retval) { - /* everything went fine, so just leave */ - break; - } - dev_err(&djrcv_dev->hdev->dev, - "%s:logi_dj_recv_query_paired_devices " - "error:%d\n", __func__, retval); + if (!djrcv_dev->paired_dj_devices[dj_report->device_index]) { + /* ok, we don't know the device, just re-ask the + * receiver for the list of connected devices in + * the delayed work callback. */ + workitem.type = WORKITEM_TYPE_UNKNOWN; } - dbg_hid("%s: unexpected report type\n", __func__); } -} - -static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev, - struct dj_report *dj_report) -{ - /* We are called from atomic context (tasklet && djrcv->lock held) */ - kfifo_in(&djrcv_dev->notif_fifo, dj_report, sizeof(struct dj_report)); + kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem)); if (schedule_work(&djrcv_dev->work) == 0) { dbg_hid("%s: did not schedule the work item, was already " @@ -1051,7 +1083,7 @@ static int logi_dj_probe(struct hid_device *hdev, INIT_WORK(&djrcv_dev->work, delayedwork_callback); spin_lock_init(&djrcv_dev->lock); if (kfifo_alloc(&djrcv_dev->notif_fifo, - DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct dj_report), + DJ_MAX_NUMBER_NOTIFS * sizeof(struct dj_workitem), GFP_KERNEL)) { dev_err(&hdev->dev, "%s:failed allocating notif_fifo\n", __func__);