[PATCH 0/2] Synaptics Clickpad support

[Takashi Iwai]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Takashi Iwai]

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 <ti...@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 type,
+ unsigned int code, int value)
+{
+ struct synaptics_data *priv = dev->dev.platform_data;
+
+ if (!priv)
+ return 0;
+ if (type == EV_LED && code == LED_MUTE) {
+ priv->led_status = !!value;
+ schedule_work(&priv->led_work);
+ }
+ return 0;
+}
+
static void synaptics_check_clickpad(struct psmouse *psmouse)
{
struct synaptics_data *priv = psmouse->private;
@@ -344,6 +376,12 @@ static void synaptics_check_clickpad(struct psmouse *psmouse)
if (priv->clickpad)
printk(KERN_INFO "Synaptics: Clickpad device detected: %d\n",
priv->clickpad);
+ /* FIXME: this should be ncap[1] 0x20, but it's not really... */
+ priv->has_led = 1;
+ if (priv->has_led) {
+ printk(KERN_INFO "Synaptics: support LED control\n");
+ INIT_WORK(&priv->led_work, synaptics_led_work);
+ }
}

static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data *priv, struct synaptics_hw_state *hw)
@@ -623,10 +661,22 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
__clear_bit(BTN_RIGHT, dev->keybit); /* only left-button */
__clear_bit(BTN_MIDDLE, dev->keybit);
}
+ if (priv->has_led) {
+ __set_bit(EV_LED, dev->evbit);
+ __set_bit(LED_MUTE, dev->ledbit);
+ dev->event = synaptics_led_event;
+ dev->dev.platform_data = priv;
+ }
}

static void synaptics_disconnect(struct psmouse *psmouse)
{
+ struct synaptics_data *priv = psmouse->private;
+
+ if (priv->has_led) {
+ cancel_work_sync(&priv->led_work);
+ synaptics_set_led(psmouse, 0);
+ }
synaptics_reset(psmouse);
kfree(psmouse->private);
psmouse->private = NULL;
@@ -658,6 +708,9 @@ static int synaptics_reconnect(struct psmouse *psmouse)
return -1;
}

+ if (priv->has_led)
+ synaptics_set_led(psmouse, priv->led_status);
+
return 0;
}

@@ -713,6 +766,7 @@ int synaptics_init(struct psmouse *psmouse)
if (!priv)
return -1;

+ priv->psmouse = psmouse;
psmouse_reset(psmouse);

if (synaptics_query_hardware(psmouse)) {
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index b824851..aeca6d2 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -107,6 +107,10 @@ struct synaptics_data {
int scroll;

unsigned char clickpad;
+ unsigned char has_led;
+ unsigned char led_status;
+ struct psmouse *psmouse;
+ struct work_struct led_work;
};

void synaptics_module_init(void);
--
1.7.0.4

[Takashi Iwai]

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 <ti...@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)

{
memset(hw, 0, sizeof(struct synaptics_hw_state));
@@ -354,6 +372,13 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data
hw->scroll = (signed char)(buf[1]);
}

+ if (priv->clickpad) {
+ /* clickpad reports only the middle button, report
+ * it as the left button
+ */
+ hw->left = ((buf[0] ^ buf[3]) & 0x01) ? 1 : 0;
+ }
+
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;
@@ -593,6 +618,11 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)

dev->absres[ABS_X] = priv->x_res;
dev->absres[ABS_Y] = priv->y_res;
+
+ if (priv->clickpad) {
+ __clear_bit(BTN_RIGHT, dev->keybit); /* only left-button */
+ __clear_bit(BTN_MIDDLE, dev->keybit);

+ }
}

static void synaptics_disconnect(struct psmouse *psmouse)

@@ -702,6 +732,8 @@ int synaptics_init(struct psmouse *psmouse)
SYN_ID_MAJOR(priv->identity), SYN_ID_MINOR(priv->identity),
priv->model_id, priv->capabilities, priv->ext_cap);

+ synaptics_check_clickpad(psmouse);
+
set_input_params(psmouse->dev, priv);

/*
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index f0f40a3..b824851 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -18,6 +18,7 @@
#define SYN_QUE_SERIAL_NUMBER_SUFFIX 0x07
#define SYN_QUE_RESOLUTION 0x08
#define SYN_QUE_EXT_CAPAB 0x09
+#define SYN_QUE_EXT_CAPAB2 0x0c

/* synatics modes */
#define SYN_BIT_ABSOLUTE_MODE (1 << 7)
@@ -48,6 +49,7 @@
#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)

/* synaptics modes query bits */
#define SYN_MODE_ABSOLUTE(m) ((m) & (1 << 7))
@@ -103,6 +105,8 @@ struct synaptics_data {
unsigned char pkt_type; /* packet type - old, new, etc */
unsigned char mode; /* current mode byte */
int scroll;
+
+ unsigned char clickpad;
};

