[RFC] dmaengine: add new api for preparing simple slave transfer

[Raju, Sundaram]

Vinod,

> -----Original Message-----
> From: Koul, Vinod [mailto:vinod.koul at intel.com]
> Sent: Friday, June 10, 2011 11:39 AM
> To: Raju, Sundaram; Dan
> Cc: Russell King - ARM Linux; davinci-linux-open-
> source at linux.davincidsp.com; linux-omap at vger.kernel.org; linux-
> kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: RE: [RFC] dmaengine: add new api for preparing simple slave transfer
> 

<snip>
> >
> > The above 2 APIs in the dmaengine framework expect all the
> > peripheral/slave related attributes to be filled in the
> > dma_slave_config data structure.
> > struct dma_slave_config {
> >         enum dma_data_direction direction;
> >         dma_addr_t src_addr;
> >         dma_addr_t dst_addr;
> >         enum dma_slave_buswidth src_addr_width;
> >         enum dma_slave_buswidth dst_addr_width;
> >         u32 src_maxburst;
> >         u32 dst_maxburst;
> > };
> >
> > This data structure is passed to the offload engine via the dma_chan
> > data structure in its private pointer.
> No, this is passed thru control API you described above. Please read
> Documentation/dmaengine.txt

Yes, Vinod its my mistake. I wanted to say control API only, 
but just mixed it up with how the dma_slave_config is attached 
to each dma_chan so that the offload drivers can use it while 
preparing the descriptors.

> 
> > Now coming to the buffer related attributes, sg list is a nice way to
> > represent a disjoint buffer; all the offload engines in drivers/dma
> > create a descriptor for each of the contiguous chunk in the sg list
> > buffer and pass it to the controller.
> >
> > But many a times a client may want to transfer from a single buffer to
> > the peripheral and most of the DMA controllers have the capability to
> > transfer data in chunks/frames directly at a stretch.
> > All the attributes of a buffer, which are required for the transfer
> > can be programmed in single descriptor and submitted to the
> > DMA controller.
> >
> > So I think the slave transfer API must also have a provision to pass
> > the buffer configuration. The buffer attributes have to be passed
> > directly as an argument in the prepare API, unlike dma_slave_config
> > as they will be different for each buffer that is going to be
> > transferred.
> Can you articulate what attributes you are talking about. The
> dma_slave_config parameters don't represent buffer attributes. They
> represent the dma attributes on how you want to transfer. These things
> like bus widths, burst lengths are usually constant for the slave
> transfers, not sure why they should change per buffer?
> 

I have tried to explain the attributes in the previous mail 
I posted in this thread.

Yes, buffer attributes should not be represented in 
the dma_slave_config. It is for slave related data.
That is why had mentioned that buffer configuration should be
a separate structure and passed in the prepare API.
See quoted below:

<quote>
> struct dma_buffer_config {
> 	u32 chunk_size; /* number of bytes in a chunk */
> 	u32 frame_size; /* number of chunks in a frame */
> 	/* u32 n_frames; number of frames in the buffer */
> 	u32 inter_chunk_gap; /* number of bytes between end of a chunk 
> 				and the start of the next chunk */ 
> 	u32 inter_frame_gap; /* number of bytes between end of a frame 
> 				and the start of the next frame */ 
> 	bool sync_rate; /* 0 - a sync event is required from the 
> 				peripheral to transfer a chunk 
> 			1 - a sync event is required from the 
> 				peripheral to transfer a frame */ 
> };
> 
> The patch to add a new API for single buffer transfers alone: 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> @@ -491,6 +492,10 @@ struct dma_device {
> 		struct dma_chan *chan, struct scatterlist *sgl,
> 		unsigned int sg_len, enum dma_data_direction direction,
> 		unsigned long flags);
> +	struct dma_async_tx_descriptor *(*device_prep_slave)(
> +		struct dma_chan *chan, dma_addr_t buf_addr,
> +		unsigned int buf_len, void *buf_prop,
> +		enum dma_data_direction direction, unsigned long flags);
> 	struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
> 		struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> 		size_t period_len, enum dma_data_direction direction);
>
</quote>

Regards,
Sundaram