Re: synaptics touchpad doesn't click

Re: synaptics touchpad doesn't click

[Alex Chiang]

Hi Dmitry, Takashi,

* Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> Hi Alex,
> 
> On Sun, Dec 13, 2009 at 06:48:28PM -0700, Alex Chiang wrote:
> > Hi Dmitry,
> > 
> > I just got a shiny new piece of hardware (HP Envy 15) with a
> > Synaptics touchpad, and I can't seem to make the touchpad work
> > correctly.  In this version of the touchpad, there are actually
> > two physical buttons underneath the pad. In essence, this is a
> > touchpad that you can click too.
> > 
> > I can't get the clicking to work. Of course, I can tap to click,
> > but lacking access to the physical buttons means that I can't
> > right-click.
> > 
> > Currently, I'm not sure if this is a driver issue or perhaps
> > something higher up the stack, perhaps in X or maybe pilot error.
> > ;)
> > 
> > Distro is Ubuntu Karmic, kernel is latest upstream last pulled on
> > 13 Dec.
> > 
> > Would like some pointers on how to get started debugging.
> > 
> > Thanks,
> > /ac
> > 
> > dmesg snip:
> >  Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
> >  brd: module loaded
> >  loop: module loaded
> >  input: Macintosh mouse button emulation as /devices/virtual/input/input3
> >  [...]
> >  Synaptics Touchpad, model: 1, fw: 7.4, id: 0x1e0b1, caps: 0xd04771/0xe40000
> >  input: SynPS/2 Synaptics TouchPad as /devices/platform/i8042/serio1/input/input10
> > 
> 
> Hmm, that's the new Synaptics "clickpad"... Takashi Iwai just posted a
> coupleof patches for it on the input mailing list.

I can't respond to that thread directly because I'm not a
subscriber and can't find the message-id, so just responding
here.

Thanks, I did grab Takashi's patches and verify that they work
for me. I tested the separated patches, not the v2 combined
patch, although it doesn't make any difference based on visual
inspection of v2.

If you want, you can add my:

Tested-by: Alex Chiang <achiang@hp.com>

Thanks!
/ac

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

[-- Attachment #1: Type: text/plain, Size: 5219 bytes --]

Hi Alex,

On Mon, Dec 14, 2009 at 08:41:27PM -0700, Alex Chiang wrote:
> 
> Thanks, I did grab Takashi's patches and verify that they work
> for me. I tested the separated patches, not the v2 combined
> patch, although it doesn't make any difference based on visual
> inspection of v2.
> 
> If you want, you can add my:
> 
> Tested-by: Alex Chiang <achiang@hp.com>

Thank you very much for testing, however could you please try a slightly
different patch below (I did not quite like that the original patch
mangled device's capability field and how it was reusing 'middle' field
for different things)? It should apply on top of patch that
I am attaching. I hope I did not screw it up too much,

Thanks a lot.

-- 
Dmitry


Input: synaptics - add support for ClickPad devices

From: Takashi Iwai <tiwai@suse.de>

The new device is a button-less "clickable" touchpad, the touchpad click
is signaled the same way middle button click is signalled on touchpads
that have SYN_CAP_MIDDLE_BUTTON capability.

The touchpad will be working in what Synaptics calls "ClickZone" mode
where left, right and middle buttons are emulated as clicks in the
bottom button zone, depending on the touch position.  Dragging can be
done by keeping the button down and touching the main area again.
Unfortunately the device does not signal clicks when main area is
touched first.

Due to the fact that there is no known capability bits for ClickPads we
are forced to match on product ID.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/mouse/synaptics.c |   59 ++++++++++++++++++++++++++++++++++-----
 drivers/input/mouse/synaptics.h |    2 +
 2 files changed, 53 insertions(+), 8 deletions(-)


diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 0d60cb7..d11a673 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -41,6 +41,15 @@
 #define YMIN_NOMINAL 1408
 #define YMAX_NOMINAL 4448
 
+/*
+ * Left and right ClickPad button ranges; the gap between them is reserved
+ * for middle button.
+ */
+#define CLICKPAD_LEFT_BTN_X \
+	((XMAX_NOMINAL - XMIN_NOMINAL) * 2 / 5 + XMIN_NOMINAL)
+#define CLICKPAD_RIGHT_BTN_X \
+	((XMAX_NOMINAL - XMIN_NOMINAL) * 3 / 5 + XMIN_NOMINAL)
+
 
 /*****************************************************************************
  *	Stuff we need even when we do not want native Synaptics support
@@ -330,20 +339,52 @@ static void synaptics_parse_new_hw(unsigned char buf[],
 				   struct synaptics_data *priv,
 				   struct synaptics_hw_state *hw)
 {
-	hw->x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
-	hw->y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
+	int x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
+	int y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
 
 	hw->z = buf[2];
 	hw->w = (((buf[0] & 0x30) >> 2) |
 		 ((buf[0] & 0x04) >> 1) |
 		 ((buf[3] & 0x04) >> 2));
 
-	hw->left  = buf[0] & 0x01;
-	hw->right = buf[0] & 0x02;
+	if (SYN_CAP_CLICKPAD(priv->ext_cap)) {
+		int click = (buf[0] ^ buf[3]) & 0x01;
+
+		if (click && y < YMIN_NOMINAL) {
+			/*
+			 * User pressed in ClickZone; report new button
+			 * state but use old coordinates and don't report
+			 * any pressure to prevent pointer movement.
+			 */
+			hw->left = x < CLICKPAD_LEFT_BTN_X;
+			hw->right = x > CLICKPAD_RIGHT_BTN_X;
+			hw->middle = x >= CLICKPAD_LEFT_BTN_X &&
+				     x <= CLICKPAD_RIGHT_BTN_X;
+			hw->z = 0;
+
+		} else {
+			/*
+			 * Finger is outside of the ClickZone - report
+			 * current coordinates.
+			 */
+			hw->x = x;
+			hw->y = y;
+
+			if (!click)
+				hw->left = hw->right = hw->middle = 0;
+		}
+
+	} else {
+		hw->x = x;
+		hw->y = y;
+
+		hw->left  = buf[0] & 0x01;
+		hw->right = buf[0] & 0x02;
 
-	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
-		hw->middle = (buf[0] ^ buf[3]) & 0x01;
-		hw->scroll = hw->w == 2 ? (signed char)(buf[1]) : 0;
+		if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
+			hw->middle = (buf[0] ^ buf[3]) & 0x01;
+			hw->scroll = hw->w == 2 ? (signed char)(buf[1]) : 0;
+		}
 	}
 
 	if (SYN_CAP_FOUR_BUTTON(priv->capabilities)) {
@@ -476,8 +517,10 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 		input_report_key(dev, BTN_TOOL_TRIPLETAP, num_fingers == 3);
 	}
 
-	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities))
+	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities) ||
+	    SYN_CAP_CLICKPAD(priv->ext_cap)) {
 		input_report_key(dev, BTN_MIDDLE, hw.middle);
+	}
 
 	if (SYN_CAP_FOUR_BUTTON(priv->capabilities)) {
 		input_report_key(dev, BTN_FORWARD, hw.up);
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 838e7f2..35360a6 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -48,6 +48,8 @@
 #define SYN_CAP_VALID(c)		((((c) & 0x00ff00) >> 8) == 0x47)
 #define SYN_EXT_CAP_REQUESTS(c)		(((c) & 0x700000) >> 20)
 #define SYN_CAP_MULTI_BUTTON_NO(ec)	(((ec) & 0x00f000) >> 12)
+#define SYN_CAP_PRODUCT_ID(ec)		(((ec) & 0xff0000) >> 16)
+#define SYN_CAP_CLICKPAD(ec)		(SYN_CAP_PRODUCT_ID(ec) == 0xe4)
 
 /* synaptics modes query bits */
 #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))

[-- Attachment #2: synaptics-split-protocols.patch --]
[-- Type: text/plain, Size: 4878 bytes --]

Input: synaptics - separate old and new protocol parsing

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Split synaptics_parse_hw_state into 2 separate functions in preparation
for CLickPad support.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/mouse/synaptics.c |   98 ++++++++++++++++++++-------------------
 1 files changed, 50 insertions(+), 48 deletions(-)


diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 05689e7..0d60cb7 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -326,40 +326,34 @@ static void synaptics_pt_create(struct psmouse *psmouse)
 /*****************************************************************************
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
-
-static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data *priv, struct synaptics_hw_state *hw)
+static void synaptics_parse_new_hw(unsigned char buf[],
+				   struct synaptics_data *priv,
+				   struct synaptics_hw_state *hw)
 {
-	memset(hw, 0, sizeof(struct synaptics_hw_state));
-
-	if (SYN_MODEL_NEWABS(priv->model_id)) {
-		hw->x = (((buf[3] & 0x10) << 8) |
-			 ((buf[1] & 0x0f) << 8) |
-			 buf[4]);
-		hw->y = (((buf[3] & 0x20) << 7) |
-			 ((buf[1] & 0xf0) << 4) |
-			 buf[5]);
-
-		hw->z = buf[2];
-		hw->w = (((buf[0] & 0x30) >> 2) |
-			 ((buf[0] & 0x04) >> 1) |
-			 ((buf[3] & 0x04) >> 2));
-
-		hw->left  = (buf[0] & 0x01) ? 1 : 0;
-		hw->right = (buf[0] & 0x02) ? 1 : 0;
-
-		if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
-			hw->middle = ((buf[0] ^ buf[3]) & 0x01) ? 1 : 0;
-			if (hw->w == 2)
-				hw->scroll = (signed char)(buf[1]);
-		}
+	hw->x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
+	hw->y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
 
-		if (SYN_CAP_FOUR_BUTTON(priv->capabilities)) {
-			hw->up   = ((buf[0] ^ buf[3]) & 0x01) ? 1 : 0;
-			hw->down = ((buf[0] ^ buf[3]) & 0x02) ? 1 : 0;
-		}
+	hw->z = buf[2];
+	hw->w = (((buf[0] & 0x30) >> 2) |
+		 ((buf[0] & 0x04) >> 1) |
+		 ((buf[3] & 0x04) >> 2));
+
+	hw->left  = buf[0] & 0x01;
+	hw->right = buf[0] & 0x02;
 
-		if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) &&
-		    ((buf[0] ^ buf[3]) & 0x02)) {
+	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
+		hw->middle = (buf[0] ^ buf[3]) & 0x01;
+		hw->scroll = hw->w == 2 ? (signed char)(buf[1]) : 0;
+	}
+
+	if (SYN_CAP_FOUR_BUTTON(priv->capabilities)) {
+		hw->up   = (buf[0] ^ buf[3]) & 0x01;
+		hw->down = (buf[0] ^ buf[3]) & 0x02;
+	}
+
+	if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap)) {
+		hw->ext_buttons = 0;
+		if ((buf[0] ^ buf[3]) & 0x02) {
 			switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) {
 			default:
 				/*
@@ -368,29 +362,34 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data
 				 */
 				break;
 			case 8:
-				hw->ext_buttons |= ((buf[5] & 0x08)) ? 0x80 : 0;
-				hw->ext_buttons |= ((buf[4] & 0x08)) ? 0x40 : 0;
+				hw->ext_buttons |= (buf[5] & 0x08) << 4;
+				hw->ext_buttons |= (buf[4] & 0x08) << 3;
 			case 6:
-				hw->ext_buttons |= ((buf[5] & 0x04)) ? 0x20 : 0;
-				hw->ext_buttons |= ((buf[4] & 0x04)) ? 0x10 : 0;
+				hw->ext_buttons |= (buf[5] & 0x04) << 3;
+				hw->ext_buttons |= (buf[4] & 0x04) << 2;
 			case 4:
-				hw->ext_buttons |= ((buf[5] & 0x02)) ? 0x08 : 0;
-				hw->ext_buttons |= ((buf[4] & 0x02)) ? 0x04 : 0;
+				hw->ext_buttons |= (buf[5] & 0x02) << 2;
+				hw->ext_buttons |= (buf[4] & 0x02) << 1;
 			case 2:
-				hw->ext_buttons |= ((buf[5] & 0x01)) ? 0x02 : 0;
-				hw->ext_buttons |= ((buf[4] & 0x01)) ? 0x01 : 0;
+				hw->ext_buttons |= (buf[5] & 0x01) << 1;
+				hw->ext_buttons |= (buf[4] & 0x01) << 0;
 			}
 		}
