Re: Synaptics touchpad misdetected as clickpad [with patch]

[christos%astron.com@localhost (Christos Zoulas)]

In article <20150304214425.GI20372%falu.nl@localhost>, Rhialto  <rhialto%falu.nl@localhost> wrote:
>-=-=-=-=-=-
>
>On Tue 03 Mar 2015 at 02:30:47 +0000, Christos Zoulas wrote:
>> In article <20150303004630.GH6733%falu.nl@localhost>, Rhialto  <rhialto%falu.nl@localhost> wrote:
>> >-=-=-=-=-=-
>> >
>> >and no click pad. Which is isn't. It has 2 separate buttons below the
>> >touchpad. Attaching an usb mouse serves as a workaround.
>> >
>> >Any other info I can give?
>> 
>> You can turn debugging on and it will print more info...
>
>Ok, I enabled SYNAPTICSDEBUG and booted with -x.
>The kernel messages are now like so:
>
>
>pms0 at pckbc1 (aux slot)
>pms0: Synaptics touchpad version 6.5
>pms0: synaptics_probe: Capabilities 0xa051.
>pms0: pms_synaptics_probe_extended: Extended Buttons: 0.
>pms0: pms_synaptics_probe_extended: Extended Capabilities: 0xa0 0x00 0x00.
>pms0: pms_synaptics_probe_extended: Continued Capabilities 0x0f 0xf1 0x40.
>pms0: Palm detect, One button click pad
>
>Looking at the code which prints this, in
>src/sys/dev/pckbport/synaptics.c, function
>pms_synaptics_probe_extended(),
>
> 191         /* Ask about click pad */
> 192         if (((sc->caps & SYNAPTICS_CAP_EXTNUM) + 0x08) >=
> 193             SYNAPTICS_CONTINUED_CAPABILITIES)
> 194         {
> 195                 res = pms_sliced_command(psc->sc_kbctag,
> 196                     psc->sc_kbcslot, SYNAPTICS_CONTINUED_CAPABILITIES);
> 197                 cmd[0] = PMS_SEND_DEV_STATUS;
> 198                 res |= pckbport_poll_cmd(psc->sc_kbctag,
> 199                     psc->sc_kbcslot, cmd, 1, 3, resp, 0);
> 200                 if (res == 0) {
> 201                         u_char clickpad_type = (resp[1] & 0x1);
> 202                         clickpad_type |= ((resp[0] >> 4) & 0x1);
> 203 
> 204                         aprint_debug_dev(psc->sc_dev, "%s: Continued "
> 205                             "Capabilities 0x%02x 0x%02x 0x%02x.\n",
>__func_     _,
> 206                             resp[0], resp[1], resp[2]);
> 207                         switch (clickpad_type) {
> 208                         case 1:
> 209                                 sc->flags |=
>SYN_FLAG_HAS_ONE_BUTTON_CLICKP     AD;
> 210                                 break;
> 211                         case 2:
> 212                                 sc->flags |=
>SYN_FLAG_HAS_TWO_BUTTON_CLICKP     AD;
> 213                                 break;
> 214                         default:
> 215                                 break;
> 216                         }
> 217                 }
> 218         }
>
>I have to wonder how case 2 can ever be reached, given the
>initialisation of clickpad_type. This smells like there is something
>incorrect here with clickpad_type.
>
>If I look at what Linux does, in 
>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/input/mouse/synaptics.h?id=refs/tags/v4.0-rc2
>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/input/mouse/synaptics.c?id=refs/tags/v4.0-rc2
>
>then
>
>/*
> * The following describes response for the 0x0c query.
> *
> * byte	mask	name			meaning
> * ----	----	-------			------------
> * 1	0x01	adjustable threshold	capacitive button sensitivity
> *					can be adjusted
> * 1	0x02	report max		query 0x0d gives max coord reported
> * 1	0x04	clearpad		sensor is ClearPad product
> * 1	0x08	advanced gesture	not particularly meaningful
> * 1	0x10	clickpad bit 0		1-button ClickPad
> * 1	0x60	multifinger mode	identifies firmware finger counting
> *					(not reporting!) algorithm.
> *					Not particularly meaningful
> * 1	0x80	covered pad		W clipped to 14, 15 == pad mostly covered
> * 2	0x01	clickpad bit 1		2-button ClickPad
> * 2	0x02	deluxe LED controls	touchpad support LED commands
> *					ala multimedia control bar
> * 2	0x04	reduced filtering	firmware does less filtering on
> *					position data, driver should watch
> *					for noise.
> * 2	0x08	image sensor		image sensor tracks 5 fingers, but only
> *					reports 2.
> * 2	0x01	uniform clickpad	whole clickpad moves instead of being
> *					hinged at the top.
> * 2	0x20	report min		query 0x0f gives min coord reported
> */
>#define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100000) /* 1-button ClickPad */
>#define SYN_CAP_CLICKPAD2BTN(ex0c)	((ex0c) & 0x000100) /* 2-button ClickPad */
>
>and
>
>    if (SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 4) {
>	if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_CAPAB_0C, cap)) {
>	    psmouse_warn(psmouse,
>		     "device claims to have extended capability 0x0c, but I'm not able
>to read it.\n");
>	} else {
>	    priv->ext_cap_0c = (cap[0] << 16) | (cap[1] << 8) | cap[2];
>	}
>    }
>
>I tried this patch:
>
>
>Index: synaptics.c
>===================================================================
>RCS file: /cvsroot/src/sys/dev/pckbport/synaptics.c,v
>retrieving revision 1.32
>diff -u -r1.32 synaptics.c
>--- synaptics.c	23 May 2014 01:11:29 -0000	1.32
>+++ synaptics.c	4 Mar 2015 21:36:05 -0000
>@@ -198,7 +198,7 @@
> 		res |= pckbport_poll_cmd(psc->sc_kbctag,
> 		    psc->sc_kbcslot, cmd, 1, 3, resp, 0);
> 		if (res == 0) {
>-			u_char clickpad_type = (resp[1] & 0x1);
>+			u_char clickpad_type = (resp[1] & 0x1) << 1;
> 			clickpad_type |= ((resp[0] >> 4) & 0x1);
> 
> 			aprint_debug_dev(psc->sc_dev, "%s: Continued "
>
>which make my touchpad into a working "two button clickpad".
>(It always worked before the clickpad detection, so this doesn't seem
>essential)
>
>Given the above bit assignments, I would even propose to use the simpler
>version
>
> 201                         u_char clickpad_type = (resp[1] & 0x01);
> 202                         clickpad_type |=       (resp[0] & 0x10);
>...
> 207                         switch (clickpad_type) {
> 208                         case 0x10:
> 209                                 sc->flags |=
>SYN_FLAG_HAS_ONE_BUTTON_CLICKP     AD;
>...
> 211                         case 0x01:
> 212                                 sc->flags |=
>SYN_FLAG_HAS_TWO_BUTTON_CLICKP     AD;
>...

Sure why don't you send a full patch that also has the above comment
reproduced...

christos