David Brownell <davi...@pacbell.net> writes: > Some of the comments about my earlier EDMA patches touched on issues > in that programming interface, like: > > - The single call to allocate DMA resources is overly complex. > > - Its programming model doesn't match the hardware well: talking > about master vs. slave, not channels and parameter RAM; confusing > those two resource types (especially when allocating); etc. > > - Since the calls used a "davinci_" prefix, they wouldn't be very > appropriate for the DMA in the OMAP-L137 chip. > > These patches address those issues by restructuring resource allocation > so there are now separate alloc/free calls for both kinds of resources; > and renaming everything. The resulting calls are also put into groups > that reflect the interface structure. From the header; > > /* alloc/free DMA channels and their dedicated parameter RAM slots */ > int edma_alloc_channel(int channel, > void (*callback)(unsigned channel, u16 ch_status, void *data), > void *data, enum dma_event_q); > void edma_free_channel(unsigned channel); > > /* alloc/free parameter RAM slots */ > int edma_alloc_slot(int slot); > void edma_free_slot(unsigned slot); > > /* calls that operate on part of a parameter RAM slot */ > void edma_set_src(unsigned slot, dma_addr_t src_port, > enum address_mode mode, enum fifo_width); > void edma_set_dest(unsigned slot, dma_addr_t dest_port, > enum address_mode mode, enum fifo_width); > void edma_get_position(unsigned slot, dma_addr_t *src, dma_addr_t *dst); > void edma_set_src_index(unsigned slot, s16 src_bidx, s16 src_cidx); > void edma_set_dest_index(unsigned slot, s16 dest_bidx, s16 dest_cidx); > void edma_set_transfer_params(unsigned slot, u16 acnt, u16 bcnt, u16 ccnt, > u16 bcnt_rld, enum sync_dimension sync_mode); > void edma_link(unsigned from, unsigned to); > void edma_unlink(unsigned from); > > /* calls that operate on an entire parameter RAM slot */ > void edma_write_slot(unsigned slot, const struct edmacc_param *params); > void edma_read_slot(unsigned slot, struct edmacc_param *params); > > /* channel control operations */ > int edma_start(unsigned channel); > void edma_stop(unsigned channel); > void edma_clean_channel(unsigned channel); > void edma_pause(unsigned channel); > void edma_resume(unsigned channel); > > So, my two cents: merge these. Then the EDMA support is fair game to > merge to mainline -- there's much worse there already! -- although some > cleanups (enums and other symbols etc) may still be wanted. > > Then the MMC/SD driver can merge, and the audio driver will be able to > build (and run!) in mainline.
Excellent, thanks for this significant cleanup and major improvement in readability and structure. Pushing today, Kevin _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
