Re: [PATCH 0/16] New set of input patches

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Thu, 27 Jan 2005 17:36:05 +0100, Vojtech Pavlik <vojtech@suse.cz> wrote:
> On Thu, Jan 27, 2005 at 05:15:18PM +0100, Vojtech Pavlik wrote:
> 
> > OK. I'll go through them, and apply as appropriate. I still need to wrap
> > my mind around the start() and stop() methods and see the necessity. I
> > still think a variable in the serio struct, only accessed by the serio.c
> > core driver itself (and never by the port driver) that'd cause all
> > serio_interrupt() calls to be ignored until set in the asynchronous port
> > registration would be well enough.
> 
> I've read he start() and stop() code, and I came to the conclusion
> again that we don't need them as serio port driver methods. i8042 uses
> them to set the exists variable only and uses that variable _solely_ for
> the purpose of skipping calls to serio_interrupt(), serio_cleanup() and
> serio_unregister().
> 
> By instead checking a member of the serio struct in these functions, and
> doing nothing if not set, we achieve the same goal, without adding extra
> cruft to the interface, making it allowed to call these serio functions
> on a non-registered or half-registered serio struct, with the effect
> being defined to nothing.
> 

No, you can not peek into serio structure from a driver, not when it
was dynamically allocated and can be gone at any time. Please consider
the following screnario when shutting down 8042 when you have a MUX
with several ports:

The rough call sequence is:
i8042_exit
  serio_unregister_port
     driver->disconnect
        serio_close
           i8042->close
     free(serio)

We need to keep interrupts passed to serio core until serio_close is
completed so device properly ACKs/responds to cleanup commands.
Additionally, in i8042 close we do not free IRQ until last port is
unregistered nor we disable the port because we want to support
hotplugging. If interrupt comes after port was freed but before
serio_unregister_port has returned i8042_interrupt will call
serio_interrupt for port that was just free()ed. Special flag in serio
will not help because you need to know that port pointer is valid. You
could try pinning the port in memory buy taking a refernce but then
asynchronous unregister is not possible and it is needed in some
cases.

I think that having these 2 interface functions helps clearly define
these sequence points when port can/can not be used, simplifies logic
and alerts driver authors of this potential problem.
 
-- 
Dmitry