Hi,

here are small patches to add a support of Synaptics Clickpad devices;
more exactly, it adds the way for X11 synaptics driver detecting the
Clickpad device to handle its specific features.

Unlike my previous patch, this version gives little additions in the
kernel side.  It changes the bit-mask of supported buttons and
converts the button event to the left-button.  And, with the second
patch, it adds a capability of LED input event.

The corresponding patches required for xorg synaptics driver will be
posted to xorg ML soon later.  If anyone needs them beforehand, I can
mail them beforehand, of course.


thanks,

Takashi
--

[ message continues ]

The new Synaptics devices have an LED on the top-left corner.
This is controlled via the command 0x0a with parameters 0x88 or 0x10.

The detection of the LED isn't clear yet.  It should have been the new
capability bits that indicate the presence, but on real machines, it
doesn't fit.  So, for the time being, the driver checks the product id
in the ext capability bits and assumes that LED exists on the known
devices.

The support of LED is controlled via a normal input event with EV_LED
bit mask.  It supports LED_MUTE bit.  X driver can detect the LED
support by checking these bits.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/input/mouse/synaptics.c |   54 +++++++++++++++++++++++++++++++++++++++
 drivers/input/mouse/synaptics.h |    4 +++
 2 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 6a51542..ea874bd 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -28,6 +28,7 @@
 #include <linux/input.h>
 #include <linux/serio.h>
 #include <linux/libps2.h>
+#include <linux/workqueue.h>
 #include <linux/slab.h>
 #include "psmouse.h"
 #include "synaptics.h"
@@ -328,6 +329,37 @@ static void synaptics_pt_create(struct psmouse *psmouse)
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
 
+static void synaptics_set_led(struct psmouse *psmouse, int on)
+{
+	unsigned char param[1];
+
+	if (psmouse_sliced_command(psmouse, on ? 0x88 : 0x10))
+		return;
+	param[0] = 0x0a;
+	ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_SETRATE);
+}
+
+static void synaptics_led_work(struct work_struct *work)
+{
+	struct synaptics_data *priv = container_of(work, struct synaptics_data, led_work);
+	synaptics_set_led(priv->psmouse, priv->led_status);
+}
+
+/* input event handler: changed by x11 synaptics driver */
+static int synaptics_led_event(struct input_dev *dev, unsigned int ...
[ message continues ]

Could we use generic LED API for this? It is not really 'mute' led
after all...
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--

[ message continues ]

Yep, LED_MUTE is not for "muting" touchpads.

-- 
Dmitry
--

[ message continues ]

At Thu, 15 Apr 2010 21:12:18 +0200,

Yeah, actually I started implementing with LED ADI at first.

But, then it turned out to be that it's much easier to use the
existing LED input bits since this LED is really tightly coupled with
the synaptics input device.  An individual LED device makes hard to
find out the corresponding input device.

If we assume there is only one synaptics and only one synaptics-LED

If the problem is the misuse of LED_MUTE bit, how about adding a new
LED bit, e.g. LED_TOUCHPAD?


thanks,

Takashi
--

[ message continues ]

At Fri, 16 Apr 2010 10:00:20 +0200,

The revised patch with an addition of LED_TOUCHPAD is below.


thanks,

Takashi

===

From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] input: Add LED support to Synaptics device

The new Synaptics devices have an LED on the top-left corner.
This is controlled via the command 0x0a with parameters 0x88 or 0x10.

The detection of the LED isn't clear yet.  It should have been the new
capability bits that indicate the presence, but on real machines, it
doesn't fit.  So, for the time being, the driver checks the product id
in the ext capability bits and assumes that LED exists on the known
devices.

The support of LED is controlled via a normal input event with EV_LED
bit mask.  A new LED bitmask, LED_TOUCHPAD, is added.  X driver can
detect the LED support by checking these bits.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/input/mouse/synaptics.c |   64 +++++++++++++++++++++++++++++++++++++++
 drivers/input/mouse/synaptics.h |    5 +++
 include/linux/input.h           |    1 +
 3 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index c7b5285..bffa474 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -28,6 +28,7 @@
 #include <linux/input.h>
 #include <linux/serio.h>
 #include <linux/libps2.h>
+#include <linux/workqueue.h>
 #include <linux/slab.h>
 #include "psmouse.h"
 #include "synaptics.h"
@@ -339,6 +340,52 @@ static void synaptics_pt_create(struct psmouse *psmouse)
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
 
