2005-11-23 16:52:51

by Hans-Christian Egtvedt

[permalink] [raw]
Subject: [PATCH 2.6.14.2] Updated itmtouch kernel usb input driver (1/1)

I have made a new release of the itmtouch driver, version 1.3.1. Basicly
just some parameters accesable for users to ease usability and
compatibility with X.

Attached is the patch against Linux 2.6.14.2.

For full source please download:
http://www.mivu.no/itmtouch/itmtouch-1.3.1.tar.bz2
http://www.mivu.no/itmtouch/itmtouch-1.3.1.tar.gz

I have testet the driver with my LG L1510SF touch screen and it works.
Any feedback is most welcome.

--
Best regards
Hans-Christian Egtvedt


Attachments:
itmtouch-1.3.1-linux-2.6.14.2.patch (7.76 kB)

2005-11-23 16:58:16

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [PATCH 2.6.14.2] Updated itmtouch kernel usb input driver (1/1)

On Wed, Nov 23, 2005 at 05:52:44PM +0100, Hans-Christian Egtvedt wrote:
> I have made a new release of the itmtouch driver, version 1.3.1. Basicly
> just some parameters accesable for users to ease usability and
> compatibility with X.
>
> Attached is the patch against Linux 2.6.14.2.
>
> For full source please download:
> http://www.mivu.no/itmtouch/itmtouch-1.3.1.tar.bz2
> http://www.mivu.no/itmtouch/itmtouch-1.3.1.tar.gz
>
> I have testet the driver with my LG L1510SF touch screen and it works.
> Any feedback is most welcome.

Disabling input_sync() is wrong. Very wrong. If you're trying to do it
instead of debouncing, do the debouncing. While I believe processing of
events doesn't belong into the driver, fixing hardware deficiencies does
to a certain degree.

> --
> Best regards
> Hans-Christian Egtvedt

