[PATCH 1/4] uapi/input.h: Add INPUT_PROP_TOPBUTTONPAD device property

[PATCH 1/4] uapi/input.h: Add INPUT_PROP_TOPBUTTONPAD device property

[Hans de Goede]

On some newer laptops with a trackpoint the physical buttons for the
trackpoint have been removed to allow for a larger touchpad. On these
laptops the buttonpad has clearly marked areas on the top which are to be
used as trackpad buttons.

Users of the event device-node need to know about this, so that they can
properly interpret BTN_LEFT events as being a left / right / middle click
depending on where on the button pad the clicking finger is.

This commits adds a INPUT_PROP_TOPBUTTONPAD device property which drivers
for such buttonpads will use to signal to the user that this buttonpad not
only has the normal bottom button area, but also a top button area.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/uapi/linux/input.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index bd24470..f484952 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -164,6 +164,7 @@ struct input_keymap_entry {
 #define INPUT_PROP_DIRECT		0x01	/* direct input devices */
 #define INPUT_PROP_BUTTONPAD		0x02	/* has button(s) under pad */
 #define INPUT_PROP_SEMI_MT		0x03	/* touch rectangle only */
+#define INPUT_PROP_TOPBUTTONPAD		0x04	/* softbuttons at top of pad */
 
 #define INPUT_PROP_MAX			0x1f
 #define INPUT_PROP_CNT			(INPUT_PROP_MAX + 1)
-- 
1.9.0

[PATCH 2/4] serio: Add pnp_id to struct serio

[Hans de Goede]

Serio device drivers need access to the pnp_id of the serio port, windows
drivers bind by the pnp-id and some drivers need to know the exact pnp-id
used so they know exactly with which hardware model / revision they are
dealing with.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/linux/serio.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/serio.h b/include/linux/serio.h
index 9f779c7..6532440 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -16,6 +16,7 @@
 #include <linux/mutex.h>
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
+#include <linux/pnp.h>
 #include <uapi/linux/serio.h>
 
 struct serio {
@@ -28,6 +29,7 @@ struct serio {
 	bool manual_bind;
 
 	struct serio_device_id id;
+	struct pnp_id *pnp_id;
 
 	spinlock_t lock;		/* protects critical sections from port's interrupt handler */
 
-- 
1.9.0

Re: [PATCH 2/4] serio: Add pnp_id to struct serio

[Dmitry Torokhov]

Hi Hans,

On Wed, Apr 09, 2014 at 04:02:59PM +0200, Hans de Goede wrote:
> Serio device drivers need access to the pnp_id of the serio port, windows
> drivers bind by the pnp-id and some drivers need to know the exact pnp-id
> used so they know exactly with which hardware model / revision they are
> dealing with.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  include/linux/serio.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/serio.h b/include/linux/serio.h
> index 9f779c7..6532440 100644
> --- a/include/linux/serio.h
> +++ b/include/linux/serio.h
> @@ -16,6 +16,7 @@
>  #include <linux/mutex.h>
>  #include <linux/device.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/pnp.h>
>  #include <uapi/linux/serio.h>
>  
>  struct serio {
> @@ -28,6 +29,7 @@ struct serio {
>  	bool manual_bind;
>  
>  	struct serio_device_id id;
> +	struct pnp_id *pnp_id;

Why do we need this if we are already adding generic 'firmware_id'
attribute?

Thanks.

-- 
Dmitry

Re: [PATCH 2/4] serio: Add pnp_id to struct serio

[Hans de Goede]

Hi,

On 04/09/2014 08:06 PM, Dmitry Torokhov wrote:
> Hi Hans,
> 
> On Wed, Apr 09, 2014 at 04:02:59PM +0200, Hans de Goede wrote:
>> Serio device drivers need access to the pnp_id of the serio port, windows
>> drivers bind by the pnp-id and some drivers need to know the exact pnp-id
>> used so they know exactly with which hardware model / revision they are
>> dealing with.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  include/linux/serio.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/serio.h b/include/linux/serio.h
>> index 9f779c7..6532440 100644
>> --- a/include/linux/serio.h
>> +++ b/include/linux/serio.h
>> @@ -16,6 +16,7 @@
>>  #include <linux/mutex.h>
>>  #include <linux/device.h>
>>  #include <linux/mod_devicetable.h>
>> +#include <linux/pnp.h>
>>  #include <uapi/linux/serio.h>
>>  
>>  struct serio {
>> @@ -28,6 +29,7 @@ struct serio {
>>  	bool manual_bind;
>>  
>>  	struct serio_device_id id;
>> +	struct pnp_id *pnp_id;
> 
> Why do we need this if we are already adding generic 'firmware_id'
> attribute?

In patch 4/4 we do pnp-id matching in synaptics.c to set the new
INPUT_PROP_TOPBUTTONPAD property on relevant touchpads. Since the kernel
has a special pnp_id type + pnp_id matching code, the correct thing to
do is to use that for in kernel pnp-id matching, which means that we
need to propagate the pnp-id to the serio drivers.

Regards,

Hans

Re: [PATCH 2/4] serio: Add pnp_id to struct serio

[Dmitry Torokhov]

On Thu, Apr 10, 2014 at 10:03:21AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 04/09/2014 08:06 PM, Dmitry Torokhov wrote:
> > Hi Hans,
> > 
> > On Wed, Apr 09, 2014 at 04:02:59PM +0200, Hans de Goede wrote:
> >> Serio device drivers need access to the pnp_id of the serio port, windows
> >> drivers bind by the pnp-id and some drivers need to know the exact pnp-id
> >> used so they know exactly with which hardware model / revision they are
> >> dealing with.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>  include/linux/serio.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/include/linux/serio.h b/include/linux/serio.h
> >> index 9f779c7..6532440 100644
> >> --- a/include/linux/serio.h
> >> +++ b/include/linux/serio.h
> >> @@ -16,6 +16,7 @@
> >>  #include <linux/mutex.h>
> >>  #include <linux/device.h>
> >>  #include <linux/mod_devicetable.h>
> >> +#include <linux/pnp.h>
> >>  #include <uapi/linux/serio.h>
> >>  
> >>  struct serio {
> >> @@ -28,6 +29,7 @@ struct serio {
> >>  	bool manual_bind;
> >>  
> >>  	struct serio_device_id id;
> >> +	struct pnp_id *pnp_id;
> > 
> > Why do we need this if we are already adding generic 'firmware_id'
> > attribute?
> 
> In patch 4/4 we do pnp-id matching in synaptics.c to set the new
> INPUT_PROP_TOPBUTTONPAD property on relevant touchpads. Since the kernel
> has a special pnp_id type + pnp_id matching code, the correct thing to
> do is to use that for in kernel pnp-id matching, which means that we
> need to propagate the pnp-id to the serio drivers.

I disagree that it is good enough reason for putting platform-specific
data in a generic structure. If you really want to use generic PNP
matching code you can just convert firmware_id back to pnp_id in
Synaptics driver.

Thanks.

-- 
Dmitry

[PATCH 3/4] input/serio/8042_pnp: set serio->pnp_id for pnp instantiated 8042 ports

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/serio/i8042-x86ia64io.h | 4 ++++
 drivers/input/serio/i8042.c           | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index 2416c74..1d2e13f 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -725,6 +725,8 @@ static int i8042_pnp_kbd_probe(struct pnp_dev *dev, const struct pnp_device_id *
 	if (pnp_irq_valid(dev,0))
 		i8042_pnp_kbd_irq = pnp_irq(dev, 0);
 
+	i8042_kbd_pnp_id = dev->id;
+
 	strlcpy(i8042_pnp_kbd_name, did->id, sizeof(i8042_pnp_kbd_name));
 	if (strlen(pnp_dev_name(dev))) {
 		strlcat(i8042_pnp_kbd_name, ":", sizeof(i8042_pnp_kbd_name));
@@ -751,6 +753,8 @@ static int i8042_pnp_aux_probe(struct pnp_dev *dev, const struct pnp_device_id *
 	if (pnp_irq_valid(dev, 0))
 		i8042_pnp_aux_irq = pnp_irq(dev, 0);
 
+	i8042_aux_pnp_id = dev->id;
+
 	strlcpy(i8042_pnp_aux_name, did->id, sizeof(i8042_pnp_aux_name));
 	if (strlen(pnp_dev_name(dev))) {
 		strlcat(i8042_pnp_aux_name, ":", sizeof(i8042_pnp_aux_name));
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 3807c3e..4a3fdf4 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -89,6 +89,8 @@ MODULE_PARM_DESC(debug, "Turn i8042 debugging mode on and off");
 static bool i8042_bypass_aux_irq_test;
 static char i8042_kbd_firmware_id[128];
 static char i8042_aux_firmware_id[128];
+static struct pnp_id *i8042_kbd_pnp_id;
+static struct pnp_id *i8042_aux_pnp_id;
 
 #include "i8042.h"
 
@@ -1217,6 +1219,7 @@ static int __init i8042_create_kbd_port(void)
 	serio->stop		= i8042_stop;
 	serio->close		= i8042_port_close;
 	serio->port_data	= port;
+	serio->pnp_id		= i8042_kbd_pnp_id;
 	serio->dev.parent	= &i8042_platform_device->dev;
 	strlcpy(serio->name, "i8042 KBD port", sizeof(serio->name));
 	strlcpy(serio->phys, I8042_KBD_PHYS_DESC, sizeof(serio->phys));
@@ -1246,6 +1249,7 @@ static int __init i8042_create_aux_port(int idx)
 	serio->port_data	= port;
 	serio->dev.parent	= &i8042_platform_device->dev;
 	if (idx < 0) {
+		serio->pnp_id = i8042_aux_pnp_id;
 		strlcpy(serio->name, "i8042 AUX port", sizeof(serio->name));
 		strlcpy(serio->phys, I8042_AUX_PHYS_DESC, sizeof(serio->phys));
 		strlcpy(serio->firmware_id, i8042_aux_firmware_id,
-- 
1.9.0

[PATCH 4/4] synaptices: Report INPUT_PROP_TOPBUTTONPAD property for touchpads with top buttonareas

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/synaptics.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index d8d49d1..4637cc7 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -118,6 +118,16 @@ void synaptics_reset(struct psmouse *psmouse)
 
 #ifdef CONFIG_MOUSE_PS2_SYNAPTICS
 
+static const char *topbuttonpad_pnp_ids[] = {
+	"LEN0033", /* Helix */
+	"LEN0034", /* T540, X1 Carbon 2nd */
+	"LEN0035", /* X240 */
+	"LEN0036", /* T440 */
+	"LEN0042", /* Yoga */
+	"LEN2004", /* L440 */
+	NULL
+};
+
 /*****************************************************************************
  *	Synaptics communications functions
  ****************************************************************************/
@@ -1255,8 +1265,10 @@ static void set_abs_position_params(struct input_dev *dev,
 	input_abs_set_res(dev, y_code, priv->y_res);
 }
 
-static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
+static void set_input_params(struct psmouse *psmouse,
+			     struct synaptics_data *priv)
 {
+	struct input_dev *dev = psmouse->dev;
 	int i;
 
 	/* Things that apply to both modes */
@@ -1325,6 +1337,15 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 
 	if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
 		__set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
+		/* See if this buttonpad has a top button area */
+		for (i = 0; topbuttonpad_pnp_ids[i]; i++) {
+			if (compare_pnp_id(psmouse->ps2dev.serio->pnp_id,
+					   topbuttonpad_pnp_ids[i])) {
+				__set_bit(INPUT_PROP_TOPBUTTONPAD,
+					  dev->propbit);
+				break;
+			}
+		}
 		/* Clickpads report only left button */
 		__clear_bit(BTN_RIGHT, dev->keybit);
 		__clear_bit(BTN_MIDDLE, dev->keybit);
@@ -1593,7 +1614,7 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 		     priv->capabilities, priv->ext_cap, priv->ext_cap_0c,
 		     priv->board_id, priv->firmware_id);
 
-	set_input_params(psmouse->dev, priv);
+	set_input_params(psmouse, priv);
 
 	/*
 	 * Encode touchpad model so that it can be used to set
-- 
1.9.0