From patchwork Mon Dec 21 20:06:50 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 7897651 Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 447349F32E for ; Mon, 21 Dec 2015 20:06:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0CD9C20524 for ; Mon, 21 Dec 2015 20:06:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BBD92204D2 for ; Mon, 21 Dec 2015 20:06:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751342AbbLUUGz (ORCPT ); Mon, 21 Dec 2015 15:06:55 -0500 Received: from mail-pf0-f171.google.com ([209.85.192.171]:34724 "EHLO mail-pf0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750985AbbLUUGy (ORCPT ); Mon, 21 Dec 2015 15:06:54 -0500 Received: by mail-pf0-f171.google.com with SMTP id u7so47070476pfb.1; Mon, 21 Dec 2015 12:06:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=Pin20rbwfRidTGaDGPvpkbPabtw4Ciigiw3aP1sFEt8=; b=qhQlTbqGxXa6BY81+pGUYxjt3ilkuhy4C0h/hi2Zg7AJZ5J5hDG5eRJKCGIdPgUc2k e42UkjAvVS7ybq9+gv5RTl+kJxFtsseGdjiQ0frazuG7QfHFTTQZ0eZyDuNyc/IgUpEZ ENb5Xj+zTAUuQgHCTY/Bp1jFNB0WOiLWh1abTXKL+9YlGw6rv8v+Q4RLwwt5MdeI5dz2 Af5RymUxlnnSkTyo8yU4vE2Sc8SC7k7+ZkWWCfXZzMQtf9TV7EG0IwzIeGqMSplsXUkd AV4Cz9mUxMzBgkgjtbHNn1JIAojwAfTpkPqwu1s8G0cuS46YczjuZ8eOcjz+KjY/6Xyv sBWg== X-Received: by 10.98.16.22 with SMTP id y22mr29690778pfi.27.1450728414017; Mon, 21 Dec 2015 12:06:54 -0800 (PST) Received: from dtor-ws ([2620:0:1000:1301:857d:afc8:ad96:20e5]) by smtp.gmail.com with ESMTPSA id kk5sm24702835pab.16.2015.12.21.12.06.51 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 21 Dec 2015 12:06:52 -0800 (PST) Date: Mon, 21 Dec 2015 12:06:50 -0800 From: Dmitry Torokhov To: Pavel Rojtberg Cc: Clement Calmels , linux-input@vger.kernel.org, "Pierre-Loup A. Griffais" , linux-kernel@vger.kernel.org Subject: Re: [PATCH] Input: xpad - use LED API when identifying wireless controllers Message-ID: <20151221200650.GB40090@dtor-ws> References: <20151216224408.GA14261@dtor-ws> <20151219221709.5b5947ec@gromit> <20151220075504.GA35575@dtor-ws> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham 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 On Sun, Dec 20, 2015 at 01:49:25PM +0100, Pavel Rojtberg wrote: > 2015-12-20 8:55 GMT+01:00 Dmitry Torokhov : > > On Sat, Dec 19, 2015 at 10:17:09PM +0100, Clement Calmels wrote: > >> On Wed, 16 Dec 2015 14:44:08 -0800 > >> Dmitry Torokhov wrote: > >> > >> > When lighting up the segment identifying wireless controller, Instead > >> > of sending command directly to the controller, let's do it via LED > >> > API (usinf led_set_brightness) so that LED object state is in sync > >> > with controller state and we'll light up the correct segment on > >> > resume as well. > >> > > >> > Signed-off-by: Dmitry Torokhov > >> > --- > >> > > >> > I do not have the hardware so please try this out. > >> > > >> > drivers/input/joystick/xpad.c | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/input/joystick/xpad.c > >> > b/drivers/input/joystick/xpad.c index 36328b3..00a766b 100644 > >> > --- a/drivers/input/joystick/xpad.c > >> > +++ b/drivers/input/joystick/xpad.c > >> > @@ -1118,7 +1118,7 @@ static void xpad_send_led_command(struct > >> > usb_xpad *xpad, int command) */ > >> > static void xpad_identify_controller(struct usb_xpad *xpad) > >> > { > >> > - xpad_send_led_command(xpad, (xpad->pad_nr % 4) + 2); > >> > + led_set_brightness(&xpad->led->led_cdev, (xpad->pad_nr % 4) > >> > + 2); } > >> > > >> > static void xpad_led_set(struct led_classdev *led_cdev, > >> > >> Hi Dimitri, > > > > Hi Clement, > > > >> > >> My hardware: two wireless xpad 360 using a single usb receiver. > >> > >> Power on the first gamepad => light the "1" led. > >> Power on the second gamepad => light the "2" led. > >> > >> Suspend the PC (systemctl suspend): the two gamepads are "disconnected" > >> => blinking circle. > >> > >> Resume the PC, the two gamepads keep blinking but are working (tested > >> with jstest). > >> > >> Power off and on the gamepad => still blinking. > >> Reload (rmmod/modprobe) the xpad module => still blinking. > >> > >> That said, without your patch, the behavior is exactly the same. > >> It seems that the suspend/resume broke the led feature. Even using > >> the /sys/class/leds/xpad0/brigthness sysfs entry does not work. > >> > > > > OK, so the patch does work (the device is still correctly identified), > > but resume requires additional patches. Could you try 'xpad' branch of > > my tree on kernel.org [1] and let me know if it works for you? > at least on my machine your follow up patch was also required which I merged at: > https://github.com/paroj/xpad/tree/dtor > > with this patch the controllers still continue blinking after resume, > however the URB > will still work so they respond to subsequent LED commands triggered > via sysfs (or if they are powered off and on). > > I also verified that a LED command is triggered upon resume - however > the controller ignores that. > Maybe it is sent too early/ in the wrong order. I wonder if below helps this. diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c index 30b2fa8..dd7a2af 100644 --- a/drivers/input/joystick/xpad.c +++ b/drivers/input/joystick/xpad.c @@ -346,9 +346,10 @@ struct usb_xpad { dma_addr_t idata_dma; struct urb *irq_out; /* urb for interrupt out report */ + struct usb_anchor irq_out_anchor; bool irq_out_active; /* we must not use an active URB */ - unsigned char *odata; /* output data */ u8 odata_serial; /* serial number for xbox one protocol */ + unsigned char *odata; /* output data */ dma_addr_t odata_dma; spinlock_t odata_lock; @@ -764,11 +765,13 @@ static int xpad_try_sending_next_out_packet(struct usb_xpad *xpad) int error; if (!xpad->irq_out_active && xpad_prepare_next_out_packet(xpad)) { + usb_anchor_urb(xpad->irq_out, &xpad->irq_out_anchor); error = usb_submit_urb(xpad->irq_out, GFP_ATOMIC); if (error) { dev_err(&xpad->intf->dev, "%s - usb_submit_urb failed with result %d\n", __func__, error); + usb_unanchor_urb(xpad->irq_out); return -EIO; } @@ -811,11 +814,13 @@ static void xpad_irq_out(struct urb *urb) } if (xpad->irq_out_active) { + usb_anchor_urb(urb, &xpad->irq_out_anchor); error = usb_submit_urb(urb, GFP_ATOMIC); if (error) { dev_err(dev, "%s - usb_submit_urb failed with result %d\n", __func__, error); + usb_unanchor_urb(urb); xpad->irq_out_active = false; } } @@ -832,6 +837,8 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad) if (xpad->xtype == XTYPE_UNKNOWN) return 0; + init_usb_anchor(&xpad->irq_out_anchor); + xpad->odata = usb_alloc_coherent(xpad->udev, XPAD_PKT_LEN, GFP_KERNEL, &xpad->odata_dma); if (!xpad->odata) { @@ -864,6 +871,18 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad) fail1: return error; } +static void xpad_stop_output(struct usb_xpad *xpad) +{ + if (xpad->xtype != XTYPE_UNKNOWN) { + if (!usb_wait_anchor_empty_timeout(&xpad->irq_out_anchor, + 5000)) { + dev_warn(&xpad->intf->dev, + "timed out waiting for output URB to complete, killing\n"); + usb_kill_anchored_urbs(&xpad->irq_out_anchor); + } + } +} + static void xpad_deinit_output(struct usb_xpad *xpad) { if (xpad->xtype != XTYPE_UNKNOWN) { @@ -924,7 +943,15 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad) packet->len = 5; packet->pending = true; - retval = usb_submit_urb(xpad->irq_out, GFP_KERNEL) ? -EIO : 0; + usb_anchor_urb(xpad->irq_out, &xpad->irq_out_anchor); + retval = usb_submit_urb(xpad->irq_out, GFP_KERNEL); + if (retval) { + dev_err(&xpad->intf->dev, + "%s - usb_submit_urb failed with result %d\n", + __func__, retval); + usb_unanchor_urb(xpad->irq_out); + retval = -EIO; + } spin_unlock_irqrestore(&xpad->odata_lock, flags); @@ -1514,15 +1541,14 @@ static void xpad_disconnect(struct usb_interface *intf) * Now that both input device and LED device are gone we can * stop output URB. */ - if (xpad->xtype == XTYPE_XBOX360W) - usb_kill_urb(xpad->irq_out); + xpad_stop_output(xpad); + + xpad_deinit_output(xpad); usb_free_urb(xpad->irq_in); usb_free_coherent(xpad->udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma); - xpad_deinit_output(xpad); - kfree(xpad); usb_set_intfdata(intf, NULL); @@ -1548,8 +1574,7 @@ static int xpad_suspend(struct usb_interface *intf, pm_message_t message) mutex_unlock(&input->mutex); } - if (xpad->xtype != XTYPE_UNKNOWN) - usb_kill_urb(xpad->irq_out); + xpad_stop_output(xpad); return 0; }