can a valid bus_dma_tag_t be NULL ?

[M. Warner Losh]

In message: <20070219020725.A56400 at xorpc.icir.org>
            Luigi Rizzo <rizzo at 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 at freebsd.org mailing list
: http://lists.freebsd.org/mailman/listinfo/freebsd-current
: To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
: