From patchwork Thu Jul 18 13:21:30 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nestor Lopez Casado X-Patchwork-Id: 2829645 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 5DA45C0AB2 for ; Thu, 18 Jul 2013 13:21:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EE291201DF for ; Thu, 18 Jul 2013 13:21:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6423D201E4 for ; Thu, 18 Jul 2013 13:21:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758607Ab3GRNVz (ORCPT ); Thu, 18 Jul 2013 09:21:55 -0400 Received: from na3sys009aog133.obsmtp.com ([74.125.149.82]:43353 "EHLO na3sys009aog133.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757244Ab3GRNVy (ORCPT ); Thu, 18 Jul 2013 09:21:54 -0400 Received: from mail-ea0-f173.google.com ([209.85.215.173]) (using TLSv1) by na3sys009aob133.postini.com ([74.125.148.12]) with SMTP ID DSNKUefr8J/n4qIw35hrUB7ntU5jmG+qVSby@postini.com; Thu, 18 Jul 2013 06:21:53 PDT Received: by mail-ea0-f173.google.com with SMTP id g15so1749098eak.18 for ; Thu, 18 Jul 2013 06:21:51 -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=ThxoczJp61NPrVN32YDxHOviqciLXhjxRHFKH1/Vuco=; b=NF84Cz8aY0Yj7L9nL4odgnPxYgjsjWeJ1WXUr6euuHN3rXPW2fd0msbn6eacm+aBrX E5Q2X6GmIoefeRuy3VIGqijSLp1J7rsYUbnK7pZS5MS12CUxQvkk+9uPy9Ilq2KUmo7P nxe4Oixatcz21vr+LKrRvInmAT8+lzQoCotHzwDLEYK9sfaPwB6C+6dM4NdeqiStw2j5 CYRdXbQe1nYMfFMWktwRD0N1qPAqRtoJVknZxg7Uhb4TWJcxPDf0UD5MXbovmemioPFt y/swns4msDwCur/a3pjX0CeFSN+bSAhJ6uEWjGsF/o09VSpKADNnAZir3ZVy6MqTnVSr E2kQ== X-Received: by 10.15.32.67 with SMTP id z43mr11471026eeu.24.1374153711743; Thu, 18 Jul 2013 06:21:51 -0700 (PDT) X-Received: by 10.15.32.67 with SMTP id z43mr11471010eeu.24.1374153711583; Thu, 18 Jul 2013 06:21:51 -0700 (PDT) Received: from ubuntu.localdomain ([195.81.139.13]) by mx.google.com with ESMTPSA id n42sm19190981eeh.15.2013.07.18.06.21.49 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 18 Jul 2013 06:21:50 -0700 (PDT) From: Nestor Lopez Casado To: jkosina@suse.cz Cc: benjamin.tissoires@gmail.com, adlr@chromium.org, joseph.salisbury@canonical.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Nestor Lopez Casado Subject: [PATCH 1/2] Revert "Revert "HID: Fix logitech-dj: missing Unifying device issue"" Date: Thu, 18 Jul 2013 06:21:30 -0700 Message-Id: <1374153691-25100-1-git-send-email-nlopezcasad@logitech.com> X-Mailer: git-send-email 1.7.9.5 X-Gm-Message-State: ALoCoQnRBSn7TLu92UJ0irk8XMvpEqZewzDJx1NnfEiqetn9+6yJhuf6s9ulmocJn2GbCeZn0lEofeygdgWfVgTKf05FnEgCImMCXOZf4RlNrHrMqmIcIOMbrcnz/4lbRg7IlXmag1jngGlK5ScGQUDISCJaB29f9RrBMlhSd36hHh4sXkltH8I= Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This reverts commit 8af6c08830b1ae114d1a8b548b1f8b056e068887. This patch re-adds the workaround introduced by 596264082f10dd4 which was reverted by 8af6c08830b1ae114. The original patch 596264 was needed to overcome a situation where the hid-core would drop incoming reports while probe() was being executed. This issue was solved by c849a6143bec520af which added hid_device_io_start() and hid_device_io_stop() that enable a specific hid driver to opt-in for input reports while its probe() is being executed. Commit a9dd22b730857347 modified hid-logitech-dj so as to use the functionality added to hid-core. Having done that, workaround 596264 was no longer necessary and was reverted by 8af6c08. We now encounter a different problem that ends up 'again' thwarting the Unifying receiver enumeration. The problem is time and usb controller dependent. Ocasionally the reports sent to the usb receiver to start the paired devices enumeration fail with -EPIPE and the receiver never gets to enumerate the paired devices. With dcd9006b1b053c7b1c the problem was "hidden" as the call to the usb driver became asynchronous and none was catching the error from the failing URB. As the root cause for this failing SET_REPORT is not understood yet, -possibly a race on the usb controller drivers or a problem with the Unifying receiver- reintroducing this workaround solves the problem. Overall what this workaround does is: If an input report from an unknown device is received, then a (re)enumeration is performed. related bug: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1194649 Signed-off-by: Nestor Lopez Casado --- drivers/hid/hid-logitech-dj.c | 45 +++++++++++++++++++++++++++++++++++++++++ drivers/hid/hid-logitech-dj.h | 1 + 2 files changed, 46 insertions(+) diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index db3192b..0d13389 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -192,6 +192,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) @@ -232,6 +233,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; } @@ -242,6 +244,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", @@ -305,6 +313,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__); @@ -337,6 +346,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__); } } @@ -367,6 +395,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; } @@ -397,6 +431,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; } @@ -444,6 +484,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; @@ -455,6 +499,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 {