> --- /usr/src/linux-2.6.14.2/drivers/usb/input/itmtouch.c 2005-11-23 16:02:55.000000000 +0100
> +++ itmtouch.c 2005-11-23 17:16:09.000000000 +0100
> @@ -22,15 +22,27 @@
> * driver. CC -- 2003/9/29
> *
> * History
> - * 1.0 & 1.1 2003 (CC) [email protected]
> - * Original version for 2.4.x kernels
> + * 1.0 & 1.1 2003 (CC) [email protected]
> + * - Original version for 2.4.x kernels
> *
> - * 1.2 02/03/2005 (HCE) [email protected]
> - * Complete rewrite to support Linux 2.6.10, thanks to mtouchusb.c for hints.
> - * Unfortunately no calibration support at this time.
> + * 1.2 02/03/2005 (HCE) [email protected]
> + * - Complete rewrite to support Linux 2.6.10, thanks to mtouchusb.c for hints.
> + * - Unfortunately no calibration support at this time.
> *
> * 1.2.1 09/03/2005 (HCE) [email protected]
> - * Code cleanup and adjusting syntax to start matching kernel standards
> + * - Code cleanup and adjusting syntax to start matching kernel standards
> + *
> + * 1.2.2 10/03/2005 (HCE) [email protected]
> + * - Code cleanup
> + *
> + * 1.3 17/03/2005 (HCE) [email protected]
> + * - Added parameter for swapping X- and Y-axis (swapxy).
> + * - General code cleanup
> + *
> + * 1.3.1 23/11/2005 (HCE) [email protected]
> + * - Added parameter nosync for disabling input_sync. Panel is unusable
> + * without this, but people should be able to chose.
> + * - Added swapx and swapy to make it easier to adopt to X drivers
> *
> *****************************************************************************/
>
> @@ -52,13 +64,15 @@
>
> /* only an 8 byte buffer necessary for a single packet */
> #define ITM_BUFSIZE 8
> +/* support a maximum of 4 such touchscreens at once */
> +#define MAXTOUCH 4
> #define PATH_SIZE 64
>
> #define USB_VENDOR_ID_ITMINC 0x0403
> #define USB_PRODUCT_ID_TOUCHPANEL 0xf9e9
>
> #define DRIVER_AUTHOR "Hans-Christian Egtvedt <[email protected]>"
> -#define DRIVER_VERSION "v1.2.1"
> +#define DRIVER_VERSION "v1.3.1"
> #define DRIVER_DESC "USB ITM Inc Touch Panel Driver"
> #define DRIVER_LICENSE "GPL"
>
> @@ -66,14 +80,25 @@
> MODULE_DESCRIPTION( DRIVER_DESC );
> MODULE_LICENSE( DRIVER_LICENSE );
>
> +static int swapxy, swapx, swapy, nosync;
> +
> +module_param(swapxy, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> +MODULE_PARM_DESC(swapxy, "If set the X- and Y-axis are swapped.");
> +module_param(swapx, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> +MODULE_PARM_DESC(swapx, "If set the X-axis is reversed in direction.");
> +module_param(swapy, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> +MODULE_PARM_DESC(swapy, "If set the Y-axis is reversed in direction.");
> +module_param(nosync, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> +MODULE_PARM_DESC(nosync, "If set input_sync is disabled.");
> +
> struct itmtouch_dev {
> struct usb_device *usbdev; /* usb device */
> struct input_dev inputdev; /* input device */
> struct urb *readurb; /* urb */
> char rbuf[ITM_BUFSIZE]; /* data */
> int users;
> - char name[128];
> - char phys[64];
> + char name[128];
> + char phys[64];
> };
>
> static struct usb_device_id itmtouch_ids [] = {
> @@ -83,17 +108,18 @@
>
> static void itmtouch_irq(struct urb *urb, struct pt_regs *regs)
> {
> - struct itmtouch_dev * itmtouch = urb->context;
> - unsigned char *data = urb->transfer_buffer;
> + struct itmtouch_dev *itmtouch = urb->context;
> struct input_dev *dev = &itmtouch->inputdev;
> + unsigned int x, y, abs, button;
> int retval;
> + u8 *data;
>
> switch (urb->status) {
> case 0:
> /* success */
> break;
> case -ETIMEDOUT:
> - /* this urb is timing out */
> + /* this urb is timing out, device unplugged? */
> dbg("%s - urb timed out - was the device unplugged?",
> __FUNCTION__);
> return;
> @@ -110,33 +136,65 @@
> goto exit;
> }
>
> + data = (u8 *)(urb->transfer_buffer);
> +
> + if (swapx)
> + x = (data[1] & 0x1F) << 7 | (data[4] & 0x7F);
> + else
> + x = 4096 - ((data[1] & 0x1F) << 7 | (data[4] & 0x7F));
> +
> + if (swapy)
> + y = 4096 - ((data[0] & 0x1F) << 7 | (data[3] & 0x7F));
> + else
> + y = (data[0] & 0x1F) << 7 | (data[3] & 0x7F);
> +
> + abs = (data[2] & 0x1) << 7 | (data[5] & 0x7F);
> +
> + /* Value is 0x80 when pressed and 0xA0 when released */
> + button = !(data[7] & 0x20);
> +
> input_regs(dev, regs);
>
> - /* if pressure has been released, then don't report X/Y */
> - if (data[7] & 0x20) {
> - input_report_abs(dev, ABS_X, (data[0] & 0x1F) << 7 | (data[3] & 0x7F));
> - input_report_abs(dev, ABS_Y, (data[1] & 0x1F) << 7 | (data[4] & 0x7F));
> + if (button) {
> + if (swapxy) {
> + input_report_abs(dev, ABS_X, y);
> + input_report_abs(dev, ABS_Y, x);
> + }
> + else {
> + input_report_abs(dev, ABS_X, x);
> + input_report_abs(dev, ABS_Y, y);
> + }
> }
>
> - input_report_abs(dev, ABS_PRESSURE, (data[2] & 1) << 7 | (data[5] & 0x7F));
> - input_report_key(dev, BTN_TOUCH, ~data[7] & 0x20);
> - input_sync(dev);
> + input_report_abs(dev, ABS_PRESSURE, abs);
> + input_report_key(dev, BTN_TOUCH, button);
> +
> + /* Only use input_sync() if specified by user, this breaks the behavior
> + * of the panel. If you are experiencing double clicks, turn off sync. */
> + if (!nosync)
> + input_sync(dev);
>
> exit:
> - retval = usb_submit_urb (urb, GFP_ATOMIC);
> + retval = usb_submit_urb(urb, GFP_ATOMIC);
> if (retval)
> printk(KERN_ERR "%s - usb_submit_urb failed with result: %d",
> - __FUNCTION__, retval);
> + __FUNCTION__, retval);
> }
>
> static int itmtouch_open(struct input_dev *input)
> {
> struct itmtouch_dev *itmtouch = input->private;
>
> + if (itmtouch->users++)
> + return 0;
> +
> itmtouch->readurb->dev = itmtouch->usbdev;
>
> if (usb_submit_urb(itmtouch->readurb, GFP_KERNEL))
> + {
> + itmtouch->users--;
> return -EIO;
> + }
>
> return 0;
> }
> @@ -145,7 +203,8 @@
> {
> struct itmtouch_dev *itmtouch = input->private;
>
> - usb_kill_urb(itmtouch->readurb);
> + if (!--itmtouch->users)
> + usb_kill_urb(itmtouch->readurb);
> }
>
> static int itmtouch_probe(struct usb_interface *intf, const struct usb_device_id *id)
> @@ -158,13 +217,18 @@
> unsigned int maxp;
> char path[PATH_SIZE];
>
> + /* Setting interface */
> interface = intf->cur_altsetting;
> +
> + /* Setting endpoint */
> endpoint = &interface->endpoint[0].desc;
>
> - if (!(itmtouch = kzalloc(sizeof(struct itmtouch_dev), GFP_KERNEL))) {
> + /* allocate memory space */
> + if (!(itmtouch = kmalloc(sizeof(struct itmtouch_dev), GFP_KERNEL))) {
> err("%s - Out of memory.", __FUNCTION__);
> return -ENOMEM;
> }
> + memset(itmtouch, 0, sizeof(struct itmtouch_dev));
>
> itmtouch->usbdev = udev;
>
> @@ -180,7 +244,10 @@
>
> itmtouch->inputdev.name = itmtouch->name;
> itmtouch->inputdev.phys = itmtouch->phys;
> - usb_to_input_id(udev, &itmtouch->inputdev.id);
> + itmtouch->inputdev.id.bustype = BUS_USB;
> + itmtouch->inputdev.id.vendor = udev->descriptor.idVendor;
> + itmtouch->inputdev.id.product = udev->descriptor.idProduct;
> + itmtouch->inputdev.id.version = udev->descriptor.bcdDevice;
> itmtouch->inputdev.dev = &intf->dev;
>
> if (!strlen(itmtouch->name))
> @@ -199,8 +266,10 @@
> pipe = usb_rcvintpipe(itmtouch->usbdev, endpoint->bEndpointAddress);
> maxp = usb_maxpacket(udev, pipe, usb_pipeout(pipe));
>
> - if (maxp > ITM_BUFSIZE)
> + if (maxp > ITM_BUFSIZE) {
> + printk(KERN_WARNING "itmtouch: WARNING: packet size > ITM_BUFSIZE\n");
> maxp = ITM_BUFSIZE;
> + }
>
> itmtouch->readurb = usb_alloc_urb(0, GFP_KERNEL);
>
> @@ -210,12 +279,18 @@
> return -ENOMEM;
> }
>
> - usb_fill_int_urb(itmtouch->readurb, itmtouch->usbdev, pipe, itmtouch->rbuf,
> - maxp, itmtouch_irq, itmtouch, endpoint->bInterval);
> + usb_fill_int_urb(itmtouch->readurb,
> + itmtouch->usbdev,
> + pipe,
> + itmtouch->rbuf,
> + maxp,
> + itmtouch_irq,
> + itmtouch,
> + endpoint->bInterval);
>
> input_register_device(&itmtouch->inputdev);
>
> - printk(KERN_INFO "itmtouch: %s registered on %s\n", itmtouch->name, path);
> + printk(KERN_INFO "itmtouch: %s on %s\n", itmtouch->name, path);
> usb_set_intfdata(intf, itmtouch);
>
> return 0;


--
Vojtech Pavlik
SuSE Labs, SuSE CR

2005-11-24 02:01:39

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 2.6.14.2] Updated itmtouch kernel usb input driver (1/1)

On Wednesday 23 November 2005 11:58, Vojtech Pavlik wrote:
> > ?static int itmtouch_open(struct input_dev *input)
> > ?{
> > ??????struct itmtouch_dev *itmtouch = input->private;
> > ?
> > +?????if (itmtouch->users++)
> > +?????????????return 0;
> > +

Why are you adding this? input_open/close are serialized and called
only once when needed.

> > ??????itmtouch->readurb->dev = itmtouch->usbdev;
> > ?
> > ??????if (usb_submit_urb(itmtouch->readurb, GFP_KERNEL))
> > +?????{
> > +?????????????itmtouch->users--;
> > ??????????????return -EIO;
> > +?????}
> > ?

Brace should go on the same line with "if".

> > -?????usb_to_input_id(udev, &itmtouch->inputdev.id);
> > +?????itmtouch->inputdev.id.bustype = BUS_USB;
> > +?????itmtouch->inputdev.id.vendor = udev->descriptor.idVendor;
> > +?????itmtouch->inputdev.id.product = udev->descriptor.idProduct;
> > +?????itmtouch->inputdev.id.version = udev->descriptor.bcdDevice;
> > ??????itmtouch->inputdev.dev = &intf->dev;

Why are you replacing perfectly good code with incorrect one (endianess
issues)?

Plus you need to convert it to dynamic input_dev allocation for newer
kernels.

--
Dmitry

2005-11-30 19:31:04

by Hans-Christian Egtvedt

[permalink] [raw]
Subject: Re: [PATCH 2.6.14.2] Updated itmtouch kernel usb input driver (1/1)

On Wed, 2005-11-23 at 21:01 -0500, Dmitry Torokhov wrote:
> On Wednesday 23 November 2005 11:58, Vojtech Pavlik wrote:
> > > static int itmtouch_open(struct input_dev *input)
> > > {
> > > struct itmtouch_dev *itmtouch = input->private;
> > >
> > > + if (itmtouch->users++)
> > > + return 0;
> > > +
>
> Why are you adding this? input_open/close are serialized and called
> only once when needed.

I see I shiped my patch a bit hastly, this is dropped as discussed the
very first time I submitted this driver (Mars I think). I had totally
forgotten about this serializing.

> > > itmtouch->readurb->dev = itmtouch->usbdev;
> > >
> > > if (usb_submit_urb(itmtouch->readurb, GFP_KERNEL))
> > > + {
> > > + itmtouch->users--;
> > > return -EIO;
> > > + }
> > >
>
> Brace should go on the same line with "if".

Typo, corrected.

> > > - usb_to_input_id(udev, &itmtouch->inputdev.id);
> > > + itmtouch->inputdev.id.bustype = BUS_USB;
> > > + itmtouch->inputdev.id.vendor = udev->descriptor.idVendor;
> > > + itmtouch->inputdev.id.product = udev->descriptor.idProduct;
> > > + itmtouch->inputdev.id.version = udev->descriptor.bcdDevice;
> > > itmtouch->inputdev.dev = &intf->dev;
>
> Why are you replacing perfectly good code with incorrect one (endianess
> issues)?

This was already present but not syncronized with my local repository,
corrected.

> Plus you need to convert it to dynamic input_dev allocation for newer
> kernels.

This is new for me, I will look into it.

I have also removed the nosync parameter since it was not welcome. I
can't reproduce the double click fault with my LG L1510SF screen with
the 2.6.14 kernel. So my guess is it is either a bug in some screens
(I've changed mine just a couple of weeks ago) or an old bug in the
kernel which has been solved.

Thanks for the feedback.

I've attached a patch to match all the comments.

--
Hans-Christian Egtvedt <[email protected]>
MIVU Solutions


Attachments:
itmtouch-1.3.2-linux-2.6.14.2.patch (5.26 kB)