Message-ID: <s5heic04p2p.wl%tiwai@suse.de>
Date:	Fri, 08 Oct 2010 18:38:38 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Chase Douglas <chase.douglas@...onical.com>
Cc:	linux-input@...r.kernel.org, xorg-devel@...ts.x.org,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Chris Bagwell <chris@...bagwell.com>,
	Andy Whitcroft <apw@...onical.com>,
	Henrik Rydberg <rydberg@...omail.se>,
	linux-kernel@...r.kernel.org,
	Peter Hutterer <peter.hutterer@...-t.net>,
	Duncan McGreggor <duncan.mcgreggor@...onical.com>
Subject: Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

At Fri, 08 Oct 2010 18:37:22 +0200,
Takashi Iwai wrote:
> 
> At Fri,  8 Oct 2010 10:57:57 -0400,
> Chase Douglas wrote:
> > 
> > Tobyn Bertram reverse engineered the multitouch protocol for Synaptics devices.
> > I've been able to take his work and produce a series of commits to enable MT
> > and multifinger (MF) support.
> > 
> > Unfortunately, there's a tricky issue with some Synaptics touchpads that have
> > integrated buttons. For example, the left and right buttons on the touchpad of
> > my Dell Mini 1012 consist of the lower ~20% of the touchpad surface. The
> > touchpad physically clicks under these areas.
> > 
> > The X synaptics input module now has a parameter to disable touches occuring
> > over the button area, but this solution still doesn't work perfectly. If you
> > click a button and drag with another finger near the clicking finger, the
> > touchpad gets confused.
> > 
> > Now that we have full MT support, we can try to handle this scenario better.
> > What I've found to work best is to make touches vanish if they occur over the
> > button area of the trackpad while any button is held. This works in conjunction
> > with the X synaptics driver to disable single touch control over the button
> > area. With full MT support, the touchpad doesn't seem to get confused when a
> > click and drag occurs with two fingers close to each other, and it enables MT
> > gestures and MF support across the entire trackpad when no buttons are held.
> > 
> > The first question is whether this seems appropriate to others, or if some
> > other method would work better. Secondarily, should the solution occur in the
> > kernel, like I have in the third patch of this series, or should it occur in
> > the X input module? Although we don't have this information today, we may be
> > able to query the touchpad in the future to know the area of the integrated
> > buttons. If that were possible, would the recommended location for the hack
> > change?
> 
> Great!  Finally someone found it out!
> I found this and made a series of patches in 4 months ago.  Since
> then, Novell legal prohibited me to send the patches to the upstream
> due to "possible patent infringing".  Now you cracked out.  Yay.
> 
> FWIW, my corresponding patch is below.  It really looks similar in the
> end ;)  I added a kconfig just to be safer.
> 
> Regarding the "clickpad" support: in my case, I implemented almost
> everything about it in xorg driver.  I'm going to submit xorg
> patches.

BTW, yet another kernel patch is missing; the support of embedded LED.
I've posted this once, but it seems forgotten since then.  Reposted
below.


Takashi

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

The new Synaptics devices have an LED on the top-left corner.
This patch adds a new LED class device to control it.  It's created
dynamically upon synaptics device probing.

The LED is controlled via the command 0x0a with parameters 0x88 or 0x10.
This seems only on/off control although other value might be accepted.

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.

Signed-off-by: Takashi Iwai <tiwai@...e.de>

---
 drivers/input/mouse/Kconfig     |    9 +++
 drivers/input/mouse/synaptics.c |  111 ++++++++++++++++++++++++++++++++++++++++
 drivers/input/mouse/synaptics.h |    3 +
 3 files changed, 123 insertions(+)

--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -19,6 +19,7 @@ config MOUSE_PS2
 	select SERIO_LIBPS2
 	select SERIO_I8042 if X86
 	select SERIO_GSCPS2 if GSC
+	select LEDS_CLASS if MOUSE_PS2_SYNAPICS_LED
 	help
 	  Say Y here if you have a PS/2 mouse connected to your system. This
 	  includes the standard 2 or 3-button PS/2 mouse, as well as PS/2
@@ -67,6 +68,14 @@ config MOUSE_PS2_SYNAPTICS
 
 	  If unsure, say Y.
 
+config MOUSE_PS2_SYNAPTICS_LED
+	bool "Support embedded LED on Synaptics devices"
+	depends on MOUSE_PS2_SYNAPTICS
+	select NEW_LEDS
+	help
+	  Say Y here if you have a Synaptics device with an embedded LED.
+	  This will enable LED class driver to control the LED device.
+
 config MOUSE_PS2_LIFEBOOK
 	bool "Fujitsu Lifebook PS/2 mouse protocol extension" if EMBEDDED
 	default y
--- 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/leds.h>
 #include <linux/slab.h>
 #include "psmouse.h"
 #include "synaptics.h"
@@ -353,6 +354,110 @@ static void synaptics_pt_create(struct p
 	serio_register_port(serio);
 }
 