+static void synaptics_set_led(struct psmouse *psmouse, int on)
+{
+	unsigned char param[1];
+
+	if (psmouse_sliced_command(psmouse, on ? 0x88 : 0x10))
+		return;
+	param[0] = 0x0a;
+	ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_SETRATE);
+}
+
+static void synaptics_led_work(struct ...
[ message continues ]

Sorry Takashi, but I will not add any new LED types to input. Even
current input LEDs are going to be accessible thought standard LED
framework (even though I have not apploed Samuel's patch yet I do think
it would move kernel in the right direction).

Thanks.

-- 
Dmitry
--

[ message continues ]

At Tue, 20 Apr 2010 22:43:03 -0700,

Hrm, OK, we can live in other way, too.  For touchpad, it wouldn't be
a big problem because it's likely a single device.

But, how can we link a led class device and another device, in
general?

Also, another remaining question is the lifetime of led device.
The mouse device tends to be re-assigned often, e.g. at each time you
suspend/hibernate.  Should led device also be removed and revived at
each time, or should we keep it and just ignore event?  If we remove
it, how can we avoid race?


thanks,

Takashi
--

[ message continues ]

I think this question is to Richard, I am a bit fuzzy on LED subsystem.

We go to a great lengths to properly resume the device keeping the same
input_dev structure and the same event node (for historical reasons
really, nowadays X is hotplug aware so we could do without), do you see

I am not sure what race you see - psmouse protocol switch should not
race so if you remove LED there you shoudl be race-free. If you see
something racing I am definitely interested in hearing about it.

Thanks!

-- 
Dmitry
--

[ message continues ]

At Tue, 20 Apr 2010 23:39:54 -0700,


Maybe my test system is old; it's not Xorg 1.8 and not using dynamic X
config.  At least, X driver loses sometimes the device upon resume,

Well, what I thought of is when the led sysfs doesn't exist even
though the input device is created.  Or, vice versa, the led sysfs
remains while the input device is deactivated.

But this might not be a problem when I create the led device in
probe.  Let's see in the real world.


thanks,

Takashi
--

[ message continues ]

Add the detection of Synaptics Clickpad device.
The device can be detected a new query command 0x0c.  The clickpad
flags are in cap[0]:4 and cap[1]:0 bits.  But, the driver checks
first the product id bits in the ext capabilities to be sure, so
that it skips the new check on older devices.

When the device is detected, the driver now reports only the left
button as the supported buttons so that X11 driver can detect that
the device is Clickpad.  A Clickpad device gives the button events
only as the middle button.  The kernel driver morphs to the left
button.  The real handling of Clickpad is done rather in X driver
side.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/input/mouse/synaptics.c |   32 ++++++++++++++++++++++++++++++++
 drivers/input/mouse/synaptics.h |    4 ++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 026df60..6a51542 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -328,6 +328,24 @@ static void synaptics_pt_create(struct psmouse *psmouse)
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
 
+static void synaptics_check_clickpad(struct psmouse *psmouse)
+{
+	struct synaptics_data *priv = psmouse->private;
+	unsigned char ncap[3];
+
+	/* check the new capability bits only on known working devices */
+	if (SYN_CAP_PRODUCT_ID(priv->ext_cap) != 0xe4)
+		return;
+	if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_CAPAB2, ncap))
+		return;
+	printk(KERN_INFO "Synaptics: newcap: %02x:%02x:%02x\n",
+	       ncap[0], ncap[1], ncap[2]);
+	priv->clickpad = ((ncap[0] & 0x10) >> 4) | ((ncap[1] & 0x01) << 1);
+	if (priv->clickpad)
+		printk(KERN_INFO "Synaptics: Clickpad device detected: %d\n",
+		       priv->clickpad);
+}
+
 static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data *priv, struct synaptics_hw_state *hw)
 {
 ...
[ message continues ]

Hi Takashi,


Instead of looking at the product id, can we check the number of
supported extended capabilities queries and act accordingly, like the
patch below?

Thanks.

-- 
Dmitry


Input: Add support of Synaptics Clickpad device

From: Takashi Iwai <tiwai@suse.de>

The new type of touchpads can be detected via a new query command 0x0c.
The clickpad flags are in cap[0]:4 and cap[1]:0 bits.

When the device is detected, the driver now reports only the left
button as the supported buttons so that X11 driver can detect that
the device is Clickpad.  A Clickpad device gives the button events
only as the middle button.  The kernel driver morphs to the left
button.  The real handling of Clickpad is done rather in X driver
side.

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

 drivers/input/mouse/synaptics.c |   33 +++++++++++++++++++++++++++++----
 drivers/input/mouse/synaptics.h |    4 ++++
 2 files changed, 33 insertions(+), 4 deletions(-)


diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 026df60..c7b5285 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -137,7 +137,8 @@ static int synaptics_capability(struct psmouse *psmouse)
 	if (synaptics_send_cmd(psmouse, SYN_QUE_CAPABILITIES, cap))
 		return -1;
 	priv->capabilities = (cap[0] << 16) | (cap[1] << 8) | cap[2];
-	priv->ext_cap = 0;
+	priv->ext_cap = priv->ext_cap_0c = 0;
+
 	if (!SYN_CAP_VALID(priv->capabilities))
 		return -1;
 
@@ -162,6 +163,16 @@ static int synaptics_capability(struct psmouse *psmouse)
 				priv->ext_cap &= 0xff0fff;
 		}
 	}
+
+	if (SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 4) {
+		if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_CAPAB_0C, cap)) {
+			printk(KERN_ERR "Synaptics claims to have extended capability 0x0c,"
+			       " but I'm not able to read it.");
+		} else {
+			priv->ext_cap_0c = (cap[0] << 16) | (cap[1] << 8) | cap[2];
+		}
+	}
+
 	return ...
[ message continues ]

Hi Dmitry,

At Mon, 19 Apr 2010 01:32:22 -0700,


Here missing a newline, BTW.


thanks,

Takashi
--

[ message continues ]

Fixed. Thank you for testing. I have that patch in 'for-linus' for .34.
Please push your synaptics X changes upstream as well.

-- 
Dmitry
--

[ message continues ]

At Tue, 20 Apr 2010 22:44:55 -0700,

Thanks.  It was already submitted.
I'm going to repost the revised X patch later.


Takashi
--

[ message continues ]