linux-kernel - RE: [RFC] dmaengine: add new api for preparing simple slave transfer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <E0D41E29EB0DAC4E9F3FF173962E9E940301FE3405@dbde02.ent.ti.com>
Date:	Fri, 10 Jun 2011 16:43:53 +0530
From:	"Raju, Sundaram" <sundaram@...com>
To:	"Koul, Vinod" <vinod.koul@...el.com>,
	Dan <dan.j.williams@...el.com>
CC:	Russell King - ARM Linux <linux@....linux.org.uk>,
	"davinci-linux-open-source@...ux.davincidsp.com" 
	<davinci-linux-open-source@...ux.davincidsp.com>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [RFC] dmaengine: add new api for preparing simple slave transfer

Vinod,

> -----Original Message-----
> From: Koul, Vinod [mailto:vinod.koul@...el.com]
> Sent: Friday, June 10, 2011 11:39 AM
> To: Raju, Sundaram; Dan
> Cc: Russell King - ARM Linux; davinci-linux-open-
> source@...ux.davincidsp.com; linux-omap@...r.kernel.org; linux-
> kernel@...r.kernel.org; linux-arm-kernel@...ts.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