2004-06-01 08:18:44

by Tuukka Toivonen

[permalink] [raw]
Subject: Re: SERIO_USERDEV patch for 2.6

On Sun, 30 May 2004, Vojtech Pavlik wrote:

>The newest I could find:
>linux-2.6.5-userdev.20040507.patch

Yes, it's still the newest. Applies cleanly against 2.6.6 too.
I didn't want to add features before getting some feedback.

>Coexisting would mean that when someone wants to open the raw device,
>the serio layer would disconnect the psmouse driver, and give control to
>the raw device instead. I believe that could work.

If the user is careful, in practice both kernel and userspace drivers
can work simultaneously with the current code. But if you prefer otherwise,
I'll change it, no problem. However, this could take a few days
or so (busy with real work).

Dmitry suggested adding a kernel parameter to specify which ports would
allow to be read in raw mode and which would be handled by kernel drivers.
In my opinion, almost the same is achieved more conveniently by handling
in raw mode simply exactly those ports that are opened from userspace
(ie. "cat serio1" would disconnect kernel driver), and everything else by
kernel drivers.
No additional parameters would be then necessary, nor module reloads
to change anything.

The only exception I can see is if the kernel driver is loaded first
and its autodetection confuses a device. Or, for example, kernel
driver sets Touchpad into its custom mode, but then user wants to run
standard PS/2 mouse driver in user space. I don't know if the latter
driver could reset the Touchpad back into PS/2 mouse emulation mode.

>I'd like to keep it separate from the
>serio.c file, although it's obvious it'll require to be linked to it
>statically, because it needs hooks there

Agreed. I'll do that (serio-dev.c). Disadvantage: the functions
can not be anymore inline (unless I put lots of stuff into header
file) which may make code less efficient. Hopefully not significantly.

Also, I have to rename serio.c into serio-core.c so that it can be linked
into serio.o with serio-dev.o. The diff will be a bit ugly.

>It's indeed time for me to examine the SERIO_USERDEV patch, and I

Thanks for that :)

>I don't need to test your [San Dan Lee's] userspace drivers,
>as I'm not interested in those.

Fair enough. Just provide a mechanism that allows user space
protocol drivers, and let other people do them if they insist ;)


2004-06-03 10:22:33

by Tuukka Toivonen

[permalink] [raw]
Subject: [PATCH] Re: SERIO_USERDEV patch for 2.6

New version released which addresses the issues you mentioned:
wget \
http://www.ee.oulu.fi/~tuukkat/tmp/linux-2.6.6-userdev.20040603.patch

>On Sun, 30 May 2004, Vojtech Pavlik wrote:
>>Coexisting would mean that when someone wants to open the raw device,
>>the serio layer would disconnect the psmouse driver, and give control to
>>the raw device instead. I believe that could work.

Done, with slight modification: if the device is opened in read-only mode,
the kernel drivers are not disconnected. This way the serio port could be
controlled via IOCTL even when assigned to kernel drivers, if it ever
becomes useful. Useful also for debugging etc.

>>I'd like to keep it separate from the
>>serio.c file, although it's obvious it'll require to be linked to it
>>statically, because it needs hooks there

Done. It's now in serio-dev.c. I also added serio-dev.h, which
unfortunately is slightly messy so that I was able to inline some
functions. It would be trivial to clean it up, if I wouldn't inline
those functions.

I also had to rename serio.c into serio-core.c, which makes the actual
changes into the file unobvious from the patch above. I made a separate
diff about that, shown below (just for easy comparison).

The last change I made converts slashes '/' into underscores '_' (so
eg. isa0060/serio0 is changed to isa0060_serio0 in /proc/misc, /dev
with devfs, and /sys) because Sau Dan Lee reported that slashes don't work
well with sysfs.

I tested the patch in a couple of machines, especially the new features.
Seems to work finely.

Compared to Dmitry's rawdev patch, this has a number of advantages:

- Supports all serio ports without any additional
kernel parameters. Binding between kernel/user driver is done
automatically.

