linux-kernel - Re: [PATCH 6/6] Input: elantech

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 19 Aug 2011 16:29:57 +0800
From:	JJ Ding <jj_ding@....com.tw>
To:	Seth Forshee <seth.forshee@...onical.com>
Cc:	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Aaron Huang <aaron_huang@....com.tw>,
	Tom Lin <tom_lin@....com.tw>,
	Eric Piel <E.A.B.Piel@...elft.nl>,
	Daniel Kurtz <djkurtz@...omium.org>,
	Chase Douglas <chase.douglas@...onical.com>,
	Henrik Rydberg <rydberg@...omail.se>,
	Alessandro Rubini <rubini@...l.unipv.it>
Subject: Re: [PATCH 6/6] Input: elantech - add v3 hardware support

Hi Seth,

On Thu, 18 Aug 2011 12:39:59 -0500, Seth Forshee <seth.forshee@...onical.com> wrote:
> On Thu, Aug 18, 2011 at 09:57:09AM +0800, JJ Ding wrote:
> > @@ -352,6 +374,94 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
> >  	input_sync(dev);
> >  }
> >  
> > +/*
> > + * firmware tells us there's noise.
> > + */
> > +static inline int debounce(unsigned int x, unsigned int y)
> > +{
> > +	return (x == 0xfff) && (y == 0xfff);
> > +}
> 
> This is problematic in my testing, in two ways.
> 
> First, it never actually triggers, because the y values passed to it are
> not the raw data from the packets and as such are not 0xfff anymore in
> the cases you're trying to detect.
> 
> Second, I get these packets with 1 and 2 finger touches on the Samsung
> NF310, but you only check it in the 3 finger case.
> 
> As a result, I'm getting some reports with negative values for the y
> position.
Thank you for reporting this bug.
As for now, the following should fix this issue.
I will correct this in v2.
---
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index e13a719..fa842f7 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -410,11 +410,12 @@ static void elantech_report_absolute_v3(struct psmouse *psmouse,
                 * byte 4:  .   .   .   .  y11 y10 y9  y8
                 * byte 5: y7  y6  y5  y4  y3  y2  y1  y0
                 */
-               y1 = etd->y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
+               y1 = ((packet[4] & 0x0f) << 8) | packet[5];
 
                if (fingers == 3 && debounce(x1, y1))
                        return;
 
+               y1 = etd->y_max - y1;
                break;
 
        case 2:
---
> > +
> > +/*
> > + * Interpret complete data packets and report absolute mode input events for
> > + * hardware version 3. (12 byte packets for two fingers)
> > + */
> > +static void elantech_report_absolute_v3(struct psmouse *psmouse,
> > +					int packet_type)
> > +{
> > +	struct input_dev *dev = psmouse->dev;
> > +	struct elantech_data *etd = psmouse->private;
> > +	unsigned char *packet = psmouse->packet;
> > +	unsigned int fingers = 0, x1 = 0, y1 = 0, x2 = 0, y2 = 0;
> > +	unsigned int width = 0, pres = 0;
> > +
> > +	/* byte 0: n1  n0   .   .   .   .   R   L */
> > +	fingers = (packet[0] & 0xc0) >> 6;
> > +
> > +	switch (fingers) {
> > +	case 3:
> > +	case 1:
> > +		/*
> > +		 * byte 1:  .   .   .   .  x11 x10 x9  x8
> > +		 * byte 2: x7  x6  x5  x4  x4  x2  x1  x0
> > +		 */
> > +		x1 = ((packet[1] & 0x0f) << 8) | packet[2];
> > +		/*
> > +		 * byte 4:  .   .   .   .  y11 y10 y9  y8
> > +		 * byte 5: y7  y6  y5  y4  y3  y2  y1  y0
> > +		 */
> > +		y1 = etd->y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
> > +
> > +		if (fingers == 3 && debounce(x1, y1))
> > +			return;
> > +
> > +		break;
> > +
> > +	case 2:
> > +		if (packet_type == PACKET_V3_HEAD) {
> > +			/*
> > +			 * byte 1:   .    .    .    .  ax11 ax10 ax9  ax8
> > +			 * byte 2: ax7  ax6  ax5  ax4  ax3  ax2  ax1  ax0
> > +			 */
> > +			etd->prev_x = ((packet[1] & 0x0f) << 8) | packet[2];
> > +			/*
> > +			 * byte 4:   .    .    .    .  ay11 ay10 ay9  ay8
> > +			 * byte 5: ay7  ay6  ay5  ay4  ay3  ay2  ay1  ay0
> > +			 */
> > +			etd->prev_y = etd->y_max -
> > +				(((packet[4] & 0x0f) << 8) | packet[5]);
> > +			/*
> > +			 * wait for next packet
> > +			 */
> > +			return;
> > +		}
> > +
> > +		/* packet_type == PACKET_V3_TAIL */
> > +		x1 = etd->prev_x;
> > +		y1 = etd->prev_y;
> > +		x2 = ((packet[1] & 0x0f) << 8) | packet[2];
> > +		y2 = etd->y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
> > +		break;
> > +	}
> > +
> > +	pres = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4);
> > +	width = ((packet[0] & 0x30) >> 2) | ((packet[3] & 0x30) >> 4);
> > +
> > +	input_report_key(dev, BTN_TOUCH, fingers != 0);
> > +	input_report_abs(dev, ABS_X, x1);
> > +	input_report_abs(dev, ABS_Y, y1);
> 
> You should only report the ABS_[XY] coordinates when fingers != 0. The
> xorg synaptics module sees the values reported in that case as
> legitimate. This is causing me to see strange behaviors when scrolling
> with two-finger drags.
AFAIK, though v2 and v3 differ in packet format, they really report the
same data to the userspace. In this version of v3 support, I even try to
make v2 and v3 report all the data in the same sequnce. If you're seeing
this issue, maybe we should do the same with v2?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/