[PATCH 00/15] Input: xpad: updates

[PATCH 00/15] Input: xpad: updates

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

This series contains several bug-fixes and updates for the xpad driver from
my github repository. See the issue tracker for full discussion of the
individual patches:
https://github.com/paroj/xpad/issues?q=is%3Aissue+is%3Aclosed

Patches 1-2 improve device support

Patches 3-5 are refactorings in preparation for the following changes

Patch 6 makes the x360 wireless pads support the x360 wired button interface.
See the commit message for details and a link to the preceding discussion.

Patches 7-10 implement "on demand" creation/ deletion of input 
devices. To this end pad enumeration had to be changed from an monotonic
counter to a ida() based enumeration. See Patch 10 for rationale.

Patches 11-13 properly handle incoming messages when the irq_out is busy 

Patch 14 fixes issues of the x360 wireless pads after suspend/ resume.

Patch 15 improves Xbox One Force Feedback support

Dario Scarpa (1):
  Input: xpad: fix Razer Atrox Arcade Stick button mapping

Erik Lundgren (1):
  Input: xpad: add Covert Forces edition of the Xbox One controller

Pavel Rojtberg (10):
  Input: xpad: clarify LED enumeration
  Input: xpad: remove needless bulk out URB used for LED setup
  Input: xpad: factor out URB submission in xpad_play_effect
  Input: xpad: x360w: report dpad as buttons and axes
  Input: xpad: query Wireless controller state at init
  Input: xpad: use ida() for finding the pad_nr
  Input: xpad: do not submit active URBs
  Input: xpad: replace mutex by spinlock
  Input: xpad: re-submit pending ff and led requests
  Input: xpad: workaround dead irq_out after suspend/ resume

Pierre-Loup A. Griffais (3):
  Input: xpad: move the input device creation to a new function
  Input: xpad: handle "present" and "gone" correctly
  Input: xpad: update Xbox One Force Feedback Support

 drivers/input/joystick/drivers/input/joystick/xpad.c | 666 ++++++++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 407 insertions(+), 259 deletions(-)

-- 
1.9.1

[PATCH 01/15] Input: xpad: add Covert Forces edition of the Xbox One controller

[Pavel Rojtberg]

From: Erik Lundgren <eriklundgren93@gmail.com>

