svn commit: r355461 - head/sys/arm/mv

[Warner Losh]

On Fri, Dec 6, 2019 at 1:17 PM Ian Lepore <ian at freebsd.org> wrote:

> On Fri, 2019-12-06 at 20:05 +0000, Luiz Otavio O Souza wrote:
> > Author: loos
> > Date: Fri Dec  6 20:05:08 2019
> > New Revision: 355461
> > URL: https://svnweb.freebsd.org/changeset/base/355461
> >
> > Log:
> >   Fix the ARM64 build, include the necessary <sys/mutex.h> header.
> >
> >   While here, call device_delete_children() to detach and dealloc all
> > the
> >   existent children and handle the child's detach errors properly.
> >
> >   Reported by:        jenkins, hselasky, ian
> >   Sponsored by:       Rubicon Communications, LLC (Netgate)
> >
> > Modified:
> >   head/sys/arm/mv/a37x0_spi.c
> >
> > Modified: head/sys/arm/mv/a37x0_spi.c
> > =====================================================================
> > =========
> > --- head/sys/arm/mv/a37x0_spi.c       Fri Dec  6 19:33:39 2019
> (r355
> > 460)
> > +++ head/sys/arm/mv/a37x0_spi.c       Fri Dec  6 20:05:08 2019
> (r355
> > 461)
> > @@ -29,9 +29,9 @@ __FBSDID("$FreeBSD$");
> >  #include <sys/param.h>
> >  #include <sys/systm.h>
> >  #include <sys/bus.h>
> > -
> >  #include <sys/kernel.h>
> >  #include <sys/module.h>
> > +#include <sys/mutex.h>
> >  #include <sys/rman.h>
> >
> >  #include <machine/bus.h>
> > @@ -228,9 +228,11 @@ a37x0_spi_attach(device_t dev)
> >  static int
> >  a37x0_spi_detach(device_t dev)
> >  {
> > +     int err;
> >       struct a37x0_spi_softc *sc;
> >
> > -     bus_generic_detach(dev);
> > +     if ((err = device_delete_children(dev)) != 0)
> > +             return (err);
> >       sc = device_get_softc(dev);
> >       mtx_destroy(&sc->sc_mtx);
> >       if (sc->sc_intrhand)
>
> Oops, not quite right; I should have been more explicit.  Something
> more like this:
>
>   if ((err = bus_generic_detach(dev)) != 0)
>       return (err);
>   device_delete_children(dev);
>
> The delete is basically cannot-fail (as long as they're not attached),
> but bus_generic_detach() can fail if the detach method of any
> child/grandchilden fails.
>
> Hrm.  You know, now that I've just looked in the code for
> device_delete_children(), it appears it will call detach if necessary,
> so maybe your approach is fine.  I've just never done it that way.  If
> anyone knows a reason why one approach is better than the other, say
> so.  Otherwise I guess we should call this good.
>

Once upon a time, delete_children would fail if anything was attached
(well, before that was there were for loops everywhere that did this
thing). Now it calls detach so we can move that common code into one
routine. You see a mix in the tree because not all uses were updated when
functionality increased. I'd prefer the approach that Luiz did, honestly.

Warner