From owner-freebsd-current@FreeBSD.ORG Thu Feb 22 21:58:20 2007 Return-Path: X-Original-To: current@freebsd.org Delivered-To: freebsd-current@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id D862816A405 for ; Thu, 22 Feb 2007 21:58:20 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (vc4-2-0-87.dsl.netrack.net [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 8454E13C4B7 for ; Thu, 22 Feb 2007 21:58:20 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.13.4/8.13.4) with ESMTP id l1MLuNOt016194; Thu, 22 Feb 2007 14:56:23 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Thu, 22 Feb 2007 14:56:23 -0700 (MST) Message-Id: <20070222.145623.-1350496831.imp@bsdimp.com> To: rizzo@icir.org From: "M. Warner Losh" In-Reply-To: <20070219020725.A56400@xorpc.icir.org> References: <20070219020725.A56400@xorpc.icir.org> X-Mailer: Mew version 4.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0 (harmony.bsdimp.com [127.0.0.1]); Thu, 22 Feb 2007 14:56:23 -0700 (MST) Cc: current@freebsd.org Subject: Re: can a valid bus_dma_tag_t be NULL ? X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Feb 2007 21:58:21 -0000 In message: <20070219020725.A56400@xorpc.icir.org> Luigi Rizzo writes: : I am trying to cleanup some code that allocates dma-able : regions and has to release it in case some of the step : goes wrong. : : The original code (if_iwi.c, but the pattern is repeated in other : drivers too) is the one below. Now, rather than using multiple : labels, is there a value for the various fields (bus_dma_tag_t, : bus_dmamap_t, fw_virtaddr, fw_physaddr) that tells me : that the resource has not been allocated, or i should keep : track of the success/failure of the various calls separately ? : : E.g. i imagine that a NULL fw_virtaddr means failure, however : bus_dmamap_load() worries me because the callback may happen later, : and also i seem to remember that one should not make assumptions : on bus_dma_tag_t == NULL ... : : comments anyone ? And, is -stable different from -current ? Some drivers unwisely assume bus_dma_tag_t == NULL is bad, but there's nothing in the docs that say it is. aha does it right: void aha_free(struct aha_softc *aha) { switch (aha->init_level) { default: case 8: { struct sg_map_node *sg_map; while ((sg_map = SLIST_FIRST(&aha->sg_maps))!= NULL) { SLIST_REMOVE_HEAD(&aha->sg_maps, links); bus_dmamap_unload(aha->sg_dmat, sg_map->sg_dmamap); bus_dmamem_free(aha->sg_dmat, sg_map->sg_vaddr, sg_map->sg_dmamap); free(sg_map, M_DEVBUF); } bus_dma_tag_destroy(aha->sg_dmat); } case 7: bus_dmamap_unload(aha->ccb_dmat, aha->ccb_dmamap); case 6: bus_dmamap_destroy(aha->ccb_dmat, aha->ccb_dmamap); bus_dmamem_free(aha->ccb_dmat, aha->aha_ccb_array, aha->ccb_dmamap); case 5: bus_dma_tag_destroy(aha->ccb_dmat); case 4: bus_dmamap_unload(aha->mailbox_dmat, aha->mailbox_dmamap); case 3: bus_dmamem_free(aha->mailbox_dmat, aha->in_boxes, aha->mailbox_dmamap); bus_dmamap_destroy(aha->mailbox_dmat, aha->mailbox_dmamap); case 2: bus_dma_tag_destroy(aha->buffer_dmat); case 1: bus_dma_tag_destroy(aha->mailbox_dmat); case 0: break; } } and the alloc code looks like: /* DMA tag for mapping buffers into device visible space. */ if (bus_dma_tag_create( /* parent */ aha->parent_dmat, /* alignment */ 1, /* boundary */ 0, /* lowaddr */ BUS_SPACE_MAXADDR, /* highaddr */ BUS_SPACE_MAXADDR, /* filter */ NULL, /* filterarg */ NULL, /* maxsize */ MAXBSIZE, /* nsegments */ AHA_NSEG, /* maxsegsz */ BUS_SPACE_MAXSIZE_24BIT, /* flags */ BUS_DMA_ALLOCNOW, /* lockfunc */ busdma_lock_mutex, /* lockarg */ &Giant, &aha->buffer_dmat) != 0) { goto error_exit; } aha->init_level++; /* DMA tag for our mailboxes */ if (bus_dma_tag_create( /* parent */ aha->parent_dmat, /* alignment */ 1, /* boundary */ 0, /* lowaddr */ BUS_SPACE_MAXADDR, /* highaddr */ BUS_SPACE_MAXADDR, /* filter */ NULL, /* filterarg */ NULL, /* maxsize */ aha->num_boxes * (sizeof(aha_mbox_in_t) + sizeof(aha_mbox_out_t)), /* nsegments */ 1, /* maxsegsz */ BUS_SPACE_MAXSIZE_24BIT, /* flags */ 0, /* lockfunc */ busdma_lock_mutex, /* lockarg */ &Giant, &aha->mailbox_dmat) != 0) { goto error_exit; } aha->init_level++; // etc It is a bit verbose... Warner : cheers : luigi : : : if (bus_dma_tag_create(NULL, 4, 0, BUS_SPACE_MAXADDR_32BIT, : BUS_SPACE_MAXADDR, NULL, NULL, sc->fw_dma_size, 1, sc->fw_dma_size, : 0, NULL, NULL, &sc->fw_dmat) != 0) { : device_printf(sc->sc_dev, : "could not create firmware DMA tag\n"); : IWI_LOCK(sc); : goto fail; : } : if (bus_dmamem_alloc(sc->fw_dmat, &sc->fw_virtaddr, 0, : &sc->fw_map) != 0) { : device_printf(sc->sc_dev, : "could not allocate firmware DMA memory\n"); : IWI_LOCK(sc); : goto fail2; : } : if (bus_dmamap_load(sc->fw_dmat, sc->fw_map, sc->fw_virtaddr, : sc->fw_dma_size, iwi_dma_map_addr, &sc->fw_physaddr, 0) != 0) { : device_printf(sc->sc_dev, "could not load firmware DMA map\n"); : IWI_LOCK(sc); : goto fail3; : } : : ... use the memory ... : : bus_dmamap_unload(sc->fw_dmat, sc->fw_map); : fail3: bus_dmamem_free(sc->fw_dmat, sc->fw_virtaddr, sc->fw_map); : fail2: bus_dma_tag_destroy(sc->fw_dmat); : fail: ... : : --- : _______________________________________________ : freebsd-current@freebsd.org mailing list : http://lists.freebsd.org/mailman/listinfo/freebsd-current : To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org" :