Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 0/7] Input: xpad: Fix wireless controller connection and leds
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input

Here's a series of patches from Pierre-Loup and I that rework the xpad
driver to fix the issue where when a wireless xpad controller is plugged
in, 4 joystick devices are created, no matter how many are really
attached to the system.  Pierre-Loup's patches fix this by dynamically
creating the devices only when they are found by the wireless
controller.

Bonus is that the LEDs now work correctly with this series too, when
mixing wireless and wired, we can properly identify which device is
which on the X-LED.  We also now name the LED device based on the
joystick id, not on an atomic number that keeps incrementing, never
decrementing when removed from the system.

Note, patch 4/7 is a crazy hack to get the minor number of the joystick
id that we have registered with the system.  Pierre-Loup came up with
this, and I really can't figure out any other way to do it, the joydev
layer doesn't even know this information, otherwise I would have added
it to that layer so the xpad driver could call it.  Am I missing
something here that we could do instead?

The first patch in the series fixes a really annoying bug when plugging
in one of these controllers to a USB 3 system.  The URB type is
incorrect so the xhci driver complains about it, rightly so.  I think
the original code was incorrect, but left it alone incase there is
really a crazy device with a bulk endpoint instead of interrupt.

Also, even with this series applied, the xpad driver needs a bunch of
work.  The LED device seems pointless as it doesn't actually work, and
given that the number/name of the LED device was impossible to actually
map to the proper xpad device, there's no way userspace code could ever
actually use it.  I think it should be removed entirely.  There's also
some other USB things that should be cleaned up, the bulk urb doesn't
need to be always running (with lots of disconnect/connect cycles you
can get a warning about it running when trying to submit it again), and
the urb callbacks seem a bit strange.  I'll work on those issues next...

This series was based on a larger patch from Pierre-Loup that I broke up
into pieces, and added some of my own where needed to resolve other
issues.


^ permalink raw reply

* [PATCH 2/7] Input: xpad: set the LEDs properly on XBox Wireless controllers
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Pierre-Loup A. Griffais, Greg Kroah-Hartman
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>

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

Add the logic to set the LEDs on XBox Wireless controllers.  Command
sequence found by sniffing the Windows data stream when plugging the
device in.

Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/joystick/xpad.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 517829f6a58b..aabff9140aaa 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -715,15 +715,37 @@ struct xpad_led {
 
 static void xpad_send_led_command(struct usb_xpad *xpad, int command)
 {
-	if (command >= 0 && command < 14) {
-		mutex_lock(&xpad->odata_mutex);
+	if (command > 15)
+		return;
+
+	mutex_lock(&xpad->odata_mutex);
+
+	switch (xpad->xtype) {
+	case XTYPE_XBOX360:
 		xpad->odata[0] = 0x01;
 		xpad->odata[1] = 0x03;
 		xpad->odata[2] = command;
 		xpad->irq_out->transfer_buffer_length = 3;
-		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
-		mutex_unlock(&xpad->odata_mutex);
+		break;
+	case XTYPE_XBOX360W:
+		xpad->odata[0] = 0x00;
+		xpad->odata[1] = 0x00;
+		xpad->odata[2] = 0x08;
+		xpad->odata[3] = 0x40 + (command % 0x0e);
+		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;
+		break;
 	}
+
+	usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+	mutex_unlock(&xpad->odata_mutex);
 }
 
 static void xpad_led_set(struct led_classdev *led_cdev,
@@ -743,7 +765,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
 	struct led_classdev *led_cdev;
 	int error;
 
-	if (xpad->xtype != XTYPE_XBOX360)
+	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX360W)
 		return 0;
 
 	xpad->led = led = kzalloc(sizeof(struct xpad_led), GFP_KERNEL);
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 3/7] Input: xpad: move the input device creation to a new function
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Pierre-Loup A. Griffais, Greg Kroah-Hartman
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>

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 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>
---
 drivers/input/joystick/xpad.c | 171 ++++++++++++++++++++++++------------------
 1 file changed, 97 insertions(+), 74 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index aabff9140aaa..5b5a84dae54a 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -293,6 +293,7 @@ struct usb_xpad {
 
 	int mapping;			/* map d-pad to buttons or to axes */
 	int xtype;			/* type of xbox device */
+	const char *name;		/* name of the device */
 };
 
 /*
@@ -858,70 +859,21 @@ static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
 	}
 }
 
-static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id)
+static int xpad_init_input(struct usb_xpad *xpad)
 {
-	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 i, error;
 
-	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))
-			break;
-	}
-
-	xpad = kzalloc(sizeof(struct usb_xpad), GFP_KERNEL);
 	input_dev = input_allocate_device();
-	if (!xpad || !input_dev) {
-		error = -ENOMEM;
-		goto fail1;
-	}
-
-	xpad->idata = usb_alloc_coherent(udev, XPAD_PKT_LEN,
-					 GFP_KERNEL, &xpad->idata_dma);
-	if (!xpad->idata) {
-		error = -ENOMEM;
-		goto fail1;
-	}
-
-	xpad->irq_in = usb_alloc_urb(0, GFP_KERNEL);
-	if (!xpad->irq_in) {
-		error = -ENOMEM;
-		goto fail2;
-	}
-
-	xpad->udev = udev;
-	xpad->intf = intf;
-	xpad->mapping = xpad_device[i].mapping;
-	xpad->xtype = xpad_device[i].xtype;
-
-	if (xpad->xtype == XTYPE_UNKNOWN) {
-		if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
-			if (intf->cur_altsetting->desc.bInterfaceProtocol == 129)
-				xpad->xtype = XTYPE_XBOX360W;
-			else
-				xpad->xtype = XTYPE_XBOX360;
-		} else
-			xpad->xtype = XTYPE_XBOX;
-
-		if (dpad_to_buttons)
-			xpad->mapping |= MAP_DPAD_TO_BUTTONS;
-		if (triggers_to_buttons)
-			xpad->mapping |= MAP_TRIGGERS_TO_BUTTONS;
-		if (sticks_to_null)
-			xpad->mapping |= MAP_STICKS_TO_NULL;
-	}
+	if (!input_dev)
+		return -ENOMEM;
 
 	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->name = xpad->name;
 	input_dev->phys = xpad->phys;
-	usb_to_input_id(udev, &input_dev->id);
-	input_dev->dev.parent = &intf->dev;
+	usb_to_input_id(xpad->udev, &input_dev->id);
+	input_dev->dev.parent = &xpad->intf->dev;
 
 	input_set_drvdata(input_dev, xpad);
 
@@ -966,17 +918,92 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 			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;
+		goto fail_init_ff;
 
 	error = xpad_led_probe(xpad);
 	if (error)
-		goto fail5;
+		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 usb_endpoint_descriptor *ep_irq_in;
+	int i, error;
+
+	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))
+			break;
+	}
+
+	xpad = kzalloc(sizeof(struct usb_xpad), GFP_KERNEL);
+	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) {
+		error = -ENOMEM;
+		goto fail1;
+	}
+
+	xpad->irq_in = usb_alloc_urb(0, GFP_KERNEL);
+	if (!xpad->irq_in) {
+		error = -ENOMEM;
+		goto fail2;
+	}
+
+	xpad->udev = udev;
+	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) {
+			if (intf->cur_altsetting->desc.bInterfaceProtocol == 129)
+				xpad->xtype = XTYPE_XBOX360W;
+			else
+				xpad->xtype = XTYPE_XBOX360;
+		} else
+			xpad->xtype = XTYPE_XBOX;
+
+		if (dpad_to_buttons)
+			xpad->mapping |= MAP_DPAD_TO_BUTTONS;
+		if (triggers_to_buttons)
+			xpad->mapping |= MAP_TRIGGERS_TO_BUTTONS;
+		if (sticks_to_null)
+			xpad->mapping |= MAP_STICKS_TO_NULL;
+	}
+
+	error = xpad_init_output(intf, xpad);
+	if (error)
+		goto fail3;
 
 	ep_irq_in = &intf->cur_altsetting->endpoint[0].desc;
 	usb_fill_int_urb(xpad->irq_in, udev,
@@ -986,10 +1013,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) {
@@ -1000,7 +1023,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		xpad->bulk_out = usb_alloc_urb(0, GFP_KERNEL);
 		if (!xpad->bulk_out) {
 			error = -ENOMEM;
-			goto fail7;
+			goto fail4;
 		}
 
 		xpad->bdata = kzalloc(XPAD_PKT_LEN, GFP_KERNEL);
@@ -1048,24 +1071,24 @@ 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)
+		if (error) {
+			usb_kill_urb(xpad->irq_in);
 			goto fail9;
+		}
 	}
+	xpad->pad_present = 1;
+	error = xpad_init_input(xpad);
+	if (error)
+		goto fail9;
 
 	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);
- 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;
 
 }
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 4/7] Input: xpad: Set the correct LED number
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Pierre-Loup A. Griffais, Greg Kroah-Hartman
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>

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

The LED number should not just be incremented every time, that doesn't
work, set the number based on the number it actually is.

Note, the joydev subsystem doesn't allow us to easily find out the minor
number, so we have to walk all devices in order to find a joystick
device by looking at the name of the device.  Odds are, this isn't the
best way to do it, but I don't know of any other way at the moment.

Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/joystick/xpad.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 5b5a84dae54a..7997ae89a877 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -293,6 +293,7 @@ struct usb_xpad {
 
 	int mapping;			/* map d-pad to buttons or to axes */
 	int xtype;			/* type of xbox device */
+	int joydev_id;			/* the minor of the device */
 	const char *name;		/* name of the device */
 };
 