void synaptics_module_init(void);
--
1.7.0.4

[Pavel Machek]

Hi!

> 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.

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

[Dmitry Torokhov]

On Thu, Apr 15, 2010 at 09:12:18PM +0200, Pavel Machek wrote:
> Hi!
>
> > 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.
>
> Could we use generic LED API for this? It is not really 'mute' led
> after all...

Yep, LED_MUTE is not for "muting" touchpads.

--
Dmitry

[Takashi Iwai]

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

Pavel Machek wrote:
>
> Hi!
>
> > 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.
>
> Could we use generic LED API for this?

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
device, then yes, the situation can be a bit easier, though.

> It is not really 'mute' led after all...

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

thanks,

Takashi

[Dmitry Torokhov]

Hi Takashi,

On Wed, Apr 14, 2010 at 05:10:22PM +0200, Takashi Iwai wrote:
> 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.
>

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 <ti...@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 <ti...@suse.de>
Signed-off-by: Dmitry Torokhov <dt...@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 0;
}

@@ -348,7 +359,15 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data
hw->left = (buf[0] & 0x01) ? 1 : 0;
hw->right = (buf[0] & 0x02) ? 1 : 0;

- if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
+ if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
+ /*
+ * Clickpad's button is transmitted as middle button,
+ * however, since it is primary button, we will report
+ * it as BTN_LEFT.

+ */
+ hw->left = ((buf[0] ^ buf[3]) & 0x01) ? 1 : 0;
+

+ } else 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]);

@@ -593,6 +612,12 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)

dev->absres[ABS_X] = priv->x_res;
dev->absres[ABS_Y] = priv->y_res;
+

+ if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
+ /* Clickpads report only left button */
+ __clear_bit(BTN_RIGHT, dev->keybit);

+ __clear_bit(BTN_MIDDLE, dev->keybit);
+ }
}

static void synaptics_disconnect(struct psmouse *psmouse)

@@ -697,10 +722,10 @@ int synaptics_init(struct psmouse *psmouse)

priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS;

- printk(KERN_INFO "Synaptics Touchpad, model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx\n",
+ printk(KERN_INFO "Synaptics Touchpad, model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx\n",
SYN_ID_MODEL(priv->identity),
SYN_ID_MAJOR(priv->identity), SYN_ID_MINOR(priv->identity),
- priv->model_id, priv->capabilities, priv->ext_cap);
+ priv->model_id, priv->capabilities, priv->ext_cap, priv->ext_cap_0c);

set_input_params(psmouse->dev, priv);

diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index f0f40a3..ae37c5d 100644

--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -18,6 +18,7 @@
#define SYN_QUE_SERIAL_NUMBER_SUFFIX 0x07
#define SYN_QUE_RESOLUTION 0x08
#define SYN_QUE_EXT_CAPAB 0x09

+#define SYN_QUE_EXT_CAPAB_0C 0x0c

/* synatics modes */
#define SYN_BIT_ABSOLUTE_MODE (1 << 7)

@@ -48,6 +49,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(ex0c) ((ex0c) & 0x100100)

/* synaptics modes query bits */
#define SYN_MODE_ABSOLUTE(m) ((m) & (1 << 7))

@@ -96,6 +99,7 @@ struct synaptics_data {
unsigned long int model_id; /* Model-ID */
unsigned long int capabilities; /* Capabilities */
unsigned long int ext_cap; /* Extended Capabilities */
+ unsigned long int ext_cap_0c; /* Ext Caps from 0x0c query */
unsigned long int identity; /* Identification */
int x_res; /* X resolution in units/mm */
int y_res; /* Y resolution in units/mm */

[Takashi Iwai]

Hi Dmitry,

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

Dmitry Torokhov wrote:
>
> Hi Takashi,
>
> On Wed, Apr 14, 2010 at 05:10:22PM +0200, Takashi Iwai wrote:
> > 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.
> >
>
> Instead of looking at the product id, can we check the number of
> supported extended capabilities queries and act accordingly, like the
> patch below?

Yes, it worked. (Though, I've tested only new machines.)

> @@ -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.");

Here missing a newline, BTW.

thanks,

Takashi

[Takashi Iwai]

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 <ti...@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 <ti...@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)

+ if (type == EV_LED && code == LED_TOUCHPAD) {

+ priv->led_status = !!value;
+ schedule_work(&priv->led_work);
+ }
+ return 0;
+}
+