-	} else {
-		hw->x = (((buf[1] & 0x1f) << 8) | buf[2]);
-		hw->y = (((buf[4] & 0x1f) << 8) | buf[5]);
+	}
+}
 
-		hw->z = (((buf[0] & 0x30) << 2) | (buf[3] & 0x3F));
-		hw->w = (((buf[1] & 0x80) >> 4) | ((buf[0] & 0x04) >> 1));
+static void synaptics_parse_old_hw(unsigned char buf[],
+				   struct synaptics_data *priv,
+				   struct synaptics_hw_state *hw)
+{
+	hw->x = (((buf[1] & 0x1f) << 8) | buf[2]);
+	hw->y = (((buf[4] & 0x1f) << 8) | buf[5]);
 
-		hw->left  = (buf[0] & 0x01) ? 1 : 0;
-		hw->right = (buf[0] & 0x02) ? 1 : 0;
-	}
+	hw->z = (((buf[0] & 0x30) << 2) | (buf[3] & 0x3F));
+	hw->w = (((buf[1] & 0x80) >> 4) | ((buf[0] & 0x04) >> 1));
+
+	hw->left  = buf[0] & 0x01;
+	hw->right = buf[0] & 0x02;
 }
 
 /*
@@ -405,7 +404,10 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	int finger_width;
 	int i;
 
-	synaptics_parse_hw_state(psmouse->packet, priv, &hw);
+	if (SYN_MODEL_NEWABS(priv->model_id))
+		synaptics_parse_new_hw(psmouse->packet, priv, &hw);
+	else
+		synaptics_parse_old_hw(psmouse->packet, priv, &hw);
 
 	if (hw.scroll) {
 		priv->scroll += hw.scroll;

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Mon, 14 Dec 2009 22:26:28 -0800,
Dmitry Torokhov wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> Hi Alex,
> 
> On Mon, Dec 14, 2009 at 08:41:27PM -0700, Alex Chiang wrote:
> > 
> > Thanks, I did grab Takashi's patches and verify that they work
> > for me. I tested the separated patches, not the v2 combined
> > patch, although it doesn't make any difference based on visual
> > inspection of v2.
> > 
> > If you want, you can add my:
> > 
> > Tested-by: Alex Chiang <achiang@hp.com>
> 
> Thank you very much for testing, however could you please try a slightly
> different patch below (I did not quite like that the original patch
> mangled device's capability field and how it was reusing 'middle' field
> for different things)? It should apply on top of patch that
> I am attaching. I hope I did not screw it up too much,

I can't test the patch right now since I'm at home, but I'm afraid
it's a bit different behavior.  Namely,

> @@ -330,20 +339,52 @@ static void synaptics_parse_new_hw(unsigned char buf[],
>  				   struct synaptics_data *priv,
>  				   struct synaptics_hw_state *hw)
>  {
> -	hw->x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> -	hw->y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
> +	int x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> +	int y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
>  
>  	hw->z = buf[2];
>  	hw->w = (((buf[0] & 0x30) >> 2) |
>  		 ((buf[0] & 0x04) >> 1) |
>  		 ((buf[3] & 0x04) >> 2));
>  
> -	hw->left  = buf[0] & 0x01;
> -	hw->right = buf[0] & 0x02;
> +	if (SYN_CAP_CLICKPAD(priv->ext_cap)) {
> +		int click = (buf[0] ^ buf[3]) & 0x01;
> +
> +		if (click && y < YMIN_NOMINAL) {
> +			/*
> +			 * User pressed in ClickZone; report new button
> +			 * state but use old coordinates and don't report
> +			 * any pressure to prevent pointer movement.
> +			 */
> +			hw->left = x < CLICKPAD_LEFT_BTN_X;
> +			hw->right = x > CLICKPAD_RIGHT_BTN_X;
> +			hw->middle = x >= CLICKPAD_LEFT_BTN_X &&
> +				     x <= CLICKPAD_RIGHT_BTN_X;
> +			hw->z = 0;
> +
> +		} else {
> +			/*
> +			 * Finger is outside of the ClickZone - report
> +			 * current coordinates.
> +			 */
> +			hw->x = x;
> +			hw->y = y;
> +
> +			if (!click)
> +				hw->left = hw->right = hw->middle = 0;
> +		}

Here, when you touch outside the button area, left/right/middle are
always zero because hw was initialized.  So the above code gives the
click "released" state outside the button area.

In my original patch, the button states are kept.  This makes the
second touch while pressing the pad as dragging.  Since the button
choice is done by the first touch, you can't determine the button
position by the current touch.  Thus, remembering the last button
states is mandatory for ClickZone mode.


thanks,

Takashi

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

