All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] NEXIO (or iNexio) support for usbtouchscreen
@ 2009-11-16 14:14 Ondrej Zary
  2009-11-17 15:25 ` Oliver Neukum
  0 siblings, 1 reply; 14+ messages in thread
From: Ondrej Zary @ 2009-11-16 14:14 UTC (permalink / raw)
  To: daniel.ritz; +Cc: linux-kernel

Hello,
this adds support for NEXIO (or iNexio) USB touchscreens to usbtouchscreen
driver. Tested with NEX170MRT 17" LCD monitor with integrated touchscreen
(with xserver-xorg-input-evtouch 0.8.8-1):

T:  Bus=02 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 54 Spd=12  MxCh= 0
D:  Ver= 1.10 Cls=02(comm.) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
P:  Vendor=1870 ProdID=0001 Rev= 1.00
S:  Manufacturer=iNexio
S:  Product=iNexio USB
C:* #Ifs= 2 Cfg#= 1 Atr=c0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=02 Prot=00 Driver=(none)
E:  Ad=83(I) Atr=03(Int.) MxPS=   8 Ivl=255ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=(none)
E:  Ad=01(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms

No datasheet is available, this was written by capturing some data with
SniffUSB in Windows:
http://www.rainbow-software.org/linux_files/nexio/

The device is a bit specific (read: broken) in at least 5 ways:
1. It's very slow to initialize (takes many seconds)
2. The driver must communicate with the device or it disconnects
   (that's why the irq URB is sent during init)
3. The endpoints are mixed up somehow, irq URB works on EP 0x82, the
   first interface with endpoint 0x83 is probably useless
4. It has weird limited multi-touch capability
5. It sends the X and Y range only in touch data, not during initialization


Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c.orig	2009-09-10 00:13:59.000000000 +0200
+++ linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c	2009-11-16 14:37:15.000000000 +0100
@@ -13,6 +13,7 @@
  *  - IdealTEK URTC1000
  *  - General Touch
  *  - GoTop Super_Q2/GogoPen/PenPower tablets
+ *  - NEXIO/iNexio
  *
  * Copyright (C) 2004-2007 by Daniel Ritz <daniel.ritz@gmx.ch>
  * Copyright (C) by Todd E. Johnson (mtouchusb.c)
@@ -71,6 +72,8 @@
 	int min_yc, max_yc;
 	int min_press, max_press;
 	int rept_size;
+	bool no_urb_in_open;
+	int endpoint, interval;
 
 	void (*process_pkt) (struct usbtouch_usb *usbtouch, unsigned char *pkt, int len);
 
@@ -92,7 +95,7 @@
 	dma_addr_t data_dma;
 	unsigned char *buffer;
 	int buf_len;
-	struct urb *irq;
+	struct urb *irq, *ack;
 	struct usb_device *udev;
 	struct input_dev *input;
 	struct usbtouch_device_info *type;
@@ -118,6 +121,7 @@
 	DEVTYPE_IDEALTEK,
 	DEVTYPE_GENERAL_TOUCH,
 	DEVTYPE_GOTOP,
+	DEVTYPE_NEXIO,
 };
 
 #define USB_DEVICE_HID_CLASS(vend, prod) \
@@ -191,6 +195,17 @@
 	{USB_DEVICE(0x08f2, 0x00f4), .driver_info = DEVTYPE_GOTOP},
 #endif
 
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	/* ignore the comm interface */
+	{USB_DEVICE_AND_INTERFACE_INFO(0x10f0, 0x2002, 0x02, 0x02, 0x00),
+		.driver_info = DEVTYPE_IGNORE},
+	{USB_DEVICE_AND_INTERFACE_INFO(0x1870, 0x0001, 0x02, 0x02, 0x00),
+		.driver_info = DEVTYPE_IGNORE},
+	/* normal device IDs */
+	{USB_DEVICE(0x10f0, 0x2002), .driver_info = DEVTYPE_NEXIO},
+	{USB_DEVICE(0x1870, 0x0001), .driver_info = DEVTYPE_NEXIO},
+#endif
+
 	{}
 };
 
@@ -563,6 +578,167 @@
 }
 #endif
 
+/*****************************************************************************
+ * NEXIO Part
+ */
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+
+#define NEXIO_OUTPUT_EP	0x01
+#define NEXIO_INPUT_EP	0x82
+#define NEXIO_TIMEOUT	5000
+#define NEXIO_BUFSIZE	1024
+#define NEXIO_THRESHOLD	50
+
+struct nexio_touch_packet {
+	u8	flags;		/* 0xe1 = touch, 0xe1 = release */
+	__be16	data_len;	/* total bytes of touch data */
+	__be16	x_len;		/* bytes for X axis */
+	__be16	y_len;		/* bytes for Y axis */
+	u8	data[];
+} __attribute__ ((packed));
+
+static unsigned char nexio_ack[2] = { 0xaa, 0x02 };
+
+static int nexio_init(struct usbtouch_usb *usbtouch)
+{
+	struct usb_device *dev = usbtouch->udev;
+	int ret = -ENOMEM;
+	int actual_len;
+	unsigned char *buf;
+	unsigned char init[4] = { 0x82, 0x04, 0x0a, 0x0f };
+	char *firmware_ver;
+
+	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
+	if (!buf)
+		goto err_nobuf;
+	/* two reads */
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* send init command */
+	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP), init,
+			   sizeof(init), &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* read it back */
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			  NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* read firmware version */
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	buf[buf[1]] = 0;	/* second byte is data(string) length */
+	firmware_ver = kstrdup(&buf[2], GFP_KERNEL);
+	/* read device name */
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0) {
+		kfree(firmware_ver);
+		goto err_out;
+	}
+	buf[buf[1]] = 0;
+	printk(KERN_INFO "Nexio device: %s, firmware version %s\n",
+	       &buf[2], firmware_ver);
+	kfree(firmware_ver);
+
+	/* prepare ACK URB */
+	usbtouch->ack = usb_alloc_urb(0, GFP_KERNEL);
+	if (!usbtouch->ack) {
+		dbg("%s - usb_alloc_urb failed: ack_urb", __func__);
+		return 0;
+	}
+	usb_fill_bulk_urb(usbtouch->ack, usbtouch->udev,
+			 usb_sndbulkpipe(usbtouch->udev, NEXIO_OUTPUT_EP),
+			 nexio_ack, sizeof(nexio_ack), NULL, usbtouch);
+	/* submit first IRQ URB */
+	ret = usb_submit_urb(usbtouch->irq, GFP_KERNEL);
+err_out:
+	kfree(buf);
+err_nobuf:
+	return ret;
+}
+
+static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt)
+{
+	int x, y, begin_x, begin_y, end_x, end_y, w, h, ret;
+	struct nexio_touch_packet *packet = (void *) pkt;
+
+	/* got touch data? */
+	if ((pkt[0] & 0xe0) != 0xe0)
+		return 0;
+
+	/* send ACK */
+	ret = usb_submit_urb(usbtouch->ack, GFP_KERNEL);
+
+	if (!usbtouch->type->max_xc) {
+		usbtouch->type->max_xc = 2 * be16_to_cpu(packet->x_len);
+		input_set_abs_params(usbtouch->input, ABS_X, 0,
+				     2 * be16_to_cpu(packet->x_len), 0, 0);
+		usbtouch->type->max_yc = 2 * be16_to_cpu(packet->y_len);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0,
+				     2 * be16_to_cpu(packet->y_len), 0, 0);
+	}
+	/* The device reports state of IR sensors on X and Y axes.
+	 * Each byte represents "darkness" percentage (0-100) of one element.
+	 * 17" touchscreen reports only 64 x 52 bytes so the resolution is low.
+	 * This also means that there's a limited multi-touch capability but
+	 * it's disabled (and untested) here as there's no X driver for that.
+	 */
+	begin_x = end_x = begin_y = end_y = -1;
+	for (x = 0; x < be16_to_cpu(packet->x_len); x++) {
+		if (begin_x == -1 && packet->data[x] > NEXIO_THRESHOLD) {
+			begin_x = x;
+			continue;
+		}
+		if (end_x == -1 && begin_x != -1 && packet->data[x] < NEXIO_THRESHOLD) {
+			end_x = x - 1;
+			for (y = be16_to_cpu(packet->x_len);
+			     y < be16_to_cpu(packet->data_len); y++) {
+				if (begin_y == -1 && packet->data[y] > NEXIO_THRESHOLD) {
+					begin_y = y - be16_to_cpu(packet->x_len);
+					continue;
+				}
+				if (end_y == -1 &&
+				    begin_y != -1 && packet->data[y] < NEXIO_THRESHOLD) {
+					end_y = y - 1 - be16_to_cpu(packet->x_len);
+					w = end_x - begin_x;
+					h = end_y - begin_y;
+					/* multi-touch */
+/*					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MAJOR, max(w,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MINOR, min(x,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_X, 2*begin_x+w);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_Y, 2*begin_y+h);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_ORIENTATION, w > h);
+					input_mt_sync(usbtouch->input);*/
+					/* single touch */
+					usbtouch->x = 2 * begin_x + w;
+					usbtouch->y = 2 * begin_y + h;
+					usbtouch->touch = packet->flags & 0x01;
+					begin_y = end_y = -1;
+					return 1;
+				}
+			}
+			begin_x = end_x = -1;
+		}
+
+	}
+	return 0;
+}
+#endif
+
 
 /*****************************************************************************
  * the different device descriptors
@@ -702,6 +878,17 @@
 		.read_data	= gotop_read_data,
 	},
 #endif
+
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	[DEVTYPE_NEXIO] = {
+		.rept_size	= 128,
+		.no_urb_in_open = 1,
+		.endpoint	= NEXIO_INPUT_EP,
+		.interval	= 0xff,
+		.read_data	= nexio_read_data,
+		.init		= nexio_init,
+	},
+#endif
 };
 
 
@@ -852,8 +1039,9 @@
 
 	usbtouch->irq->dev = usbtouch->udev;
 
-	if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
-		return -EIO;
+	if (!usbtouch->type->no_urb_in_open)
+		if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
+			return -EIO;
 
 	return 0;
 }
@@ -960,10 +1148,12 @@
 		                     type->max_press, 0, 0);
 
 	usb_fill_int_urb(usbtouch->irq, usbtouch->udev,
-			 usb_rcvintpipe(usbtouch->udev, endpoint->bEndpointAddress),
+			 usb_rcvintpipe(usbtouch->udev,
+					type->endpoint ? type->endpoint
+							: endpoint->bEndpointAddress),
 			 usbtouch->data, type->rept_size,
-			 usbtouch_irq, usbtouch, endpoint->bInterval);
-
+			 usbtouch_irq, usbtouch,
+			 type->interval ? type->interval : endpoint->bInterval);
 	usbtouch->irq->dev = usbtouch->udev;
 	usbtouch->irq->transfer_dma = usbtouch->data_dma;
 	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
@@ -1009,6 +1199,7 @@
 	usb_kill_urb(usbtouch->irq);
 	input_unregister_device(usbtouch->input);
 	usb_free_urb(usbtouch->irq);
+	usb_free_urb(usbtouch->ack);
 	usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
 	kfree(usbtouch);
 }


-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] NEXIO (or iNexio) support for usbtouchscreen
  2009-11-16 14:14 [PATCH] NEXIO (or iNexio) support for usbtouchscreen Ondrej Zary
@ 2009-11-17 15:25 ` Oliver Neukum
  2009-11-18 12:18   ` [PATCH v2] [RFC] " Ondrej Zary
  0 siblings, 1 reply; 14+ messages in thread
From: Oliver Neukum @ 2009-11-17 15:25 UTC (permalink / raw)
  To: Ondrej Zary, linux-usb; +Cc: daniel.ritz, linux-kernel

Am Montag, 16. November 2009 15:14:59 schrieb Ondrej Zary:

Hi,

firstly can you please send patches with -up? It makes them more readable.
> ---

> @@ -92,7 +95,7 @@
>  	dma_addr_t data_dma;
>  	unsigned char *buffer;
>  	int buf_len;
> -	struct urb *irq;
> +	struct urb *irq, *ack;

Where is this urb handled in case of disconnection?

> +#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
> +	/* ignore the comm interface */
> +	{USB_DEVICE_AND_INTERFACE_INFO(0x10f0, 0x2002, 0x02, 0x02, 0x00),
> +		.driver_info = DEVTYPE_IGNORE},
> +	{USB_DEVICE_AND_INTERFACE_INFO(0x1870, 0x0001, 0x02, 0x02, 0x00),
> +		.driver_info = DEVTYPE_IGNORE},
> +	/* normal device IDs */
> +	{USB_DEVICE(0x10f0, 0x2002), .driver_info = DEVTYPE_NEXIO},
> +	{USB_DEVICE(0x1870, 0x0001), .driver_info = DEVTYPE_NEXIO},

Why not go for the interfaces you want?

> +static unsigned char nexio_ack[2] = { 0xaa, 0x02 };
> +
> +static int nexio_init(struct usbtouch_usb *usbtouch)
> +{
> +	struct usb_device *dev = usbtouch->udev;
> +	int ret = -ENOMEM;
> +	int actual_len;
> +	unsigned char *buf;
> +	unsigned char init[4] = { 0x82, 0x04, 0x0a, 0x0f };
> +	char *firmware_ver;
> +
> +	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
> +	if (!buf)
> +		goto err_nobuf;
> +	/* two reads */
> +	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
> +			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
> +	if (ret < 0)
> +		goto err_out;
> +	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
> +			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
> +	if (ret < 0)
> +		goto err_out;
> +	/* send init command */
> +	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP), init,
> +			   sizeof(init), &actual_len, NEXIO_TIMEOUT);

DMA on the kernel stack

	Regards
		Oliver



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] [RFC] NEXIO (or iNexio) support for usbtouchscreen
  2009-11-17 15:25 ` Oliver Neukum
@ 2009-11-18 12:18   ` Ondrej Zary
  2009-11-18 15:25     ` Oliver Neukum
  0 siblings, 1 reply; 14+ messages in thread
From: Ondrej Zary @ 2009-11-18 12:18 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: linux-usb, daniel.ritz, linux-kernel

On Tuesday 17 November 2009, Oliver Neukum wrote:
> Am Montag, 16. November 2009 15:14:59 schrieb Ondrej Zary:
>
> Hi,
>
> firstly can you please send patches with -up? It makes them more readable.
>
> > ---
> >
> > @@ -92,7 +95,7 @@
> >  	dma_addr_t data_dma;
> >  	unsigned char *buffer;
> >  	int buf_len;
> > -	struct urb *irq;
> > +	struct urb *irq, *ack;
>
> Where is this urb handled in case of disconnection?

See the new patch below.

> > +#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
> > +	/* ignore the comm interface */
> > +	{USB_DEVICE_AND_INTERFACE_INFO(0x10f0, 0x2002, 0x02, 0x02, 0x00),
> > +		.driver_info = DEVTYPE_IGNORE},
> > +	{USB_DEVICE_AND_INTERFACE_INFO(0x1870, 0x0001, 0x02, 0x02, 0x00),
> > +		.driver_info = DEVTYPE_IGNORE},
> > +	/* normal device IDs */
> > +	{USB_DEVICE(0x10f0, 0x2002), .driver_info = DEVTYPE_NEXIO},
> > +	{USB_DEVICE(0x1870, 0x0001), .driver_info = DEVTYPE_NEXIO},
>
> Why not go for the interfaces you want?

See the new patch below.

> > +static unsigned char nexio_ack[2] = { 0xaa, 0x02 };
> > +
> > +static int nexio_init(struct usbtouch_usb *usbtouch)
> > +{
> > +	struct usb_device *dev = usbtouch->udev;
> > +	int ret = -ENOMEM;
> > +	int actual_len;
> > +	unsigned char *buf;
> > +	unsigned char init[4] = { 0x82, 0x04, 0x0a, 0x0f };
> > +	char *firmware_ver;
> > +
> > +	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
> > +	if (!buf)
> > +		goto err_nobuf;
> > +	/* two reads */
> > +	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
> > +			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
> > +	if (ret < 0)
> > +		goto err_out;
> > +	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
> > +			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
> > +	if (ret < 0)
> > +		goto err_out;
> > +	/* send init command */
> > +	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP), init,
> > +			   sizeof(init), &actual_len, NEXIO_TIMEOUT);
>
> DMA on the kernel stack

I don't know any details about this - what's the correct way to do this?
I tried moving the init[] array outside the function but that didn't work at
all - compiled fine but device was not reporting the name firmware version.
I ended up with copying it to that kmalloced buf.

I'm worried about nexio_ack - that shouldn't work too then. Maybe it really
does not and the device does not care.

I also found another problem: when I disconnect the USB cable, kernel oopses:
BUG: unable to handle kernel NULL pointer dereference
IP: [<f7c794fa>] start_unlink_async+0x63/0xfc

Call Trace:
ehci_urb_dequeue+0x7c/0x11a [ehci_hcd]
unlink1+0xaa/0xc7 [usbcore]
usb_hcd_unlink_urb+0x57/0x84 [usbcore]
usb_kill_urb+0x40/0xbe [usbcore]
default_wake_function+0x0/0x2b
usb_start_wait_urb+0x6e/0xb0 [usbcore]
usb_control_msg+0x10a/0x136 [usbcore]
hub_port_status+0x77/0xf7 [usbcore]
hub_thread+0x56d/0xe14 [usbcore]
autoremove_wake_function+0x0/0x4f
hub_thread+0x0/0xe14 [usbcore]
kthread+0x7a/0x7f
kthread+0x0/0x7f

Are there any other problems with my code that can cause this oops?




--- linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c.orig	2009-09-10 00:13:59.000000000 +0200
+++ linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c	2009-11-18 12:32:19.000000000 +0100
@@ -13,6 +13,7 @@
  *  - IdealTEK URTC1000
  *  - General Touch
  *  - GoTop Super_Q2/GogoPen/PenPower tablets
+ *  - NEXIO/iNexio
  *
  * Copyright (C) 2004-2007 by Daniel Ritz <daniel.ritz@gmx.ch>
  * Copyright (C) by Todd E. Johnson (mtouchusb.c)
@@ -71,6 +72,8 @@ struct usbtouch_device_info {
 	int min_yc, max_yc;
 	int min_press, max_press;
 	int rept_size;
+	bool no_urb_in_open;
+	int endpoint, interval;
 
 	void (*process_pkt) (struct usbtouch_usb *usbtouch, unsigned char *pkt, int len);
 
@@ -92,7 +95,7 @@ struct usbtouch_usb {
 	dma_addr_t data_dma;
 	unsigned char *buffer;
 	int buf_len;
-	struct urb *irq;
+	struct urb *irq, *ack;
 	struct usb_device *udev;
 	struct input_dev *input;
 	struct usbtouch_device_info *type;
@@ -118,6 +121,7 @@ enum {
 	DEVTYPE_IDEALTEK,
 	DEVTYPE_GENERAL_TOUCH,
 	DEVTYPE_GOTOP,
+	DEVTYPE_NEXIO,
 };
 
 #define USB_DEVICE_HID_CLASS(vend, prod) \
@@ -191,6 +195,14 @@ static struct usb_device_id usbtouch_dev
 	{USB_DEVICE(0x08f2, 0x00f4), .driver_info = DEVTYPE_GOTOP},
 #endif
 
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	/* data interface only */
+	{USB_DEVICE_AND_INTERFACE_INFO(0x10f0, 0x2002, 0x0a, 0x00, 0x00),
+		.driver_info = DEVTYPE_NEXIO},
+	{USB_DEVICE_AND_INTERFACE_INFO(0x1870, 0x0001, 0x0a, 0x00, 0x00),
+		.driver_info = DEVTYPE_NEXIO},
+#endif
+
 	{}
 };
 
@@ -563,6 +575,170 @@ static int gotop_read_data(struct usbtou
 }
 #endif
 
+/*****************************************************************************
+ * NEXIO Part
+ */
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+
+#define NEXIO_OUTPUT_EP	0x01
+#define NEXIO_INPUT_EP	0x82
+#define NEXIO_TIMEOUT	5000
+#define NEXIO_BUFSIZE	1024
+#define NEXIO_THRESHOLD	50
+
+struct nexio_touch_packet {
+	u8	flags;		/* 0xe1 = touch, 0xe1 = release */
+	__be16	data_len;	/* total bytes of touch data */
+	__be16	x_len;		/* bytes for X axis */
+	__be16	y_len;		/* bytes for Y axis */
+	u8	data[];
+} __attribute__ ((packed));
+
+static unsigned char nexio_ack_pkt[2] = { 0xaa, 0x02 };
+static unsigned char nexio_init_pkt[4] = { 0x82, 0x04, 0x0a, 0x0f };
+
+static int nexio_init(struct usbtouch_usb *usbtouch)
+{
+	struct usb_device *dev = usbtouch->udev;
+	int ret = -ENOMEM;
+	int actual_len;
+	unsigned char *buf;
+	char *firmware_ver;
+
+	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
+	if (!buf)
+		goto err_nobuf;
+	/* two reads */
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* send init command */
+	memcpy(buf, nexio_init_pkt, sizeof(nexio_init_pkt));
+	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP),
+			   buf, sizeof(nexio_init_pkt), &actual_len,
+			   NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* read it back */
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* read firmware version */
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	buf[buf[1]] = 0;	/* second byte is data(string) length */
+	firmware_ver = kstrdup(&buf[2], GFP_KERNEL);
+	/* read device name */
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0) {
+		kfree(firmware_ver);
+		goto err_out;
+	}
+	buf[buf[1]] = 0;
+	printk(KERN_INFO "Nexio device: %s, firmware version %s\n",
+	       &buf[2], firmware_ver);
+	kfree(firmware_ver);
+
+	/* prepare ACK URB */
+	usbtouch->ack = usb_alloc_urb(0, GFP_KERNEL);
+	if (!usbtouch->ack) {
+		dbg("%s - usb_alloc_urb failed: ack_urb", __func__);
+		return 0;
+	}
+	usb_fill_bulk_urb(usbtouch->ack, usbtouch->udev,
+			  usb_sndbulkpipe(usbtouch->udev, NEXIO_OUTPUT_EP),
+			  nexio_ack_pkt, sizeof(nexio_ack_pkt), NULL,
+			  usbtouch);
+	/* submit first IRQ URB */
+	ret = usb_submit_urb(usbtouch->irq, GFP_KERNEL);
+err_out:
+	kfree(buf);
+err_nobuf:
+	return ret;
+}
+
+static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt)
+{
+	int x, y, begin_x, begin_y, end_x, end_y, w, h, ret;
+	struct nexio_touch_packet *packet = (void *) pkt;
+
+	/* got touch data? */
+	if ((pkt[0] & 0xe0) != 0xe0)
+		return 0;
+
+	/* send ACK */
+	ret = usb_submit_urb(usbtouch->ack, GFP_KERNEL);
+
+	if (!usbtouch->type->max_xc) {
+		usbtouch->type->max_xc = 2 * be16_to_cpu(packet->x_len);
+		input_set_abs_params(usbtouch->input, ABS_X, 0,
+				     2 * be16_to_cpu(packet->x_len), 0, 0);
+		usbtouch->type->max_yc = 2 * be16_to_cpu(packet->y_len);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0,
+				     2 * be16_to_cpu(packet->y_len), 0, 0);
+	}
+	/* The device reports state of IR sensors on X and Y axes.
+	 * Each byte represents "darkness" percentage (0-100) of one element.
+	 * 17" touchscreen reports only 64 x 52 bytes so the resolution is low.
+	 * This also means that there's a limited multi-touch capability but
+	 * it's disabled (and untested) here as there's no X driver for that.
+	 */
+	begin_x = end_x = begin_y = end_y = -1;
+	for (x = 0; x < be16_to_cpu(packet->x_len); x++) {
+		if (begin_x == -1 && packet->data[x] > NEXIO_THRESHOLD) {
+			begin_x = x;
+			continue;
+		}
+		if (end_x == -1 && begin_x != -1 && packet->data[x] < NEXIO_THRESHOLD) {
+			end_x = x - 1;
+			for (y = be16_to_cpu(packet->x_len);
+			     y < be16_to_cpu(packet->data_len); y++) {
+				if (begin_y == -1 && packet->data[y] > NEXIO_THRESHOLD) {
+					begin_y = y - be16_to_cpu(packet->x_len);
+					continue;
+				}
+				if (end_y == -1 &&
+				    begin_y != -1 && packet->data[y] < NEXIO_THRESHOLD) {
+					end_y = y - 1 - be16_to_cpu(packet->x_len);
+					w = end_x - begin_x;
+					h = end_y - begin_y;
+					/* multi-touch */
+/*					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MAJOR, max(w,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MINOR, min(x,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_X, 2*begin_x+w);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_Y, 2*begin_y+h);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_ORIENTATION, w > h);
+					input_mt_sync(usbtouch->input);*/
+					/* single touch */
+					usbtouch->x = 2 * begin_x + w;
+					usbtouch->y = 2 * begin_y + h;
+					usbtouch->touch = packet->flags & 0x01;
+					begin_y = end_y = -1;
+					return 1;
+				}
+			}
+			begin_x = end_x = -1;
+		}
+
+	}
+	return 0;
+}
+#endif
+
 
 /*****************************************************************************
  * the different device descriptors
@@ -702,6 +878,17 @@ static struct usbtouch_device_info usbto
 		.read_data	= gotop_read_data,
 	},
 #endif
+
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	[DEVTYPE_NEXIO] = {
+		.rept_size	= 128,
+		.no_urb_in_open = 1,
+		.endpoint	= NEXIO_INPUT_EP,
+		.interval	= 0xff,
+		.read_data	= nexio_read_data,
+		.init		= nexio_init,
+	},
+#endif
 };
 
 
@@ -852,8 +1039,9 @@ static int usbtouch_open(struct input_de
 
 	usbtouch->irq->dev = usbtouch->udev;
 
-	if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
-		return -EIO;
+	if (!usbtouch->type->no_urb_in_open)
+		if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
+			return -EIO;
 
 	return 0;
 }
@@ -862,7 +1050,8 @@ static void usbtouch_close(struct input_
 {
 	struct usbtouch_usb *usbtouch = input_get_drvdata(input);
 
-	usb_kill_urb(usbtouch->irq);
+	if (!usbtouch->type->no_urb_in_open)
+		usb_kill_urb(usbtouch->irq);
 }
 
 
@@ -960,10 +1149,12 @@ static int usbtouch_probe(struct usb_int
 		                     type->max_press, 0, 0);
 
 	usb_fill_int_urb(usbtouch->irq, usbtouch->udev,
-			 usb_rcvintpipe(usbtouch->udev, endpoint->bEndpointAddress),
+			 usb_rcvintpipe(usbtouch->udev,
+					type->endpoint ? type->endpoint
+							: endpoint->bEndpointAddress),
 			 usbtouch->data, type->rept_size,
-			 usbtouch_irq, usbtouch, endpoint->bInterval);
-
+			 usbtouch_irq, usbtouch,
+			 type->interval ? type->interval : endpoint->bInterval);
 	usbtouch->irq->dev = usbtouch->udev;
 	usbtouch->irq->transfer_dma = usbtouch->data_dma;
 	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
@@ -1007,8 +1198,10 @@ static void usbtouch_disconnect(struct u
 	dbg("%s - usbtouch is initialized, cleaning up", __func__);
 	usb_set_intfdata(intf, NULL);
 	usb_kill_urb(usbtouch->irq);
+	usb_kill_urb(usbtouch->ack);
 	input_unregister_device(usbtouch->input);
 	usb_free_urb(usbtouch->irq);
+	usb_free_urb(usbtouch->ack);
 	usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
 	kfree(usbtouch);
 }


-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] [RFC] NEXIO (or iNexio) support for usbtouchscreen
  2009-11-18 12:18   ` [PATCH v2] [RFC] " Ondrej Zary
@ 2009-11-18 15:25     ` Oliver Neukum
  2009-11-19 12:40       ` [PATCH v3] " Ondrej Zary
  0 siblings, 1 reply; 14+ messages in thread
From: Oliver Neukum @ 2009-11-18 15:25 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: linux-usb, daniel.ritz, linux-kernel

Am Mittwoch, 18. November 2009 13:18:43 schrieb Ondrej Zary:

> > > +	/* send init command */
> > > +	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP), init,
> > > +			   sizeof(init), &actual_len, NEXIO_TIMEOUT);
> >
> > DMA on the kernel stack
> 
> I don't know any details about this - what's the correct way to do this?

You must kmalloc the buffer.

> I tried moving the init[] array outside the function but that didn't work
>  at all - compiled fine but device was not reporting the name firmware
>  version. I ended up with copying it to that kmalloced buf.

That's correct.

> 
> I'm worried about nexio_ack - that shouldn't work too then. Maybe it really
> does not and the device does not care.

It works on some architectures. But it is still buggy.

> I also found another problem: when I disconnect the USB cable, kernel
>  oopses: BUG: unable to handle kernel NULL pointer dereference
> IP: [<f7c794fa>] start_unlink_async+0x63/0xfc
> 
> Call Trace:
> ehci_urb_dequeue+0x7c/0x11a [ehci_hcd]
> unlink1+0xaa/0xc7 [usbcore]
> usb_hcd_unlink_urb+0x57/0x84 [usbcore]
> usb_kill_urb+0x40/0xbe [usbcore]
> default_wake_function+0x0/0x2b
> usb_start_wait_urb+0x6e/0xb0 [usbcore]
> usb_control_msg+0x10a/0x136 [usbcore]
> hub_port_status+0x77/0xf7 [usbcore]
> hub_thread+0x56d/0xe14 [usbcore]
> autoremove_wake_function+0x0/0x4f
> hub_thread+0x0/0xe14 [usbcore]
> kthread+0x7a/0x7f
> kthread+0x0/0x7f
> 
> Are there any other problems with my code that can cause this oops?

Odd.

> +	/* read firmware version */
> +	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
> +			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
> +	if (ret < 0)
> +		goto err_out;
> +	buf[buf[1]] = 0;	/* second byte is data(string) length */

You'd better check buf[1] for sanity.

> +	firmware_ver = kstrdup(&buf[2], GFP_KERNEL);

Are you sure this is safe?

> +	/* read device name */
> +	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
> +			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
> +	if (ret < 0) {
> +		kfree(firmware_ver);
> +		goto err_out;
> +	}
> +	buf[buf[1]] = 0;
> +	printk(KERN_INFO "Nexio device: %s, firmware version %s\n",
> +	       &buf[2], firmware_ver);
> +	kfree(firmware_ver);
> +
> +	/* prepare ACK URB */
> +	usbtouch->ack = usb_alloc_urb(0, GFP_KERNEL);
> +	if (!usbtouch->ack) {
> +		dbg("%s - usb_alloc_urb failed: ack_urb", __func__);
> +		return 0;

Are you sure this shouldn't error out?

> +	}
> +	usb_fill_bulk_urb(usbtouch->ack, usbtouch->udev,
> +			  usb_sndbulkpipe(usbtouch->udev, NEXIO_OUTPUT_EP),
> +			  nexio_ack_pkt, sizeof(nexio_ack_pkt), NULL,

DMA on the heap.

> +			  usbtouch);
> +	/* submit first IRQ URB */
> +	ret = usb_submit_urb(usbtouch->irq, GFP_KERNEL);
> +err_out:
> +	kfree(buf);
> +err_nobuf:
> +	return ret;
> +}
> +
> +static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char
>  *pkt) +{
> +	int x, y, begin_x, begin_y, end_x, end_y, w, h, ret;
> +	struct nexio_touch_packet *packet = (void *) pkt;
> +
> +	/* got touch data? */
> +	if ((pkt[0] & 0xe0) != 0xe0)
> +		return 0;
> +
> +	/* send ACK */
> +	ret = usb_submit_urb(usbtouch->ack, GFP_KERNEL);

In interrupt context. You must use GFP_ATOMIC.

> +
> +	if (!usbtouch->type->max_xc) {
> +		usbtouch->type->max_xc = 2 * be16_to_cpu(packet->x_len);
> +		input_set_abs_params(usbtouch->input, ABS_X, 0,
> +				     2 * be16_to_cpu(packet->x_len), 0, 0);
> +		usbtouch->type->max_yc = 2 * be16_to_cpu(packet->y_len);
> +		input_set_abs_params(usbtouch->input, ABS_Y, 0,
> +				     2 * be16_to_cpu(packet->y_len), 0, 0);
> +	}
> +	/* The device reports state of IR sensors on X and Y axes.
> +	 * Each byte represents "darkness" percentage (0-100) of one element.
> +	 * 17" touchscreen reports only 64 x 52 bytes so the resolution is low.
> +	 * This also means that there's a limited multi-touch capability but
> +	 * it's disabled (and untested) here as there's no X driver for that.
> +	 */
> +	begin_x = end_x = begin_y = end_y = -1;
> +	for (x = 0; x < be16_to_cpu(packet->x_len); x++) {
> +		if (begin_x == -1 && packet->data[x] > NEXIO_THRESHOLD) {
> +			begin_x = x;
> +			continue;
> +		}
> +		if (end_x == -1 && begin_x != -1 && packet->data[x] < NEXIO_THRESHOLD) {
> +			end_x = x - 1;
> +			for (y = be16_to_cpu(packet->x_len);
> +			     y < be16_to_cpu(packet->data_len); y++) {
> +				if (begin_y == -1 && packet->data[y] > NEXIO_THRESHOLD) {
> +					begin_y = y - be16_to_cpu(packet->x_len);
> +					continue;
> +				}
> +				if (end_y == -1 &&
> +				    begin_y != -1 && packet->data[y] < NEXIO_THRESHOLD) {
> +					end_y = y - 1 - be16_to_cpu(packet->x_len);
> +					w = end_x - begin_x;
> +					h = end_y - begin_y;
> +					/* multi-touch */
> +/*					input_report_abs(usbtouch->input,
> +						    ABS_MT_TOUCH_MAJOR, max(w,h));
> +					input_report_abs(usbtouch->input,
> +						    ABS_MT_TOUCH_MINOR, min(x,h));
> +					input_report_abs(usbtouch->input,
> +						    ABS_MT_POSITION_X, 2*begin_x+w);
> +					input_report_abs(usbtouch->input,
> +						    ABS_MT_POSITION_Y, 2*begin_y+h);
> +					input_report_abs(usbtouch->input,
> +						    ABS_MT_ORIENTATION, w > h);
> +					input_mt_sync(usbtouch->input);*/
> +					/* single touch */
> +					usbtouch->x = 2 * begin_x + w;
> +					usbtouch->y = 2 * begin_y + h;
> +					usbtouch->touch = packet->flags & 0x01;
> +					begin_y = end_y = -1;
> +					return 1;
> +				}
> +			}
> +			begin_x = end_x = -1;
> +		}
> +
> +	}
> +	return 0;
> +}
> +#endif
> +

> @@ -1007,8 +1198,10 @@ static void usbtouch_disconnect(struct u
>  	dbg("%s - usbtouch is initialized, cleaning up", __func__);
>  	usb_set_intfdata(intf, NULL);
>  	usb_kill_urb(usbtouch->irq);
> +	usb_kill_urb(usbtouch->ack);

Have you checked the order is correct?

>  	input_unregister_device(usbtouch->input);
>  	usb_free_urb(usbtouch->irq);
> +	usb_free_urb(usbtouch->ack);
>  	usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
>  	kfree(usbtouch);
>  }
> 

	Regards
		Oliver


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] [RFC] NEXIO (or iNexio) support for usbtouchscreen
  2009-11-18 15:25     ` Oliver Neukum
@ 2009-11-19 12:40       ` Ondrej Zary
  2009-11-19 16:56         ` Oliver Neukum
  0 siblings, 1 reply; 14+ messages in thread
From: Ondrej Zary @ 2009-11-19 12:40 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: linux-usb, daniel.ritz, linux-kernel

On Wednesday 18 November 2009, Oliver Neukum wrote:
> Am Mittwoch, 18. November 2009 13:18:43 schrieb Ondrej Zary:
> > > > +	/* send init command */
> > > > +	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP),
> > > > init, +			   sizeof(init), &actual_len, NEXIO_TIMEOUT);
> > >
> > > DMA on the kernel stack
> >
> > I don't know any details about this - what's the correct way to do this?
>
> You must kmalloc the buffer.

OK, thanks.

> > I tried moving the init[] array outside the function but that didn't work
> >  at all - compiled fine but device was not reporting the name firmware
> >  version. I ended up with copying it to that kmalloced buf.
>
> That's correct.
>
> > I'm worried about nexio_ack - that shouldn't work too then. Maybe it
> > really does not and the device does not care.
>
> It works on some architectures. But it is still buggy.

See the new patch below.

> > I also found another problem: when I disconnect the USB cable, kernel
> >  oopses: BUG: unable to handle kernel NULL pointer dereference
> > IP: [<f7c794fa>] start_unlink_async+0x63/0xfc
> >
> > Call Trace:
> > ehci_urb_dequeue+0x7c/0x11a [ehci_hcd]
> > unlink1+0xaa/0xc7 [usbcore]
> > usb_hcd_unlink_urb+0x57/0x84 [usbcore]
> > usb_kill_urb+0x40/0xbe [usbcore]
> > default_wake_function+0x0/0x2b
> > usb_start_wait_urb+0x6e/0xb0 [usbcore]
> > usb_control_msg+0x10a/0x136 [usbcore]
> > hub_port_status+0x77/0xf7 [usbcore]
> > hub_thread+0x56d/0xe14 [usbcore]
> > autoremove_wake_function+0x0/0x4f
> > hub_thread+0x0/0xe14 [usbcore]
> > kthread+0x7a/0x7f
> > kthread+0x0/0x7f
> >
> > Are there any other problems with my code that can cause this oops?
>
> Odd.
>
> > +	/* read firmware version */
> > +	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
> > +			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
> > +	if (ret < 0)
> > +		goto err_out;
> > +	buf[buf[1]] = 0;	/* second byte is data(string) length */
>
> You'd better check buf[1] for sanity.
>
> > +	firmware_ver = kstrdup(&buf[2], GFP_KERNEL);
>
> Are you sure this is safe?

This was ugly, rewritten in new patch.

> > +	/* read device name */
> > +	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
> > +			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
> > +	if (ret < 0) {
> > +		kfree(firmware_ver);
> > +		goto err_out;
> > +	}
> > +	buf[buf[1]] = 0;
> > +	printk(KERN_INFO "Nexio device: %s, firmware version %s\n",
> > +	       &buf[2], firmware_ver);
> > +	kfree(firmware_ver);
> > +
> > +	/* prepare ACK URB */
> > +	usbtouch->ack = usb_alloc_urb(0, GFP_KERNEL);
> > +	if (!usbtouch->ack) {
> > +		dbg("%s - usb_alloc_urb failed: ack_urb", __func__);
> > +		return 0;
>
> Are you sure this shouldn't error out?

Oops, that was a copy&paste bug.

> > +	}
> > +	usb_fill_bulk_urb(usbtouch->ack, usbtouch->udev,
> > +			  usb_sndbulkpipe(usbtouch->udev, NEXIO_OUTPUT_EP),
> > +			  nexio_ack_pkt, sizeof(nexio_ack_pkt), NULL,
>
> DMA on the heap.

See the new patch below.

> > +			  usbtouch);
> > +	/* submit first IRQ URB */
> > +	ret = usb_submit_urb(usbtouch->irq, GFP_KERNEL);
> > +err_out:
> > +	kfree(buf);
> > +err_nobuf:
> > +	return ret;
> > +}
> > +
> > +static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char
> >  *pkt) +{
> > +	int x, y, begin_x, begin_y, end_x, end_y, w, h, ret;
> > +	struct nexio_touch_packet *packet = (void *) pkt;
> > +
> > +	/* got touch data? */
> > +	if ((pkt[0] & 0xe0) != 0xe0)
> > +		return 0;
> > +
> > +	/* send ACK */
> > +	ret = usb_submit_urb(usbtouch->ack, GFP_KERNEL);
>
> In interrupt context. You must use GFP_ATOMIC.

See the new patch below.

> > +
> > +	if (!usbtouch->type->max_xc) {
> > +		usbtouch->type->max_xc = 2 * be16_to_cpu(packet->x_len);
> > +		input_set_abs_params(usbtouch->input, ABS_X, 0,
> > +				     2 * be16_to_cpu(packet->x_len), 0, 0);
> > +		usbtouch->type->max_yc = 2 * be16_to_cpu(packet->y_len);
> > +		input_set_abs_params(usbtouch->input, ABS_Y, 0,
> > +				     2 * be16_to_cpu(packet->y_len), 0, 0);
> > +	}
> > +	/* The device reports state of IR sensors on X and Y axes.
> > +	 * Each byte represents "darkness" percentage (0-100) of one element.
> > +	 * 17" touchscreen reports only 64 x 52 bytes so the resolution is low.
> > +	 * This also means that there's a limited multi-touch capability but
> > +	 * it's disabled (and untested) here as there's no X driver for that.
> > +	 */
> > +	begin_x = end_x = begin_y = end_y = -1;
> > +	for (x = 0; x < be16_to_cpu(packet->x_len); x++) {
> > +		if (begin_x == -1 && packet->data[x] > NEXIO_THRESHOLD) {
> > +			begin_x = x;
> > +			continue;
> > +		}
> > +		if (end_x == -1 && begin_x != -1 && packet->data[x] < NEXIO_THRESHOLD)
> > { +			end_x = x - 1;
> > +			for (y = be16_to_cpu(packet->x_len);
> > +			     y < be16_to_cpu(packet->data_len); y++) {
> > +				if (begin_y == -1 && packet->data[y] > NEXIO_THRESHOLD) {
> > +					begin_y = y - be16_to_cpu(packet->x_len);
> > +					continue;
> > +				}
> > +				if (end_y == -1 &&
> > +				    begin_y != -1 && packet->data[y] < NEXIO_THRESHOLD) {
> > +					end_y = y - 1 - be16_to_cpu(packet->x_len);
> > +					w = end_x - begin_x;
> > +					h = end_y - begin_y;
> > +					/* multi-touch */
> > +/*					input_report_abs(usbtouch->input,
> > +						    ABS_MT_TOUCH_MAJOR, max(w,h));
> > +					input_report_abs(usbtouch->input,
> > +						    ABS_MT_TOUCH_MINOR, min(x,h));
> > +					input_report_abs(usbtouch->input,
> > +						    ABS_MT_POSITION_X, 2*begin_x+w);
> > +					input_report_abs(usbtouch->input,
> > +						    ABS_MT_POSITION_Y, 2*begin_y+h);
> > +					input_report_abs(usbtouch->input,
> > +						    ABS_MT_ORIENTATION, w > h);
> > +					input_mt_sync(usbtouch->input);*/
> > +					/* single touch */
> > +					usbtouch->x = 2 * begin_x + w;
> > +					usbtouch->y = 2 * begin_y + h;
> > +					usbtouch->touch = packet->flags & 0x01;
> > +					begin_y = end_y = -1;
> > +					return 1;
> > +				}
> > +			}
> > +			begin_x = end_x = -1;
> > +		}
> > +
> > +	}
> > +	return 0;
> > +}
> > +#endif
> > +
> >
> > @@ -1007,8 +1198,10 @@ static void usbtouch_disconnect(struct u
> >  	dbg("%s - usbtouch is initialized, cleaning up", __func__);
> >  	usb_set_intfdata(intf, NULL);
> >  	usb_kill_urb(usbtouch->irq);
> > +	usb_kill_urb(usbtouch->ack);
>
> Have you checked the order is correct?

Does ordering of usb_kill_urb() matter? Haven't found anything about this in 
documentation.

> >  	input_unregister_device(usbtouch->input);
> >  	usb_free_urb(usbtouch->irq);
> > +	usb_free_urb(usbtouch->ack);
> >  	usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
> >  	kfree(usbtouch);
> >  }
>
> 	Regards
> 		Oliver


 - exit() function and priv pointer instead of adding device-specific parts to
   generic code
 - no more DMA on stack or heap
 - cleaned-up initialization.



--- linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c.orig	2009-09-10 00:13:59.000000000 +0200
+++ linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c	2009-11-19 13:31:52.000000000 +0100
@@ -13,6 +13,7 @@
  *  - IdealTEK URTC1000
  *  - General Touch
  *  - GoTop Super_Q2/GogoPen/PenPower tablets
+ *  - NEXIO/iNexio
  *
  * Copyright (C) 2004-2007 by Daniel Ritz <daniel.ritz@gmx.ch>
  * Copyright (C) by Todd E. Johnson (mtouchusb.c)
@@ -71,6 +72,8 @@ struct usbtouch_device_info {
 	int min_yc, max_yc;
 	int min_press, max_press;
 	int rept_size;
+	bool no_urb_in_open;
+	int endpoint, interval;
 
 	void (*process_pkt) (struct usbtouch_usb *usbtouch, unsigned char *pkt, int len);
 
@@ -84,6 +87,7 @@ struct usbtouch_device_info {
 
 	int  (*read_data)   (struct usbtouch_usb *usbtouch, unsigned char *pkt);
 	int  (*init)        (struct usbtouch_usb *usbtouch);
+	void (*exit)	    (struct usbtouch_usb *usbtouch);
 };
 
 /* a usbtouch device */
@@ -98,6 +102,7 @@ struct usbtouch_usb {
 	struct usbtouch_device_info *type;
 	char name[128];
 	char phys[64];
+	void *priv;
 
 	int x, y;
 	int touch, press;
@@ -118,6 +123,7 @@ enum {
 	DEVTYPE_IDEALTEK,
 	DEVTYPE_GENERAL_TOUCH,
 	DEVTYPE_GOTOP,
+	DEVTYPE_NEXIO,
 };
 
 #define USB_DEVICE_HID_CLASS(vend, prod) \
@@ -191,6 +197,14 @@ static struct usb_device_id usbtouch_dev
 	{USB_DEVICE(0x08f2, 0x00f4), .driver_info = DEVTYPE_GOTOP},
 #endif
 
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	/* data interface only */
+	{USB_DEVICE_AND_INTERFACE_INFO(0x10f0, 0x2002, 0x0a, 0x00, 0x00),
+		.driver_info = DEVTYPE_NEXIO},
+	{USB_DEVICE_AND_INTERFACE_INFO(0x1870, 0x0001, 0x0a, 0x00, 0x00),
+		.driver_info = DEVTYPE_NEXIO},
+#endif
+
 	{}
 };
 
@@ -563,6 +577,190 @@ static int gotop_read_data(struct usbtou
 }
 #endif
 
+/*****************************************************************************
+ * NEXIO Part
+ */
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+
+#define NEXIO_OUTPUT_EP	0x01
+#define NEXIO_INPUT_EP	0x82
+#define NEXIO_TIMEOUT	5000
+#define NEXIO_BUFSIZE	1024
+#define NEXIO_THRESHOLD	50
+
+struct nexio_priv {
+	struct urb *ack;
+	char ack_buf[2];
+};
+
+struct nexio_touch_packet {
+	u8	flags;		/* 0xe1 = touch, 0xe1 = release */
+	__be16	data_len;	/* total bytes of touch data */
+	__be16	x_len;		/* bytes for X axis */
+	__be16	y_len;		/* bytes for Y axis */
+	u8	data[];
+} __attribute__ ((packed));
+
+static unsigned char nexio_ack_pkt[2] = { 0xaa, 0x02 };
+static unsigned char nexio_init_pkt[4] = { 0x82, 0x04, 0x0a, 0x0f };
+
+static int nexio_init(struct usbtouch_usb *usbtouch)
+{
+	struct usb_device *dev = usbtouch->udev;
+	struct nexio_priv *priv;
+	int ret = -ENOMEM;
+	int actual_len, i;
+	unsigned char *buf;
+	char *firmware_ver = NULL, *device_name = NULL;
+
+	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
+	if (!buf)
+		goto err_out;
+	/* two empty reads */
+	for (i = 0; i < 2; i++) {
+		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
+				   buf, NEXIO_BUFSIZE, &actual_len,
+				   NEXIO_TIMEOUT);
+		if (ret < 0)
+			goto err_out;
+	}
+	/* send init command */
+	memcpy(buf, nexio_init_pkt, sizeof(nexio_init_pkt));
+	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP),
+			   buf, sizeof(nexio_init_pkt), &actual_len,
+			   NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* read replies */
+	for (i = 0; i < 3; i++) {
+		memset(buf, 0, NEXIO_BUFSIZE);
+		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
+				   buf, NEXIO_BUFSIZE, &actual_len,
+				   NEXIO_TIMEOUT);
+		if (ret < 0 || actual_len < 1 || buf[1] != actual_len)
+			continue;
+		switch (buf[0]) {
+			case 0x83:	/* firmware version */
+				firmware_ver = kstrdup(&buf[2], GFP_KERNEL);
+				break;
+			case 0x84:	/* device name */
+				device_name = kstrdup(&buf[2], GFP_KERNEL);
+				break;
+		}
+	}
+	printk(KERN_INFO "Nexio device: %s, firmware version: %s\n",
+	       device_name, firmware_ver);
+	kfree(firmware_ver);
+	kfree(device_name);
+
+	/* prepare ACK URB */
+	usbtouch->priv = kmalloc(sizeof(struct nexio_priv), GFP_KERNEL);
+	if (!usbtouch->priv) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
+	priv = usbtouch->priv;
+	memcpy(priv->ack_buf, nexio_ack_pkt, sizeof(nexio_ack_pkt));
+	priv->ack = usb_alloc_urb(0, GFP_KERNEL);
+	if (!priv->ack) {
+		dbg("%s - usb_alloc_urb failed: usbtouch->ack", __func__);
+		ret = -ENOMEM;
+		goto err_out;
+	}
+	usb_fill_bulk_urb(priv->ack, usbtouch->udev,
+			  usb_sndbulkpipe(usbtouch->udev, NEXIO_OUTPUT_EP),
+			  priv->ack_buf, sizeof(priv->ack_buf), NULL,
+			  usbtouch);
+	/* submit first IRQ URB */
+	ret = usb_submit_urb(usbtouch->irq, GFP_KERNEL);
+err_out:
+	kfree(buf);
+	return ret;
+}
+
+static void nexio_exit(struct usbtouch_usb *usbtouch)
+{
+	struct nexio_priv *priv = usbtouch->priv;
+
+	usb_kill_urb(priv->ack);
+	usb_free_urb(priv->ack);
+	kfree(priv);
+}
+
+static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt)
+{
+	int x, y, begin_x, begin_y, end_x, end_y, w, h, ret;
+	struct nexio_touch_packet *packet = (void *) pkt;
+	struct nexio_priv *priv = usbtouch->priv;
+
+	/* got touch data? */
+	if ((pkt[0] & 0xe0) != 0xe0)
+		return 0;
+
+	/* send ACK */
+	ret = usb_submit_urb(priv->ack, GFP_ATOMIC);
+
+	if (!usbtouch->type->max_xc) {
+		usbtouch->type->max_xc = 2 * be16_to_cpu(packet->x_len);
+		input_set_abs_params(usbtouch->input, ABS_X, 0,
+				     2 * be16_to_cpu(packet->x_len), 0, 0);
+		usbtouch->type->max_yc = 2 * be16_to_cpu(packet->y_len);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0,
+				     2 * be16_to_cpu(packet->y_len), 0, 0);
+	}
+	/* The device reports state of IR sensors on X and Y axes.
+	 * Each byte represents "darkness" percentage (0-100) of one element.
+	 * 17" touchscreen reports only 64 x 52 bytes so the resolution is low.
+	 * This also means that there's a limited multi-touch capability but
+	 * it's disabled (and untested) here as there's no X driver for that.
+	 */
+	begin_x = end_x = begin_y = end_y = -1;
+	for (x = 0; x < be16_to_cpu(packet->x_len); x++) {
+		if (begin_x == -1 && packet->data[x] > NEXIO_THRESHOLD) {
+			begin_x = x;
+			continue;
+		}
+		if (end_x == -1 && begin_x != -1 && packet->data[x] < NEXIO_THRESHOLD) {
+			end_x = x - 1;
+			for (y = be16_to_cpu(packet->x_len);
+			     y < be16_to_cpu(packet->data_len); y++) {
+				if (begin_y == -1 && packet->data[y] > NEXIO_THRESHOLD) {
+					begin_y = y - be16_to_cpu(packet->x_len);
+					continue;
+				}
+				if (end_y == -1 &&
+				    begin_y != -1 && packet->data[y] < NEXIO_THRESHOLD) {
+					end_y = y - 1 - be16_to_cpu(packet->x_len);
+					w = end_x - begin_x;
+					h = end_y - begin_y;
+					/* multi-touch */
+/*					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MAJOR, max(w,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MINOR, min(x,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_X, 2*begin_x+w);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_Y, 2*begin_y+h);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_ORIENTATION, w > h);
+					input_mt_sync(usbtouch->input);*/
+					/* single touch */
+					usbtouch->x = 2 * begin_x + w;
+					usbtouch->y = 2 * begin_y + h;
+					usbtouch->touch = packet->flags & 0x01;
+					begin_y = end_y = -1;
+					return 1;
+				}
+			}
+			begin_x = end_x = -1;
+		}
+
+	}
+	return 0;
+}
+#endif
+
 
 /*****************************************************************************
  * the different device descriptors
@@ -702,6 +900,18 @@ static struct usbtouch_device_info usbto
 		.read_data	= gotop_read_data,
 	},
 #endif
+
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	[DEVTYPE_NEXIO] = {
+		.rept_size	= 128,
+		.no_urb_in_open = 1,
+		.endpoint	= NEXIO_INPUT_EP,
+		.interval	= 0xff,
+		.read_data	= nexio_read_data,
+		.init		= nexio_init,
+		.exit		= nexio_exit,
+	},
+#endif
 };
 
 
@@ -852,8 +1062,9 @@ static int usbtouch_open(struct input_de
 
 	usbtouch->irq->dev = usbtouch->udev;
 
-	if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
-		return -EIO;
+	if (!usbtouch->type->no_urb_in_open)
+		if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
+			return -EIO;
 
 	return 0;
 }
@@ -862,7 +1073,8 @@ static void usbtouch_close(struct input_
 {
 	struct usbtouch_usb *usbtouch = input_get_drvdata(input);
 
-	usb_kill_urb(usbtouch->irq);
+	if (!usbtouch->type->no_urb_in_open)
+		usb_kill_urb(usbtouch->irq);
 }
 
 
@@ -960,10 +1172,12 @@ static int usbtouch_probe(struct usb_int
 		                     type->max_press, 0, 0);
 
 	usb_fill_int_urb(usbtouch->irq, usbtouch->udev,
-			 usb_rcvintpipe(usbtouch->udev, endpoint->bEndpointAddress),
+			 usb_rcvintpipe(usbtouch->udev,
+					type->endpoint ? type->endpoint
+							: endpoint->bEndpointAddress),
 			 usbtouch->data, type->rept_size,
-			 usbtouch_irq, usbtouch, endpoint->bInterval);
-
+			 usbtouch_irq, usbtouch,
+			 type->interval ? type->interval : endpoint->bInterval);
 	usbtouch->irq->dev = usbtouch->udev;
 	usbtouch->irq->transfer_dma = usbtouch->data_dma;
 	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
@@ -1006,6 +1220,8 @@ static void usbtouch_disconnect(struct u
 
 	dbg("%s - usbtouch is initialized, cleaning up", __func__);
 	usb_set_intfdata(intf, NULL);
+	if (usbtouch->type->exit)
+		usbtouch->type->exit(usbtouch);
 	usb_kill_urb(usbtouch->irq);
 	input_unregister_device(usbtouch->input);
 	usb_free_urb(usbtouch->irq);



-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] [RFC] NEXIO (or iNexio) support for usbtouchscreen
  2009-11-19 12:40       ` [PATCH v3] " Ondrej Zary
@ 2009-11-19 16:56         ` Oliver Neukum
  2009-11-20  9:21           ` [PATCH v4] " Ondrej Zary
  0 siblings, 1 reply; 14+ messages in thread
From: Oliver Neukum @ 2009-11-19 16:56 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: linux-usb, daniel.ritz, linux-kernel


> +struct nexio_priv {
> +	struct urb *ack;
> +	char ack_buf[2];
> +};

No. Every buffer needs to have an exclusive cache line for DMA
to work on the incoherent archotectures. Therefore you must allocate
each buffer with its own kmalloc.

> +struct nexio_touch_packet {
> +	u8	flags;		/* 0xe1 = touch, 0xe1 = release */
> +	__be16	data_len;	/* total bytes of touch data */
> +	__be16	x_len;		/* bytes for X axis */
> +	__be16	y_len;		/* bytes for Y axis */
> +	u8	data[];
> +} __attribute__ ((packed));
> +
> +static unsigned char nexio_ack_pkt[2] = { 0xaa, 0x02 };
> +static unsigned char nexio_init_pkt[4] = { 0x82, 0x04, 0x0a, 0x0f };
> +
> +static int nexio_init(struct usbtouch_usb *usbtouch)
> +{
> +	struct usb_device *dev = usbtouch->udev;
> +	struct nexio_priv *priv;
> +	int ret = -ENOMEM;
> +	int actual_len, i;
> +	unsigned char *buf;
> +	char *firmware_ver = NULL, *device_name = NULL;
> +
> +	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
> +	if (!buf)
> +		goto err_out;
> +	/* two empty reads */
> +	for (i = 0; i < 2; i++) {
> +		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
> +				   buf, NEXIO_BUFSIZE, &actual_len,
> +				   NEXIO_TIMEOUT);
> +		if (ret < 0)
> +			goto err_out;
> +	}
> +	/* send init command */
> +	memcpy(buf, nexio_init_pkt, sizeof(nexio_init_pkt));
> +	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP),
> +			   buf, sizeof(nexio_init_pkt), &actual_len,
> +			   NEXIO_TIMEOUT);
> +	if (ret < 0)
> +		goto err_out;
> +	/* read replies */
> +	for (i = 0; i < 3; i++) {
> +		memset(buf, 0, NEXIO_BUFSIZE);
> +		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
> +				   buf, NEXIO_BUFSIZE, &actual_len,
> +				   NEXIO_TIMEOUT);
> +		if (ret < 0 || actual_len < 1 || buf[1] != actual_len)
> +			continue;
> +		switch (buf[0]) {
> +			case 0x83:	/* firmware version */
> +				firmware_ver = kstrdup(&buf[2], GFP_KERNEL);
> +				break;
> +			case 0x84:	/* device name */
> +				device_name = kstrdup(&buf[2], GFP_KERNEL);
> +				break;
> +		}
> +	}
> +	printk(KERN_INFO "Nexio device: %s, firmware version: %s\n",
> +	       device_name, firmware_ver);

How do you know device_name and firmware_ver are not NULL?