It is identical to the Xbox One controller but has a different product
ID

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index f8850f9..c3ba5be 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -125,6 +125,7 @@ static const struct xpad_device {
 	{ 0x045e, 0x0289, "Microsoft X-Box pad v2 (US)", 0, XTYPE_XBOX },
 	{ 0x045e, 0x028e, "Microsoft X-Box 360 pad", 0, XTYPE_XBOX360 },
 	{ 0x045e, 0x02d1, "Microsoft X-Box One pad", 0, XTYPE_XBOXONE },
+	{ 0x045e, 0x02dd, "Microsoft X-Box One pad (Covert Forces)", 0, XTYPE_XBOXONE },
 	{ 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
 	{ 0x045e, 0x0719, "Xbox 360 Wireless Receiver", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
 	{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
-- 
1.9.1

Re: [PATCH 01/15] Input: xpad: add Covert Forces edition of the Xbox One controller

[Dmitry Torokhov]

On Thu, Oct 01, 2015 at 10:57:12PM +0200, Pavel Rojtberg wrote:
> From: Erik Lundgren <eriklundgren93@gmail.com>
> 
> It is identical to the Xbox One controller but has a different product
> ID
> 
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>

Applied, thank you.

> ---
>  drivers/input/joystick/xpad.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index f8850f9..c3ba5be 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -125,6 +125,7 @@ static const struct xpad_device {
>  	{ 0x045e, 0x0289, "Microsoft X-Box pad v2 (US)", 0, XTYPE_XBOX },
>  	{ 0x045e, 0x028e, "Microsoft X-Box 360 pad", 0, XTYPE_XBOX360 },
>  	{ 0x045e, 0x02d1, "Microsoft X-Box One pad", 0, XTYPE_XBOXONE },
> +	{ 0x045e, 0x02dd, "Microsoft X-Box One pad (Covert Forces)", 0, XTYPE_XBOXONE },
>  	{ 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
>  	{ 0x045e, 0x0719, "Xbox 360 Wireless Receiver", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
>  	{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
> -- 
> 1.9.1
> 

-- 
Dmitry

[PATCH 02/15] Input: xpad: fix Razer Atrox Arcade Stick button mapping

[Pavel Rojtberg]

From: Dario Scarpa <dario.scarpa@duskzone.it>

The "Razer Atrox Arcade Stick" features 10 buttons,
and two of them (LT/RT) don't work properly.
Change its definition in xpad_device[] (mapping field) to fix.

Signed-off-by: Dario Scarpa <dario.scarpa@duskzone.it>
Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 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 c3ba5be..e3b393c 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -205,7 +205,7 @@ static const struct xpad_device {
 	{ 0x1bad, 0xf900, "Harmonix Xbox 360 Controller", 0, XTYPE_XBOX360 },
 	{ 0x1bad, 0xf901, "Gamestop Xbox 360 Controller", 0, XTYPE_XBOX360 },
 	{ 0x1bad, 0xf903, "Tron Xbox 360 controller", 0, XTYPE_XBOX360 },
-	{ 0x24c6, 0x5000, "Razer Atrox Arcade Stick", 0, XTYPE_XBOX360 },
+	{ 0x24c6, 0x5000, "Razer Atrox Arcade Stick", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOX360 },
 	{ 0x24c6, 0x5300, "PowerA MINI PROEX Controller", 0, XTYPE_XBOX360 },
 	{ 0x24c6, 0x5303, "Xbox Airflo wired controller", 0, XTYPE_XBOX360 },
 	{ 0x24c6, 0x5500, "Hori XBOX 360 EX 2 with Turbo", 0, XTYPE_XBOX360 },
-- 
1.9.1

Re: [PATCH 02/15] Input: xpad: fix Razer Atrox Arcade Stick button mapping

[Dmitry Torokhov]

On Thu, Oct 01, 2015 at 10:57:13PM +0200, Pavel Rojtberg wrote:
> From: Dario Scarpa <dario.scarpa@duskzone.it>
> 
> The "Razer Atrox Arcade Stick" features 10 buttons,
> and two of them (LT/RT) don't work properly.
> Change its definition in xpad_device[] (mapping field) to fix.
> 
> Signed-off-by: Dario Scarpa <dario.scarpa@duskzone.it>
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>

Applied, thank you.

> ---
>  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 c3ba5be..e3b393c 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -205,7 +205,7 @@ static const struct xpad_device {
>  	{ 0x1bad, 0xf900, "Harmonix Xbox 360 Controller", 0, XTYPE_XBOX360 },
>  	{ 0x1bad, 0xf901, "Gamestop Xbox 360 Controller", 0, XTYPE_XBOX360 },
>  	{ 0x1bad, 0xf903, "Tron Xbox 360 controller", 0, XTYPE_XBOX360 },
> -	{ 0x24c6, 0x5000, "Razer Atrox Arcade Stick", 0, XTYPE_XBOX360 },
> +	{ 0x24c6, 0x5000, "Razer Atrox Arcade Stick", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOX360 },
>  	{ 0x24c6, 0x5300, "PowerA MINI PROEX Controller", 0, XTYPE_XBOX360 },
>  	{ 0x24c6, 0x5303, "Xbox Airflo wired controller", 0, XTYPE_XBOX360 },
>  	{ 0x24c6, 0x5500, "Hori XBOX 360 EX 2 with Turbo", 0, XTYPE_XBOX360 },
> -- 
> 1.9.1
> 

-- 
Dmitry

[PATCH 03/15] Input: xpad: clarify LED enumeration

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

this changes helps understanding the code
1. rename led_no -> pad_nr: the number stored there is not the LED Nr -
it gets translated later on to a LED Nr in xpad_identify_controller
2. move all comments regarding xpad_identify_controller to the function
definition to prevent inconsistent documentation.

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index e3b393c..ab62d47 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -345,7 +345,7 @@ struct usb_xpad {
 
 	int mapping;			/* map d-pad to buttons or to axes */
 	int xtype;			/* type of xbox device */
-	unsigned long led_no;		/* led to lit on xbox360 controllers */
+	unsigned long pad_nr;		/* the order x360 pads were attached */
 };
 
 /*
@@ -357,7 +357,6 @@ struct usb_xpad {
  *	The used report descriptor was taken from ITO Takayukis website:
  *	 http://euc.jp/periphs/xbox-controller.ja.html
  */
-
 static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
 {
 	struct input_dev *dev = xpad->dev;
@@ -506,7 +505,6 @@ static void xpad_identify_controller(struct usb_xpad *xpad);
  * 01.1 - Pad state (Bytes 4+) valid
  *
  */
-
 static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
 {
 	/* Presence change */
@@ -514,10 +512,6 @@ static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned cha
 		if (data[1] & 0x80) {
 			xpad->pad_present = 1;
 			usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
-			/*
-			 * Light up the segment corresponding to
-			 * controller number.
-			 */
 			xpad_identify_controller(xpad);
 		} else
 			xpad->pad_present = 0;
@@ -891,6 +885,7 @@ struct xpad_led {
 };
 
 /**
+ * set the LEDs on Xbox360 / Wireless Controllers
  * @param command
  *  0: off
  *  1: all blink, then previous setting
@@ -943,10 +938,12 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
 	mutex_unlock(&xpad->odata_mutex);
 }
 
+/*
+ * Light up the segment corresponding to the pad number on Xbox 360 Controllers
+ */
 static void xpad_identify_controller(struct usb_xpad *xpad)
 {
-	/* Light up the segment corresponding to controller number */
-	xpad_send_led_command(xpad, (xpad->led_no % 4) + 2);
+	xpad_send_led_command(xpad, (xpad->pad_nr % 4) + 2);
 }
 
 static void xpad_led_set(struct led_classdev *led_cdev,
@@ -972,9 +969,9 @@ static int xpad_led_probe(struct usb_xpad *xpad)
 	if (!led)
 		return -ENOMEM;
 
-	xpad->led_no = atomic_inc_return(&led_seq);
+	xpad->pad_nr = atomic_inc_return(&led_seq);
 
-	snprintf(led->name, sizeof(led->name), "xpad%lu", xpad->led_no);
+	snprintf(led->name, sizeof(led->name), "xpad%lu", xpad->pad_nr);
 	led->xpad = xpad;
 
 	led_cdev = &led->led_cdev;
@@ -988,7 +985,6 @@ static int xpad_led_probe(struct usb_xpad *xpad)
 		return error;
 	}
 
-	/* Light up the segment corresponding to controller number */
 	xpad_identify_controller(xpad);
 
 	return 0;
-- 
1.9.1

Re: [PATCH 03/15] Input: xpad: clarify LED enumeration

[Dmitry Torokhov]

On Thu, Oct 01, 2015 at 10:57:14PM +0200, Pavel Rojtberg wrote:
> From: Pavel Rojtberg <rojtberg@gmail.com>
> 
> this changes helps understanding the code
> 1. rename led_no -> pad_nr: the number stored there is not the LED Nr -
> it gets translated later on to a LED Nr in xpad_identify_controller
> 2. move all comments regarding xpad_identify_controller to the function
> definition to prevent inconsistent documentation.

I like the comments at xpad_identify_controller() call sites to remind
me that "identify" means identify it for the user, not for the kernel to
determine what kind of device we are dealing with.

So half-applied.

Thanks.

> 
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
> ---
>  drivers/input/joystick/xpad.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index e3b393c..ab62d47 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -345,7 +345,7 @@ struct usb_xpad {
>  
>  	int mapping;			/* map d-pad to buttons or to axes */
>  	int xtype;			/* type of xbox device */
> -	unsigned long led_no;		/* led to lit on xbox360 controllers */
> +	unsigned long pad_nr;		/* the order x360 pads were attached */
>  };
>  
>  /*
> @@ -357,7 +357,6 @@ struct usb_xpad {
>   *	The used report descriptor was taken from ITO Takayukis website:
>   *	 http://euc.jp/periphs/xbox-controller.ja.html
>   */
> -
>  static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
>  {
>  	struct input_dev *dev = xpad->dev;
> @@ -506,7 +505,6 @@ static void xpad_identify_controller(struct usb_xpad *xpad);
>   * 01.1 - Pad state (Bytes 4+) valid
>   *
>   */
> -
>  static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
>  {
>  	/* Presence change */
> @@ -514,10 +512,6 @@ static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned cha
>  		if (data[1] & 0x80) {
>  			xpad->pad_present = 1;
>  			usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
> -			/*
> -			 * Light up the segment corresponding to
> -			 * controller number.
> -			 */
>  			xpad_identify_controller(xpad);
>  		} else
>  			xpad->pad_present = 0;
> @@ -891,6 +885,7 @@ struct xpad_led {
>  };
>  
>  /**
> + * set the LEDs on Xbox360 / Wireless Controllers
>   * @param command
>   *  0: off
>   *  1: all blink, then previous setting
> @@ -943,10 +938,12 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
>  	mutex_unlock(&xpad->odata_mutex);
>  }
>  
> +/*
> + * Light up the segment corresponding to the pad number on Xbox 360 Controllers
> + */
>  static void xpad_identify_controller(struct usb_xpad *xpad)
>  {
> -	/* Light up the segment corresponding to controller number */
> -	xpad_send_led_command(xpad, (xpad->led_no % 4) + 2);
> +	xpad_send_led_command(xpad, (xpad->pad_nr % 4) + 2);
>  }
>  
>  static void xpad_led_set(struct led_classdev *led_cdev,
> @@ -972,9 +969,9 @@ static int xpad_led_probe(struct usb_xpad *xpad)
>  	if (!led)
>  		return -ENOMEM;
>  
> -	xpad->led_no = atomic_inc_return(&led_seq);
> +	xpad->pad_nr = atomic_inc_return(&led_seq);
>  
> -	snprintf(led->name, sizeof(led->name), "xpad%lu", xpad->led_no);
> +	snprintf(led->name, sizeof(led->name), "xpad%lu", xpad->pad_nr);
>  	led->xpad = xpad;
>  
>  	led_cdev = &led->led_cdev;
> @@ -988,7 +985,6 @@ static int xpad_led_probe(struct usb_xpad *xpad)
>  		return error;
>  	}
>  
> -	/* Light up the segment corresponding to controller number */
>  	xpad_identify_controller(xpad);
>  
>  	return 0;
> -- 
> 1.9.1
> 

-- 
Dmitry

[PATCH 04/15] Input: xpad: remove needless bulk out URB used for LED setup

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

this code was probably wrong ever since and is redundant with
xpad_send_led_command.

Both try to send a similar command to the xbox360 controller. However
xpad_send_led_command correctly uses the pad_nr instead of
bInterfaceNumber to select the led and re-uses the irq_out URB instead
of creating a new one.

Note that this change only affects the two supported wireless
controllers. Tested using the xbox360 wireless controller (PC).

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 79 +-----------------------------------------------------------------
 1 file changed, 1 insertion(+), 78 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index ab62d47..a379346 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -329,9 +329,6 @@ struct usb_xpad {
 	unsigned char *idata;		/* input data */
 	dma_addr_t idata_dma;
 
-	struct urb *bulk_out;
-	unsigned char *bdata;
-
 	struct urb *irq_out;		/* urb for interrupt out report */
 	unsigned char *odata;		/* output data */
 	dma_addr_t odata_dma;
@@ -511,7 +508,6 @@ static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned cha
 	if (data[0] & 0x08) {
 		if (data[1] & 0x80) {
 			xpad->pad_present = 1;
-			usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
 			xpad_identify_controller(xpad);
 		} else
 			xpad->pad_present = 0;
@@ -669,28 +665,6 @@ exit:
 			__func__, retval);
 }
 
-static void xpad_bulk_out(struct urb *urb)
-{
-	struct usb_xpad *xpad = urb->context;
-	struct device *dev = &xpad->intf->dev;
-
-	switch (urb->status) {
-	case 0:
-		/* success */
-		break;
-	case -ECONNRESET:
-	case -ENOENT:
-	case -ESHUTDOWN:
-		/* this urb is terminated, clean up */
-		dev_dbg(dev, "%s - urb shutting down with status: %d\n",
-			__func__, urb->status);
-		break;
-	default:
-		dev_dbg(dev, "%s - nonzero urb status received: %d\n",
-			__func__, urb->status);
-	}
-}
-
 static void xpad_irq_out(struct urb *urb)
 {
 	struct usb_xpad *xpad = urb->context;
@@ -1216,52 +1190,6 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 
 	if (xpad->xtype == XTYPE_XBOX360W) {
 		/*
-		 * Setup the message to set the LEDs on the
-		 * controller when it shows up
-		 */
-		xpad->bulk_out = usb_alloc_urb(0, GFP_KERNEL);
-		if (!xpad->bulk_out) {
-			error = -ENOMEM;
-			goto fail7;
-		}
-
-		xpad->bdata = kzalloc(XPAD_PKT_LEN, GFP_KERNEL);
-		if (!xpad->bdata) {
-			error = -ENOMEM;
-			goto fail8;
-		}
-
-		xpad->bdata[2] = 0x08;
-		switch (intf->cur_altsetting->desc.bInterfaceNumber) {
-		case 0:
-			xpad->bdata[3] = 0x42;
-			break;
-		case 2:
-			xpad->bdata[3] = 0x43;
-			break;
-		case 4:
-			xpad->bdata[3] = 0x44;
-			break;
-		case 6:
-			xpad->bdata[3] = 0x45;
-		}
-
-		ep_irq_in = &intf->cur_altsetting->endpoint[1].desc;
-		if (usb_endpoint_is_bulk_out(ep_irq_in)) {
-			usb_fill_bulk_urb(xpad->bulk_out, udev,
-					  usb_sndbulkpipe(udev,
-							  ep_irq_in->bEndpointAddress),
-					  xpad->bdata, XPAD_PKT_LEN,
-					  xpad_bulk_out, xpad);
-		} else {
-			usb_fill_int_urb(xpad->bulk_out, udev,
-					 usb_sndintpipe(udev,
-							ep_irq_in->bEndpointAddress),
-					 xpad->bdata, XPAD_PKT_LEN,
-					 xpad_bulk_out, xpad, 0);
-		}
-
-		/*
 		 * Submit the int URB immediately rather than waiting for open
 		 * because we get status messages from the device whether
 		 * or not any controllers are attached.  In fact, it's
@@ -1271,13 +1199,11 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		xpad->irq_in->dev = xpad->udev;
 		error = usb_submit_urb(xpad->irq_in, GFP_KERNEL);
 		if (error)
-			goto fail9;
+			goto fail7;
 	}
 
 	return 0;
 
- fail9:	kfree(xpad->bdata);
- fail8:	usb_free_urb(xpad->bulk_out);
  fail7:	input_unregister_device(input_dev);
 	input_dev = NULL;
  fail6:	xpad_led_disconnect(xpad);
@@ -1301,8 +1227,6 @@ static void xpad_disconnect(struct usb_interface *intf)
 	xpad_deinit_output(xpad);
 
 	if (xpad->xtype == XTYPE_XBOX360W) {
-		usb_kill_urb(xpad->bulk_out);
-		usb_free_urb(xpad->bulk_out);
 		usb_kill_urb(xpad->irq_in);
 	}
 
@@ -1310,7 +1234,6 @@ static void xpad_disconnect(struct usb_interface *intf)
 	usb_free_coherent(xpad->udev, XPAD_PKT_LEN,
 			xpad->idata, xpad->idata_dma);
 
-	kfree(xpad->bdata);
 	kfree(xpad);
 
 	usb_set_intfdata(intf, NULL);
-- 
1.9.1

Re: [PATCH 04/15] Input: xpad: remove needless bulk out URB used for LED setup

[Dmitry Torokhov]

On Thu, Oct 01, 2015 at 10:57:15PM +0200, Pavel Rojtberg wrote:
> From: Pavel Rojtberg <rojtberg@gmail.com>
> 
> this code was probably wrong ever since and is redundant with
> xpad_send_led_command.
> 
> Both try to send a similar command to the xbox360 controller. However
> xpad_send_led_command correctly uses the pad_nr instead of
> bInterfaceNumber to select the led and re-uses the irq_out URB instead
> of creating a new one.
> 
> Note that this change only affects the two supported wireless
> controllers. Tested using the xbox360 wireless controller (PC).

Applied, thank you.

> 
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
> ---
>  drivers/input/joystick/xpad.c | 79 +-----------------------------------------------------------------
>  1 file changed, 1 insertion(+), 78 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index ab62d47..a379346 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -329,9 +329,6 @@ struct usb_xpad {
>  	unsigned char *idata;		/* input data */
>  	dma_addr_t idata_dma;
>  
> -	struct urb *bulk_out;
> -	unsigned char *bdata;
> -
>  	struct urb *irq_out;		/* urb for interrupt out report */
>  	unsigned char *odata;		/* output data */
>  	dma_addr_t odata_dma;
> @@ -511,7 +508,6 @@ static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned cha
>  	if (data[0] & 0x08) {
>  		if (data[1] & 0x80) {
>  			xpad->pad_present = 1;
> -			usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
>  			xpad_identify_controller(xpad);
>  		} else
>  			xpad->pad_present = 0;
> @@ -669,28 +665,6 @@ exit:
>  			__func__, retval);
>  }
>  
> -static void xpad_bulk_out(struct urb *urb)
> -{
> -	struct usb_xpad *xpad = urb->context;
> -	struct device *dev = &xpad->intf->dev;
> -
> -	switch (urb->status) {
> -	case 0:
> -		/* success */
> -		break;
> -	case -ECONNRESET:
> -	case -ENOENT:
> -	case -ESHUTDOWN:
> -		/* this urb is terminated, clean up */
> -		dev_dbg(dev, "%s - urb shutting down with status: %d\n",
> -			__func__, urb->status);
> -		break;
> -	default:
> -		dev_dbg(dev, "%s - nonzero urb status received: %d\n",
> -			__func__, urb->status);
> -	}
> -}
> -
>  static void xpad_irq_out(struct urb *urb)
>  {
>  	struct usb_xpad *xpad = urb->context;
> @@ -1216,52 +1190,6 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  
>  	if (xpad->xtype == XTYPE_XBOX360W) {
>  		/*
> -		 * Setup the message to set the LEDs on the
> -		 * controller when it shows up
> -		 */
> -		xpad->bulk_out = usb_alloc_urb(0, GFP_KERNEL);
> -		if (!xpad->bulk_out) {
> -			error = -ENOMEM;
> -			goto fail7;
> -		}
> -
> -		xpad->bdata = kzalloc(XPAD_PKT_LEN, GFP_KERNEL);
> -		if (!xpad->bdata) {
> -			error = -ENOMEM;
> -			goto fail8;
> -		}
> -
> -		xpad->bdata[2] = 0x08;
> -		switch (intf->cur_altsetting->desc.bInterfaceNumber) {
> -		case 0:
> -			xpad->bdata[3] = 0x42;
> -			break;
> -		case 2:
> -			xpad->bdata[3] = 0x43;
> -			break;
> -		case 4:
> -			xpad->bdata[3] = 0x44;
> -			break;
> -		case 6:
> -			xpad->bdata[3] = 0x45;
> -		}
> -
> -		ep_irq_in = &intf->cur_altsetting->endpoint[1].desc;
> -		if (usb_endpoint_is_bulk_out(ep_irq_in)) {
> -			usb_fill_bulk_urb(xpad->bulk_out, udev,
> -					  usb_sndbulkpipe(udev,
> -							  ep_irq_in->bEndpointAddress),
> -					  xpad->bdata, XPAD_PKT_LEN,
> -					  xpad_bulk_out, xpad);
> -		} else {
> -			usb_fill_int_urb(xpad->bulk_out, udev,
> -					 usb_sndintpipe(udev,
> -							ep_irq_in->bEndpointAddress),
> -					 xpad->bdata, XPAD_PKT_LEN,
> -					 xpad_bulk_out, xpad, 0);
> -		}
> -
> -		/*
>  		 * Submit the int URB immediately rather than waiting for open
>  		 * because we get status messages from the device whether
>  		 * or not any controllers are attached.  In fact, it's
> @@ -1271,13 +1199,11 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  		xpad->irq_in->dev = xpad->udev;
>  		error = usb_submit_urb(xpad->irq_in, GFP_KERNEL);
>  		if (error)
> -			goto fail9;
> +			goto fail7;
>  	}
>  
>  	return 0;
>  
> - fail9:	kfree(xpad->bdata);
> - fail8:	usb_free_urb(xpad->bulk_out);
>   fail7:	input_unregister_device(input_dev);
>  	input_dev = NULL;
>   fail6:	xpad_led_disconnect(xpad);
> @@ -1301,8 +1227,6 @@ static void xpad_disconnect(struct usb_interface *intf)
>  	xpad_deinit_output(xpad);
>  
>  	if (xpad->xtype == XTYPE_XBOX360W) {
> -		usb_kill_urb(xpad->bulk_out);
> -		usb_free_urb(xpad->bulk_out);
>  		usb_kill_urb(xpad->irq_in);
>  	}
>  
> @@ -1310,7 +1234,6 @@ static void xpad_disconnect(struct usb_interface *intf)
>  	usb_free_coherent(xpad->udev, XPAD_PKT_LEN,
>  			xpad->idata, xpad->idata_dma);
>  
> -	kfree(xpad->bdata);
>  	kfree(xpad);
>  
>  	usb_set_intfdata(intf, NULL);
> -- 
> 1.9.1
> 

-- 
Dmitry

[PATCH 05/15] Input: xpad: factor out URB submission in xpad_play_effect

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

Move submisson logic to a single point at the end of the function.
This makes it easy to add locking/ queuing code later on.

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 140 ++++++++++++++++++++++++++++++++---------------------------------
 1 file changed, 69 insertions(+), 71 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index a379346..1195dbb 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -759,80 +759,78 @@ static void xpad_deinit_output(struct usb_xpad *xpad)
 static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
 {
 	struct usb_xpad *xpad = input_get_drvdata(dev);
+	__u16 strong;
+	__u16 weak;
 
-	if (effect->type == FF_RUMBLE) {
-		__u16 strong = effect->u.rumble.strong_magnitude;
-		__u16 weak = effect->u.rumble.weak_magnitude;
-
-		switch (xpad->xtype) {
-
-		case XTYPE_XBOX:
-			xpad->odata[0] = 0x00;
-			xpad->odata[1] = 0x06;
-			xpad->odata[2] = 0x00;
-			xpad->odata[3] = strong / 256;	/* left actuator */
-			xpad->odata[4] = 0x00;
-			xpad->odata[5] = weak / 256;	/* right actuator */
-			xpad->irq_out->transfer_buffer_length = 6;
-
-			return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
-
-		case XTYPE_XBOX360:
-			xpad->odata[0] = 0x00;
-			xpad->odata[1] = 0x08;
-			xpad->odata[2] = 0x00;
-			xpad->odata[3] = strong / 256;  /* left actuator? */
-			xpad->odata[4] = weak / 256;	/* right actuator? */
-			xpad->odata[5] = 0x00;
-			xpad->odata[6] = 0x00;
-			xpad->odata[7] = 0x00;
-			xpad->irq_out->transfer_buffer_length = 8;
-
-			return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
-
-		case XTYPE_XBOX360W:
-			xpad->odata[0] = 0x00;
-			xpad->odata[1] = 0x01;
-			xpad->odata[2] = 0x0F;
-			xpad->odata[3] = 0xC0;
-			xpad->odata[4] = 0x00;
-			xpad->odata[5] = strong / 256;
-			xpad->odata[6] = weak / 256;
-			xpad->odata[7] = 0x00;
-			xpad->odata[8] = 0x00;
-			xpad->odata[9] = 0x00;
-			xpad->odata[10] = 0x00;
-			xpad->odata[11] = 0x00;
-			xpad->irq_out->transfer_buffer_length = 12;
-
-			return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
-
-		case XTYPE_XBOXONE:
-			xpad->odata[0] = 0x09; /* activate rumble */
-			xpad->odata[1] = 0x08;
-			xpad->odata[2] = 0x00;
-			xpad->odata[3] = 0x08; /* continuous effect */
-			xpad->odata[4] = 0x00; /* simple rumble mode */
-			xpad->odata[5] = 0x03; /* L and R actuator only */
-			xpad->odata[6] = 0x00; /* TODO: LT actuator */
-			xpad->odata[7] = 0x00; /* TODO: RT actuator */
-			xpad->odata[8] = strong / 256;	/* left actuator */
-			xpad->odata[9] = weak / 256;	/* right actuator */
-			xpad->odata[10] = 0x80;	/* length of pulse */
-			xpad->odata[11] = 0x00;	/* stop period of pulse */
-			xpad->irq_out->transfer_buffer_length = 12;
-
-			return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
-
-		default:
-			dev_dbg(&xpad->dev->dev,
-				"%s - rumble command sent to unsupported xpad type: %d\n",
-				__func__, xpad->xtype);
-			return -1;
-		}
+	if (effect->type != FF_RUMBLE)
+		return 0;
+
+	strong = effect->u.rumble.strong_magnitude;
+	weak = effect->u.rumble.weak_magnitude;
+
+	switch (xpad->xtype) {
+	case XTYPE_XBOX:
+		xpad->odata[0] = 0x00;
+		xpad->odata[1] = 0x06;
+		xpad->odata[2] = 0x00;
+		xpad->odata[3] = strong / 256;	/* left actuator */
+		xpad->odata[4] = 0x00;
+		xpad->odata[5] = weak / 256;	/* right actuator */
+		xpad->irq_out->transfer_buffer_length = 6;
+		break;
+
+	case XTYPE_XBOX360:
+		xpad->odata[0] = 0x00;
+		xpad->odata[1] = 0x08;
+		xpad->odata[2] = 0x00;
+		xpad->odata[3] = strong / 256;  /* left actuator? */
+		xpad->odata[4] = weak / 256;	/* right actuator? */
+		xpad->odata[5] = 0x00;
+		xpad->odata[6] = 0x00;
+		xpad->odata[7] = 0x00;
+		xpad->irq_out->transfer_buffer_length = 8;
+		break;
+
+	case XTYPE_XBOX360W:
+		xpad->odata[0] = 0x00;
+		xpad->odata[1] = 0x01;
+		xpad->odata[2] = 0x0F;
+		xpad->odata[3] = 0xC0;
+		xpad->odata[4] = 0x00;
+		xpad->odata[5] = strong / 256;
+		xpad->odata[6] = weak / 256;
+		xpad->odata[7] = 0x00;
+		xpad->odata[8] = 0x00;
+		xpad->odata[9] = 0x00;
+		xpad->odata[10] = 0x00;
+		xpad->odata[11] = 0x00;
+		xpad->irq_out->transfer_buffer_length = 12;
+		break;
+
+	case XTYPE_XBOXONE:
+		xpad->odata[0] = 0x09; /* activate rumble */
+		xpad->odata[1] = 0x08;
+		xpad->odata[2] = 0x00;
+		xpad->odata[3] = 0x08; /* continuous effect */
+		xpad->odata[4] = 0x00; /* simple rumble mode */
+		xpad->odata[5] = 0x03; /* L and R actuator only */
+		xpad->odata[6] = 0x00; /* TODO: LT actuator */
+		xpad->odata[7] = 0x00; /* TODO: RT actuator */
+		xpad->odata[8] = strong / 256;	/* left actuator */
+		xpad->odata[9] = weak / 256;	/* right actuator */
+		xpad->odata[10] = 0x80;	/* length of pulse */
+		xpad->odata[11] = 0x00;	/* stop period of pulse */
+		xpad->irq_out->transfer_buffer_length = 12;
+		break;
+
+	default:
+		dev_dbg(&xpad->dev->dev,
+			"%s - rumble command sent to unsupported xpad type: %d\n",
+			__func__, xpad->xtype);
+		return -EINVAL;
 	}
 
-	return 0;
+	return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
 }
 
 static int xpad_init_ff(struct usb_xpad *xpad)
-- 
1.9.1

Re: [PATCH 05/15] Input: xpad: factor out URB submission in xpad_play_effect

[Dmitry Torokhov]

On Thu, Oct 01, 2015 at 10:57:16PM +0200, Pavel Rojtberg wrote:
> From: Pavel Rojtberg <rojtberg@gmail.com>
> 
> Move submisson logic to a single point at the end of the function.
> This makes it easy to add locking/ queuing code later on.
> 
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>

Applied, thank you.

> ---
>  drivers/input/joystick/xpad.c | 140 ++++++++++++++++++++++++++++++++---------------------------------
>  1 file changed, 69 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index a379346..1195dbb 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -759,80 +759,78 @@ static void xpad_deinit_output(struct usb_xpad *xpad)
>  static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
>  {
>  	struct usb_xpad *xpad = input_get_drvdata(dev);
> +	__u16 strong;
> +	__u16 weak;
>  
> -	if (effect->type == FF_RUMBLE) {
> -		__u16 strong = effect->u.rumble.strong_magnitude;
> -		__u16 weak = effect->u.rumble.weak_magnitude;
> -
> -		switch (xpad->xtype) {
> -
> -		case XTYPE_XBOX:
> -			xpad->odata[0] = 0x00;
> -			xpad->odata[1] = 0x06;
> -			xpad->odata[2] = 0x00;
> -			xpad->odata[3] = strong / 256;	/* left actuator */
> -			xpad->odata[4] = 0x00;
> -			xpad->odata[5] = weak / 256;	/* right actuator */
> -			xpad->irq_out->transfer_buffer_length = 6;
> -
> -			return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
> -
> -		case XTYPE_XBOX360:
> -			xpad->odata[0] = 0x00;
> -			xpad->odata[1] = 0x08;
> -			xpad->odata[2] = 0x00;
> -			xpad->odata[3] = strong / 256;  /* left actuator? */
> -			xpad->odata[4] = weak / 256;	/* right actuator? */
> -			xpad->odata[5] = 0x00;
> -			xpad->odata[6] = 0x00;
> -			xpad->odata[7] = 0x00;
> -			xpad->irq_out->transfer_buffer_length = 8;
> -
> -			return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
> -
> -		case XTYPE_XBOX360W:
> -			xpad->odata[0] = 0x00;
> -			xpad->odata[1] = 0x01;
> -			xpad->odata[2] = 0x0F;
> -			xpad->odata[3] = 0xC0;
> -			xpad->odata[4] = 0x00;
> -			xpad->odata[5] = strong / 256;
> -			xpad->odata[6] = weak / 256;
> -			xpad->odata[7] = 0x00;
> -			xpad->odata[8] = 0x00;
> -			xpad->odata[9] = 0x00;
> -			xpad->odata[10] = 0x00;
> -			xpad->odata[11] = 0x00;
> -			xpad->irq_out->transfer_buffer_length = 12;
> -
> -			return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
> -
> -		case XTYPE_XBOXONE:
> -			xpad->odata[0] = 0x09; /* activate rumble */
> -			xpad->odata[1] = 0x08;
> -			xpad->odata[2] = 0x00;
> -			xpad->odata[3] = 0x08; /* continuous effect */
> -			xpad->odata[4] = 0x00; /* simple rumble mode */
> -			xpad->odata[5] = 0x03; /* L and R actuator only */
> -			xpad->odata[6] = 0x00; /* TODO: LT actuator */
> -			xpad->odata[7] = 0x00; /* TODO: RT actuator */
> -			xpad->odata[8] = strong / 256;	/* left actuator */
> -			xpad->odata[9] = weak / 256;	/* right actuator */
> -			xpad->odata[10] = 0x80;	/* length of pulse */
> -			xpad->odata[11] = 0x00;	/* stop period of pulse */
> -			xpad->irq_out->transfer_buffer_length = 12;
> -
> -			return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
> -
> -		default:
> -			dev_dbg(&xpad->dev->dev,
> -				"%s - rumble command sent to unsupported xpad type: %d\n",
> -				__func__, xpad->xtype);
> -			return -1;
> -		}
> +	if (effect->type != FF_RUMBLE)
> +		return 0;
> +
> +	strong = effect->u.rumble.strong_magnitude;
> +	weak = effect->u.rumble.weak_magnitude;
> +
> +	switch (xpad->xtype) {
> +	case XTYPE_XBOX:
> +		xpad->odata[0] = 0x00;
> +		xpad->odata[1] = 0x06;
> +		xpad->odata[2] = 0x00;
> +		xpad->odata[3] = strong / 256;	/* left actuator */
> +		xpad->odata[4] = 0x00;
> +		xpad->odata[5] = weak / 256;	/* right actuator */
> +		xpad->irq_out->transfer_buffer_length = 6;
> +		break;
> +
> +	case XTYPE_XBOX360:
> +		xpad->odata[0] = 0x00;
> +		xpad->odata[1] = 0x08;
> +		xpad->odata[2] = 0x00;
> +		xpad->odata[3] = strong / 256;  /* left actuator? */
> +		xpad->odata[4] = weak / 256;	/* right actuator? */
> +		xpad->odata[5] = 0x00;
> +		xpad->odata[6] = 0x00;
> +		xpad->odata[7] = 0x00;
> +		xpad->irq_out->transfer_buffer_length = 8;
> +		break;
> +
> +	case XTYPE_XBOX360W:
> +		xpad->odata[0] = 0x00;
> +		xpad->odata[1] = 0x01;
> +		xpad->odata[2] = 0x0F;
> +		xpad->odata[3] = 0xC0;
> +		xpad->odata[4] = 0x00;
> +		xpad->odata[5] = strong / 256;
> +		xpad->odata[6] = weak / 256;
> +		xpad->odata[7] = 0x00;
> +		xpad->odata[8] = 0x00;
> +		xpad->odata[9] = 0x00;
> +		xpad->odata[10] = 0x00;
> +		xpad->odata[11] = 0x00;
> +		xpad->irq_out->transfer_buffer_length = 12;
> +		break;
> +
> +	case XTYPE_XBOXONE:
> +		xpad->odata[0] = 0x09; /* activate rumble */
> +		xpad->odata[1] = 0x08;
> +		xpad->odata[2] = 0x00;
> +		xpad->odata[3] = 0x08; /* continuous effect */
> +		xpad->odata[4] = 0x00; /* simple rumble mode */
> +		xpad->odata[5] = 0x03; /* L and R actuator only */
> +		xpad->odata[6] = 0x00; /* TODO: LT actuator */
> +		xpad->odata[7] = 0x00; /* TODO: RT actuator */
> +		xpad->odata[8] = strong / 256;	/* left actuator */
> +		xpad->odata[9] = weak / 256;	/* right actuator */
> +		xpad->odata[10] = 0x80;	/* length of pulse */
> +		xpad->odata[11] = 0x00;	/* stop period of pulse */
> +		xpad->irq_out->transfer_buffer_length = 12;
> +		break;
> +
> +	default:
> +		dev_dbg(&xpad->dev->dev,
> +			"%s - rumble command sent to unsupported xpad type: %d\n",
> +			__func__, xpad->xtype);
> +		return -EINVAL;
>  	}
>  
> -	return 0;
> +	return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
>  }
>  
>  static int xpad_init_ff(struct usb_xpad *xpad)
> -- 
> 1.9.1
> 

-- 
Dmitry

[PATCH 06/15] Input: xpad: x360w: report dpad as buttons and axes

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

as discussed here[0], x360w is the only pad that maps dpad_to_button.
This is bad for downstream developers as they have to differ between
x360 and x360w which is not intuitive.

This patch implements the suggested solution of exposing the dpad both
as axes and as buttons. This retains backward compability with software
already dealing with the difference while makes new software work as
expected across x360/ x360w pads.

[0] http://www.spinics.net/lists/linux-input/msg34421.html

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 1195dbb..15da2a3 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -436,7 +436,14 @@ static void xpad360_process_packet(struct usb_xpad *xpad,
 		input_report_key(dev, BTN_TRIGGER_HAPPY2, data[2] & 0x08);
 		input_report_key(dev, BTN_TRIGGER_HAPPY3, data[2] & 0x01);
 		input_report_key(dev, BTN_TRIGGER_HAPPY4, data[2] & 0x02);
-	} else {
+	}
+	/* this should be a simple else block. However historically xbox360w
+	 * has mapped DPAD to buttons while xbox360 did not.
+	 * This made no sense, but now we can not just switch back and have to
+	 * support both behaviors.
+	 */
+	if (!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
+			xpad->xtype == XTYPE_XBOX360W) {
 		input_report_abs(dev, ABS_HAT0X,
 				 !!(data[2] & 0x08) - !!(data[2] & 0x04));
 		input_report_abs(dev, ABS_HAT0Y,
@@ -1144,7 +1151,14 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 	if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
 		for (i = 0; xpad_btn_pad[i] >= 0; i++)
 			__set_bit(xpad_btn_pad[i], input_dev->keybit);
-	} else {
+	}
+	/* this should be a simple else block. However historically xbox360w
+	 * has mapped DPAD to buttons while xbox360 did not.
+	 * This made no sense, but now we can not just switch back and have to
+	 * support both behaviors.
+	 */
+	if(!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
+			xpad->xtype == XTYPE_XBOX360W) {
 		for (i = 0; xpad_abs_pad[i] >= 0; i++)
 			xpad_set_up_abs(input_dev, xpad_abs_pad[i]);
 	}
-- 
1.9.1

Re: [PATCH 06/15] Input: xpad: x360w: report dpad as buttons and axes

[Dmitry Torokhov]

On Thu, Oct 01, 2015 at 10:57:17PM +0200, Pavel Rojtberg wrote:
> From: Pavel Rojtberg <rojtberg@gmail.com>
> 
> as discussed here[0], x360w is the only pad that maps dpad_to_button.
> This is bad for downstream developers as they have to differ between
> x360 and x360w which is not intuitive.
> 
> This patch implements the suggested solution of exposing the dpad both
> as axes and as buttons. This retains backward compability with software
> already dealing with the difference while makes new software work as
> expected across x360/ x360w pads.
> 
> [0] http://www.spinics.net/lists/linux-input/msg34421.html
> 
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>

Applied, thank you.

> ---
>  drivers/input/joystick/xpad.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 1195dbb..15da2a3 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -436,7 +436,14 @@ static void xpad360_process_packet(struct usb_xpad *xpad,
>  		input_report_key(dev, BTN_TRIGGER_HAPPY2, data[2] & 0x08);
>  		input_report_key(dev, BTN_TRIGGER_HAPPY3, data[2] & 0x01);
>  		input_report_key(dev, BTN_TRIGGER_HAPPY4, data[2] & 0x02);
> -	} else {
> +	}
> +	/* this should be a simple else block. However historically xbox360w
> +	 * has mapped DPAD to buttons while xbox360 did not.
> +	 * This made no sense, but now we can not just switch back and have to
> +	 * support both behaviors.
> +	 */
> +	if (!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
> +			xpad->xtype == XTYPE_XBOX360W) {
>  		input_report_abs(dev, ABS_HAT0X,
>  				 !!(data[2] & 0x08) - !!(data[2] & 0x04));
>  		input_report_abs(dev, ABS_HAT0Y,
> @@ -1144,7 +1151,14 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  	if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
>  		for (i = 0; xpad_btn_pad[i] >= 0; i++)
>  			__set_bit(xpad_btn_pad[i], input_dev->keybit);
> -	} else {
> +	}
> +	/* this should be a simple else block. However historically xbox360w
> +	 * has mapped DPAD to buttons while xbox360 did not.
> +	 * This made no sense, but now we can not just switch back and have to
> +	 * support both behaviors.
> +	 */
> +	if(!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
> +			xpad->xtype == XTYPE_XBOX360W) {
>  		for (i = 0; xpad_abs_pad[i] >= 0; i++)
>  			xpad_set_up_abs(input_dev, xpad_abs_pad[i]);
>  	}
> -- 
> 1.9.1
> 

-- 
Dmitry

[PATCH 07/15] Input: xpad: move the input device creation to a new function

[Pavel Rojtberg]

From: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>

To allow us to later create / destroy the input device from the urb
callback, we need to initialize/ deinitialize the input device from a
separate function.  So pull that logic out now to make later patches
more "obvious" as to what they do.

Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 199 +++++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 113 insertions(+), 86 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 15da2a3..43490ea 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -343,6 +343,7 @@ struct usb_xpad {
 	int mapping;			/* map d-pad to buttons or to axes */
 	int xtype;			/* type of xbox device */
 	unsigned long pad_nr;		/* the order x360 pads were attached */
+	const char *name;		/* name of the device */
 };
 
 /*
@@ -1044,11 +1045,101 @@ static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
 	}
 }
 
+static int xpad_init_input(struct usb_xpad *xpad)
+{
+	struct input_dev *input_dev;
+	int i, error;
+
+	input_dev = input_allocate_device();
+	if (!input_dev)
+		return -ENOMEM;
+
+	xpad->dev = input_dev;
+	input_dev->name = xpad->name;
+	input_dev->phys = xpad->phys;
+	usb_to_input_id(xpad->udev, &input_dev->id);
+	input_dev->dev.parent = &xpad->intf->dev;
+
+	input_set_drvdata(input_dev, xpad);
+
+	input_dev->open = xpad_open;
+	input_dev->close = xpad_close;
+
+	input_dev->evbit[0] = BIT_MASK(EV_KEY);
+
+	if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
+		input_dev->evbit[0] |= BIT_MASK(EV_ABS);
+		/* set up axes */
+		for (i = 0; xpad_abs[i] >= 0; i++)
+			xpad_set_up_abs(input_dev, xpad_abs[i]);
+	}
+
+	/* set up standard buttons */
+	for (i = 0; xpad_common_btn[i] >= 0; i++)
+		__set_bit(xpad_common_btn[i], input_dev->keybit);
+
+	/* set up model-specific ones */
+	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX360W ||
+	    xpad->xtype == XTYPE_XBOXONE) {
+		for (i = 0; xpad360_btn[i] >= 0; i++)
+			__set_bit(xpad360_btn[i], input_dev->keybit);
+	} else {
+		for (i = 0; xpad_btn[i] >= 0; i++)
+			__set_bit(xpad_btn[i], input_dev->keybit);
+	}
+
+	if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
+		for (i = 0; xpad_btn_pad[i] >= 0; i++)
+			__set_bit(xpad_btn_pad[i], input_dev->keybit);
+	}
+	/* this should be a simple else block. However historically xbox360w
+	 * has mapped DPAD to buttons while xbox360 did not.
+	 * This made no sense, but now we can not just switch back and have to
+	 * support both behaviors.
+	 */
+	if(!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
+			xpad->xtype == XTYPE_XBOX360W) {
+		for (i = 0; xpad_abs_pad[i] >= 0; i++)
+			xpad_set_up_abs(input_dev, xpad_abs_pad[i]);
+	}
+
+	if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
+		for (i = 0; xpad_btn_triggers[i] >= 0; i++)
+			__set_bit(xpad_btn_triggers[i], input_dev->keybit);
+	} else {
+		for (i = 0; xpad_abs_triggers[i] >= 0; i++)
+			xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
+	}
+
+	error = xpad_init_ff(xpad);
+	if (error)
+		goto fail_init_ff;
+
+	error = xpad_led_probe(xpad);
+	if (error)
+		goto fail_init_led;
+
+	error = input_register_device(xpad->dev);
+	if (error)
+		goto fail_input_register;
+
+	return 0;
+
+fail_input_register:
+	xpad_led_disconnect(xpad);
+
+fail_init_led:
+	input_ff_destroy(input_dev);
+
+fail_init_ff:
+	input_free_device(input_dev);
+	return error;
+}
+
 static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct usb_xpad *xpad;
-	struct input_dev *input_dev;
 	struct usb_endpoint_descriptor *ep_irq_in;
 	int ep_irq_in_idx;
 	int i, error;
@@ -1070,12 +1161,14 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 	}
 
 	xpad = kzalloc(sizeof(struct usb_xpad), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!xpad || !input_dev) {
+	if (!xpad) {
 		error = -ENOMEM;
 		goto fail1;
 	}
 
+	usb_make_path(udev, xpad->phys, sizeof(xpad->phys));
+	strlcat(xpad->phys, "/input0", sizeof(xpad->phys));
+
 	xpad->idata = usb_alloc_coherent(udev, XPAD_PKT_LEN,
 					 GFP_KERNEL, &xpad->idata_dma);
 	if (!xpad->idata) {
@@ -1093,6 +1186,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 	xpad->intf = intf;
 	xpad->mapping = xpad_device[i].mapping;
 	xpad->xtype = xpad_device[i].xtype;
+	xpad->name = xpad_device[i].name;
 
 	if (xpad->xtype == XTYPE_UNKNOWN) {
 		if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
@@ -1111,78 +1205,10 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 			xpad->mapping |= MAP_STICKS_TO_NULL;
 	}
 
-	xpad->dev = input_dev;
-	usb_make_path(udev, xpad->phys, sizeof(xpad->phys));
-	strlcat(xpad->phys, "/input0", sizeof(xpad->phys));
-
-	input_dev->name = xpad_device[i].name;
-	input_dev->phys = xpad->phys;
-	usb_to_input_id(udev, &input_dev->id);
-	input_dev->dev.parent = &intf->dev;
-
-	input_set_drvdata(input_dev, xpad);
-
-	input_dev->open = xpad_open;
-	input_dev->close = xpad_close;
-
-	input_dev->evbit[0] = BIT_MASK(EV_KEY);
-
-	if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
-		input_dev->evbit[0] |= BIT_MASK(EV_ABS);
-		/* set up axes */
-		for (i = 0; xpad_abs[i] >= 0; i++)
-			xpad_set_up_abs(input_dev, xpad_abs[i]);
-	}
-
-	/* set up standard buttons */
-	for (i = 0; xpad_common_btn[i] >= 0; i++)
-		__set_bit(xpad_common_btn[i], input_dev->keybit);
-
-	/* set up model-specific ones */
-	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX360W ||
-	    xpad->xtype == XTYPE_XBOXONE) {
-		for (i = 0; xpad360_btn[i] >= 0; i++)
-			__set_bit(xpad360_btn[i], input_dev->keybit);
-	} else {
-		for (i = 0; xpad_btn[i] >= 0; i++)
-			__set_bit(xpad_btn[i], input_dev->keybit);
-	}
-
-	if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
-		for (i = 0; xpad_btn_pad[i] >= 0; i++)
-			__set_bit(xpad_btn_pad[i], input_dev->keybit);
-	}
-	/* this should be a simple else block. However historically xbox360w
-	 * has mapped DPAD to buttons while xbox360 did not.
-	 * This made no sense, but now we can not just switch back and have to
-	 * support both behaviors.
-	 */
-	if(!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
-			xpad->xtype == XTYPE_XBOX360W) {
-		for (i = 0; xpad_abs_pad[i] >= 0; i++)
-			xpad_set_up_abs(input_dev, xpad_abs_pad[i]);
-	}
-
-	if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
-		for (i = 0; xpad_btn_triggers[i] >= 0; i++)
-			__set_bit(xpad_btn_triggers[i], input_dev->keybit);
-	} else {
-		for (i = 0; xpad_abs_triggers[i] >= 0; i++)
-			xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
-	}
-
 	error = xpad_init_output(intf, xpad);
 	if (error)
 		goto fail3;
 
-	error = xpad_init_ff(xpad);
-	if (error)
-		goto fail4;
-
-	error = xpad_led_probe(xpad);
-	if (error)
-		goto fail5;
-
 	/* Xbox One controller has in/out endpoints swapped. */
 	ep_irq_in_idx = xpad->xtype == XTYPE_XBOXONE ? 1 : 0;
 	ep_irq_in = &intf->cur_altsetting->endpoint[ep_irq_in_idx].desc;
@@ -1194,10 +1220,6 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 	xpad->irq_in->transfer_dma = xpad->idata_dma;
 	xpad->irq_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 
-	error = input_register_device(xpad->dev);
-	if (error)
-		goto fail6;
-
 	usb_set_intfdata(intf, xpad);
 
 	if (xpad->xtype == XTYPE_XBOX360W) {
@@ -1210,32 +1232,37 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		 */
 		xpad->irq_in->dev = xpad->udev;
 		error = usb_submit_urb(xpad->irq_in, GFP_KERNEL);
-		if (error)
-			goto fail7;
+		if (error) {
+			usb_kill_urb(xpad->irq_in);
+			goto fail4;
+		}
 	}
+	xpad->pad_present = 1;
+	error = xpad_init_input(xpad);
+	if (error)
+		goto fail4;
 
 	return 0;
 
- fail7:	input_unregister_device(input_dev);
-	input_dev = NULL;
- fail6:	xpad_led_disconnect(xpad);
- fail5:	if (input_dev)
-		input_ff_destroy(input_dev);
  fail4:	xpad_deinit_output(xpad);
  fail3:	usb_free_urb(xpad->irq_in);
  fail2:	usb_free_coherent(udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma);
- fail1:	input_free_device(input_dev);
-	kfree(xpad);
+ fail1:	kfree(xpad);
 	return error;
 
 }
 
+static void xpad_deinit_input(struct usb_xpad *xpad)
+{
+	xpad_led_disconnect(xpad);
+	input_unregister_device(xpad->dev);
+}
+
 static void xpad_disconnect(struct usb_interface *intf)
 {
 	struct usb_xpad *xpad = usb_get_intfdata (intf);
 
-	xpad_led_disconnect(xpad);
-	input_unregister_device(xpad->dev);
+	xpad_deinit_input(xpad);
 	xpad_deinit_output(xpad);
 
 	if (xpad->xtype == XTYPE_XBOX360W) {
-- 
1.9.1

Re: [PATCH 07/15] Input: xpad: move the input device creation to a new function

[Dmitry Torokhov]

On Thu, Oct 01, 2015 at 10:57:18PM +0200, Pavel Rojtberg wrote:
> From: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
> 
> To allow us to later create / destroy the input device from the urb
> callback, we need to initialize/ deinitialize the input device from a
> separate function.  So pull that logic out now to make later patches
> more "obvious" as to what they do.
> 
> Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
> ---
>  drivers/input/joystick/xpad.c | 199 +++++++++++++++++++++++++++++++++++++----------------------------
>  1 file changed, 113 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 15da2a3..43490ea 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -343,6 +343,7 @@ struct usb_xpad {
>  	int mapping;			/* map d-pad to buttons or to axes */
>  	int xtype;			/* type of xbox device */
>  	unsigned long pad_nr;		/* the order x360 pads were attached */
> +	const char *name;		/* name of the device */
>  };
>  
>  /*
> @@ -1044,11 +1045,101 @@ static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
>  	}
>  }
>  
> +static int xpad_init_input(struct usb_xpad *xpad)
> +{
> +	struct input_dev *input_dev;
> +	int i, error;
> +
> +	input_dev = input_allocate_device();
> +	if (!input_dev)
> +		return -ENOMEM;
> +
> +	xpad->dev = input_dev;
> +	input_dev->name = xpad->name;
> +	input_dev->phys = xpad->phys;
> +	usb_to_input_id(xpad->udev, &input_dev->id);
> +	input_dev->dev.parent = &xpad->intf->dev;
> +
> +	input_set_drvdata(input_dev, xpad);
> +
> +	input_dev->open = xpad_open;
> +	input_dev->close = xpad_close;
> +
> +	input_dev->evbit[0] = BIT_MASK(EV_KEY);
> +
> +	if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
> +		input_dev->evbit[0] |= BIT_MASK(EV_ABS);
> +		/* set up axes */
> +		for (i = 0; xpad_abs[i] >= 0; i++)
> +			xpad_set_up_abs(input_dev, xpad_abs[i]);
> +	}
> +
> +	/* set up standard buttons */
> +	for (i = 0; xpad_common_btn[i] >= 0; i++)
> +		__set_bit(xpad_common_btn[i], input_dev->keybit);
> +
> +	/* set up model-specific ones */
> +	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX360W ||
> +	    xpad->xtype == XTYPE_XBOXONE) {
> +		for (i = 0; xpad360_btn[i] >= 0; i++)
> +			__set_bit(xpad360_btn[i], input_dev->keybit);
> +	} else {
> +		for (i = 0; xpad_btn[i] >= 0; i++)
> +			__set_bit(xpad_btn[i], input_dev->keybit);
> +	}
> +
> +	if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
> +		for (i = 0; xpad_btn_pad[i] >= 0; i++)
> +			__set_bit(xpad_btn_pad[i], input_dev->keybit);
> +	}
> +	/* this should be a simple else block. However historically xbox360w
> +	 * has mapped DPAD to buttons while xbox360 did not.
> +	 * This made no sense, but now we can not just switch back and have to
> +	 * support both behaviors.
> +	 */
> +	if(!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
> +			xpad->xtype == XTYPE_XBOX360W) {
> +		for (i = 0; xpad_abs_pad[i] >= 0; i++)
> +			xpad_set_up_abs(input_dev, xpad_abs_pad[i]);
> +	}
> +
> +	if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
> +		for (i = 0; xpad_btn_triggers[i] >= 0; i++)
> +			__set_bit(xpad_btn_triggers[i], input_dev->keybit);
> +	} else {
> +		for (i = 0; xpad_abs_triggers[i] >= 0; i++)
> +			xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
> +	}
> +
> +	error = xpad_init_ff(xpad);
> +	if (error)
> +		goto fail_init_ff;
> +
> +	error = xpad_led_probe(xpad);
> +	if (error)
> +		goto fail_init_led;
> +
> +	error = input_register_device(xpad->dev);
> +	if (error)
> +		goto fail_input_register;
> +
> +	return 0;
> +
> +fail_input_register:
> +	xpad_led_disconnect(xpad);
> +
> +fail_init_led:
> +	input_ff_destroy(input_dev);
> +
> +fail_init_ff:
> +	input_free_device(input_dev);
> +	return error;
> +}
> +
>  static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  {
>  	struct usb_device *udev = interface_to_usbdev(intf);
>  	struct usb_xpad *xpad;
> -	struct input_dev *input_dev;
>  	struct usb_endpoint_descriptor *ep_irq_in;
>  	int ep_irq_in_idx;
>  	int i, error;
> @@ -1070,12 +1161,14 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  	}
>  
>  	xpad = kzalloc(sizeof(struct usb_xpad), GFP_KERNEL);
> -	input_dev = input_allocate_device();
> -	if (!xpad || !input_dev) {
> +	if (!xpad) {
>  		error = -ENOMEM;
>  		goto fail1;
>  	}
>  
> +	usb_make_path(udev, xpad->phys, sizeof(xpad->phys));
> +	strlcat(xpad->phys, "/input0", sizeof(xpad->phys));
> +
>  	xpad->idata = usb_alloc_coherent(udev, XPAD_PKT_LEN,
>  					 GFP_KERNEL, &xpad->idata_dma);
>  	if (!xpad->idata) {
> @@ -1093,6 +1186,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  	xpad->intf = intf;
>  	xpad->mapping = xpad_device[i].mapping;
>  	xpad->xtype = xpad_device[i].xtype;
> +	xpad->name = xpad_device[i].name;
>  
>  	if (xpad->xtype == XTYPE_UNKNOWN) {
>  		if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
> @@ -1111,78 +1205,10 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  			xpad->mapping |= MAP_STICKS_TO_NULL;
>  	}
>  
> -	xpad->dev = input_dev;
> -	usb_make_path(udev, xpad->phys, sizeof(xpad->phys));
> -	strlcat(xpad->phys, "/input0", sizeof(xpad->phys));
> -
> -	input_dev->name = xpad_device[i].name;
> -	input_dev->phys = xpad->phys;
> -	usb_to_input_id(udev, &input_dev->id);
> -	input_dev->dev.parent = &intf->dev;
> -
> -	input_set_drvdata(input_dev, xpad);
> -
> -	input_dev->open = xpad_open;
> -	input_dev->close = xpad_close;
> -
> -	input_dev->evbit[0] = BIT_MASK(EV_KEY);
> -
> -	if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
> -		input_dev->evbit[0] |= BIT_MASK(EV_ABS);
> -		/* set up axes */
> -		for (i = 0; xpad_abs[i] >= 0; i++)
> -			xpad_set_up_abs(input_dev, xpad_abs[i]);
> -	}
> -
> -	/* set up standard buttons */
> -	for (i = 0; xpad_common_btn[i] >= 0; i++)
> -		__set_bit(xpad_common_btn[i], input_dev->keybit);
> -
> -	/* set up model-specific ones */
> -	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX360W ||
> -	    xpad->xtype == XTYPE_XBOXONE) {
> -		for (i = 0; xpad360_btn[i] >= 0; i++)
> -			__set_bit(xpad360_btn[i], input_dev->keybit);
> -	} else {
> -		for (i = 0; xpad_btn[i] >= 0; i++)
> -			__set_bit(xpad_btn[i], input_dev->keybit);
> -	}
> -
> -	if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
> -		for (i = 0; xpad_btn_pad[i] >= 0; i++)
> -			__set_bit(xpad_btn_pad[i], input_dev->keybit);
> -	}
> -	/* this should be a simple else block. However historically xbox360w
> -	 * has mapped DPAD to buttons while xbox360 did not.
> -	 * This made no sense, but now we can not just switch back and have to
> -	 * support both behaviors.
> -	 */
> -	if(!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
> -			xpad->xtype == XTYPE_XBOX360W) {
> -		for (i = 0; xpad_abs_pad[i] >= 0; i++)
> -			xpad_set_up_abs(input_dev, xpad_abs_pad[i]);
> -	}
> -
> -	if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
> -		for (i = 0; xpad_btn_triggers[i] >= 0; i++)
> -			__set_bit(xpad_btn_triggers[i], input_dev->keybit);
> -	} else {
> -		for (i = 0; xpad_abs_triggers[i] >= 0; i++)
> -			xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
> -	}
> -
>  	error = xpad_init_output(intf, xpad);
>  	if (error)
>  		goto fail3;
>  
> -	error = xpad_init_ff(xpad);
> -	if (error)
> -		goto fail4;
> -
> -	error = xpad_led_probe(xpad);
> -	if (error)
> -		goto fail5;
> -
>  	/* Xbox One controller has in/out endpoints swapped. */
>  	ep_irq_in_idx = xpad->xtype == XTYPE_XBOXONE ? 1 : 0;
>  	ep_irq_in = &intf->cur_altsetting->endpoint[ep_irq_in_idx].desc;
> @@ -1194,10 +1220,6 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  	xpad->irq_in->transfer_dma = xpad->idata_dma;
>  	xpad->irq_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>  
> -	error = input_register_device(xpad->dev);
> -	if (error)
> -		goto fail6;
> -
>  	usb_set_intfdata(intf, xpad);
>  
>  	if (xpad->xtype == XTYPE_XBOX360W) {
> @@ -1210,32 +1232,37 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  		 */
>  		xpad->irq_in->dev = xpad->udev;
>  		error = usb_submit_urb(xpad->irq_in, GFP_KERNEL);
> -		if (error)
> -			goto fail7;
> +		if (error) {
> +			usb_kill_urb(xpad->irq_in);

Why do we need to kill urb if its submission failed?

> +			goto fail4;
> +		}
>  	}
> +	xpad->pad_present = 1;
> +	error = xpad_init_input(xpad);

This is too late, at least in the context of this patch. The packets may
already be flowing.

I moved this chunk up.

> +	if (error)
> +		goto fail4;
>  
>  	return 0;
>  
> - fail7:	input_unregister_device(input_dev);
> -	input_dev = NULL;
> - fail6:	xpad_led_disconnect(xpad);
> - fail5:	if (input_dev)
> -		input_ff_destroy(input_dev);
>   fail4:	xpad_deinit_output(xpad);
>   fail3:	usb_free_urb(xpad->irq_in);
>   fail2:	usb_free_coherent(udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma);
> - fail1:	input_free_device(input_dev);
> -	kfree(xpad);
> + fail1:	kfree(xpad);
>  	return error;
>  
>  }
>  
> +static void xpad_deinit_input(struct usb_xpad *xpad)
> +{
> +	xpad_led_disconnect(xpad);
> +	input_unregister_device(xpad->dev);
> +}
> +
>  static void xpad_disconnect(struct usb_interface *intf)
>  {
>  	struct usb_xpad *xpad = usb_get_intfdata (intf);
>  
> -	xpad_led_disconnect(xpad);
> -	input_unregister_device(xpad->dev);
> +	xpad_deinit_input(xpad);
>  	xpad_deinit_output(xpad);
>  
>  	if (xpad->xtype == XTYPE_XBOX360W) {
> -- 
> 1.9.1
> 

Thanks.

-- 
Dmitry

Re: [PATCH 07/15] Input: xpad: move the input device creation to a new function

[Pavel Rojtberg]

Hey Dimitry,

I have seen you have also applied
"[PATCH 08/15] Input: xpad: query Wireless controller state at init".
However this change is unfortunately incomplete without
"[PATCH 09/15] Input: xpad: handle "present" and "gone" correctly".

As is both the presence as well as the LED packets are sent immediately 
at init
which triggers the the "URB xxxx submitted while active" Warning and causes
any initialization to fail.

Attached is a fixup against current input/ next of the chunk that is in
[PATCH 09/15], but should have been in [PATCH 08/15].
Sorry for the inconvenience.

do not call xpad_identify_controller at init: it conflicts with
the already sent presence packet and will be called by
xpad360w_process_packet as needed anyway.

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
  drivers/input/joystick/xpad.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index d382d48..acb8859 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1001,8 +1001,6 @@ static int xpad_led_probe(struct usb_xpad *xpad)
      if (error)
          goto err_free_id;

-    /* Light up the segment corresponding to controller number */
-    xpad_identify_controller(xpad);
      return 0;

  err_free_id:
-- 
1.9.1


Am 10.10.2015 um 20:00 schrieb Dmitry Torokhov:
> On Thu, Oct 01, 2015 at 10:57:18PM +0200, Pavel Rojtberg wrote:
>> From: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
>>
>> To allow us to later create / destroy the input device from the urb
>> callback, we need to initialize/ deinitialize the input device from a
>> separate function.  So pull that logic out now to make later patches
>> more "obvious" as to what they do.
>>
>> Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
>> ---
>>   drivers/input/joystick/xpad.c | 199 +++++++++++++++++++++++++++++++++++++----------------------------
>>   1 file changed, 113 insertions(+), 86 deletions(-)
>>
>> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
>> index 15da2a3..43490ea 100644
>> --- a/drivers/input/joystick/xpad.c
>> +++ b/drivers/input/joystick/xpad.c
>> @@ -343,6 +343,7 @@ struct usb_xpad {
>>   	int mapping;			/* map d-pad to buttons or to axes */
>>   	int xtype;			/* type of xbox device */
>>   	unsigned long pad_nr;		/* the order x360 pads were attached */
>> +	const char *name;		/* name of the device */
>>   };
>>   
>>   /*
>> @@ -1044,11 +1045,101 @@ static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
>>   	}
>>   }
>>   
>> +static int xpad_init_input(struct usb_xpad *xpad)
>> +{
>> +	struct input_dev *input_dev;
>> +	int i, error;
>> +
>> +	input_dev = input_allocate_device();
>> +	if (!input_dev)
>> +		return -ENOMEM;
>> +
>> +	xpad->dev = input_dev;
>> +	input_dev->name = xpad->name;
>> +	input_dev->phys = xpad->phys;
>> +	usb_to_input_id(xpad->udev, &input_dev->id);
>> +	input_dev->dev.parent = &xpad->intf->dev;
>> +
>> +	input_set_drvdata(input_dev, xpad);
>> +
>> +	input_dev->open = xpad_open;
>> +	input_dev->close = xpad_close;
>> +
>> +	input_dev->evbit[0] = BIT_MASK(EV_KEY);
>> +
>> +	if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
>> +		input_dev->evbit[0] |= BIT_MASK(EV_ABS);
>> +		/* set up axes */
>> +		for (i = 0; xpad_abs[i] >= 0; i++)
>> +			xpad_set_up_abs(input_dev, xpad_abs[i]);
>> +	}
>> +
>> +	/* set up standard buttons */
>> +	for (i = 0; xpad_common_btn[i] >= 0; i++)
>> +		__set_bit(xpad_common_btn[i], input_dev->keybit);
>> +
>> +	/* set up model-specific ones */
>> +	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX360W ||
>> +	    xpad->xtype == XTYPE_XBOXONE) {
>> +		for (i = 0; xpad360_btn[i] >= 0; i++)
>> +			__set_bit(xpad360_btn[i], input_dev->keybit);
>> +	} else {
>> +		for (i = 0; xpad_btn[i] >= 0; i++)
>> +			__set_bit(xpad_btn[i], input_dev->keybit);
>> +	}
>> +
>> +	if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
>> +		for (i = 0; xpad_btn_pad[i] >= 0; i++)
>> +			__set_bit(xpad_btn_pad[i], input_dev->keybit);
>> +	}
>> +	/* this should be a simple else block. However historically xbox360w
>> +	 * has mapped DPAD to buttons while xbox360 did not.
>> +	 * This made no sense, but now we can not just switch back and have to
>> +	 * support both behaviors.
>> +	 */
>> +	if(!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
>> +			xpad->xtype == XTYPE_XBOX360W) {
>> +		for (i = 0; xpad_abs_pad[i] >= 0; i++)
>> +			xpad_set_up_abs(input_dev, xpad_abs_pad[i]);
>> +	}
>> +
>> +	if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
>> +		for (i = 0; xpad_btn_triggers[i] >= 0; i++)
>> +			__set_bit(xpad_btn_triggers[i], input_dev->keybit);
>> +	} else {
>> +		for (i = 0; xpad_abs_triggers[i] >= 0; i++)
>> +			xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
>> +	}
>> +
>> +	error = xpad_init_ff(xpad);
>> +	if (error)
>> +		goto fail_init_ff;
>> +
>> +	error = xpad_led_probe(xpad);
>> +	if (error)
>> +		goto fail_init_led;
>> +
>> +	error = input_register_device(xpad->dev);
>> +	if (error)
>> +		goto fail_input_register;
>> +
>> +	return 0;
>> +
>> +fail_input_register:
>> +	xpad_led_disconnect(xpad);
>> +
>> +fail_init_led:
>> +	input_ff_destroy(input_dev);
>> +
>> +fail_init_ff:
>> +	input_free_device(input_dev);
>> +	return error;
>> +}
>> +
>>   static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id)
>>   {
>>   	struct usb_device *udev = interface_to_usbdev(intf);
>>   	struct usb_xpad *xpad;
>> -	struct input_dev *input_dev;
>>   	struct usb_endpoint_descriptor *ep_irq_in;
>>   	int ep_irq_in_idx;
>>   	int i, error;
>> @@ -1070,12 +1161,14 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>>   	}
>>   
>>   	xpad = kzalloc(sizeof(struct usb_xpad), GFP_KERNEL);
>> -	input_dev = input_allocate_device();
>> -	if (!xpad || !input_dev) {
>> +	if (!xpad) {
>>   		error = -ENOMEM;
>>   		goto fail1;
>>   	}
>>   
>> +	usb_make_path(udev, xpad->phys, sizeof(xpad->phys));
>> +	strlcat(xpad->phys, "/input0", sizeof(xpad->phys));
>> +
>>   	xpad->idata = usb_alloc_coherent(udev, XPAD_PKT_LEN,
>>   					 GFP_KERNEL, &xpad->idata_dma);
>>   	if (!xpad->idata) {
>> @@ -1093,6 +1186,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>>   	xpad->intf = intf;
>>   	xpad->mapping = xpad_device[i].mapping;
>>   	xpad->xtype = xpad_device[i].xtype;
>> +	xpad->name = xpad_device[i].name;
>>   
>>   	if (xpad->xtype == XTYPE_UNKNOWN) {
>>   		if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
>> @@ -1111,78 +1205,10 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>>   			xpad->mapping |= MAP_STICKS_TO_NULL;
>>   	}
>>   
>> -	xpad->dev = input_dev;
>> -	usb_make_path(udev, xpad->phys, sizeof(xpad->phys));
>> -	strlcat(xpad->phys, "/input0", sizeof(xpad->phys));
>> -
>> -	input_dev->name = xpad_device[i].name;
>> -	input_dev->phys = xpad->phys;
>> -	usb_to_input_id(udev, &input_dev->id);
>> -	input_dev->dev.parent = &intf->dev;
>> -
>> -	input_set_drvdata(input_dev, xpad);
>> -
>> -	input_dev->open = xpad_open;
>> -	input_dev->close = xpad_close;
>> -
>> -	input_dev->evbit[0] = BIT_MASK(EV_KEY);
>> -
>> -	if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
>> -		input_dev->evbit[0] |= BIT_MASK(EV_ABS);
>> -		/* set up axes */
>> -		for (i = 0; xpad_abs[i] >= 0; i++)
>> -			xpad_set_up_abs(input_dev, xpad_abs[i]);
>> -	}
>> -
>> -	/* set up standard buttons */
>> -	for (i = 0; xpad_common_btn[i] >= 0; i++)
>> -		__set_bit(xpad_common_btn[i], input_dev->keybit);
>> -
>> -	/* set up model-specific ones */
>> -	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX360W ||
>> -	    xpad->xtype == XTYPE_XBOXONE) {
>> -		for (i = 0; xpad360_btn[i] >= 0; i++)
>> -			__set_bit(xpad360_btn[i], input_dev->keybit);
>> -	} else {
>> -		for (i = 0; xpad_btn[i] >= 0; i++)
>> -			__set_bit(xpad_btn[i], input_dev->keybit);
>> -	}
>> -
>> -	if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
>> -		for (i = 0; xpad_btn_pad[i] >= 0; i++)
>> -			__set_bit(xpad_btn_pad[i], input_dev->keybit);
>> -	}
>> -	/* this should be a simple else block. However historically xbox360w
>> -	 * has mapped DPAD to buttons while xbox360 did not.
>> -	 * This made no sense, but now we can not just switch back and have to
>> -	 * support both behaviors.
>> -	 */
>> -	if(!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
>> -			xpad->xtype == XTYPE_XBOX360W) {
>> -		for (i = 0; xpad_abs_pad[i] >= 0; i++)
>> -			xpad_set_up_abs(input_dev, xpad_abs_pad[i]);
>> -	}
>> -
>> -	if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
>> -		for (i = 0; xpad_btn_triggers[i] >= 0; i++)
>> -			__set_bit(xpad_btn_triggers[i], input_dev->keybit);
>> -	} else {
>> -		for (i = 0; xpad_abs_triggers[i] >= 0; i++)
>> -			xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
>> -	}
>> -
>>   	error = xpad_init_output(intf, xpad);
>>   	if (error)
>>   		goto fail3;
>>   
>> -	error = xpad_init_ff(xpad);
>> -	if (error)
>> -		goto fail4;
>> -
>> -	error = xpad_led_probe(xpad);
>> -	if (error)
>> -		goto fail5;
>> -
>>   	/* Xbox One controller has in/out endpoints swapped. */
>>   	ep_irq_in_idx = xpad->xtype == XTYPE_XBOXONE ? 1 : 0;
>>   	ep_irq_in = &intf->cur_altsetting->endpoint[ep_irq_in_idx].desc;
>> @@ -1194,10 +1220,6 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>>   	xpad->irq_in->transfer_dma = xpad->idata_dma;
>>   	xpad->irq_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>>   
>> -	error = input_register_device(xpad->dev);
>> -	if (error)
>> -		goto fail6;
>> -
>>   	usb_set_intfdata(intf, xpad);
>>   
>>   	if (xpad->xtype == XTYPE_XBOX360W) {
>> @@ -1210,32 +1232,37 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>>   		 */
>>   		xpad->irq_in->dev = xpad->udev;
>>   		error = usb_submit_urb(xpad->irq_in, GFP_KERNEL);
>> -		if (error)
>> -			goto fail7;
>> +		if (error) {
>> +			usb_kill_urb(xpad->irq_in);
> Why do we need to kill urb if its submission failed?
>
>> +			goto fail4;
>> +		}
>>   	}
>> +	xpad->pad_present = 1;
>> +	error = xpad_init_input(xpad);
> This is too late, at least in the context of this patch. The packets may
> already be flowing.
>
> I moved this chunk up.
>
>> +	if (error)
>> +		goto fail4;
>>   
>>   	return 0;
>>   
>> - fail7:	input_unregister_device(input_dev);
>> -	input_dev = NULL;
>> - fail6:	xpad_led_disconnect(xpad);
>> - fail5:	if (input_dev)
>> -		input_ff_destroy(input_dev);
>>    fail4:	xpad_deinit_output(xpad);
>>    fail3:	usb_free_urb(xpad->irq_in);
>>    fail2:	usb_free_coherent(udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma);
>> - fail1:	input_free_device(input_dev);
>> -	kfree(xpad);
>> + fail1:	kfree(xpad);
>>   	return error;
>>   
>>   }
>>   
>> +static void xpad_deinit_input(struct usb_xpad *xpad)
>> +{
>> +	xpad_led_disconnect(xpad);
>> +	input_unregister_device(xpad->dev);
>> +}
>> +
>>   static void xpad_disconnect(struct usb_interface *intf)
>>   {
>>   	struct usb_xpad *xpad = usb_get_intfdata (intf);
>>   
>> -	xpad_led_disconnect(xpad);
>> -	input_unregister_device(xpad->dev);
>> +	xpad_deinit_input(xpad);
>>   	xpad_deinit_output(xpad);
>>   
>>   	if (xpad->xtype == XTYPE_XBOX360W) {
>> -- 
>> 1.9.1
>>
> Thanks.
>

Re: [PATCH 07/15] Input: xpad: move the input device creation to a new function

[Dmitry Torokhov]

Hi Pavel,

On Thu, Oct 15, 2015 at 09:19:35PM +0200, Pavel Rojtberg wrote:
> Hey Dimitry,
> 
> I have seen you have also applied
> "[PATCH 08/15] Input: xpad: query Wireless controller state at init".
> However this change is unfortunately incomplete without
> "[PATCH 09/15] Input: xpad: handle "present" and "gone" correctly".
> 
> As is both the presence as well as the LED packets are sent
> immediately at init
> which triggers the the "URB xxxx submitted while active" Warning and causes
> any initialization to fail.
> 
> Attached is a fixup against current input/ next of the chunk that is in
> [PATCH 09/15], but should have been in [PATCH 08/15].
> Sorry for the inconvenience.
> 
> do not call xpad_identify_controller at init: it conflicts with
> the already sent presence packet and will be called by
> xpad360w_process_packet as needed anyway.

I see. But I believe we should only do that for wireless controllers,
because we send the presence request only for XTYPE_XBOX360W and LEDs
are also present on non-wireless variant, right?

So I think we want:

	if (xpad->xtype == XTYPE_XBOX360) {
		/*
		 * Light up the segment corresponding to controller
		 * number on wired devices. On wireless we'll do that
		 * when they respond to "presence" packet.
		 */
		xpad_identify_controller(xpad);
	}

Thanks.

-- 
Dmitry

Re: [PATCH 07/15] Input: xpad: move the input device creation to a new function

[Pavel Rojtberg]

Am 17.10.2015 um 18:49 schrieb Dmitry Torokhov:
> Hi Pavel,
>
> On Thu, Oct 15, 2015 at 09:19:35PM +0200, Pavel Rojtberg wrote:
>> Hey Dimitry,
>>
>> I have seen you have also applied
>> "[PATCH 08/15] Input: xpad: query Wireless controller state at init".
>> However this change is unfortunately incomplete without
>> "[PATCH 09/15] Input: xpad: handle "present" and "gone" correctly".
>>
>> As is both the presence as well as the LED packets are sent
>> immediately at init
>> which triggers the the "URB xxxx submitted while active" Warning and causes
>> any initialization to fail.
>>
>> Attached is a fixup against current input/ next of the chunk that is in
>> [PATCH 09/15], but should have been in [PATCH 08/15].
>> Sorry for the inconvenience.
>>
>> do not call xpad_identify_controller at init: it conflicts with
>> the already sent presence packet and will be called by
>> xpad360w_process_packet as needed anyway.
> I see. But I believe we should only do that for wireless controllers,
> because we send the presence request only for XTYPE_XBOX360W and LEDs
> are also present on non-wireless variant, right?
>
> So I think we want:
>
> 	if (xpad->xtype == XTYPE_XBOX360) {
> 		/*
> 		 * Light up the segment corresponding to controller
> 		 * number on wired devices. On wireless we'll do that
> 		 * when they respond to "presence" packet.
> 		 */
> 		xpad_identify_controller(xpad);
> 	}
>
> Thanks.
yes, you are right.

[PATCH 08/15] Input: xpad: query Wireless controller state at init

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

When we initialize the driver/device, we really don't know how many
controllers are connected.  So send a "query presence" command to the
base-station. (Command discovered by Zachary Lund)

presence packet taken from:
https://github.com/computerquip/xpad5

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 43490ea..7d66d77 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1236,6 +1236,30 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 			usb_kill_urb(xpad->irq_in);
 			goto fail4;
 		}
+
+		/*
+		 * send presence packet
+		 * This will force the controller to resend connection packets.
+		 * This is useful in the case we activate the module after the
+		 * adapter has been plugged in, as it won't automatically
+		 * send us info about the controllers.
+		 */
+		mutex_lock(&xpad->odata_mutex);
+		xpad->odata[0] = 0x08;
+		xpad->odata[1] = 0x00;
+		xpad->odata[2] = 0x0F;
+		xpad->odata[3] = 0xC0;
+		xpad->odata[4] = 0x00;
+		xpad->odata[5] = 0x00;
+		xpad->odata[6] = 0x00;
+		xpad->odata[7] = 0x00;
+		xpad->odata[8] = 0x00;
+		xpad->odata[9] = 0x00;
+		xpad->odata[10] = 0x00;
+		xpad->odata[11] = 0x00;
+		xpad->irq_out->transfer_buffer_length = 12;
+		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+		mutex_unlock(&xpad->odata_mutex);
 	}
 	xpad->pad_present = 1;
 	error = xpad_init_input(xpad);
