I think this approach is wrong, insane, and fragile. DVF_ACTIVE
doesn't work precisely that way.
A better approach would be a whole-tree change such that attach
functions are non-void. This has been proposed a few times, but
noone ever sat down and did the whole-tree work.
> could we allow config_attach() to 'fail' without leaving dead device behind?
> ie. with gpioow(4) it's easy to make a load of these while testing stuff,
> which does lead to inconsistent unit numbering unless you manually detach
> the broken ones before attaching another..
>
> for devices which are found during autoconf, i guess DETACH_QUIET would
> be wanted to keep dmesg pretty, as the error messages printed from any
> ca->ca_attach() should be enough, and would mean no visible change for
> drivers failing out with this:)
>
> for smaller diff i chose to use the DVF_ACTIVE, while i'd actually prefer
> adding "DVF_FAILED" or something. untested morning-coffee-qual.diff below.
>
> comments?
>
> -Artturi
>
>
> diff --git sys/dev/gpio/gpioow.c sys/dev/gpio/gpioow.c
> index 64d79ab0cb8..912537b7de7 100644
> --- sys/dev/gpio/gpioow.c
> +++ sys/dev/gpio/gpioow.c
> @@ -101,7 +101,7 @@ gpioow_attach(struct device *parent, struct device *self,
> void *aux)
> /* Check that we have enough pins */
> if (gpio_npins(ga->ga_mask) != GPIOOW_NPINS) {
> printf(": invalid pin mask\n");
> - return;
> + goto fail_nomap;
> }
>
> /* Map pins */
> @@ -110,7 +110,7 @@ gpioow_attach(struct device *parent, struct device *self,
> void *aux)
> if (gpio_pin_map(sc->sc_gpio, ga->ga_offset, ga->ga_mask,
> &sc->sc_map)) {
> printf(": can't map pins\n");
> - return;
> + goto fail_nomap;
> }
>
> /* Configure data pin */
> @@ -153,6 +153,8 @@ gpioow_attach(struct device *parent, struct device *self,
> void *aux)
>
> fail:
> gpio_pin_unmap(sc->sc_gpio, &sc->sc_map);
> +fail_nomap:
> + config_defer_failure(self);
> }
>
> int
> diff --git sys/kern/subr_autoconf.c sys/kern/subr_autoconf.c
> index 3fff153e8ad..9a797712510 100644
> --- sys/kern/subr_autoconf.c
> +++ sys/kern/subr_autoconf.c
> @@ -409,6 +409,11 @@ config_attach(struct device *parent, void *match, void
> *aux, cfprint_t print)
> if (--autoconf_attdet == 0)
> wakeup(&autoconf_attdet);
> mtx_leave(&autoconf_attdet_mtx);
> +
> + if ((dev->dv_flags & DVF_ACTIVE) == 0) {
> + if (config_detach(dev, 0) == 0)
> + dev = NULL;
> + }
> return (dev);
> }
>
> @@ -678,6 +683,17 @@ config_defer(struct device *dev, void (*func)(struct
> device *))
> config_pending_incr();
> }
>
> +/*
> + * Defer the detachment of the specified device until the end
> + * of its own configuration.
> + */
> +void
> +config_defer_failure(struct device *dev)
> +{
> +
> + dev->dv_flags &= ~DVF_ACTIVE;
> +}
> +
> /*
> * Defer the configuration of the specified device until after
> * root file system is mounted.
> diff --git sys/sys/device.h sys/sys/device.h
> index 00a1f6ad2a6..3820cc6e0f5 100644
> --- sys/sys/device.h
> +++ sys/sys/device.h
> @@ -185,6 +185,7 @@ int config_activate_children(struct device *, int);
> struct device *config_make_softc(struct device *parent,
> struct cfdata *cf);
> void config_defer(struct device *, void (*)(struct device *));
> +void config_defer_failure(struct device *);
> void config_pending_incr(void);
> void config_pending_decr(void);
> void config_mountroot(struct device *, void (*)(struct device *));
>