> +	kfree(firmware_ver);
> +	kfree(device_name);
> +
> +	/* prepare ACK URB */
> +	usbtouch->priv = kmalloc(sizeof(struct nexio_priv), GFP_KERNEL);
> +	if (!usbtouch->priv) {
> +		ret = -ENOMEM;
> +		goto err_out;
> +	}
> +	priv = usbtouch->priv;
> +	memcpy(priv->ack_buf, nexio_ack_pkt, sizeof(nexio_ack_pkt));
> +	priv->ack = usb_alloc_urb(0, GFP_KERNEL);
> +	if (!priv->ack) {

When would usbtouch->priv be freed in the error case?

> +		dbg("%s - usb_alloc_urb failed: usbtouch->ack", __func__);
> +		ret = -ENOMEM;
> +		goto err_out;
> +	}
> +	usb_fill_bulk_urb(priv->ack, usbtouch->udev,
> +			  usb_sndbulkpipe(usbtouch->udev, NEXIO_OUTPUT_EP),
> +			  priv->ack_buf, sizeof(priv->ack_buf), NULL,
> +			  usbtouch);
> +	/* submit first IRQ URB */
> +	ret = usb_submit_urb(usbtouch->irq, GFP_KERNEL);

If this fails, when is priv->ack freed?

> +err_out:
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static void nexio_exit(struct usbtouch_usb *usbtouch)
> +{
> +	struct nexio_priv *priv = usbtouch->priv;
> +
> +	usb_kill_urb(priv->ack);
> +	usb_free_urb(priv->ack);
> +	kfree(priv);
> +}
> +
> 
> 
> @@ -862,7 +1073,8 @@ static void usbtouch_close(struct input_
>  {
>  	struct usbtouch_usb *usbtouch = input_get_drvdata(input);
> 
> -	usb_kill_urb(usbtouch->irq);
> +	if (!usbtouch->type->no_urb_in_open)
> +		usb_kill_urb(usbtouch->irq);
>  }
> 
> 
> @@ -960,10 +1172,12 @@ static int usbtouch_probe(struct usb_int
>  		                     type->max_press, 0, 0);
> 
>  	usb_fill_int_urb(usbtouch->irq, usbtouch->udev,
> -			 usb_rcvintpipe(usbtouch->udev, endpoint->bEndpointAddress),
> +			 usb_rcvintpipe(usbtouch->udev,
> +					type->endpoint ? type->endpoint
> +							: endpoint->bEndpointAddress),
>  			 usbtouch->data, type->rept_size,
> -			 usbtouch_irq, usbtouch, endpoint->bInterval);
> -
> +			 usbtouch_irq, usbtouch,
> +			 type->interval ? type->interval : endpoint->bInterval);
>  	usbtouch->irq->dev = usbtouch->udev;
>  	usbtouch->irq->transfer_dma = usbtouch->data_dma;
>  	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> @@ -1006,6 +1220,8 @@ static void usbtouch_disconnect(struct u
> 
>  	dbg("%s - usbtouch is initialized, cleaning up", __func__);
>  	usb_set_intfdata(intf, NULL);
> +	if (usbtouch->type->exit)
> +		usbtouch->type->exit(usbtouch);
>  	usb_kill_urb(usbtouch->irq);

You've reversed the order. Order is important. If priv->irq
can cause priv->ack to be submitted, it must be killed first.
If priv->ack may submit priv->irq the reverse order is needed.
I don't know which is correct, but only that order may be important.

>  	input_unregister_device(usbtouch->input);

What tells you that open() isn't called at this point reversing
usb_kill_urb() you've already done?

>  	usb_free_urb(usbtouch->irq);
> 

	Regards
		Oliver

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4] [RFC] NEXIO (or iNexio) support for usbtouchscreen
  2009-11-19 16:56         ` Oliver Neukum
@ 2009-11-20  9:21           ` Ondrej Zary
  2009-11-20 18:43             ` Oliver Neukum
  0 siblings, 1 reply; 14+ messages in thread
From: Ondrej Zary @ 2009-11-20  9:21 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: linux-usb, daniel.ritz, linux-kernel

On Thursday 19 November 2009, Oliver Neukum wrote:
> > +struct nexio_priv {
> > +	struct urb *ack;
> > +	char ack_buf[2];
> > +};
>
> No. Every buffer needs to have an exclusive cache line for DMA
> to work on the incoherent archotectures. Therefore you must allocate
> each buffer with its own kmalloc.

OK, thanks for your patience.

> > +struct nexio_touch_packet {
> > +	u8	flags;		/* 0xe1 = touch, 0xe1 = release */
> > +	__be16	data_len;	/* total bytes of touch data */
> > +	__be16	x_len;		/* bytes for X axis */
> > +	__be16	y_len;		/* bytes for Y axis */
> > +	u8	data[];
> > +} __attribute__ ((packed));
> > +
> > +static unsigned char nexio_ack_pkt[2] = { 0xaa, 0x02 };
> > +static unsigned char nexio_init_pkt[4] = { 0x82, 0x04, 0x0a, 0x0f };
> > +
> > +static int nexio_init(struct usbtouch_usb *usbtouch)
> > +{
> > +	struct usb_device *dev = usbtouch->udev;
> > +	struct nexio_priv *priv;
> > +	int ret = -ENOMEM;
> > +	int actual_len, i;
> > +	unsigned char *buf;
> > +	char *firmware_ver = NULL, *device_name = NULL;
> > +
> > +	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
> > +	if (!buf)
> > +		goto err_out;
> > +	/* two empty reads */
> > +	for (i = 0; i < 2; i++) {
> > +		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
> > +				   buf, NEXIO_BUFSIZE, &actual_len,
> > +				   NEXIO_TIMEOUT);
> > +		if (ret < 0)
> > +			goto err_out;
> > +	}
> > +	/* send init command */
> > +	memcpy(buf, nexio_init_pkt, sizeof(nexio_init_pkt));
> > +	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP),
> > +			   buf, sizeof(nexio_init_pkt), &actual_len,
> > +			   NEXIO_TIMEOUT);
> > +	if (ret < 0)
> > +		goto err_out;
> > +	/* read replies */
> > +	for (i = 0; i < 3; i++) {
> > +		memset(buf, 0, NEXIO_BUFSIZE);
> > +		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
> > +				   buf, NEXIO_BUFSIZE, &actual_len,
> > +				   NEXIO_TIMEOUT);
> > +		if (ret < 0 || actual_len < 1 || buf[1] != actual_len)
> > +			continue;
> > +		switch (buf[0]) {
> > +			case 0x83:	/* firmware version */
> > +				firmware_ver = kstrdup(&buf[2], GFP_KERNEL);
> > +				break;
> > +			case 0x84:	/* device name */
> > +				device_name = kstrdup(&buf[2], GFP_KERNEL);
> > +				break;
> > +		}
> > +	}
> > +	printk(KERN_INFO "Nexio device: %s, firmware version: %s\n",
> > +	       device_name, firmware_ver);
>
> How do you know device_name and firmware_ver are not NULL?

printk works fine with NULL, it prints <NULL>. Is it necessary to add more 
code only to make the output nice?

>
> > +	kfree(firmware_ver);
> > +	kfree(device_name);
> > +
> > +	/* prepare ACK URB */
> > +	usbtouch->priv = kmalloc(sizeof(struct nexio_priv), GFP_KERNEL);
> > +	if (!usbtouch->priv) {
> > +		ret = -ENOMEM;
> > +		goto err_out;
> > +	}
> > +	priv = usbtouch->priv;
> > +	memcpy(priv->ack_buf, nexio_ack_pkt, sizeof(nexio_ack_pkt));
> > +	priv->ack = usb_alloc_urb(0, GFP_KERNEL);
> > +	if (!priv->ack) {
>
> When would usbtouch->priv be freed in the error case?

See the new patch below.

> > +		dbg("%s - usb_alloc_urb failed: usbtouch->ack", __func__);
> > +		ret = -ENOMEM;
> > +		goto err_out;
> > +	}
> > +	usb_fill_bulk_urb(priv->ack, usbtouch->udev,
> > +			  usb_sndbulkpipe(usbtouch->udev, NEXIO_OUTPUT_EP),
> > +			  priv->ack_buf, sizeof(priv->ack_buf), NULL,
> > +			  usbtouch);
> > +	/* submit first IRQ URB */
> > +	ret = usb_submit_urb(usbtouch->irq, GFP_KERNEL);
>
> If this fails, when is priv->ack freed?

Looks like I'm constantly missing this. See the new patch below.

> > +err_out:
> > +	kfree(buf);
> > +	return ret;
> > +}
> > +
> > +static void nexio_exit(struct usbtouch_usb *usbtouch)
> > +{
> > +	struct nexio_priv *priv = usbtouch->priv;
> > +
> > +	usb_kill_urb(priv->ack);
> > +	usb_free_urb(priv->ack);
> > +	kfree(priv);
> > +}
> > +
> >
> >
> > @@ -862,7 +1073,8 @@ static void usbtouch_close(struct input_
> >  {
> >  	struct usbtouch_usb *usbtouch = input_get_drvdata(input);
> >
> > -	usb_kill_urb(usbtouch->irq);
> > +	if (!usbtouch->type->no_urb_in_open)
> > +		usb_kill_urb(usbtouch->irq);
> >  }
> >
> >
> > @@ -960,10 +1172,12 @@ static int usbtouch_probe(struct usb_int
> >  		                     type->max_press, 0, 0);
> >
> >  	usb_fill_int_urb(usbtouch->irq, usbtouch->udev,
> > -			 usb_rcvintpipe(usbtouch->udev, endpoint->bEndpointAddress),
> > +			 usb_rcvintpipe(usbtouch->udev,
> > +					type->endpoint ? type->endpoint
> > +							: endpoint->bEndpointAddress),
> >  			 usbtouch->data, type->rept_size,
> > -			 usbtouch_irq, usbtouch, endpoint->bInterval);
> > -
> > +			 usbtouch_irq, usbtouch,
> > +			 type->interval ? type->interval : endpoint->bInterval);
> >  	usbtouch->irq->dev = usbtouch->udev;
> >  	usbtouch->irq->transfer_dma = usbtouch->data_dma;
> >  	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> > @@ -1006,6 +1220,8 @@ static void usbtouch_disconnect(struct u
> >
> >  	dbg("%s - usbtouch is initialized, cleaning up", __func__);
> >  	usb_set_intfdata(intf, NULL);
> > +	if (usbtouch->type->exit)
> > +		usbtouch->type->exit(usbtouch);
> >  	usb_kill_urb(usbtouch->irq);
>
> You've reversed the order. Order is important. If priv->irq
> can cause priv->ack to be submitted, it must be killed first.
> If priv->ack may submit priv->irq the reverse order is needed.
> I don't know which is correct, but only that order may be important.

Thanks, I see now. irq may submit ack, ack never submits anything. So the 
correct order is: "first kill irq, then ack".

> >  	input_unregister_device(usbtouch->input);
>
> What tells you that open() isn't called at this point reversing
> usb_kill_urb() you've already done?

Looks like a bug in the original usbtouchscreen code. There's no locking.
Will a spinlock in usbtouch_open() and usbtouch_disconnect() fix it? Or 
do you see more problems here?



--- linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c.orig	2009-09-10 00:13:59.000000000 +0200
+++ linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c	2009-11-20 09:32:39.000000000 +0100
@@ -13,6 +13,7 @@
  *  - IdealTEK URTC1000
  *  - General Touch
  *  - GoTop Super_Q2/GogoPen/PenPower tablets
+ *  - NEXIO/iNexio
  *
  * Copyright (C) 2004-2007 by Daniel Ritz <daniel.ritz@gmx.ch>
  * Copyright (C) by Todd E. Johnson (mtouchusb.c)
@@ -71,6 +72,8 @@ struct usbtouch_device_info {
 	int min_yc, max_yc;
 	int min_press, max_press;
 	int rept_size;
+	bool no_urb_in_open;
+	int endpoint, interval;
 
 	void (*process_pkt) (struct usbtouch_usb *usbtouch, unsigned char *pkt, int len);
 
@@ -84,6 +87,7 @@ struct usbtouch_device_info {
 
 	int  (*read_data)   (struct usbtouch_usb *usbtouch, unsigned char *pkt);
 	int  (*init)        (struct usbtouch_usb *usbtouch);
+	void (*exit)	    (struct usbtouch_usb *usbtouch);
 };
 
 /* a usbtouch device */
@@ -98,6 +102,7 @@ struct usbtouch_usb {
 	struct usbtouch_device_info *type;
 	char name[128];
 	char phys[64];
+	void *priv;
 
 	int x, y;
 	int touch, press;
@@ -118,6 +123,7 @@ enum {
 	DEVTYPE_IDEALTEK,
 	DEVTYPE_GENERAL_TOUCH,
 	DEVTYPE_GOTOP,
+	DEVTYPE_NEXIO,
 };
 
 #define USB_DEVICE_HID_CLASS(vend, prod) \
@@ -191,6 +197,14 @@ static struct usb_device_id usbtouch_dev
 	{USB_DEVICE(0x08f2, 0x00f4), .driver_info = DEVTYPE_GOTOP},
 #endif
 
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	/* data interface only */
+	{USB_DEVICE_AND_INTERFACE_INFO(0x10f0, 0x2002, 0x0a, 0x00, 0x00),
+		.driver_info = DEVTYPE_NEXIO},
+	{USB_DEVICE_AND_INTERFACE_INFO(0x1870, 0x0001, 0x0a, 0x00, 0x00),
+		.driver_info = DEVTYPE_NEXIO},
+#endif
+
 	{}
 };
 
@@ -563,6 +577,200 @@ static int gotop_read_data(struct usbtou
 }
 #endif
 
+/*****************************************************************************
+ * NEXIO Part
+ */
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+
+#define NEXIO_OUTPUT_EP	0x01
+#define NEXIO_INPUT_EP	0x82
+#define NEXIO_TIMEOUT	5000
+#define NEXIO_BUFSIZE	1024
+#define NEXIO_THRESHOLD	50
+
+struct nexio_priv {
+	struct urb *ack;
+	unsigned char *ack_buf;
+};
+
+struct nexio_touch_packet {
+	u8	flags;		/* 0xe1 = touch, 0xe1 = release */
+	__be16	data_len;	/* total bytes of touch data */
+	__be16	x_len;		/* bytes for X axis */
+	__be16	y_len;		/* bytes for Y axis */
+	u8	data[];
+} __attribute__ ((packed));
+
+static unsigned char nexio_ack_pkt[2] = { 0xaa, 0x02 };
+static unsigned char nexio_init_pkt[4] = { 0x82, 0x04, 0x0a, 0x0f };
+
+static int nexio_init(struct usbtouch_usb *usbtouch)
+{
+	struct usb_device *dev = usbtouch->udev;
+	struct nexio_priv *priv;
+	int ret = -ENOMEM;
+	int actual_len, i;
+	unsigned char *buf;
+	char *firmware_ver = NULL, *device_name = NULL;
+
+	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
+	if (!buf)
+		goto out_buf;
+	/* two empty reads */
+	for (i = 0; i < 2; i++) {
+		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
+				   buf, NEXIO_BUFSIZE, &actual_len,
+				   NEXIO_TIMEOUT);
+		if (ret < 0)
+			goto out_buf;
+	}
+	/* send init command */
+	memcpy(buf, nexio_init_pkt, sizeof(nexio_init_pkt));
+	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP),
+			   buf, sizeof(nexio_init_pkt), &actual_len,
+			   NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto out_buf;
+	/* read replies */
+	for (i = 0; i < 3; i++) {
+		memset(buf, 0, NEXIO_BUFSIZE);
+		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
+				   buf, NEXIO_BUFSIZE, &actual_len,
+				   NEXIO_TIMEOUT);
+		if (ret < 0 || actual_len < 1 || buf[1] != actual_len)
+			continue;
+		switch (buf[0]) {
+			case 0x83:	/* firmware version */
+				firmware_ver = kstrdup(&buf[2], GFP_KERNEL);
+				break;
+			case 0x84:	/* device name */
+				device_name = kstrdup(&buf[2], GFP_KERNEL);
+				break;
+		}
+	}
+	printk(KERN_INFO "Nexio device: %s, firmware version: %s\n",
+	       device_name, firmware_ver);
+	kfree(firmware_ver);
+	kfree(device_name);
+
+	/* prepare ACK URB */
+	ret = -ENOMEM;
+	usbtouch->priv = kmalloc(sizeof(struct nexio_priv), GFP_KERNEL);
+	if (!usbtouch->priv)
+		goto out_buf;
+	priv = usbtouch->priv;
+	priv->ack_buf = kmalloc(sizeof(nexio_ack_pkt), GFP_KERNEL);
+	if (!priv->ack_buf)
+		goto err_priv;
+	memcpy(priv->ack_buf, nexio_ack_pkt, sizeof(nexio_ack_pkt));
+	priv->ack = usb_alloc_urb(0, GFP_KERNEL);
+	if (!priv->ack) {
+		dbg("%s - usb_alloc_urb failed: usbtouch->ack", __func__);
+		goto err_ack_buf;
+	}
+	usb_fill_bulk_urb(priv->ack, usbtouch->udev,
+			  usb_sndbulkpipe(usbtouch->udev, NEXIO_OUTPUT_EP),
+			  priv->ack_buf, sizeof(priv->ack_buf), NULL,
+			  usbtouch);
+	/* submit first IRQ URB */
+	ret = usb_submit_urb(usbtouch->irq, GFP_KERNEL);
+	if (ret < 0) {
+		usb_free_urb(priv->ack);
+		goto err_ack_buf;
+	}
+	goto out_buf;
+err_ack_buf:
+	kfree(priv->ack_buf);
+err_priv:
+	kfree(priv);
+out_buf:
+	kfree(buf);
+	return ret;
+}
+
+static void nexio_exit(struct usbtouch_usb *usbtouch)
+{
+	struct nexio_priv *priv = usbtouch->priv;
+
+	usb_kill_urb(priv->ack);
+	usb_free_urb(priv->ack);
+	kfree(priv);
+}
+
+static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt)
+{
+	int x, y, begin_x, begin_y, end_x, end_y, w, h, ret;
+	struct nexio_touch_packet *packet = (void *) pkt;
+	struct nexio_priv *priv = usbtouch->priv;
+
+	/* got touch data? */
+	if ((pkt[0] & 0xe0) != 0xe0)
+		return 0;
+
+	/* send ACK */
+	ret = usb_submit_urb(priv->ack, GFP_ATOMIC);
+
+	if (!usbtouch->type->max_xc) {
+		usbtouch->type->max_xc = 2 * be16_to_cpu(packet->x_len);
+		input_set_abs_params(usbtouch->input, ABS_X, 0,
+				     2 * be16_to_cpu(packet->x_len), 0, 0);
+		usbtouch->type->max_yc = 2 * be16_to_cpu(packet->y_len);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0,
+				     2 * be16_to_cpu(packet->y_len), 0, 0);
+	}
+	/* The device reports state of IR sensors on X and Y axes.
+	 * Each byte represents "darkness" percentage (0-100) of one element.
+	 * 17" touchscreen reports only 64 x 52 bytes so the resolution is low.
+	 * This also means that there's a limited multi-touch capability but
+	 * it's disabled (and untested) here as there's no X driver for that.
+	 */
+	begin_x = end_x = begin_y = end_y = -1;
+	for (x = 0; x < be16_to_cpu(packet->x_len); x++) {
+		if (begin_x == -1 && packet->data[x] > NEXIO_THRESHOLD) {
+			begin_x = x;
+			continue;
+		}
+		if (end_x == -1 && begin_x != -1 && packet->data[x] < NEXIO_THRESHOLD) {
+			end_x = x - 1;
+			for (y = be16_to_cpu(packet->x_len);
+			     y < be16_to_cpu(packet->data_len); y++) {
+				if (begin_y == -1 && packet->data[y] > NEXIO_THRESHOLD) {
+					begin_y = y - be16_to_cpu(packet->x_len);
+					continue;
+				}
+				if (end_y == -1 &&
+				    begin_y != -1 && packet->data[y] < NEXIO_THRESHOLD) {
+					end_y = y - 1 - be16_to_cpu(packet->x_len);
+					w = end_x - begin_x;
+					h = end_y - begin_y;
+					/* multi-touch */
+/*					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MAJOR, max(w,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MINOR, min(x,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_X, 2*begin_x+w);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_Y, 2*begin_y+h);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_ORIENTATION, w > h);
+					input_mt_sync(usbtouch->input);*/
+					/* single touch */
+					usbtouch->x = 2 * begin_x + w;
+					usbtouch->y = 2 * begin_y + h;
+					usbtouch->touch = packet->flags & 0x01;
+					begin_y = end_y = -1;
+					return 1;
+				}
+			}
+			begin_x = end_x = -1;
+		}
+
+	}
+	return 0;
+}
+#endif
+
 
 /*****************************************************************************
  * the different device descriptors
@@ -702,6 +910,18 @@ static struct usbtouch_device_info usbto
 		.read_data	= gotop_read_data,
 	},
 #endif
+
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	[DEVTYPE_NEXIO] = {
+		.rept_size	= 128,
+		.no_urb_in_open = 1,
+		.endpoint	= NEXIO_INPUT_EP,
+		.interval	= 0xff,
+		.read_data	= nexio_read_data,
+		.init		= nexio_init,
+		.exit		= nexio_exit,
+	},
+#endif
 };
 
 
@@ -852,8 +1072,9 @@ static int usbtouch_open(struct input_de
 
 	usbtouch->irq->dev = usbtouch->udev;
 
-	if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
-		return -EIO;
+	if (!usbtouch->type->no_urb_in_open)
+		if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
+			return -EIO;
 
 	return 0;
 }
@@ -862,7 +1083,8 @@ static void usbtouch_close(struct input_
 {
 	struct usbtouch_usb *usbtouch = input_get_drvdata(input);
 
-	usb_kill_urb(usbtouch->irq);
+	if (!usbtouch->type->no_urb_in_open)
+		usb_kill_urb(usbtouch->irq);
 }
 
 
@@ -960,10 +1182,12 @@ static int usbtouch_probe(struct usb_int
 		                     type->max_press, 0, 0);
 
 	usb_fill_int_urb(usbtouch->irq, usbtouch->udev,
-			 usb_rcvintpipe(usbtouch->udev, endpoint->bEndpointAddress),
+			 usb_rcvintpipe(usbtouch->udev,
+					type->endpoint ? type->endpoint
+							: endpoint->bEndpointAddress),
 			 usbtouch->data, type->rept_size,
-			 usbtouch_irq, usbtouch, endpoint->bInterval);
-
+			 usbtouch_irq, usbtouch,
+			 type->interval ? type->interval : endpoint->bInterval);
 	usbtouch->irq->dev = usbtouch->udev;
 	usbtouch->irq->transfer_dma = usbtouch->data_dma;
 	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
@@ -1009,6 +1233,8 @@ static void usbtouch_disconnect(struct u
 	usb_kill_urb(usbtouch->irq);
 	input_unregister_device(usbtouch->input);
 	usb_free_urb(usbtouch->irq);
+	if (usbtouch->type->exit)
+		usbtouch->type->exit(usbtouch);
 	usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
 	kfree(usbtouch);
 }



-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] [RFC] NEXIO (or iNexio) support for usbtouchscreen
  2009-11-20  9:21           ` [PATCH v4] " Ondrej Zary
