From owner-svn-src-head@freebsd.org Fri Dec 6 20:21:37 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id DEEB81B55CB for ; Fri, 6 Dec 2019 20:21:37 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qk1-x729.google.com (mail-qk1-x729.google.com [IPv6:2607:f8b0:4864:20::729]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 47V3rx0NSyz3Mb7 for ; Fri, 6 Dec 2019 20:21:36 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qk1-x729.google.com with SMTP id f7so2708236qkd.7 for ; Fri, 06 Dec 2019 12:21:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=H6o7LB6plxnzAII3A9Ta78o0q1IwvApczhpxMDRJQrc=; b=iO7dgyEWgTNfQBCR4stmrnq92FX23/xVF6a5qc5GRozMoN7/4WFJZWsoo8rF19iI2t 5SzGblCoY66RD/ENm8WRYrL1CP1rn+TFgvUtXMq+EjMDnT2ZLptU2XGG1h3ADnTeWc3o be0cRRLTUMXFWWtrMbSCyn2QIOuGz7iOXO3jxGjREn73Ht7b6dKSMr+IBwUlOnnTW2Mh vxXAT5US48F+qlcMBOmrh3ihmtByPwTaUmMfR63QCrC2BCQ5lRzVwDmzkSjHy/YUrMnX ry2BXa0Pg2AtiBNq+N8Z3GXxCSImYsm9TCTqp/cUDnGTbwQa5EOCQioZhur7vTIHrAvI coXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=H6o7LB6plxnzAII3A9Ta78o0q1IwvApczhpxMDRJQrc=; b=WUzPkdyAul5ZAlxBrKYMJmwkmIp4ZGieCyoUD3dH3IFNiDnbxVMboe4Wz9NtTR8Z1n tT1ndldvSyvudj1RW8S6vpv0LBBeeiwD7GFWCH7AtRYJVueyZBCl8YDUQjp2QSmi5Bty InwhMxIqySKJESe9bscKEuU6MuAmuNA5dleAjlAxzE3GpP9cEYr5DaNVJs7Hp77Onove HxsEEfz7PMdUvvTVCZoXvtqP2v0UEBo6X2KOTdAMI+YOmer0Lfb+qhe+eTwzn6giA82A B4SXY4qFoR64sG1VLH/0U3H4TzWalDqCO+mtdUSOYB8TcIQtT4pAbFf8KRbRlOROLKsL SCyA== X-Gm-Message-State: APjAAAVGdkXHcs8sY8fHBL6fqTFlMYgDnmh+FSVaMyDSWvlm2y+Jlvil 7wbKmVISz+hiRbYBQ53jpYGL9yZ8sfFYU/B7+ZTTRGa3bH4= X-Google-Smtp-Source: APXvYqwW3hVtuMWII9Lh2KsrjicqtSHRKQltb5/be43/N5MJ2fT4UA5E6rMFwkiKRB+fEnWyCTfjUqEl472X2Dq/agc= X-Received: by 2002:a37:b0c5:: with SMTP id z188mr9465527qke.215.1575663695120; Fri, 06 Dec 2019 12:21:35 -0800 (PST) MIME-Version: 1.0 References: <201912062005.xB6K58OX065449@repo.freebsd.org> <4e4999fec53ec82f9cfa9a441be09b884cb77e4d.camel@freebsd.org> In-Reply-To: <4e4999fec53ec82f9cfa9a441be09b884cb77e4d.camel@freebsd.org> From: Warner Losh Date: Fri, 6 Dec 2019 13:21:24 -0700 Message-ID: Subject: Re: svn commit: r355461 - head/sys/arm/mv To: Ian Lepore Cc: Luiz Otavio O Souza , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 47V3rx0NSyz3Mb7 X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bsdimp-com.20150623.gappssmtp.com header.s=20150623 header.b=iO7dgyEW; dmarc=none; spf=none (mx1.freebsd.org: domain of wlosh@bsdimp.com has no SPF policy when checking 2607:f8b0:4864:20::729) smtp.mailfrom=wlosh@bsdimp.com X-Spamd-Result: default: False [-3.68 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; R_DKIM_ALLOW(-0.20)[bsdimp-com.20150623.gappssmtp.com:s=20150623]; FROM_HAS_DN(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-head@freebsd.org]; DMARC_NA(0.00)[bsdimp.com]; URI_COUNT_ODD(1.00)[3]; RCPT_COUNT_FIVE(0.00)[5]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[bsdimp-com.20150623.gappssmtp.com:+]; RCVD_IN_DNSWL_NONE(0.00)[9.2.7.0.0.0.0.0.0.0.0.0.0.0.0.0.0.2.0.0.4.6.8.4.0.b.8.f.7.0.6.2.list.dnswl.org : 127.0.5.0]; R_SPF_NA(0.00)[]; FORGED_SENDER(0.30)[imp@bsdimp.com,wlosh@bsdimp.com]; MIME_TRACE(0.00)[0:+,1:+,2:~]; IP_SCORE(-2.68)[ip: (-9.20), ipnet: 2607:f8b0::/32(-2.23), asn: 15169(-1.93), country: US(-0.05)]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; FROM_NEQ_ENVFROM(0.00)[imp@bsdimp.com,wlosh@bsdimp.com]; RCVD_TLS_ALL(0.00)[]; RCVD_COUNT_TWO(0.00)[2] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Dec 2019 20:21:37 -0000 On Fri, Dec 6, 2019 at 1:17 PM Ian Lepore 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 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 > > #include > > #include > > - > > #include > > #include > > +#include > > #include > > > > #include > > @@ -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