Date: Thu, 22 Feb 2007 14:56:23 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: rizzo@icir.org Cc: current@freebsd.org Subject: Re: can a valid bus_dma_tag_t be NULL ? Message-ID: <20070222.145623.-1350496831.imp@bsdimp.com> In-Reply-To: <20070219020725.A56400@xorpc.icir.org> References: <20070219020725.A56400@xorpc.icir.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <20070219020725.A56400@xorpc.icir.org>
Luigi Rizzo <rizzo@icir.org> 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"
:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070222.145623.-1350496831.imp>