linuxconsole-dev

event mixer patch

[Charles D. <cd...@mv...>]

The attached patch creates a mixer device for the event interface, as
already exists for mice; indeed, it borrows (very) heavily from the
existing mousedev driver. create the device with "mknod events c 13
95". This patch also addresses an (unrelated) oops which turned up in
testing, though there's no promise that it fixes more than it induces.

Enjoy!

Re: event mixer patch

[Paul M. <pm...@mv...>]

On Mon, Jun 18, 2001 at 10:09:56PM -0700, Charles Duffy wrote:
> @@ -354,7 +354,8 @@
>  	while (*handlerptr && (*handlerptr != handler))
>  		handlerptr = &((*handlerptr)->next);
>  
> -	*handlerptr = (*handlerptr)->next;
> +	if(*handlerptr)
> +		*handlerptr = (*handlerptr)->next;
>  
>  /*
>   * Remove minors.

This brings up a few other potential issues in various other portions of
the code. This current approach is far too ugly, there are all sorts of
opportunities where it could be dereferencing NULL, etc.

A more general solution for this kind of finding/removal of nodes from
the linked list would likely be a better solution.

What does everyone think about something like the attached patch as a
more unified solution?

Regards,

-- 
Paul Mundt <pm...@mv...>
MontaVista Software, Inc.

Re: event mixer patch

[James S. <jsi...@tr...>]

> This brings up a few other potential issues in various other portions of
> the code. This current approach is far too ugly, there are all sorts of
> opportunities where it could be dereferencing NULL, etc.
> 
> A more general solution for this kind of finding/removal of nodes from
> the linked list would likely be a better solution.
> 
> What does everyone think about something like the attached patch as a
> more unified solution?

Their is linux/include/linux/list.h which is a generic interface for
double link list. From what I see their is no single link list
implementation.

Re: event mixer patch

[Paul M. <pm...@mv...>]

On Tue, Jun 19, 2001 at 10:33:17AM -0700, James Simmons wrote:
> > This brings up a few other potential issues in various other portions of
> > the code. This current approach is far too ugly, there are all sorts of
> > opportunities where it could be dereferencing NULL, etc.
> >=20
> > A more general solution for this kind of finding/removal of nodes from
> > the linked list would likely be a better solution.
> >=20
> > What does everyone think about something like the attached patch as a
> > more unified solution?
>=20
> Their is linux/include/linux/list.h which is a generic interface for
> double link list. From what I see their is no single link list
> implementation.

I meant a more unified solution as it applied to the input code. It would
be a nuisance to rewrite all the list handling in the input code to utilize
a doubly linked list, and I don't think there's quite enough demand to
bother writing a single linked list implementation.

The patch is simply to clean up some of the input code and to avoid code
duplication and a few potential oopses from occasionally dereferencing a
NULL pointer.

On top of that, the patch is pretty much isolated to drivers/input/input.c
and suits its purpose fairly well IMO.

Regards,

--=20
Paul Mundt <pm...@mv...>
MontaVista Software, Inc.

Re: event mixer patch

[James S. <jsi...@tr...>]

> I meant a more unified solution as it applied to the input code. 

Okay. Misunderstood you.

> The patch is simply to clean up some of the input code and to avoid code
> duplication and a few potential oopses from occasionally dereferencing a
> NULL pointer.

Is this a seperate patch or the patch from Charles? I looked at his patch
and it has only one line change in input.c. Very nice patch. After I'm
done with power management today I will incorporate it. Probable some
time tomorrow unless Vojtech has any objections.

@@ -354,7 +354,8 @@
        while (*handlerptr && (*handlerptr != handler))
                handlerptr = &((*handlerptr)->next);

-       *handlerptr = (*handlerptr)->next;
+       if(*handlerptr)
+               *handlerptr = (*handlerptr)->next;

Re: event mixer patch

[Franz S. <Fra...@la...>]

On Tuesday 19 June 2001 23:15, James Simmons wrote:
> > I meant a more unified solution as it applied to the input code.
>
> Okay. Misunderstood you.
>
> > The patch is simply to clean up some of the input code and to avoid code
> > duplication and a few potential oopses from occasionally dereferencing a
> > NULL pointer.
>
> Is this a seperate patch or the patch from Charles? I looked at his patch
> and it has only one line change in input.c. Very nice patch. After I'm
> done with power management today I will incorporate it. Probable some
> time tomorrow unless Vojtech has any objections.

Last time I asked Vojtech wasn't very pleased of that idea, eg. it doesn't 
make  much sense to mix in EV_ABS devices.

Maybe it should be a separate module providing a whole class of event mix 
devices, with all EV_KEY and EV_REL mixed on /dev/eventmix0 by default. Then 
there could be ioctl's to attach and detach single event devices onto the 
mixer devices.

Franz.

Re: event mixer patch

[James S. <jsi...@tr...>]

> > Is this a seperate patch or the patch from Charles? I looked at his patch
> > and it has only one line change in input.c. Very nice patch. After I'm
> > done with power management today I will incorporate it. Probable some
> > time tomorrow unless Vojtech has any objections.
> 
> Last time I asked Vojtech wasn't very pleased of that idea, eg. it doesn't 
> make  much sense to mix in EV_ABS devices.

Your right. I have this problem with a userland program I wrote. The way
I got around that was to emulate each absolute device as a relative
device. 

> Maybe it should be a separate module providing a whole class of event mix 
> devices, with all EV_KEY and EV_REL mixed on /dev/eventmix0 by default. Then 
> there could be ioctl's to attach and detach single event devices onto the 
> mixer devices.

The real underlying problem is knowing when a device gets attached and
detached. How do we know that? It is not easy to solve. A mixer sort of
gets around this problem. Then we run into the problem of EV_ABS type
devices. The mixer device could be used to tell us when a new device is
added or subtracted. Then we can decided to add the new device to the
mixer. 

Re: event mixer patch

[Charles D. <cd...@mv...>]

On Tue, Jun 19, 2001 at 03:48:01PM -0700, James Simmons wrote:
> > Maybe it should be a separate module providing a whole class of
> > event mix devices, with all EV_KEY and EV_REL mixed on
> > /dev/eventmix0 by default. Then there could be ioctl's to attach
> > and detach single event devices onto the mixer devices.
>=20
> The real underlying problem is knowing when a device gets attached
> and detached. How do we know that? It is not easy to solve. A mixer
> sort of gets around this problem. Then we run into the problem of
> EV_ABS type devices. The mixer device could be used to tell us when
> a new device is added or subtracted. Then we can decided to add the
> new device to the mixer.=20

If some changes were made to the data structure, it should be possible
to provide attach/detach notices through the mixer, as well as
informing applications of which device an event came from.

I'll try writing a mixer device which does this some time between now
and this weekend, inclusive.

Re: event mixer patch

[Vojtech P. <vo...@su...>]

On Tue, Jun 19, 2001 at 03:48:01PM -0700, James Simmons wrote:
> 
> > > Is this a seperate patch or the patch from Charles? I looked at his patch
> > > and it has only one line change in input.c. Very nice patch. After I'm
> > > done with power management today I will incorporate it. Probable some
> > > time tomorrow unless Vojtech has any objections.
> > 
> > Last time I asked Vojtech wasn't very pleased of that idea, eg. it doesn't 
> > make  much sense to mix in EV_ABS devices.
> 
> Your right. I have this problem with a userland program I wrote. The way
> I got around that was to emulate each absolute device as a relative
> device. 
> 
> > Maybe it should be a separate module providing a whole class of event mix 
> > devices, with all EV_KEY and EV_REL mixed on /dev/eventmix0 by default. Then 
> > there could be ioctl's to attach and detach single event devices onto the 
> > mixer devices.
> 
> The real underlying problem is knowing when a device gets attached and
> detached. 

USB currently handles this with a select() on /proc/bus/usb/devices. I
think we could do the same. Actually a /proc/..../input/devices file
would be quite useful.

As for detaching, evdev now returns -ENODEV on read() when a device is
detached.

> How do we know that? It is not easy to solve. A mixer sort of
> gets around this problem. Then we run into the problem of EV_ABS type
> devices. The mixer device could be used to tell us when a new device is
> added or subtracted. Then we can decided to add the new device to the
> mixer. 

-- 
Vojtech Pavlik
SuSE Labs

Re: event mixer patch

[James S. <jsi...@tr...>]

> USB currently handles this with a select() on /proc/bus/usb/devices. I
> think we could do the same. Actually a /proc/..../input/devices file
> would be quite useful.

:-) Do you have example code for the usb system on how to detect this. I
could really use this for work.

Re: event mixer patch

[Vojtech P. <vo...@su...>]

On Fri, Jun 22, 2001 at 01:18:40PM -0700, James Simmons wrote:

> > USB currently handles this with a select() on /proc/bus/usb/devices. I
> > think we could do the same. Actually a /proc/..../input/devices file
> > would be quite useful.
> 
> :-) Do you have example code for the usb system on how to detect this. I
> could really use this for work.

simple, really. open(fd,/proc/bus/usb/devices), select(fd). Select will
stop blocking once the file changes.

-- 
Vojtech Pavlik
SuSE Labs

Re: event mixer patch

[James S. <jsi...@tr...>]

> simple, really. open(fd,/proc/bus/usb/devices), select(fd). Select will
> stop blocking once the file changes.

That I got. How do you determine which device got attached or detected?

Re: event mixer patch

[Vojtech P. <vo...@su...>]

On Sun, Jun 24, 2001 at 08:26:45AM -0700, James Simmons wrote:
> 
> > simple, really. open(fd,/proc/bus/usb/devices), select(fd). Select will
> > stop blocking once the file changes.
> 
> That I got. How do you determine which device got attached or detected?

You read the file contents, of course. You get a list of attached
devices, all with names and vendor/product IDs, possibly more. You
compare it to what you've read last time, the difference is devices that
connected/disconnected.

-- 
Vojtech Pavlik
SuSE Labs

Re: event mixer patch

[Matan Ziv-Av <ma...@sv...>]

On Sun, 24 Jun 2001, Vojtech Pavlik wrote:

> On Sun, Jun 24, 2001 at 08:26:45AM -0700, James Simmons wrote:
> >
> > > simple, really. open(fd,/proc/bus/usb/devices), select(fd). Select will
> > > stop blocking once the file changes.
> >
> > That I got. How do you determine which device got attached ordetected?
> 
> You read the file contents, of course. You get a list of attached
> devices, all with names and vendor/product IDs, possibly more. You
> compare it to what you've read last time, the difference is devices that
> connected/disconnected.


What about non usb devices?
Conncecting/disconnecting devices should be solved in the input/evdev
layer, without relying on other subsystems. 

-- 
Matan Ziv-Av.                         ma...@sv...

Re: event mixer patch

[Vojtech P. <vo...@su...>]

On Mon, Jun 25, 2001 at 06:17:35PM +0300, Matan Ziv-Av wrote:
> On Sun, 24 Jun 2001, Vojtech Pavlik wrote:
> 
> > On Sun, Jun 24, 2001 at 08:26:45AM -0700, James Simmons wrote:
> > >
> > > > simple, really. open(fd,/proc/bus/usb/devices), select(fd). Select will
> > > > stop blocking once the file changes.
> > >
> > > That I got. How do you determine which device got attached ordetected?
> > 
> > You read the file contents, of course. You get a list of attached
> > devices, all with names and vendor/product IDs, possibly more. You
> > compare it to what you've read last time, the difference is devices that
> > connected/disconnected.
> 
> 
> What about non usb devices?
> Conncecting/disconnecting devices should be solved in the input/evdev
> layer, without relying on other subsystems. 

I was suggesting creating /proc/input/devices, working the same way as
the USB one does.

-- 
Vojtech Pavlik
SuSE Labs

Re: event mixer patch

[Charles D. <cd...@mv...>]

On Tue, Jun 19, 2001 at 11:39:55PM +0200, Franz Sirl wrote:
> Last time I asked Vojtech wasn't very pleased of that idea, eg. it
> doesn't make much sense to mix in EV_ABS devices.

True... at least, not unless there's a means of distinguishing between
them.  How about something similar to the following, applied on top of
my earlier patch? (Changing the type of event to unsigned char and
adding an extra field for the purpose would be cleaner, or having an
entirely different data type emitted by the mixer... this is intended
as more of a conceptual demonstration than anything else).

One advantage of using a modified evdev rather than an entirely
separate module, btw, is the ability to pass out a device number which
can be used by the userland program to open the appropriate
device-specific interface as desired. Separating the mixer from evdev
itself leaves no easy way to do that.

--- evdev.c.simplemix   Tue Jun 19 16:49:48 2001
+++ evdev.c     Tue Jun 19 20:58:15 2001
@@ -75,7 +75,12 @@
                while(list) {

                        get_fast_time(&list->buffer[list->head].time);
-                       list->buffer[list->head].type = type;
+                       if(*evdev == &evdev_mix) {
+                               list->buffer[list->head].type = type |
+                                       (((struct evdev*)handle->private)->minor << 8);
+                       } else {
+                               list->buffer[list->head].type = type;
+                       }
                        list->buffer[list->head].code = code;
                        list->buffer[list->head].value = value;
                        list->head = (list->head + 1) & (EVDEV_BUFFER_SIZE - 1);

Re: event mixer patch

[Franz S. <Fra...@la...>]

At 00:48 20.06.2001, James Simmons wrote:

> > > Is this a seperate patch or the patch from Charles? I looked at his patch
> > > and it has only one line change in input.c. Very nice patch. After I'm
> > > done with power management today I will incorporate it. Probable some
> > > time tomorrow unless Vojtech has any objections.
> >
> > Last time I asked Vojtech wasn't very pleased of that idea, eg. it doesn't
> > make  much sense to mix in EV_ABS devices.
>
>Your right. I have this problem with a userland program I wrote. The way
>I got around that was to emulate each absolute device as a relative
>device.

But do we need to support EV_ABS devices at all on mixer devices? I would 
rather restrict us to applications that make sense, at least I can't think 
of a legitimate reason to mix them.


> > Maybe it should be a separate module providing a whole class of event mix
> > devices, with all EV_KEY and EV_REL mixed on /dev/eventmix0 by default. 
> Then
> > there could be ioctl's to attach and detach single event devices onto the
> > mixer devices.
>
>The real underlying problem is knowing when a device gets attached and
>detached. How do we know that? It is not easy to solve. A mixer sort of
>gets around this problem. Then we run into the problem of EV_ABS type
>devices. The mixer device could be used to tell us when a new device is
>added or subtracted. Then we can decided to add the new device to the
>mixer.

Well, I thought the hotplug stuff works for USB? I have never used it 
though, cause I have currently no need for it.
Excuse me if I'm wrong here, but I thought the procedure would look like 
this for example:

  - USB mouse is plugged in and attached to eventX
  - hotplug scripts get called
  - scripts determine if THIS mouse is assigned to a mixer device
  - scripts issues ioctl("attach eventX to eventmixY") if assigned (maybe a 
packet controlled /dev/eventmixcontrol makes more sense than an ioctl)

The application using eventmixY won't even notice anything, except for the 
additional packets.

Hmm, if we had a single /dev/eventstatus device receiving 
CONNECT/DISCONNECT messages (or should we send them per eventX device as 
EV_STATUS messages?) we would be independent of the hotplug system, dunno 
if that has advantages/disadvantages...

Anyway, thinking about the 3 major scenarios for the input system usually 
helps me:

1. 1 user, 1/2 head, multi inputs: currently the major linux use I guess, 
so anything we do should give these users a nice plug-and-play feeling. Eg. 
plugging in a keyboard should make it immediately useable (currently not 
possible with _event_ devices, unless the application scans them all)

2. multi user, multi head, multi input: here we have to make sure that we 
don't loose the user/input assignement, eg. unplugging/replugging an USB 
joystick should let me continue to play the game :-)

3. embedded/industrial use: here we have all possible kinds of sensors and 
actuators handled by specialized applications and so we just have to give 
the applications all the necessary information to allow them to react 
properly. Eg. should a bus failure result in a EV_DETACH(reason bus 
failure) message?

The simple eventmix patch only helps for point 1 above, that's why I think 
it should be extended to be useful for point 2/3 too..

Franz.

Re: event mixer patch

[James S. <jsi...@tr...>]

> But do we need to support EV_ABS devices at all on mixer devices? I would 
> rather restrict us to applications that make sense, at least I can't think 
> of a legitimate reason to mix them.

Yes if the only device is a touchscreen which usually have absolute value.
You could then get software like gpm or X running in no time using the
PS/2 mouse interface. 
 
> Hmm, if we had a single /dev/eventstatus device receiving 
> CONNECT/DISCONNECT messages (or should we send them per eventX device as 
> EV_STATUS messages?) we would be independent of the hotplug system, dunno 
> if that has advantages/disadvantages...

Disconnect is easy to handle. You get -ENODEV when you attempt a read. It
is the connect that is harder to deal with.
 
> Anyway, thinking about the 3 major scenarios for the input system usually 
> helps me:
> 
> 1. 1 user, 1/2 head, multi inputs: currently the major linux use I guess, 
> so anything we do should give these users a nice plug-and-play feeling. Eg. 
> plugging in a keyboard should make it immediately useable (currently not 
> possible with _event_ devices, unless the application scans them all)

Kernel wise i.e the console system it auto detects. As for userland I have
encountered this problem. I still haven't found a solution except to have
the user run a app to make everything aware a new device has been plugged
in. This is done by scanning the /dev/input directory. This is not a good
solution IMHO. If you a embedded device and lack the software to start the
hardware or lack the ability to login into the serial console to setup the
device then you are in trouble. 

> 2. multi user, multi head, multi input: here we have to make sure that we 
> don't loose the user/input assignement, eg. unplugging/replugging an USB 
> joystick should let me continue to play the game :-)

Ah. The topology problem. The solution here is to make a particular device
belong to a particular group. If I plug in a joystick I don't so other
person at some other desk being to able to see what my values are. They
could cheat at the game then. The trick is then getting that joystick once
plugged back in to the correct desktop. This is very trick. I still
haven't figured it out yet.

> 3. embedded/industrial use: here we have all possible kinds of sensors and 
> actuators handled by specialized applications and so we just have to give 
> the applications all the necessary information to allow them to react 
> properly. Eg. should a bus failure result in a EV_DETACH(reason bus 
> failure) message?

Hum??

> The simple eventmix patch only helps for point 1 above, that's why I think 
> it should be extended to be useful for point 2/3 too..

P.S
   I like to down the road work on a input filesystem which I hope will
also address these problems. I remember in one discussion with Viro about
needing a xxdevctrl file for each device file system. I question Viro if
we really needed it. When it comes to hotplug and power management perhaps
he was right.

Re: event mixer patch

[Paul M. <pm...@mv...>]

On Tue, Jun 19, 2001 at 02:15:22PM -0700, James Simmons wrote:
> > The patch is simply to clean up some of the input code and to avoid code
> > duplication and a few potential oopses from occasionally dereferencing a
> > NULL pointer.
> 
> Is this a seperate patch or the patch from Charles? I looked at his patch
> and it has only one line change in input.c. Very nice patch. After I'm
> done with power management today I will incorporate it. Probable some
> time tomorrow unless Vojtech has any objections.
> 
Seperate patch, it was attached to my reply to Charles' original message.
I've attached it to this one as well. I'll commit it if no one has any
problems with it.

Regards,

-- 
Paul Mundt <pm...@mv...>
MontaVista Software, Inc.

Re: event mixer patch

[James S. <jsi...@tr...>]

I don't see a problem with this patch but lets see what Vojtech has to
say.  

Re: event mixer patch

[James S. <jsi...@tr...>]

> Seperate patch, it was attached to my reply to Charles' original message.
> I've attached it to this one as well. I'll commit it if no one has any
> problems with it.

Tested it and added it. I also noticed it could be used for mousedev.c and
a few other files in the input directory.

Re: event mixer patch

[Vojtech P. <vo...@su...>]

On Tue, Jun 19, 2001 at 10:33:17AM -0700, James Simmons wrote:
> 
> > This brings up a few other potential issues in various other portions of
> > the code. This current approach is far too ugly, there are all sorts of
> > opportunities where it could be dereferencing NULL, etc.
> > 
> > A more general solution for this kind of finding/removal of nodes from
> > the linked list would likely be a better solution.
> > 
> > What does everyone think about something like the attached patch as a
> > more unified solution?
> 
> Their is linux/include/linux/list.h which is a generic interface for
> double link list. From what I see their is no single link list
> implementation.

I'd agree with rewriting the input stuff to use doubly linked lists
where applicable. The removing stuff in the signle linked ones used
there is rather ugly.

-- 
Vojtech Pavlik
SuSE Labs