@@ -789,11 +790,6 @@ static int xpad_led_probe(struct usb_xpad *xpad)
 		return error;
 	}
 
-	/*
-	 * Light up the segment corresponding to controller number
-	 */
-	xpad_send_led_command(xpad, (led_no % 4) + 2);
-
 	return 0;
 }
 
@@ -809,6 +805,7 @@ static void xpad_led_disconnect(struct usb_xpad *xpad)
 #else
 static int xpad_led_probe(struct usb_xpad *xpad) { return 0; }
 static void xpad_led_disconnect(struct usb_xpad *xpad) { }
+static void xpad_send_led_command(struct usb_xpad *xpad, int command) { }
 #endif
 
 
@@ -859,9 +856,17 @@ static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
 	}
 }
 
+static int xpad_find_joydev(struct device *dev, void *data)
+{
+	if (strstr(dev_name(dev), "js"))
+		return 1;
+	return 0;
+}
+
 static int xpad_init_input(struct usb_xpad *xpad)
 {
 	struct input_dev *input_dev;
+	struct device *joydev_dev;
 	int i, error;
 
 	input_dev = input_allocate_device();
@@ -930,6 +935,17 @@ static int xpad_init_input(struct usb_xpad *xpad)
 	if (error)
 		goto fail_input_register;
 
+	joydev_dev = device_find_child(&xpad->dev->dev, NULL,
+				       xpad_find_joydev);
+	if (joydev_dev) {
+		dev_dbg(&xpad->dev->dev, "Found xpad with minor %i\n",
+			MINOR(joydev_dev->devt));
+		xpad->joydev_id = MINOR(joydev_dev->devt);
+
+		/* Light up the segment corresponding to controller number */
+		xpad_send_led_command(xpad, (xpad->joydev_id % 4) + 2);
+	}
+
 	return 0;
 
 fail_input_register:
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 5/7] Input: xpad: disconnect all Wireless controllers at init
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Pierre-Loup A. Griffais, Greg Kroah-Hartman
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>

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

We initializing the driver/device, we really don't know how many
controllers are connected.  So send a "disconnect all" command to the
base-station, and let the user pair the controllers in the order in
which they want them assigned.

Note, this means we now do not "preallocate" all 4 devices when a single
wireless base station is seen, but require the device to be properly
connected to the base station before that can happen.  The allocation of
the device happens in the next patch, not here, so in a way, this patch
breaks all wireless devices...

Because we want to talk to the device, we have to set up the output urbs
no matter if we have CONFIG_JOYSTICK_XPAD_FF or
CONFIG_JOYSTICK_XPAD_LEDS enabled not, so take out the code that allows
those variables and code to be disabled (it's so small it should never
matter...)

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

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 7997ae89a877..7a07b95790d7 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -278,12 +278,10 @@ struct usb_xpad {
 	struct urb *bulk_out;
 	unsigned char *bdata;
 
-#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
 	struct urb *irq_out;		/* urb for interrupt out report */
 	unsigned char *odata;		/* output data */
 	dma_addr_t odata_dma;
 	struct mutex odata_mutex;
-#endif
 
 #if defined(CONFIG_JOYSTICK_XPAD_LEDS)
 	struct xpad_led *led;
@@ -537,7 +535,6 @@ static void xpad_bulk_out(struct urb *urb)
 	}
 }
 
-#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
 static void xpad_irq_out(struct urb *urb)
 {
 	struct usb_xpad *xpad = urb->context;
@@ -623,11 +620,6 @@ static void xpad_deinit_output(struct usb_xpad *xpad)
 				xpad->odata, xpad->odata_dma);
 	}
 }
-#else
-static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad) { return 0; }
-static void xpad_deinit_output(struct usb_xpad *xpad) {}
-static void xpad_stop_output(struct usb_xpad *xpad) {}
-#endif
 
 #ifdef CONFIG_JOYSTICK_XPAD_FF
 static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
@@ -1091,11 +1083,41 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 			usb_kill_urb(xpad->irq_in);
 			goto fail9;
 		}
+
+		/*
+		 * We don't know how to check the controller state at driver
+		 * load, so just disconnect them all, requiring the user to
+		 * repair the device in the order they want them used.  Good
+		 * point is that we don't connect devices in "random" order,
+		 * but the extra step seems a bit harsh as other operating
+		 * systems don't require this at the moment.
+		 *
+		 * Power-off packet information came from an OS-X
+		 * reverse-engineered driver located at:
+		 * http://tattiebogle.net/index.php/ProjectRoot/Xbox360Controller/OsxDriver#toc1
+		 */
+		mutex_lock(&xpad->odata_mutex);
+		xpad->odata[0] = 0x00;
+		xpad->odata[1] = 0x00;
+		xpad->odata[2] = 0x08;
+		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);
+	} else {
+		xpad->pad_present = 1;
+		error = xpad_init_input(xpad);
+		if (error)
+			goto fail9;
 	}
-	xpad->pad_present = 1;
-	error = xpad_init_input(xpad);
-	if (error)
-		goto fail9;
 
 	return 0;
 
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 6/7] Input: xpad: handle "present" and "gone" correctly
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Pierre-Loup A. Griffais, Greg Kroah-Hartman
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>

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 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>
---
 drivers/input/joystick/xpad.c | 50 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 7a07b95790d7..d342d41a7a0d 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -293,8 +293,11 @@ struct usb_xpad {
 	int xtype;			/* type of xbox device */
 	int joydev_id;			/* the minor of the device */
 	const char *name;		/* name of the device */
+	struct work_struct work;	/* to init/remove device from callback */
 };
 
+static int xpad_init_input(struct usb_xpad *xpad);
+
 /*
  *	xpad_process_packet
  *
@@ -437,6 +440,22 @@ static void xpad360_process_packet(struct usb_xpad *xpad,
 	input_sync(dev);
 }
 
+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 {
+		input_unregister_device(xpad->dev);
+	}
+}
+
 /*
  * xpad360w_process_packet
  *
@@ -451,16 +470,22 @@ static void xpad360_process_packet(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 */
 	if (data[0] & 0x08) {
 		if (data[1] & 0x80) {
-			xpad->pad_present = 1;
-			usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
-		} else
-			xpad->pad_present = 0;
+			if (!xpad->pad_present) {
+				xpad->pad_present = 1;
+				usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
+				schedule_work(&xpad->work);
+			}
+		} else {
+			if (xpad->pad_present) {
+				xpad->pad_present = 0;
+				schedule_work(&xpad->work);
+			}
+		}
 	}
 
 	/* Valid pad data */
