From: Andrew Morton <akpm@osdl.org>
To: "Magnus Vigerlöf" <wigge@bigfoot.com>
Cc: linux-input@atrey.karlin.mff.cuni.cz,
linux-kernel@vger.kernel.org, wigge@bigfoot.com,
"Ping Cheng" <pingc@wacom.com>
Subject: Re: [RFC] input: Wacom tablet driver for simple X hotplugging
Date: Tue, 25 Jul 2006 12:06:45 -0700 [thread overview]
Message-ID: <20060725120645.b4dc98ab.akpm@osdl.org> (raw)
In-Reply-To: <20060721211341.5366.93270.sendpatchset@pipe>
On Fri, 21 Jul 2006 23:13:41 +0200
Magnus Vigerlöf <wigge@bigfoot.com> 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?
>
> 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.
> +#include <asm/semaphore.h
Semaphores are deprecated. Unless you actually _need_ the sempahore's
counting feature, please use mutexes.
> +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.
> +#ifdef OPENFAILCOUNT
> +module_param(openfailcount, short, 0644);
> +MODULE_PARM_DESC(openfailcount, "Number of upcoming open that will fail"
> + " with -ENODEV.");
> +#endif
What is this??
> +static int str_to_user(const char *str, unsigned int maxlen,
> + void __user *p)
> +{
> + int len;
> +
> + 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.
> +/* ################################################################### */
> +/* 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))
> + 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().
> + 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).
> + /* 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);
> + }
> + }
> + else
> + printk(KERN_WARNING WP_MODNAME
> + ": Failed to open '%s'\n", dev->name);
> + }
> +}
Again, just emitting a printk seems insufficient here.
next prev parent reply other threads:[~2006-07-25 19:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-07-21 21:13 [RFC] input: Wacom tablet driver for simple X hotplugging Magnus Vigerlöf
2006-07-22 2:09 ` Dmitry Torokhov
2006-07-22 10:00 ` Magnus Vigerlöf
2006-07-23 5:24 ` Dmitry Torokhov
2006-07-24 16:28 ` Magnus Vigerlöf
2006-07-25 18:00 ` Zephaniah E. Hull
2006-07-24 15:11 ` Daniel Stone
2006-07-24 15:22 ` Dmitry Torokhov
2006-07-24 15:23 ` Odd values in /proc Damien Pacaud
2006-07-25 19:56 ` Jan Engelhardt
2006-07-25 19:06 ` Andrew Morton [this message]
2006-07-29 23:45 ` [RFC] input: Wacom tablet driver for simple X hotplugging Magnus Vigerlöf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060725120645.b4dc98ab.akpm@osdl.org \
--to=akpm@osdl.org \
--cc=linux-input@atrey.karlin.mff.cuni.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=pingc@wacom.com \
--cc=wigge@bigfoot.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).