Re: [PATCH 6/6] Input: elantech - add v3 hardware support

Seth Forshee
2011-08-18T17:40:06Z

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.

> +
> +/*
> + * 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.