From patchwork Sun Jan 18 16:07:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jim Keir X-Patchwork-Id: 5653831 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 F330DC058D for ; Sun, 18 Jan 2015 17:23:50 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D2BD920353 for ; Sun, 18 Jan 2015 17:23:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 051EC201F2 for ; Sun, 18 Jan 2015 17:23:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752559AbbARRXq (ORCPT ); Sun, 18 Jan 2015 12:23:46 -0500 Received: from a8-93.smtp-out.amazonses.com ([54.240.8.93]:45169 "EHLO a8-93.smtp-out.amazonses.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751931AbbARRXp (ORCPT ); Sun, 18 Jan 2015 12:23:45 -0500 X-Greylist: delayed 3579 seconds by postgrey-1.27 at vger.kernel.org; Sun, 18 Jan 2015 12:23:45 EST DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=224i4yxa5dv7c2xz3womw6peuasteono; d=amazonses.com; t=1421597238; h=Message-ID:Date:From:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:Content-Transfer-Encoding:Feedback-ID; bh=u6h7kkllRCUKgyjLJfFWDS2xWYDbxmLvG+TdKGEVhIs=; b=jTL6ZbmwIOhfYrYy/0/hI/s6p2IRm0J2iauNIgdDZvSIM5cPtqmM35NAJiPFGbNk eISELBfCVS4TL120GTAuH1r/ogiHyUMelUOjvzyHH+25/UzmeXS6GAjVFGcBKX/E4Do Pt7jdRn8q8j4RDBGkOcmmyL1S7+Smfzdl7EzqwDg= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=wskikwaygy6dq7o6mewsvwbivuod352d; d=oracledbadirect.com; t=1421597238; h=Message-ID:Date:From:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:Content-Transfer-Encoding; bh=u6h7kkllRCUKgyjLJfFWDS2xWYDbxmLvG+TdKGEVhIs=; b=hJXYy7NiC4fao8KuTqtlLYiIJRyP7jNXAFiCEKucQrPko6asenwnfdrQSbuZUD/i pHfXq/1VVoYoKv9pevZwvk9gubXbW7WvR3gryv9ZxcvsFopqqRua4PizBMisn1VL4rV cx2hNURC2wZiBYgH84UwpiHS+9R6gMROq2r3fz04= X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Message-ID: <0000014afdcc643d-a943b494-4c10-4fa0-a85b-d36e89d6b09a-000000@email.amazonses.com> Date: Sun, 18 Jan 2015 16:07:18 +0000 From: Jim Keir User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: linux-input@vger.kernel.org, linux-usb@vger.kernel.org, jkosina@suse.cz CC: linux-kernel@vger.kernel.org Subject: [PATCH 001/001] usbhid: Fix initialisation and force effect modifications for the Microsoft Sidewinder Force Feedback Pro 2 joystick References: <54BBD8BA.7090906@yahoo.co.uk> In-Reply-To: <54BBD8BA.7090906@yahoo.co.uk> X-Forwarded-Message-Id: <54BBD8BA.7090906@yahoo.co.uk> X-SES-Outgoing: 2015.01.18-54.240.8.93 Feedback-ID: us-east-1.amlta2VpckBvcmFjbGVkYmFkaXJlY3QuY29t:AmazonSES Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Jim Keir Signed-off-by: Jim Keir Currently the SWFF2 driver fails during initialisation, making the force capability of the joystick unusable. Further, there is a long-standing bug in the same driver where commands to update force parameters are addressed to the last-created force effect instead of the specified one, making it impossible to modify effects after their creation. Three bugs are addressed: 1) The FF2 driver (usbhid/hid-pidff.c) sends commands to the stick during ff_init. However, this is called inside a block where driver_input_lock is locked, so the results of these initial commands are discarded. This one is the "killer", without this nothing else works. ff_init issues commands using "hid_hw_request". This eventually goes to hid_input_report, which returns -EBUSY because driver_input_lock is locked. The change is to delay the ff_init call in hid-core.c until after this lock has been released. 2) The usbhid driver ignores an endpoint stall when sending control commands, causing the first few commands of the hid-pidff.c initialisation to get lost. usbhid/hid-core.c has been modified by copying lines into "hid_ctrl" from the "hid_irq_in" function in the same file. 3) The FF2 driver (usbhid/hid-pidff.c) does not set the effect ID when uploading an effect. The result is that the initial upload works but subsequent uploads to modify effect parameters are all directed at the last-created effect. The targeted effect ID must be passed back to the device when effect parameters are changed. This is done at the start of "pidff_set_condition_report", "pidff_set_periodic_report" etc. based on the value of "pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0]". However, this value is only ever set during pidff_request_effect_upload. The result is stored in "pidff->pid_id[effect->id]" at the end of pid_upload_effect, for later use. However, if an effect is modified and re-sent then this identifier is not being copied back from pidff->pid_id[effect->id] before sending the command to the device. The fix is to do this at the start of pidff_upload_effect. This patch taken against kernel 3.13.0 --- -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 905e40a..a608ee6 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1546,9 +1546,8 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask) return -ENODEV; } - if ((hdev->claimed & HID_CLAIMED_INPUT) && - (connect_mask & HID_CONNECT_FF) && hdev->ff_init) - hdev->ff_init(hdev); + /* Removed ff_init() call from here. It does device I/O but this + * is blocked because driver_input_lock is currently locked. */ len = 0; if (hdev->claimed & HID_CLAIMED_INPUT) @@ -2029,6 +2028,13 @@ static int hid_device_probe(struct device *dev) unlock: if (!hdev->io_started) up(&hdev->driver_input_lock); + + if ((hdev->claimed & HID_CLAIMED_INPUT) && hdev->ff_init) { + /* Late init of PID force-feedback drivers moved to after + * unlock of driver_input_lock */ + hdev->ff_init(hdev); + } + unlock_driver_lock: up(&hdev->driver_lock); return ret; diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 029965e..5d34dd7 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -505,7 +505,12 @@ static void hid_ctrl(struct urb *urb) case -EPROTO: /* protocol error or unplug */ case -ECONNRESET: /* unlink */ case -ENOENT: + break; case -EPIPE: /* report not available */ + usbhid_mark_busy(usbhid); + clear_bit(HID_IN_RUNNING, &usbhid->iofl); + set_bit(HID_CLEAR_HALT, &usbhid->iofl); + schedule_work(&usbhid->reset_work); break; default: /* error */ hid_warn(urb->dev, "ctrl urb status %d received\n", status); diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c index 10b6167..3f8ea63 100644 --- a/drivers/hid/usbhid/hid-pidff.c +++ b/drivers/hid/usbhid/hid-pidff.c @@ -568,6 +568,13 @@ static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *effect, int type_id; int error; + pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] = 0; + + if (old && effect) { + pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] = + pidff->pid_id[effect->id]; + } + switch (effect->type) { case FF_CONSTANT: if (!old) { @@ -701,10 +708,14 @@ static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *effect, return -EINVAL; } - if (!old) + if (!old) { pidff->pid_id[effect->id] = pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0]; + hid_dbg(pidff->hid, "Created new effect of type 0x%02x with h/w ID %d, driver ID %d\n", + effect->type, pidff->pid_id[effect->id], effect->id); + } + hid_dbg(pidff->hid, "uploaded\n"); return 0;