-- 
1.9.1

[PATCH 09/15] Input: xpad: handle "present" and "gone" correctly

[Pavel Rojtberg]

From: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>

Handle the "a new device is present" message properly by dynamically
creating the input device at this point in time. This means we now do
not "preallocate" all 4 devices when a single
wireless base station is seen. This requires a workqueue as we are in
interrupt context when we learn about this.

Also properly disconnect any devices that we are told are removed.

Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 7d66d77..31bcd78 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -344,8 +344,12 @@ struct usb_xpad {
 	int xtype;			/* type of xbox device */
 	unsigned long pad_nr;		/* the order x360 pads were attached */
 	const char *name;		/* name of the device */
+	struct work_struct work;	/* init/remove device from callback */
 };
 
+static int xpad_init_input(struct usb_xpad *xpad);
+static void xpad_deinit_input(struct usb_xpad *xpad);
+
 /*
  *	xpad_process_packet
  *
@@ -496,6 +500,22 @@ static void xpad360_process_packet(struct usb_xpad *xpad,
 
 static void xpad_identify_controller(struct usb_xpad *xpad);
 
+static void presence_work_function(struct work_struct *work)
+{
+	struct usb_xpad *xpad = container_of(work, struct usb_xpad, work);
+	int error;
+
+	if (xpad->pad_present) {
+		error = xpad_init_input(xpad);
+		if (error) {
+			/* complain only, not much else we can do here */
+			dev_err(&xpad->dev->dev, "unable to init device\n");
+		}
+	} else {
+		xpad_deinit_input(xpad);
+	}
+}
+
 /*
  * xpad360w_process_packet
  *
@@ -512,13 +532,16 @@ static void xpad_identify_controller(struct usb_xpad *xpad);
  */
 static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
 {
+	int presence;
+
 	/* Presence change */
 	if (data[0] & 0x08) {
-		if (data[1] & 0x80) {
-			xpad->pad_present = 1;
-			xpad_identify_controller(xpad);
-		} else
-			xpad->pad_present = 0;
+		presence = (data[1] & 0x80) != 0;
+
+		if (xpad->pad_present != presence) {
+			xpad->pad_present = presence;
+			schedule_work(&xpad->work);
+		}
 	}
 
 	/* Valid pad data */
@@ -965,8 +988,6 @@ static int xpad_led_probe(struct usb_xpad *xpad)
 		return error;
 	}
 
