RFC: device attachment/detachment

[Christoph Egger <Christoph_Egger%gmx.de@localhost>]

Hi,

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.

Attached patch modifies the driver's attachment function to
return an error code, zero for success.
I modified all drivers necessary to compile an amd64 GENERIC
kernel plus bwi(4). The intended functional change is in
subr_autoconf.c, there's no intended functional change in the
drivers.

The full blown patch (500KB) is at
http://www.netbsd.org/~cegger/autoconf.diff

bwi(4) attachment always fails on my machine
due to lack of Draft-N support in a very early support.

W/o the patch 'drvctl -d bwi0' crashes in various places
due to assumptions even in MI code.

With the patch, bwi0 detaches:

bwi0 at pci3 dev 0 function 0: Broadcom Wireless
bwi0: interrupting at ioapic0 pin 16
bwi0: bwi_attach
bwi0: bwi_power_on
bwi0: regwin: type 0x800, rev 19, vendor 0x4243
bwi0: BBP id 0x4321, BBP rev 0x3, BBP pkg 0
bwi0: nregwin 5, cap 0x0864000d
bwi0: regwin: type 0x812, rev 12, vendor 0x4243
bwi0: MAC: rev 12
bwi0: regwin: type 0x820, rev 4, vendor 0x4243
bwi0: regwin: type 0x804, rev 13, vendor 0x4243
bwi0: bus regwin already exists
bwi0: regwin: type 0x817, rev 4, vendor 0x4243
bwi0: bwi_power_on
bwi0: clksrc CS_OSC
bwi0: clkfreq min 990000, max 1010000
bwi0: power on delay 3
bwi0: bus rev 4
bwi0: PCIE is enabled
bwi0: card flags 0x3844
bwi0: 0th led, act 91, lowact 0
bwi0: 1th led, act 91, lowact 0
bwi0: 2th led, act 41, lowact 0
bwi0: 3th led, act 41, lowact 0
bwi0: MAC was already disabled
bwi0: PHY is linked
bwi0: PHY type 4, rev 2, ver 5
bwi0: unsupported PHY type 4
bwi0: detached


Christoph

Index: sys/kern/subr_autoconf.c
===================================================================
RCS file: /cvsroot/src/sys/kern/subr_autoconf.c,v
retrieving revision 1.175
diff -u -p -r1.175 subr_autoconf.c
--- sys/kern/subr_autoconf.c    1 May 2009 08:27:41 -0000       1.175
+++ sys/kern/subr_autoconf.c    1 May 2009 22:51:53 -0000
@@ -1314,6 +1314,7 @@ config_attach_loc(device_t parent, cfdat
        device_t dev;
        struct cftable *ct;
        const char *drvname;
+       int error = 0;
 
 #if defined(SPLASHSCREEN) && defined(SPLASHSCREEN_PROGRESS)
        if (splash_progress_state)
@@ -1321,8 +1322,11 @@ config_attach_loc(device_t parent, cfdat
 #endif
 
        dev = config_devalloc(parent, cf, locs);
-       if (!dev)
-               panic("config_attach: allocation of device softc failed");
+       if (!dev) {
+               aprint_debug("%s: allocation of device softc failed",
+                   __func__);
+               return NULL;
+       }
 
        /* XXX redundant - see below? */
        if (cf->cf_fstate != FSTATE_STAR) {
@@ -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
 
        if (!device_pmf_is_registered(dev))
                aprint_debug_dev(dev, "WARNING: power management not 
supported\n");
 
        config_process_deferred(&deferred_config_queue, dev);
        return dev;
+
+fail:
+       config_devunlink(dev);
+       aprint_normal_dev(dev, "detached\n");
+       config_devdealloc(dev);
+       return NULL;
 }
 
 device_t
@@ -1421,10 +1435,14 @@ device_t
 config_attach_pseudo(cfdata_t cf)
 {
        device_t dev;
+       int error = 0;
 
        dev = config_devalloc(ROOT, cf, NULL);
-       if (!dev)
+       if (!dev) {
+               aprint_debug("%s: allocation of device softc failed",
+                   __func__);
                return NULL;
+       }
 
        /* XXX mark busy in cfdata */
 
@@ -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);
        config_process_deferred(&deferred_config_queue, dev);
        return dev;
+
+fail:
+       config_devunlink(dev);
+       aprint_normal_dev(dev, "detached\n");
+       config_devdealloc(dev);
+       return NULL;
 }
 
 /*
Index: sys/sys/device.h
===================================================================
RCS file: /cvsroot/src/sys/sys/device.h,v
retrieving revision 1.117
diff -u -p -r1.117 device.h
--- sys/sys/device.h    2 Apr 2009 00:09:34 -0000       1.117
+++ sys/sys/device.h    1 May 2009 22:51:53 -0000
@@ -290,7 +290,7 @@ struct cfattach {
        size_t    ca_devsize;           /* size of dev data (for malloc) */
        int       ca_flags;             /* flags for driver allocation etc */
        int     (*ca_match)(device_t, cfdata_t, void *);
-       void    (*ca_attach)(device_t, device_t, void *);
+       int     (*ca_attach)(device_t, device_t, void *);
        int     (*ca_detach)(device_t, int);
        int     (*ca_activate)(device_t, devact_t);
        /* technically, the next 2 belong into "struct cfdriver" */