Today, userspace sets up an input device based on the data it emits.
This is not always enough; a tablet and a touchscreen may emit exactly
the same data, for instance, but the former should be set up with a
pointer whereas the latter does not need to. Recently, a new type of
touchpad has emerged where the buttons are under the pad, which changes
handling logic without changing the emitted data. This patch introduces
a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
about the device resulting in proper setup.
Suggested-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Henrik Rydberg <[email protected]>
Cc: Ping Cheng <[email protected]>
Cc: Chris Bagwell <[email protected]>
---
Hi all,
Here is a patch attempting to formulate Dmitry's device type idea. It
compiles, but further testing awaits a general consesus on the device
types and capabilities to start out with. Are the ones listed here apt
for the job?
Cheers,
Henrik
drivers/input/evdev.c | 6 ++++++
include/linux/input.h | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index e3f7fc6..db78592 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -632,6 +632,12 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
return -EFAULT;
return 0;
+ case EVIOCGDEVINFO:
+ if (copy_to_user(p, &dev->devinfo,
+ sizeof(struct input_devinfo)))
+ return -EFAULT;
+ return 0;
+
case EVIOCGREP:
if (!test_bit(EV_REP, dev->evbit))
return -ENOSYS;
diff --git a/include/linux/input.h b/include/linux/input.h
index 6ef4446..8c58d2a 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -57,6 +57,21 @@ struct input_absinfo {
};
/**
+ * struct input_devinfo - device information via EVIOCGDEVINFO ioctl
+ * @types: bitmask of types (DEVTYPE_*) matching this device
+ * @capabilities: bitmask of capabilities (DEVCAPS_*) of this device
+ *
+ * This struct provides information about the device needed for
+ * automatic setup in userspace, such as if the device is direct
+ * (touchscreen) or indirect (touchpad), and if there are other
+ * special considerations, such as the touchpad also being a button.
+ */
+struct input_devinfo {
+ __u32 types;
+ __u32 capabilities;
+};
+
+/**
* struct input_keymap_entry - used by EVIOCGKEYCODE/EVIOCSKEYCODE ioctls
* @scancode: scancode represented in machine-endian form.
* @len: length of the scancode that resides in @scancode buffer.
@@ -91,6 +106,7 @@ struct input_keymap_entry {
#define EVIOCGNAME(len) _IOC(_IOC_READ, 'E', 0x06, len) /* get device name */
#define EVIOCGPHYS(len) _IOC(_IOC_READ, 'E', 0x07, len) /* get physical location */
#define EVIOCGUNIQ(len) _IOC(_IOC_READ, 'E', 0x08, len) /* get unique identifier */
+#define EVIOCGDEVINFO _IOR('E', 0x09, struct input_devinfo) /* get device information */
#define EVIOCGKEY(len) _IOC(_IOC_READ, 'E', 0x18, len) /* get global key state */
#define EVIOCGLED(len) _IOC(_IOC_READ, 'E', 0x19, len) /* get all LEDs */
@@ -108,6 +124,23 @@ struct input_keymap_entry {
#define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */
/*
+ * Device types
+ */
+
+#define DEVTYPE_KEYBOARD 0
+#define DEVTYPE_MOUSE 1
+#define DEVTYPE_JOYSTICK 2
+#define DEVTYPE_TOUCHPAD 3
+#define DEVTYPE_TABLET 4
+#define DEVTYPE_TOUCHSCREEN 5
+
+/*
+ * Device capabilities
+ */
+
+#define DEVCAPS_PAD_IS_BUTTON 1
+
+/*
* Event types
*/
@@ -1177,6 +1210,7 @@ struct input_dev {
const char *phys;
const char *uniq;
struct input_id id;
+ struct input_devinfo devinfo;
unsigned long evbit[BITS_TO_LONGS(EV_CNT)];
unsigned long keybit[BITS_TO_LONGS(KEY_CNT)];
--
1.7.1
Hi Henrik,
On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
> Today, userspace sets up an input device based on the data it emits.
> This is not always enough; a tablet and a touchscreen may emit exactly
> the same data, for instance, but the former should be set up with a
> pointer whereas the latter does not need to. Recently, a new type of
> touchpad has emerged where the buttons are under the pad, which changes
> handling logic without changing the emitted data. This patch introduces
> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> about the device resulting in proper setup.
If we agree that the new ioctl is suitable we'llalso need to wireit up
through sysfs. Also, can we keep all definitions to INPUT_ namespace?
Thanks.
--
Dmitry
On Tue, Dec 7, 2010 at 10:16, Dmitry Torokhov <[email protected]> wrote:
> Hi Henrik,
>
> On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
>> Today, userspace sets up an input device based on the data it emits.
>> This is not always enough; a tablet and a touchscreen may emit exactly
>> the same data, for instance, but the former should be set up with a
>> pointer whereas the latter does not need to. Recently, a new type of
>> touchpad has emerged where the buttons are under the pad, which changes
>> handling logic without changing the emitted data. This patch introduces
>> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
>> about the device resulting in proper setup.
>
> If we agree that the new ioctl is suitable we'llalso need to wireit up
> through sysfs. Also, can we keep all definitions to INPUT_ namespace?
Please don't add new ioctls which are not extensible. The ioctl should
carry the length or the version of the structure it asks for, so it
can be extended in the future. Sysfs should be good enough for such
interface though.
Kay
On Tue, Dec 07, 2010 at 11:48:28AM +0100, Kay Sievers wrote:
> On Tue, Dec 7, 2010 at 10:16, Dmitry Torokhov <[email protected]> wrote:
> > Hi Henrik,
> >
> > On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
> >> Today, userspace sets up an input device based on the data it emits.
> >> This is not always enough; a tablet and a touchscreen may emit exactly
> >> the same data, for instance, but the former should be set up with a
> >> pointer whereas the latter does not need to. Recently, a new type of
> >> touchpad has emerged where the buttons are under the pad, which changes
> >> handling logic without changing the emitted data. This patch introduces
> >> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> >> about the device resulting in proper setup.
> >
> > If we agree that the new ioctl is suitable we'llalso need to wireit up
> > through sysfs. Also, can we keep all definitions to INPUT_ namespace?
>
> Please don't add new ioctls which are not extensible. The ioctl should
> carry the length or the version of the structure it asks for, so it
> can be extended in the future.
Size of ioctl data is encoded in ioctl, it can be extended easily. For
examples take a look at how EVIOCGKEYCODE and EVIOCGSKEYCODE are handled
in recent kernels.
> Sysfs should be good enough for such
> interface though.
>
--
Dmitry
On Tue, Dec 7, 2010 at 11:56, Dmitry Torokhov <[email protected]> wrote:
> On Tue, Dec 07, 2010 at 11:48:28AM +0100, Kay Sievers wrote:
>> Please don't add new ioctls which are not extensible. The ioctl should
>> carry the length or the version of the structure it asks for, so it
>> can be extended in the future.
>
> Size of ioctl data is encoded in ioctl, it can be extended easily. For
> examples take a look at how EVIOCGKEYCODE and EVIOCGSKEYCODE are handled
> in recent kernels.
Oh, how does that work? With the ioctl call, userspace has to supply
the size it expects to be returned from the kernel. How does the
kernel otherwise know how much it is allowed to copy to the user?
Kay
On Tuesday 07 December 2010, Kay Sievers wrote:
> On Tue, Dec 7, 2010 at 10:16, Dmitry Torokhov <[email protected]> wrote:
> > Hi Henrik,
> >
> > On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
> >> Today, userspace sets up an input device based on the data it emits.
> >> This is not always enough; a tablet and a touchscreen may emit exactly
> >> the same data, for instance, but the former should be set up with a
> >> pointer whereas the latter does not need to. Recently, a new type of
> >> touchpad has emerged where the buttons are under the pad, which changes
> >> handling logic without changing the emitted data. This patch introduces
> >> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> >> about the device resulting in proper setup.
> >
> > If we agree that the new ioctl is suitable we'llalso need to wireit up
> > through sysfs. Also, can we keep all definitions to INPUT_ namespace?
>
> Please don't add new ioctls which are not extensible. The ioctl should
> carry the length or the version of the structure it asks for, so it
> can be extended in the future. Sysfs should be good enough for such
> interface though.
Please never add any ioctls that have a version or length field!
Ideally ioctls should have only scalar arguments, not structures,
so I'd recommend splitting it into two, so you can read type and
capability parameters separately.
Arnd
On Tuesday 07 December 2010, Kay Sievers wrote:
> On Tue, Dec 7, 2010 at 11:56, Dmitry Torokhov <[email protected]> wrote:
> > On Tue, Dec 07, 2010 at 11:48:28AM +0100, Kay Sievers wrote:
>
> >> Please don't add new ioctls which are not extensible. The ioctl should
> >> carry the length or the version of the structure it asks for, so it
> >> can be extended in the future.
> >
> > Size of ioctl data is encoded in ioctl, it can be extended easily. For
> > examples take a look at how EVIOCGKEYCODE and EVIOCGSKEYCODE are handled
> > in recent kernels.
>
> Oh, how does that work? With the ioctl call, userspace has to supply
> the size it expects to be returned from the kernel. How does the
> kernel otherwise know how much it is allowed to copy to the user?
The ioctl command number itself is calculated from the size of the
data that gets passed:
#define EVIOCGDEVINFO _IOR('E', 0x09, struct input_devinfo)
If struct input_devinfo ever changes (which it can, but should not),
the command changes as well.
Arnd
On Tue, Dec 7, 2010 at 13:40, Arnd Bergmann <[email protected]> wrote:
> On Tuesday 07 December 2010, Kay Sievers wrote:
>> On Tue, Dec 7, 2010 at 10:16, Dmitry Torokhov <[email protected]> wrote:
>> > Hi Henrik,
>> >
>> > On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
>> >> Today, userspace sets up an input device based on the data it emits.
>> >> This is not always enough; a tablet and a touchscreen may emit exactly
>> >> the same data, for instance, but the former should be set up with a
>> >> pointer whereas the latter does not need to. Recently, a new type of
>> >> touchpad has emerged where the buttons are under the pad, which changes
>> >> handling logic without changing the emitted data. This patch introduces
>> >> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
>> >> about the device resulting in proper setup.
>> >
>> > If we agree that the new ioctl is suitable we'llalso need to wireit up
>> > through sysfs. Also, can we keep all definitions to INPUT_ namespace?
>>
>> Please don't add new ioctls which are not extensible. The ioctl should
>> carry the length or the version of the structure it asks for, so it
>> can be extended in the future. Sysfs should be good enough for such
>> interface though.
>
> Please never add any ioctls that have a version or length field!
>
> Ideally ioctls should have only scalar arguments, not structures,
> so I'd recommend splitting it into two, so you can read type and
> capability parameters separately.
Please don't add new ioctls with a single value only. :) That's what
sysfs is for, today.
The problem is that sysfs or ioctls with a single value can not handle
atomic queries of multiple values, and (rightfully) needing that in
some cases is a reason to add an ioctl() instead of sysfs.
Kay
On Tue, Dec 7, 2010 at 13:44, Arnd Bergmann <[email protected]> wrote:
> On Tuesday 07 December 2010, Kay Sievers wrote:
>> On Tue, Dec 7, 2010 at 11:56, Dmitry Torokhov <[email protected]> wrote:
>> > On Tue, Dec 07, 2010 at 11:48:28AM +0100, Kay Sievers wrote:
>>
>> >> Please don't add new ioctls which are not extensible. The ioctl should
>> >> carry the length or the version of the structure it asks for, so it
>> >> can be extended in the future.
>> >
>> > Size of ioctl data is encoded in ioctl, it can be extended easily. For
>> > examples take a look at how EVIOCGKEYCODE and EVIOCGSKEYCODE are handled
>> > in recent kernels.
>>
>> Oh, how does that work? With the ioctl call, userspace has to supply
>> the size it expects to be returned from the kernel. How does the
>> kernel otherwise know how much it is allowed to copy to the user?
>
> The ioctl command number itself is calculated from the size of the
> data that gets passed:
>
> #define EVIOCGDEVINFO _IOR('E', 0x09, struct input_devinfo)
>
> If struct input_devinfo ever changes (which it can, but should not),
> the command changes as well.
So unlike statet, it's not extensible, and this struct and this ioctl
can never change?
Kay
On Tuesday 07 December 2010, Kay Sievers wrote:
> >> Oh, how does that work? With the ioctl call, userspace has to supply
> >> the size it expects to be returned from the kernel. How does the
> >> kernel otherwise know how much it is allowed to copy to the user?
> >
> > The ioctl command number itself is calculated from the size of the
> > data that gets passed:
> >
> > #define EVIOCGDEVINFO _IOR('E', 0x09, struct input_devinfo)
> >
> > If struct input_devinfo ever changes (which it can, but should not),
> > the command changes as well.
>
> So unlike statet, it's not extensible, and this struct and this ioctl
> can never change?
Exactly. We have plenty of unused ioctl numbers free though.
Arnd
On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
> /**
> + * struct input_devinfo - device information via EVIOCGDEVINFO ioctl
> + * @types: bitmask of types (DEVTYPE_*) matching this device
> + * @capabilities: bitmask of capabilities (DEVCAPS_*) of this device
> + *
> + * This struct provides information about the device needed for
> + * automatic setup in userspace, such as if the device is direct
> + * (touchscreen) or indirect (touchpad), and if there are other
> + * special considerations, such as the touchpad also being a button.
> + */
> +struct input_devinfo {
> + __u32 types;
> + __u32 capabilities;
> +};
Why use an ioctl for this at all? It's just 2 simple values that don't
need to be atomically read at the same time. What's wrong with 2 more
sysfs files?
thanks,
greg k-h
On Tue, Dec 07, 2010 at 08:22:22AM -0800, Greg KH wrote:
> On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
> > /**
> > + * struct input_devinfo - device information via EVIOCGDEVINFO ioctl
> > + * @types: bitmask of types (DEVTYPE_*) matching this device
> > + * @capabilities: bitmask of capabilities (DEVCAPS_*) of this device
> > + *
> > + * This struct provides information about the device needed for
> > + * automatic setup in userspace, such as if the device is direct
> > + * (touchscreen) or indirect (touchpad), and if there are other
> > + * special considerations, such as the touchpad also being a button.
> > + */
> > +struct input_devinfo {
> > + __u32 types;
> > + __u32 capabilities;
> > +};
>
> Why use an ioctl for this at all? It's just 2 simple values that don't
> need to be atomically read at the same time. What's wrong with 2 more
> sysfs files?
>
Because if you already dealing with a file descriptor ioctl is much more
convenient. Sysfs can be nice (and that is why we'll add it as well) but
it is not a recipe for the world peace.
--
Dmitry
On Tue, Dec 07, 2010 at 01:40:26PM +0100, Arnd Bergmann wrote:
> On Tuesday 07 December 2010, Kay Sievers wrote:
> > On Tue, Dec 7, 2010 at 10:16, Dmitry Torokhov <[email protected]> wrote:
> > > Hi Henrik,
> > >
> > > On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
> > >> Today, userspace sets up an input device based on the data it emits.
> > >> This is not always enough; a tablet and a touchscreen may emit exactly
> > >> the same data, for instance, but the former should be set up with a
> > >> pointer whereas the latter does not need to. Recently, a new type of
> > >> touchpad has emerged where the buttons are under the pad, which changes
> > >> handling logic without changing the emitted data. This patch introduces
> > >> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> > >> about the device resulting in proper setup.
> > >
> > > If we agree that the new ioctl is suitable we'llalso need to wireit up
> > > through sysfs. Also, can we keep all definitions to INPUT_ namespace?
> >
> > Please don't add new ioctls which are not extensible. The ioctl should
> > carry the length or the version of the structure it asks for, so it
> > can be extended in the future. Sysfs should be good enough for such
> > interface though.
>
> Please never add any ioctls that have a version or length field!
>
> Ideally ioctls should have only scalar arguments, not structures,
> so I'd recommend splitting it into two, so you can read type and
> capability parameters separately.
Yes, I think it is a good idea to turn it into 2 scalar ioctls.
--
Dmitry
On Mon, Dec 6, 2010 at 11:25 PM, Henrik Rydberg <[email protected]> wrote:
> Today, userspace sets up an input device based on the data it emits.
> This is not always enough; a tablet and a touchscreen may emit exactly
> the same data, for instance, but the former should be set up with a
> pointer whereas the latter does not need to. Recently, a new type of
> touchpad has emerged where the buttons are under the pad, which changes
> handling logic without changing the emitted data. This patch introduces
> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> about the device resulting in proper setup.
>
> Suggested-by: Dmitry Torokhov <[email protected]>
> Signed-off-by: Henrik Rydberg <[email protected]>
> Cc: Ping Cheng <[email protected]>
> Cc: Chris Bagwell <[email protected]>
> ---
> Hi all,
>
> Here is a patch attempting to formulate Dmitry's device type idea. It
> compiles, but further testing awaits a general consesus on the device
> types and capabilities to start out with. Are the ones listed here apt
> for the job?
>
> Cheers,
> Henrik
>
> ?drivers/input/evdev.c | ? ?6 ++++++
> ?include/linux/input.h | ? 34 ++++++++++++++++++++++++++++++++++
> ?2 files changed, 40 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index e3f7fc6..db78592 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -632,6 +632,12 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> ? ? ? ? ? ? ? ? ? ? ? ?return -EFAULT;
> ? ? ? ? ? ? ? ?return 0;
>
> + ? ? ? case EVIOCGDEVINFO:
> + ? ? ? ? ? ? ? if (copy_to_user(p, &dev->devinfo,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sizeof(struct input_devinfo)))
> + ? ? ? ? ? ? ? ? ? ? ? return -EFAULT;
> + ? ? ? ? ? ? ? return 0;
> +
> ? ? ? ?case EVIOCGREP:
> ? ? ? ? ? ? ? ?if (!test_bit(EV_REP, dev->evbit))
> ? ? ? ? ? ? ? ? ? ? ? ?return -ENOSYS;
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 6ef4446..8c58d2a 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -57,6 +57,21 @@ struct input_absinfo {
> ?};
>
> ?/**
> + * struct input_devinfo - device information via EVIOCGDEVINFO ioctl
> + * @types: bitmask of types (DEVTYPE_*) matching this device
> + * @capabilities: bitmask of capabilities (DEVCAPS_*) of this device
> + *
> + * This struct provides information about the device needed for
> + * automatic setup in userspace, such as if the device is direct
> + * (touchscreen) or indirect (touchpad), and if there are other
> + * special considerations, such as the touchpad also being a button.
I guess you are talking about the pad is also a button. What about the
pad has buttons? And there could be more than one button on it. Can we
pass the number of buttons on the pad as well?
> + */
> +struct input_devinfo {
> + ? ? ? __u32 types;
> + ? ? ? __u32 capabilities;
> +};
> +
> +/**
> ?* struct input_keymap_entry - used by EVIOCGKEYCODE/EVIOCSKEYCODE ioctls
> ?* @scancode: scancode represented in machine-endian form.
> ?* @len: length of the scancode that resides in @scancode buffer.
> @@ -91,6 +106,7 @@ struct input_keymap_entry {
> ?#define EVIOCGNAME(len) ? ? ? ? ? ? ? ?_IOC(_IOC_READ, 'E', 0x06, len) ? ? ? ? /* get device name */
> ?#define EVIOCGPHYS(len) ? ? ? ? ? ? ? ?_IOC(_IOC_READ, 'E', 0x07, len) ? ? ? ? /* get physical location */
> ?#define EVIOCGUNIQ(len) ? ? ? ? ? ? ? ?_IOC(_IOC_READ, 'E', 0x08, len) ? ? ? ? /* get unique identifier */
> +#define EVIOCGDEVINFO ? ? ? ? ?_IOR('E', 0x09, struct input_devinfo) ? /* get device information */
>
> ?#define EVIOCGKEY(len) ? ? ? ? _IOC(_IOC_READ, 'E', 0x18, len) ? ? ? ? /* get global key state */
> ?#define EVIOCGLED(len) ? ? ? ? _IOC(_IOC_READ, 'E', 0x19, len) ? ? ? ? /* get all LEDs */
> @@ -108,6 +124,23 @@ struct input_keymap_entry {
> ?#define EVIOCGRAB ? ? ? ? ? ? ?_IOW('E', 0x90, int) ? ? ? ? ? ? ? ? ? ?/* Grab/Release device */
>
> ?/*
> + * Device types
> + */
> +
> +#define DEVTYPE_KEYBOARD ? ? ? 0
> +#define DEVTYPE_MOUSE ? ? ? ? ?1
> +#define DEVTYPE_JOYSTICK ? ? ? 2
> +#define DEVTYPE_TOUCHPAD ? ? ? 3
> +#define DEVTYPE_TABLET ? ? ? ? 4
> +#define DEVTYPE_TOUCHSCREEN ? ?5
TOUCHSCREEN can be finger touch screen and pen touch screen. They are
different types for clients. Should we add one for pen touch screen,
something like DEVTYPE_TABLETPC?
> +
> +/*
> + * Device capabilities
> + */
> +
> +#define DEVCAPS_PAD_IS_BUTTON ?1
Do we need the following:
+#define DEVCAPS_PAD_HAS_BUTTONS 2
+#define DEVCAPS_PAD_HAS_RINGS 3
+#define DEVCAPS_PAD_HAS_REL_WHEELS 4
+#define DEVCAPS_PAD_HAS_ABS_WHEELS 5
+#define DEVCAPS_PAD_HAS_STRIPS 6
Thank you.
Ping
> +
> +/*
> ?* Event types
> ?*/
>
> @@ -1177,6 +1210,7 @@ struct input_dev {
> ? ? ? ?const char *phys;
> ? ? ? ?const char *uniq;
> ? ? ? ?struct input_id id;
> + ? ? ? struct input_devinfo devinfo;
>
> ? ? ? ?unsigned long evbit[BITS_TO_LONGS(EV_CNT)];
> ? ? ? ?unsigned long keybit[BITS_TO_LONGS(KEY_CNT)];
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
On Tue, Dec 7, 2010 at 1:25 AM, Henrik Rydberg <[email protected]> wrote:
> Today, userspace sets up an input device based on the data it emits.
> This is not always enough; a tablet and a touchscreen may emit exactly
> the same data, for instance, but the former should be set up with a
> pointer whereas the latter does not need to. Recently, a new type of
> touchpad has emerged where the buttons are under the pad, which changes
> handling logic without changing the emitted data. This patch introduces
> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> about the device resulting in proper setup.
>
> Suggested-by: Dmitry Torokhov <[email protected]>
> Signed-off-by: Henrik Rydberg <[email protected]>
> Cc: Ping Cheng <[email protected]>
> Cc: Chris Bagwell <[email protected]>
> ---
> Hi all,
>
> Here is a patch attempting to formulate Dmitry's device type idea. It
> compiles, but further testing awaits a general consesus on the device
> types and capabilities to start out with. Are the ones listed here apt
> for the job?
>
> Cheers,
> Henrik
>
> ?drivers/input/evdev.c | ? ?6 ++++++
> ?include/linux/input.h | ? 34 ++++++++++++++++++++++++++++++++++
> ?2 files changed, 40 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index e3f7fc6..db78592 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -632,6 +632,12 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> ? ? ? ? ? ? ? ? ? ? ? ?return -EFAULT;
> ? ? ? ? ? ? ? ?return 0;
>
> + ? ? ? case EVIOCGDEVINFO:
> + ? ? ? ? ? ? ? if (copy_to_user(p, &dev->devinfo,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sizeof(struct input_devinfo)))
> + ? ? ? ? ? ? ? ? ? ? ? return -EFAULT;
> + ? ? ? ? ? ? ? return 0;
> +
> ? ? ? ?case EVIOCGREP:
> ? ? ? ? ? ? ? ?if (!test_bit(EV_REP, dev->evbit))
> ? ? ? ? ? ? ? ? ? ? ? ?return -ENOSYS;
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 6ef4446..8c58d2a 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -57,6 +57,21 @@ struct input_absinfo {
> ?};
>
> ?/**
> + * struct input_devinfo - device information via EVIOCGDEVINFO ioctl
> + * @types: bitmask of types (DEVTYPE_*) matching this device
> + * @capabilities: bitmask of capabilities (DEVCAPS_*) of this device
> + *
> + * This struct provides information about the device needed for
> + * automatic setup in userspace, such as if the device is direct
> + * (touchscreen) or indirect (touchpad), and if there are other
> + * special considerations, such as the touchpad also being a button.
> + */
> +struct input_devinfo {
> + ? ? ? __u32 types;
> + ? ? ? __u32 capabilities;
> +};
> +
"types" sounds like it can support being more then one thing at a time
but I don't think that is intent from below names. Should rename to
"type"?
As example, I think combo devices that support fingers and pens will
be either TOUCHSCREEN or TABLET and declare combo part with
BTN_TOOL_PEN+BTN_TOOL_FINGER.
> +/**
> ?* struct input_keymap_entry - used by EVIOCGKEYCODE/EVIOCSKEYCODE ioctls
> ?* @scancode: scancode represented in machine-endian form.
> ?* @len: length of the scancode that resides in @scancode buffer.
> @@ -91,6 +106,7 @@ struct input_keymap_entry {
> ?#define EVIOCGNAME(len) ? ? ? ? ? ? ? ?_IOC(_IOC_READ, 'E', 0x06, len) ? ? ? ? /* get device name */
> ?#define EVIOCGPHYS(len) ? ? ? ? ? ? ? ?_IOC(_IOC_READ, 'E', 0x07, len) ? ? ? ? /* get physical location */
> ?#define EVIOCGUNIQ(len) ? ? ? ? ? ? ? ?_IOC(_IOC_READ, 'E', 0x08, len) ? ? ? ? /* get unique identifier */
> +#define EVIOCGDEVINFO ? ? ? ? ?_IOR('E', 0x09, struct input_devinfo) ? /* get device information */
>
> ?#define EVIOCGKEY(len) ? ? ? ? _IOC(_IOC_READ, 'E', 0x18, len) ? ? ? ? /* get global key state */
> ?#define EVIOCGLED(len) ? ? ? ? _IOC(_IOC_READ, 'E', 0x19, len) ? ? ? ? /* get all LEDs */
> @@ -108,6 +124,23 @@ struct input_keymap_entry {
> ?#define EVIOCGRAB ? ? ? ? ? ? ?_IOW('E', 0x90, int) ? ? ? ? ? ? ? ? ? ?/* Grab/Release device */
>
> ?/*
> + * Device types
> + */
> +
> +#define DEVTYPE_KEYBOARD ? ? ? 0
> +#define DEVTYPE_MOUSE ? ? ? ? ?1
> +#define DEVTYPE_JOYSTICK ? ? ? 2
> +#define DEVTYPE_TOUCHPAD ? ? ? 3
> +#define DEVTYPE_TABLET ? ? ? ? 4
> +#define DEVTYPE_TOUCHSCREEN ? ?5
> +
> +/*
> + * Device capabilities
> + */
> +
> +#define DEVCAPS_PAD_IS_BUTTON ?1
> +
DEVCAPS_BUTTONPAD or DEVCAPS_CLICKPAD may be good choice if you want
to save some characters.
Once user knows this capability, they will want to know area of pad
that is reserved for buttons. Today it seems universally fixed to
lower 20% of pad so doesn't need to be queried... but someday it may
need a query. I don't guess there is a generic way to query more
details about specific capabilities.
Thanks for prototype... Looking forward to using it once finalized.
Chris
> +/*
> ?* Event types
> ?*/
>
> @@ -1177,6 +1210,7 @@ struct input_dev {
> ? ? ? ?const char *phys;
> ? ? ? ?const char *uniq;
> ? ? ? ?struct input_id id;
> + ? ? ? struct input_devinfo devinfo;
>
> ? ? ? ?unsigned long evbit[BITS_TO_LONGS(EV_CNT)];
> ? ? ? ?unsigned long keybit[BITS_TO_LONGS(KEY_CNT)];
> --
> 1.7.1
>
>
On Tue, Dec 07, 2010 at 10:48:04AM -0800, Ping Cheng wrote:
> On Mon, Dec 6, 2010 at 11:25 PM, Henrik Rydberg <[email protected]> wrote:
> > Today, userspace sets up an input device based on the data it emits.
> > This is not always enough; a tablet and a touchscreen may emit exactly
> > the same data, for instance, but the former should be set up with a
> > pointer whereas the latter does not need to. Recently, a new type of
> > touchpad has emerged where the buttons are under the pad, which changes
> > handling logic without changing the emitted data. This patch introduces
> > a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> > about the device resulting in proper setup.
> >
> > Suggested-by: Dmitry Torokhov <[email protected]>
> > Signed-off-by: Henrik Rydberg <[email protected]>
> > Cc: Ping Cheng <[email protected]>
> > Cc: Chris Bagwell <[email protected]>
> > ---
> > Hi all,
> >
> > Here is a patch attempting to formulate Dmitry's device type idea. It
> > compiles, but further testing awaits a general consesus on the device
> > types and capabilities to start out with. Are the ones listed here apt
> > for the job?
> >
> > Cheers,
> > Henrik
> >
> > ?drivers/input/evdev.c | ? ?6 ++++++
> > ?include/linux/input.h | ? 34 ++++++++++++++++++++++++++++++++++
> > ?2 files changed, 40 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> > index e3f7fc6..db78592 100644
> > --- a/drivers/input/evdev.c
> > +++ b/drivers/input/evdev.c
> > @@ -632,6 +632,12 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> > ? ? ? ? ? ? ? ? ? ? ? ?return -EFAULT;
> > ? ? ? ? ? ? ? ?return 0;
> >
> > + ? ? ? case EVIOCGDEVINFO:
> > + ? ? ? ? ? ? ? if (copy_to_user(p, &dev->devinfo,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sizeof(struct input_devinfo)))
> > + ? ? ? ? ? ? ? ? ? ? ? return -EFAULT;
> > + ? ? ? ? ? ? ? return 0;
> > +
> > ? ? ? ?case EVIOCGREP:
> > ? ? ? ? ? ? ? ?if (!test_bit(EV_REP, dev->evbit))
> > ? ? ? ? ? ? ? ? ? ? ? ?return -ENOSYS;
> > diff --git a/include/linux/input.h b/include/linux/input.h
> > index 6ef4446..8c58d2a 100644
> > --- a/include/linux/input.h
> > +++ b/include/linux/input.h
> > @@ -57,6 +57,21 @@ struct input_absinfo {
> > ?};
> >
> > ?/**
> > + * struct input_devinfo - device information via EVIOCGDEVINFO ioctl
> > + * @types: bitmask of types (DEVTYPE_*) matching this device
> > + * @capabilities: bitmask of capabilities (DEVCAPS_*) of this device
> > + *
> > + * This struct provides information about the device needed for
> > + * automatic setup in userspace, such as if the device is direct
> > + * (touchscreen) or indirect (touchpad), and if there are other
> > + * special considerations, such as the touchpad also being a button.
>
> I guess you are talking about the pad is also a button. What about the
> pad has buttons? And there could be more than one button on it. Can we
> pass the number of buttons on the pad as well?
>
> > + */
> > +struct input_devinfo {
> > + ? ? ? __u32 types;
> > + ? ? ? __u32 capabilities;
> > +};
> > +
> > +/**
> > ?* struct input_keymap_entry - used by EVIOCGKEYCODE/EVIOCSKEYCODE ioctls
> > ?* @scancode: scancode represented in machine-endian form.
> > ?* @len: length of the scancode that resides in @scancode buffer.
> > @@ -91,6 +106,7 @@ struct input_keymap_entry {
> > ?#define EVIOCGNAME(len) ? ? ? ? ? ? ? ?_IOC(_IOC_READ, 'E', 0x06, len) ? ? ? ? /* get device name */
> > ?#define EVIOCGPHYS(len) ? ? ? ? ? ? ? ?_IOC(_IOC_READ, 'E', 0x07, len) ? ? ? ? /* get physical location */
> > ?#define EVIOCGUNIQ(len) ? ? ? ? ? ? ? ?_IOC(_IOC_READ, 'E', 0x08, len) ? ? ? ? /* get unique identifier */
> > +#define EVIOCGDEVINFO ? ? ? ? ?_IOR('E', 0x09, struct input_devinfo) ? /* get device information */
> >
> > ?#define EVIOCGKEY(len) ? ? ? ? _IOC(_IOC_READ, 'E', 0x18, len) ? ? ? ? /* get global key state */
> > ?#define EVIOCGLED(len) ? ? ? ? _IOC(_IOC_READ, 'E', 0x19, len) ? ? ? ? /* get all LEDs */
> > @@ -108,6 +124,23 @@ struct input_keymap_entry {
> > ?#define EVIOCGRAB ? ? ? ? ? ? ?_IOW('E', 0x90, int) ? ? ? ? ? ? ? ? ? ?/* Grab/Release device */
> >
> > ?/*
> > + * Device types
> > + */
> > +
> > +#define DEVTYPE_KEYBOARD ? ? ? 0
> > +#define DEVTYPE_MOUSE ? ? ? ? ?1
> > +#define DEVTYPE_JOYSTICK ? ? ? 2
> > +#define DEVTYPE_TOUCHPAD ? ? ? 3
> > +#define DEVTYPE_TABLET ? ? ? ? 4
> > +#define DEVTYPE_TOUCHSCREEN ? ?5
>
> TOUCHSCREEN can be finger touch screen and pen touch screen. They are
> different types for clients. Should we add one for pen touch screen,
> something like DEVTYPE_TABLETPC?
No, they are exactly the same as far as this classification is
concerned. No matter what you are touching the working surface with the
expected behavior is that, unlike tablet (which can also be handle
fingers and/or pen) you do not need to have on-screen pointer tracking
your movements.
>
> > +
> > +/*
> > + * Device capabilities
> > + */
> > +
> > +#define DEVCAPS_PAD_IS_BUTTON ?1
>
> Do we need the following:
>
> +#define DEVCAPS_PAD_HAS_BUTTONS 2
> +#define DEVCAPS_PAD_HAS_RINGS 3
> +#define DEVCAPS_PAD_HAS_REL_WHEELS 4
> +#define DEVCAPS_PAD_HAS_ABS_WHEELS 5
> +#define DEVCAPS_PAD_HAS_STRIPS 6
No, you can get all this data from the XXXbits in the device (well, sans
rings/strips as we do not have such events yet). The
DEVCAPS_PAD_IS_BUTTON is to tell userspace that pad itself serves as a
button. The device still emits BTN_LEFT (primary button event) for such
devices but userspace needs to handle it a bit differently.
--
Dmitry
On Tue, Dec 07, 2010 at 01:18:41PM -0600, Chris Bagwell wrote:
> On Tue, Dec 7, 2010 at 1:25 AM, Henrik Rydberg <[email protected]> wrote:
> > Today, userspace sets up an input device based on the data it emits.
> > This is not always enough; a tablet and a touchscreen may emit exactly
> > the same data, for instance, but the former should be set up with a
> > pointer whereas the latter does not need to. Recently, a new type of
> > touchpad has emerged where the buttons are under the pad, which changes
> > handling logic without changing the emitted data. This patch introduces
> > a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> > about the device resulting in proper setup.
> >
> > Suggested-by: Dmitry Torokhov <[email protected]>
> > Signed-off-by: Henrik Rydberg <[email protected]>
> > Cc: Ping Cheng <[email protected]>
> > Cc: Chris Bagwell <[email protected]>
> > ---
> > Hi all,
> >
> > Here is a patch attempting to formulate Dmitry's device type idea. It
> > compiles, but further testing awaits a general consesus on the device
> > types and capabilities to start out with. Are the ones listed here apt
> > for the job?
> >
> > Cheers,
> > Henrik
> >
> > ?drivers/input/evdev.c | ? ?6 ++++++
> > ?include/linux/input.h | ? 34 ++++++++++++++++++++++++++++++++++
> > ?2 files changed, 40 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> > index e3f7fc6..db78592 100644
> > --- a/drivers/input/evdev.c
> > +++ b/drivers/input/evdev.c
> > @@ -632,6 +632,12 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> > ? ? ? ? ? ? ? ? ? ? ? ?return -EFAULT;
> > ? ? ? ? ? ? ? ?return 0;
> >
> > + ? ? ? case EVIOCGDEVINFO:
> > + ? ? ? ? ? ? ? if (copy_to_user(p, &dev->devinfo,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sizeof(struct input_devinfo)))
> > + ? ? ? ? ? ? ? ? ? ? ? return -EFAULT;
> > + ? ? ? ? ? ? ? return 0;
> > +
> > ? ? ? ?case EVIOCGREP:
> > ? ? ? ? ? ? ? ?if (!test_bit(EV_REP, dev->evbit))
> > ? ? ? ? ? ? ? ? ? ? ? ?return -ENOSYS;
> > diff --git a/include/linux/input.h b/include/linux/input.h
> > index 6ef4446..8c58d2a 100644
> > --- a/include/linux/input.h
> > +++ b/include/linux/input.h
> > @@ -57,6 +57,21 @@ struct input_absinfo {
> > ?};
> >
> > ?/**
> > + * struct input_devinfo - device information via EVIOCGDEVINFO ioctl
> > + * @types: bitmask of types (DEVTYPE_*) matching this device
> > + * @capabilities: bitmask of capabilities (DEVCAPS_*) of this device
> > + *
> > + * This struct provides information about the device needed for
> > + * automatic setup in userspace, such as if the device is direct
> > + * (touchscreen) or indirect (touchpad), and if there are other
> > + * special considerations, such as the touchpad also being a button.
> > + */
> > +struct input_devinfo {
> > + ? ? ? __u32 types;
> > + ? ? ? __u32 capabilities;
> > +};
> > +
>
> "types" sounds like it can support being more then one thing at a time
> but I don't think that is intent from below names. Should rename to
> "type"?
I'd be wary of restricting device to be only one type. There are some
that are mutually exclusive (toucscreen/touchpad/tablet) but I'd leave
option of mixed-type devices open.
--
Dmitry
On 12/07/2010 10:16 AM, Dmitry Torokhov wrote:
> Hi Henrik,
>
> On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
>> Today, userspace sets up an input device based on the data it emits.
>> This is not always enough; a tablet and a touchscreen may emit exactly
>> the same data, for instance, but the former should be set up with a
>> pointer whereas the latter does not need to. Recently, a new type of
>> touchpad has emerged where the buttons are under the pad, which changes
>> handling logic without changing the emitted data. This patch introduces
>> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
>> about the device resulting in proper setup.
>
> If we agree that the new ioctl is suitable we'llalso need to wireit up
> through sysfs. Also, can we keep all definitions to INPUT_ namespace?
>
> Thanks.
>
Thanks all for the comments, this is what I extract from them:
* Split struct into separate calls, although this still seems debated.
* Use INPUT_ namespace
* Add sysfs version
* Keep bitmask for types. Is the list of types complete?
* Still only one data point for capabilities. Anything else?
Sounds about right?
Thanks,
Henrik
On Tue, Dec 07, 2010 at 08:57:14PM +0100, Henrik Rydberg wrote:
> On 12/07/2010 10:16 AM, Dmitry Torokhov wrote:
>
> > Hi Henrik,
> >
> > On Tue, Dec 07, 2010 at 08:25:26AM +0100, Henrik Rydberg wrote:
> >> Today, userspace sets up an input device based on the data it emits.
> >> This is not always enough; a tablet and a touchscreen may emit exactly
> >> the same data, for instance, but the former should be set up with a
> >> pointer whereas the latter does not need to. Recently, a new type of
> >> touchpad has emerged where the buttons are under the pad, which changes
> >> handling logic without changing the emitted data. This patch introduces
> >> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> >> about the device resulting in proper setup.
> >
> > If we agree that the new ioctl is suitable we'llalso need to wireit up
> > through sysfs. Also, can we keep all definitions to INPUT_ namespace?
> >
> > Thanks.
> >
>
>
> Thanks all for the comments, this is what I extract from them:
>
> * Split struct into separate calls, although this still seems debated.
>
> * Use INPUT_ namespace
>
> * Add sysfs version
>
> * Keep bitmask for types. Is the list of types complete?
Most likely not but it can be easily extended on as-needed basis.
>
> * Still only one data point for capabilities. Anything else?
Again, no need to define everything upfront.
One concern is that while 32 distinct device types should be enough
should we plan for larger capability array? Should we use variable
length ioctl - like EVIOCGKEY(len) - even though Arnd does not like
them?
Also, we already have /sys/class/input/input0/capabilities/ so should we
call new data item something else? Quirks maybe?
--
Dmitry
> One concern is that while 32 distinct device types should be enough
> should we plan for larger capability array? Should we use variable
> length ioctl - like EVIOCGKEY(len) - even though Arnd does not like
> them?
Sounds good, but then again the struct approach feels quite extensible too...
> Also, we already have /sys/class/input/input0/capabilities/ so should we
> call new data item something else? Quirks maybe?
Works fine in my book.
Thanks,
Henrik
On Tue, 07 Dec 2010 08:25:26 +0100, Henrik Rydberg said:
> Today, userspace sets up an input device based on the data it emits.
> This is not always enough; a tablet and a touchscreen may emit exactly
> the same data, for instance, but the former should be set up with a
> pointer whereas the latter does not need to. Recently, a new type of
> touchpad has emerged where the buttons are under the pad, which changes
> handling logic without changing the emitted data. This patch introduces
> a new ioctl, EVIOCGDEVINFO, which allows userspace to extract information
> about the device resulting in proper setup.
> /*
> + * Device types
> + */
> +
> +#define DEVTYPE_KEYBOARD 0
> +#define DEVTYPE_MOUSE 1
> +#define DEVTYPE_JOYSTICK 2
> +#define DEVTYPE_TOUCHPAD 3
> +#define DEVTYPE_TABLET 4
> +#define DEVTYPE_TOUCHSCREEN 5
Add a #define DEVTYPE_UNKNOWN 0 and push everybody else down one. That way,
an uninitialized 'struct input_devinfo' won't claim to be a keyboard.
>
>> /*
>> + * Device types
>> + */
>> +
>> +#define DEVTYPE_KEYBOARD 0
>> +#define DEVTYPE_MOUSE 1
>> +#define DEVTYPE_JOYSTICK 2
>> +#define DEVTYPE_TOUCHPAD 3
>> +#define DEVTYPE_TABLET 4
>> +#define DEVTYPE_TOUCHSCREEN 5
>
> Add a #define DEVTYPE_UNKNOWN 0 and push everybody else down one. That way,
> an uninitialized 'struct input_devinfo' won't claim to be a keyboard.
Since types is a bitmask, the initial value will actually be unclaimed already.
To be explicit, for a keyboard, types = 1 << 0 = 1.
Thanks,
Henrik
On Wed, Dec 08, 2010 at 08:04:50PM +0100, Henrik Rydberg wrote:
> > One concern is that while 32 distinct device types should be enough
>
> > should we plan for larger capability array? Should we use variable
> > length ioctl - like EVIOCGKEY(len) - even though Arnd does not like
> > them?
>
>
> Sounds good, but then again the struct approach feels quite extensible too...
>
Actually the more I think about it the less I like idea of extending the
struct because while keeping ABI is pretty easy there are other issues.
I'll CC you on EVIOCGKEYCODE patch so you can see my concerns.
--
Dmitry