Date: Thu, 20 Mar 2003 22:23:52 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: nate@root.org Cc: current@FreeBSD.ORG, wpaul@FreeBSD.ORG Subject: Re: Updated if_* attach/detach patches Message-ID: <20030320.222352.122290134.imp@bsdimp.com> In-Reply-To: <Pine.BSF.4.21.0303191032480.12313-100000@root.org> References: <Pine.BSF.4.21.0303191032480.12313-100000@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <Pine.BSF.4.21.0303191032480.12313-100000@root.org>
Nate Lawson <nate@root.org> writes:
: - dc: move interrupt allocation back where it was before. It was unnecessary
: to move it
Why's that? If dc is on a shared interrupt line, then dc_intr is
going to be called, potentially, before the rest of the attach routine
finishes. Keep in mind that attach routines run with interrupts
enabled in many interesting cases (including cardbus). You can't call
bus_setup_intr until the very end of attach. Since there's no locking
here, bad things would happen if an interrupt fired, no? It looks
like dc might be safe (since it returns right away if DC_ISR is zero
for the bits it knows about)...
: - pcn: add missing bzero of softc
softc is automatically bzero'd.
: - rl: move irq allocation before ether_ifattach. Problems could have been
: caused by allocating the irq after enabling interrupts on the card.
Same problem as the dc driver. ether_ifattach isn't going to start
the interface, so you are safe waiting until after ether_ifattach to
do this. And for the rl driver there's no check to see if the RL_ISR
has bits set before we acquire the lock... Even with that, it might
be safe, but I'm less sure.
Why are you checking device_is_alive() in detach? detach won't get
called if that isn't the case. Oh, I see, you've added calls... In
general, in the drivers I've written I have a foo_alloc() and
foo_dealloc() to get and release the resources rather than overloading
detach to do this. Well, foo_alloc isn't always possible in the newer
world order where locking matters. foo_dalloc can safely be written,
however.
You want to device_delete_child before calling bus_generic_detatch().
but the old driver does it backwards, so that's not a huge deal.
For dc and rl you might want to call bus_child_present(dev) in the
detach routines and *NOT* call foo_stop() if the card isn't present.
wi actually sets gone in detach:
/* check if device was removed */
sc->wi_gone = !bus_child_present(dev);
which might not be a horrible idea.
Warner
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030320.222352.122290134.imp>