On Tue, Dec 15, 2009 at 07:40:55AM +0100, Takashi Iwai wrote:
> At Mon, 14 Dec 2009 22:26:28 -0800,
> Dmitry Torokhov wrote:
> > 
> > [1  <text/plain; us-ascii (7bit)>]
> > Hi Alex,
> > 
> > On Mon, Dec 14, 2009 at 08:41:27PM -0700, Alex Chiang wrote:
> > > 
> > > Thanks, I did grab Takashi's patches and verify that they work
> > > for me. I tested the separated patches, not the v2 combined
> > > patch, although it doesn't make any difference based on visual
> > > inspection of v2.
> > > 
> > > If you want, you can add my:
> > > 
> > > Tested-by: Alex Chiang <achiang@hp.com>
> > 
> > Thank you very much for testing, however could you please try a slightly
> > different patch below (I did not quite like that the original patch
> > mangled device's capability field and how it was reusing 'middle' field
> > for different things)? It should apply on top of patch that
> > I am attaching. I hope I did not screw it up too much,
> 
> I can't test the patch right now since I'm at home, but I'm afraid
> it's a bit different behavior.  Namely,
> 
> > @@ -330,20 +339,52 @@ static void synaptics_parse_new_hw(unsigned char buf[],
> >  				   struct synaptics_data *priv,
> >  				   struct synaptics_hw_state *hw)
> >  {
> > -	hw->x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> > -	hw->y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
> > +	int x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> > +	int y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
> >  
> >  	hw->z = buf[2];
> >  	hw->w = (((buf[0] & 0x30) >> 2) |
> >  		 ((buf[0] & 0x04) >> 1) |
> >  		 ((buf[3] & 0x04) >> 2));
> >  
> > -	hw->left  = buf[0] & 0x01;
> > -	hw->right = buf[0] & 0x02;
> > +	if (SYN_CAP_CLICKPAD(priv->ext_cap)) {
> > +		int click = (buf[0] ^ buf[3]) & 0x01;
> > +
> > +		if (click && y < YMIN_NOMINAL) {
> > +			/*
> > +			 * User pressed in ClickZone; report new button
> > +			 * state but use old coordinates and don't report
> > +			 * any pressure to prevent pointer movement.
> > +			 */
> > +			hw->left = x < CLICKPAD_LEFT_BTN_X;
> > +			hw->right = x > CLICKPAD_RIGHT_BTN_X;
> > +			hw->middle = x >= CLICKPAD_LEFT_BTN_X &&
> > +				     x <= CLICKPAD_RIGHT_BTN_X;
> > +			hw->z = 0;
> > +
> > +		} else {
> > +			/*
> > +			 * Finger is outside of the ClickZone - report
> > +			 * current coordinates.
> > +			 */
> > +			hw->x = x;
> > +			hw->y = y;
> > +
> > +			if (!click)
> > +				hw->left = hw->right = hw->middle = 0;
> > +		}
> 
> Here, when you touch outside the button area, left/right/middle are
> always zero because hw was initialized.  So the above code gives the
> click "released" state outside the button area.
> 

No, I got rid of resetting hw state to 0.

-- 
Dmitry

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Mon, 14 Dec 2009 23:33:58 -0800,
Dmitry Torokhov wrote:
> 
> On Tue, Dec 15, 2009 at 07:40:55AM +0100, Takashi Iwai wrote:
> > At Mon, 14 Dec 2009 22:26:28 -0800,
> > Dmitry Torokhov wrote:
> > > 
> > > [1  <text/plain; us-ascii (7bit)>]
> > > Hi Alex,
> > > 
> > > On Mon, Dec 14, 2009 at 08:41:27PM -0700, Alex Chiang wrote:
> > > > 
> > > > Thanks, I did grab Takashi's patches and verify that they work
> > > > for me. I tested the separated patches, not the v2 combined
> > > > patch, although it doesn't make any difference based on visual
> > > > inspection of v2.
> > > > 
> > > > If you want, you can add my:
> > > > 
> > > > Tested-by: Alex Chiang <achiang@hp.com>
> > > 
> > > Thank you very much for testing, however could you please try a slightly
> > > different patch below (I did not quite like that the original patch
> > > mangled device's capability field and how it was reusing 'middle' field
> > > for different things)? It should apply on top of patch that
> > > I am attaching. I hope I did not screw it up too much,
> > 
> > I can't test the patch right now since I'm at home, but I'm afraid
> > it's a bit different behavior.  Namely,
> > 
> > > @@ -330,20 +339,52 @@ static void synaptics_parse_new_hw(unsigned char buf[],
> > >  				   struct synaptics_data *priv,
> > >  				   struct synaptics_hw_state *hw)
> > >  {
> > > -	hw->x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> > > -	hw->y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
> > > +	int x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> > > +	int y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
> > >  
> > >  	hw->z = buf[2];
> > >  	hw->w = (((buf[0] & 0x30) >> 2) |
> > >  		 ((buf[0] & 0x04) >> 1) |
> > >  		 ((buf[3] & 0x04) >> 2));
> > >  
> > > -	hw->left  = buf[0] & 0x01;
> > > -	hw->right = buf[0] & 0x02;
> > > +	if (SYN_CAP_CLICKPAD(priv->ext_cap)) {
> > > +		int click = (buf[0] ^ buf[3]) & 0x01;
> > > +
> > > +		if (click && y < YMIN_NOMINAL) {
> > > +			/*
> > > +			 * User pressed in ClickZone; report new button
> > > +			 * state but use old coordinates and don't report
> > > +			 * any pressure to prevent pointer movement.
> > > +			 */
> > > +			hw->left = x < CLICKPAD_LEFT_BTN_X;
> > > +			hw->right = x > CLICKPAD_RIGHT_BTN_X;
> > > +			hw->middle = x >= CLICKPAD_LEFT_BTN_X &&
> > > +				     x <= CLICKPAD_RIGHT_BTN_X;
> > > +			hw->z = 0;
> > > +
> > > +		} else {
> > > +			/*
> > > +			 * Finger is outside of the ClickZone - report
> > > +			 * current coordinates.
> > > +			 */
> > > +			hw->x = x;
> > > +			hw->y = y;
> > > +
> > > +			if (!click)
> > > +				hw->left = hw->right = hw->middle = 0;
> > > +		}
> > 
> > Here, when you touch outside the button area, left/right/middle are
> > always zero because hw was initialized.  So the above code gives the
> > click "released" state outside the button area.
> > 
> 
> No, I got rid of resetting hw state to 0.

Ah, it's in the second patch.

But looking at that one, hw is a local variable and still doesn't
inherit from the previous state, no?  If so, the button state will be
just a random value.


thanks,

Takashi

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

On Tue, Dec 15, 2009 at 08:41:05AM +0100, Takashi Iwai wrote:
> At Mon, 14 Dec 2009 23:33:58 -0800,
> Dmitry Torokhov wrote:
> > 
> > On Tue, Dec 15, 2009 at 07:40:55AM +0100, Takashi Iwai wrote:
> > > At Mon, 14 Dec 2009 22:26:28 -0800,
> > > Dmitry Torokhov wrote:
> > > > 
> > > > [1  <text/plain; us-ascii (7bit)>]
> > > > Hi Alex,
> > > > 
> > > > On Mon, Dec 14, 2009 at 08:41:27PM -0700, Alex Chiang wrote:
> > > > > 
> > > > > Thanks, I did grab Takashi's patches and verify that they work
> > > > > for me. I tested the separated patches, not the v2 combined
> > > > > patch, although it doesn't make any difference based on visual
> > > > > inspection of v2.
> > > > > 
> > > > > If you want, you can add my:
> > > > > 
> > > > > Tested-by: Alex Chiang <achiang@hp.com>
> > > > 
> > > > Thank you very much for testing, however could you please try a slightly
> > > > different patch below (I did not quite like that the original patch
> > > > mangled device's capability field and how it was reusing 'middle' field
> > > > for different things)? It should apply on top of patch that
> > > > I am attaching. I hope I did not screw it up too much,
> > > 
> > > I can't test the patch right now since I'm at home, but I'm afraid
> > > it's a bit different behavior.  Namely,
> > > 
> > > > @@ -330,20 +339,52 @@ static void synaptics_parse_new_hw(unsigned char buf[],
> > > >  				   struct synaptics_data *priv,
> > > >  				   struct synaptics_hw_state *hw)
> > > >  {
> > > > -	hw->x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> > > > -	hw->y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
> > > > +	int x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> > > > +	int y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
> > > >  
> > > >  	hw->z = buf[2];
> > > >  	hw->w = (((buf[0] & 0x30) >> 2) |
> > > >  		 ((buf[0] & 0x04) >> 1) |
> > > >  		 ((buf[3] & 0x04) >> 2));
> > > >  
> > > > -	hw->left  = buf[0] & 0x01;
> > > > -	hw->right = buf[0] & 0x02;
> > > > +	if (SYN_CAP_CLICKPAD(priv->ext_cap)) {
> > > > +		int click = (buf[0] ^ buf[3]) & 0x01;
> > > > +
> > > > +		if (click && y < YMIN_NOMINAL) {
> > > > +			/*
> > > > +			 * User pressed in ClickZone; report new button
> > > > +			 * state but use old coordinates and don't report
> > > > +			 * any pressure to prevent pointer movement.
> > > > +			 */
> > > > +			hw->left = x < CLICKPAD_LEFT_BTN_X;
> > > > +			hw->right = x > CLICKPAD_RIGHT_BTN_X;
> > > > +			hw->middle = x >= CLICKPAD_LEFT_BTN_X &&
> > > > +				     x <= CLICKPAD_RIGHT_BTN_X;
> > > > +			hw->z = 0;
> > > > +
> > > > +		} else {
> > > > +			/*
> > > > +			 * Finger is outside of the ClickZone - report
> > > > +			 * current coordinates.
> > > > +			 */
> > > > +			hw->x = x;
> > > > +			hw->y = y;
> > > > +
> > > > +			if (!click)
> > > > +				hw->left = hw->right = hw->middle = 0;
> > > > +		}
> > > 
> > > Here, when you touch outside the button area, left/right/middle are
> > > always zero because hw was initialized.  So the above code gives the
> > > click "released" state outside the button area.
> > > 
> > 
> > No, I got rid of resetting hw state to 0.
> 
> Ah, it's in the second patch.
> 
> But looking at that one, hw is a local variable and still doesn't
> inherit from the previous state, no?  If so, the button state will be
> just a random value.
> 

Indeed, we need to keep the state in synaptics now, thanks for noticing.
The updated patch is below.

-- 
Dmitry


Input: synaptics - add support for ClickPad devices

From: Takashi Iwai <tiwai@suse.de>

The new device is a button-less "clickable" touchpad, the touchpad click
is signaled the same way middle button click is signalled on touchpads
that have SYN_CAP_MIDDLE_BUTTON capability.

The touchpad will be working in what Synaptics calls "ClickZone" mode
where left, right and middle buttons are emulated as clicks in the
bottom button zone, depending on the touch position.  Dragging can be
done by keeping the button down and touching the main area again.
Unfortunately the device does not signal clicks when main area is
touched first.

Due to the fact that there is no known capability bits for ClickPads we
are forced to match on product ID.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/mouse/synaptics.c |  119 +++++++++++++++++++++++++++------------
 drivers/input/mouse/synaptics.h |    5 +-
 2 files changed, 86 insertions(+), 38 deletions(-)


diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 7047558..34df70f 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -41,6 +41,15 @@
 #define YMIN_NOMINAL 1408
 #define YMAX_NOMINAL 4448
 
+/*
+ * Left and right ClickPad button ranges; the gap between them is reserved
+ * for middle button.
+ */
+#define CLICKPAD_LEFT_BTN_X \
+	((XMAX_NOMINAL - XMIN_NOMINAL) * 2 / 5 + XMIN_NOMINAL)
+#define CLICKPAD_RIGHT_BTN_X \
+	((XMAX_NOMINAL - XMIN_NOMINAL) * 3 / 5 + XMIN_NOMINAL)
+
 
 /*****************************************************************************
  *	Stuff we need even when we do not want native Synaptics support
@@ -327,23 +336,56 @@ static void synaptics_pt_create(struct psmouse *psmouse)
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
 static void synaptics_parse_new_hw(unsigned char buf[],
-				   struct synaptics_data *priv,
-				   struct synaptics_hw_state *hw)
+				   struct synaptics_data *priv)
 {
-	hw->x = ((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4];
-	hw->y = ((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5];
+	struct synaptics_hw_state *hw = &priv->hw;
+	int x = ((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4];
+	int y = ((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5];
 
 	hw->z = buf[2];
 	hw->w = ((buf[0] & 0x30) >> 2) |
 		((buf[0] & 0x04) >> 1) |
 		((buf[3] & 0x04) >> 2);
 
-	hw->left  = buf[0] & 0x01;
-	hw->right = buf[0] & 0x02;
+	if (SYN_CAP_CLICKPAD(priv->ext_cap)) {
+		int click = (buf[0] ^ buf[3]) & 0x01;
 
-	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
-		hw->middle = (buf[0] ^ buf[3]) & 0x01;
-		hw->scroll = hw->w == 2 ? (signed char)buf[1] : 0;
+		if (click && y < YMIN_NOMINAL) {
+			/*
+			 * User pressed in ClickZone; report new button
+			 * state but use :w
+			 * old coordinates and don't report
+			 * any pressure to prevent pointer movement.
+			 */
+			hw->left = x < CLICKPAD_LEFT_BTN_X;
+			hw->right = x > CLICKPAD_RIGHT_BTN_X;
+			hw->middle = x >= CLICKPAD_LEFT_BTN_X &&
+				     x <= CLICKPAD_RIGHT_BTN_X;
+			hw->z = 0;
+
+		} else {
+			/*
+			 * Finger is outside of the ClickZone - report
+			 * current coordinates.
+			 */
+			hw->x = x;
+			hw->y = y;
+
+			if (!click)
+				hw->left = hw->right = hw->middle = 0;
+		}
+
+	} else {
+		hw->x = x;
+		hw->y = y;
+
+		hw->left  = buf[0] & 0x01;
+		hw->right = buf[0] & 0x02;
+
+		if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
+			hw->middle = (buf[0] ^ buf[3]) & 0x01;
+			hw->scroll = hw->w == 2 ? (signed char)buf[1] : 0;
+		}
 	}
 
 	if (SYN_CAP_FOUR_BUTTON(priv->capabilities)) {
@@ -379,9 +421,10 @@ static void synaptics_parse_new_hw(unsigned char buf[],
 }
 
 static void synaptics_parse_old_hw(unsigned char buf[],
-				   struct synaptics_data *priv,
-				   struct synaptics_hw_state *hw)
+				   struct synaptics_data *priv)
 {
+	struct synaptics_hw_state *hw = &priv->hw;
+
 	hw->x = ((buf[1] & 0x1f) << 8) | buf[2];
 	hw->y = ((buf[4] & 0x1f) << 8) | buf[5];
 
@@ -399,44 +442,44 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 {
 	struct input_dev *dev = psmouse->dev;
 	struct synaptics_data *priv = psmouse->private;
-	struct synaptics_hw_state hw;
+	struct synaptics_hw_state *hw = &priv->hw;
 	int num_fingers;
 	int finger_width;
 	int i;
 
 	if (SYN_MODEL_NEWABS(priv->model_id))
-		synaptics_parse_new_hw(psmouse->packet, priv, &hw);
+		synaptics_parse_new_hw(psmouse->packet, priv);
 	else
-		synaptics_parse_old_hw(psmouse->packet, priv, &hw);
+		synaptics_parse_old_hw(psmouse->packet, priv);
 
-	if (hw.scroll) {
-		priv->scroll += hw.scroll;
+	if (hw->scroll) {
+		priv->scroll += hw->scroll;
 
 		while (priv->scroll >= 4) {
-			input_report_key(dev, BTN_BACK, !hw.down);
+			input_report_key(dev, BTN_BACK, !hw->down);
 			input_sync(dev);
-			input_report_key(dev, BTN_BACK, hw.down);
+			input_report_key(dev, BTN_BACK, hw->down);
 			input_sync(dev);
 			priv->scroll -= 4;
 		}
 		while (priv->scroll <= -4) {
-			input_report_key(dev, BTN_FORWARD, !hw.up);
+			input_report_key(dev, BTN_FORWARD, !hw->up);
 			input_sync(dev);
-			input_report_key(dev, BTN_FORWARD, hw.up);
+			input_report_key(dev, BTN_FORWARD, hw->up);
 			input_sync(dev);
 			priv->scroll += 4;
 		}
 		return;
 	}
 
-	if (hw.z > 0) {
+	if (hw->z > 0) {
 		num_fingers = 1;
 		finger_width = 5;
 		if (SYN_CAP_EXTENDED(priv->capabilities)) {
-			switch (hw.w) {
+			switch (hw->w) {
 			case 0 ... 1:
 				if (SYN_CAP_MULTIFINGER(priv->capabilities))
-					num_fingers = hw.w + 2;
+					num_fingers = hw->w + 2;
 				break;
 			case 2:
 				if (SYN_MODEL_PEN(priv->model_id))
@@ -444,7 +487,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 				break;
 			case 4 ... 15:
 				if (SYN_CAP_PALMDETECT(priv->capabilities))
-					finger_width = hw.w;
+					finger_width = hw->w;
 				break;
 			}
 		}
@@ -457,35 +500,37 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	 * BTN_TOUCH has to be first as mousedev relies on it when doing
 	 * absolute -> relative conversion
 	 */
-	if (hw.z > 30) input_report_key(dev, BTN_TOUCH, 1);
-	if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0);
+	if (hw->z > 30) input_report_key(dev, BTN_TOUCH, 1);
+	if (hw->z < 25) input_report_key(dev, BTN_TOUCH, 0);
 
-	if (hw.z > 0) {
-		input_report_abs(dev, ABS_X, hw.x);
-		input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y);
+	if (hw->z > 0) {
+		input_report_abs(dev, ABS_X, hw->x);
+		input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw->y);
 	}
-	input_report_abs(dev, ABS_PRESSURE, hw.z);
+	input_report_abs(dev, ABS_PRESSURE, hw->z);
 
 	input_report_abs(dev, ABS_TOOL_WIDTH, finger_width);
 	input_report_key(dev, BTN_TOOL_FINGER, num_fingers == 1);
-	input_report_key(dev, BTN_LEFT, hw.left);
-	input_report_key(dev, BTN_RIGHT, hw.right);
+	input_report_key(dev, BTN_LEFT, hw->left);
+	input_report_key(dev, BTN_RIGHT, hw->right);
 
 	if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
 		input_report_key(dev, BTN_TOOL_DOUBLETAP, num_fingers == 2);
 		input_report_key(dev, BTN_TOOL_TRIPLETAP, num_fingers == 3);
 	}
 
-	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities))
-		input_report_key(dev, BTN_MIDDLE, hw.middle);
+	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities) ||
+	    SYN_CAP_CLICKPAD(priv->ext_cap)) {
+		input_report_key(dev, BTN_MIDDLE, hw->middle);
+	}
 
 	if (SYN_CAP_FOUR_BUTTON(priv->capabilities)) {
-		input_report_key(dev, BTN_FORWARD, hw.up);
-		input_report_key(dev, BTN_BACK, hw.down);
+		input_report_key(dev, BTN_FORWARD, hw->up);
+		input_report_key(dev, BTN_BACK, hw->down);
 	}
 
 	for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++)
-		input_report_key(dev, BTN_0 + i, hw.ext_buttons & (1 << i));
+		input_report_key(dev, BTN_0 + i, hw->ext_buttons & (1 << i));
 
 	input_sync(dev);
 }
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 838e7f2..e00c53d 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -48,6 +48,8 @@
 #define SYN_CAP_VALID(c)		((((c) & 0x00ff00) >> 8) == 0x47)
 #define SYN_EXT_CAP_REQUESTS(c)		(((c) & 0x700000) >> 20)
 #define SYN_CAP_MULTI_BUTTON_NO(ec)	(((ec) & 0x00f000) >> 12)
+#define SYN_CAP_PRODUCT_ID(ec)		(((ec) & 0xff0000) >> 16)
+#define SYN_CAP_CLICKPAD(ec)		(SYN_CAP_PRODUCT_ID(ec) == 0xe4)
 
 /* synaptics modes query bits */
 #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))
@@ -100,9 +102,10 @@ struct synaptics_data {
 	int x_res;				/* X resolution in units/mm */
 	int y_res;				/* Y resolution in units/mm */
 
+	struct synaptics_hw_state hw;
+	int scroll;
 	unsigned char pkt_type;			/* packet type - old, new, etc */
 	unsigned char mode;			/* current mode byte */
-	int scroll;
 };
 
 void synaptics_module_init(void);

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Tue, 15 Dec 2009 00:25:18 -0800,
Dmitry Torokhov wrote:
> 
> On Tue, Dec 15, 2009 at 08:41:05AM +0100, Takashi Iwai wrote:
> > At Mon, 14 Dec 2009 23:33:58 -0800,
> > Dmitry Torokhov wrote:
> > > 
> > > On Tue, Dec 15, 2009 at 07:40:55AM +0100, Takashi Iwai wrote:
> > > > At Mon, 14 Dec 2009 22:26:28 -0800,
> > > > Dmitry Torokhov wrote:
> > > > > 
> > > > > [1  <text/plain; us-ascii (7bit)>]
> > > > > Hi Alex,
> > > > > 
> > > > > On Mon, Dec 14, 2009 at 08:41:27PM -0700, Alex Chiang wrote:
> > > > > > 
> > > > > > Thanks, I did grab Takashi's patches and verify that they work
> > > > > > for me. I tested the separated patches, not the v2 combined
> > > > > > patch, although it doesn't make any difference based on visual
> > > > > > inspection of v2.
> > > > > > 
> > > > > > If you want, you can add my:
> > > > > > 
> > > > > > Tested-by: Alex Chiang <achiang@hp.com>
> > > > > 
> > > > > Thank you very much for testing, however could you please try a slightly
> > > > > different patch below (I did not quite like that the original patch
> > > > > mangled device's capability field and how it was reusing 'middle' field
> > > > > for different things)? It should apply on top of patch that
> > > > > I am attaching. I hope I did not screw it up too much,
> > > > 
> > > > I can't test the patch right now since I'm at home, but I'm afraid
> > > > it's a bit different behavior.  Namely,
> > > > 
> > > > > @@ -330,20 +339,52 @@ static void synaptics_parse_new_hw(unsigned char buf[],
> > > > >  				   struct synaptics_data *priv,
> > > > >  				   struct synaptics_hw_state *hw)
> > > > >  {
> > > > > -	hw->x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> > > > > -	hw->y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
> > > > > +	int x = (((buf[3] & 0x10) << 8) | ((buf[1] & 0x0f) << 8) | buf[4]);
> > > > > +	int y = (((buf[3] & 0x20) << 7) | ((buf[1] & 0xf0) << 4) | buf[5]);
> > > > >  
> > > > >  	hw->z = buf[2];
> > > > >  	hw->w = (((buf[0] & 0x30) >> 2) |
> > > > >  		 ((buf[0] & 0x04) >> 1) |
> > > > >  		 ((buf[3] & 0x04) >> 2));
> > > > >  
> > > > > -	hw->left  = buf[0] & 0x01;
> > > > > -	hw->right = buf[0] & 0x02;
> > > > > +	if (SYN_CAP_CLICKPAD(priv->ext_cap)) {
> > > > > +		int click = (buf[0] ^ buf[3]) & 0x01;
> > > > > +
> > > > > +		if (click && y < YMIN_NOMINAL) {
> > > > > +			/*
> > > > > +			 * User pressed in ClickZone; report new button
> > > > > +			 * state but use old coordinates and don't report
> > > > > +			 * any pressure to prevent pointer movement.
> > > > > +			 */
> > > > > +			hw->left = x < CLICKPAD_LEFT_BTN_X;
> > > > > +			hw->right = x > CLICKPAD_RIGHT_BTN_X;
> > > > > +			hw->middle = x >= CLICKPAD_LEFT_BTN_X &&
> > > > > +				     x <= CLICKPAD_RIGHT_BTN_X;
> > > > > +			hw->z = 0;
> > > > > +
> > > > > +		} else {
> > > > > +			/*
> > > > > +			 * Finger is outside of the ClickZone - report
> > > > > +			 * current coordinates.
> > > > > +			 */
> > > > > +			hw->x = x;
> > > > > +			hw->y = y;
> > > > > +
> > > > > +			if (!click)
> > > > > +				hw->left = hw->right = hw->middle = 0;
> > > > > +		}
> > > > 
> > > > Here, when you touch outside the button area, left/right/middle are
> > > > always zero because hw was initialized.  So the above code gives the
> > > > click "released" state outside the button area.
> > > > 
> > > 
> > > No, I got rid of resetting hw state to 0.
> > 
> > Ah, it's in the second patch.
> > 
> > But looking at that one, hw is a local variable and still doesn't
> > inherit from the previous state, no?  If so, the button state will be
> > just a random value.
> > 
> 
> Indeed, we need to keep the state in synaptics now, thanks for noticing.
> The updated patch is below.

Setting BTN_MIDDLE bit in set_input_params() is missing for clickpad.

Otherwise it looks OK to me (and the patch description is much better
than mine ;)  Will test the patch later.


thanks,

Takashi

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Tue, 15 Dec 2009 00:25:18 -0800,
Dmitry Torokhov wrote:
> 
> Indeed, we need to keep the state in synaptics now, thanks for noticing.
> The updated patch is below.

Also, one minor thing I noticed:

> -	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
> -		hw->middle = (buf[0] ^ buf[3]) & 0x01;
> -		hw->scroll = hw->w == 2 ? (signed char)buf[1] : 0;
> +		if (click && y < YMIN_NOMINAL) {

In my original patch, the position reporting in the button area is
disabled no matter whether clicked or not.  This was intentional
because I find it quite annoying that the mouse pointer moves slightly
when I click.  I often missed the target when I pressed strongly,
because my finger slipped a millimeter before the click state got
active.

This is a matter of taste, though.


> +			/*
> +			 * User pressed in ClickZone; report new button
> +			 * state but use :w

It's not in "w" field...


thanks,

Takashi

Re: synaptics touchpad doesn't click

[Alex Chiang]

* Dmitry Torokhov <dmitry.torokhov@gmail.com>:
>
> The updated patch is below.
> 
> -- 
> Dmitry

Should I test this one or wait one more iteration to address
Takashi's last comments?

Thanks,
/ac

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> >
> > The updated patch is below.
> > 
> > -- 
> > Dmitry
> 
> Should I test this one or wait one more iteration to address
> Takashi's last comments?
> 

Actually I think we took the wrong direction with the original patch and
we should do what other buttonless devices (bcm5974) do: report touchpad
click as left button and have Synaptics X driver provide enhanced
support. This way we can have both modes (ClickZones and ClickButtons)
and users will get to chose (provided that someone takes time to add
that support to Synaptics driver of course ;) ).

Could you tell me if the below works for you?

-- 
Dmitry


Input: synaptics - add support for ClickPad devices

From: Takashi Iwai <tiwai@suse.de>

The new device is a button-less "clickable" touchpad, it does not have
physical left, right nor middle buttons. Instead the entire touchpad
area can be "clicked" and such click is signalled the same way middle
button state is signalled on touchpads that have SYN_CAP_MIDDLE_BUTTON
capability.

The driver will reporting touchpad clicks as primary button (BTN_LEFT)
events, the same way driver for bcm5974 handles buttonless touchpads on
MacBooks, allowing limited support for legacy software. Enhanced support,
including "ClickButtons" and "ClickZone" modes to emulate right and
middle nuttons, is left to userspace components.

The device will not signal BTN_RIGHT event and will remove it from the
capability list. Since all Synaptics devices up until now had both left
and right buttons present the absence of BTN_RIGHT capability can be
used to identify ClickPad devices.

Due to the fact that there is no known capability bits for ClickPads we
are forced to match on product ID.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/mouse/synaptics.c |   21 ++++++++++++++++-----
 drivers/input/mouse/synaptics.h |    2 ++
 2 files changed, 18 insertions(+), 5 deletions(-)


diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 7047558..633b975 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -338,12 +338,16 @@ static void synaptics_parse_new_hw(unsigned char buf[],
 		((buf[0] & 0x04) >> 1) |
 		((buf[3] & 0x04) >> 2);
 
-	hw->left  = buf[0] & 0x01;
-	hw->right = buf[0] & 0x02;
+	if (SYN_CAP_CLICKPAD(priv->ext_cap)) {
+		hw->left = (buf[0] ^ buf[3]) & 0x01;
+	} else {
+		hw->left  = buf[0] & 0x01;
+		hw->right = buf[0] & 0x02;
 
-	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
-		hw->middle = (buf[0] ^ buf[3]) & 0x01;
-		hw->scroll = hw->w == 2 ? (signed char)buf[1] : 0;
+		if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
+			hw->middle = (buf[0] ^ buf[3]) & 0x01;
+			hw->scroll = hw->w == 2 ? (signed char)buf[1] : 0;
+		}
 	}
 
 	if (SYN_CAP_FOUR_BUTTON(priv->capabilities)) {
@@ -592,6 +596,13 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	__clear_bit(REL_X, dev->relbit);
 	__clear_bit(REL_Y, dev->relbit);
 
+	/*
+	 * ClickPads are buttonless devices. We report the touchpad clicks
+	 * as BTN_LEFT but there is no BTN_RIGHT or BTN_MIDDLE.
+	 */
+	if (SYN_CAP_CLICKPAD(priv->ext_cap))
+		__clear_bit(BTN_RIGHT, dev->keybit);
+
 	dev->absres[ABS_X] = priv->x_res;
 	dev->absres[ABS_Y] = priv->y_res;
 }
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 838e7f2..343dfc0 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -48,6 +48,8 @@
 #define SYN_CAP_VALID(c)		((((c) & 0x00ff00) >> 8) == 0x47)
 #define SYN_EXT_CAP_REQUESTS(c)		(((c) & 0x700000) >> 20)
 #define SYN_CAP_MULTI_BUTTON_NO(ec)	(((ec) & 0x00f000) >> 12)
+#define SYN_CAP_PRODUCT_ID(ec)          (((ec) & 0xff0000) >> 16)
+#define SYN_CAP_CLICKPAD(ec)            (SYN_CAP_PRODUCT_ID(ec) == 0xe4)
 
 /* synaptics modes query bits */
 #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Tue, 15 Dec 2009 18:59:34 -0800,
Dmitry Torokhov wrote:
> 
> On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > >
> > > The updated patch is below.
> > > 
> > > -- 
> > > Dmitry
> > 
> > Should I test this one or wait one more iteration to address
> > Takashi's last comments?
> > 
> 
> Actually I think we took the wrong direction with the original patch and
> we should do what other buttonless devices (bcm5974) do: report touchpad
> click as left button and have Synaptics X driver provide enhanced
> support. This way we can have both modes (ClickZones and ClickButtons)
> and users will get to chose (provided that someone takes time to add
> that support to Synaptics driver of course ;) ).

My concern is, still, how would you identify this device.  Will you
extend also some ioctls to expose caps and extcaps?  Otherwise it's
difficult to identify this device automatically from the user-space.

The user-space can know that it's button-less, yes.  But, how can it
know whether the device should be emulated via ClickZone?
We can use a driver option to x11 synaptics driver for that, as I
already sent you another patch.  However, the driver option is
nowadays not preferred because xorg.conf is being dead on new
systems...

Or maybe HAL (or whatever upcoming one) can check the vendor/product
id of the machine (not the device) to provide the information.  OTOH
this will also need frequent updates.


Takashi

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

On Wed, Dec 16, 2009 at 07:50:54AM +0100, Takashi Iwai wrote:
> At Tue, 15 Dec 2009 18:59:34 -0800,
> Dmitry Torokhov wrote:
> > 
> > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > >
> > > > The updated patch is below.
> > > > 
> > > > -- 
> > > > Dmitry
> > > 
> > > Should I test this one or wait one more iteration to address
> > > Takashi's last comments?
> > > 
> > 
> > Actually I think we took the wrong direction with the original patch and
> > we should do what other buttonless devices (bcm5974) do: report touchpad
> > click as left button and have Synaptics X driver provide enhanced
> > support. This way we can have both modes (ClickZones and ClickButtons)
> > and users will get to chose (provided that someone takes time to add
> > that support to Synaptics driver of course ;) ).
> 
> My concern is, still, how would you identify this device.  Will you
> extend also some ioctls to expose caps and extcaps?  Otherwise it's
> difficult to identify this device automatically from the user-space.
> 

No.. Synaptics without right button == ClickPad.

> The user-space can know that it's button-less, yes.  But, how can it
> know whether the device should be emulated via ClickZone?
> We can use a driver option to x11 synaptics driver for that, as I
> already sent you another patch.  However, the driver option is
> nowadays not preferred because xorg.conf is being dead on new
> systems...

Driver still takes options, from UDEV/HAL. We could pick one behavior
by default and ovverride, by box vendor/model (DMI).

> 
> Or maybe HAL (or whatever upcoming one) can check the vendor/product
> id of the machine (not the device) to provide the information.  OTOH
> this will also need frequent updates.

Hopefully vendors won;t be flip/flopping between ClickZone and
ClickButtons too much. Still option is better than hardcoding ClickZone
for everyone.

-- 
Dmitry

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Tue, 15 Dec 2009 22:56:53 -0800,
Dmitry Torokhov wrote:
> 
> On Wed, Dec 16, 2009 at 07:50:54AM +0100, Takashi Iwai wrote:
> > At Tue, 15 Dec 2009 18:59:34 -0800,
> > Dmitry Torokhov wrote:
> > > 
> > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > >
> > > > > The updated patch is below.
> > > > > 
> > > > > -- 
> > > > > Dmitry
> > > > 
> > > > Should I test this one or wait one more iteration to address
> > > > Takashi's last comments?
> > > > 
> > > 
> > > Actually I think we took the wrong direction with the original patch and
> > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > click as left button and have Synaptics X driver provide enhanced
> > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > and users will get to chose (provided that someone takes time to add
> > > that support to Synaptics driver of course ;) ).
> > 
> > My concern is, still, how would you identify this device.  Will you
> > extend also some ioctls to expose caps and extcaps?  Otherwise it's
> > difficult to identify this device automatically from the user-space.
> > 
> 
> No.. Synaptics without right button == ClickPad.

So, is there only Clickpad device that has a single button?
No other option?

> > The user-space can know that it's button-less, yes.  But, how can it
> > know whether the device should be emulated via ClickZone?
> > We can use a driver option to x11 synaptics driver for that, as I
> > already sent you another patch.  However, the driver option is
> > nowadays not preferred because xorg.conf is being dead on new
> > systems...
> 
> Driver still takes options, from UDEV/HAL. We could pick one behavior
> by default and ovverride, by box vendor/model (DMI).
> 
> > 
> > Or maybe HAL (or whatever upcoming one) can check the vendor/product
> > id of the machine (not the device) to provide the information.  OTOH
> > this will also need frequent updates.
> 
> Hopefully vendors won;t be flip/flopping between ClickZone and
> ClickButtons too much. Still option is better than hardcoding ClickZone
> for everyone.

Yeah, I agree that hard-coding isn't good, and that's why I first
posted separated patches.  OTOH, the kernel-side hack makes the device
working *as is* even without changing anything else.


Takashi

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

On Wed, Dec 16, 2009 at 08:14:15AM +0100, Takashi Iwai wrote:
> At Tue, 15 Dec 2009 22:56:53 -0800,
> Dmitry Torokhov wrote:
> > 
> > On Wed, Dec 16, 2009 at 07:50:54AM +0100, Takashi Iwai wrote:
> > > At Tue, 15 Dec 2009 18:59:34 -0800,
> > > Dmitry Torokhov wrote:
> > > > 
> > > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > > >
> > > > > > The updated patch is below.
> > > > > > 
> > > > > > -- 
> > > > > > Dmitry
> > > > > 
> > > > > Should I test this one or wait one more iteration to address
> > > > > Takashi's last comments?
> > > > > 
> > > > 
> > > > Actually I think we took the wrong direction with the original patch and
> > > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > > click as left button and have Synaptics X driver provide enhanced
> > > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > > and users will get to chose (provided that someone takes time to add
> > > > that support to Synaptics driver of course ;) ).
> > > 
> > > My concern is, still, how would you identify this device.  Will you
> > > extend also some ioctls to expose caps and extcaps?  Otherwise it's
> > > difficult to identify this device automatically from the user-space.
> > > 
> > 
> > No.. Synaptics without right button == ClickPad.
> 
> So, is there only Clickpad device that has a single button?
> No other option?
> 

I have not seen any other Synaptics devices using less than 2 buttons.

> > > The user-space can know that it's button-less, yes.  But, how can it
> > > know whether the device should be emulated via ClickZone?
> > > We can use a driver option to x11 synaptics driver for that, as I
> > > already sent you another patch.  However, the driver option is
> > > nowadays not preferred because xorg.conf is being dead on new
> > > systems...
> > 
> > Driver still takes options, from UDEV/HAL. We could pick one behavior
> > by default and ovverride, by box vendor/model (DMI).
> > 
> > > 
> > > Or maybe HAL (or whatever upcoming one) can check the vendor/product
> > > id of the machine (not the device) to provide the information.  OTOH
> > > this will also need frequent updates.
> > 
> > Hopefully vendors won;t be flip/flopping between ClickZone and
> > ClickButtons too much. Still option is better than hardcoding ClickZone
> > for everyone.
> 
> Yeah, I agree that hard-coding isn't good, and that's why I first
> posted separated patches.  OTOH, the kernel-side hack makes the device
> working *as is* even without changing anything else.
>

For "ClickZone" - yes, but not all users would want this I guess and not
all laptops will have that zone marked. Pushing it off to userspace
gives more flexibility, including the ability to change zone size, etc.

-- 
Dmitry

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Wed, 16 Dec 2009 00:23:16 -0800,
Dmitry Torokhov wrote:
> 
> On Wed, Dec 16, 2009 at 08:14:15AM +0100, Takashi Iwai wrote:
> > At Tue, 15 Dec 2009 22:56:53 -0800,
> > Dmitry Torokhov wrote:
> > > 
> > > On Wed, Dec 16, 2009 at 07:50:54AM +0100, Takashi Iwai wrote:
> > > > At Tue, 15 Dec 2009 18:59:34 -0800,
> > > > Dmitry Torokhov wrote:
> > > > > 
> > > > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > > > >
> > > > > > > The updated patch is below.
> > > > > > > 
> > > > > > > -- 
> > > > > > > Dmitry
> > > > > > 
> > > > > > Should I test this one or wait one more iteration to address
> > > > > > Takashi's last comments?
> > > > > > 
> > > > > 
> > > > > Actually I think we took the wrong direction with the original patch and
> > > > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > > > click as left button and have Synaptics X driver provide enhanced
> > > > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > > > and users will get to chose (provided that someone takes time to add
> > > > > that support to Synaptics driver of course ;) ).
> > > > 
> > > > My concern is, still, how would you identify this device.  Will you
> > > > extend also some ioctls to expose caps and extcaps?  Otherwise it's
> > > > difficult to identify this device automatically from the user-space.
> > > > 
> > > 
> > > No.. Synaptics without right button == ClickPad.
> > 
> > So, is there only Clickpad device that has a single button?
> > No other option?
> > 
> 
> I have not seen any other Synaptics devices using less than 2 buttons.
> 
> > > > The user-space can know that it's button-less, yes.  But, how can it
> > > > know whether the device should be emulated via ClickZone?
> > > > We can use a driver option to x11 synaptics driver for that, as I
> > > > already sent you another patch.  However, the driver option is
> > > > nowadays not preferred because xorg.conf is being dead on new
> > > > systems...
> > > 
> > > Driver still takes options, from UDEV/HAL. We could pick one behavior
> > > by default and ovverride, by box vendor/model (DMI).
> > > 
> > > > 
> > > > Or maybe HAL (or whatever upcoming one) can check the vendor/product
> > > > id of the machine (not the device) to provide the information.  OTOH
> > > > this will also need frequent updates.
> > > 
> > > Hopefully vendors won;t be flip/flopping between ClickZone and
> > > ClickButtons too much. Still option is better than hardcoding ClickZone
> > > for everyone.
> > 
> > Yeah, I agree that hard-coding isn't good, and that's why I first
> > posted separated patches.  OTOH, the kernel-side hack makes the device
> > working *as is* even without changing anything else.
> >
> 
> For "ClickZone" - yes, but not all users would want this I guess and not
> all laptops will have that zone marked. Pushing it off to userspace
> gives more flexibility, including the ability to change zone size, etc.

As mentioned, I agree basically for the user-space implementation for
a long term solution.  My point is that the kernel hack can be
regarded as a quick hack for a short term while you have no patch yet
for the user-space driver.  It'll take time until the user-space
update will be deployed in many distros while people tend to update
only the kernel.


Takashi

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

On Wed, Dec 16, 2009 at 10:17:39AM +0100, Takashi Iwai wrote:
> At Wed, 16 Dec 2009 00:23:16 -0800,
> Dmitry Torokhov wrote:
> > 
> > On Wed, Dec 16, 2009 at 08:14:15AM +0100, Takashi Iwai wrote:
> > > At Tue, 15 Dec 2009 22:56:53 -0800,
> > > Dmitry Torokhov wrote:
> > > > 
> > > > On Wed, Dec 16, 2009 at 07:50:54AM +0100, Takashi Iwai wrote:
> > > > > At Tue, 15 Dec 2009 18:59:34 -0800,
> > > > > Dmitry Torokhov wrote:
> > > > > > 
> > > > > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > > > > >
> > > > > > > > The updated patch is below.
> > > > > > > > 
> > > > > > > > -- 
> > > > > > > > Dmitry
> > > > > > > 
> > > > > > > Should I test this one or wait one more iteration to address
> > > > > > > Takashi's last comments?
> > > > > > > 
> > > > > > 
> > > > > > Actually I think we took the wrong direction with the original patch and
> > > > > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > > > > click as left button and have Synaptics X driver provide enhanced
> > > > > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > > > > and users will get to chose (provided that someone takes time to add
> > > > > > that support to Synaptics driver of course ;) ).
> > > > > 
> > > > > My concern is, still, how would you identify this device.  Will you
> > > > > extend also some ioctls to expose caps and extcaps?  Otherwise it's
> > > > > difficult to identify this device automatically from the user-space.
> > > > > 
> > > > 
> > > > No.. Synaptics without right button == ClickPad.
> > > 
> > > So, is there only Clickpad device that has a single button?
> > > No other option?
> > > 
> > 
> > I have not seen any other Synaptics devices using less than 2 buttons.
> > 
> > > > > The user-space can know that it's button-less, yes.  But, how can it
> > > > > know whether the device should be emulated via ClickZone?
> > > > > We can use a driver option to x11 synaptics driver for that, as I
> > > > > already sent you another patch.  However, the driver option is
> > > > > nowadays not preferred because xorg.conf is being dead on new
> > > > > systems...
> > > > 
> > > > Driver still takes options, from UDEV/HAL. We could pick one behavior
> > > > by default and ovverride, by box vendor/model (DMI).
> > > > 
> > > > > 
> > > > > Or maybe HAL (or whatever upcoming one) can check the vendor/product
> > > > > id of the machine (not the device) to provide the information.  OTOH
> > > > > this will also need frequent updates.
> > > > 
> > > > Hopefully vendors won;t be flip/flopping between ClickZone and
> > > > ClickButtons too much. Still option is better than hardcoding ClickZone
> > > > for everyone.
> > > 
> > > Yeah, I agree that hard-coding isn't good, and that's why I first
> > > posted separated patches.  OTOH, the kernel-side hack makes the device
> > > working *as is* even without changing anything else.
> > >
> > 
> > For "ClickZone" - yes, but not all users would want this I guess and not
> > all laptops will have that zone marked. Pushing it off to userspace
> > gives more flexibility, including the ability to change zone size, etc.
> 
> As mentioned, I agree basically for the user-space implementation for
> a long term solution.  My point is that the kernel hack can be
> regarded as a quick hack for a short term while you have no patch yet
> for the user-space driver.  It'll take time until the user-space
> update will be deployed in many distros while people tend to update
> only the kernel.

I strongly disagree here. The fact that some users are more likely to
update kernel than userspace bits is immaterial, besides such users can
easily patch their kernels with out-of-tree patches. Additionally kernel
patch would prevent writing proper userspace support because userspace
will not be able to reconstruct full device state (you are filtering out
some events).

-- 
Dmitry

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Wed, 16 Dec 2009 09:24:45 -0800,
Dmitry Torokhov wrote:
> 
> On Wed, Dec 16, 2009 at 10:17:39AM +0100, Takashi Iwai wrote:
> > At Wed, 16 Dec 2009 00:23:16 -0800,
> > Dmitry Torokhov wrote:
> > > 
> > > On Wed, Dec 16, 2009 at 08:14:15AM +0100, Takashi Iwai wrote:
> > > > At Tue, 15 Dec 2009 22:56:53 -0800,
> > > > Dmitry Torokhov wrote:
> > > > > 
> > > > > On Wed, Dec 16, 2009 at 07:50:54AM +0100, Takashi Iwai wrote:
> > > > > > At Tue, 15 Dec 2009 18:59:34 -0800,
> > > > > > Dmitry Torokhov wrote:
> > > > > > > 
> > > > > > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > > > > > >
> > > > > > > > > The updated patch is below.
> > > > > > > > > 
> > > > > > > > > -- 
> > > > > > > > > Dmitry
> > > > > > > > 
> > > > > > > > Should I test this one or wait one more iteration to address
> > > > > > > > Takashi's last comments?
> > > > > > > > 
> > > > > > > 
> > > > > > > Actually I think we took the wrong direction with the original patch and
> > > > > > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > > > > > click as left button and have Synaptics X driver provide enhanced
> > > > > > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > > > > > and users will get to chose (provided that someone takes time to add
> > > > > > > that support to Synaptics driver of course ;) ).
> > > > > > 
> > > > > > My concern is, still, how would you identify this device.  Will you
> > > > > > extend also some ioctls to expose caps and extcaps?  Otherwise it's
> > > > > > difficult to identify this device automatically from the user-space.
> > > > > > 
> > > > > 
> > > > > No.. Synaptics without right button == ClickPad.
> > > > 
> > > > So, is there only Clickpad device that has a single button?
> > > > No other option?
> > > > 
> > > 
> > > I have not seen any other Synaptics devices using less than 2 buttons.
> > > 
> > > > > > The user-space can know that it's button-less, yes.  But, how can it
> > > > > > know whether the device should be emulated via ClickZone?
> > > > > > We can use a driver option to x11 synaptics driver for that, as I
> > > > > > already sent you another patch.  However, the driver option is
> > > > > > nowadays not preferred because xorg.conf is being dead on new
> > > > > > systems...
> > > > > 
> > > > > Driver still takes options, from UDEV/HAL. We could pick one behavior
> > > > > by default and ovverride, by box vendor/model (DMI).
> > > > > 
> > > > > > 
> > > > > > Or maybe HAL (or whatever upcoming one) can check the vendor/product
> > > > > > id of the machine (not the device) to provide the information.  OTOH
> > > > > > this will also need frequent updates.
> > > > > 
> > > > > Hopefully vendors won;t be flip/flopping between ClickZone and
> > > > > ClickButtons too much. Still option is better than hardcoding ClickZone
> > > > > for everyone.
> > > > 
> > > > Yeah, I agree that hard-coding isn't good, and that's why I first
> > > > posted separated patches.  OTOH, the kernel-side hack makes the device
> > > > working *as is* even without changing anything else.
> > > >
> > > 
> > > For "ClickZone" - yes, but not all users would want this I guess and not
> > > all laptops will have that zone marked. Pushing it off to userspace
> > > gives more flexibility, including the ability to change zone size, etc.
> > 
> > As mentioned, I agree basically for the user-space implementation for
> > a long term solution.  My point is that the kernel hack can be
> > regarded as a quick hack for a short term while you have no patch yet
> > for the user-space driver.  It'll take time until the user-space
> > update will be deployed in many distros while people tend to update
> > only the kernel.
> 
> I strongly disagree here. The fact that some users are more likely to
> update kernel than userspace bits is immaterial, besides such users can
> easily patch their kernels with out-of-tree patches. Additionally kernel
> patch would prevent writing proper userspace support because userspace
> will not be able to reconstruct full device state (you are filtering out
> some events).

Dmitry, as I clearly wrote a couple of times, I *DO* agree with the
user-space solution.  But you have no that solution yet ready, right?
That's why I mentioned about the kernel patch, for a poor man who
can't use the device properly yet.  If you can get things ready for
use, there can't be any complain.

So, honestly, I don't care which approach you take.  But I just hope
it'll be finished quickly enough :)


thanks,

Takashi

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Wed, 16 Dec 2009 07:50:54 +0100,
I wrote:
> 
> At Tue, 15 Dec 2009 18:59:34 -0800,
> Dmitry Torokhov wrote:
> > 
> > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > >
> > > > The updated patch is below.
> > > > 
> > > > -- 
> > > > Dmitry
> > > 
> > > Should I test this one or wait one more iteration to address
> > > Takashi's last comments?
> > > 
> > 
> > Actually I think we took the wrong direction with the original patch and
> > we should do what other buttonless devices (bcm5974) do: report touchpad
> > click as left button and have Synaptics X driver provide enhanced
> > support. This way we can have both modes (ClickZones and ClickButtons)
> > and users will get to chose (provided that someone takes time to add
> > that support to Synaptics driver of course ;) ).
> 
> My concern is, still, how would you identify this device.  Will you
> extend also some ioctls to expose caps and extcaps?  Otherwise it's
> difficult to identify this device automatically from the user-space.
> 
> The user-space can know that it's button-less, yes.  But, how can it
> know whether the device should be emulated via ClickZone?
> We can use a driver option to x11 synaptics driver for that, as I
> already sent you another patch.  However, the driver option is
> nowadays not preferred because xorg.conf is being dead on new
> systems...

FWIW, the patch below is to add clickpad feature to x11 synaptics driver
1.1.3.  It's almost straightforward conversion from my kernel patch.
(The vertical coordinate had to be flipped, though.)


Takashi

---
---
 include/synaptics-properties.h |    3 ++
 include/synaptics.h            |    1 
 man/synaptics.man              |    5 ++++
 src/synaptics.c                |   42 +++++++++++++++++++++++++++++++++++++++++
 src/synapticsstr.h             |    1 
 tools/synclient.c              |    2 +
 6 files changed, 54 insertions(+)

--- a/include/synaptics.h
+++ b/include/synaptics.h
@@ -134,6 +134,7 @@
     double press_motion_min_factor;	    /* factor applied on speed when finger pressure is at minimum */
     double press_motion_max_factor; 	    /* factor applied on speed when finger pressure is at minimum */
     Bool grab_event_device;		    /* grab event device for exclusive use? */
+    Bool clickpad;			    /* clickpad mode */
 } SynapticsSHM;
 
 /*
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -499,6 +499,7 @@
     pars->press_motion_min_factor = xf86SetRealOption(opts, "PressureMotionMinFactor", 1.0);
     pars->press_motion_max_factor = xf86SetRealOption(opts, "PressureMotionMaxFactor", 1.0);
     pars->grab_event_device = xf86SetBoolOption(opts, "GrabEventDevice", TRUE);
+    pars->clickpad = xf86SetBoolOption(opts, "Clickpad", FALSE);
 
     /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge parameters */
     if (pars->top_edge > pars->bottom_edge) {
@@ -1902,6 +1903,44 @@
     }
 }
 
+/* left and right clickpad button ranges;
+ * the gap between them is interpreted as a middle-button click
+ */
+#define CLICKPAD_LEFT_BTN_X(p)				\
+    (((p)->maxx - (p)->minx) * 2 / 5 + (p)->minx)
+#define CLICKPAD_RIGHT_BTN_X(p)				\
+    (((p)->maxx - (p)->minx) * 3 / 5 + (p)->minx)
+
+static void
+MangleClickpad(SynapticsPrivate *priv, struct SynapticsHwState *hw)
+{
+    SynapticsSHM *para = priv->synpara;
+
+    /* clickpad mode reports Y over YMAX as a button area */
+    if (hw->y > priv->maxy) {
+	/* clickpad reports only the middle button, and we need
+	 * to fake left/right buttons depending on the touch position
+	 */
+	if (hw->left || hw->middle || hw->right) { /* clicked? */
+	    hw->left = hw->middle = hw->right = 0;
+	    if (hw->x < CLICKPAD_LEFT_BTN_X(priv));
+		hw->left = 1;
+	    else if (hw->x > CLICKPAD_RIGHT_BTN_X(priv))
+		hw->right = 1;
+	    else
+		hw->middle = 1;
+	}
+	/* don't move pointer in the button area */
+	hw->x = para->x;
+	hw->y = para->y;
+	hw->z = 0;
+    } else if (hw->left || hw->middle || hw->right) {
+	/* dragging */
+	hw->left = para->left;
+	hw->right = para->right;
+	hw->middle = para->middle;
+    }
+}
 
 /*
  * React on changes in the hardware state. This function is called every time
@@ -1924,6 +1963,9 @@
     int timeleft;
     int i;
 
+    if (para->clickpad)
+	MangleClickpad(priv, hw);
+
     /* update hardware state in shared memory */
     para->x = hw->x;
     para->y = hw->y;
--- a/src/synapticsstr.h
+++ b/src/synapticsstr.h
@@ -154,6 +154,7 @@
     Bool has_pressure;			/* device reports pressure */
 
     enum TouchpadModel model;          /* The detected model */
+    Bool clickpad;			/* clickpad mode */
 } SynapticsPrivate;
 
 #endif /* _SYNAPTICSSTR_H_ */
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -411,6 +411,11 @@
 .
 This can be achieved by switching to a text console and then switching
 back to X.
+.TP 7
+.BI "Option \*qClickpad\*q \*q" boolean \*q
+Supports Clickpad.  The driver handles a middle-button click in the
+special low area as left/right/middle buttons according to the position.
+This mode works only with evdev protocol.
 .
 .
 .LP
--- a/tools/synclient.c
+++ b/tools/synclient.c
@@ -209,6 +209,8 @@
 		SYNAPTICS_PROP_PRESSURE_MOTION_FACTOR,	0 /*float*/,	1),
     DEFINE_PAR("GrabEventDevice",      grab_event_device,       PT_BOOL,   0, 1,
 		SYNAPTICS_PROP_GRAB,	8,	0),
+    DEFINE_PAR("Clickpad",             clickpad,                PT_BOOL,   0, 1,
+		SYNAPTICS_PROP_CLICKPAD,	8,	0),
     { NULL, 0, 0, 0, 0 }
 };
 
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -142,4 +142,7 @@
 /* 8 bit (BOOL) */
 #define SYNAPTICS_PROP_GRAB "Synaptics Grab Event Device"
 
+/* 8 bit (BOOL) */
+#define SYNAPTICS_PROP_CLICKPAD "Synaptics Clickpad Mode"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */

Re: synaptics touchpad doesn't click

[Alex Chiang]

* Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > >
> > > The updated patch is below.
> > > 
> > > -- 
> > > Dmitry
> > 
> > Should I test this one or wait one more iteration to address
> > Takashi's last comments?
> > 
> 
> Actually I think we took the wrong direction with the original patch and
> we should do what other buttonless devices (bcm5974) do: report touchpad
> click as left button and have Synaptics X driver provide enhanced
> support. This way we can have both modes (ClickZones and ClickButtons)
> and users will get to chose (provided that someone takes time to add
> that support to Synaptics driver of course ;) ).
> 
> Could you tell me if the below works for you?

Left clicks work. Right/middle do not (as expected, I guess).

I liked the behaviour of Takashi's patch a little better wrt what
happens when you try to click/drag, because he disabled movement
in the click area.

My vodka consumption is minimal and I don't have the shakes, but
I found it difficult to precisely hit the edge of a window and
click / drag to resize...

/ac

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

On Tue, Dec 15, 2009 at 11:52:00PM -0700, Alex Chiang wrote:
> * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > >
> > > > The updated patch is below.
> > > > 
> > > > -- 
> > > > Dmitry
> > > 
> > > Should I test this one or wait one more iteration to address
> > > Takashi's last comments?
> > > 
> > 
> > Actually I think we took the wrong direction with the original patch and
> > we should do what other buttonless devices (bcm5974) do: report touchpad
> > click as left button and have Synaptics X driver provide enhanced
> > support. This way we can have both modes (ClickZones and ClickButtons)
> > and users will get to chose (provided that someone takes time to add
> > that support to Synaptics driver of course ;) ).
> > 
> > Could you tell me if the below works for you?
> 
> Left clicks work. Right/middle do not (as expected, I guess).
> 
> I liked the behaviour of Takashi's patch a little better wrt what
> happens when you try to click/drag, because he disabled movement
> in the click area.

Are the clicks register only in that special area? Reading Synaptics
product description it sounds like entire surface should be clickable.

> My vodka consumption is minimal and I don't have the shakes, but
> I found it difficult to precisely hit the edge of a window and
> click / drag to resize...
> 

-- 
Dmitry

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Tue, 15 Dec 2009 23:03:43 -0800,
Dmitry Torokhov wrote:
> 
> On Tue, Dec 15, 2009 at 11:52:00PM -0700, Alex Chiang wrote:
> > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > >
> > > > > The updated patch is below.
> > > > > 
> > > > > -- 
> > > > > Dmitry
> > > > 
> > > > Should I test this one or wait one more iteration to address
> > > > Takashi's last comments?
> > > > 
> > > 
> > > Actually I think we took the wrong direction with the original patch and
> > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > click as left button and have Synaptics X driver provide enhanced
> > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > and users will get to chose (provided that someone takes time to add
> > > that support to Synaptics driver of course ;) ).
> > > 
> > > Could you tell me if the below works for you?
> > 
> > Left clicks work. Right/middle do not (as expected, I guess).
> > 
> > I liked the behaviour of Takashi's patch a little better wrt what
> > happens when you try to click/drag, because he disabled movement
> > in the click area.
> 
> Are the clicks register only in that special area? Reading Synaptics
> product description it sounds like entire surface should be clickable.

In the case of ClickZone, the click in the button area is taken as
clicks, if I understand correctly.

The problem is, however, not about where to click.  It's about the
precision.  When you click a pad, your finger slips a little bit in
millimeter.  This already corresponds to a few pixels, and the pointer
moves away from the point you wanted to click.

In my original patch, I disabled the pointer movement in the button
area before click for avoiding this problem.


Takashi

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

On Wed, Dec 16, 2009 at 08:11:05AM +0100, Takashi Iwai wrote:
> At Tue, 15 Dec 2009 23:03:43 -0800,
> Dmitry Torokhov wrote:
> > 
> > On Tue, Dec 15, 2009 at 11:52:00PM -0700, Alex Chiang wrote:
> > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > > >
> > > > > > The updated patch is below.
> > > > > > 
> > > > > > -- 
> > > > > > Dmitry
> > > > > 
> > > > > Should I test this one or wait one more iteration to address
> > > > > Takashi's last comments?
> > > > > 
> > > > 
> > > > Actually I think we took the wrong direction with the original patch and
> > > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > > click as left button and have Synaptics X driver provide enhanced
> > > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > > and users will get to chose (provided that someone takes time to add
> > > > that support to Synaptics driver of course ;) ).
> > > > 
> > > > Could you tell me if the below works for you?
> > > 
> > > Left clicks work. Right/middle do not (as expected, I guess).
> > > 
> > > I liked the behaviour of Takashi's patch a little better wrt what
> > > happens when you try to click/drag, because he disabled movement
> > > in the click area.
> > 
> > Are the clicks register only in that special area? Reading Synaptics
> > product description it sounds like entire surface should be clickable.
> 
> In the case of ClickZone, the click in the button area is taken as
> clicks, if I understand correctly.
> 
> The problem is, however, not about where to click.  It's about the
> precision.  When you click a pad, your finger slips a little bit in
> millimeter.  This already corresponds to a few pixels, and the pointer
> moves away from the point you wanted to click.
>

Right, this will have to be dealt with in X driver.

> In my original patch, I disabled the pointer movement in the button
> area before click for avoiding this problem.
>

However it will not work if you want to do 'ClickButtons' mode where
you don't assign secific areas for clicking.

-- 
Dmitry

Re: synaptics touchpad doesn't click

[Takashi Iwai]

At Wed, 16 Dec 2009 00:20:20 -0800,
Dmitry Torokhov wrote:
> 
> On Wed, Dec 16, 2009 at 08:11:05AM +0100, Takashi Iwai wrote:
> > At Tue, 15 Dec 2009 23:03:43 -0800,
> > Dmitry Torokhov wrote:
> > > 
> > > On Tue, Dec 15, 2009 at 11:52:00PM -0700, Alex Chiang wrote:
> > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > > > >
> > > > > > > The updated patch is below.
> > > > > > > 
> > > > > > > -- 
> > > > > > > Dmitry
> > > > > > 
> > > > > > Should I test this one or wait one more iteration to address
> > > > > > Takashi's last comments?
> > > > > > 
> > > > > 
> > > > > Actually I think we took the wrong direction with the original patch and
> > > > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > > > click as left button and have Synaptics X driver provide enhanced
> > > > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > > > and users will get to chose (provided that someone takes time to add
> > > > > that support to Synaptics driver of course ;) ).
> > > > > 
> > > > > Could you tell me if the below works for you?
> > > > 
> > > > Left clicks work. Right/middle do not (as expected, I guess).
> > > > 
> > > > I liked the behaviour of Takashi's patch a little better wrt what
> > > > happens when you try to click/drag, because he disabled movement
> > > > in the click area.
> > > 
> > > Are the clicks register only in that special area? Reading Synaptics
> > > product description it sounds like entire surface should be clickable.
> > 
> > In the case of ClickZone, the click in the button area is taken as
> > clicks, if I understand correctly.
> > 
> > The problem is, however, not about where to click.  It's about the
> > precision.  When you click a pad, your finger slips a little bit in
> > millimeter.  This already corresponds to a few pixels, and the pointer
> > moves away from the point you wanted to click.
> >
> 
> Right, this will have to be dealt with in X driver.
> 
> > In my original patch, I disabled the pointer movement in the button
> > area before click for avoiding this problem.
> >
> 
> However it will not work if you want to do 'ClickButtons' mode where
> you don't assign secific areas for clicking.

Yes.  And the area is more or less fixed.


Takashi

Re: synaptics touchpad doesn't click

[Alex Chiang]

* Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> On Tue, Dec 15, 2009 at 11:52:00PM -0700, Alex Chiang wrote:
> > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > >
> > > > > The updated patch is below.
> > > > > 
> > > > > -- 
> > > > > Dmitry
> > > > 
> > > > Should I test this one or wait one more iteration to address
> > > > Takashi's last comments?
> > > > 
> > > 
> > > Actually I think we took the wrong direction with the original patch and
> > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > click as left button and have Synaptics X driver provide enhanced
> > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > and users will get to chose (provided that someone takes time to add
> > > that support to Synaptics driver of course ;) ).
> > > 
> > > Could you tell me if the below works for you?
> > 
> > Left clicks work. Right/middle do not (as expected, I guess).
> > 
> > I liked the behaviour of Takashi's patch a little better wrt what
> > happens when you try to click/drag, because he disabled movement
> > in the click area.
> 
> Are the clicks register only in that special area? Reading Synaptics
> product description it sounds like entire surface should be clickable.

Well, maybe in theory the entire surface is clickable.

My observation is that the physical design resembles a lever on a
fulcrum though, with the fulcrum at the edge near the keyboard.

Pressing the click zone is mechanically nice, since you are away
from the fulcrum. Moving "up" towards the keyboard, there seems
to be much less mechanical advantage, and the surface doesn't
seem so clicky.

But really, I was talking about what Takashi mentioned in a later
mail, namely that trying to actually click with the click zone in
a precise manner is difficult because as you press the surface
down, the touch pad registers some movement, and moves the
pointer away from the area you're trying to click.

Fitts would have a fit. ;)

/ac

Re: synaptics touchpad doesn't click

[Dmitry Torokhov]

On Wed, Dec 16, 2009 at 10:31:59AM -0700, Alex Chiang wrote:
> * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > On Tue, Dec 15, 2009 at 11:52:00PM -0700, Alex Chiang wrote:
> > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > On Tue, Dec 15, 2009 at 06:05:06PM -0700, Alex Chiang wrote:
> > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > > > > >
> > > > > > The updated patch is below.
> > > > > > 
> > > > > > -- 
> > > > > > Dmitry
> > > > > 
> > > > > Should I test this one or wait one more iteration to address
> > > > > Takashi's last comments?
> > > > > 
> > > > 
> > > > Actually I think we took the wrong direction with the original patch and
> > > > we should do what other buttonless devices (bcm5974) do: report touchpad
> > > > click as left button and have Synaptics X driver provide enhanced
> > > > support. This way we can have both modes (ClickZones and ClickButtons)
> > > > and users will get to chose (provided that someone takes time to add
> > > > that support to Synaptics driver of course ;) ).
> > > > 
> > > > Could you tell me if the below works for you?
> > > 
> > > Left clicks work. Right/middle do not (as expected, I guess).
> > > 
> > > I liked the behaviour of Takashi's patch a little better wrt what
> > > happens when you try to click/drag, because he disabled movement
> > > in the click area.
> > 
> > Are the clicks register only in that special area? Reading Synaptics
> > product description it sounds like entire surface should be clickable.
> 
> Well, maybe in theory the entire surface is clickable.
> 
> My observation is that the physical design resembles a lever on a
> fulcrum though, with the fulcrum at the edge near the keyboard.
> 
> Pressing the click zone is mechanically nice, since you are away
> from the fulcrum. Moving "up" towards the keyboard, there seems
> to be much less mechanical advantage, and the surface doesn't
> seem so clicky.
> 

I am going off the following:

	http://www.synaptics.com/solutions/products/clickpad

I am no sure whether there are 2 different flavors or if they are all
the same and the only difference is whether the buttons are marked or
not.

> But really, I was talking about what Takashi mentioned in a later
> mail, namely that trying to actually click with the click zone in
> a precise manner is difficult because as you press the surface
> down, the touch pad registers some movement, and moves the
> pointer away from the area you're trying to click.
> 
> Fitts would have a fit. ;)

That will have to be handled in synaptics X driver.

-- 
Dmitry