Re: [PATCH] Input: Lenovo S10-3t's touchpad support

Dmitry Torokhov
2010-11-27T07:55:39Z

Hi Yan,

On Sat, Nov 27, 2010 at 11:56:59AM +0800, Yan Li wrote:
> This is for kernel bug #18122 and MeeGo bug #4807.
>
> Current code detects Clickpad by checking the 8 and 20 bits of 0x0c
> cap. However, the code returns true if either of those bits is 1,
> while it should only return true when both are 1. This has lead to the
> touchpad on Lenovo S10-3t be mistakenly recognized as Clickpad and its
> BTN_LEFT and BTN_RIGHT blocked.
>
> So far we've found that the S10-3ts are shipped with two slightly
> different models of touchpads, of which the 0x0c cap is either
> 0x5a0400 or 0x4a0500. They are not Clickpad and return BTN_LEFT and
> BTN_RIGHT normally.

Hmm, this is weird. According to my data:

>> Treat it as a two-bit field.
>> 0x00 == not a clickpad
>> 0x01 == 1 button clickpad
>> 0x02 == 2 button clickpad
>> 0x03 == reserved

Moreover, Takashi's HP returns 0x5a 0x04 0x00 in response to 0x0c query
and _is_ a clickpad.

Christopher, was there any more updates to the protocol by any chance?

>
> This patch fixed this issue by checking both sign bits are 1. Tested
> on my S10-3t and worked well.
>
> Signed-off-by: Yan Li <yan.i.li@intel.com>
> ---
> drivers/input/mouse/synaptics.h | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index 613a365..0c1083c 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -51,7 +51,11 @@
> #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(ex0c) ((ex0c) & 0x100100)
> +/* Synaptics' ClickPad has both 8th and 20th bits set in the 0x0c
> + * cap. Other models (like those shipped with Lenovo S10-3t) may have
> + * either one of them set but not both, and they are *not* ClickPad
> + * although they look similar. */
> +#define SYN_CAP_CLICKPAD(ex0c) ((ex0c) & 0x100100 == 0x100100)

In C comparison operators have higher precedence than bitwise ones. Your
expression reduces to ((ex0c) & 1) which is not correct. The proper
expression would be:

#define SYN_CAP_CLICKPAD(ex0c) (((ex0c) & 0x100100) == 0x100100)

but it really contradicts the data I have...

Thanks.

--
Dmitry