Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 31 Jul 2005 23:27:36 +0200 (CEST)
From:      Marcin Koziej <mkoziej@mion.elka.pw.edu.pl>
To:        FreeBSD-gnats-submit@FreeBSD.org
Subject:   kern/84411: [PATCH] psm drivers adds bad buttons for Synaptics touchpad
Message-ID:  <200507312127.j6VLRabM003405@nat.dembego6.waw.pl>
Resent-Message-ID: <200507312230.j6VMUMV2015258@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         84411
>Category:       kern
>Synopsis:       [PATCH] psm drivers adds bad buttons for Synaptics touchpad
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sun Jul 31 22:30:22 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Marcin Koziej <mkoziej@mion.elka.pw.edu.pl>
>Release:        FreeBSD 6.0-BETA1 i386
>Organization:
>Environment:
System: FreeBSD carnivore 6.0-BETA1 FreeBSD 6.0-BETA1 #0: Sun Jul 31 14:24:31 CEST 2005 root@carnivore:/home/src/sys/i386/compile/AVI i386

ACER ASPIRE 1525WLMi laptop with Synaptics touchpad.

>Description:
	The psm driver running Synaptics touchpad (hw.psm.synaptics_support=1) in wide mode reports extended button presses which user didn't press. 
	Extended buttons can be reported when moving mouse with right key pressed.

	This is due incorrect interpretation of packets. The extended buttons are reported only when RightButtonBit ^ ExtendedFlagBit != 0
The code incorrectly reports the extended buttons when ExtendedFlagBit != 0

	Additionally, `tapping' code is supposed to report:
		Button 1 when one finger is tapping (4 <= w)
		Button 3 when two fingers are tapping (w == 0)
		Button 2 when thre fingers are tapping (w == 1)
	In reality, button 3 is sometimes reported when tapping with one finger.
	This happens because touchpad reports two fingers, but with very little pressure ( <= 3). This situation should not trigger button 3.
	
>How-To-Repeat:
	Using touchpad with extended buttons, move mouse with right button pressed. extended buttons will be reported. (For instance, doing it over firefox window will scroll down and up or go back/forward randomly)

	Try to tap with one finger. You will get a right button press once in a while. (For instance, tap on the firefox window a little bit, the popup menu will show up)

>Fix:

	The psm-extbuttons patch fixes problems with wrong buttons being reported.
	The changes were according to excellent documentation from Synaptics (http://www.synaptics.com/support/dev_support.cfm).

	Changes: 
	psm-extbuttons.patch:
		Query for extended capability list was fixed, added query for extended buttons number. Two new fields in struct synapticshw, capMiddle (Touchad with middle button) and extButtons (Number of extended buttons).
		Improved reporting of extended buttons for touchpads
			fourButtons (Left, Right, Up, Down) (untested)
			middle button (Left, Right, Middle) (untested)
			extended buttons (up to 8 extended buttons support) (tested 4btns, correct)
	psm-tap.patch:
		Added a condition to only report right button when z value (pressure) is more then 3. (It fixes the problem completely for my touchpad. However, testing on other touchpads is propably needed)

--- psm-extbuttons.patch begins here ---
--- sys/dev/atkbdc/psm.c.orig	Sat Jul 30 17:53:18 2005
+++ sys/dev/atkbdc/psm.c	Sun Jul 31 13:25:55 2005
@@ -2511,13 +2511,14 @@
 
 	case MOUSE_MODEL_SYNAPTICS:
 	    /* TouchPad PS/2 absolute mode message format
+	     * with capFourButtons:
 	     *
 	     *  Bits:        7   6   5   4   3   2   1   0 (LSB)
 	     *  ------------------------------------------------
 	     *  ipacket[0]:  1   0  W3  W2   0  W1   R   L
 	     *  ipacket[1]: Yb  Ya  Y9  Y8  Xb  Xa  X9  X8
 	     *  ipacket[2]: Z7  Z6  Z5  Z4  Z3  Z2  Z1  Z0
-	     *  ipacket[3]:  1   1  Yc  Xc   0  W0   D   U
+	     *  ipacket[3]:  1   1  Yc  Xc   0  W0 D^R U^L
 	     *  ipacket[4]: X7  X6  X5  X4  X3  X2  X1  X0
 	     *  ipacket[5]: Y7  Y6  Y5  Y4  Y3  Y2  Y1  Y0
 	     *
@@ -2531,6 +2532,20 @@
 	     *  Y: x position
 	     *  Z: pressure
 	     *
+		 * Without capFourButtons but with extButtons and/or capMiddle
+		 *  Bits:        7   6   5   4      3      2      1      0 (LSB)
+	     *  ------------------------------------------------------
+	     *  ipacket[3]:  1   1  Yc  Xc      0     W0    E^R    M^L
+	     *  ipacket[4]: X7  X6  X5  X4  X3|b7  X2|b5  X1|b3  X0|b1
+	     *  ipacket[5]: Y7  Y6  Y5  Y4  Y3|b8  Y2|b6  Y1|b4  Y0|b2
+		 * 
+		 * Legend:
+		 * M: middle physical mouse button
+		 * E: extended mouse buttons repored instead of low bits of X and Y
+		 * b1-b8: extended mouse buttons. Only ((extButtons + 1) >> 1) bits 
+		 *        are used in packet 4 and 5, for reading X and Y value 
+		 *        they should be zeroed.
+		 *
 	     * Absolute reportable limits:    0 - 6143.
 	     * Typical bezel limits:       1472 - 5472.
 	     * Typical edge marings:       1632 - 5312.
@@ -2590,38 +2605,41 @@
 	    if (pb->ipacket[0] & 0x02)
 		  touchpad_buttons |= MOUSE_BUTTON3DOWN;
 
-	    if (sc->synhw.capExtended && sc->synhw.capFourButtons) {
-		if ((pb->ipacket[3] & 0x01) && (pb->ipacket[0] & 0x01) == 0)
-		    touchpad_buttons |= MOUSE_BUTTON4DOWN;
-		if ((pb->ipacket[3] & 0x02) && (pb->ipacket[0] & 0x02) == 0)
-		    touchpad_buttons |= MOUSE_BUTTON5DOWN;
-	    }
-
-	    /* 
-	     * In newer pads - bit 0x02 in the third byte of
-	     * the packet indicates that we have an extended
-	     * button press.
-	     */
-	    if (pb->ipacket[3] & 0x02) {
-	        /* 
-		 * if directional_scrolls is not 1, we treat
-	     	 * any of the scrolling directions as middle-click.
-	     	 */
-		if (sc->syninfo.directional_scrolls) {
-		    if (pb->ipacket[4] & 0x01)
-			touchpad_buttons |= MOUSE_BUTTON4DOWN;
-		    if (pb->ipacket[5] & 0x01)
-			touchpad_buttons |= MOUSE_BUTTON5DOWN;
-		    if (pb->ipacket[4] & 0x02)
-			touchpad_buttons |= MOUSE_BUTTON6DOWN;
-   		    if (pb->ipacket[5] & 0x02)
-			touchpad_buttons |= MOUSE_BUTTON7DOWN;
-		} else {
-		    if ((pb->ipacket[4] & 0x0F) || (pb->ipacket[5] & 0x0F))
-			touchpad_buttons |= MOUSE_BUTTON2DOWN;
+		if (sc->synhw.capExtended && sc->synhw.capFourButtons) {
+			if ((pb->ipacket[3] ^ pb->ipacket[0]) & 0x01)
+				touchpad_buttons |= MOUSE_BUTTON4DOWN;
+			if ((pb->ipacket[3] ^ pb->ipacket[0]) & 0x02)
+				touchpad_buttons |= MOUSE_BUTTON5DOWN;
+		}
+		else if (sc->synhw.capExtended)
+		{
+			/* Middle button */
+			if ((pb->ipacket[3] & sc->synhw.capMiddle) && ((pb->ipacket[0] ^ pb->ipacket[3]) & 0x01))
+				touchpad_buttons |= MOUSE_BUTTON2DOWN;
+			/* Extended buttons */
+			if (sc->synhw.extButtons > 0 && ((pb->ipacket[0] ^ pb->ipacket[3]) & 0x02))
+			{
+				if (sc->syninfo.directional_scrolls) {
+					/* Iterate over extButtons bits, see legend. */
+					int cb=0;
+					for (cb = 0; cb < sc->synhw.extButtons; cb++)
+					{
+						if (pb->ipacket[4 + (cb%2)] & (1 << (cb >> 1)) )
+							touchpad_buttons |= (MOUSE_BUTTON4DOWN << cb);
+					}
+				} else {
+					/* Or just report middle button as the user requested */
+					touchpad_buttons |= MOUSE_BUTTON2DOWN;
+				}
+				/*
+				 * Zeros out bits used by buttons.
+				 * ( sc->synhw.extButons + 1 ) >> 1  bits will be masked out in both bytes.
+				 * ( 1 << x ) - 1 computest the mask for x bits.
+				 */
+				pb->ipacket[4] &= ~((1 << ((sc->synhw.extButtons+1) >> 1)) - 1);
+				pb->ipacket[5] &= ~((1 << ((sc->synhw.extButtons+1) >> 1)) - 1);
+			}
 		}
-
-	    }
 
 	    ms.button = touchpad_buttons | guest_buttons;
 		
@@ -3247,7 +3265,8 @@
 {
     int status[3];
     KBDC kbdc;
-
+	unsigned char nExtQueries=0;
+	
     if (!synaptics_support)
 	return (FALSE);
 
@@ -3366,7 +3385,7 @@
     /* Read the extended capability bits */
     if (mouse_ext_command(kbdc, 2) == 0)
 	return (FALSE);
-    if (get_mouse_status(kbdc, status, 0, 3) != 3)
+    if (get_mouse_status(kbdc, status, 0, 2) != 2)
 	return (FALSE);
     if (status[1] != 0x47) {
 	printf("  Failed to read extended capability bits\n");
@@ -3375,13 +3394,14 @@
 
     /* Set the different capabilities when they exist */
     if ((status[0] & 0x80) >> 7) {
-	sc->synhw.capExtended    = (status[0] & 0x80) >> 7;
-    	sc->synhw.capPassthrough = (status[2] & 0x80) >> 7;
-    	sc->synhw.capSleep       = (status[2] & 0x10) >> 4;
-    	sc->synhw.capFourButtons = (status[2] & 0x08) >> 3;
-    	sc->synhw.capMultiFinger = (status[2] & 0x02) >> 1;
-    	sc->synhw.capPalmDetect  = (status[2] & 0x01);
-	
+    	sc->synhw.capExtended    = (status[0] & 0x80) >> 7;
+    	nExtQueries              = (status[0] & 0x70) >> 4;		// 3 bit.   
+    	sc->synhw.capPassthrough = (status[1] & 0x80) >> 7;
+    	sc->synhw.capSleep       = (status[1] & 0x10) >> 4;
+    	sc->synhw.capFourButtons = (status[1] & 0x08) >> 3;
+    	sc->synhw.capMultiFinger = (status[1] & 0x02) >> 1;
+    	sc->synhw.capPalmDetect  = (status[1] & 0x01);
+		sc->synhw.capMiddle		 = sc->synhw.capFourButtons ? 0 : (status[0] & 0x04) >> 2;
 	if (verbose >= 2) {
 	    printf("  Extended capabilities:\n");
 	    printf("   capExtended: %d\n", sc->synhw.capExtended);
@@ -3390,6 +3410,7 @@
 	    printf("   capFourButtons: %d\n", sc->synhw.capFourButtons);
 	    printf("   capMultiFinger: %d\n", sc->synhw.capMultiFinger);
 	    printf("   capPalmDetect: %d\n", sc->synhw.capPalmDetect);
+		printf("   Number of extended queries: %d\n", nExtQueries);
 	}
 
 	/*
@@ -3443,8 +3464,20 @@
      *
      * XXX: I'm not sure this is used anywhere.
      */
+	sc->synhw.extButtons = 0;
     if (sc->synhw.capExtended && sc->synhw.capFourButtons)
-	sc->hw.buttons = 4;
+		sc->hw.buttons = 4;
+    else if (sc->synhw.capExtended && nExtQueries>0)
+	{
+		 if (mouse_ext_command(kbdc, 9) == 0)
+			return (FALSE);
+		if (get_mouse_status(kbdc, status, 0, 3) != 3)
+			return (FALSE);
+		sc->synhw.extButtons = (status[1] & 0xf0) >> 4;
+		sc->hw.buttons = 2 + sc->synhw.capMiddle + sc->synhw.extButtons;
+	}
+	else
+		sc->hw.buttons = 2 + sc->synhw.capMiddle;
 
     return (TRUE);
 }
--- sys/sys/mouse.h.orig	Sat Jul 30 18:03:30 2005
+++ sys/sys/mouse.h	Sun Jul 31 11:56:25 2005
@@ -103,10 +103,12 @@
 	int infoGeometry;
 	int capExtended;
 	int capSleep;
+	int capMiddle;
 	int capFourButtons;
 	int capMultiFinger;
 	int capPalmDetect;
 	int capPassthrough;
+	int extButtons;
 } synapticshw_t;
 
 /* iftype */
--- psm-extbuttons.patch ends here ---

--- psm-tap.patch begins here ---
--- sys/dev/atkbdc/psm.c.orig	Sat Jul 30 17:53:18 2005
+++ sys/dev/atkbdc/psm.c	Sun Jul 31 13:25:55 2005
@@ -2741,7 +2759,7 @@
 
 		if (sc->zmax > tap_threshold &&
 		    timevalcmp(&sc->lastsoftintr, &sc->taptimeout, <=)) {
-			if (w == 0)
+			if (w == 0 && z > 3)
 			    ms.button |= MOUSE_BUTTON3DOWN;
 			else if (w == 1)
 			    ms.button |= MOUSE_BUTTON2DOWN;
--- psm-tap.patch ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200507312127.j6VLRabM003405>