From owner-svn-src-all@freebsd.org Fri Dec 6 20:21:37 2019 Return-Path: Delivered-To: svn-src-all@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 DEF061B55CC for ; Fri, 6 Dec 2019 20:21:37 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qk1-x72f.google.com (mail-qk1-x72f.google.com [IPv6:2607:f8b0:4864:20::72f]) (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 47V3rx14Hjz3MbC for ; Fri, 6 Dec 2019 20:21:36 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qk1-x72f.google.com with SMTP id i18so7533395qkl.11 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=tVr0HqO+udLGLPxG8BjCU4PTxQEfVtJ70eHXo/q9sLiMkfqhQ9s7u34dKBSeqeV8+i g52M0pPUzu+blrLFNADVH/dEqsLBbBlUrtGhWrovurcnehW8wnuk0axTf/BmDLeDYnG1 E4bNQ6+ULB4wOuLVMd0jSxRh5gqdSy3axXo6f+ky3Wt+J8lXi+rJIIPv7WJjQRjf5tJO hyghhD8cIEn203FvNmfEuoQ1OKjVuVabj+wx7Dg7nU40EaMx49UzEPlFrtdsKkyfaLdy 3TcKKbzG/5aQnU2o9kyhT+qB4ZapMwQTt85/aqfK0hSQRGjuKG0iKzOia5U6eQAePxge JVGQ== X-Gm-Message-State: APjAAAXjdQZpP040tYgaNg9Bk5LCUdK7i58hDG8XR8uvbRA+4wFSVULH LGOPVZKqgWTMuYFEwE0TXf6tS3PZ2B9K94aEir8eiw== 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: 47V3rx14Hjz3MbC 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::72f) smtp.mailfrom=wlosh@bsdimp.com X-Spamd-Result: default: False [-3.72 / 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-all@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)[f.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.72)[ip: (-9.38), 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-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" 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