On Thursday 27 January 2005 17:16, Vojtech Pavlik wrote: > On Thu, Jan 27, 2005 at 01:18:55PM -0500, Dmitry Torokhov wrote: > > On Thu, 27 Jan 2005 17:36:05 +0100, Vojtech Pavlik <[EMAIL PROTECTED]> > > 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. > > You're right. I forgot that serio isn't anymore tied to the driver and > can cease to exist on its own asynchronously. However, I'm still not > sure whether it's worth it. We might as well simply drop the unregister > call in i8042_open for AUX completely and forget about asynchronous > unregisters and use normal refcounting. As far as grep knows, it's the > only user.
I am pretty sure I will need asynchronous unregister in some form when I finish dynamic protocol switching in psmouse (those darned pass-through ports!). Plus again, having these 2 methods will draw driver writers' attention to the existence of this particular problem. -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