@@ -507,10 +532,13 @@ static void xpad_irq_in(struct urb *urb)
 	}
 
 exit:
-	retval = usb_submit_urb(urb, GFP_ATOMIC);
-	if (retval)
-		dev_err(dev, "%s - usb_submit_urb failed with result %d\n",
-			__func__, retval);
+	if (xpad->pad_present) {
+		retval = usb_submit_urb(urb, GFP_ATOMIC);
+		if (retval)
+			dev_err(dev,
+				"%s - usb_submit_urb failed with result %d\n",
+				__func__, retval);
+	}
 }
 
 static void xpad_bulk_out(struct urb *urb)
@@ -991,6 +1019,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) {
@@ -1136,7 +1165,8 @@ 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);
+	if (xpad->pad_present)
+		input_unregister_device(xpad->dev);
 	xpad_deinit_output(xpad);
 
 	if (xpad->xtype == XTYPE_XBOX360W) {
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 7/7] Input: xpad: properly name the LED class devices
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Greg Kroah-Hartman, Pierre-Loup A. Griffais
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>

Don't just increment the LED device number, but use the joystick id so
that you have a chance to associate the LED device to the correct xpad
device by the name, instead of having to use the sysfs tree, which
really doesn't work.

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

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index d342d41a7a0d..ae156de46a12 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -781,8 +781,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(0);
-	long led_no;
 	struct xpad_led *led;
 	struct led_classdev *led_cdev;
 	int error;
@@ -794,9 +792,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
 	if (!led)
 		return -ENOMEM;
 
-	led_no = (long)atomic_inc_return(&led_seq) - 1;
-
-	snprintf(led->name, sizeof(led->name), "xpad%ld", led_no);
+	snprintf(led->name, sizeof(led->name), "xpad%d", xpad->joydev_id);
 	led->xpad = xpad;
 
 	led_cdev = &led->led_cdev;
@@ -944,16 +940,17 @@ static int xpad_init_input(struct usb_xpad *xpad)
 	}
 
 	error = xpad_init_ff(xpad);
-	if (error)
-		goto fail_init_ff;
-
-	error = xpad_led_probe(xpad);
-	if (error)
-		goto fail_init_led;
+	if (error) {
+		input_free_device(input_dev);
+		return error;
+	}
 
 	error = input_register_device(xpad->dev);
-	if (error)
-		goto fail_input_register;
+	if (error) {
+		input_ff_destroy(input_dev);
+		input_free_device(input_dev);
+		return error;
+	}
 
 	joydev_dev = device_find_child(&xpad->dev->dev, NULL,
 				       xpad_find_joydev);
@@ -966,17 +963,14 @@ static int xpad_init_input(struct usb_xpad *xpad)
 		xpad_send_led_command(xpad, (xpad->joydev_id % 4) + 2);
 	}
 
-	return 0;
-
-fail_input_register:
-	xpad_led_disconnect(xpad);
-
-fail_init_led:
-	input_ff_destroy(input_dev);
+	error = xpad_led_probe(xpad);
+	if (error) {
+		input_ff_destroy(input_dev);
+		input_unregister_device(input_dev);
+		return error;
+	}
 
-fail_init_ff:
-	input_free_device(input_dev);
-	return error;
+	return 0;
 }
 
 static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id)
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 1/7] Input: xpad: use proper endpoint type
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Greg Kroah-Hartman, Pierre-Loup A. Griffais
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>

The xpad wireless endpoint is not a bulk endpoint on my devices, but
rather an interrupt one, so the USB core complains when it is submitted.
I'm guessing that the author really did mean that this should be an
interrupt urb, but as there are a zillion different xpad devices out
there, let's cover out bases and handle both bulk and interrupt
endpoints just as easily.

Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/joystick/xpad.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 603fe0dd3682..517829f6a58b 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1003,9 +1003,19 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		}
 
 		ep_irq_in = &intf->cur_altsetting->endpoint[1].desc;
-		usb_fill_bulk_urb(xpad->bulk_out, udev,
-				usb_sndbulkpipe(udev, ep_irq_in->bEndpointAddress),
-				xpad->bdata, XPAD_PKT_LEN, xpad_bulk_out, xpad);
+		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
-- 
1.8.5.3


^ permalink raw reply related

* Re: Sony Vaio Duo 11: getting middle mouse button to work
From: Benjamin Tissoires @ 2014-01-31 16:20 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Mattia Dongili, platform-driver-x86, Jiri Kosina, linux-input
In-Reply-To: <13914184.mVhhUrzN60@myon.chronox.de>

On Thu, Jan 30, 2014 at 11:31 PM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Donnerstag, 30. Januar 2014, 22:50:25 schrieb Benjamin Tissoires:
>
> Hi Benjamin,
>
>> > Can you please help me how to do that? I install Windows in a KVM with
>> > the mouse device as a USB-passthrough device -- shall I detach the Linux
>> > USB driver?
>>
>> No, you don't have to detach the Linux USB driver (it's done for you
>> by KVM IIRC).
>
> Correct, libvirtd is the one that detaches the driver when I start the VM.
>>
>> > Now, shall I use hid-recorder on that device or rather usbmon while
>> > Windows is booting?
>>
>> usbmon is the tool you need. hid-recorder will not work because the
>> hid subsystem will be disconnected as the device is used by Windows.
>>
>> You can even use wireshark now. It gives you a nice interface and
>> interpretation of the USB packets :)
>
> Please see attached for the USB sniffing on the USB bus with the Crucialtec
> touch pad when Windows starts up and shuts down. To drive the device, Windows
> uses the Crucialtec driver according to its device settings display.
>
> The attached log contains the log information for the mouse (device 001:005)
> and the USB bridge (device 001:002). I cut out the listing for the touch pad
> which is also attached to that bridge.
>
> The strange thing, however, is the following: the mouse works in Windows when
> I do not sniff on the USB bus using usbmon. But as soon as I start sniffing,
> it does not work any more.
>
> Is that log helpful?

