From patchwork Mon Dec 21 13:26:31 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mika Westerberg X-Patchwork-Id: 7895891 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.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id F056EBEEE5 for ; Mon, 21 Dec 2015 13:27:10 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D53C82034E for ; Mon, 21 Dec 2015 13:27:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9F0652034F for ; Mon, 21 Dec 2015 13:27:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750866AbbLUN1H (ORCPT ); Mon, 21 Dec 2015 08:27:07 -0500 Received: from mga01.intel.com ([192.55.52.88]:7158 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750785AbbLUN1G (ORCPT ); Mon, 21 Dec 2015 08:27:06 -0500 Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 21 Dec 2015 05:27:05 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,459,1444719600"; d="scan'208";a="866325789" Received: from black.fi.intel.com ([10.237.72.93]) by fmsmga001.fm.intel.com with ESMTP; 21 Dec 2015 05:27:02 -0800 Received: by black.fi.intel.com (Postfix, from userid 1001) id 926C3FD; Mon, 21 Dec 2015 15:26:31 +0200 (EET) From: Mika Westerberg To: Jiri Kosina , Benjamin Tissoires Cc: Nish Aravamudan , Andrew Duggan , Gabriele Mazzotta , Seth Forshee , Dan Carpenter , Linus Torvalds , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Mika Westerberg Subject: [PATCH] HID: i2c-hid: Prevent sending reports from racing with device reset Date: Mon, 21 Dec 2015 15:26:31 +0200 Message-Id: <1450704391-47008-1-git-send-email-mika.westerberg@linux.intel.com> X-Mailer: git-send-email 2.6.4 Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Spam-Status: No, score=-6.9 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 When an i2c-hid device is resumed from system sleep the driver resets the device to be sure it is in known state. The device is expected to issue an interrupt when reset is complete. This reset might take few milliseconds to complete so if the HID driver on top (hid-rmi) starts to set up the device by sending feature reports etc. the device might not issue the reset complete interrupt anymore. Below is what happens to touchpad on Lenovo Yoga 900 during resume from system sleep: [ 24.790951] i2c_hid i2c-SYNA2B29:00: i2c_hid_hwreset [ 24.790973] i2c_hid i2c-SYNA2B29:00: i2c_hid_set_power [ 24.790982] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=22 00 00 08 [ 24.793011] i2c_hid i2c-SYNA2B29:00: resetting... [ 24.793016] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=22 00 00 01 Here i2c-hid sends reset command to the touchpad. [ 24.794012] i2c_hid i2c-SYNA2B29:00: input: 06 00 01 00 00 00 [ 24.794051] i2c_hid i2c-SYNA2B29:00: i2c_hid_set_or_send_report [ 24.794059] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=22 00 3f 03 0f 23 00 04 00 0f 01 Now hid-rmi puts the touchpad to correct mode by sending it a feature report. This makes the touchpad not to issue reset complete interrupt. [ 24.796092] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: waiting... i2c-hid starts to wait for the reset interrupt to trigger which never happens. [ 24.798304] i2c_hid i2c-SYNA2B29:00: i2c_hid_set_or_send_report [ 24.798313] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=25 00 17 00 09 01 42 00 2e 00 19 19 00 10 cc 06 74 04 0f 19 00 00 00 00 00 Yet another output report from hid-rmi driver. [ 29.795630] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: finished. [ 29.795637] i2c_hid i2c-SYNA2B29:00: failed to reset device. After 5 seconds i2c-hid driver times out. [ 29.795642] i2c_hid i2c-SYNA2B29:00: i2c_hid_set_power [ 29.795649] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=22 00 01 08 [ 29.797576] dpm_run_callback(): i2c_hid_resume+0x0/0xb0 returns -61 [ 29.797584] PM: Device i2c-SYNA2B29:00 failed to resume: error -61 After this the touchpad does not work anymore (and also resume itself gets slowed down because of the timeout). Prevent sending of feature/output reports while the device is being reset by adding a mutex which is held during that time. Reported-by: Nish Aravamudan Reported-by: Linus Torvalds Suggested-by: Benjamin Tissoires Signed-off-by: Mika Westerberg Reviewed-by: Benjamin Tissoires --- drivers/hid/i2c-hid/i2c-hid.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index 10bd8e6e4c9c..fc5ef1c25ed4 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -151,6 +151,7 @@ struct i2c_hid { struct i2c_hid_platform_data pdata; bool irq_wake_enabled; + struct mutex reset_lock; }; static int __i2c_hid_command(struct i2c_client *client, @@ -356,9 +357,16 @@ static int i2c_hid_hwreset(struct i2c_client *client) i2c_hid_dbg(ihid, "%s\n", __func__); + /* + * This prevents sending feature reports while the device is + * being reset. Otherwise we may lose the reset complete + * interrupt. + */ + mutex_lock(&ihid->reset_lock); + ret = i2c_hid_set_power(client, I2C_HID_PWR_ON); if (ret) - return ret; + goto out_unlock; i2c_hid_dbg(ihid, "resetting...\n"); @@ -366,10 +374,11 @@ static int i2c_hid_hwreset(struct i2c_client *client) if (ret) { dev_err(&client->dev, "failed to reset device.\n"); i2c_hid_set_power(client, I2C_HID_PWR_SLEEP); - return ret; } - return 0; +out_unlock: + mutex_unlock(&ihid->reset_lock); + return ret; } static void i2c_hid_get_input(struct i2c_hid *ihid) @@ -587,12 +596,15 @@ static int i2c_hid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t count, unsigned char report_type, bool use_data) { struct i2c_client *client = hid->driver_data; + struct i2c_hid *ihid = i2c_get_clientdata(client); int report_id = buf[0]; int ret; if (report_type == HID_INPUT_REPORT) return -EINVAL; + mutex_lock(&ihid->reset_lock); + if (report_id) { buf++; count--; @@ -605,6 +617,8 @@ static int i2c_hid_output_raw_report(struct hid_device *hid, __u8 *buf, if (report_id && ret >= 0) ret++; /* add report_id to the number of transfered bytes */ + mutex_unlock(&ihid->reset_lock); + return ret; } @@ -990,6 +1004,7 @@ static int i2c_hid_probe(struct i2c_client *client, ihid->wHIDDescRegister = cpu_to_le16(hidRegister); init_waitqueue_head(&ihid->wait); + mutex_init(&ihid->reset_lock); /* we need to allocate the command buffer without knowing the maximum * size of the reports. Let's use HID_MIN_BUFFER_SIZE, then we do the