@ 2009-11-20 18:43             ` Oliver Neukum
  2009-11-20 22:41               ` Ondrej Zary
  0 siblings, 1 reply; 14+ messages in thread
From: Oliver Neukum @ 2009-11-20 18:43 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: linux-usb, daniel.ritz, linux-kernel

Am Freitag, 20. November 2009 10:21:43 schrieb Ondrej Zary:
> On Thursday 19 November 2009, Oliver Neukum wrote:
> > > +struct nexio_priv {
> > > +	struct urb *ack;
> > > +	char ack_buf[2];
> > > +};
> >
> > No. Every buffer needs to have an exclusive cache line for DMA
> > to work on the incoherent archotectures. Therefore you must allocate
> > each buffer with its own kmalloc.
> 
> OK, thanks for your patience.

No problem. I should have explained better.

> > > +	/* read replies */
> > > +	for (i = 0; i < 3; i++) {
> > > +		memset(buf, 0, NEXIO_BUFSIZE);
> > > +		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
> > > +				   buf, NEXIO_BUFSIZE, &actual_len,
> > > +				   NEXIO_TIMEOUT);
> > > +		if (ret < 0 || actual_len < 1 || buf[1] != actual_len)
> > > +			continue;
> > > +		switch (buf[0]) {
> > > +			case 0x83:	/* firmware version */
> > > +				firmware_ver = kstrdup(&buf[2], GFP_KERNEL);

On second thought this is not nice. If a device is broken enough to report
a name or a firmware version twice, you produce a memory leak.
Do you know very buggy devices to exist?

> > > +				break;
> > > +			case 0x84:	/* device name */
> > > +				device_name = kstrdup(&buf[2], GFP_KERNEL);
> > > +				break;
> > > +		}
> > > +	}
> > > +	printk(KERN_INFO "Nexio device: %s, firmware version: %s\n",
> > > +	       device_name, firmware_ver);
> >
> > How do you know device_name and firmware_ver are not NULL?
> 
> printk works fine with NULL, it prints <NULL>. Is it necessary to add more
> code only to make the output nice?

No, for niceness it is not necessary. The question is whether you want
to treat this as an error or print a warning. That is a matter of taste.
 

 
> Looks like a bug in the original usbtouchscreen code. There's no locking.
> Will a spinlock in usbtouch_open() and usbtouch_disconnect() fix it? Or
> do you see more problems here?

You must not call usb_kill_urb() with a spinlock held.
I'll lokk at the usbtouchscreen code.
The new version looks good to me.

	Regards
		Oliver

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] [RFC] NEXIO (or iNexio) support for usbtouchscreen
  2009-11-20 18:43             ` Oliver Neukum
@ 2009-11-20 22:41               ` Ondrej Zary
  2009-11-21  9:17                 ` Oliver Neukum
  0 siblings, 1 reply; 14+ messages in thread
From: Ondrej Zary @ 2009-11-20 22:41 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: linux-usb, daniel.ritz, linux-kernel

On Friday 20 November 2009 19:43:46 Oliver Neukum wrote:
> Am Freitag, 20. November 2009 10:21:43 schrieb Ondrej Zary:
> > On Thursday 19 November 2009, Oliver Neukum wrote:
> > > > +struct nexio_priv {
> > > > +	struct urb *ack;
> > > > +	char ack_buf[2];
> > > > +};
> > >
> > > No. Every buffer needs to have an exclusive cache line for DMA
> > > to work on the incoherent archotectures. Therefore you must allocate
> > > each buffer with its own kmalloc.
> >
> > OK, thanks for your patience.
>
> No problem. I should have explained better.
>
> > > > +	/* read replies */
> > > > +	for (i = 0; i < 3; i++) {
> > > > +		memset(buf, 0, NEXIO_BUFSIZE);
> > > > +		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP),
> > > > +				   buf, NEXIO_BUFSIZE, &actual_len,
> > > > +				   NEXIO_TIMEOUT);
> > > > +		if (ret < 0 || actual_len < 1 || buf[1] != actual_len)
> > > > +			continue;
> > > > +		switch (buf[0]) {
> > > > +			case 0x83:	/* firmware version */
> > > > +				firmware_ver = kstrdup(&buf[2], GFP_KERNEL);
>
> On second thought this is not nice. If a device is broken enough to report
> a name or a firmware version twice, you produce a memory leak.
> Do you know very buggy devices to exist?

Oh yes, that might be a problem - I'll add a NULL check before kstrdup. And 
maybe it should not be hardcoded to 3 reads - had to check this.

> > > > +				break;
> > > > +			case 0x84:	/* device name */
> > > > +				device_name = kstrdup(&buf[2], GFP_KERNEL);
> > > > +				break;
> > > > +		}
> > > > +	}
> > > > +	printk(KERN_INFO "Nexio device: %s, firmware version: %s\n",
> > > > +	       device_name, firmware_ver);
> > >
> > > How do you know device_name and firmware_ver are not NULL?
> >
> > printk works fine with NULL, it prints <NULL>. Is it necessary to add
> > more code only to make the output nice?
>
> No, for niceness it is not necessary. The question is whether you want
> to treat this as an error or print a warning. That is a matter of taste.

As there's no datasheet and I have only one device (with one firmware 
version), ignoring it looks like the best "solution".

I'll also want to remove NEXIO_INPUT_EP and NEXIO_OUTPUT_EP constants - the 
endpoint addresses can be found at runtine (there's only one input and one 
output endpoint). I think that to do this in nexio_init(), it needs to 
know "struct usb_interface *" instead of "struct usb_device *". I have a 
patch ready (but forgot to take it so it needs to wait until next week) that 
replaces "struct usb_device *udev" in struct usbtouch_usb with "struct 
usb_interface *interface" - is it a good idea?

>
> > Looks like a bug in the original usbtouchscreen code. There's no locking.
> > Will a spinlock in usbtouch_open() and usbtouch_disconnect() fix it? Or
> > do you see more problems here?
>
> You must not call usb_kill_urb() with a spinlock held.
> I'll lokk at the usbtouchscreen code.
> The new version looks good to me.
>
> 	Regards
> 		Oliver
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] [RFC] NEXIO (or iNexio) support for usbtouchscreen
  2009-11-20 22:41               ` Ondrej Zary
@ 2009-11-21  9:17                 ` Oliver Neukum
  2009-11-23 10:04                   ` [PATCH 1/3] usbtouchscreen: convert from usb_device to usb_interface Ondrej Zary
                                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Oliver Neukum @ 2009-11-21  9:17 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: linux-usb, daniel.ritz, linux-kernel