Not really :(
I would have expected to see something like that:
XXXXXXXXXXXXXXXX XXXXXXXXXX S Co:1:XXX:0 s 21 09 0202 0002 0004 4 = XXXXXXXX
XXXXXXXXXXXXXXXX XXXXXXXXXX C Co:1:XXX:0 0 4 >

(SET_REPORT on the report ID 02 with a data length of 4)

The capture seems to confuse the windows driver and it is not
initialized correctly.

Long shot: maybe if you start the capture before attaching the device
to the Windows VM (or launching it) it will be in a better shape.

I would say that as long as the device do not work under the VM with
the capture, we are really screwed.

Cheers,
Benjamin

^ permalink raw reply

* Re: [PATCH RFC] Input: Add Microchip AR1021 i2c touchscreen
From: Dmitry Torokhov @ 2014-01-31 17:15 UTC (permalink / raw)
  To: Christian Gmeiner; +Cc: linux-input
In-Reply-To: <CAH9NwWfb1zVrgeKB7+fPyS80yC6BuokvmA4TE0tr7BAs_jwQSA@mail.gmail.com>

Hi Chrisitian,

On Fri, Jan 31, 2014 at 12:40:19PM +0100, Christian Gmeiner wrote:
> >> --- /dev/null
> >> +++ b/drivers/input/touchscreen/ar1021_i2c.c
> >> @@ -0,0 +1,201 @@
> >> +/*
> >> + * Microchip AR1021 driver for I2C
> >> + *
> >> + * Author: Christian Gmeiner <christian.gmeiner@gmail.com>
> >> + *
> >> + * License: GPL as published by the FSF.

By the way, you probably do not want GPL v1 to apply... Maybe say GPL v2
or GPL v2 and later (depending on your preference and the license of the
code you used as a base)?

> >> +
> >> +static int ar1021_i2c_resume(struct device *dev)
> >> +{
> >> +     struct i2c_client *client = to_i2c_client(dev);
> >> +
> >> +     enable_irq(client->irq);
> >
> > You do not want to enable IRQ if there are no users (nobody opened
> > device).
> >
> 
> Okay.. but then I also do not need the disable_irq(..) call in
> ar1021_i2c_suspend
> and can totally remove the PM stuff - or?

No, I think you still need the PM methods, you just need to check if
device is opened (take dev->mutex, check dev->users) and decide if you
need to enable/disable IRQ or not.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH RFC] Input: Add Microchip AR1021 i2c touchscreen
From: Dmitry Torokhov @ 2014-01-31 17:16 UTC (permalink / raw)
  To: Christian Gmeiner; +Cc: linux-input
In-Reply-To: <20140131171521.GA20872@core.coreip.homeip.net>

On Fri, Jan 31, 2014 at 09:15:21AM -0800, Dmitry Torokhov wrote:
> Hi Chrisitian,
> 
> On Fri, Jan 31, 2014 at 12:40:19PM +0100, Christian Gmeiner wrote:
> > >> --- /dev/null
> > >> +++ b/drivers/input/touchscreen/ar1021_i2c.c
> > >> @@ -0,0 +1,201 @@
> > >> +/*
> > >> + * Microchip AR1021 driver for I2C
> > >> + *
> > >> + * Author: Christian Gmeiner <christian.gmeiner@gmail.com>
> > >> + *
> > >> + * License: GPL as published by the FSF.
> 
> By the way, you probably do not want GPL v1 to apply... Maybe say GPL v2
> or GPL v2 and later (depending on your preference and the license of the
> code you used as a base)?
> 
> > >> +
> > >> +static int ar1021_i2c_resume(struct device *dev)
> > >> +{
> > >> +     struct i2c_client *client = to_i2c_client(dev);
> > >> +
> > >> +     enable_irq(client->irq);
> > >
> > > You do not want to enable IRQ if there are no users (nobody opened
> > > device).
> > >
> > 
> > Okay.. but then I also do not need the disable_irq(..) call in
> > ar1021_i2c_suspend
> > and can totally remove the PM stuff - or?
> 
> No, I think you still need the PM methods, you just need to check if
> device is opened (take dev->mutex, check dev->users) and decide if you
> need to enable/disable IRQ or not.

Hmm, on the other hand enable/disable does the counting for you so maybe
you should leave it all as it was.

-- 
Dmitry

^ permalink raw reply

* [PATCH 0/6] HID: Add a stable method for retrieving the client MAC address of a HID device
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik

Currently there is no reliable way for a device module or hidraw application to
retrieve the client MAC address of the associated wireless device.  This series
of patches adds a stable way of retrieving this information.

The first patch adds a new client_addr member to the hid_device struct.  This
member, when populated, will be guaranteed to contain the client hardware
address of a connected wireless device.

The second patch modifies HIDP to populate the client_addr member of the 
hid_device struct with the client MAC address of the connected wireless device.

The third patch adds support for setting the client_addr value in a uhid
device.

The final patches add support for retrieving the client_addr value as a string
in the hidraw interface via a new ioctl called HIDIOCGRAWCLIENTADDR.  The
hidraw documentation and example program are also updated.

^ permalink raw reply

* [PATCH 1/6] HID: Add the client_addr member to the hid_device struct
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>

Add the client_addr member to the hid_device struct for storing the client
address of a connected device.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 include/linux/hid.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 31b9d29..283fc6e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -495,6 +495,7 @@ struct hid_device {							/* device report descriptor */
 	char name[128];							/* Device name */
 	char phys[64];							/* Device physical location */
 	char uniq[64];							/* Device unique identifier (serial #) */
+	__u8 client_addr[6];						/* Device client address (if wireless) */
 
 	void *driver_data;
 
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 2/6] HID: hidp: Store the device MAC in client_addr.
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>

Have hidp store the client device MAC address in client_addr

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 net/bluetooth/hidp/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 292e619..f256f33 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -772,6 +772,9 @@ static int hidp_setup_hid(struct hidp_session *session,
 	snprintf(hid->uniq, sizeof(hid->uniq), "%pMR",
 		 &l2cap_pi(session->ctrl_sock->sk)->chan->dst);
 
+	memcpy(hid->client_addr, &l2cap_pi(session->ctrl_sock->sk)->chan->dst,
+		sizeof(hid->client_addr));
+
 	hid->dev.parent = &session->conn->hcon->dev;
 	hid->ll_driver = &hidp_hid_driver;
 
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 3/6] HID: Add support for setting client_addr in uhid.
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>

Add support for setting the client address of a device in uhid.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 drivers/hid/uhid.c        | 5 +++++
 include/uapi/linux/uhid.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index cedc6da..cfd2b93 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -259,6 +259,7 @@ struct uhid_create_req_compat {
 	__u8 name[128];
 	__u8 phys[64];
 	__u8 uniq[64];
+	__u8 client_addr[6];
 
 	compat_uptr_t rd_data;
 	__u16 rd_size;
@@ -308,6 +309,8 @@ static int uhid_event_from_user(const char __user *buffer, size_t len,
 				sizeof(compat->phys));
 			memcpy(event->u.create.uniq, compat->uniq,
 				sizeof(compat->uniq));
+			memcpy(event->u.create.client_addr, compat->client_addr,
+				sizeof(compat->client_addr));
 
 			event->u.create.rd_data = compat_ptr(compat->rd_data);
 			event->u.create.rd_size = compat->rd_size;
@@ -375,6 +378,8 @@ static int uhid_dev_create(struct uhid_device *uhid,
 	hid->phys[63] = 0;
 	strncpy(hid->uniq, ev->u.create.uniq, 63);
 	hid->uniq[63] = 0;
+	memcpy(hid->client_addr, ev->u.create.client_addr,
+		sizeof(hid->client_addr));
 
 	hid->ll_driver = &uhid_hid_driver;
 	hid->hid_get_raw_report = uhid_hid_get_raw;
diff --git a/include/uapi/linux/uhid.h b/include/uapi/linux/uhid.h
index 414b74b..5e48acc 100644
--- a/include/uapi/linux/uhid.h
+++ b/include/uapi/linux/uhid.h
@@ -40,6 +40,7 @@ struct uhid_create_req {
 	__u8 name[128];
 	__u8 phys[64];
 	__u8 uniq[64];
+	__u8 client_addr[6];
 	__u8 __user *rd_data;
 	__u16 rd_size;
 
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 5/6] HID: Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw documentation
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>

Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw documentation.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 Documentation/hid/hidraw.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/hid/hidraw.txt b/Documentation/hid/hidraw.txt
index 029e6cb..737fd1f 100644
--- a/Documentation/hid/hidraw.txt
+++ b/Documentation/hid/hidraw.txt
@@ -93,6 +93,11 @@ For USB devices, the string contains the physical path to the device (the
 USB controller, hubs, ports, etc).  For Bluetooth devices, the string
 contains the hardware (MAC) address of the device.
 
+HIDIOCGRAWCLIENTADDR(len): Get Client Address
+This ioctl returns a string representing the client address of the device.
+For Bluetooth devices, the string contains the hardware (MAC) address of the
+device.  For USB devices, the returned string is always 00:00:00:00:00:00.
+
 HIDIOCSFEATURE(len): Send a Feature Report
 This ioctl will send a feature report to the device.  Per the HID
 specification, feature reports are always sent using the control endpoint.
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 4/6] HID: Add the HIDIOCGRAWCLIENTADDR ioctl to hidraw
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>

Add a new HIDIOCGRAWCLIENTADDR ioctl to hidraw for retrieving the client
address.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 drivers/hid/hidraw.c        | 19 +++++++++++++++++++
 include/uapi/linux/hidraw.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index cb0137b..07c328b 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -447,6 +447,25 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
 						-EFAULT : len;
 					break;
 				}
+
+				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWCLIENTADDR(0))) {
+					char addr_str[32];
+					int len;
+					ret = snprintf(addr_str, sizeof(addr_str),
+							"%pMR", hid->client_addr);
+					if (ret < 0)
+						break;
+					else if (ret > sizeof(addr_str)-1)
+						len = sizeof(addr_str);
+					else
+						len = ret + 1;
+
+					if (len > _IOC_SIZE(cmd))
+						len = _IOC_SIZE(cmd);
+					ret = copy_to_user(user_arg, addr_str, len) ?
+						-EFAULT : len;
+					break;
+				}
 			}
 
 		ret = -ENOTTY;
diff --git a/include/uapi/linux/hidraw.h b/include/uapi/linux/hidraw.h
index f5b7329..5b6f4fa 100644
--- a/include/uapi/linux/hidraw.h
+++ b/include/uapi/linux/hidraw.h
@@ -38,6 +38,7 @@ struct hidraw_devinfo {
 /* The first byte of SFEATURE and GFEATURE is the report number */
 #define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
 #define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
+#define HIDIOCGRAWCLIENTADDR(len) _IOC(_IOC_READ, 'H', 0x08, len)
 
 #define HIDRAW_FIRST_MINOR 0
 #define HIDRAW_MAX_DEVICES 64
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 6/6] HID: Update the hidraw sample with the usage of HIDIOCGRAWCLIENTADDR
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>

Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw sample program.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 samples/hidraw/hid-example.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/samples/hidraw/hid-example.c b/samples/hidraw/hid-example.c
index 512a7e5..dbf072d 100644
--- a/samples/hidraw/hid-example.c
+++ b/samples/hidraw/hid-example.c
@@ -23,6 +23,10 @@
 #define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
 #define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
 #endif
+#ifndef HIDIOCGRAWCLIENTADDR
+#warning Please have your distro update the userspace kernel headers
+#define HIDIOCGRAWCLIENTADDR(len) _IOC(_IOC_READ, 'H', 0x08, len)
+#endif
 
 /* Unix */
 #include <sys/ioctl.h>
@@ -93,6 +97,13 @@ int main(int argc, char **argv)
 	else
 		printf("Raw Phys: %s\n", buf);
 
+	/* Get Client Address */
+	res = ioctl(fd, HIDIOCGRAWCLIENTADDR(256), buf);
+	if (res < 0)
+		perror("HIDIOCGRAWCLIENTADDR");
+	else
+		printf("Raw Client Address: %s\n", buf);
+
 	/* Get Raw Info */
 	res = ioctl(fd, HIDIOCGRAWINFO, &info);
 	if (res < 0) {
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCH 0/6] HID: Add a stable method for retrieving the client MAC address of a HID device
From: Benjamin Tissoires @ 2014-01-31 19:10 UTC (permalink / raw)
  To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>

Hi Frank,

just a quick review:

On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Currently there is no reliable way for a device module or hidraw application to
> retrieve the client MAC address of the associated wireless device.  This series
> of patches adds a stable way of retrieving this information.

Well, if I look at the uevent of a Bluetooth mouse I have:

$ cat /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-2/3-2\:1.0/bluetooth/hci0/hci0\:43/0005\:046D\:B00D.001F/uevent
DRIVER=hid-generic
HID_ID=0005:0000046D:0000B00D
HID_NAME=Ultrathin Touch Mouse
HID_PHYS=00:10:60:ea:df:ae
HID_UNIQ=00:1f:20:96:33:47
MODALIAS=hid:b0005g0001v0000046Dp0000B00D

I would say that HID_UNIQ is the client MAC address set by hidp, no?
So you don't need to duplicate the info by adding a new field in
hid_device.

I also have few general comments on the patches, I'll comment in them.

Cheers,
Benjamin

>
> The first patch adds a new client_addr member to the hid_device struct.  This
> member, when populated, will be guaranteed to contain the client hardware
> address of a connected wireless device.
>
> The second patch modifies HIDP to populate the client_addr member of the
> hid_device struct with the client MAC address of the connected wireless device.
>
> The third patch adds support for setting the client_addr value in a uhid
> device.
>
> The final patches add support for retrieving the client_addr value as a string
> in the hidraw interface via a new ioctl called HIDIOCGRAWCLIENTADDR.  The
> hidraw documentation and example program are also updated.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/6] HID: Add the client_addr member to the hid_device struct
From: Benjamin Tissoires @ 2014-01-31 19:12 UTC (permalink / raw)
  To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-2-git-send-email-frank.praznik@oh.rr.com>

On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add the client_addr member to the hid_device struct for storing the client
> address of a connected device.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
>  include/linux/hid.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 31b9d29..283fc6e 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -495,6 +495,7 @@ struct hid_device {                                                 /* device report descriptor */
>         char name[128];                                                 /* Device name */
>         char phys[64];                                                  /* Device physical location */
>         char uniq[64];                                                  /* Device unique identifier (serial #) */
> +       __u8 client_addr[6];                                            /* Device client address (if wireless) */

It is good to make small patches, but this one should definitively be
squashed with 2/6.
You should be able to say why you add a field in a struct by looking
at the code in the patch.

Cheers,
Benjamin

>
>         void *driver_data;
>
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 3/6] HID: Add support for setting client_addr in uhid.
From: Benjamin Tissoires @ 2014-01-31 19:16 UTC (permalink / raw)
  To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-4-git-send-email-frank.praznik@oh.rr.com>

On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add support for setting the client address of a device in uhid.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
>  drivers/hid/uhid.c        | 5 +++++
>  include/uapi/linux/uhid.h | 1 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index cedc6da..cfd2b93 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -259,6 +259,7 @@ struct uhid_create_req_compat {
>         __u8 name[128];
>         __u8 phys[64];
>         __u8 uniq[64];
> +       __u8 client_addr[6];
>
>         compat_uptr_t rd_data;
>         __u16 rd_size;
> @@ -308,6 +309,8 @@ static int uhid_event_from_user(const char __user *buffer, size_t len,
>                                 sizeof(compat->phys));
>                         memcpy(event->u.create.uniq, compat->uniq,
>                                 sizeof(compat->uniq));
> +                       memcpy(event->u.create.client_addr, compat->client_addr,
> +                               sizeof(compat->client_addr));
>
>                         event->u.create.rd_data = compat_ptr(compat->rd_data);
>                         event->u.create.rd_size = compat->rd_size;
> @@ -375,6 +378,8 @@ static int uhid_dev_create(struct uhid_device *uhid,
>         hid->phys[63] = 0;
>         strncpy(hid->uniq, ev->u.create.uniq, 63);
>         hid->uniq[63] = 0;
> +       memcpy(hid->client_addr, ev->u.create.client_addr,
> +               sizeof(hid->client_addr));
>
>         hid->ll_driver = &uhid_hid_driver;
>         hid->hid_get_raw_report = uhid_hid_get_raw;
> diff --git a/include/uapi/linux/uhid.h b/include/uapi/linux/uhid.h
> index 414b74b..5e48acc 100644
> --- a/include/uapi/linux/uhid.h
> +++ b/include/uapi/linux/uhid.h
> @@ -40,6 +40,7 @@ struct uhid_create_req {
>         __u8 name[128];
>         __u8 phys[64];
>         __u8 uniq[64];
> +       __u8 client_addr[6];

This is definitively wrong. By that I mean adding a field in the
middle of a struct in a user-space API without handling the case where
a user-space program will use the old API.

You are breaking the API and the ABI here, and you will lead to kernel
crash if the program using this has not been recompiled.
You can add fields in a struct, don't misinterpret me, but you have to
be sure that you will support both old and new API/ABI.

And yes, there are some users (programs) of uhid which would be broken by this.

Cheers,
Benjamin

>         __u8 __user *rd_data;
>         __u16 rd_size;
>
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 5/6] HID: Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw documentation
From: Benjamin Tissoires @ 2014-01-31 19:17 UTC (permalink / raw)
  To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-6-git-send-email-frank.praznik@oh.rr.com>

On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw documentation.

This one should be squashed with the previous one (same reason in 1/6)

>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
>  Documentation/hid/hidraw.txt | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/hid/hidraw.txt b/Documentation/hid/hidraw.txt
> index 029e6cb..737fd1f 100644
> --- a/Documentation/hid/hidraw.txt
> +++ b/Documentation/hid/hidraw.txt
> @@ -93,6 +93,11 @@ For USB devices, the string contains the physical path to the device (the
>  USB controller, hubs, ports, etc).  For Bluetooth devices, the string
>  contains the hardware (MAC) address of the device.
>
> +HIDIOCGRAWCLIENTADDR(len): Get Client Address
> +This ioctl returns a string representing the client address of the device.
> +For Bluetooth devices, the string contains the hardware (MAC) address of the
> +device.  For USB devices, the returned string is always 00:00:00:00:00:00.
> +
>  HIDIOCSFEATURE(len): Send a Feature Report
>  This ioctl will send a feature report to the device.  Per the HID
>  specification, feature reports are always sent using the control endpoint.
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 6/6] HID: Update the hidraw sample with the usage of HIDIOCGRAWCLIENTADDR
From: Benjamin Tissoires @ 2014-01-31 19:18 UTC (permalink / raw)
  To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-7-git-send-email-frank.praznik@oh.rr.com>

On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw sample program.
>

I would personally squash this one with the previous one, but this is
more a matter of taste IMO.

> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
>  samples/hidraw/hid-example.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/samples/hidraw/hid-example.c b/samples/hidraw/hid-example.c
> index 512a7e5..dbf072d 100644
> --- a/samples/hidraw/hid-example.c
> +++ b/samples/hidraw/hid-example.c
> @@ -23,6 +23,10 @@
>  #define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
>  #define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
>  #endif
> +#ifndef HIDIOCGRAWCLIENTADDR
> +#warning Please have your distro update the userspace kernel headers
> +#define HIDIOCGRAWCLIENTADDR(len) _IOC(_IOC_READ, 'H', 0x08, len)
> +#endif
>
>  /* Unix */
>  #include <sys/ioctl.h>
> @@ -93,6 +97,13 @@ int main(int argc, char **argv)
>         else
>                 printf("Raw Phys: %s\n", buf);
>
> +       /* Get Client Address */
> +       res = ioctl(fd, HIDIOCGRAWCLIENTADDR(256), buf);
> +       if (res < 0)
> +               perror("HIDIOCGRAWCLIENTADDR");
> +       else
> +               printf("Raw Client Address: %s\n", buf);
> +
>         /* Get Raw Info */
>         res = ioctl(fd, HIDIOCGRAWINFO, &info);
>         if (res < 0) {
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 4/6] HID: Add the HIDIOCGRAWCLIENTADDR ioctl to hidraw
From: Benjamin Tissoires @ 2014-01-31 19:19 UTC (permalink / raw)
  To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-5-git-send-email-frank.praznik@oh.rr.com>

On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add a new HIDIOCGRAWCLIENTADDR ioctl to hidraw for retrieving the client
> address.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
>  drivers/hid/hidraw.c        | 19 +++++++++++++++++++
>  include/uapi/linux/hidraw.h |  1 +
>  2 files changed, 20 insertions(+)
>
> diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
> index cb0137b..07c328b 100644
> --- a/drivers/hid/hidraw.c
> +++ b/drivers/hid/hidraw.c
> @@ -447,6 +447,25 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
>                                                 -EFAULT : len;
>                                         break;
>                                 }
> +
> +                               if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWCLIENTADDR(0))) {

Maybe we are just lacking an ioctl to retrieve hid->uniq...

Cheers,
Benjamin

> +                                       char addr_str[32];
> +                                       int len;
> +                                       ret = snprintf(addr_str, sizeof(addr_str),
> +                                                       "%pMR", hid->client_addr);
> +                                       if (ret < 0)
> +                                               break;
> +                                       else if (ret > sizeof(addr_str)-1)
> +                                               len = sizeof(addr_str);
> +                                       else
> +                                               len = ret + 1;
> +
> +                                       if (len > _IOC_SIZE(cmd))
> +                                               len = _IOC_SIZE(cmd);
> +                                       ret = copy_to_user(user_arg, addr_str, len) ?
> +                                               -EFAULT : len;
> +                                       break;
> +                               }
>                         }
>
>                 ret = -ENOTTY;
> diff --git a/include/uapi/linux/hidraw.h b/include/uapi/linux/hidraw.h
> index f5b7329..5b6f4fa 100644
> --- a/include/uapi/linux/hidraw.h
> +++ b/include/uapi/linux/hidraw.h
> @@ -38,6 +38,7 @@ struct hidraw_devinfo {
>  /* The first byte of SFEATURE and GFEATURE is the report number */
>  #define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
>  #define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
> +#define HIDIOCGRAWCLIENTADDR(len) _IOC(_IOC_READ, 'H', 0x08, len)
>
>  #define HIDRAW_FIRST_MINOR 0
>  #define HIDRAW_MAX_DEVICES 64
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v1] HID: hid-sensor-hub: Processing for duplicate physical ids
From: Srinivas Pandruvada @ 2014-01-31 20:04 UTC (permalink / raw)
  To: jkosina; +Cc: jic23, linux-input, Srinivas Pandruvada, Archana Patni

In HID sensor hub, HID physical ids are used to represent different
sensors. For example physical id of 0x73 in usage page = 0x20, represents
an accelerometer. The HID sensor hub driver uses this physical ids to
create platform devices using MFD. There is 1:1 correspondence between
an phy id and a client driver.
But in some cases these physical ids are reused. There is a phy id 0xe1,
which specifies a custom sensor, which can exist multiple times to represent
various custom sensors. In this case there can be multiple instances
of client MFD drivers, processing specific custom sensor. In this
case when client driver looks for report id or a field index, it
should still get the report id specific to its own type. This is
also true for reports, they should be directed towards correct instance.
This change introduce a way to parse and tie physical devices to their
correct instance.
Summary of changes:
- To get physical ids, use collections. If a collection of type=physical
exist then use usage id as in the name of platform device name
- As part of the platform data, we assign a hdsev instance, which has
start and end of collection indexes. Using these indexes attributes
can be tied to correct MFD client instances
- When a report is received, call callback with correct hsdev instance.
In this way using its private data stored as part of its registry, it
can distinguish different sensors even when they have same physical and
logical ids.
This patch is co-authored with Archana Patni<archna.patni@intel.com>.

