From patchwork Sun Nov 25 18:48:28 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew de los Reyes X-Patchwork-Id: 1799541 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id DDAC1DF264 for ; Sun, 25 Nov 2012 18:48:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753263Ab2KYSst (ORCPT ); Sun, 25 Nov 2012 13:48:49 -0500 Received: from mail-ia0-f174.google.com ([209.85.210.174]:51258 "EHLO mail-ia0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753246Ab2KYSss (ORCPT ); Sun, 25 Nov 2012 13:48:48 -0500 Received: by mail-ia0-f174.google.com with SMTP id y25so7516265iay.19 for ; Sun, 25 Nov 2012 10:48:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:from:date:x-google-sender-auth:message-id :subject:to:content-type; bh=fRdQZ5gEoVk4HbxtYVWNXF4AHMp+BLsNP5cuHLpPp4s=; b=J3+g4pkWfsNsEqMIU42INylORl+5hT+yTopCW8ImttFYDFN53Y4m5F2p0gi6U1I5qB f51Umo1BmFPt5z3MWZQhhitW+ZhnfShXPUc+NZb9LhITNwLFfCPFqI8kLEUdHsd9dOJh 7LrhsZj0ynqa9F+2qFV3quSzBrw+448UtE0Zw1GipH4nim91y0l8IE+l8Qte+ObWdExh KmSzqSdv57D9OMoBzvHZsiswreCXa+XhX9aGOOqR7aO+CZ89epVywlYcQBnb3I0oe4mV 6qKtncQj4JpAjlVkHZlxLu+JfBLlw79YLqjqDAwBjySlB/NXhRb1LeP5bYlLLy7f6fip pgPw== Received: by 10.50.151.238 with SMTP id ut14mr11633108igb.72.1353869328301; Sun, 25 Nov 2012 10:48:48 -0800 (PST) MIME-Version: 1.0 Received: by 10.42.154.9 with HTTP; Sun, 25 Nov 2012 10:48:28 -0800 (PST) From: Andrew de los Reyes Date: Sun, 25 Nov 2012 10:48:28 -0800 X-Google-Sender-Auth: 06RT6wxp73coLXPNB5OkfIz3ni4 Message-ID: Subject: [PATCH] HID: Separate struct hid_device's driver_lock into two locks. To: Linux Input , Jiri Kosina Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org Hi Linux-Input, Jiri, Benjamin Tissoires and Nestor Lopez Casado have been helping me to add Linux support for new Logitech Touch Mice (T620, T400). After running into a road-block in hid-core, and solving it with this patch, we thought it was good to show the community and see if this is okay, or if there's a better solution that we've missed. Thanks for your comments, -andrew This patch separates struct hid_device's driver_lock into two. The goal is to allow hid device drivers to receive input during their probe() function call. This is necessary because some drivers need to communicate with the device to determine parameters needed during probe (e.g., size of a multi-touch surface). Historically, three functions used driver_lock: - hid_device_probe: blocks to acquire lock - hid_device_remove: blocks to acquire lock - hid_input_report: if locked returns -EBUSY, else acquires lock This patch adds another lock (driver_input_lock) which is used to block input from occurring. The lock behavior is now: - hid_device_probe: blocks to acq. driver_lock, then driver_input_lock - hid_device_remove: blocks to acq. driver_lock, then driver_input_lock - hid_input_report: if driver_input_lock locked returns -EBUSY, else acquires driver_input_lock This results in no behavior change for existing devices and drivers. However, during a probe() function call in a driver, that driver may now selectively unlock driver_input_lock to let input events come through, then re-lock. --- drivers/hid/hid-core.c | 15 +++++++++++++-- include/linux/hid.h | 3 ++- 2 files changed, 15 insertions(+), 3 deletions(-) struct hid_ll_driver *ll_driver; diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 4da66b4..3eb4398 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1097,7 +1097,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i if (!hid) return -ENODEV; - if (down_trylock(&hid->driver_lock)) + if (down_trylock(&hid->driver_input_lock)) return -EBUSY; if (!hid->driver) { @@ -1150,7 +1150,7 @@ nomem: hid_report_raw_event(hid, type, data, size, interrupt); unlock: - up(&hid->driver_lock); + up(&hid->driver_input_lock); return ret; } EXPORT_SYMBOL_GPL(hid_input_report); @@ -1703,6 +1703,10 @@ static int hid_device_probe(struct device *dev) if (down_interruptible(&hdev->driver_lock)) return -EINTR; + if (down_interruptible(&hdev->driver_input_lock)) { + up(&hdev->driver_lock); + return -EINTR; + } if (!hdev->driver) { id = hid_match_device(hdev, hdrv); @@ -1726,6 +1730,7 @@ static int hid_device_probe(struct device *dev) hdev->driver = NULL; } unlock: + up(&hdev->driver_input_lock); up(&hdev->driver_lock); return ret; } @@ -1737,6 +1742,10 @@ static int hid_device_remove(struct device *dev) if (down_interruptible(&hdev->driver_lock)) return -EINTR; + if (down_interruptible(&hdev->driver_input_lock)) { + up(&hdev->driver_lock); + return -EINTR; + } hdrv = hdev->driver; if (hdrv) { @@ -1747,6 +1756,7 @@ static int hid_device_remove(struct device *dev) hdev->driver = NULL; } + up(&hdev->driver_input_lock); up(&hdev->driver_lock); return 0; } @@ -2126,6 +2136,7 @@ struct hid_device *hid_allocate_device(void) init_waitqueue_head(&hdev->debug_wait); INIT_LIST_HEAD(&hdev->debug_list); sema_init(&hdev->driver_lock, 1); + sema_init(&hdev->driver_input_lock, 1); return hdev; err: diff --git a/include/linux/hid.h b/include/linux/hid.h index 3a95da6..7e9d235 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -481,7 +481,8 @@ struct hid_device { /* device report descriptor */ unsigned country; /* HID country */ struct hid_report_enum report_enum[HID_REPORT_TYPES]; - struct semaphore driver_lock; /* protects the current driver */ + struct semaphore driver_lock; /* protects the current driver, except during input */ + struct semaphore driver_input_lock; /* protects the current driver */ struct device dev; /* device */ struct hid_driver *driver;