Am Freitag, 20. November 2009 23:41:29 schrieb Ondrej Zary:
> I'll also want to remove NEXIO_INPUT_EP and NEXIO_OUTPUT_EP constants -
>  the  endpoint addresses can be found at runtine (there's only one input
>  and one output endpoint). I think that to do this in nexio_init(), it
>  needs to know "struct usb_interface *" instead of "struct usb_device *". I
>  have a patch ready (but forgot to take it so it needs to wait until next
>  week) that replaces "struct usb_device *udev" in struct usbtouch_usb with
>  "struct usb_interface *interface" - is it a good idea?

Yes, it is a good idea. Everything else also looks good.

	Regards
		Oliver

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] usbtouchscreen: convert from usb_device to usb_interface
  2009-11-21  9:17                 ` Oliver Neukum
@ 2009-11-23 10:04                   ` Ondrej Zary
  2009-11-23 10:04                   ` [PATCH 2/3] usbtouchscreen: find input endpoint automatically Ondrej Zary
  2009-11-23 10:05                   ` [PATCH 3/3] usbtouchscreen: add NEXIO (or iNexio) support Ondrej Zary
  2 siblings, 0 replies; 14+ messages in thread
From: Ondrej Zary @ 2009-11-23 10:04 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: linux-usb, daniel.ritz, linux-kernel