-	xpad_identify_controller(xpad);
-
 	return 0;
 }
 
@@ -1123,6 +1144,8 @@ static int xpad_init_input(struct usb_xpad *xpad)
 	if (error)
 		goto fail_input_register;
 
+	xpad_identify_controller(xpad);
+
 	return 0;
 
 fail_input_register:
@@ -1187,6 +1210,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 	xpad->mapping = xpad_device[i].mapping;
 	xpad->xtype = xpad_device[i].xtype;
 	xpad->name = xpad_device[i].name;
+	INIT_WORK(&xpad->work, presence_work_function);
 
 	if (xpad->xtype == XTYPE_UNKNOWN) {
 		if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
@@ -1260,11 +1284,12 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		xpad->irq_out->transfer_buffer_length = 12;
 		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
 		mutex_unlock(&xpad->odata_mutex);
+	} else {
+		xpad->pad_present = 1;
+		error = xpad_init_input(xpad);
+		if (error)
+			goto fail4;
 	}
-	xpad->pad_present = 1;
-	error = xpad_init_input(xpad);
-	if (error)
-		goto fail4;
 
 	return 0;
 
@@ -1286,7 +1311,8 @@ static void xpad_disconnect(struct usb_interface *intf)
 {
 	struct usb_xpad *xpad = usb_get_intfdata (intf);
 
-	xpad_deinit_input(xpad);
+	if (xpad->pad_present)
+		xpad_deinit_input(xpad);
 	xpad_deinit_output(xpad);
 
 	if (xpad->xtype == XTYPE_XBOX360W) {
@@ -1297,6 +1323,8 @@ static void xpad_disconnect(struct usb_interface *intf)
 	usb_free_coherent(xpad->udev, XPAD_PKT_LEN,
 			xpad->idata, xpad->idata_dma);
 
+	cancel_work_sync(&xpad->work);
+
 	kfree(xpad);
 
 	usb_set_intfdata(intf, NULL);
-- 
1.9.1

Re: [PATCH 09/15] Input: xpad: handle "present" and "gone" correctly

[Dmitry Torokhov]

Hi Pavel,

On Thu, Oct 01, 2015 at 10:57:20PM +0200, Pavel Rojtberg wrote:
> From: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
> 
> Handle the "a new device is present" message properly by dynamically
> creating the input device at this point in time. This means we now do
> not "preallocate" all 4 devices when a single
> wireless base station is seen. This requires a workqueue as we are in
> interrupt context when we learn about this.
> 
> Also properly disconnect any devices that we are told are removed.
> 
> Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
> ---
>  drivers/input/joystick/xpad.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 7d66d77..31bcd78 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -344,8 +344,12 @@ struct usb_xpad {
>  	int xtype;			/* type of xbox device */
>  	unsigned long pad_nr;		/* the order x360 pads were attached */
>  	const char *name;		/* name of the device */
> +	struct work_struct work;	/* init/remove device from callback */
>  };
>  
> +static int xpad_init_input(struct usb_xpad *xpad);
> +static void xpad_deinit_input(struct usb_xpad *xpad);
> +
>  /*
>   *	xpad_process_packet
>   *
> @@ -496,6 +500,22 @@ static void xpad360_process_packet(struct usb_xpad *xpad,
>  
>  static void xpad_identify_controller(struct usb_xpad *xpad);
>  
> +static void presence_work_function(struct work_struct *work)
> +{
> +	struct usb_xpad *xpad = container_of(work, struct usb_xpad, work);
> +	int error;
> +
> +	if (xpad->pad_present) {
> +		error = xpad_init_input(xpad);
> +		if (error) {
> +			/* complain only, not much else we can do here */
> +			dev_err(&xpad->dev->dev, "unable to init device\n");
> +		}
> +	} else {
> +		xpad_deinit_input(xpad);
> +	}
> +}
> +
>  /*
>   * xpad360w_process_packet
>   *
> @@ -512,13 +532,16 @@ static void xpad_identify_controller(struct usb_xpad *xpad);
>   */
>  static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
>  {
> +	int presence;
> +
>  	/* Presence change */
>  	if (data[0] & 0x08) {
> -		if (data[1] & 0x80) {
> -			xpad->pad_present = 1;
> -			xpad_identify_controller(xpad);
> -		} else
> -			xpad->pad_present = 0;
> +		presence = (data[1] & 0x80) != 0;
> +
> +		if (xpad->pad_present != presence) {
> +			xpad->pad_present = presence;
> +			schedule_work(&xpad->work);
> +		}
>  	}
>  
>  	/* Valid pad data */
> @@ -965,8 +988,6 @@ static int xpad_led_probe(struct usb_xpad *xpad)
>  		return error;
>  	}
>  
> -	xpad_identify_controller(xpad);
> -
>  	return 0;
>  }
>  
> @@ -1123,6 +1144,8 @@ static int xpad_init_input(struct usb_xpad *xpad)
>  	if (error)
>  		goto fail_input_register;
>  
> +	xpad_identify_controller(xpad);
> +
>  	return 0;
>  
>  fail_input_register:
> @@ -1187,6 +1210,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  	xpad->mapping = xpad_device[i].mapping;
>  	xpad->xtype = xpad_device[i].xtype;
>  	xpad->name = xpad_device[i].name;
> +	INIT_WORK(&xpad->work, presence_work_function);
>  
>  	if (xpad->xtype == XTYPE_UNKNOWN) {
>  		if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
> @@ -1260,11 +1284,12 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  		xpad->irq_out->transfer_buffer_length = 12;
>  		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
>  		mutex_unlock(&xpad->odata_mutex);
> +	} else {
> +		xpad->pad_present = 1;
> +		error = xpad_init_input(xpad);
> +		if (error)
> +			goto fail4;
>  	}
> -	xpad->pad_present = 1;
> -	error = xpad_init_input(xpad);
> -	if (error)
> -		goto fail4;
>  
>  	return 0;
>  
> @@ -1286,7 +1311,8 @@ static void xpad_disconnect(struct usb_interface *intf)
>  {
>  	struct usb_xpad *xpad = usb_get_intfdata (intf);
>  
> -	xpad_deinit_input(xpad);
> +	if (xpad->pad_present)
> +		xpad_deinit_input(xpad);
>  	xpad_deinit_output(xpad);
>  
>  	if (xpad->xtype == XTYPE_XBOX360W) {
> @@ -1297,6 +1323,8 @@ static void xpad_disconnect(struct usb_interface *intf)
>  	usb_free_coherent(xpad->udev, XPAD_PKT_LEN,
>  			xpad->idata, xpad->idata_dma);
>  
> +	cancel_work_sync(&xpad->work);
> +

This is too late. You need to stop the IO, then cancel the work and then
see if input device is created or not and destroy it if it is present.
BTW current logic for X360W is broken in this regard as well.

>  	kfree(xpad);
>  
>  	usb_set_intfdata(intf, NULL);
> -- 
> 1.9.1
> 

Thanks.

-- 
Dmitry

[PATCH 10/15] Input: xpad: use ida() for finding the pad_nr

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

The pad_nr corresponds to the lit up LED on the controller. Therefore
there should be no gaps when enumerating. Currently a LED is only
re-assigned after a controller is re-connected 4 times.

This patch uses ida to track connected pads - this way we can re-assign
freed up pad number immediately.

Consider the following case:
1. pad A is connected and gets pad_nr = 0
2. pad B is connected and gets pad_nr = 1
3. pad A is disconnected
4. pad A is connected again

using ida() controller A now correctly gets pad_nr = 0 again.

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 31bcd78..7d53e8e 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -342,11 +342,14 @@ struct usb_xpad {
 
 	int mapping;			/* map d-pad to buttons or to axes */
 	int xtype;			/* type of xbox device */
-	unsigned long pad_nr;		/* the order x360 pads were attached */
+	int pad_nr;			/* the order x360 pads were attached */
 	const char *name;		/* name of the device */
 	struct work_struct work;	/* init/remove device from callback */
 };
 
+static DEFINE_SPINLOCK(xpad_pad_seq_lock);
+static DEFINE_IDA(xpad_pad_seq);
+
 static int xpad_init_input(struct usb_xpad *xpad);
 static void xpad_deinit_input(struct usb_xpad *xpad);
 
@@ -946,6 +949,24 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
  */
 static void xpad_identify_controller(struct usb_xpad *xpad)
 {
+	int res = 0;
+
+	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX360W)
+		return;
+
+	while(res == -EAGAIN) {
+		if(!ida_pre_get(&xpad_pad_seq, GFP_KERNEL)) {
+			dev_dbg(&xpad->dev->dev,
+				"%s - ida_pre_get failed. giving up\n",
+				__func__);
+			return;
+		}
+
+		spin_lock(&xpad_pad_seq_lock);
+		res = ida_get_new(&xpad_pad_seq, &xpad->pad_nr);
+		spin_unlock(&xpad_pad_seq_lock);
+	}
+
 	xpad_send_led_command(xpad, (xpad->pad_nr % 4) + 2);
 }
 
@@ -960,7 +981,6 @@ static void xpad_led_set(struct led_classdev *led_cdev,
 
 static int xpad_led_probe(struct usb_xpad *xpad)
 {
-	static atomic_t led_seq = ATOMIC_INIT(-1);
 	struct xpad_led *led;
 	struct led_classdev *led_cdev;
 	int error;
@@ -972,9 +992,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
 	if (!led)
 		return -ENOMEM;
 
-	xpad->pad_nr = atomic_inc_return(&led_seq);
-
-	snprintf(led->name, sizeof(led->name), "xpad%lu", xpad->pad_nr);
+	snprintf(led->name, sizeof(led->name), "xpad%d", xpad->pad_nr);
 	led->xpad = xpad;
 
 	led_cdev = &led->led_cdev;
@@ -1132,6 +1150,8 @@ static int xpad_init_input(struct usb_xpad *xpad)
 			xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
 	}
 
+	xpad_identify_controller(xpad);
+
 	error = xpad_init_ff(xpad);
 	if (error)
 		goto fail_init_ff;
@@ -1144,8 +1164,6 @@ static int xpad_init_input(struct usb_xpad *xpad)
 	if (error)
 		goto fail_input_register;
 
-	xpad_identify_controller(xpad);
-
 	return 0;
 
 fail_input_register:
@@ -1305,6 +1323,11 @@ static void xpad_deinit_input(struct usb_xpad *xpad)
 {
 	xpad_led_disconnect(xpad);
 	input_unregister_device(xpad->dev);
+
+	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX360W)
+		return;
+
+	ida_remove(&xpad_pad_seq, xpad->pad_nr);
 }
 
 static void xpad_disconnect(struct usb_interface *intf)
-- 
1.9.1

Re: [PATCH 10/15] Input: xpad: use ida() for finding the pad_nr

[Pavel Rojtberg]

sorry, I messed up this one. This would be the correct patch.
Should I re-spin the whole series or would it be too much noise?

Subject: [PATCH 10/15] Input: xpad: use ida() for finding the pad_nr

The pad_nr corresponds to the lit up LED on the controller. Therefore
there should be no gaps when enumerating. Currently a LED is only
re-assigned after a controller is re-connected 4 times.

This patch uses ida to track connected pads - this way we can re-assign
freed up pad number immediately.

Consider the following case:
1. pad A is connected and gets pad_nr = 0
2. pad B is connected and gets pad_nr = 1
3. pad A is disconnected
4. pad A is connected again

using ida_simple_get() controller A now correctly gets pad_nr = 0 again.

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
  drivers/input/joystick/xpad.c | 30 +++++++++++++++++++++++-------
  1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 31bcd78..b83ea3c 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -342,11 +342,13 @@ struct usb_xpad {

      int mapping;            /* map d-pad to buttons or to axes */
      int xtype;            /* type of xbox device */
-    unsigned long pad_nr;        /* the order x360 pads were attached */
+    int pad_nr;            /* the order x360 pads were attached */
      const char *name;        /* name of the device */
      struct work_struct work;    /* init/remove device from callback */
  };

+static DEFINE_IDA(xpad_pad_seq);
+
  static int xpad_init_input(struct usb_xpad *xpad);
  static void xpad_deinit_input(struct usb_xpad *xpad);

@@ -946,6 +948,16 @@ static void xpad_send_led_command(struct usb_xpad 
*xpad, int command)
   */
  static void xpad_identify_controller(struct usb_xpad *xpad)
  {
+    if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX360W)
+        return;
+
+    xpad->pad_nr = ida_simple_get(&xpad_pad_seq, 0, 0, GFP_KERNEL);
+
+    if(xpad->pad_nr < 0) {
+        dev_dbg(&xpad->dev->dev, "%s - ida_get failed\n", __func__);
+        return;
+    }
+
      xpad_send_led_command(xpad, (xpad->pad_nr % 4) + 2);
  }

@@ -960,7 +972,6 @@ static void xpad_led_set(struct led_classdev *led_cdev,

  static int xpad_led_probe(struct usb_xpad *xpad)
  {
-    static atomic_t led_seq = ATOMIC_INIT(-1);
      struct xpad_led *led;
      struct led_classdev *led_cdev;
      int error;
@@ -972,9 +983,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
      if (!led)
          return -ENOMEM;

-    xpad->pad_nr = atomic_inc_return(&led_seq);
-
-    snprintf(led->name, sizeof(led->name), "xpad%lu", xpad->pad_nr);
+    snprintf(led->name, sizeof(led->name), "xpad%d", xpad->pad_nr);
      led->xpad = xpad;

      led_cdev = &led->led_cdev;
@@ -1132,6 +1141,8 @@ static int xpad_init_input(struct usb_xpad *xpad)
              xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
      }

+    xpad_identify_controller(xpad);
+
      error = xpad_init_ff(xpad);
      if (error)
          goto fail_init_ff;
@@ -1144,8 +1155,6 @@ static int xpad_init_input(struct usb_xpad *xpad)
      if (error)
          goto fail_input_register;

-    xpad_identify_controller(xpad);
-
      return 0;

  fail_input_register:
@@ -1305,6 +1314,13 @@ static void xpad_deinit_input(struct usb_xpad *xpad)
  {
      xpad_led_disconnect(xpad);
      input_unregister_device(xpad->dev);
+
+    if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX360W)
+        return;
+
+    if(xpad->pad_nr > -1) {
+        ida_simple_remove(&xpad_pad_seq, xpad->pad_nr);
+    }
  }

  static void xpad_disconnect(struct usb_interface *intf)
