From patchwork Fri Sep 21 10:21:34 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nestor Lopez Casado X-Patchwork-Id: 1490961 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id B78963FE65 for ; Fri, 21 Sep 2012 10:22:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754158Ab2IUKWz (ORCPT ); Fri, 21 Sep 2012 06:22:55 -0400 Received: from na3sys009aog107.obsmtp.com ([74.125.149.197]:44872 "EHLO na3sys009aog107.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753483Ab2IUKWy (ORCPT ); Fri, 21 Sep 2012 06:22:54 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]) (using TLSv1) by na3sys009aob107.postini.com ([74.125.148.12]) with SMTP ID DSNKUFw//XAf0/KqnqYGLPQEWJX7kdkZeCBj@postini.com; Fri, 21 Sep 2012 03:22:54 PDT Received: by wibhr14 with SMTP id hr14so1871436wib.1 for ; Fri, 21 Sep 2012 03:22:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:x-gm-message-state; bh=btDqb0HXdOC2t3JqBU9s1hMkjyMGhjSKaqbcYT3aPec=; b=hT85P5eH56XZXuomficdQNSWxKdjZQxIUNUEy74D/nazxk5wO0TQoGUlA6CoKuOOab w0Sbe9UofjG1NmdbNr4ZHIzwvLEh5qA2kD6ZX+RJbe+tLHSkfZPnMETcXQK22mXMsSV7 ko603sE3gc9/N1K8o6a2SO0K4CPuUeb691DIwomqMV/m4yVtcKxlCQwPy7EB8kCrGCaQ T7D2lMVfWcB8SP4i/xTCsx5jISs96Vdaw9sX18Ho99+vf26t12Y7hqpqNucsKuKq4MMn Oq5s4+u7srlY2awYNzw/hvwaw/Ol/vSIgGKy91oxHjr1RF5tb5MrwkBkwaH9zNVqIOUs 2I6w== Received: by 10.180.96.3 with SMTP id do3mr3620793wib.5.1348222971700; Fri, 21 Sep 2012 03:22:51 -0700 (PDT) Received: from localhost.localdomain ([195.81.139.13]) by mx.google.com with ESMTPS id o2sm15860147wiz.11.2012.09.21.03.22.49 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 21 Sep 2012 03:22:51 -0700 (PDT) From: Nestor Lopez Casado To: Jiri Kosina Cc: Benjamin Tissoires , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, 685360@bugs.debian.org, john.ettedgui@gmail.com, barz621@gmail.com, joy@debbugs.entuzijast.net, dh.herrmann@googlemail.com, Nestor Lopez Casado Subject: [PATCH 1/1] HID: Fix missing Unifying device issue Date: Fri, 21 Sep 2012 12:21:34 +0200 Message-Id: <1348222894-6767-1-git-send-email-nlopezcasad@logitech.com> X-Mailer: git-send-email 1.7.5.3 X-Gm-Message-State: ALoCoQn6Z+V1DcD/cvbQ+cdAjjX6e9A1iZ7mAbmKFo4nx8wqX9/DhUpdSkSezl2AydO8XS441Lgw Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org This patch fixes an issue introduced after commit 4ea5454203d991ec After that commit, hid-core silently discards any incoming packet that arrives while any hid driver's probe function is being executed. This broke the enumeration process of hid-logitech-dj, that must receive control packets in-band with the mouse and keyboard packets. Discarding mouse or keyboard data at the very begining is usually fine, but it is not the case for control packets. This patch forces a re-enumeration of the paired devices when a packet arrives that comes from an unknown device. Based on a patch originally written by Benjamin Tissoires. Signed-off-by: Nestor Lopez Casado --- Hello Jiri, David, I suggest we include this patch as this solves the issue with the Unifying devices while we decide the future of the lock in hid-core. Cheers, Nestor drivers/hid/hid-logitech-dj.c | 45 +++++++++++++++++++++++++++++++++++++++++ drivers/hid/hid-logitech-dj.h | 1 + 2 files changed, 46 insertions(+), 0 deletions(-) diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index 4d524b5..9500f2f 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -193,6 +193,7 @@ static struct hid_ll_driver logi_dj_ll_driver; static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf, size_t count, unsigned char report_type); +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) @@ -233,6 +234,7 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, 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; } @@ -243,6 +245,12 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, return; } + if (djrcv_dev->paired_dj_devices[dj_report->device_index]) { + /* The device is already known. No need to reallocate it. */ + dbg_hid("%s: device is already known\n", __func__); + return; + } + dj_hiddev = hid_allocate_device(); if (IS_ERR(dj_hiddev)) { dev_err(&djrcv_hdev->dev, "%s: hid_allocate_device failed\n", @@ -306,6 +314,7 @@ static void delayedwork_callback(struct work_struct *work) struct dj_report dj_report; unsigned long flags; int count; + int retval; dbg_hid("%s\n", __func__); @@ -338,6 +347,25 @@ static void delayedwork_callback(struct work_struct *work) logi_dj_recv_destroy_djhid_device(djrcv_dev, &dj_report); break; default: + /* A normal report (i. e. not belonging to a pair/unpair notification) + * arriving here, means that the report arrived but we did not have a + * paired dj_device associated to the report's device_index, this + * means that the original "device paired" notification corresponding + * 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); + } dbg_hid("%s: unexpected report type\n", __func__); } } @@ -368,6 +396,12 @@ static void logi_dj_recv_forward_null_report(struct dj_receiver_dev *djrcv_dev, if (!djdev) { dbg_hid("djrcv_dev->paired_dj_devices[dj_report->device_index]" " is NULL, index %d\n", dj_report->device_index); + kfifo_in(&djrcv_dev->notif_fifo, dj_report, sizeof(struct dj_report)); + + if (schedule_work(&djrcv_dev->work) == 0) { + dbg_hid("%s: did not schedule the work item, was already " + "queued\n", __func__); + } return; } @@ -398,6 +432,12 @@ static void logi_dj_recv_forward_report(struct dj_receiver_dev *djrcv_dev, if (dj_device == NULL) { dbg_hid("djrcv_dev->paired_dj_devices[dj_report->device_index]" " is NULL, index %d\n", dj_report->device_index); + kfifo_in(&djrcv_dev->notif_fifo, dj_report, sizeof(struct dj_report)); + + if (schedule_work(&djrcv_dev->work) == 0) { + dbg_hid("%s: did not schedule the work item, was already " + "queued\n", __func__); + } return; } @@ -439,6 +479,10 @@ static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev *djrcv_dev) struct dj_report *dj_report; int retval; + /* no need to protect djrcv_dev->querying_devices */ + if (djrcv_dev->querying_devices) + return 0; + dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL); if (!dj_report) return -ENOMEM; @@ -450,6 +494,7 @@ static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev *djrcv_dev) return retval; } + static int logi_dj_recv_switch_to_dj_mode(struct dj_receiver_dev *djrcv_dev, unsigned timeout) { diff --git a/drivers/hid/hid-logitech-dj.h b/drivers/hid/hid-logitech-dj.h index fd28a5e..4a40003 100644 --- a/drivers/hid/hid-logitech-dj.h +++ b/drivers/hid/hid-logitech-dj.h @@ -101,6 +101,7 @@ struct dj_receiver_dev { struct work_struct work; struct kfifo notif_fifo; spinlock_t lock; + bool querying_devices; }; struct dj_device {