input: Wacom tablet driver for simple X hotplugging
RFC 1/1 input: Wacom tablet driver for simple X hotplugging
Hi,
I've been working on a device driver that simplifies hotplugging Wacom tablets when running X.
I've posted the driver on the linuxwacom m-l and the response so far has been positive as it makes hot-plugging Wacom tablets even easier, but questions has been rised if this couldn't be made into a generic module for other devices than Wacom tablets.
The 'problem' is that the normal evdev-driver removes the inputX device when the physical device is unplugged and this causes the X-server to close the device and not re-open it until the user switches VT.
My device driver (patch attached below) works pretty much like the evdev-driver except that it it registers one device when it is loaded and simulates a Wacom tablet even when none is connected to the system. This way the X-server will never notice the absence of a tablet and whenever a tablet is (re)connected it will work without any tricks.
I'd appreciate whether you think this is a viable idea to make it as a generic driver instead or should I continue with the Wacom-specific one. I know the 'right' thing would be to make X truly hot-plug aware, but this driver is something that would be possible to use in current systems without any problems.
If it is a viable idea; Which other devices/types of device do you think could be of interest to handle in a similar fashion? Tablets of different makes/models are obvious, but are there any others that would benefit from a similar driver?
And third, should something like this be a separate driver or should the functionallity be included in the evdev-driver?
Note, the patch is included for idea visualization and not by any means something I consider ready to be submitted. I know I have a few issues that I must sort out first, however it does work in its current state.
BR
Magnus
---
--
drivers/input/Kconfig +19 -0
@@ -117,6 +117,25 @@ config INPUT_EVDEV To compile this driver as a module, choose M here: the module will be called evdev. +config INPUT_WACOMPROXY + tristate "Wacom tablet proxy device" + default y + depends on EXPERIMENTAL + ---help--- + Say Y here if you want a proxy device that will emulate the + latest connected Wacom device when no Wacom tablet is connected. + The initial device it will emulate is a Wacom Penpartner. Only + one Wacom tablet will be available at a time, and the tablets + are opened in the order they are connected. The device is + accessible as a single char device 10:xx - /dev/input/wacomproxy. + + The minor device used can be found in the + /sys/class/input/wacomproxy/dev file once the module has + been loaded. + + To compile this driver as a module, choose M here, the module + will be called wacomproxy. + config INPUT_EVBUG tristate "Event debugging" ---help---
drivers/input/Makefile +1 -0
@@ -11,6 +11,7 @@ obj-$(CONFIG_INPUT_EVDEV) += evdev.o obj-$(CONFIG_INPUT_TSDEV) += tsdev.o obj-$(CONFIG_INPUT_POWER) += power.o obj-$(CONFIG_INPUT_EVBUG) += evbug.o +obj-$(CONFIG_INPUT_WACOMPROXY) += wacomproxy.o obj-$(CONFIG_INPUT_KEYBOARD) += keyboard/ obj-$(CONFIG_INPUT_MOUSE) += mouse/
drivers/input/wacomproxy.c +732 -0
732 lines not on this page · raw message
Hi Magnus,
Quoted 6 lines · Magnus Vigerlöf
I'd appreciate whether you think this is a viable idea to make it as a generic driver instead or should I continue with the Wacom-specific one. I know the 'right' thing would be to make X truly hot-plug aware, but this driver is something that would be possible to use in current systems without any problems.
Yes, I think fixing X would ultimately be time better spent.
Quoted 5 lines
If it is a viable idea; Which other devices/types of device do you think could be of interest to handle in a similar fashion? Tablets of different makes/models are obvious, but are there any others that would benefit from a similar driver?
I do not think that creating device-specific "drivers" is a good idea even short term, especially in kernel. If you want a "persistent" device just create a userspace daemon and listen for hotplug events. When you see the input device you interested in grab it and pipe all data into somewhere. Next time you see hotplug event for the same device release the old instance and grab the new one. In cases when final recepient of events uses ioctls to query input devices capabilities you can create uinput feed back into kernel. This way your program will work for all types of input devices and no kernel changes are needed.
Signature · 1 line
Dmitry
Quoted 10 lines · Dmitry Torokhov
Hi Magnus,
Quoted 5 lines · Magnus Vigerlöf
I'd appreciate whether you think this is a viable idea to make it as a generic driver instead or should I continue with the Wacom-specific one. I know the 'right' thing would be to make X truly hot-plug aware, but this driver is something that would be possible to use in current systems without any problems.
Yes, I think fixing X would ultimately be time better spent.
As a long term fix, I'll consider getting involved with that.. However, I'd like to have something that works with my current setup, hence the.. temporary driver... Ok, I'm still going to use it as it solves one bad and one annoying thing I have with my Wacom tablet, but I'll keep it outside the kernel while better ways to solve this are made.
Quoted 5 lines
Quoted 4 lines
If it is a viable idea; Which other devices/types of device do you think could be of interest to handle in a similar fashion? Tablets of different makes/models are obvious, but are there any others that would benefit from a similar driver?
Quoted 9 lines
I do not think that creating device-specific "drivers" is a good idea even short term, especially in kernel. If you want a "persistent" device just create a userspace daemon and listen for hotplug events. When you see the input device you interested in grab it and pipe all data into somewhere. Next time you see hotplug event for the same device release the old instance and grab the new one. In cases when final recepient of events uses ioctls to query input devices capabilities you can create uinput feed back into kernel. This way your program will work for all types of input devices and no kernel changes are needed.
Creating userspace device drivers is something new for me. If you have some pointers to information about it I would be grateful (I've found the FUSD framework through google). From what I can read from the doc of FUSD, I'll have to open the inputX device if I want events from the tablet to propagate, and I'm afraid I might hit the oops in evdev I described in my previous thread if I do that.
So.. Are the locking issues in evdev something that is about to be fixed soon or should I contribute? Or do you think the issue will be completely irrelevant for a user-space driver?
Thanks
Magnus
Quoted 12 lines · Magnus Vigerlöf
Quoted 10 lines · Dmitry Torokhov
I do not think that creating device-specific "drivers" is a good idea even short term, especially in kernel. If you want a "persistent" device just create a userspace daemon and listen for hotplug events. When you see the input device you interested in grab it and pipe all data into somewhere. Next time you see hotplug event for the same device release the old instance and grab the new one. In cases when final recepient of events uses ioctls to query input devices capabilities you can create uinput feed back into kernel. This way your program will work for all types of input devices and no kernel changes are needed.
Quoted 7 lines
Creating userspace device drivers is something new for me. If you have some pointers to information about it I would be grateful (I've found the FUSD framework through google). From what I can read from the doc of FUSD, I'll have to open the inputX device if I want events from the tablet to propagate, and I'm afraid I might hit the oops in evdev I described in my previous thread if I do that.
No, I was not talking about FUSD, just uinput driver that is in kernel proper. Take a look at this:
http://svn.navi.cx/misc/trunk/inputpipe/
It allows making input devices "network-transparent" and for example use joystick physically connected to one box to play game on another. Hmm, actually it is almost what you need, you just need modify server to multiplex events into single device instead of creating separate input devices.
Quoted 4 lines
So.. Are the locking issues in evdev something that is about to be fixed soon or should I contribute? Or do you think the issue will be completely irrelevant for a user-space driver?
No, I think you will still have the same issues with locking, unfortunately I can't commint on a specific date when they will be resolved. Patches are always welcome of course.
Signature · 1 line
Dmitry
On Sunday 23 July 2006 07:24, Dmitry Torokhov wrote:
[...]
Quoted 10 lines
No, I was not talking about FUSD, just uinput driver that is in kernel proper. Take a look at this:
http://svn.navi.cx/misc/trunk/inputpipe/
It allows making input devices "network-transparent" and for example use joystick physically connected to one box to play game on another. Hmm, actually it is almost what you need, you just need modify server to multiplex events into single device instead of creating separate input devices.
Interesting.. This might be handy for other projects. However, implementing the same module again, but in userspace, for something that really should be handled in the X-server (and will in time), nah.. I think I'll see what the X-guys are up to and see how long they've come instead. Maybe I can contribute there in some way.
Quoted 7 lines
Quoted 3 lines
So.. Are the locking issues in evdev something that is about to be fixed soon or should I contribute? Or do you think the issue will be completely irrelevant for a user-space driver?
No, I think you will still have the same issues with locking, unfortunately I can't commint on a specific date when they will be resolved. Patches are always welcome of course.
Just wanted to know if you had something ongoing regarding this.. I'll see what I can come up with.
/Magnus
Quoted 13 lines · Magnus Vigerlöf
On Sunday 23 July 2006 07:24, Dmitry Torokhov wrote:
[...]Quoted 10 lines
No, I was not talking about FUSD, just uinput driver that is in kernel proper. Take a look at this:
http://svn.navi.cx/misc/trunk/inputpipe/
It allows making input devices "network-transparent" and for example use joystick physically connected to one box to play game on another. Hmm, actually it is almost what you need, you just need modify server to multiplex events into single device instead of creating separate input devices.
Quoted 5 lines
Interesting.. This might be handy for other projects. However, implementing the same module again, but in userspace, for something that really should be handled in the X-server (and will in time), nah.. I think I'll see what the X-guys are up to and see how long they've come instead. Maybe I can contribute there in some way.
You get basic hotplug of input devices using xf86-input-evdev with current versions, though you'd need current git for reasonable quality tablet support. That may still lack some of the wacom specific bits. (I'd have to look to see what the wacom driver does that xf86-input-evdev doesn't do.)
Hotplug with arbitrary drivers is a harder job, and I'm still not sure if we're talking having that done in 7.2 or 7.3. But it is coming.
All of that said, I'm going to have to agree with others, don't do it in the kernel, because the need to do it in the kernel is going away.
Zephaniah E. Hull.
Signature · 7 lines
1024D/E65A7801 Zephaniah E. Hull <[email protected]> 92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801 CCs of replies from mailing lists are requested. A little knowledge is a dangerous thing. So is a lot. -- Albert Einstein.
Quoted 9 lines · Dmitry Torokhov
Quoted 6 lines · Magnus Vigerlöf
I'd appreciate whether you think this is a viable idea to make it as a generic driver instead or should I continue with the Wacom-specific one. I know the 'right' thing would be to make X truly hot-plug aware, but this driver is something that would be possible to use in current systems without any problems.
Yes, I think fixing X would ultimately be time better spent.
It's already mostly done, and should hopefully land for 7.2. It's a neat concept, along the lines of /dev/input/mice, but the time for that kind of pathological braindamage is long gone.
Cheers,
Daniel
Dmitry Torokhov
maintainerQuoted 9 lines · Daniel Stone
Quoted 8 lines · Dmitry Torokhov
Quoted 6 lines · Magnus Vigerlöf
I'd appreciate whether you think this is a viable idea to make it as a generic driver instead or should I continue with the Wacom-specific one. I know the 'right' thing would be to make X truly hot-plug aware, but this driver is something that would be possible to use in current systems without any problems.
Quoted 6 lines
Quoted 1 line
Yes, I think fixing X would ultimately be time better spent.
It's already mostly done, and should hopefully land for 7.2. It's a neat concept, along the lines of
/dev/input/mice, but the time for that kind of pathological braindamage is long gone.
Oh that is the great news!
Signature · 1 line
Dmitry
Odd values in /proc
In reply to Daniel StoneHi all,
I was recently fooling with /proc for a school project and found out that ps and top are not doing the same calculation about memory load.
For instance :
ps aux | grep X
3323 1.5 5.9 64948 53996
ok, so the value for the VSS size read by ps is : 64948 and the value for the RSS size is 53996
Let's look at top now :
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
3323 root 15 0 191m 52m 7284 S 11.9 6.0 39:05.93 X
Ok, so for Top , the Virtual mem size is 191m. This is somewhat different to the 64948Kb found by Ps.
Am I wrong by thinking that these two values should be the same ?
I've done the same test with other programs and most of the times, the values are the same.
I really don't understand it ;)
I digged a bit deeper to see where the difference comes from.
There are at least two different files in /proc that gives you the mem load of a particular process.
Those files are : /proc/[PID]/statm and /proc/[PID]/status
let's look at those files for this example
serty2@serty2 ~ $ more /proc/3323/status
Name: X State: S (sleeping) SleepAVG: 98% Tgid: 3323 Pid: 3323 PPid: 3320 TracerPid: 0 Uid: 0 0 0 0 Gid: 0 0 0 0 FDSize: 256 Groups: VmSize: 65080 kB VmLck: 0 kB VmRSS: 54128 kB VmData: 46772 kB VmStk: 84 kB VmExe: 1532 kB VmLib: 13124 kB VmPTE: 224 kB Threads: 1 SigPnd: 0000000000000000 ShdPnd: 0000000000000000 SigBlk: 0000000000000000 SigIgn: 0000000000001000 SigCgt: 00000001d1806ecb CapInh: 00000000fffffeff CapPrm: 00000000fffffeff CapEff: 00000000fffffeff
Ok , so the VmSize is 65080 , that really looks like the value we got from ps ;)
(not the exact same because of the time delay between the two tests)
Let's look at the other file :
serty2@serty2 ~ $ more /proc/3323/statm
49166 13532 1821 383 0 11714 0
as stated in proc's man , the first value is the total program size in mem pages.
then : (49166 * 4096) / (1024²) =192Mb
We're pretty close to the value given by Top .
I don't understand why there is a difference for those values for some programs.
And most important of it, I really don't know what value to use ;)
I think I would go for the value in /proc/[PID]/statm because the /proc/[PID]/status is supposed to be only a aggregate of information found in other files and made "human-readable".
But then, I'm not sure.
Does anyone know which file holds the right value ?
Thanks for any comments
Damien
PS : please excuse my english mistakes ;)
Re: Odd values in /proc
Quoted 4 lines
Hi all,
I was recently fooling with /proc for a school project and found out that ps and top are not doing the same calculation about memory load.
Take a third one and compare: pmap pidof X
(and preferably post that)
Jan Engelhardt
Quoted 34 lines · On Fri, 21 Jul 2006 23:13:41 +0200 Magnus Vigerlöf <[email protected]> wrote:
Hi,
I've been working on a device driver that simplifies hotplugging Wacom tablets when running X.
I've posted the driver on the linuxwacom m-l and the response so far has been positive as it makes hot-plugging Wacom tablets even easier, but questions has been rised if this couldn't be made into a generic module for other devices than Wacom tablets.
The 'problem' is that the normal evdev-driver removes the inputX device when the physical device is unplugged and this causes the X-server to close the device and not re-open it until the user switches VT.
My device driver (patch attached below) works pretty much like the evdev-driver except that it it registers one device when it is loaded and simulates a Wacom tablet even when none is connected to the system. This way the X-server will never notice the absence of a tablet and whenever a tablet is (re)connected it will work without any tricks.
I'd appreciate whether you think this is a viable idea to make it as a generic driver instead or should I continue with the Wacom-specific one. I know the 'right' thing would be to make X truly hot-plug aware, but this driver is something that would be possible to use in current systems without any problems.
If it is a viable idea; Which other devices/types of device do you think could be of interest to handle in a similar fashion? Tablets of different makes/models are obvious, but are there any others that would benefit from a similar driver?
Quoted 9 lines
And third, should something like this be a separate driver or should the functionallity be included in the evdev-driver?
Note, the patch is included for idea visualization and not by any means something I consider ready to be submitted. I know I have a few issues that I must sort out first, however it does work in its current state.
The patch adds rather a lot of trailing whitespace. I trim that off when applying; others do not.
Quoted 1 line
+#include <
asm/semaphore.h
Semaphores are deprecated. Unless you actually _need_ the sempahore's counting feature, please use mutexes.
drivers/input/wacomproxy.c line 104
Quoted 3 lines
drivers/input/wacomproxy.c +3
-0+static int exclusivegrab =0 ; +static int debugconnect =0 ; +static short openfailcount =0 ;
Avoid the initialisation-to-zero. The C runtime will zero these anyway, and the explicit initialisation will move these variables into the data section and hence into the on-disk vmlinux inage.
drivers/input/wacomproxy.c line 109
Quoted 5 lines
drivers/input/wacomproxy.c +5
-0+#ifdef OPENFAILCOUNT +module_param (openfailcount, short,0644 ); +MODULE_PARM_DESC (openfailcount,"Number of upcoming open that will fail" +" with -ENODEV." ); +#endif
What is this??
Quoted 4 lines
drivers/input/wacomproxy.c +4
-0+static int str_to_user (const char *str,unsigned int maxlen, +void __user *p) +{ +int len;
drivers/input/wacomproxy.c line 241
Quoted 10 lines
drivers/input/wacomproxy.c +10
-0+ +if (!str) +return -ENOENT ; + + len =strlen (str) +1 ; +if (len > maxlen) + len = maxlen; + +return copy_to_user (p, str, len) ? -EFAULT : len; +}
If (len > maxlen) this will send userspace a non-null-terminated string.
Quoted 23 lines
drivers/input/wacomproxy.c +23
-0+/* ################################################################### */ +/* Module File Methods */ +/* ################################################################### */ +static int wp_open (struct inode *inodp,struct file *filp) +{ +struct wp_filenode *list; + +if (openfailcount >0 ) { + openfailcount--; +return -ENODEV ; + } + +if (!(list =kzalloc (sizeof (*list),GFP_KERNEL ))) +return -ENOMEM ; + + filp->private_data = list; + list->tabletid = wp_proxyhndl.tabletid; + +down (&wp_sema); +list_add_tail (&list->node, &wp_users); +if (wp_currenthndl != &wp_proxyhndl) { +if (!(wp_currenthndl->flags &WP_FLAG_OPEN )) { +if (!input_open_device (&wp_currenthndl->handle))
drivers/input/wacomproxy.c line 275
Quoted 10 lines
drivers/input/wacomproxy.c +10
-0+ wp_currenthndl->flags |=WP_FLAG_OPEN ; + else + printk(KERN_WARNING WP_MODNAME +": Failed to open '%s'\n" , + wp_currenthndl->devdesc); + } + } +up (&wp_sema); +return 0 ; +}
If input_open_device() fails then the whole open() should fail. That way there will be no close(), flush() or release().
drivers/input/wacomproxy.c line 375
Quoted 1 line
drivers/input/wacomproxy.c +1
-0+if (copy_to_user (ip, &wp_proxydev.id,sizeof (struct input_id )))
We put a space between the if' and the (' (review the entire patch for this).
Quoted 8 lines
drivers/input/wacomproxy.c +8
-0+/* the id as all must close/reopen again anyway. */ + else if (!list_empty(&wp_users)) { +if (!input_open_device (&devh->handle)) { + devh->flags |=WP_FLAG_OPEN ; +if (wp_devgrabcount >0 ) { +if (!input_grab_device (&devh->handle)) + devh->flags |=WP_FLAG_GRAB ; +else
drivers/input/wacomproxy.c line 579
Quoted 10 lines
drivers/input/wacomproxy.c +10
-0+ printk(KERN_WARNING WP_MODNAME +": Failed to grab '%s'\n" , + dev->name); + } + } +else + printk(KERN_WARNING WP_MODNAME +": Failed to open '%s'\n" , dev->name); + } +}
Again, just emitting a printk seems insufficient here.
Thanks for the comments. They are appreciated even though the module never will be submitted to the kernel.
On Tuesday 25 July 2006 21:06, Andrew Morton wrote:
[...]
Quoted 7 lines
Quoted 4 lines
Note, the patch is included for idea visualization and not by any means something I consider ready to be submitted. I know I have a few issues that I must sort out first, however it does work in its current state.
The patch adds rather a lot of trailing whitespace. I trim that off when applying; others do not.
Hmm... These I can't see.. For me trailing WS can be two things; WS between the last real character and newline and empty lines at the end of the file. I hate these two cases of trailing WS myself and I can't see any of them in the patch, so what is it I'm missing here? (Ok, I see one space at the beginning of one line, but that's not 'a lot' so it can't be that.)
Quoted 13 lines
Quoted 1 line
+#include <
asm/semaphore.hSemaphores are deprecated. Unless you actually _need_ the sempahore's counting feature, please use mutexes.
drivers/input/wacomproxy.c line 104
Quoted 3 lines
drivers/input/wacomproxy.c +3
-0+static int exclusivegrab =0 ; +static int debugconnect =0 ; +static short openfailcount =0 ;Avoid the initialisation-to-zero. The C runtime will zero these anyway, and the explicit initialisation will move these variables into the data section and hence into the on-disk vmlinux inage.
Quoted 7 lines
drivers/input/wacomproxy.c line 109
Quoted 5 lines
drivers/input/wacomproxy.c +5
-0+#ifdef OPENFAILCOUNT +module_param (openfailcount, short,0644 ); +MODULE_PARM_DESC (openfailcount,"Number of upcoming open that will fail" +" with -ENODEV." ); +#endif What is this??
I should have put a note on this. It is something that enables me to force X to fail to open and to read from the device. That way it will be possible to reload the module without restarting X. Useful while developing and would have been removed if I ever would have released it to the kernel.
Quoted 46 lines
Quoted 4 lines
drivers/input/wacomproxy.c +4
-0+static int str_to_user (const char *str,unsigned int maxlen, +void __user *p) +{ +int len;drivers/input/wacomproxy.c line 241
Quoted 10 lines
drivers/input/wacomproxy.c +10
-0+ +if (!str) +return -ENOENT ; + + len =strlen (str) +1 ; +if (len > maxlen) + len = maxlen; + +return copy_to_user (p, str, len) ? -EFAULT : len; +}If (len > maxlen) this will send userspace a non-null-terminated string.
Quoted 29 lines
+/* ###################################################################
*/ +/* Module File Methods
*/ +/*
################################################################### */drivers/input/wacomproxy.c +25
-0+static int wp_open (struct inode *inodp,struct file *filp) +{ +struct wp_filenode *list; + +if (openfailcount >0 ) { + openfailcount--; +return -ENODEV ; + } + +if (!(list =kzalloc (sizeof (*list),GFP_KERNEL ))) +return -ENOMEM ; + + filp->private_data = list; + list->tabletid = wp_proxyhndl.tabletid; + +down (&wp_sema); +list_add_tail (&list->node, &wp_users); +if (wp_currenthndl != &wp_proxyhndl) { +if (!(wp_currenthndl->flags &WP_FLAG_OPEN )) { +if (!input_open_device (&wp_currenthndl->handle)) + wp_currenthndl->flags |=WP_FLAG_OPEN ; +else + printk(KERN_WARNING WP_MODNAME +": Failed to open '%s'\n" , + wp_currenthndl->devdesc);
Quoted 8 lines
drivers/input/wacomproxy.c line 275
Quoted 5 lines
drivers/input/wacomproxy.c +5
-0+ } + } +up (&wp_sema); +return 0 ; +}If
input_open_device()fails then the wholeopen()should fail. That way there will be noclose(),flush()orrelease().
Yes, that would be the correct behavior. It could however result in some inconsistent behavior. As applications that open the device before attaching the tablet succeeds, and applications that opens it after the tablet is attached will fail (if input_open_device fails as well).
Maybe only opening the device when connecting it or if it is the first one that opens it would be the best way to handle it?
Quoted 17 lines
drivers/input/wacomproxy.c line 375
Quoted 1 line
drivers/input/wacomproxy.c +1
-0+if (copy_to_user (ip, &wp_proxydev.id,sizeof (struct input_id )))We put a space between the
if' and the(' (review the entire patch for this).Quoted 12 lines
drivers/input/wacomproxy.c +12
-0+/* the id as all must close/reopen again anyway. */ + else if (!list_empty(&wp_users)) { +if (!input_open_device (&devh->handle)) { + devh->flags |=WP_FLAG_OPEN ; +if (wp_devgrabcount >0 ) { +if (!input_grab_device (&devh->handle)) + devh->flags |=WP_FLAG_GRAB ; +else + printk(KERN_WARNING WP_MODNAME +": Failed to grab '%s'\n" , + dev->name); + }
Quoted 8 lines
drivers/input/wacomproxy.c line 579
Quoted 6 lines
drivers/input/wacomproxy.c +6
-0+ } +else + printk(KERN_WARNING WP_MODNAME +": Failed to open '%s'\n" , dev->name); + } +}Again, just emitting a printk seems insufficient here.
The above code is called when a new tablet is added/removed so it's not possible to return something 'nice'. It might not even be the newly attached tablet that it fails to open. So in this case is it really possible to do anything else than a printk?
Thanks
Magnus