From patchwork Sat Apr 13 10:47:47 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Herrmann X-Patchwork-Id: 2440331 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 C8E4D3FD1A for ; Sat, 13 Apr 2013 10:48:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754052Ab3DMKsh (ORCPT ); Sat, 13 Apr 2013 06:48:37 -0400 Received: from mail-ee0-f47.google.com ([74.125.83.47]:59071 "EHLO mail-ee0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753371Ab3DMKsg (ORCPT ); Sat, 13 Apr 2013 06:48:36 -0400 Received: by mail-ee0-f47.google.com with SMTP id t10so1569873eei.34 for ; Sat, 13 Apr 2013 03:48:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=YbtHUY12OUHxdMBtH+xnRKtglH5KgqpKikBCwwcSbvY=; b=p+3Kr6jGtaeP/ZqDUw+/26M1Dz05+/+GQscnDY1ZiaMP16XDmuHAk5U7HUYVcSj3CI b0DWGCr72bLa3dcWaekUAd6qrkgK9JOmjFhuqbUbkRm2XbAA7Tm6VTY57fkzDKsZwKNr XLIMZCpk2GdnXtJOQH/UiVIz+RWyr78te6HKy7Gjfu17TGFHP2+31hzUUhOhzWAW/KA+ HAfa6N4OAig2W3CdcuSF1A+w+jrIlwuxW8I3JcEDmY2iacnsN9QORFnp5pN/g8USKngt ud4jIfJ6QFf7CqddJTbGdmPFDb/XJBM7/KhSmQ8YN2OBnFxRGfb3ksmpEhyIIpwAj9y1 vueQ== X-Received: by 10.15.22.76 with SMTP id e52mr38530063eeu.7.1365850115291; Sat, 13 Apr 2013 03:48:35 -0700 (PDT) Received: from localhost.localdomain (stgt-5f71827d.pool.mediaWays.net. [95.113.130.125]) by mx.google.com with ESMTPS id t4sm15859796eel.0.2013.04.13.03.48.33 (version=TLSv1.2 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sat, 13 Apr 2013 03:48:34 -0700 (PDT) From: David Herrmann To: linux-input@vger.kernel.org Cc: Jiri Kosina , David Herrmann Subject: [PATCH 06/21] HID: wiimote: wake up if output queue failed Date: Sat, 13 Apr 2013 12:47:47 +0200 Message-Id: <1365850082-3585-7-git-send-email-dh.herrmann@gmail.com> X-Mailer: git-send-email 1.8.2.1 In-Reply-To: <1365850082-3585-1-git-send-email-dh.herrmann@gmail.com> References: <1365850082-3585-1-git-send-email-dh.herrmann@gmail.com> Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org Our output queue is asynchronous but synchronous reports may wait for a response to their request. Therefore, wake them up unconditionally if an output report couldn't be sent. But keep the report ID intact so we don't incorrectly assume our request succeeded. Note that the underlying connection is required to be reliable and does retransmission itself. So it is safe to assume that if the transmission fails, the device is in inconsistent state. Hence, we abort every request if any output report fails. No need to verify which report failed. Signed-off-by: David Herrmann --- drivers/hid/hid-wiimote-core.c | 24 +++++++++++++++++++----- drivers/hid/hid-wiimote.h | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c index e47c949..73969d4 100644 --- a/drivers/hid/hid-wiimote-core.c +++ b/drivers/hid/hid-wiimote-core.c @@ -58,11 +58,11 @@ static enum power_supply_property wiimote_battery_props[] = { /* output queue handling */ -static ssize_t wiimote_hid_send(struct hid_device *hdev, __u8 *buffer, - size_t count) +static int wiimote_hid_send(struct hid_device *hdev, __u8 *buffer, + size_t count) { __u8 *buf; - ssize_t ret; + int ret; if (!hdev->hid_output_raw_report) return -ENODEV; @@ -84,14 +84,20 @@ static void wiimote_queue_worker(struct work_struct *work) struct wiimote_data *wdata = container_of(queue, struct wiimote_data, queue); unsigned long flags; + int ret; spin_lock_irqsave(&wdata->queue.lock, flags); while (wdata->queue.head != wdata->queue.tail) { spin_unlock_irqrestore(&wdata->queue.lock, flags); - wiimote_hid_send(wdata->hdev, + ret = wiimote_hid_send(wdata->hdev, wdata->queue.outq[wdata->queue.tail].data, wdata->queue.outq[wdata->queue.tail].size); + if (ret < 0) { + spin_lock_irqsave(&wdata->state.lock, flags); + wiimote_cmd_abort(wdata); + spin_unlock_irqrestore(&wdata->state.lock, flags); + } spin_lock_irqsave(&wdata->queue.lock, flags); wdata->queue.tail = (wdata->queue.tail + 1) % WIIMOTE_BUFSIZE; @@ -108,7 +114,9 @@ static void wiimote_queue(struct wiimote_data *wdata, const __u8 *buffer, if (count > HID_MAX_BUFFER_SIZE) { hid_warn(wdata->hdev, "Sending too large output report\n"); - return; + + spin_lock_irqsave(&wdata->queue.lock, flags); + goto out_error; } /* @@ -134,8 +142,14 @@ static void wiimote_queue(struct wiimote_data *wdata, const __u8 *buffer, wdata->queue.head = newhead; } else { hid_warn(wdata->hdev, "Output queue is full"); + goto out_error; } + goto out_unlock; + +out_error: + wiimote_cmd_abort(wdata); +out_unlock: spin_unlock_irqrestore(&wdata->queue.lock, flags); } diff --git a/drivers/hid/hid-wiimote.h b/drivers/hid/hid-wiimote.h index a3a449a..74b7a47 100644 --- a/drivers/hid/hid-wiimote.h +++ b/drivers/hid/hid-wiimote.h @@ -195,6 +195,16 @@ static inline void wiimote_cmd_complete(struct wiimote_data *wdata) complete(&wdata->state.ready); } +/* requires the state.lock spinlock to be held */ +static inline void wiimote_cmd_abort(struct wiimote_data *wdata) +{ + /* Abort synchronous request by waking up the sleeping caller. But + * reset the state.cmd field to an invalid value so no further event + * handlers will work with it. */ + wdata->state.cmd = WIIPROTO_REQ_MAX; + complete(&wdata->state.ready); +} + static inline int wiimote_cmd_acquire(struct wiimote_data *wdata) { return mutex_lock_interruptible(&wdata->state.sync) ? -ERESTARTSYS : 0; @@ -223,11 +233,17 @@ static inline int wiimote_cmd_wait(struct wiimote_data *wdata) { int ret; + /* The completion acts as implicit memory barrier so we can safely + * assume that state.cmd is set on success/failure and isn't accessed + * by any other thread, anymore. */ + ret = wait_for_completion_interruptible_timeout(&wdata->state.ready, HZ); if (ret < 0) return -ERESTARTSYS; else if (ret == 0) return -EIO; + else if (wdata->state.cmd != WIIPROTO_REQ_NULL) + return -EIO; else return 0; } @@ -236,9 +252,12 @@ static inline int wiimote_cmd_wait_noint(struct wiimote_data *wdata) { unsigned long ret; + /* no locking needed; see wiimote_cmd_wait() */ ret = wait_for_completion_timeout(&wdata->state.ready, HZ); if (!ret) return -EIO; + else if (wdata->state.cmd != WIIPROTO_REQ_NULL) + return -EIO; else return 0; }