[PATCH 0/2] input: Add INPUT_PROP_POINTING_STICK property

[PATCH 0/2] input: Add INPUT_PROP_POINTING_STICK property

[Hans de Goede]

Hi,

It is useful for userspace to know that they're not dealing with a regular
mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
can e.g. automatically enable middle button scrollwheel emulation.

It is impossible to tell the difference from the evdev info without resorting
to putting a list of device / driver names in userspace, this is undesirable.

These patches add a property which allows userspace to see if a device is a
pointing stick, and set it on all the pointing stick drivers.

Note these patch apply on top of the current dtor/input.git for-linus
branch. Specifically they depend upon a2418fc4a13 (Input: elantech - add
support for trackpoint found on some v3 models).

Regards,

Hans

[PATCH 1/2] input: Add INPUT_PROP_POINTING_STICK property

[Hans de Goede]

It is useful for userspace to know that there not dealing with a regular
mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
can e.g. automatically enable middle button scrollwheel emulation.

It is impossible to tell the difference from the evdev info without resorting
to putting a list of device / driver names in userspace, this is undesirable.

Add a property which allows userspace to see if a device is a pointing
stick, and set it on all the pointing stick drivers.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c          | 3 +++
 drivers/input/mouse/elantech.c      | 3 +++
 drivers/input/mouse/synaptics_usb.c | 1 +
 drivers/input/mouse/trackpoint.c    | 2 ++
 include/uapi/linux/input.h          | 1 +
 5 files changed, 10 insertions(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index a59a1a6..9de3c25 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2373,6 +2373,9 @@ int alps_init(struct psmouse *psmouse)
 	dev2->keybit[BIT_WORD(BTN_LEFT)] =
 		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
 
+	if (priv->flags & ALPS_DUALPOINT)
+		__set_bit(INPUT_PROP_POINTING_STICK, dev2->propbit);
+
 	if (input_register_device(priv->dev2))
 		goto init_fail;
 
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 63533e5..c767f7e 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1607,6 +1607,9 @@ int elantech_init(struct psmouse *psmouse)
 		tp_dev->keybit[BIT_WORD(BTN_LEFT)] =
 			BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) |
 			BIT_MASK(BTN_RIGHT);
+
+		__set_bit(INPUT_PROP_POINTING_STICK, tp_dev->propbit);
+
 		error = input_register_device(etd->tp_dev);
 		if (error < 0)
 			goto init_fail_tp_reg;
diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
index e122bda..db3973d 100644
--- a/drivers/input/mouse/synaptics_usb.c
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -387,6 +387,7 @@ static int synusb_probe(struct usb_interface *intf,
 		__set_bit(EV_REL, input_dev->evbit);
 		__set_bit(REL_X, input_dev->relbit);
 		__set_bit(REL_Y, input_dev->relbit);
+		__set_bit(INPUT_PROP_POINTING_STICK, input_dev->propbit);
 		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 127, 0, 0);
 	} else {
 		input_set_abs_params(input_dev, ABS_X,
diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index ca843b6..b377462 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -393,6 +393,8 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
 	if ((button_info & 0x0f) >= 3)
 		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
 
+	__set_bit(INPUT_PROP_POINTING_STICK, psmouse->dev->propbit);
+
 	trackpoint_defaults(psmouse->private);
 
 	error = trackpoint_power_on_reset(&psmouse->ps2dev);
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 19df18c..1874ebe 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -165,6 +165,7 @@ struct input_keymap_entry {
 #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_POINTING_STICK	0x05	/* is a pointing stick */
 
 #define INPUT_PROP_MAX			0x1f
 #define INPUT_PROP_CNT			(INPUT_PROP_MAX + 1)
-- 
2.1.0

[PATCH 2/2] input/mouse: Add missing POINTER / DIRECT properties to a bunch of drivers

[Hans de Goede]

I've not done a full audit of all mouse drivers, I noticed these ones were
missing the POINTER property while working on the POINTING_STICK property.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c          | 1 +
 drivers/input/mouse/elantech.c      | 1 +
 drivers/input/mouse/psmouse-base.c  | 2 ++
 drivers/input/mouse/synaptics_usb.c | 5 +++++
 drivers/input/mouse/trackpoint.c    | 1 +
 5 files changed, 10 insertions(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 9de3c25..d7020ad 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2373,6 +2373,7 @@ int alps_init(struct psmouse *psmouse)
 	dev2->keybit[BIT_WORD(BTN_LEFT)] =
 		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
 
+	__set_bit(INPUT_PROP_POINTER, dev2->propbit);
 	if (priv->flags & ALPS_DUALPOINT)
 		__set_bit(INPUT_PROP_POINTING_STICK, dev2->propbit);
 
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index c767f7e..daaf82f 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1608,6 +1608,7 @@ int elantech_init(struct psmouse *psmouse)
 			BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) |
 			BIT_MASK(BTN_RIGHT);
 
+		__set_bit(INPUT_PROP_POINTER, tp_dev->propbit);
 		__set_bit(INPUT_PROP_POINTING_STICK, tp_dev->propbit);
 
 		error = input_register_device(etd->tp_dev);
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 5c1237d..a44d655 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -685,6 +685,8 @@ static void psmouse_apply_defaults(struct psmouse *psmouse)
 	__set_bit(REL_X, input_dev->relbit);
 	__set_bit(REL_Y, input_dev->relbit);
 
+	__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
+
 	psmouse->set_rate = psmouse_set_rate;
 	psmouse->set_resolution = psmouse_set_resolution;
 	psmouse->poll = psmouse_poll;
diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
index db3973d..6bcc018 100644
--- a/drivers/input/mouse/synaptics_usb.c
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -402,6 +402,11 @@ static int synusb_probe(struct usb_interface *intf,
 		__set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
 	}
 
+	if (synusb->flags & SYNUSB_TOUCHSCREEN)
+		__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
+	else
+		__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
+
 	__set_bit(BTN_LEFT, input_dev->keybit);
 	__set_bit(BTN_RIGHT, input_dev->keybit);
 	__set_bit(BTN_MIDDLE, input_dev->keybit);
diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index b377462..30c8b69 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -393,6 +393,7 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
 	if ((button_info & 0x0f) >= 3)
 		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
 
+	__set_bit(INPUT_PROP_POINTER, psmouse->dev->propbit);
 	__set_bit(INPUT_PROP_POINTING_STICK, psmouse->dev->propbit);
 
 	trackpoint_defaults(psmouse->private);
-- 
2.1.0

Re: [PATCH 0/2] input: Add INPUT_PROP_POINTING_STICK property

[David Herrmann]

Hi

On Tue, Sep 2, 2014 at 2:43 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> It is useful for userspace to know that they're not dealing with a regular
> mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
> can e.g. automatically enable middle button scrollwheel emulation.
>
> It is impossible to tell the difference from the evdev info without resorting
> to putting a list of device / driver names in userspace, this is undesirable.

..so it is better to put that table into the kernel?

I thought the plan was to avoid putting more hardware tables into the
kernel. All those tables are always loaded into RAM, whereas solutions
like udev hwdb can provide the exact same features in user-space but
avoid loading it into RAM, except if used. hwdb is pretty nice to
store information about hardware. It is not meant for configuration or
volatile data, but rather as read-only lookup table for hardware
information.

You could easily extend the hwdb with a trackpoint variable that marks
specific devices as trackpoints.

Anyhow, if you guys want it in the kernel, go ahead. I just don't see
the advantage of having it in kernel-space.

Thanks
David

Re: [PATCH 0/2] input: Add INPUT_PROP_POINTING_STICK property

[Benjamin Tissoires]

Hi David,

On Sep 02 2014 or thereabouts, David Herrmann wrote:
> Hi
> 
> On Tue, Sep 2, 2014 at 2:43 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Hi,
> >
> > It is useful for userspace to know that they're not dealing with a regular
> > mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
> > can e.g. automatically enable middle button scrollwheel emulation.
> >
> > It is impossible to tell the difference from the evdev info without resorting
> > to putting a list of device / driver names in userspace, this is undesirable.
> 
> ..so it is better to put that table into the kernel?
> 
> I thought the plan was to avoid putting more hardware tables into the
> kernel. All those tables are always loaded into RAM, whereas solutions
> like udev hwdb can provide the exact same features in user-space but
> avoid loading it into RAM, except if used. hwdb is pretty nice to
> store information about hardware. It is not meant for configuration or
> volatile data, but rather as read-only lookup table for hardware
> information.

I think you misread Hans in this case. Hans stated:
- if this is not provided by the kernel, we have to create a user space
  table with all old and new future devices
- if we put this in the kernel, then we only have to add this property
  to the few generic drivers which present this feature to the user
  space, and done. See patch 1/2. There is no table in the kernel.

> 
> You could easily extend the hwdb with a trackpoint variable that marks
> specific devices as trackpoints.

I don't think this is reasonable. You will have to review all laptops
with trackstick to add them into this (huge) db.

> 
> Anyhow, if you guys want it in the kernel, go ahead. I just don't see
> the advantage of having it in kernel-space.
> 

Again, it's either 10 lines in the kernel, and we forget about it, or we
have to carry a db which has to be updated with every single different
laptop model available. I am definitively in favor of those 10 lines.

Cheers,
Benjamin

Re: [PATCH 1/2] input: Add INPUT_PROP_POINTING_STICK property

[Benjamin Tissoires]

On Sep 02 2014 or thereabouts, Hans de Goede wrote:
> It is useful for userspace to know that there not dealing with a regular
> mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
> can e.g. automatically enable middle button scrollwheel emulation.
> 
> It is impossible to tell the difference from the evdev info without resorting
> to putting a list of device / driver names in userspace, this is undesirable.
> 
> Add a property which allows userspace to see if a device is a pointing
> stick, and set it on all the pointing stick drivers.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---

This one is Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

>  drivers/input/mouse/alps.c          | 3 +++
>  drivers/input/mouse/elantech.c      | 3 +++
>  drivers/input/mouse/synaptics_usb.c | 1 +
>  drivers/input/mouse/trackpoint.c    | 2 ++
>  include/uapi/linux/input.h          | 1 +
>  5 files changed, 10 insertions(+)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index a59a1a6..9de3c25 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2373,6 +2373,9 @@ int alps_init(struct psmouse *psmouse)
>  	dev2->keybit[BIT_WORD(BTN_LEFT)] =
>  		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
>  
> +	if (priv->flags & ALPS_DUALPOINT)
> +		__set_bit(INPUT_PROP_POINTING_STICK, dev2->propbit);
> +
>  	if (input_register_device(priv->dev2))
>  		goto init_fail;
>  
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 63533e5..c767f7e 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1607,6 +1607,9 @@ int elantech_init(struct psmouse *psmouse)
>  		tp_dev->keybit[BIT_WORD(BTN_LEFT)] =
>  			BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) |
>  			BIT_MASK(BTN_RIGHT);
> +
> +		__set_bit(INPUT_PROP_POINTING_STICK, tp_dev->propbit);
> +
>  		error = input_register_device(etd->tp_dev);
>  		if (error < 0)
>  			goto init_fail_tp_reg;
> diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
> index e122bda..db3973d 100644
> --- a/drivers/input/mouse/synaptics_usb.c
> +++ b/drivers/input/mouse/synaptics_usb.c
> @@ -387,6 +387,7 @@ static int synusb_probe(struct usb_interface *intf,
>  		__set_bit(EV_REL, input_dev->evbit);
>  		__set_bit(REL_X, input_dev->relbit);
>  		__set_bit(REL_Y, input_dev->relbit);
> +		__set_bit(INPUT_PROP_POINTING_STICK, input_dev->propbit);
>  		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 127, 0, 0);
>  	} else {
>  		input_set_abs_params(input_dev, ABS_X,
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index ca843b6..b377462 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -393,6 +393,8 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>  	if ((button_info & 0x0f) >= 3)
>  		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
>  
> +	__set_bit(INPUT_PROP_POINTING_STICK, psmouse->dev->propbit);
> +
>  	trackpoint_defaults(psmouse->private);
>  
>  	error = trackpoint_power_on_reset(&psmouse->ps2dev);
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 19df18c..1874ebe 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -165,6 +165,7 @@ struct input_keymap_entry {
>  #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_POINTING_STICK	0x05	/* is a pointing stick */
>  
>  #define INPUT_PROP_MAX			0x1f
>  #define INPUT_PROP_CNT			(INPUT_PROP_MAX + 1)
> -- 
> 2.1.0
> 

Re: [PATCH 2/2] input/mouse: Add missing POINTER / DIRECT properties to a bunch of drivers

[Benjamin Tissoires]

On Sep 02 2014 or thereabouts, Hans de Goede wrote:
> I've not done a full audit of all mouse drivers, I noticed these ones were
> missing the POINTER property while working on the POINTING_STICK property.

I am not sure about this one. INPUT_PROP_POINTER is the default behavior
when no properties are set. The current Xorg/libinput code should
already be aware of that and uses this to be able to be backward
compatible.
Adding the info now, will not change anything in the user-space code and
we will not be able to remove the compat code before a long time.

So, really, I am not in favor, nor I will opposite my vote regarding
this patch.

Cheers,
Benjamin

> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/input/mouse/alps.c          | 1 +
>  drivers/input/mouse/elantech.c      | 1 +
>  drivers/input/mouse/psmouse-base.c  | 2 ++
>  drivers/input/mouse/synaptics_usb.c | 5 +++++
>  drivers/input/mouse/trackpoint.c    | 1 +
>  5 files changed, 10 insertions(+)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 9de3c25..d7020ad 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2373,6 +2373,7 @@ int alps_init(struct psmouse *psmouse)
>  	dev2->keybit[BIT_WORD(BTN_LEFT)] =
>  		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
>  
> +	__set_bit(INPUT_PROP_POINTER, dev2->propbit);
>  	if (priv->flags & ALPS_DUALPOINT)
>  		__set_bit(INPUT_PROP_POINTING_STICK, dev2->propbit);
>  
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index c767f7e..daaf82f 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1608,6 +1608,7 @@ int elantech_init(struct psmouse *psmouse)
>  			BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) |
>  			BIT_MASK(BTN_RIGHT);
>  
> +		__set_bit(INPUT_PROP_POINTER, tp_dev->propbit);
>  		__set_bit(INPUT_PROP_POINTING_STICK, tp_dev->propbit);
>  
>  		error = input_register_device(etd->tp_dev);
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index 5c1237d..a44d655 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -685,6 +685,8 @@ static void psmouse_apply_defaults(struct psmouse *psmouse)
>  	__set_bit(REL_X, input_dev->relbit);
>  	__set_bit(REL_Y, input_dev->relbit);
>  
> +	__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> +
>  	psmouse->set_rate = psmouse_set_rate;
>  	psmouse->set_resolution = psmouse_set_resolution;
>  	psmouse->poll = psmouse_poll;
> diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
> index db3973d..6bcc018 100644
> --- a/drivers/input/mouse/synaptics_usb.c
> +++ b/drivers/input/mouse/synaptics_usb.c
> @@ -402,6 +402,11 @@ static int synusb_probe(struct usb_interface *intf,
>  		__set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
>  	}
>  
> +	if (synusb->flags & SYNUSB_TOUCHSCREEN)
> +		__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
> +	else
> +		__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> +
>  	__set_bit(BTN_LEFT, input_dev->keybit);
>  	__set_bit(BTN_RIGHT, input_dev->keybit);
>  	__set_bit(BTN_MIDDLE, input_dev->keybit);
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index b377462..30c8b69 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -393,6 +393,7 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>  	if ((button_info & 0x0f) >= 3)
>  		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
>  
> +	__set_bit(INPUT_PROP_POINTER, psmouse->dev->propbit);
>  	__set_bit(INPUT_PROP_POINTING_STICK, psmouse->dev->propbit);
>  
>  	trackpoint_defaults(psmouse->private);
> -- 
> 2.1.0
> 

Re: [PATCH 0/2] input: Add INPUT_PROP_POINTING_STICK property

[David Herrmann]

Hi

On Tue, Sep 2, 2014 at 4:40 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> On 09/02/2014 02:55 PM, David Herrmann wrote:
>> Hi
>>
>> On Tue, Sep 2, 2014 at 2:43 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> It is useful for userspace to know that they're not dealing with a regular
>>> mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
>>> can e.g. automatically enable middle button scrollwheel emulation.
>>>
>>> It is impossible to tell the difference from the evdev info without resorting
>>> to putting a list of device / driver names in userspace, this is undesirable.
>>
>> ..so it is better to put that table into the kernel?
>
> No not a table, at the kernel level we actually have different drivers
> for pointer sticks and for other mouse (and mouse like devices), so this is
> a simple matter of adding a single line of code to all of 4 drivers.
>
> We could have a table in userspace matching the driver/model strings used
> by those 4 drivers, but what if a 5th pops up ?

Ok, for now it's pretty easy as all "pointing stick" drivers are
already separate in the kernel. But imagine a new generation of
"pointing stick" devices is an HID device handled by hid-input.c. What
do you do? To set the INPUT_PROPERTY bit, you need to add a driver for
this vid/did combination. Seems overkill, so we'd probably not set
this property bit for this device and let user-space deal with it. Now
we end up with two places to store such information.

We already fix scancode/keycode mappings via hwdb as it would be
excessive to write kernel drivers for all devices just to fix those.
Imo, device properties are very similar to this. We have to draw a
line between properties the kernel should detect and expose, and
properties we detect in user-space. We already gave up defining ABS_
axes for everything (I mean, we report all kinds of data via ABS_X,
ranging from trackpad, to pressure and accelerometer data). User-space
needs to detect what devices it deals with, to know what the different
axes event-bits mean.

Yeah, the POINTING_STICK property turns out to be trivial, so please
go ahead. I just want to remind you, that we need a user-space
database, anyway. And hwdb entries can be added via simple distro
updates (even users can regenerate it themselves). Kernel driver
changes are way more heavy.. and slow.

Thanks
David

Re: [PATCH 1/2] input: Add INPUT_PROP_POINTING_STICK property

[Peter Hutterer]

On Tue, Sep 02, 2014 at 02:43:23PM +0200, Hans de Goede wrote:
> It is useful for userspace to know that there not dealing with a regular
> mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
> can e.g. automatically enable middle button scrollwheel emulation.
> 
> It is impossible to tell the difference from the evdev info without resorting
> to putting a list of device / driver names in userspace, this is undesirable.
> 
> Add a property which allows userspace to see if a device is a pointing
> stick, and set it on all the pointing stick drivers.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Peter Hutterer <peter.hutterer@who-t.net>

Cheers,
   Peter

> ---
>  drivers/input/mouse/alps.c          | 3 +++
>  drivers/input/mouse/elantech.c      | 3 +++
>  drivers/input/mouse/synaptics_usb.c | 1 +
>  drivers/input/mouse/trackpoint.c    | 2 ++
>  include/uapi/linux/input.h          | 1 +
>  5 files changed, 10 insertions(+)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index a59a1a6..9de3c25 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2373,6 +2373,9 @@ int alps_init(struct psmouse *psmouse)
>  	dev2->keybit[BIT_WORD(BTN_LEFT)] =
>  		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
>  
> +	if (priv->flags & ALPS_DUALPOINT)
> +		__set_bit(INPUT_PROP_POINTING_STICK, dev2->propbit);
> +
>  	if (input_register_device(priv->dev2))
>  		goto init_fail;
>  
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 63533e5..c767f7e 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1607,6 +1607,9 @@ int elantech_init(struct psmouse *psmouse)
>  		tp_dev->keybit[BIT_WORD(BTN_LEFT)] =
>  			BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) |
>  			BIT_MASK(BTN_RIGHT);
> +
> +		__set_bit(INPUT_PROP_POINTING_STICK, tp_dev->propbit);
> +
>  		error = input_register_device(etd->tp_dev);
>  		if (error < 0)
>  			goto init_fail_tp_reg;
> diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
> index e122bda..db3973d 100644
> --- a/drivers/input/mouse/synaptics_usb.c
> +++ b/drivers/input/mouse/synaptics_usb.c
> @@ -387,6 +387,7 @@ static int synusb_probe(struct usb_interface *intf,
>  		__set_bit(EV_REL, input_dev->evbit);
>  		__set_bit(REL_X, input_dev->relbit);
>  		__set_bit(REL_Y, input_dev->relbit);
> +		__set_bit(INPUT_PROP_POINTING_STICK, input_dev->propbit);
>  		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 127, 0, 0);
>  	} else {
>  		input_set_abs_params(input_dev, ABS_X,
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index ca843b6..b377462 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -393,6 +393,8 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>  	if ((button_info & 0x0f) >= 3)
>  		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
>  
> +	__set_bit(INPUT_PROP_POINTING_STICK, psmouse->dev->propbit);
> +
>  	trackpoint_defaults(psmouse->private);
>  
>  	error = trackpoint_power_on_reset(&psmouse->ps2dev);
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 19df18c..1874ebe 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -165,6 +165,7 @@ struct input_keymap_entry {
>  #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_POINTING_STICK	0x05	/* is a pointing stick */
>  
>  #define INPUT_PROP_MAX			0x1f
>  #define INPUT_PROP_CNT			(INPUT_PROP_MAX + 1)
> -- 
> 2.1.0

Re: [PATCH 2/2] input/mouse: Add missing POINTER / DIRECT properties to a bunch of drivers

[Peter Hutterer]

On Tue, Sep 02, 2014 at 10:38:34AM -0400, Benjamin Tissoires wrote:
> On Sep 02 2014 or thereabouts, Hans de Goede wrote:
> > I've not done a full audit of all mouse drivers, I noticed these ones were
> > missing the POINTER property while working on the POINTING_STICK property.
> 
> I am not sure about this one. INPUT_PROP_POINTER is the default behavior
> when no properties are set. The current Xorg/libinput code should
> already be aware of that and uses this to be able to be backward
> compatible.
> Adding the info now, will not change anything in the user-space code and
> we will not be able to remove the compat code before a long time.
> 
> So, really, I am not in favor, nor I will opposite my vote regarding
> this patch.

If we could reliably use the property in userspace it would make detecting
things like joysticks or accelerometers a lot easier. So setting it where we
know it's a pointer device may be helpful.

Cheers,
   Peter

> > 
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> >  drivers/input/mouse/alps.c          | 1 +
> >  drivers/input/mouse/elantech.c      | 1 +
> >  drivers/input/mouse/psmouse-base.c  | 2 ++
> >  drivers/input/mouse/synaptics_usb.c | 5 +++++
> >  drivers/input/mouse/trackpoint.c    | 1 +
> >  5 files changed, 10 insertions(+)
> > 
> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> > index 9de3c25..d7020ad 100644
> > --- a/drivers/input/mouse/alps.c
> > +++ b/drivers/input/mouse/alps.c
> > @@ -2373,6 +2373,7 @@ int alps_init(struct psmouse *psmouse)
> >  	dev2->keybit[BIT_WORD(BTN_LEFT)] =
> >  		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
> >  
> > +	__set_bit(INPUT_PROP_POINTER, dev2->propbit);
> >  	if (priv->flags & ALPS_DUALPOINT)
> >  		__set_bit(INPUT_PROP_POINTING_STICK, dev2->propbit);
> >  
> > diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> > index c767f7e..daaf82f 100644
> > --- a/drivers/input/mouse/elantech.c
> > +++ b/drivers/input/mouse/elantech.c
> > @@ -1608,6 +1608,7 @@ int elantech_init(struct psmouse *psmouse)
> >  			BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) |
> >  			BIT_MASK(BTN_RIGHT);
> >  
> > +		__set_bit(INPUT_PROP_POINTER, tp_dev->propbit);
> >  		__set_bit(INPUT_PROP_POINTING_STICK, tp_dev->propbit);
> >  
> >  		error = input_register_device(etd->tp_dev);
> > diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> > index 5c1237d..a44d655 100644
> > --- a/drivers/input/mouse/psmouse-base.c
> > +++ b/drivers/input/mouse/psmouse-base.c
> > @@ -685,6 +685,8 @@ static void psmouse_apply_defaults(struct psmouse *psmouse)
> >  	__set_bit(REL_X, input_dev->relbit);
> >  	__set_bit(REL_Y, input_dev->relbit);
> >  
> > +	__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> > +
> >  	psmouse->set_rate = psmouse_set_rate;
> >  	psmouse->set_resolution = psmouse_set_resolution;
> >  	psmouse->poll = psmouse_poll;
> > diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
> > index db3973d..6bcc018 100644
> > --- a/drivers/input/mouse/synaptics_usb.c
> > +++ b/drivers/input/mouse/synaptics_usb.c
> > @@ -402,6 +402,11 @@ static int synusb_probe(struct usb_interface *intf,
> >  		__set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
> >  	}
> >  
> > +	if (synusb->flags & SYNUSB_TOUCHSCREEN)
> > +		__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
> > +	else
> > +		__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> > +
> >  	__set_bit(BTN_LEFT, input_dev->keybit);
> >  	__set_bit(BTN_RIGHT, input_dev->keybit);
> >  	__set_bit(BTN_MIDDLE, input_dev->keybit);
> > diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> > index b377462..30c8b69 100644
> > --- a/drivers/input/mouse/trackpoint.c
> > +++ b/drivers/input/mouse/trackpoint.c
> > @@ -393,6 +393,7 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> >  	if ((button_info & 0x0f) >= 3)
> >  		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
> >  
> > +	__set_bit(INPUT_PROP_POINTER, psmouse->dev->propbit);
> >  	__set_bit(INPUT_PROP_POINTING_STICK, psmouse->dev->propbit);
> >  
> >  	trackpoint_defaults(psmouse->private);
> > -- 
> > 2.1.0
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 0/2] input: Add INPUT_PROP_POINTING_STICK property

[Peter Hutterer]

On Tue, Sep 02, 2014 at 04:50:53PM +0200, David Herrmann wrote:
> Hi
> 
> On Tue, Sep 2, 2014 at 4:40 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> > On 09/02/2014 02:55 PM, David Herrmann wrote:
> >> Hi
> >>
> >> On Tue, Sep 2, 2014 at 2:43 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> >>> Hi,
> >>>
> >>> It is useful for userspace to know that they're not dealing with a regular
> >>> mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
> >>> can e.g. automatically enable middle button scrollwheel emulation.
> >>>
> >>> It is impossible to tell the difference from the evdev info without resorting
> >>> to putting a list of device / driver names in userspace, this is undesirable.
> >>
> >> ..so it is better to put that table into the kernel?
> >
> > No not a table, at the kernel level we actually have different drivers
> > for pointer sticks and for other mouse (and mouse like devices), so this is
> > a simple matter of adding a single line of code to all of 4 drivers.
> >
> > We could have a table in userspace matching the driver/model strings used
> > by those 4 drivers, but what if a 5th pops up ?
> 
> Ok, for now it's pretty easy as all "pointing stick" drivers are
> already separate in the kernel. But imagine a new generation of
> "pointing stick" devices is an HID device handled by hid-input.c. What
> do you do? To set the INPUT_PROPERTY bit, you need to add a driver for
> this vid/did combination. Seems overkill, so we'd probably not set
> this property bit for this device and let user-space deal with it. Now
> we end up with two places to store such information.

right now, there's no ioctl to set a property on a device. that'd be a
requirement for a udev hwdb-like thing, unless you want clients to rely on
pulling the information from two places and merging it in some strategy
that's not well defined yet.

I'm all for a userspace hwdb, but this is some bigger project that afaik
hasn't been worked on beyond the libinputmapper discussions we had a year or
so ago.

Cheers,
   Peter

> We already fix scancode/keycode mappings via hwdb as it would be
> excessive to write kernel drivers for all devices just to fix those.
> Imo, device properties are very similar to this. We have to draw a
> line between properties the kernel should detect and expose, and
> properties we detect in user-space. We already gave up defining ABS_
> axes for everything (I mean, we report all kinds of data via ABS_X,
> ranging from trackpad, to pressure and accelerometer data). User-space
> needs to detect what devices it deals with, to know what the different
> axes event-bits mean.
> 
> Yeah, the POINTING_STICK property turns out to be trivial, so please
> go ahead. I just want to remind you, that we need a user-space
> database, anyway. And hwdb entries can be added via simple distro
> updates (even users can regenerate it themselves). Kernel driver
> changes are way more heavy.. and slow.
> 
> Thanks
> David
> --

Re: [PATCH 0/2] input: Add INPUT_PROP_POINTING_STICK property

[Dmitry Torokhov]

On Tue, Sep 02, 2014 at 02:43:22PM +0200, Hans de Goede wrote:
> Hi,
> 
> It is useful for userspace to know that they're not dealing with a regular
> mouse but rather with a pointing stick (e.g. a trackpoint) so that userspace
> can e.g. automatically enable middle button scrollwheel emulation.
> 
> It is impossible to tell the difference from the evdev info without resorting
> to putting a list of device / driver names in userspace, this is undesirable.
> 
> These patches add a property which allows userspace to see if a device is a
> pointing stick, and set it on all the pointing stick drivers.
> 
> Note these patch apply on top of the current dtor/input.git for-linus
> branch. Specifically they depend upon a2418fc4a13 (Input: elantech - add
> support for trackpoint found on some v3 models).

Applied both, thank you.

-- 
Dmitry