- Supports multiple drivers on the same port, unlike Dmitry's rawdev,
which works just similarly as 2.4.x psaux which didn't work with e.g.
X11 and gpm together without a repeater feed, because multiple
drivers are fighting for the byte streams.
(Doesn't still allow multiple kernel drivers nor a kernel driver
with a complete user drivers with write access since the required
infrastructure is missing in serio.c aka serio-core.c: kernel internal
drivers can't lock a port).

- Dmitry's rawdev appears to assign port numbers quite randomly. If some
changes in i8042.c are done which register the ports in different order,
rawdev would give ports different numbers, and since it doesn't appear
to otherwise be possible to detect from userland which is which, drivers
would use bad ports.
It's even worse if also other port drivers than i8042.c would support
raw devices, because then the module loading order would matter.
(or so it looks from the code, I didn't actually try it).

[Do not apply the patch below, apply the complete patch given above]

--- linux-2.6.6/drivers/input/serio/serio.c Wed Jun 2 20:23:11 2004
+++ linux-2.6.6userdev/drivers/input/serio/serio-core.c Wed Jun 2 23:30:21 2004
@@ -30,6 +30,8 @@
* Changes:
* 20 Jul. 2003 Daniele Bellucci <[email protected]>
* Minor cleanups.
+ * 31 May 2004 Tuukka Toivonen <[email protected]>
+ * Added hooks for serio-dev.c, renamed from serio.c.
*/

#include <linux/stddef.h>
@@ -43,6 +45,10 @@
#include <linux/suspend.h>
#include <linux/slab.h>

+#ifdef CONFIG_SERIO_USERDEV
+#include "serio-dev.h"
+#endif
+
MODULE_AUTHOR("Vojtech Pavlik <[email protected]>");
MODULE_DESCRIPTION("Serio abstraction core");
MODULE_LICENSE("GPL");
@@ -67,16 +73,20 @@
struct list_head node;
};

-static DECLARE_MUTEX(serio_sem);
-static LIST_HEAD(serio_list);
+DECLARE_MUTEX(serio_sem);
+LIST_HEAD(serio_list);
static LIST_HEAD(serio_dev_list);
static LIST_HEAD(serio_event_list);
static int serio_pid;

-static void serio_find_dev(struct serio *serio)
+void serio_find_dev(struct serio *serio)
{
struct serio_dev *dev;

+#ifdef CONFIG_SERIO_USERDEV
+ if (serio_userdev_writers(serio) > 0)
+ return;
+#endif
list_for_each_entry(dev, &serio_dev_list, node) {
if (serio->dev)
break;
@@ -189,18 +199,25 @@
irqreturn_t serio_interrupt(struct serio *serio,
unsigned char data, unsigned int flags, struct pt_regs *regs)
{
- irqreturn_t ret = IRQ_NONE;
+ irqreturn_t r, ret = IRQ_NONE;
+ int rescan = 1;

- if (serio->dev && serio->dev->interrupt) {
- ret = serio->dev->interrupt(serio, data, flags, regs);
- } else {
- if (!flags) {
- if ((serio->type == SERIO_8042 ||
- serio->type == SERIO_8042_XL) && (data != 0xaa))
- return ret;
- serio_rescan(serio);
- ret = IRQ_HANDLED;
- }
+#ifdef CONFIG_SERIO_USERDEV
+ ret = serio_userdev_newbyte(serio, data);
+ if (ret == IRQ_HANDLED)
+ rescan = 0;
+#endif
+ if (serio->dev && serio->dev->interrupt) {
+ r = serio->dev->interrupt(serio, data, flags, regs);
+ if (ret == IRQ_NONE) ret = r;
+ rescan = 0;
+ }
+ if (rescan && !flags) {
+ if ((serio->type == SERIO_8042 ||
+ serio->type == SERIO_8042_XL) && (data != 0xaa))
+ return ret;
+ serio_rescan(serio);
+ ret = IRQ_HANDLED;
}
return ret;
}
@@ -231,6 +248,9 @@
{
list_add_tail(&serio->node, &serio_list);
serio_find_dev(serio);
+#ifdef CONFIG_SERIO_USERDEV
+ serio_userdev_init(serio);
+#endif
}

void serio_unregister_port(struct serio *serio)
@@ -257,6 +277,11 @@
*/
void __serio_unregister_port(struct serio *serio)
{
+#ifdef CONFIG_SERIO_USERDEV
+ /* Here we assume that the interrupt handler is not anymore
+ * called and is not still executing from previous call */
+ serio_userdev_cleanup(serio);
+#endif
serio_invalidate_pending_events(serio);
list_del_init(&serio->node);
if (serio->dev && serio->dev->disconnect)
@@ -268,9 +293,14 @@
struct serio *serio;
down(&serio_sem);
list_add_tail(&dev->node, &serio_dev_list);
- list_for_each_entry(serio, &serio_list, node)
+ list_for_each_entry(serio, &serio_list, node) {
+#ifdef CONFIG_SERIO_USERDEV
+ if (serio_userdev_writers(serio) > 0)
+ continue;
+#endif
if (!serio->dev && dev->connect)
dev->connect(serio, dev);
+ }
up(&serio_sem);
}

@@ -293,6 +323,12 @@
int serio_open(struct serio *serio, struct serio_dev *dev)
{
serio->dev = dev;
+#ifdef CONFIG_SERIO_USERDEV
+ /* When there are processes having the userdev open,
+ * the port is already open and needs not to be re-opened. */
+ if (serio_userdev_users(serio) > 0)
+ return 0;
+#endif
if (serio->open(serio)) {
serio->dev = NULL;
return -1;
@@ -303,7 +339,11 @@
/* called from serio_dev->connect/disconnect methods under serio_sem */
void serio_close(struct serio *serio)
{
- serio->close(serio);
+#ifdef CONFIG_SERIO_USERDEV
+ /* Close only if the port is not open by userspace drivers */
+ if (serio_userdev_users(serio) <= 0)
+#endif
+ serio->close(serio);
serio->dev = NULL;
}

2004-06-03 12:21:26

by Sau Dan Lee

[permalink] [raw]
Subject: Re: [PATCH] Re: SERIO_USERDEV patch for 2.6

>>>>> "Tuukka" == Tuukka Toivonen <[email protected]> writes:


>> On Sun, 30 May 2004, Vojtech Pavlik wrote:
>>> Coexisting would mean that when someone wants to open the raw
>>> device, the serio layer would disconnect the psmouse driver,
>>> and give control to the raw device instead. I believe that
>>> could work.

Tuukka> Done, with slight modification: if the device is opened in
Tuukka> read-only mode, the kernel drivers are not
Tuukka> disconnected. This way the serio port could be controlled
Tuukka> via IOCTL even when assigned to kernel drivers, if it ever
Tuukka> becomes useful. Useful also for debugging etc.

Thanks for the great work, Tuukka! :)

I've just tried it. I loaded 'psmouse' and 'mousedev' and
/proc/bus/input/devices does show the PS/2 AUX port being treated as a
mouse device, handled by 'mousedev'. Then, I start gpm on
/dev/misc/isa0060_serio1, and it works successfully. The mouse device
disappears from /proc/bus/input/devices. When I kill gpm, the kernel
gets control again and the mouse device shows up again in
/proc/.../devices.



Tuukka> The last change I made converts slashes '/' into
Tuukka> underscores '_' (so eg. isa0060/serio0 is changed to
Tuukka> isa0060_serio0 in /proc/misc, /dev with devfs, and /sys)
Tuukka> because Sau Dan Lee reported that slashes don't work well
Tuukka> with sysfs.

It now works in sysfs.

$ cat /sys/class/misc/psaux/dev
10:1
$ cat /sys/class/misc/isa0060_serio1/dev
10:63


Tuukka> I tested the patch in a couple of machines, especially the
Tuukka> new features. Seems to work finely.

Works fine on mine, too.



Tuukka> Compared to Dmitry's rawdev patch, this has a number of
Tuukka> advantages:

Tuukka> - Supports all serio ports without any additional kernel
Tuukka> parameters. Binding between kernel/user driver is done
Tuukka> automatically.

This is wonderful! Or consider my serio_switch patch. We don't need
to want for 2.6.10 when they would eventually add that together with
the sysfs interface.


Tuukka> - Supports multiple drivers on the same port, unlike
Tuukka> Dmitry's rawdev, which works just similarly as 2.4.x psaux
Tuukka> which didn't work with e.g. X11 and gpm together without
Tuukka> a repeater feed, because multiple drivers are fighting for
Tuukka> the byte streams.

On my system X and gpm do live with each other without conflicts, both
accessing /dev/psaux (either on 2.4 or 2.6+SERIO_USERDEV). Both
programs can detect VT switching and close the port when losing
"focus".


Tuukka> (Doesn't still allow multiple kernel drivers nor a kernel
Tuukka> driver with a complete user drivers with write access
Tuukka> since the required infrastructure is missing in serio.c
Tuukka> aka serio-core.c: kernel internal drivers can't lock a
Tuukka> port).

That's a part of the design of the whole serio subsystem. Unless the
design is reworked, you can't do much about it. The basic assumption
is: each port can be connected to only one device, whether the device
is in kernel or userland. That our SERIO_USERDEV allows userland to
overhear (O_RDONLY) the traffic is a special bonus feature.



--
Sau Dan LEE ???u??(Big5) ~{@nJX6X~}(HZ)

E-mail: [email protected]
Home page: http://www.informatik.uni-freiburg.de/~danlee

2004-06-03 13:47:12

by Luciano Moreira - igLnx

[permalink] [raw]
Subject: Re: [PATCH] Re: SERIO_USERDEV patch for 2.6

unsubscribe linux-kernel

>
>

2004-06-03 21:22:21

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [PATCH] Re: SERIO_USERDEV patch for 2.6

On Thu, Jun 03, 2004 at 01:21:21PM +0300, Tuukka Toivonen wrote:
> New version released which addresses the issues you mentioned:
> wget \
> http://www.ee.oulu.fi/~tuukkat/tmp/linux-2.6.6-userdev.20040603.patch
>
> >On Sun, 30 May 2004, Vojtech Pavlik wrote:
> >>Coexisting would mean that when someone wants to open the raw device,
> >>the serio layer would disconnect the psmouse driver, and give control to
> >>the raw device instead. I believe that could work.
>
> Done, with slight modification: if the device is opened in read-only mode,
> the kernel drivers are not disconnected. This way the serio port could be
> controlled via IOCTL even when assigned to kernel drivers, if it ever
> becomes useful. Useful also for debugging etc.
>
> >>I'd like to keep it separate from the
> >>serio.c file, although it's obvious it'll require to be linked to it
> >>statically, because it needs hooks there
>
> Done. It's now in serio-dev.c. I also added serio-dev.h, which
> unfortunately is slightly messy so that I was able to inline some
> functions. It would be trivial to clean it up, if I wouldn't inline
> those functions.
>
> I also had to rename serio.c into serio-core.c, which makes the actual
> changes into the file unobvious from the patch above. I made a separate
> diff about that, shown below (just for easy comparison).
>
> The last change I made converts slashes '/' into underscores '_' (so
> eg. isa0060/serio0 is changed to isa0060_serio0 in /proc/misc, /dev
> with devfs, and /sys) because Sau Dan Lee reported that slashes don't work
> well with sysfs.
>
> I tested the patch in a couple of machines, especially the new features.
> Seems to work finely.

I like it. Give me a while to evaluate your and Dmitry's approach in
relation to future ...

--
Vojtech Pavlik
SuSE Labs, SuSE CR