Convert usbtouchscreen from storing usb_device to usb_interface. This is
needed for multi-interface touchscreen devices such as iNexio.


Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c.orig	2009-11-23 09:56:27.000000000 +0100
+++ linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c	2009-11-23 09:58:20.000000000 +0100
@@ -93,7 +93,7 @@ struct usbtouch_usb {
 	unsigned char *buffer;
 	int buf_len;
 	struct urb *irq;
-	struct usb_device *udev;
+	struct usb_interface *interface;
 	struct input_dev *input;
 	struct usbtouch_device_info *type;
 	char name[128];
@@ -280,8 +280,9 @@ static int mtouch_read_data(struct usbto
 static int mtouch_init(struct usbtouch_usb *usbtouch)
 {
 	int ret, i;
+	struct usb_device *udev = interface_to_usbdev(usbtouch->interface);
 
-	ret = usb_control_msg(usbtouch->udev, usb_rcvctrlpipe(usbtouch->udev, 0),
+	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
 	                      MTOUCHUSB_RESET,
 	                      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
 	                      1, 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
@@ -292,7 +293,7 @@ static int mtouch_init(struct usbtouch_u
 	msleep(150);
 
 	for (i = 0; i < 3; i++) {
-		ret = usb_control_msg(usbtouch->udev, usb_rcvctrlpipe(usbtouch->udev, 0),
+		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
 				      MTOUCHUSB_ASYNC_REPORT,
 				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
 				      1, 1, NULL, 0, USB_CTRL_SET_TIMEOUT);
@@ -425,7 +426,7 @@ static int gunze_read_data(struct usbtou
 
 static int dmc_tsc10_init(struct usbtouch_usb *usbtouch)
 {
-	struct usb_device *dev = usbtouch->udev;
+	struct usb_device *dev = interface_to_usbdev(usbtouch->interface);
 	int ret = -ENOMEM;
 	unsigned char *buf;
 
@@ -850,7 +851,7 @@ static int usbtouch_open(struct input_de
 {
 	struct usbtouch_usb *usbtouch = input_get_drvdata(input);
 
-	usbtouch->irq->dev = usbtouch->udev;
+	usbtouch->irq->dev = interface_to_usbdev(usbtouch->interface);
 
 	if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
 		return -EIO;
@@ -920,7 +921,7 @@ static int usbtouch_probe(struct usb_int
 		goto out_free_buffers;
 	}
 
-	usbtouch->udev = udev;
+	usbtouch->interface = intf;
 	usbtouch->input = input_dev;
 
 	if (udev->manufacturer)
@@ -959,12 +960,12 @@ static int usbtouch_probe(struct usb_int
 		input_set_abs_params(input_dev, ABS_PRESSURE, type->min_press,
 		                     type->max_press, 0, 0);
 
-	usb_fill_int_urb(usbtouch->irq, usbtouch->udev,
-			 usb_rcvintpipe(usbtouch->udev, endpoint->bEndpointAddress),
+	usb_fill_int_urb(usbtouch->irq, udev,
+			 usb_rcvintpipe(udev, endpoint->bEndpointAddress),
 			 usbtouch->data, type->rept_size,
 			 usbtouch_irq, usbtouch, endpoint->bInterval);
 
-	usbtouch->irq->dev = usbtouch->udev;
+	usbtouch->irq->dev = udev;
 	usbtouch->irq->transfer_dma = usbtouch->data_dma;
 	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 

-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] usbtouchscreen: find input endpoint automatically
  2009-11-21  9:17                 ` Oliver Neukum
  2009-11-23 10:04                   ` [PATCH 1/3] usbtouchscreen: convert from usb_device to usb_interface Ondrej Zary
@ 2009-11-23 10:04                   ` Ondrej Zary
  2009-11-23 10:05                   ` [PATCH 3/3] usbtouchscreen: add NEXIO (or iNexio) support Ondrej Zary
  2 siblings, 0 replies; 14+ messages in thread
From: Ondrej Zary @ 2009-11-23 10:04 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: linux-usb, daniel.ritz, linux-kernel

Find input enpoint automatically instead of assuming that the first one is OK.
This is needed for devices with multiple endpoints such as iNexio where the
first endpoint might be output.


Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c.1	2009-11-23 10:30:14.000000000 +0100
+++ linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c	2009-11-23 10:57:14.000000000 +0100
@@ -882,17 +882,25 @@ static int usbtouch_probe(struct usb_int
 	struct usbtouch_usb *usbtouch;
 	struct input_dev *input_dev;
 	struct usb_host_interface *interface;
-	struct usb_endpoint_descriptor *endpoint;
+	struct usb_endpoint_descriptor *endpoint = NULL;
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct usbtouch_device_info *type;
 	int err = -ENOMEM;
+	int i;
 
 	/* some devices are ignored */
 	if (id->driver_info == DEVTYPE_IGNORE)
 		return -ENODEV;
 
 	interface = intf->cur_altsetting;
-	endpoint = &interface->endpoint[0].desc;
+	/* find first input endpoint */
+	for (i = 0; i < interface->desc.bNumEndpoints; i++)
+		if (usb_endpoint_dir_in(&interface->endpoint[i].desc)) {
+			endpoint = &interface->endpoint[i].desc;
+			break;
+		}
+	if (!endpoint)
+		return -ENXIO;
 
 	usbtouch = kzalloc(sizeof(struct usbtouch_usb), GFP_KERNEL);
 	input_dev = input_allocate_device();


-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/3] usbtouchscreen: add NEXIO (or iNexio) support
  2009-11-21  9:17                 ` Oliver Neukum
  2009-11-23 10:04                   ` [PATCH 1/3] usbtouchscreen: convert from usb_device to usb_interface Ondrej Zary
  2009-11-23 10:04                   ` [PATCH 2/3] usbtouchscreen: find input endpoint automatically Ondrej Zary
@ 2009-11-23 10:05                   ` Ondrej Zary
  2 siblings, 0 replies; 14+ messages in thread
From: Ondrej Zary @ 2009-11-23 10:05 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: linux-usb, daniel.ritz, linux-kernel

Add support for NEXIO (or iNexio) USB touchscreens to usbtouchscreen driver.
Tested with NEX170MRT 17" LCD monitor with integrated touchscreen
(with xserver-xorg-input-evtouch 0.8.8-1):

T:  Bus=02 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 54 Spd=12  MxCh= 0
D:  Ver= 1.10 Cls=02(comm.) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
P:  Vendor=1870 ProdID=0001 Rev= 1.00
S:  Manufacturer=iNexio
S:  Product=iNexio USB
C:* #Ifs= 2 Cfg#= 1 Atr=c0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=02 Prot=00 Driver=(none)
E:  Ad=83(I) Atr=03(Int.) MxPS=   8 Ivl=255ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=(none)
E:  Ad=01(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms

No datasheet is available, this was written by capturing some data with
SniffUSB in Windows:
http://www.rainbow-software.org/linux_files/nexio/


Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c.2	2009-11-23 10:40:08.000000000 +0100
+++ linux-source-2.6.31/drivers/input/touchscreen/usbtouchscreen.c	2009-11-23 10:52:38.000000000 +0100
@@ -13,6 +13,7 @@
  *  - IdealTEK URTC1000
  *  - General Touch
  *  - GoTop Super_Q2/GogoPen/PenPower tablets
+ *  - NEXIO/iNexio
  *
  * Copyright (C) 2004-2007 by Daniel Ritz <daniel.ritz@gmx.ch>
  * Copyright (C) by Todd E. Johnson (mtouchusb.c)
@@ -71,6 +72,7 @@ struct usbtouch_device_info {
 	int min_yc, max_yc;
 	int min_press, max_press;
 	int rept_size;
+	bool no_urb_in_open;
 
 	void (*process_pkt) (struct usbtouch_usb *usbtouch, unsigned char *pkt, int len);
 
@@ -84,6 +86,7 @@ struct usbtouch_device_info {
 
 	int  (*read_data)   (struct usbtouch_usb *usbtouch, unsigned char *pkt);
 	int  (*init)        (struct usbtouch_usb *usbtouch);
+	void (*exit)	    (struct usbtouch_usb *usbtouch);
 };
 
 /* a usbtouch device */
@@ -98,6 +101,7 @@ struct usbtouch_usb {
 	struct usbtouch_device_info *type;
 	char name[128];
 	char phys[64];
+	void *priv;
 
 	int x, y;
 	int touch, press;
@@ -118,6 +122,7 @@ enum {
 	DEVTYPE_IDEALTEK,
 	DEVTYPE_GENERAL_TOUCH,
 	DEVTYPE_GOTOP,
+	DEVTYPE_NEXIO,
 };
 
 #define USB_DEVICE_HID_CLASS(vend, prod) \
@@ -191,6 +196,14 @@ static struct usb_device_id usbtouch_dev
 	{USB_DEVICE(0x08f2, 0x00f4), .driver_info = DEVTYPE_GOTOP},
 #endif
 
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	/* data interface only */
+	{USB_DEVICE_AND_INTERFACE_INFO(0x10f0, 0x2002, 0x0a, 0x00, 0x00),
+		.driver_info = DEVTYPE_NEXIO},
+	{USB_DEVICE_AND_INTERFACE_INFO(0x1870, 0x0001, 0x0a, 0x00, 0x00),
+		.driver_info = DEVTYPE_NEXIO},
+#endif
+
 	{}
 };
 
@@ -564,6 +577,215 @@ static int gotop_read_data(struct usbtou
 }
 #endif
 
+/*****************************************************************************
+ * NEXIO Part
+ */
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+
+#define NEXIO_TIMEOUT	5000
+#define NEXIO_BUFSIZE	1024
+#define NEXIO_THRESHOLD	50
+
+struct nexio_priv {
+	struct urb *ack;
+	unsigned char *ack_buf;
+};
+
+struct nexio_touch_packet {
+	u8	flags;		/* 0xe1 = touch, 0xe1 = release */
+	__be16	data_len;	/* total bytes of touch data */
+	__be16	x_len;		/* bytes for X axis */
+	__be16	y_len;		/* bytes for Y axis */
+	u8	data[];
+} __attribute__ ((packed));
+
+static unsigned char nexio_ack_pkt[2] = { 0xaa, 0x02 };
+static unsigned char nexio_init_pkt[4] = { 0x82, 0x04, 0x0a, 0x0f };
+
+static int nexio_init(struct usbtouch_usb *usbtouch)
+{
+	struct usb_device *dev = interface_to_usbdev(usbtouch->interface);
+	struct usb_host_interface *interface = usbtouch->interface->cur_altsetting;
+	struct nexio_priv *priv;
+	int ret = -ENOMEM;
+	int actual_len, i;
+	unsigned char *buf;
+	char *firmware_ver = NULL, *device_name = NULL;
+	int input_ep = 0, output_ep = 0;
+
+	/* find first input and output endpoint */
+	for (i = 0; i < interface->desc.bNumEndpoints; i++) {
+		if (!input_ep &&
+		    usb_endpoint_dir_in(&interface->endpoint[i].desc))
+			input_ep = interface->endpoint[i].desc.bEndpointAddress;
+		if (!output_ep &&
+		    usb_endpoint_dir_out(&interface->endpoint[i].desc))
+			output_ep = interface->endpoint[i].desc.bEndpointAddress;
+	}
+	if (!input_ep || !output_ep)
+		return -ENXIO;
+
+	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
+	if (!buf)
+		goto out_buf;
+	/* two empty reads */
+	for (i = 0; i < 2; i++) {
+		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, input_ep),
+				   buf, NEXIO_BUFSIZE, &actual_len,
+				   NEXIO_TIMEOUT);
+		if (ret < 0)
+			goto out_buf;
+	}
+	/* send init command */
+	memcpy(buf, nexio_init_pkt, sizeof(nexio_init_pkt));
+	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, output_ep),
+			   buf, sizeof(nexio_init_pkt), &actual_len,
+			   NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto out_buf;
+	/* read replies */
+	for (i = 0; i < 3; i++) {
+		memset(buf, 0, NEXIO_BUFSIZE);
+		ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, input_ep),
+				   buf, NEXIO_BUFSIZE, &actual_len,
+				   NEXIO_TIMEOUT);
+		if (ret < 0 || actual_len < 1 || buf[1] != actual_len)
+			continue;
+		switch (buf[0]) {
+			case 0x83:	/* firmware version */
+				if (!firmware_ver)
+					firmware_ver = kstrdup(&buf[2],
+							       GFP_KERNEL);
+				break;
+			case 0x84:	/* device name */
+				if (!device_name)
+					device_name = kstrdup(&buf[2],
+							      GFP_KERNEL);
+				break;
+		}
+	}
+	printk(KERN_INFO "Nexio device: %s, firmware version: %s\n",
+	       device_name, firmware_ver);
+	kfree(firmware_ver);
+	kfree(device_name);
+
+	/* prepare ACK URB */
+	ret = -ENOMEM;
+	usbtouch->priv = kmalloc(sizeof(struct nexio_priv), GFP_KERNEL);
+	if (!usbtouch->priv)
+		goto out_buf;
+	priv = usbtouch->priv;
+	priv->ack_buf = kmalloc(sizeof(nexio_ack_pkt), GFP_KERNEL);
+	if (!priv->ack_buf)
+		goto err_priv;
+	memcpy(priv->ack_buf, nexio_ack_pkt, sizeof(nexio_ack_pkt));
+	priv->ack = usb_alloc_urb(0, GFP_KERNEL);
+	if (!priv->ack) {
+		dbg("%s - usb_alloc_urb failed: usbtouch->ack", __func__);
+		goto err_ack_buf;
+	}
+	usb_fill_bulk_urb(priv->ack, dev, usb_sndbulkpipe(dev, output_ep),
+			  priv->ack_buf, sizeof(priv->ack_buf), NULL,
+			  usbtouch);
+	/* submit first IRQ URB */
+	ret = usb_submit_urb(usbtouch->irq, GFP_KERNEL);
+	if (ret < 0) {
+		usb_free_urb(priv->ack);
+		goto err_ack_buf;
+	}
+	goto out_buf;
+err_ack_buf:
+	kfree(priv->ack_buf);
+err_priv:
+	kfree(priv);
+out_buf:
+	kfree(buf);
+	return ret;
+}
+
+static void nexio_exit(struct usbtouch_usb *usbtouch)
+{
+	struct nexio_priv *priv = usbtouch->priv;
+
+	usb_kill_urb(priv->ack);
+	usb_free_urb(priv->ack);
+	kfree(priv);
+}
+
+static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt)
+{
+	int x, y, begin_x, begin_y, end_x, end_y, w, h, ret;
+	struct nexio_touch_packet *packet = (void *) pkt;
+	struct nexio_priv *priv = usbtouch->priv;
+
+	/* got touch data? */
+	if ((pkt[0] & 0xe0) != 0xe0)
+		return 0;
+
+	/* send ACK */
+	ret = usb_submit_urb(priv->ack, GFP_ATOMIC);
+
+	if (!usbtouch->type->max_xc) {
+		usbtouch->type->max_xc = 2 * be16_to_cpu(packet->x_len);
+		input_set_abs_params(usbtouch->input, ABS_X, 0,
+				     2 * be16_to_cpu(packet->x_len), 0, 0);
+		usbtouch->type->max_yc = 2 * be16_to_cpu(packet->y_len);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0,
+				     2 * be16_to_cpu(packet->y_len), 0, 0);
+	}
+	/* The device reports state of IR sensors on X and Y axes.
+	 * Each byte represents "darkness" percentage (0-100) of one element.
+	 * 17" touchscreen reports only 64 x 52 bytes so the resolution is low.
+	 * This also means that there's a limited multi-touch capability but
+	 * it's disabled (and untested) here as there's no X driver for that.
+	 */
+	begin_x = end_x = begin_y = end_y = -1;
+	for (x = 0; x < be16_to_cpu(packet->x_len); x++) {
+		if (begin_x == -1 && packet->data[x] > NEXIO_THRESHOLD) {
+			begin_x = x;
+			continue;
+		}
+		if (end_x == -1 && begin_x != -1 && packet->data[x] < NEXIO_THRESHOLD) {
+			end_x = x - 1;
+			for (y = be16_to_cpu(packet->x_len);
+			     y < be16_to_cpu(packet->data_len); y++) {
+				if (begin_y == -1 && packet->data[y] > NEXIO_THRESHOLD) {
+					begin_y = y - be16_to_cpu(packet->x_len);
+					continue;
+				}
+				if (end_y == -1 &&
+				    begin_y != -1 && packet->data[y] < NEXIO_THRESHOLD) {
+					end_y = y - 1 - be16_to_cpu(packet->x_len);
+					w = end_x - begin_x;
+					h = end_y - begin_y;
+					/* multi-touch */
+/*					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MAJOR, max(w,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_TOUCH_MINOR, min(x,h));
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_X, 2*begin_x+w);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_POSITION_Y, 2*begin_y+h);
+					input_report_abs(usbtouch->input,
+						    ABS_MT_ORIENTATION, w > h);
+					input_mt_sync(usbtouch->input);*/
+					/* single touch */
+					usbtouch->x = 2 * begin_x + w;
+					usbtouch->y = 2 * begin_y + h;
+					usbtouch->touch = packet->flags & 0x01;
+					begin_y = end_y = -1;
+					return 1;
+				}
+			}
+			begin_x = end_x = -1;
+		}
+
+	}
+	return 0;
+}
+#endif
+
 
 /*****************************************************************************
  * the different device descriptors
@@ -703,6 +925,16 @@ static struct usbtouch_device_info usbto
 		.read_data	= gotop_read_data,
 	},
 #endif
+
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	[DEVTYPE_NEXIO] = {
+		.rept_size	= 128,
+		.no_urb_in_open = 1,
+		.read_data	= nexio_read_data,
+		.init		= nexio_init,
+		.exit		= nexio_exit,
+	},
+#endif
 };
 
 
@@ -853,8 +1085,9 @@ static int usbtouch_open(struct input_de
 
 	usbtouch->irq->dev = interface_to_usbdev(usbtouch->interface);
 
-	if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
-		return -EIO;
+	if (!usbtouch->type->no_urb_in_open)
+		if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
+			return -EIO;
 
 	return 0;
 }
@@ -863,7 +1096,8 @@ static void usbtouch_close(struct input_
 {
 	struct usbtouch_usb *usbtouch = input_get_drvdata(input);
 
-	usb_kill_urb(usbtouch->irq);
+	if (!usbtouch->type->no_urb_in_open)
+		usb_kill_urb(usbtouch->irq);
 }
 
 
@@ -1018,6 +1252,8 @@ static void usbtouch_disconnect(struct u
 	usb_kill_urb(usbtouch->irq);
 	input_unregister_device(usbtouch->input);
 	usb_free_urb(usbtouch->irq);
+	if (usbtouch->type->exit)
+		usbtouch->type->exit(usbtouch);
 	usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
 	kfree(usbtouch);
 }


-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] NEXIO (or iNexio) support for usbtouchscreen
@ 2009-11-09 14:37 Ondrej Zary
  0 siblings, 0 replies; 14+ messages in thread
From: Ondrej Zary @ 2009-11-09 14:37 UTC (permalink / raw)
  To: daniel.ritz; +Cc: linux-kernel

Hello,
I have a NEXIO (or iNexio) NEX170MRT LCD monitor with integrated touchscreen 
with USB interface:
T:  Bus=02 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 54 Spd=12  MxCh= 0
D:  Ver= 1.10 Cls=02(comm.) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
P:  Vendor=1870 ProdID=0001 Rev= 1.00
S:  Manufacturer=iNexio
S:  Product=iNexio USB
C:* #Ifs= 2 Cfg#= 1 Atr=c0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=02 Prot=00 Driver=usbtouchscreen
E:  Ad=83(I) Atr=03(Int.) MxPS=   8 Ivl=255ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=(none)
E:  Ad=01(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms


There's a iNexio serial touchscreen driver in kernel but it does not work for 
this touchscreen. Without a driver, it just connects and disconnects (after a 
while) repeatedly:
usb 2-2: new full speed USB device using uhci_hcd and address 3
usb 2-2: New USB device found, idVendor=1870, idProduct=0001
usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 2-2: Product: iNexio USB
usb 2-2: Manufacturer: iNexio
usb 2-2: configuration #1 chosen from 1 choice
usb 2-2: USB disconnect, address 3
usb 2-2: new full speed USB device using uhci_hcd and address 4
usb 2-2: New USB device found, idVendor=1870, idProduct=0001
usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 2-2: Product: iNexio USB
usb 2-2: Manufacturer: iNexio
usb 2-2: configuration #1 chosen from 1 choice
usb 2-2: USB disconnect, address 4


I captured (using SniffUSB) the communication between the Windows driver and 
the touchscreen after plugging in:
http://www.rainbow-software.org/linux_files/nexio/UsbSnoop-plug.log

Log with some touches after plugging in:
http://www.rainbow-software.org/linux_files/nexio/UsbSnoop-plugandtouch.log

If I understand it correctly, there's system initialization (first 3 URBs), 
then the driver starts communication:
URB 4: read, touchscreen returns 82 04 ab aa
URB 5: read, touchscreen returns nothing
URB 6: write 82 04 0a 0f
URB 7: read, touchscreen returns 84 04 0a 0f
URB 8: read, touchscreen returns 83 0a 32 2e 31 30 53 4d 53 4e (2.10SMSN - 
       firmware version as Windows driver displays)
URB 9: read, touchscreen returns 84 09 6d 2d 4e 65 78 69 6f (m-Nexio - device
       name as Windows driver displays)
URB 10: read, touchscreen returns nothing

Then 3 reads touchscreen returns nothing and returns "82 04 ab aa" on the 4th 
read.

Touchscreen sends 123 bytes of touch data:
    00000000: e1 00 74 00 40 00 34 00 00 00 11 10 0d 00 11 00
    00000010: 0a 00 01 07 03 04 02 03 00 05 00 0e 09 00 02 08
    00000020: 00 01 00 01 01 06 00 0e 00 0b 14 01 0d 06 00 00
    00000030: 02 02 00 02 11 11 06 01 00 02 01 0a 62 64 00 07
    00000040: 05 03 00 03 00 03 00 00 03 02 03 02 00 02 0a 00
    00000050: 02 05 00 06 64 64 64 01 02 04 03 0d 02 02 01 02
    00000060: 07 02 02 00 00 07 03 04 03 01 04 03 03 03 00 03
    00000070: 06 04 04 01 02 03 00 02 02 01 11
and driver acknowledges with "aa 02".

All reads are coming from endpoint 0x82 and all writes go to endpoint 0x01.

The driver continuously submits read URBs to endpoint 0x82. Touchscreen 
answers after 1 second if there's no activity or immediately with touch data.


I did this (but I don't know much about USB programming). The initialization
seems to work (outputs firmware version and device name correctly). But
the device disconnects probably because of absent communication. The first
URB should be probably sent in nexio_init().
Also the usbtouchscreen tries to bind to both device interfaces and it fails 
the second time. How to solve this?

--- drivers/input/touchscreen/usbtouchscreen.c.orig	2009-09-10 
00:13:59.000000000 +0200
+++ drivers/input/touchscreen/usbtouchscreen.c	2009-11-09 14:39:48.000000000 
+0100
@@ -13,6 +13,7 @@
  *  - IdealTEK URTC1000
  *  - General Touch
  *  - GoTop Super_Q2/GogoPen/PenPower tablets
+ *  - NEXIO/iNexio
  *
  * Copyright (C) 2004-2007 by Daniel Ritz <daniel.ritz@gmx.ch>
  * Copyright (C) by Todd E. Johnson (mtouchusb.c)
@@ -118,6 +119,7 @@
 	DEVTYPE_IDEALTEK,
 	DEVTYPE_GENERAL_TOUCH,
 	DEVTYPE_GOTOP,
+	DEVTYPE_NEXIO,
 };
 
 #define USB_DEVICE_HID_CLASS(vend, prod) \
@@ -191,6 +193,11 @@
 	{USB_DEVICE(0x08f2, 0x00f4), .driver_info = DEVTYPE_GOTOP},
 #endif
 
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	{USB_DEVICE(0x10f0, 0x2002), .driver_info = DEVTYPE_NEXIO},
+	{USB_DEVICE(0x1870, 0x0001), .driver_info = DEVTYPE_NEXIO},
+#endif
+
 	{}
 };
 
@@ -563,6 +570,87 @@
 }
 #endif
 
+/*****************************************************************************
+ * NEXIO Part
+ */
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+
+#define NEXIO_OUTPUT_EP	0x01
+#define NEXIO_INPUT_EP	0x82
+#define NEXIO_TIMEOUT	5000
+#define NEXIO_BUFSIZE	1024
+
+static int nexio_init(struct usbtouch_usb *usbtouch)
+{
+	struct usb_device *dev = usbtouch->udev;
+	int ret = -ENOMEM;
+	int actual_len;
+	unsigned char *buf;
+
+	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
+	if (!buf)
+		goto err_nobuf;
+	printk("1 read\n");
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	printk("2 read\n");
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			   NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* Send some command */
+	printk("3 write \n");
+	buf[0] = 0x82;
+	buf[1] = 0x04;
+	buf[2] = 0x0a;
+	buf[3] = 0x0f;
+	ret = usb_bulk_msg(dev, usb_sndbulkpipe(dev, NEXIO_OUTPUT_EP), buf, 4,
+			   &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* Read it back */
+	printk("4 read\n");
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, NEXIO_INPUT_EP), buf,
+			  NEXIO_BUFSIZE, &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	/* Read firmware version */
+	printk("5 read\n");
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, 0x82), buf, NEXIO_BUFSIZE,
+			   &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	buf[buf[1]] = 0;
+	printk("Firmware version %s\n", &buf[2]);
+	/* Read device name */
+	printk("6 read\n");
+	ret = usb_bulk_msg(dev, usb_rcvbulkpipe(dev, 0x82), buf, NEXIO_BUFSIZE,
+			   &actual_len, NEXIO_TIMEOUT);
+	if (ret < 0)
+		goto err_out;
+	buf[buf[1]] = 0;
+	printk("Device name %s\n", &buf[2]);
+	if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
+		return -EIO;
+err_out:
+	kfree(buf);
+err_nobuf:
+	return ret;
+}
+
+static int nexio_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+{
+	int i;
+	printk("nexio_read_data: ");
+	for (i = 0; i < pkt[1]; i++)
+		printk("%02X", pkt[i]);
+
+	return 0;
+}
+#endif
+
 
 /*****************************************************************************
  * the different device descriptors
@@ -702,6 +790,18 @@
 		.read_data	= gotop_read_data,
 	},
 #endif
+
+#ifdef CONFIG_TOUCHSCREEN_USB_NEXIO
+	[DEVTYPE_NEXIO] = {
+		.min_xc		= 0x0,
+		.max_xc		= 0x03ff,
+		.min_yc		= 0x0,
+		.max_yc		= 0x03ff,
+		.rept_size	= 4,
+		.read_data	= nexio_read_data,
+		.init		= nexio_init,
+	},
+#endif
 };
 


-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2009-11-23 10:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-16 14:14 [PATCH] NEXIO (or iNexio) support for usbtouchscreen Ondrej Zary
2009-11-17 15:25 ` Oliver Neukum
2009-11-18 12:18   ` [PATCH v2] [RFC] " Ondrej Zary
2009-11-18 15:25     ` Oliver Neukum
2009-11-19 12:40       ` [PATCH v3] " Ondrej Zary
2009-11-19 16:56         ` Oliver Neukum
2009-11-20  9:21           ` [PATCH v4] " Ondrej Zary
2009-11-20 18:43             ` Oliver Neukum
2009-11-20 22:41               ` Ondrej Zary
2009-11-21  9:17                 ` Oliver Neukum
2009-11-23 10:04                   ` [PATCH 1/3] usbtouchscreen: convert from usb_device to usb_interface Ondrej Zary
2009-11-23 10:04                   ` [PATCH 2/3] usbtouchscreen: find input endpoint automatically Ondrej Zary
2009-11-23 10:05                   ` [PATCH 3/3] usbtouchscreen: add NEXIO (or iNexio) support Ondrej Zary
  -- strict thread matches above, loose matches on Subject: below --
2009-11-09 14:37 [PATCH] NEXIO (or iNexio) support for usbtouchscreen Ondrej Zary

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.