diff mbox

make ektf3k driver report non-MT events too

Message ID 21867792.FJIVt2VMs6@crushinator
State New
Headers show

Commit Message

Matt Whitlock Aug. 9, 2013, 5:46 a.m. UTC 
On Thursday, 8 August 2013, at 1:49 pm, Seth Forshee wrote:
> On Wed, Aug 07, 2013 at 01:51:12PM -0400, Matt Whitlock wrote:
> > Joseph Salisbury requested that I submit this patch to the Ubuntu Kernel Team mailing list for review.
> > 
> > > This patch resolves a shortcoming of the ektf3k driver. Previously, the
> > > driver was reporting only multi-touch events. xf86-input-evdev 2.7.3
> > > crashes upon receiving input events from [that] driver, and 2.8.1 simply
> > > ignores the input device altogether as a workaround for the crash.
> > > 
> > > This patch adds single-touch ABS_X, ABS_Y, and ABS_PRESSURE axes, as well as
> > > the BTN_TOUCH key. Only the first finger touching reports on these
> > > single-touch axes (while continuing to report on the multi-touch axes);
> > > additional fingers report only on the multi-touch axes.
> > > 
> > > I have tested this patch on a Nexus 7 running X.org server 1.13.4 and
> > > xf86-input-evdev 2.8.1. Without the patch, evdev ignores all input from the
> > > touchscreen. With the patch, evdev generates mouse events in response to
> > > touches, as one would expect.
> > 
> > https://bugs.launchpad.net/ubuntu/+source/linux-grouper/+bug/1209049
> 
> input_mt_report_pointer_emulation() exists for exactly this purpose, so
> it would be preferable to use that instead.

Thanks for the pointer, Seth.  I am not too familiar with the kernel input layer.  I've adjusted my patch and confirmed that it still works as intended.  See the Launchpad bug or the attached.

Comments

Seth Forshee Aug. 9, 2013, 1:59 p.m. UTC | #1 
On Fri, Aug 09, 2013 at 01:46:14AM -0400, Matt Whitlock wrote:
> On Thursday, 8 August 2013, at 1:49 pm, Seth Forshee wrote:
> > On Wed, Aug 07, 2013 at 01:51:12PM -0400, Matt Whitlock wrote:
> > > Joseph Salisbury requested that I submit this patch to the Ubuntu Kernel Team mailing list for review.
> > > 
> > > > This patch resolves a shortcoming of the ektf3k driver. Previously, the
> > > > driver was reporting only multi-touch events. xf86-input-evdev 2.7.3
> > > > crashes upon receiving input events from [that] driver, and 2.8.1 simply
> > > > ignores the input device altogether as a workaround for the crash.
> > > > 
> > > > This patch adds single-touch ABS_X, ABS_Y, and ABS_PRESSURE axes, as well as
> > > > the BTN_TOUCH key. Only the first finger touching reports on these
> > > > single-touch axes (while continuing to report on the multi-touch axes);
> > > > additional fingers report only on the multi-touch axes.
> > > > 
> > > > I have tested this patch on a Nexus 7 running X.org server 1.13.4 and
> > > > xf86-input-evdev 2.8.1. Without the patch, evdev ignores all input from the
> > > > touchscreen. With the patch, evdev generates mouse events in response to
> > > > touches, as one would expect.
> > > 
> > > https://bugs.launchpad.net/ubuntu/+source/linux-grouper/+bug/1209049
> > 
> > input_mt_report_pointer_emulation() exists for exactly this purpose, so
> > it would be preferable to use that instead.
> 
> Thanks for the pointer, Seth.  I am not too familiar with the kernel input layer.  I've adjusted my patch and confirmed that it still works as intended.  See the Launchpad bug or the attached.

Great, thanks! A few more things though.

First, I realize the indentation of that driver is already a mess, but
it would improve readability if you'd make your indentation match that
of the surrounding lines.

Second, there are two other places in the file with report MT data.
Shouldn't those places also be making this call?

And as Tim indicated, we require sign-offs to establish who authored the
patch. Our general patch submission process is documented at
https://wiki.ubuntu.com/Kernel/Dev/KernelPatches (in this case upstream
submission doesn't make sense, but we use the same format and sign-off
policy as upstream). Here's a recent example of a correctly formatted
patch for grouper that you can refer to:
https://lists.ubuntu.com/archives/kernel-team/2013-August/031399.html
diff mbox

Patch

=== modified file 'drivers/input/touchscreen/ektf3k.c'
--- drivers/input/touchscreen/ektf3k.c	2013-05-02 01:22:19 +0000
+++ drivers/input/touchscreen/ektf3k.c	2013-08-09 05:28:45 +0000
@@ -929,6 +929,7 @@ 
               fbits = fbits >> 1;
               idx += 3;
 	    }
+		input_mt_report_pointer_emulation(ts->input_dev, true);
           input_sync(idev);
 	} // checksum
 	else {
@@ -956,7 +957,6 @@ 
 	if ( (num < 3) || ((checksum & 0x00ff) == buf[34])) {   
           fbits = buf[2] & 0x30;	
 	    fbits = (fbits << 4) | buf[1]; 
-	    //input_report_key(idev, BTN_TOUCH, 1);
           for(i = 0; i < FINGER_NUM; i++){
               active = fbits & 0x1;
               if(active || mTouchStatus[i]){
@@ -979,6 +979,7 @@ 
               fbits = fbits >> 1;
               idx += 3;
 	    }
+		input_mt_report_pointer_emulation(ts->input_dev, true);
           input_sync(idev);
 	} // checksum
 	else {
@@ -1505,11 +1506,15 @@ 
 	}
 	ts->input_dev->name = "elan-touchscreen";  
 
-	//set_bit(BTN_TOUCH, ts->input_dev->keybit);
+	set_bit(BTN_TOUCH, ts->input_dev->keybit);
 	ts->abs_x_max =  pdata->abs_x_max;
 	ts->abs_y_max = pdata->abs_y_max;
 	touch_debug(DEBUG_INFO, "[Elan] Max X=%d, Max Y=%d\n", ts->abs_x_max, ts->abs_y_max);
 
+	input_set_abs_params(ts->input_dev, ABS_X, pdata->abs_y_min, pdata->abs_y_max, 0, 0); // for 800 * 1280
+	input_set_abs_params(ts->input_dev, ABS_Y, pdata->abs_x_min, pdata->abs_x_max, 0, 0); // for 800 * 1280
+	input_set_abs_params(ts->input_dev, ABS_PRESSURE, 0, MAX_FINGER_PRESSURE, 0, 0);
+
 	input_mt_init_slots(ts->input_dev, FINGER_NUM);
 	input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, pdata->abs_y_min,  pdata->abs_y_max, 0, 0); // for 800 * 1280 
 	input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, pdata->abs_x_min,  pdata->abs_x_max, 0, 0);// for 800 * 1280