+#ifdef CONFIG_MOUSE_PS2_SYNAPTICS_LED
+/*
+ * LED handling:
+ * Some Synaptics devices have an embeded LED at the top-left corner.
+ */
+
+struct synaptics_led {
+	struct psmouse *psmouse;
+	struct work_struct work;
+	struct led_classdev cdev;
+};
+
+static void synaptics_set_led(struct psmouse *psmouse, int on)
+{
+	int i;
+	unsigned char cmd = on ? 0x88 : 0x10;
+
+	ps2_begin_command(&psmouse->ps2dev);
+	if (__ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11))
+		goto out;
+	for (i = 6; i >= 0; i -= 2) {
+		unsigned char d = (cmd >> i) & 3;
+		if (__ps2_command(&psmouse->ps2dev, &d, PSMOUSE_CMD_SETRES))
+			goto out;
+	}
+	cmd = 0x0a;
+	__ps2_command(&psmouse->ps2dev, &cmd, PSMOUSE_CMD_SETRATE);
+ out:
+	ps2_end_command(&psmouse->ps2dev);
+}
+
+static void synaptics_led_work(struct work_struct *work)
+{
+	struct synaptics_led *led;
+
+	led = container_of(work, struct synaptics_led, work);
+	synaptics_set_led(led->psmouse, led->cdev.brightness);
+}
+
+static void synaptics_led_cdev_brightness_set(struct led_classdev *cdev,
+					      enum led_brightness value)
+{
+	struct synaptics_led *led;
+
+	led = container_of(cdev, struct synaptics_led, cdev);
+	schedule_work(&led->work);
+}
+
+static void synaptics_sync_led(struct psmouse *psmouse)
+{
+	struct synaptics_data *priv = psmouse->private;
+
+	if (priv->led)
+		synaptics_set_led(psmouse, priv->led->cdev.brightness);
+}
+
+static int synaptics_init_led(struct psmouse *psmouse)
+{
+	struct synaptics_data *priv = psmouse->private;
+	struct synaptics_led *led;
+	int err;
+
+	/* FIXME: LED is supposedly detectable in cap0c[1] 0x20, but it seems
+	 * not working on real machines.
+	 * So we check the product id to be sure.
+	 */
+	if (!priv->ext_cap_0c || SYN_CAP_PRODUCT_ID(priv->ext_cap) != 0xe4)
+		return 0;
+
+	printk(KERN_INFO "synaptics: support LED control\n");
+	led = kzalloc(sizeof(struct synaptics_led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+	led->psmouse = psmouse;
+	INIT_WORK(&led->work, synaptics_led_work);
+	led->cdev.name = "psmouse::synaptics";
+	led->cdev.brightness_set = synaptics_led_cdev_brightness_set;
+	led->cdev.flags = LED_CORE_SUSPENDRESUME;
+	err = led_classdev_register(NULL, &led->cdev);
+	if (err < 0) {
+		kfree(led);
+		return err;
+	}
+	priv->led = led;
+	return 0;
+}
+
+static void synaptics_free_led(struct psmouse *psmouse)
+{
+	struct synaptics_data *priv = psmouse->private;
+
+	if (!priv->led)
+		return;
+	cancel_work_sync(&priv->led->work);
+	synaptics_set_led(psmouse, 0);
+	led_classdev_unregister(&priv->led->cdev);
+	kfree(priv->led);
+}
+#else
+#define synaptics_init_led(ps)	0
+#define synaptics_free_led(ps)	do {} while (0)
+#define synaptics_sync_led(ps)	do {} while (0)
+#endif
+
 /*****************************************************************************
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
@@ -647,6 +752,7 @@ static void set_input_params(struct inpu
 
 static void synaptics_disconnect(struct psmouse *psmouse)
 {
+	synaptics_free_led(psmouse);
 	synaptics_reset(psmouse);
 	kfree(psmouse->private);
 	psmouse->private = NULL;
@@ -678,6 +784,8 @@ static int synaptics_reconnect(struct ps
 		return -1;
 	}
 
+	synaptics_sync_led(psmouse);
+
 	return 0;
 }
 
@@ -752,6 +860,9 @@ int synaptics_init(struct psmouse *psmou
 		SYN_ID_MAJOR(priv->identity), SYN_ID_MINOR(priv->identity),
 		priv->model_id, priv->capabilities, priv->ext_cap, priv->ext_cap_0c);
 
+	if (synaptics_init_led(psmouse) < 0)
+		goto init_fail;
+
 	set_input_params(psmouse->dev, priv);
 
 	/*
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -97,6 +97,8 @@ struct synaptics_hw_state {
 	signed char scroll;
 };
 
+struct synaptics_led;
+
 struct synaptics_data {
 	/* Data read from the touchpad */
 	unsigned long int model_id;		/* Model-ID */
@@ -110,6 +112,7 @@ struct synaptics_data {
 	unsigned char pkt_type;			/* packet type - old, new, etc */
 	unsigned char mode;			/* current mode byte */
 	int scroll;
+	struct synaptics_led *led;
 };
 
 void synaptics_module_init(void);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists