linuxconsole-dev

Re: Role of close() in struct input_dev

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

On Tue, Jun 19, 2001 at 05:39:45PM -0700, James Simmons wrote:
> 
> > In fact, I realised the driver doesn't really know when a process does an
> > open(). Indeed, input_open_device is called only when the first process
> > opens the device, and input_close_device when all processes closed it.
> 
> Are you sure about that? For one thing each time you open /dev/mouse and
> /dev/event both devices call input_open_device. This is a case where the
> same hardware is opened twice. Also in input_close_device we have 

But it's a per-driver open, not per-process open.
 
> handle->open--;
> 
> I'm actually using this for power management.
> 
> > Frankly, I don't really like the fact that evdev can be kfreed both by
> > evdev_disconnect and evdev_connect. Furthermore, the choice is done after
> > the value of evdev->exist, whose access is not protected (possible bug if
> > the last process releases the device while the device is being
> > disconnected, on a smp machine (weird case, though))
> 
> I haven't thought about that problem but I have noticed the handle list
> isn't protected with a lock. On a SMP machine you the list being scanned
> and another thread removing or adding a new handle. 

Yep. It ain't SMP safe.

> > Anyway, it seems we have to check evdev->exist before doing anything in
> > evdev_{ioctl,read,write...}, I guess this is what caused the oops I got.
> 
> Please post the result of oops processed by ksymoops. This will help us
> find the problem.

-- 
Vojtech Pavlik
SuSE Labs

Re: Role of close() in struct input_dev

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

On Tue, Jun 19, 2001 at 10:42:27PM +0200, Johann Deneux wrote:
> On Tue, 19 Jun 2001, James Simmons wrote:
> 
> > 
> > > Now the question is: What is the role of input_dev::close/open ?
> > > Are they meant as "init/deinit the device" or "establish/break connection
> > > with processes" (or a mix of both) ?
> > 
> > If you look at input.c connect and disconnect are used by input_register
> > and input_unregister which are called when you insmod the device or at
> > boot time. So they are used to setup the hardware and the data structs for
> > the device. Now open and close are called input_open_device and
> > input_close_device. Now if look at what calls input_[opne/close]_device
> > they are all some kind of userland interface i.e evdev, mousedev, joydev.c
> > etc. 
> 
> There are parts of evdev functions called by userland, and other parts
> called by other modules. Namely, evdev_disconnect can be called by serio,
> for example.

No, evdev_disconnect will always be called by input.c, which will always
be called by some input device driver calling the unregister() function.

It can be triggered by serio/usb/rmmod/whatever else, though.

> The problem is still there: I would expect
> input_open_device/input_close_devise to be called at least the same
> number of times (perhaps we should even take care of the order).

We can't take care of the order - that is not possible. But they indeed
should be called the same number of times. If they are not, it's a bug.

-- 
Vojtech Pavlik
SuSE Labs

Re: Q: interface / way to "generate" input events ?

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

On Fri, Jun 22, 2001 at 01:41:02PM +0200, Ralf Ackermann wrote:
> Hello,
> 
> after reading input.txt in the kernel documentation my question is:
>  - Is there a way / interface to generate (feed into the kernel)
>    events into the kernel?
>    I mean - the opposite to /dev/input/eventX (that can be read) -
>    i would like to use an interface (e.g. a named pipe) that i can
>    feed data into (e.g. the bytes that normally arrive from a serially
>    attached device) so that they trigger events like a real device.
> Background - that would permit to use a network attached input device by
> just "tunneling" its data to the final destination.
> It would also permit very easy simulation / test of devices. Before i try
> myself - does that mechanism exist already? Which semantics / API does it
> have?

For complete functionality like this you'll need something ala evdev,
but turned inside out. It hasn't been written yet, though I was planning
it.

You can also write() events to evdev, but that won't create the device,
of course.

-- 
Vojtech Pavlik
SuSE Labs

Re: Role of close() in struct input_dev

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

On Tue, Jun 19, 2001 at 09:01:30PM +0200, Johann Deneux wrote:
> Hi !
> 
> I am trying to clean up my addition about multiple open-with-write-access.
> My first idea was to use a flag, only_one_writer in struct evdev, and then
> perform the necessay checks in evdev_open and evdev_release.
> 
> Vojtech suggested that we add a parameter to input_dev::open and pass
> that parameter to device drivers and let them decide. This would indeed
> allow for more flexibility.
> I thought that input_dev::close should be modified the same way.
> 
> I tried to implement this, and while doing so, I noticed that it could be
> that input_dev::open and input_dev::close are not called the same number
> of times. This may or may not be a problem, I don't know.
> 
> Here is a sample scenario:
> 1. Application 1 opens /dev/input/event0
> 2. Application 2 opens /dev/input/event0 (nothing forbids that)
> 3. evdev_disconnect is called (usb device is unplugged, input_attach is
> kiled...)
> 
> Steps 1 and 2 call input_dev::open, step 3 calls input_dev::close.

No. As it is now (or was the last time I remember it), step 2 doesn't
call input_dev->open.

> Now the question is: What is the role of input_dev::close/open ?
> Are they meant as "init/deinit the device" or "establish/break connection
> with processes" (or a mix of both) ?

Originally it was intended to init/deinit the device so that it can
have the USB IRQ running only when the device is used, same with
joysticks, which eat a considerable amount of CPU while polling.

> I would vote for the second possibility.

It's beginning to sound more reasonable, though I think there will be
problems arising with it.

> By the way, if you add a step 4 to my scenario:
> 4. Application 1 or 2 writes to /dev/input/event0
> then the application segfaults, and dmesg tells me someone tried to
> dereference a NULL pointer.

Interesting, gotta try ...

-- 
Vojtech Pavlik
SuSE Labs

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

[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

Re: Q: interface / way to "generate" input events ?

[Ralf A. <ra...@sh...>]

Using a cable and a second computer / serial port is an option for some of
the tests.

I would like to use "higher level" primitives too though - means - i would
like to feed in what a low level driver produces as its output - e.g. "key
with code xxx pressed/released" / pointer moved xxx units".
Not only the more device specific sequence for a certain device.

regards
 ralf


On Fri, 22 Jun
2001, Chris wrote:

> Ralf Ackermann wrote:
>
>
> > Background - that would permit to use a network attached input device by
> > just "tunneling" its data to the final destination.
> > It would also permit very easy simulation / test of devices. Before i try
> > myself - does that mechanism exist already? Which semantics / API does it
> > have?
>
>
> Do you have another computer to hand and a null modem cable? You could
> generate the bytes on one computer and feed them into the serial port of
> the computer your test device is on.
>
> Just a suggestion, flames welcome...
>
> Chris
>

-- 
--
Best regards,
 Ralf Ackermann

Q: interface / way to "generate" input events ?

[Ralf A. <ra...@sh...>]

Hello,

after reading input.txt in the kernel documentation my question is:
 - Is there a way / interface to generate (feed into the kernel)
   events into the kernel?
   I mean - the opposite to /dev/input/eventX (that can be read) -
   i would like to use an interface (e.g. a named pipe) that i can
   feed data into (e.g. the bytes that normally arrive from a serially
   attached device) so that they trigger events like a real device.
Background - that would permit to use a network attached input device by
just "tunneling" its data to the final destination.
It would also permit very easy simulation / test of devices. Before i try
myself - does that mechanism exist already? Which semantics / API does it
have?

best regards
 ralf

Re: Role of close() in struct input_dev

[Johann D. <jo...@Do...>]

On Wed, 20 Jun 2001, James Simmons wrote:

> 
> > Which is, I have just understood that, the wanted behavior. I mistook
> > release and flush. I added a new input_dev::flush() and 
> > input_flush_device() functions to do what I need.
> 
> Flush? This is new. How does a userland take advantage and does this
> function do what I think it does, flush all data stored for a file?

It's called by close(). And so is input_dev::release(). So what's the
difference, you may ask ?
flush() is called at every close(), release() only on the last
close(). That's what the doc says, which leaves a few questions open. I
will play with little programs to check how it exactly behaves.

> 
> > Still, evdev_open can be called several times by a process. evdev_release
> > is called only once, when the last close on the device file is done
> > (according to "Linux 2.4 Internals"). Some list elements could then be
> > left unfreed, thus causing a memory leak. Or did I miss something ???
> 
> Only if it does free all the list elements for this particular device when
> close is called.
>  

We could use evdev_flush() to individually free every list element as it's
closed, and evdev_close() to free all the remaining ones (which should
not exist, btw, as they should have been freed by a evdev_flush())

> > I also added a input_dev::accept() and input_accept_device() functions
> > (hmm, any idea about a better name ? the name misleads the reader into
> > thinking it's the device that gets accepted)
> 
> Yeah it is misleading.

accept_process() would be better. input_accept_process() would be nice,
too, but it brakes the input_{something}_device "rule"

-- 
Johann Deneux
CS student at DoCS (www.docs.uu.se/~johannd) and
ESIL (www.esil.univ-mrs.fr/~jdeneux)

Re: power management

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

> Hmmm, I'll have to take a closer look at how powermanagement is
> implement in Linux.

I have some form of power management going. It compiles but I have to test
it tomorrow. I have to still implement a userland interface to send a
event to power down. I'm hooked on send a EV_KEY with the code being
KEY_SUSPEND or KEY_POWER. As for the touchscreen diming the light should
really be apart of the fbdev api. Especially since laptops have back and
front lights but no touchscreens usually.

> > P.S
> >    serio->write can send only one char at a time? 
> 
> Yes, it has been designed for low speed devices, where the overhead
> doesn't matter. But there is not a problem with redesigning it.

The problem I'm having is I have to create packets and send them to the
serial device. For example for the power LED on the ipaq you have to
create a led packet which is:

typedef struct {
        unsigned char OffOnBlink;       /* 0=off 1=on 2=Blink */
        unsigned char TotalTime;        /* Units of 5 seconds */
        unsigned char OnTime;           /* units of 100m/s */
        unsigned char OffTime;          /* units of 100m/s */
} LED_IN;

Re: Role of close() in struct input_dev

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

> Which is, I have just understood that, the wanted behavior. I mistook
> release and flush. I added a new input_dev::flush() and 
> input_flush_device() functions to do what I need.

Flush? This is new. How does a userland take advantage and does this
function do what I think it does, flush all data stored for a file?

> Still, evdev_open can be called several times by a process. evdev_release
> is called only once, when the last close on the device file is done
> (according to "Linux 2.4 Internals"). Some list elements could then be
> left unfreed, thus causing a memory leak. Or did I miss something ???

Only if it does free all the list elements for this particular device when
close is called.
 
> I also added a input_dev::accept() and input_accept_device() functions
> (hmm, any idea about a better name ? the name misleads the reader into
> thinking it's the device that gets accepted)

Yeah it is misleading. 

> The goal of this function is to decide whether an open() call should be
> accepted or not. I use this to prevent several processes from using
> force-feedback at the same time.

I suggest you add comments about this that can be used for the docs.

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

[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

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

On Wed, Jun 20, 2001 at 01:08:54PM +0100, Justin Cormack wrote:
> Maybe a device that just gives attach, detach events would be enough? I
> am yet another person who has needed some facility like this in userspace.
> But If you just mix events, you lose the names and capabilities of the
> devices and stuff, which I need.

Not necessarily.

There are two means of keeping the names and capabilities still
available which are immediately obvious to me:

- Include the number of the real event device in the mixer's
  information, so that the application can open and query the
  appropriate /dev/input/eventXX device
- Include with each event an event number generated internally to the
  mixer (necessary if it isn't integrated into the event interface)
  and extend the ioctls to apply on a per-device basis.

Re: Role of close() in struct input_dev

[Johann D. <jo...@Do...>]

On Wed, 20 Jun 2001, Johann Deneux wrote:

> On Tue, 19 Jun 2001, James Simmons wrote:
> 
> > 
> > > In fact, I realised the driver doesn't really know when a process does an
> > > open(). Indeed, input_open_device is called only when the first process
> > > opens the device, and input_close_device when all processes closed it.
> > 
> > Are you sure about that? For one thing each time you open /dev/mouse and
> > /dev/event both devices call input_open_device. This is a case where the
> > same hardware is opened twice. [...]
> 
> This is the case because from evdev and mousedev, the device is opened
> only once. What I meant was in the case where two processes or more 
> perform an open one after another. In that case, evdev and mousedev will
> call input_open_device only for the first process.
> Same thing for input_close_device. It's called only when evdev->open falls
> to zero (unless the device is disconnected).

Which is, I have just understood that, the wanted behavior. I mistook
release and flush. I added a new input_dev::flush() and 
input_flush_device() functions to do what I need.
Still, evdev_open can be called several times by a process. evdev_release
is called only once, when the last close on the device file is done
(according to "Linux 2.4 Internals"). Some list elements could then be
left unfreed, thus causing a memory leak. Or did I miss something ???

I also added a input_dev::accept() and input_accept_device() functions
(hmm, any idea about a better name ? the name misleads the reader into
thinking it's the device that gets accepted)
The goal of this function is to decide whether an open() call should be
accepted or not. I use this to prevent several processes from using
force-feedback at the same time.

-- 
Johann Deneux
CS student at DoCS (www.docs.uu.se/~johannd) and
ESIL (www.esil.univ-mrs.fr/~jdeneux)

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: power management

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

On Tue, Jun 19, 2001 at 02:30:02PM -0700, James Simmons wrote:

> So far for power management I have:
> 
> Placed a pm_dev inside input_dev.
> 
> If the input_dev has a struct pm_dev we do the following.
> 
> Every time input_open is called pm_access is called.
> 
> When input_close is called and handle->open==0 I call pm_idle_dev
> 
> For input_unregister_device I call pm_unregister(dev->pm_dev);
> 
> I find pm_register_device too driver specific to make generic. 
> The next problem is power management for that particular device via a
> key/button. I like to power the device off/on and control the level of
> power going to the device. For example the touchscreen I'm working on. It
> would be nice to dim/brighten the frontlight. The question is how to
> implement this fine grain control.

Hmmm, I'll have to take a closer look at how powermanagement is
implement in Linux.

> P.S
>    serio->write can send only one char at a time? 

Yes, it has been designed for low speed devices, where the overhead
doesn't matter. But there is not a problem with redesigning it.

-- 
Vojtech Pavlik
SuSE Labs

Re: Role of close() in struct input_dev

[Johann D. <jo...@Do...>]

On Tue, 19 Jun 2001, James Simmons wrote:

> 
> > In fact, I realised the driver doesn't really know when a process does an
> > open(). Indeed, input_open_device is called only when the first process
> > opens the device, and input_close_device when all processes closed it.
> 
> Are you sure about that? For one thing each time you open /dev/mouse and
> /dev/event both devices call input_open_device. This is a case where the
> same hardware is opened twice. [...]

This is the case because from evdev and mousedev, the device is opened
only once. What I meant was in the case where two processes or more 
perform an open one after another. In that case, evdev and mousedev will
call input_open_device only for the first process.
Same thing for input_close_device. It's called only when evdev->open falls
to zero (unless the device is disconnected).

> 
> I'm actually using this for power management.
> 

I never took time to think about pm. My experience in this domain is close
to zero (Especially as pm allways had the bad habit to have strange
effects on smp machine)

> > Frankly, I don't really like the fact that evdev can be kfreed both by
> > evdev_disconnect and evdev_connect. Furthermore, the choice is done after
> > the value of evdev->exist, whose access is not protected (possible bug if
> > the last process releases the device while the device is being
> > disconnected, on a smp machine (weird case, though))
> 
> I haven't thought about that problem but I have noticed the handle list
> isn't protected with a lock. On a SMP machine you the list being scanned
> and another thread removing or adding a new handle.

Indeed.
 
> 
> > Anyway, it seems we have to check evdev->exist before doing anything in
> > evdev_{ioctl,read,write...}, I guess this is what caused the oops I got.
> 
> Please post the result of oops processed by ksymoops. This will help us
> find the problem.
> 
No need for that, I know where this problem comes from. evdev calls
functions from, say iforce (or any other driver), without checking if the
driver is still loaded and running. There are cases when this is not true
(if you disconnect the device while it's being used by a process). The
memory allocated for the input_dev structure has then been kfreed, and
should not be accesses. By luck (or is it done in purpose in kfree ?), the
memory area was cleaned with zeros, and the bug showed up immediately,
causing a null dereferecing.
Simply testing the value of evdev->exist at the beginning of each
userland-reachable function would solve this problem.

I made some modifications yesterday night to evdev.c. You can find them at
http://www.esil.univ-mrs.fr/~jdeneux/projects/ff/ruby-evdev-changes.tar.gz

There is still a bug in these modifications, though (input_close_device is
called too many times), so I am not happy with it. It's not ready, but
perhaps it can help you see what I would expect from these open/close
functions.
Among the changes, note:
- evdev->exist moved to list->exist, list being a member of the 
list evdev->list
- new parameter file to input_{open,close}_device and corresponding
evdev::{open/close}. I may need this one.
- only evdev_release kfrees memory kallocated in evdev_open
- if (--evdev->open) tests disappeared. Actually, evdev->open disappeared
(double use with handle->open in input ?)

About pm, I guess it could be interesting to store the state of the
device: active, suspended, shutdown, waking up, ...
Two other states could be: driver_loaded_and_ready,
driver_disconnected_from_process
The state of a process could be: process_ready, process_gone

The evdev->list element created in evdev_open would be freed only once we
have driver_disconnected_from_process and process_gone

-- 
Johann Deneux
CS student at DoCS (www.docs.uu.se/~johannd) and
ESIL (www.esil.univ-mrs.fr/~jdeneux)

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

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

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

Re: Role of close() in struct input_dev

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

> In fact, I realised the driver doesn't really know when a process does an
> open(). Indeed, input_open_device is called only when the first process
> opens the device, and input_close_device when all processes closed it.

Are you sure about that? For one thing each time you open /dev/mouse and
/dev/event both devices call input_open_device. This is a case where the
same hardware is opened twice. Also in input_close_device we have 

handle->open--;

I'm actually using this for power management.

> Frankly, I don't really like the fact that evdev can be kfreed both by
> evdev_disconnect and evdev_connect. Furthermore, the choice is done after
> the value of evdev->exist, whose access is not protected (possible bug if
> the last process releases the device while the device is being
> disconnected, on a smp machine (weird case, though))

I haven't thought about that problem but I have noticed the handle list
isn't protected with a lock. On a SMP machine you the list being scanned
and another thread removing or adding a new handle. 

> Anyway, it seems we have to check evdev->exist before doing anything in
> evdev_{ioctl,read,write...}, I guess this is what caused the oops I got.

Please post the result of oops processed by ksymoops. This will help us
find the problem.

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: power management api

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

On Tue, Jun 19, 2001 at 03:51:49PM -0700, James Simmons wrote:
>   Another item to discuss is power management. Right now we have basically
> a blanking function. I know several people have discussed wanting a api to
> handling things like frontlights and backlights. Please post what is
> needed so we can define something useful to everyone.=20
>=20
Suspend and resume for starters.. at least as far as LCD controllers are
concerned..

Regards,

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

power management api

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

Howdy!!!
 
  Another item to discuss is power management. Right now we have basically
a blanking function. I know several people have discussed wanting a api to
handling things like frontlights and backlights. Please post what is
needed so we can define something useful to everyone. 

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.