-- 
1.9.1

Re: [PATCH 10/15] Input: xpad: use ida() for finding the pad_nr

[Dmitry Torokhov]

On Fri, Oct 02, 2015 at 12:53:26AM +0200, Pavel Rojtberg wrote:
> sorry, I messed up this one. This would be the correct patch.
> Should I re-spin the whole series or would it be too much noise?
> 
> Subject: [PATCH 10/15] Input: xpad: use ida() for finding the pad_nr
> 
> The pad_nr corresponds to the lit up LED on the controller. Therefore
> there should be no gaps when enumerating. Currently a LED is only
> re-assigned after a controller is re-connected 4 times.
> 
> This patch uses ida to track connected pads - this way we can re-assign
> freed up pad number immediately.
> 
> Consider the following case:
> 1. pad A is connected and gets pad_nr = 0
> 2. pad B is connected and gets pad_nr = 1
> 3. pad A is disconnected
> 4. pad A is connected again
> 
> using ida_simple_get() controller A now correctly gets pad_nr = 0 again.
> 
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
> ---
>  drivers/input/joystick/xpad.c | 30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 31bcd78..b83ea3c 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -342,11 +342,13 @@ struct usb_xpad {
> 
>      int mapping;            /* map d-pad to buttons or to axes */
>      int xtype;            /* type of xbox device */
> -    unsigned long pad_nr;        /* the order x360 pads were attached */
> +    int pad_nr;            /* the order x360 pads were attached */
>      const char *name;        /* name of the device */
>      struct work_struct work;    /* init/remove device from callback */
>  };
> 
> +static DEFINE_IDA(xpad_pad_seq);
> +
>  static int xpad_init_input(struct usb_xpad *xpad);
>  static void xpad_deinit_input(struct usb_xpad *xpad);
> 
> @@ -946,6 +948,16 @@ static void xpad_send_led_command(struct
> usb_xpad *xpad, int command)
>   */
>  static void xpad_identify_controller(struct usb_xpad *xpad)
>  {
> +    if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX360W)
> +        return;
> +
> +    xpad->pad_nr = ida_simple_get(&xpad_pad_seq, 0, 0, GFP_KERNEL);
> +
> +    if(xpad->pad_nr < 0) {
> +        dev_dbg(&xpad->dev->dev, "%s - ida_get failed\n", __func__);
> +        return;
> +    }
> +
>      xpad_send_led_command(xpad, (xpad->pad_nr % 4) + 2);
>  }
> 
> @@ -960,7 +972,6 @@ static void xpad_led_set(struct led_classdev *led_cdev,
> 
>  static int xpad_led_probe(struct usb_xpad *xpad)
>  {
> -    static atomic_t led_seq = ATOMIC_INIT(-1);
>      struct xpad_led *led;
>      struct led_classdev *led_cdev;
>      int error;
> @@ -972,9 +983,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
>      if (!led)
>          return -ENOMEM;
> 
> -    xpad->pad_nr = atomic_inc_return(&led_seq);
> -
> -    snprintf(led->name, sizeof(led->name), "xpad%lu", xpad->pad_nr);
> +    snprintf(led->name, sizeof(led->name), "xpad%d", xpad->pad_nr);

So the name is always "xpad0"? Have you tested with more than 1 pad?

I'll fix it all up. The place where you get the new ID us the wrong
place, you needed to do it right here.

>      led->xpad = xpad;
> 
>      led_cdev = &led->led_cdev;
> @@ -1132,6 +1141,8 @@ static int xpad_init_input(struct usb_xpad *xpad)
>              xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
>      }
> 
> +    xpad_identify_controller(xpad);
> +
>      error = xpad_init_ff(xpad);
>      if (error)
>          goto fail_init_ff;
> @@ -1144,8 +1155,6 @@ static int xpad_init_input(struct usb_xpad *xpad)
>      if (error)
>          goto fail_input_register;
> 
> -    xpad_identify_controller(xpad);
> -
>      return 0;
> 
>  fail_input_register:
> @@ -1305,6 +1314,13 @@ static void xpad_deinit_input(struct usb_xpad *xpad)
>  {
>      xpad_led_disconnect(xpad);
>      input_unregister_device(xpad->dev);
> +
> +    if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX360W)
> +        return;
> +
> +    if(xpad->pad_nr > -1) {
> +        ida_simple_remove(&xpad_pad_seq, xpad->pad_nr);
> +    }
>  }
> 
>  static void xpad_disconnect(struct usb_interface *intf)
> -- 
> 1.9.1
> 