+static void synaptics_query_led(struct psmouse *psmouse)
+{
+ struct synaptics_data *priv = psmouse->private;
+
+ /* 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)
+ printk(KERN_INFO "synaptics: support LED control\n");

+ priv->has_led = 1;

+ priv->psmouse = psmouse;

+ INIT_WORK(&priv->led_work, synaptics_led_work);
+ }

+}

+
static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data *priv, struct synaptics_hw_state *hw)

{
memset(hw, 0, sizeof(struct synaptics_hw_state));
@@ -618,10 +665,22 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
__clear_bit(BTN_RIGHT, dev->keybit);

__clear_bit(BTN_MIDDLE, dev->keybit);
}
+ if (priv->has_led) {
+ __set_bit(EV_LED, dev->evbit);

+ __set_bit(LED_TOUCHPAD, dev->ledbit);

+ dev->event = synaptics_led_event;
+ dev->dev.platform_data = priv;
+ }
}

static void synaptics_disconnect(struct psmouse *psmouse)
{
+ struct synaptics_data *priv = psmouse->private;
+
+ if (priv->has_led) {
+ cancel_work_sync(&priv->led_work);
+ synaptics_set_led(psmouse, 0);
+ }
synaptics_reset(psmouse);
kfree(psmouse->private);
psmouse->private = NULL;

@@ -653,6 +712,9 @@ static int synaptics_reconnect(struct psmouse *psmouse)

return -1;
}

+ if (priv->has_led)
+ synaptics_set_led(psmouse, priv->led_status);
+
return 0;
}

@@ -727,6 +789,8 @@ int synaptics_init(struct psmouse *psmouse)
SYN_ID_MAJOR(priv->identity), SYN_ID_MINOR(priv->identity),

priv->model_id, priv->capabilities, priv->ext_cap, priv->ext_cap_0c);

+ synaptics_query_led(psmouse);

+
set_input_params(psmouse->dev, priv);

/*

diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index ae37c5d..0b989f4 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -107,6 +107,11 @@ struct synaptics_data {

unsigned char pkt_type; /* packet type - old, new, etc */
unsigned char mode; /* current mode byte */
int scroll;
+

+ unsigned char has_led;
+ unsigned char led_status;
+ struct psmouse *psmouse;
+ struct work_struct led_work;
};

void synaptics_module_init(void);

diff --git a/include/linux/input.h b/include/linux/input.h
index 7ed2251..0aea50d 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -757,6 +757,7 @@ struct input_absinfo {
#define LED_MISC 0x08
#define LED_MAIL 0x09
#define LED_CHARGING 0x0a
+#define LED_TOUCHPAD 0x0b
#define LED_MAX 0x0f
#define LED_CNT (LED_MAX+1)

--
1.7.0.4

[Dmitry Torokhov]

On Mon, Apr 19, 2010 at 12:29:49PM +0200, Takashi Iwai wrote:
> Hi Dmitry,
>
> At Mon, 19 Apr 2010 01:32:22 -0700,
> Dmitry Torokhov wrote:
> >
> > Hi Takashi,
> >
> > On Wed, Apr 14, 2010 at 05:10:22PM +0200, Takashi Iwai wrote:
> > > 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.
> > >
> >
> > Instead of looking at the product id, can we check the number of
> > supported extended capabilities queries and act accordingly, like the
> > patch below?
>
> Yes, it worked. (Though, I've tested only new machines.)
>

Seems to be working on older (at least one ;) ) as well.

>
> > @@ -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.");
>
> Here missing a newline, BTW.
>

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

--
Dmitry

[Dmitry Torokhov]

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

[Takashi Iwai]

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

[Takashi Iwai]

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

[Dmitry Torokhov]

I think this question is to Richard, I am a bit fuzzy on LED subsystem.
But in general I'd expect LED have touchpad input device as parent.

>
> 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.

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
it being recreated often?

> 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?

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

[Takashi Iwai]

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

This might work, yes.

> > 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.
>
> 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
> it being recreated often?

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,
and it required reopening.

> > 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?
>
> 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.

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

[Takashi Iwai]

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 <ti...@suse.de>
---

This is a revised version with LED class device instead of input LED
event.

drivers/input/mouse/synaptics.c | 88 +++++++++++++++++++++++++++++++++++++++
drivers/input/mouse/synaptics.h | 3 +
2 files changed, 91 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index c7b5285..fcc007c 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/leds.h>

#include <linux/slab.h>
#include "psmouse.h"
#include "synaptics.h"

@@ -335,6 +336,79 @@ static void synaptics_pt_create(struct psmouse *psmouse)
serio_register_port(serio);
}

+/*
+ * LED handling:
+ * Some Synaptics devices have an embeded LED at the top-left corner.
+ */
+
+struct synaptics_led {
+ enum led_brightness status;
+ struct psmouse *psmouse;
+ struct work_struct work;
+ struct led_classdev cdev;
+};
+

+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_led *led;
+
+ led = container_of(work, struct synaptics_data, work);
+ synaptics_set_led(led->psmouse, led->status);
+}
+
+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);
+ if (value != led->status) {
+ led->status = value;
+ schedule_work(&led->work);
+ }
+}
+
+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;
+}
+

/*****************************************************************************

* Functions to interpret the absolute mode packets
****************************************************************************/

@@ -622,6 +696,14 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)

static void synaptics_disconnect(struct psmouse *psmouse)
{
+ struct synaptics_data *priv = psmouse->private;
+

+ if (priv->led) {
+ cancel_work_sync(&priv->led->work);
+ synaptics_set_led(psmouse, 0);
+ led_classdev_unregister(&priv->led->cdev);
+ kfree(priv->led);

+ }
synaptics_reset(psmouse);
kfree(psmouse->private);
psmouse->private = NULL;

@@ -653,6 +735,9 @@ static int synaptics_reconnect(struct psmouse *psmouse)
return -1;
}

+ if (priv->led)
+ synaptics_set_led(psmouse, priv->led->status);
+
return 0;
}

@@ -727,6 +812,9 @@ int synaptics_init(struct psmouse *psmouse)

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);

/*
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h

index ae37c5d..d5bb8f4 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -94,6 +94,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 */

@@ -107,6 +109,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);
--
1.7.0.4

[Takashi Iwai]

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 <ti...@suse.de>
---

Fixed a typo in v2 patch, sorry.

+ led = container_of(work, struct synaptics_led, work);

[Dmitry Torokhov]

On Wed, Apr 21, 2010 at 04:46:39PM +0200, Takashi Iwai wrote:
> 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.
>

Tkashi,

Does it build if you disable LED subsystem? Note that I don't want
psmouse to depend on LEDs...

2+ instances of work may run simultaneously on 2 CPUs; I think you need
lcokign here. Also you need locking to prevent psmouse core access the
device (send other commands) when user accesses sysfs attributes. Also,
doesn't the device have to be in command mode before you start sending
commands to it?

I do not think synaptics_led structure uses that much space to be
allocated separately and conditionally; why don't you just put lde, work
and value directly into synaptics_data?

--
Dmitry

[Takashi Iwai]

At Wed, 21 Apr 2010 10:23:22 -0700,

Dmitry Torokhov wrote:
>
> On Wed, Apr 21, 2010 at 04:46:39PM +0200, Takashi Iwai wrote:
> > 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.
> >
>
> Tkashi,
>
> Does it build if you disable LED subsystem? Note that I don't want
> psmouse to depend on LEDs...

Oh, I missed that dependency, yes.
I'll fix to change to select LEDS on demand.

ps2_command() and co have already mutex in it. I thought this should
suffice. Meanwhile, it'd be better to protect the whole command
sequence at once. I'll change to call ps2_begin_command/end_command()
explicitly.

> Also,
> doesn't the device have to be in command mode before you start sending
> commands to it?

Looks not.

Because I didn't want to pollute synaptics.h by dependency of led
stuff, wanted to make synaptics.c rather self-contained. It'll be
ifdef'ed, and it's not beautiful to see much in the header that is
referred in other file.

thanks,

Takashi

[Takashi Iwai]

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 <ti...@suse.de>
---

Changes from v3:
- Now it has a new Kconfig and the conditional select for LED class.
Can be built without LED support, too.
- The led cdev brightness value is used instead of keeping the value in
synaptics privately.
- Add explicit protection of LED ps2 command sequence.

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

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index c714ca2..91d3517 100644
--- 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
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index c7b5285..8dc1fb5 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/leds.h>
#include <linux/slab.h>
#include "psmouse.h"
#include "synaptics.h"

@@ -335,6 +336,110 @@ static void synaptics_pt_create(struct psmouse *psmouse)
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
****************************************************************************/

@@ -622,6 +727,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)

static void synaptics_disconnect(struct psmouse *psmouse)
{

+ synaptics_free_led(psmouse);

synaptics_reset(psmouse);
kfree(psmouse->private);
psmouse->private = NULL;

@@ -653,6 +759,8 @@ static int synaptics_reconnect(struct psmouse *psmouse)
return -1;
}

+ synaptics_sync_led(psmouse);
+
return 0;
}

@@ -727,6 +835,9 @@ int synaptics_init(struct psmouse *psmouse)

--

[Pali Rohár]

> ***************** * Functions to interpret the absolute mode
> packets
>
> *************************************************************
> ***************/ @@ -622,6 +727,7 @@ static void

Hello,

what happened with this patch? There is no discussion about v4:
https://patchwork.kernel.org/patch/94026/
https://lkml.org/lkml/2010/4/22/35

Are there any problems with this patch?

--
Pali Rohár
pali....@gmail.com

[Pali Rohár]

BUMP!

--
Pali Rohár
pali....@gmail.com