Reported-by: Archana Patni <archana.patni@intel.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Archana Patni <archana.patni@intel.com>
---
 drivers/hid/hid-sensor-hub.c   | 188 ++++++++++++++++++++++-------------------
 include/linux/hid-sensor-hub.h |   6 +-
 2 files changed, 106 insertions(+), 88 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index ad2b869..5ab93cb 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -56,9 +56,9 @@ struct sensor_hub_pending {
  * @dyn_callback_lock:	spin lock to protect callback list
  * @hid_sensor_hub_client_devs:	Stores all MFD cells for a hub instance.
  * @hid_sensor_client_cnt: Number of MFD cells, (no of sensors attached).
+ * @ref_cnt:		Number of MFD clients have opened this device
  */
 struct sensor_hub_data {
-	struct hid_sensor_hub_device *hsdev;
 	struct mutex mutex;
 	spinlock_t lock;
 	struct sensor_hub_pending pending;
@@ -67,6 +67,7 @@ struct sensor_hub_data {
 	struct mfd_cell *hid_sensor_hub_client_devs;
 	int hid_sensor_client_cnt;
 	unsigned long quirks;
+	int ref_cnt;
 };
 
 /**
@@ -79,6 +80,7 @@ struct sensor_hub_data {
 struct hid_sensor_hub_callbacks_list {
 	struct list_head list;
 	u32 usage_id;
+	struct hid_sensor_hub_device *hsdev;
 	struct hid_sensor_hub_callbacks *usage_callback;
 	void *priv;
 };
@@ -97,20 +99,18 @@ static struct hid_report *sensor_hub_report(int id, struct hid_device *hdev,
 	return NULL;
 }
 
-static int sensor_hub_get_physical_device_count(
-				struct hid_report_enum *report_enum)
+static int sensor_hub_get_physical_device_count(struct hid_device *hdev)
 {
-	struct hid_report *report;
-	struct hid_field *field;
-	int cnt = 0;
+	int i;
+	int count = 0;
 
-	list_for_each_entry(report, &report_enum->report_list, list) {
-		field = report->field[0];
-		if (report->maxfield && field && field->physical)
-			cnt++;
+	for (i = 0; i < hdev->maxcollection; ++i) {
+		struct hid_collection *collection = &hdev->collection[i];
+		if (collection->type == HID_COLLECTION_PHYSICAL)
+			++count;
 	}
 
-	return cnt;
+	return count;
 }
 
 static void sensor_hub_fill_attr_info(
@@ -128,15 +128,23 @@ static void sensor_hub_fill_attr_info(
 
 static struct hid_sensor_hub_callbacks *sensor_hub_get_callback(
 					struct hid_device *hdev,
-					u32 usage_id, void **priv)
+					u32 usage_id,
+					int collection_index,
+					struct hid_sensor_hub_device **hsdev,
+					void **priv)
 {
 	struct hid_sensor_hub_callbacks_list *callback;
 	struct sensor_hub_data *pdata = hid_get_drvdata(hdev);
 
 	spin_lock(&pdata->dyn_callback_lock);
 	list_for_each_entry(callback, &pdata->dyn_callback_list, list)
-		if (callback->usage_id == usage_id) {
+		if (callback->usage_id == usage_id &&
+			(collection_index >=
+				callback->hsdev->start_collection_index) &&
+			(collection_index <
+				callback->hsdev->end_collection_index)) {
 			*priv = callback->priv;
+			*hsdev = callback->hsdev;
 			spin_unlock(&pdata->dyn_callback_lock);
 			return callback->usage_callback;
 		}
@@ -154,7 +162,8 @@ int sensor_hub_register_callback(struct hid_sensor_hub_device *hsdev,
 
 	spin_lock(&pdata->dyn_callback_lock);
 	list_for_each_entry(callback, &pdata->dyn_callback_list, list)
-		if (callback->usage_id == usage_id) {
+		if (callback->usage_id == usage_id &&
+						callback->hsdev == hsdev) {
 			spin_unlock(&pdata->dyn_callback_lock);
 			return -EINVAL;
 		}
@@ -163,6 +172,7 @@ int sensor_hub_register_callback(struct hid_sensor_hub_device *hsdev,
 		spin_unlock(&pdata->dyn_callback_lock);
 		return -ENOMEM;
 	}
+	callback->hsdev = hsdev;
 	callback->usage_callback = usage_callback;
 	callback->usage_id = usage_id;
 	callback->priv = NULL;
@@ -181,7 +191,8 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
 
 	spin_lock(&pdata->dyn_callback_lock);
 	list_for_each_entry(callback, &pdata->dyn_callback_list, list)
-		if (callback->usage_id == usage_id) {
+		if (callback->usage_id == usage_id &&
+						callback->hsdev == hsdev) {
 			list_del(&callback->list);
 			kfree(callback);
 			break;
@@ -320,8 +331,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 				struct hid_sensor_hub_attribute_info *info)
 {
 	int ret = -1;
-	int i, j;
-	int collection_index = -1;
+	int i;
 	struct hid_report *report;
 	struct hid_field *field;
 	struct hid_report_enum *report_enum;
@@ -335,44 +345,31 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 	info->units = -1;
 	info->unit_expo = -1;
 
-	for (i = 0; i < hdev->maxcollection; ++i) {
-		struct hid_collection *collection = &hdev->collection[i];
-		if (usage_id == collection->usage) {
-			collection_index = i;
-			break;
-		}
-	}
-	if (collection_index == -1)
-		goto err_ret;
-
 	report_enum = &hdev->report_enum[type];
 	list_for_each_entry(report, &report_enum->report_list, list) {
 		for (i = 0; i < report->maxfield; ++i) {
 			field = report->field[i];
-			if (field->physical == usage_id &&
-				field->logical == attr_usage_id) {
-				sensor_hub_fill_attr_info(info, i, report->id,
-							  field);
-				ret = 0;
-			} else {
-				for (j = 0; j < field->maxusage; ++j) {
-					if (field->usage[j].hid ==
-					attr_usage_id &&
-					field->usage[j].collection_index ==
-					collection_index) {
-						sensor_hub_fill_attr_info(info,
-							  i, report->id, field);
-						ret = 0;
-						break;
-					}
+			if (field->maxusage) {
+				if (field->physical == usage_id &&
+					(field->logical == attr_usage_id ||
+					field->usage[0].hid ==
+							attr_usage_id) &&
+					(field->usage[0].collection_index >=
+					hsdev->start_collection_index) &&
+					(field->usage[0].collection_index <
+					hsdev->end_collection_index)) {
+
+					sensor_hub_fill_attr_info(info, i,
+								report->id,
+								field);
+					ret = 0;
+					break;
 				}
 			}
-			if (ret == 0)
-				break;
 		}
+
 	}
 
-err_ret:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sensor_hub_input_get_attribute_info);
@@ -388,7 +385,7 @@ static int sensor_hub_suspend(struct hid_device *hdev, pm_message_t message)
 	list_for_each_entry(callback, &pdata->dyn_callback_list, list) {
 		if (callback->usage_callback->suspend)
 			callback->usage_callback->suspend(
-					pdata->hsdev, callback->priv);
+					callback->hsdev, callback->priv);
 	}
 	spin_unlock(&pdata->dyn_callback_lock);
 
@@ -405,7 +402,7 @@ static int sensor_hub_resume(struct hid_device *hdev)
 	list_for_each_entry(callback, &pdata->dyn_callback_list, list) {
 		if (callback->usage_callback->resume)
 			callback->usage_callback->resume(
-					pdata->hsdev, callback->priv);
+					callback->hsdev, callback->priv);
 	}
 	spin_unlock(&pdata->dyn_callback_lock);
 
@@ -432,6 +429,7 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
 	struct hid_sensor_hub_callbacks *callback = NULL;
 	struct hid_collection *collection = NULL;
 	void *priv = NULL;
+	struct hid_sensor_hub_device *hsdev = NULL;
 
 	hid_dbg(hdev, "sensor_hub_raw_event report id:0x%x size:%d type:%d\n",
 			 report->id, size, report->type);
@@ -466,23 +464,26 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
 				report->field[i]->usage->collection_index];
 		hid_dbg(hdev, "collection->usage %x\n",
 					collection->usage);
-		callback = sensor_hub_get_callback(pdata->hsdev->hdev,
-						report->field[i]->physical,
-							&priv);
+
+		callback = sensor_hub_get_callback(hdev,
+				report->field[i]->physical,
+				report->field[i]->usage[0].collection_index,
+				&hsdev, &priv);
+
 		if (callback && callback->capture_sample) {
 			if (report->field[i]->logical)
-				callback->capture_sample(pdata->hsdev,
+				callback->capture_sample(hsdev,
 					report->field[i]->logical, sz, ptr,
 					callback->pdev);
 			else
-				callback->capture_sample(pdata->hsdev,
+				callback->capture_sample(hsdev,
 					report->field[i]->usage->hid, sz, ptr,
 					callback->pdev);
 		}
 		ptr += sz;
 	}
 	if (callback && collection && callback->send_event)
-		callback->send_event(pdata->hsdev, collection->usage,
+		callback->send_event(hsdev, collection->usage,
 				callback->pdev);
 	spin_unlock_irqrestore(&pdata->lock, flags);
 
@@ -495,7 +496,7 @@ int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
 	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
 
 	mutex_lock(&data->mutex);
-	if (!hsdev->ref_cnt) {
+	if (!data->ref_cnt) {
 		ret = hid_hw_open(hsdev->hdev);
 		if (ret) {
 			hid_err(hsdev->hdev, "failed to open hid device\n");
@@ -503,7 +504,7 @@ int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
 			return ret;
 		}
 	}
-	hsdev->ref_cnt++;
+	data->ref_cnt++;
 	mutex_unlock(&data->mutex);
 
 	return ret;
@@ -515,8 +516,8 @@ void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev)
 	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
 
 	mutex_lock(&data->mutex);
-	hsdev->ref_cnt--;
-	if (!hsdev->ref_cnt)
+	data->ref_cnt--;
+	if (!data->ref_cnt)
 		hid_hw_close(hsdev->hdev);
 	mutex_unlock(&data->mutex);
 }
@@ -563,26 +564,19 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	struct sensor_hub_data *sd;
 	int i;
 	char *name;
-	struct hid_report *report;
-	struct hid_report_enum *report_enum;
-	struct hid_field *field;
 	int dev_cnt;
+	struct hid_sensor_hub_device *hsdev;
+	struct hid_sensor_hub_device *last_hsdev = NULL;
 
 	sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
 	if (!sd) {
 		hid_err(hdev, "cannot allocate Sensor data\n");
 		return -ENOMEM;
 	}
-	sd->hsdev = devm_kzalloc(&hdev->dev, sizeof(*sd->hsdev), GFP_KERNEL);
-	if (!sd->hsdev) {
-		hid_err(hdev, "cannot allocate hid_sensor_hub_device\n");
-		return -ENOMEM;
-	}
+
 	hid_set_drvdata(hdev, sd);
 	sd->quirks = id->driver_data;
-	sd->hsdev->hdev = hdev;
-	sd->hsdev->vendor_id = hdev->vendor;
-	sd->hsdev->product_id = hdev->product;
+
 	spin_lock_init(&sd->lock);
 	spin_lock_init(&sd->dyn_callback_lock);
 	mutex_init(&sd->mutex);
@@ -600,9 +594,8 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	}
 	INIT_LIST_HEAD(&sd->dyn_callback_list);
 	sd->hid_sensor_client_cnt = 0;
-	report_enum = &hdev->report_enum[HID_INPUT_REPORT];
 
-	dev_cnt = sensor_hub_get_physical_device_count(report_enum);
+	dev_cnt = sensor_hub_get_physical_device_count(hdev);
 	if (dev_cnt > HID_MAX_PHY_DEVICES) {
 		hid_err(hdev, "Invalid Physical device count\n");
 		ret = -EINVAL;
@@ -616,42 +609,63 @@ static int sensor_hub_probe(struct hid_device *hdev,
 			ret = -ENOMEM;
 			goto err_stop_hw;
 	}
-	list_for_each_entry(report, &report_enum->report_list, list) {
-		hid_dbg(hdev, "Report id:%x\n", report->id);
-		field = report->field[0];
-		if (report->maxfield && field &&
-					field->physical) {
+
+	for (i = 0; i < hdev->maxcollection; ++i) {
+		struct hid_collection *collection = &hdev->collection[i];
+
+		if (collection->type == HID_COLLECTION_PHYSICAL) {
+
+			hsdev = kzalloc(sizeof(*hsdev), GFP_KERNEL);
+			if (!hsdev) {
+				hid_err(hdev, "cannot allocate hid_sensor_hub_device\n");
+				ret = -ENOMEM;
+				goto err_no_mem;
+			}
+			hsdev->hdev = hdev;
+			hsdev->vendor_id = hdev->vendor;
+			hsdev->product_id = hdev->product;
+			hsdev->start_collection_index = i;
+			if (last_hsdev)
+				last_hsdev->end_collection_index = i;
+			last_hsdev = hsdev;
 			name = kasprintf(GFP_KERNEL, "HID-SENSOR-%x",
-						field->physical);
+					collection->usage);
 			if (name == NULL) {
 				hid_err(hdev, "Failed MFD device name\n");
 					ret = -ENOMEM;
-					goto err_free_names;
+					goto err_no_mem;
 			}
 			sd->hid_sensor_hub_client_devs[
-				sd->hid_sensor_client_cnt].id = PLATFORM_DEVID_AUTO;
+				sd->hid_sensor_client_cnt].id =
+							PLATFORM_DEVID_AUTO;
 			sd->hid_sensor_hub_client_devs[
 				sd->hid_sensor_client_cnt].name = name;
 			sd->hid_sensor_hub_client_devs[
 				sd->hid_sensor_client_cnt].platform_data =
-						sd->hsdev;
+							hsdev;
 			sd->hid_sensor_hub_client_devs[
 				sd->hid_sensor_client_cnt].pdata_size =
-						sizeof(*sd->hsdev);
-			hid_dbg(hdev, "Adding %s:%p\n", name, sd);
+							sizeof(*hsdev);
+			hid_dbg(hdev, "Adding %s:%d\n", name,
+					hsdev->start_collection_index);
 			sd->hid_sensor_client_cnt++;
 		}
 	}
+	if (last_hsdev)
+		last_hsdev->end_collection_index = i;
+
 	ret = mfd_add_devices(&hdev->dev, 0, sd->hid_sensor_hub_client_devs,
 		sd->hid_sensor_client_cnt, NULL, 0, NULL);
 	if (ret < 0)
-		goto err_free_names;
+		goto err_no_mem;
 
 	return ret;
 
-err_free_names:
-	for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
+err_no_mem:
+	for (i = 0; i < sd->hid_sensor_client_cnt; ++i) {
 		kfree(sd->hid_sensor_hub_client_devs[i].name);
+		kfree(sd->hid_sensor_hub_client_devs[i].platform_data);
+	}
 	kfree(sd->hid_sensor_hub_client_devs);
 err_stop_hw:
 	hid_hw_stop(hdev);
@@ -673,8 +687,10 @@ static void sensor_hub_remove(struct hid_device *hdev)
 		complete(&data->pending.ready);
 	spin_unlock_irqrestore(&data->lock, flags);
 	mfd_remove_devices(&hdev->dev);
-	for (i = 0; i < data->hid_sensor_client_cnt ; ++i)
+	for (i = 0; i < data->hid_sensor_client_cnt; ++i) {
 		kfree(data->hid_sensor_hub_client_devs[i].name);
+		kfree(data->hid_sensor_hub_client_devs[i].platform_data);
+	}
 	kfree(data->hid_sensor_hub_client_devs);
 	hid_set_drvdata(hdev, NULL);
 	mutex_destroy(&data->mutex);
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index 205eba0..b70cfd7 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -51,13 +51,15 @@ struct hid_sensor_hub_attribute_info {
  * @hdev:		Stores the hid instance.
  * @vendor_id:		Vendor id of hub device.
  * @product_id:		Product id of hub device.
- * @ref_cnt:		Number of MFD clients have opened this device
+ * @start_collection_index: Starting index for a phy type collection
+ * @end_collection_index: Last index for a phy type collection
  */
 struct hid_sensor_hub_device {
 	struct hid_device *hdev;
 	u32 vendor_id;
 	u32 product_id;
-	int ref_cnt;
+	int start_collection_index;
+	int end_collection_index;
 };
 
 /**
-- 
1.8.3.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox