On Sat, May 02, 2009 at 12:56:43AM +0200, Christoph Egger wrote: > Drivers attaches in the device tree independent if their attachment > function failed or not. The autoconf framework isn't aware of this, > because the attachment function doesn't return an error code. > > dyoung@ started to work on the drivers to detach the drivers > on shutdown/reboot. The drivers' detach function doesn't know if > the attachment failed or not. It only can carefully > check which resource was allocated and which not and free the > allocated ones. No matter what happens, one has to be careful not to free resources it didn't alloc. I don't see how anything could change that. > Attached patch modifies the driver's attachment function to > return an error code, zero for success. While I understand where this comes from, autoconf(9) has more short- comings than just that one, and fixing them will require touching every single driver in the tree, too. I'd rather see that done just once. [...] > @@ -1377,6 +1381,15 @@ config_attach_loc(device_t parent, cfdat > } > } > } > + > +#if defined(SPLASHSCREEN) && defined(SPLASHSCREEN_PROGRESS) > + if (splash_progress_state) > + splash_progress_update(splash_progress_state); > +#endif > + error = (*dev->dv_cfattach->ca_attach)(parent, dev, aux); > + if (error) > + goto fail; > + > #ifdef __HAVE_DEVICE_REGISTER > device_register(dev, aux); > #endif > @@ -1388,17 +1401,18 @@ config_attach_loc(device_t parent, cfdat > if (splash_progress_state) > splash_progress_update(splash_progress_state); > #endif > - (*dev->dv_cfattach->ca_attach)(parent, dev, aux); > -#if defined(SPLASHSCREEN) && defined(SPLASHSCREEN_PROGRESS) > - if (splash_progress_state) > - splash_progress_update(splash_progress_state); > -#endif [...] > @@ -1435,14 +1453,23 @@ config_attach_pseudo(cfdata_t cf) > > config_devlink(dev); > > + error = (*dev->dv_cfattach->ca_attach)(ROOT, dev, NULL); > + if (error) > + goto fail; > + > #if 0 /* XXXJRT not yet */ > #ifdef __HAVE_DEVICE_REGISTER > device_register(dev, NULL); /* like a root node */ > #endif > #endif > - (*dev->dv_cfattach->ca_attach)(ROOT, dev, NULL); You can't move the attach callbacks around the call to device_register(). (I'll grant you that it is #if 0'd in the pseudo- device case, but it might not stay that way eventually.) device_register() has to happen before the attach callback because the system will typically set properties for the attach callback to use during that call. I don't know how device_register() users could react if the device they've just messed with disappear immediately. It has to be checked carefully. -- Quentin Garnier - cube%cubidou.net@localhost - cube%NetBSD.org@localhost "See the look on my face from staying too long in one place [...] every time the morning breaks I know I'm closer to falling" KT Tunstall, Saving My Face, Drastic Fantastic, 2007.
Attachment:
pgpQ53zbVpxNp.pgp
Description: PGP signature