-- 
Dmitry

[PATCH 11/15] Input: xpad: do not submit active URBs

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

track the active status of the irq_out URB to prevent submission while
it is active. Failure to do so results in the "URB submitted while
active" warning/ stacktrace.

Also add missing mutex locking around xpad->odata usages.

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 7d53e8e..5eec515 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -330,6 +330,7 @@ struct usb_xpad {
 	dma_addr_t idata_dma;
 
 	struct urb *irq_out;		/* urb for interrupt out report */
+	int irq_out_active;		/* we must not use an active URB */
 	unsigned char *odata;		/* output data */
 	dma_addr_t odata_dma;
 	struct mutex odata_mutex;
@@ -710,6 +711,7 @@ static void xpad_irq_out(struct urb *urb)
 	switch (status) {
 	case 0:
 		/* success */
+		xpad->irq_out_active = 0;
 		return;
 
 	case -ECONNRESET:
@@ -718,6 +720,7 @@ static void xpad_irq_out(struct urb *urb)
 		/* this urb is terminated, clean up */
 		dev_dbg(dev, "%s - urb shutting down with status: %d\n",
 			__func__, status);
+		xpad->irq_out_active = 0;
 		return;
 
 	default:
@@ -795,6 +798,7 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
 	struct usb_xpad *xpad = input_get_drvdata(dev);
 	__u16 strong;
 	__u16 weak;
+	int retval;
 
 	if (effect->type != FF_RUMBLE)
 		return 0;
@@ -802,6 +806,8 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
 	strong = effect->u.rumble.strong_magnitude;
 	weak = effect->u.rumble.weak_magnitude;
 
+	mutex_lock(&xpad->odata_mutex);
+
 	switch (xpad->xtype) {
 	case XTYPE_XBOX:
 		xpad->odata[0] = 0x00;
@@ -858,13 +864,25 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
 		break;
 
 	default:
+		mutex_unlock(&xpad->odata_mutex);
 		dev_dbg(&xpad->dev->dev,
 			"%s - rumble command sent to unsupported xpad type: %d\n",
 			__func__, xpad->xtype);
 		return -EINVAL;
 	}
 
-	return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
+	if (!xpad->irq_out_active) {
+		retval = usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
+		xpad->irq_out_active = 1;
+	} else {
+		retval = -EIO;
+		dev_dbg(&xpad->dev->dev, "%s - dropped, irq_out is active\n",
+				__func__);
+	}
+
+	mutex_unlock(&xpad->odata_mutex);
+
+	return retval;
 }
 
 static int xpad_init_ff(struct usb_xpad *xpad)
@@ -940,7 +958,13 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
 		break;
 	}
 
-	usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+	if (!xpad->irq_out_active) {
+		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+		xpad->irq_out_active = 1;
+	} else
+		dev_dbg(&xpad->dev->dev, "%s - dropped, irq_out is active\n",
+				__func__);
+
 	mutex_unlock(&xpad->odata_mutex);
 }
 
@@ -1028,6 +1052,7 @@ static void xpad_identify_controller(struct usb_xpad *xpad) { }
 static int xpad_open(struct input_dev *dev)
 {
 	struct usb_xpad *xpad = input_get_drvdata(dev);
+	int retval;
 
 	/* URB was submitted in probe */
 	if (xpad->xtype == XTYPE_XBOX360W)
@@ -1038,11 +1063,14 @@ static int xpad_open(struct input_dev *dev)
 		return -EIO;
 
 	if (xpad->xtype == XTYPE_XBOXONE) {
+		mutex_lock(&xpad->odata_mutex);
 		/* Xbox one controller needs to be initialized. */
 		xpad->odata[0] = 0x05;
 		xpad->odata[1] = 0x20;
 		xpad->irq_out->transfer_buffer_length = 2;
-		return usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+		retval = usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+		mutex_unlock(&xpad->odata_mutex);
+		return retval;
 	}
 
 	return 0;
@@ -1300,7 +1328,14 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		xpad->odata[10] = 0x00;
 		xpad->odata[11] = 0x00;
 		xpad->irq_out->transfer_buffer_length = 12;
-		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+
+		if (!xpad->irq_out_active) {
+			usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+			xpad->irq_out_active = 1;
+		} else
+			dev_dbg(&xpad->dev->dev,
+				"%s - dropped, irq_out is active\n", __func__);
+
 		mutex_unlock(&xpad->odata_mutex);
 	} else {
 		xpad->pad_present = 1;
-- 
1.9.1

[PATCH 12/15] Input: xpad: replace mutex by spinlock

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

since we cannot use mutexes in xpad_play_effect.
see: http://www.spinics.net/lists/linux-input/msg40242.html

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 5eec515..1fd2e23 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -333,7 +333,7 @@ struct usb_xpad {
 	int irq_out_active;		/* we must not use an active URB */
 	unsigned char *odata;		/* output data */
 	dma_addr_t odata_dma;
-	struct mutex odata_mutex;
+	spinlock_t odata_lock;
 
 #if defined(CONFIG_JOYSTICK_XPAD_LEDS)
 	struct xpad_led *led;
@@ -752,7 +752,7 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
 		goto fail1;
 	}
 
-	mutex_init(&xpad->odata_mutex);
+	spin_lock_init(&xpad->odata_lock);
 
 	xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
 	if (!xpad->irq_out) {
@@ -800,13 +800,15 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
 	__u16 weak;
 	int retval;
 
+	unsigned long flags;
+
 	if (effect->type != FF_RUMBLE)
 		return 0;
 
 	strong = effect->u.rumble.strong_magnitude;
 	weak = effect->u.rumble.weak_magnitude;
 
-	mutex_lock(&xpad->odata_mutex);
+	spin_lock_irqsave(&xpad->odata_lock, flags);
 
 	switch (xpad->xtype) {
 	case XTYPE_XBOX:
@@ -864,7 +866,7 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
 		break;
 
 	default:
-		mutex_unlock(&xpad->odata_mutex);
+		spin_unlock_irqrestore(&xpad->odata_lock, flags);
 		dev_dbg(&xpad->dev->dev,
 			"%s - rumble command sent to unsupported xpad type: %d\n",
 			__func__, xpad->xtype);
@@ -880,7 +882,7 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
 				__func__);
 	}
 
-	mutex_unlock(&xpad->odata_mutex);
+	spin_unlock_irqrestore(&xpad->odata_lock, flags);
 
 	return retval;
 }
@@ -930,9 +932,11 @@ struct xpad_led {
  */
 static void xpad_send_led_command(struct usb_xpad *xpad, int command)
 {
+	unsigned long flags;
+
 	command %= 16;
 
-	mutex_lock(&xpad->odata_mutex);
+	spin_lock_irqsave(&xpad->odata_lock, flags);
 
 	switch (xpad->xtype) {
 	case XTYPE_XBOX360:
@@ -965,7 +969,7 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
 		dev_dbg(&xpad->dev->dev, "%s - dropped, irq_out is active\n",
 				__func__);
 
-	mutex_unlock(&xpad->odata_mutex);
+	spin_unlock_irqrestore(&xpad->odata_lock, flags);
 }
 
 /*
@@ -1054,6 +1058,8 @@ static int xpad_open(struct input_dev *dev)
 	struct usb_xpad *xpad = input_get_drvdata(dev);
 	int retval;
 
+	unsigned long flags;
+
 	/* URB was submitted in probe */
 	if (xpad->xtype == XTYPE_XBOX360W)
 		return 0;
@@ -1063,13 +1069,13 @@ static int xpad_open(struct input_dev *dev)
 		return -EIO;
 
 	if (xpad->xtype == XTYPE_XBOXONE) {
-		mutex_lock(&xpad->odata_mutex);
+		spin_lock_irqsave(&xpad->odata_lock, flags);
 		/* Xbox one controller needs to be initialized. */
 		xpad->odata[0] = 0x05;
 		xpad->odata[1] = 0x20;
 		xpad->irq_out->transfer_buffer_length = 2;
 		retval = usb_submit_urb(xpad->irq_out, GFP_KERNEL);
-		mutex_unlock(&xpad->odata_mutex);
+		spin_unlock_irqrestore(&xpad->odata_lock, flags);
 		return retval;
 	}
 
@@ -1213,6 +1219,8 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 	int ep_irq_in_idx;
 	int i, error;
 
+	unsigned long flags;
+
 	for (i = 0; xpad_device[i].idVendor; i++) {
 		if ((le16_to_cpu(udev->descriptor.idVendor) == xpad_device[i].idVendor) &&
 		    (le16_to_cpu(udev->descriptor.idProduct) == xpad_device[i].idProduct))
@@ -1314,7 +1322,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		 * adapter has been plugged in, as it won't automatically
 		 * send us info about the controllers.
 		 */
-		mutex_lock(&xpad->odata_mutex);
+		spin_lock_irqsave(&xpad->odata_lock, flags);
 		xpad->odata[0] = 0x08;
 		xpad->odata[1] = 0x00;
 		xpad->odata[2] = 0x0F;
@@ -1336,7 +1344,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 			dev_dbg(&xpad->dev->dev,
 				"%s - dropped, irq_out is active\n", __func__);
 
-		mutex_unlock(&xpad->odata_mutex);
+		spin_unlock_irqrestore(&xpad->odata_lock, flags);
 	} else {
 		xpad->pad_present = 1;
 		error = xpad_init_input(xpad);
-- 
1.9.1

Re: [PATCH 12/15] Input: xpad: replace mutex by spinlock

[Dmitry Torokhov]

On Thu, Oct 01, 2015 at 10:57:23PM +0200, Pavel Rojtberg wrote:
> From: Pavel Rojtberg <rojtberg@gmail.com>
> 
> since we cannot use mutexes in xpad_play_effect.
> see: http://www.spinics.net/lists/linux-input/msg40242.html

This seems to be a fixup to an earlier patch that is not in mainline.
Please fold it into the patch introducing the problem.

> 
> Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
> ---
>  drivers/input/joystick/xpad.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 5eec515..1fd2e23 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -333,7 +333,7 @@ struct usb_xpad {
>  	int irq_out_active;		/* we must not use an active URB */
>  	unsigned char *odata;		/* output data */
>  	dma_addr_t odata_dma;
> -	struct mutex odata_mutex;
> +	spinlock_t odata_lock;
>  
>  #if defined(CONFIG_JOYSTICK_XPAD_LEDS)
>  	struct xpad_led *led;
> @@ -752,7 +752,7 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
>  		goto fail1;
>  	}
>  
> -	mutex_init(&xpad->odata_mutex);
> +	spin_lock_init(&xpad->odata_lock);
>  
>  	xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
>  	if (!xpad->irq_out) {
> @@ -800,13 +800,15 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
>  	__u16 weak;
>  	int retval;
>  
> +	unsigned long flags;
> +
>  	if (effect->type != FF_RUMBLE)
>  		return 0;
>  
>  	strong = effect->u.rumble.strong_magnitude;
>  	weak = effect->u.rumble.weak_magnitude;
>  
> -	mutex_lock(&xpad->odata_mutex);
> +	spin_lock_irqsave(&xpad->odata_lock, flags);
>  
>  	switch (xpad->xtype) {
>  	case XTYPE_XBOX:
> @@ -864,7 +866,7 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
>  		break;
>  
>  	default:
> -		mutex_unlock(&xpad->odata_mutex);
> +		spin_unlock_irqrestore(&xpad->odata_lock, flags);
>  		dev_dbg(&xpad->dev->dev,
>  			"%s - rumble command sent to unsupported xpad type: %d\n",
>  			__func__, xpad->xtype);
> @@ -880,7 +882,7 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
>  				__func__);
>  	}
>  
> -	mutex_unlock(&xpad->odata_mutex);
> +	spin_unlock_irqrestore(&xpad->odata_lock, flags);
>  
>  	return retval;
>  }
> @@ -930,9 +932,11 @@ struct xpad_led {
>   */
>  static void xpad_send_led_command(struct usb_xpad *xpad, int command)
>  {
> +	unsigned long flags;
> +
>  	command %= 16;
>  
> -	mutex_lock(&xpad->odata_mutex);
> +	spin_lock_irqsave(&xpad->odata_lock, flags);
>  
>  	switch (xpad->xtype) {
>  	case XTYPE_XBOX360:
> @@ -965,7 +969,7 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
>  		dev_dbg(&xpad->dev->dev, "%s - dropped, irq_out is active\n",
>  				__func__);
>  
> -	mutex_unlock(&xpad->odata_mutex);
> +	spin_unlock_irqrestore(&xpad->odata_lock, flags);
>  }
>  
>  /*
> @@ -1054,6 +1058,8 @@ static int xpad_open(struct input_dev *dev)
>  	struct usb_xpad *xpad = input_get_drvdata(dev);
>  	int retval;
>  
> +	unsigned long flags;
> +
>  	/* URB was submitted in probe */
>  	if (xpad->xtype == XTYPE_XBOX360W)
>  		return 0;
> @@ -1063,13 +1069,13 @@ static int xpad_open(struct input_dev *dev)
>  		return -EIO;
>  
>  	if (xpad->xtype == XTYPE_XBOXONE) {
> -		mutex_lock(&xpad->odata_mutex);
> +		spin_lock_irqsave(&xpad->odata_lock, flags);
>  		/* Xbox one controller needs to be initialized. */
>  		xpad->odata[0] = 0x05;
>  		xpad->odata[1] = 0x20;
>  		xpad->irq_out->transfer_buffer_length = 2;
>  		retval = usb_submit_urb(xpad->irq_out, GFP_KERNEL);
> -		mutex_unlock(&xpad->odata_mutex);
> +		spin_unlock_irqrestore(&xpad->odata_lock, flags);
>  		return retval;
>  	}
>  
> @@ -1213,6 +1219,8 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  	int ep_irq_in_idx;
>  	int i, error;
>  
> +	unsigned long flags;
> +
>  	for (i = 0; xpad_device[i].idVendor; i++) {
>  		if ((le16_to_cpu(udev->descriptor.idVendor) == xpad_device[i].idVendor) &&
>  		    (le16_to_cpu(udev->descriptor.idProduct) == xpad_device[i].idProduct))
> @@ -1314,7 +1322,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  		 * adapter has been plugged in, as it won't automatically
>  		 * send us info about the controllers.
>  		 */
> -		mutex_lock(&xpad->odata_mutex);
> +		spin_lock_irqsave(&xpad->odata_lock, flags);
>  		xpad->odata[0] = 0x08;
>  		xpad->odata[1] = 0x00;
>  		xpad->odata[2] = 0x0F;
> @@ -1336,7 +1344,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  			dev_dbg(&xpad->dev->dev,
>  				"%s - dropped, irq_out is active\n", __func__);
>  
> -		mutex_unlock(&xpad->odata_mutex);
> +		spin_unlock_irqrestore(&xpad->odata_lock, flags);
>  	} else {
>  		xpad->pad_present = 1;
>  		error = xpad_init_input(xpad);
> -- 
> 1.9.1
> 

-- 
Dmitry

[PATCH 13/15] Input: xpad: re-submit pending ff and led requests

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

Store pending brightness and FF effect in the driver structure and
replace it with the latest requests until the device is ready to process
next request. Alternate serving LED vs FF requests to make sure one does
not starve another. See [1] for discussion. Inspired by patch of Sarah
Bessmer [2].

[1]: http://www.spinics.net/lists/linux-input/msg40708.html
[2]: http://www.spinics.net/lists/linux-input/msg31450.html

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 56 insertions(+), 10 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 1fd2e23..cd0718a 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -335,6 +335,12 @@ struct usb_xpad {
 	dma_addr_t odata_dma;
 	spinlock_t odata_lock;
 
+	unsigned char rum_odata[XPAD_PKT_LEN]; /* cache for rumble data */
+	unsigned char led_odata[XPAD_PKT_LEN]; /* cache for led data */
+	unsigned pend_rum;               /* length of cached rumble data */
+	unsigned pend_led;               /* length of cached led data */
+	int force_led;                   /* force send led cache next */
+
 #if defined(CONFIG_JOYSTICK_XPAD_LEDS)
 	struct xpad_led *led;
 #endif
@@ -705,14 +711,35 @@ static void xpad_irq_out(struct urb *urb)
 	struct usb_xpad *xpad = urb->context;
 	struct device *dev = &xpad->intf->dev;
 	int retval, status;
+	unsigned long flags;
 
 	status = urb->status;
 
 	switch (status) {
 	case 0:
 		/* success */
-		xpad->irq_out_active = 0;
-		return;
+		if(!xpad->pend_led && !xpad->pend_rum) {
+			xpad->irq_out_active = 0;
+			return;
+		}
+
+		spin_lock_irqsave(&xpad->odata_lock, flags);
+
+		if(xpad->pend_led && (!xpad->pend_rum || xpad->force_led)) {
+			xpad->irq_out->transfer_buffer_length = xpad->pend_led;
+			memcpy(xpad->odata, xpad->led_odata, xpad->pend_led);
+			xpad->pend_led = 0;
+			xpad->force_led = 0;
+			dev_dbg(dev, "%s - sending pending led\n", __func__);
+			break;
+		}
+
+		xpad->irq_out->transfer_buffer_length = xpad->pend_rum;
+		memcpy(xpad->odata, xpad->rum_odata, xpad->pend_rum);
+		xpad->pend_rum = 0;
+		xpad->force_led = 1;
+		dev_dbg(dev, "%s - sending pending rumble\n", __func__);
+		break;
 
 	case -ECONNRESET:
 	case -ENOENT:
@@ -726,11 +753,13 @@ static void xpad_irq_out(struct urb *urb)
 	default:
 		dev_dbg(dev, "%s - nonzero urb status received: %d\n",
 			__func__, status);
-		goto exit;
+
+		spin_lock_irqsave(&xpad->odata_lock, flags);
+		break;
 	}
 
-exit:
 	retval = usb_submit_urb(urb, GFP_ATOMIC);
+	spin_unlock_irqrestore(&xpad->odata_lock, flags);
 	if (retval)
 		dev_err(dev, "%s - usb_submit_urb failed with result %d\n",
 			__func__, retval);
@@ -753,6 +782,9 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
 	}
 
 	spin_lock_init(&xpad->odata_lock);
+	xpad->pend_led = 0;
+	xpad->pend_rum = 0;
+	xpad->force_led = 0;
 
 	xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
 	if (!xpad->irq_out) {
@@ -877,9 +909,17 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
 		retval = usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
 		xpad->irq_out_active = 1;
 	} else {
-		retval = -EIO;
-		dev_dbg(&xpad->dev->dev, "%s - dropped, irq_out is active\n",
-				__func__);
+		retval = 0;
+
+		if(xpad->pend_rum) {
+			dev_dbg(&xpad->dev->dev,
+				"%s - overwriting pending\n", __func__);
+
+			retval = -EIO;
+		}
+
+		xpad->pend_rum = xpad->irq_out->transfer_buffer_length;
+		memcpy(xpad->rum_odata, xpad->odata, xpad->pend_rum);
 	}
 
 	spin_unlock_irqrestore(&xpad->odata_lock, flags);
@@ -965,9 +1005,15 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
 	if (!xpad->irq_out_active) {
 		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
 		xpad->irq_out_active = 1;
-	} else
-		dev_dbg(&xpad->dev->dev, "%s - dropped, irq_out is active\n",
-				__func__);
+	} else {
+		if(xpad->pend_led) {
+			dev_dbg(&xpad->dev->dev,
+				"%s - overwriting pending\n", __func__);
+		}
+
+		xpad->pend_led = xpad->irq_out->transfer_buffer_length;
+		memcpy(xpad->led_odata, xpad->odata, xpad->pend_led);
+	}
 
 	spin_unlock_irqrestore(&xpad->odata_lock, flags);
 }
-- 
1.9.1

[PATCH 14/15] Input: xpad: workaround dead irq_out after suspend/ resume

[Pavel Rojtberg]

From: Pavel Rojtberg <rojtberg@gmail.com>

the irq_out urb is dead after suspend/ resume on my x360 wr pad. (also
reproduced by Zachary Lund [0]) Work around this by resetting the usb
device on resume. Added suspend/ resume callbacks to do so.

[0]: https://github.com/paroj/xpad/issues/6

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index cd0718a..9490d96 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1419,33 +1419,53 @@ static void xpad_deinit_input(struct usb_xpad *xpad)
 	ida_remove(&xpad_pad_seq, xpad->pad_nr);
 }
 
+static void xpad_stop_communication(struct usb_xpad *xpad) {
+	xpad_stop_output(xpad);
+
+	if (xpad->xtype == XTYPE_XBOX360W) {
+		usb_kill_urb(xpad->irq_in);
+	}
+
+	cancel_work_sync(&xpad->work);
+}
+
 static void xpad_disconnect(struct usb_interface *intf)
 {
 	struct usb_xpad *xpad = usb_get_intfdata (intf);
 
 	if (xpad->pad_present)
 		xpad_deinit_input(xpad);
-	xpad_deinit_output(xpad);
 
-	if (xpad->xtype == XTYPE_XBOX360W) {
-		usb_kill_urb(xpad->irq_in);
-	}
+	xpad_stop_communication(xpad);
+
+	xpad_deinit_output(xpad);
 
 	usb_free_urb(xpad->irq_in);
 	usb_free_coherent(xpad->udev, XPAD_PKT_LEN,
 			xpad->idata, xpad->idata_dma);
 
-	cancel_work_sync(&xpad->work);
-
 	kfree(xpad);
 
 	usb_set_intfdata(intf, NULL);
 }
 
+static int xpad_suspend(struct usb_interface *intf, pm_message_t message) {
+	struct usb_xpad *xpad = usb_get_intfdata (intf);
+	xpad_stop_communication(xpad);
+	return 0;
+}
+
+static int xpad_resume(struct usb_interface *intf) {
+	usb_queue_reset_device(intf);
+	return 0;
+}
+
 static struct usb_driver xpad_driver = {
 	.name		= "xpad",
 	.probe		= xpad_probe,
 	.disconnect	= xpad_disconnect,
+	.suspend 	= xpad_suspend,
+	.resume 	= xpad_resume,
 	.id_table	= xpad_table,
 };
 
-- 
1.9.1

[PATCH 15/15] Input: xpad: update Xbox One Force Feedback Support

[Pavel Rojtberg]

From: "Pierre-Loup A. Griffais" <githubpublic@plagman.net>

There's apparently a serial number woven into both input and output
packets; neglecting to specify a valid serial number causes the
controller to ignore the rumble packets.

The scale of the rumble was also apparently halved in the packets.

The initialization packet had to be changed to allow force feedback to
work.

see https://github.com/paroj/xpad/issues/7 for details.

Signed-off-by: Pavel Rojtberg <rojtberg@gmail.com>
---
 drivers/input/joystick/xpad.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 9490d96..8d2d4b5 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -352,6 +352,7 @@ struct usb_xpad {
 	int pad_nr;			/* the order x360 pads were attached */
 	const char *name;		/* name of the device */
 	struct work_struct work;	/* init/remove device from callback */
+	unsigned char odata_serial; /* serial number for xbox one protocol */
 };
 
 static DEFINE_SPINLOCK(xpad_pad_seq_lock);
@@ -884,17 +885,18 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
 	case XTYPE_XBOXONE:
 		xpad->odata[0] = 0x09; /* activate rumble */
 		xpad->odata[1] = 0x08;
-		xpad->odata[2] = 0x00;
+		xpad->odata[2] = xpad->odata_serial++;
 		xpad->odata[3] = 0x08; /* continuous effect */
 		xpad->odata[4] = 0x00; /* simple rumble mode */
 		xpad->odata[5] = 0x03; /* L and R actuator only */
 		xpad->odata[6] = 0x00; /* TODO: LT actuator */
 		xpad->odata[7] = 0x00; /* TODO: RT actuator */
-		xpad->odata[8] = strong / 256;	/* left actuator */
-		xpad->odata[9] = weak / 256;	/* right actuator */
+		xpad->odata[8] = strong / 512;	/* left actuator */
+		xpad->odata[9] = weak / 512;	/* right actuator */
 		xpad->odata[10] = 0x80;	/* length of pulse */
 		xpad->odata[11] = 0x00;	/* stop period of pulse */
-		xpad->irq_out->transfer_buffer_length = 12;
+		xpad->odata[12] = 0x00;
+		xpad->irq_out->transfer_buffer_length = 13;
 		break;
 
 	default:
@@ -1119,7 +1121,10 @@ static int xpad_open(struct input_dev *dev)
 		/* Xbox one controller needs to be initialized. */
 		xpad->odata[0] = 0x05;
 		xpad->odata[1] = 0x20;
-		xpad->irq_out->transfer_buffer_length = 2;
+		xpad->odata[2] = xpad->odata_serial++; /* packet serial */
+		xpad->odata[3] = 0x01; /* rumble bit enable?  */
+		xpad->odata[4] = 0x00;
+		xpad->irq_out->transfer_buffer_length = 5;
 		retval = usb_submit_urb(xpad->irq_out, GFP_KERNEL);
 		spin_unlock_irqrestore(&xpad->odata_lock, flags);
 		return retval;
-- 
1.9.1