RE: [RFC] dmaengine: add new api for preparing simple slavetransfer

[Koul, Vinod]

On Fri, 2011-06-10 at 16:43 +0530, Raju, Sundaram wrote:
> Vinod,

...
> > 
> > > 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 */ 
> > };
IIUC, you have a buffer with gaps in between (given by above params).
Is your DMA h/w capable of parsing this buffer and directly do a
transfer based on above parameters (without any work for SW), or you are
doing this in your dma driver and then programming a list of buffers?
In former case (although i haven't seen such dma controller yet), can
you share the datasheet? It would make very little sense to change the
current API for your extra special case. This special case needs to be
handled differently rather than making rule out of it!!

And in latter case you are really going the wrong path as Russel pointed
out and trying to abuse the APIs


-- 
~Vinod

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/