Patchwork [v3,1/3] dmaengine: add dma_get_slave_sg_caps()

login
register
mail settings
Submitter Matt Porter
Date Feb. 4, 2013, 7:47 p.m.
Message ID <1360007224-21735-2-git-send-email-mporter@ti.com>
Download mbox | patch
Permalink /patch/2094891/
State Not Applicable
Headers show

Comments

Matt Porter - Feb. 4, 2013, 7:47 p.m.
Add a dmaengine API to retrieve slave SG transfer capabilities.

The API is optionally implemented by dmaengine drivers and when
unimplemented will return a NULL pointer. A client driver using
this API provides the required dma channel, address width, and
burst size of the transfer. dma_get_slave_sg_caps() returns an
SG caps structure with the maximum number and size of SG segments
that the given channel can handle.

Signed-off-by: Matt Porter <mporter@ti.com>
---
 include/linux/dmaengine.h |   40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
Vinod Koul - Feb. 12, 2013, 5:08 p.m.
On Mon, Feb 04, 2013 at 02:47:02PM -0500, Matt Porter wrote:
> Add a dmaengine API to retrieve slave SG transfer capabilities.
> 
> The API is optionally implemented by dmaengine drivers and when
> unimplemented will return a NULL pointer. A client driver using
> this API provides the required dma channel, address width, and
> burst size of the transfer. dma_get_slave_sg_caps() returns an
> SG caps structure with the maximum number and size of SG segments
> that the given channel can handle.
Okay this sounds much better :-)

few points though:
- you added API for caps, but is actually calculating for given configuration
  the max allowed range. IMHO that is not caps, perhaps renaming to get_max_sg
  /some_better_name would be more apt.
- Still I like the idea of caps, but it should give H/W support capablity. If
  you want to add that, pls develop on same line...

--
~Vinod

> 
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
>  include/linux/dmaengine.h |   40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index d3201e4..5b5b220 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -371,6 +371,19 @@ struct dma_slave_config {
>  	unsigned int slave_id;
>  };
>  
> +/* struct dma_slave_sg_caps - expose SG transfer capability of a
> + * channel.
> + *
> + * @max_seg_nr: maximum number of SG segments supported on a SG/SLAVE
> + *	    channel (0 for no maximum or not a SG/SLAVE channel)
> + * @max_seg_len: maximum length of SG segments supported on a SG/SLAVE
> + *	     channel (0 for no maximum or not a SG/SLAVE channel)
> + */
> +struct dma_slave_sg_caps {
> +	u32 max_seg_nr;
> +	u32 max_seg_len;
> +};
> +
>  static inline const char *dma_chan_name(struct dma_chan *chan)
>  {
>  	return dev_name(&chan->dev->device);
> @@ -534,6 +547,7 @@ struct dma_tx_state {
>   *	struct with auxiliary transfer status information, otherwise the call
>   *	will just return a simple status code
>   * @device_issue_pending: push pending transactions to hardware
> + * @device_slave_sg_caps: return the slave SG capabilities
>   */
>  struct dma_device {
>  
> @@ -602,6 +616,9 @@ struct dma_device {
>  					    dma_cookie_t cookie,
>  					    struct dma_tx_state *txstate);
>  	void (*device_issue_pending)(struct dma_chan *chan);
> +	struct dma_slave_sg_caps *(*device_slave_sg_caps)(
> +		struct dma_chan *chan, enum dma_slave_buswidth addr_width,
> +		u32 maxburst);
>  };
>  
>  static inline int dmaengine_device_control(struct dma_chan *chan,
> @@ -969,6 +986,29 @@ dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used,
>  	}
>  }
>  
> +/**
> + * dma_get_slave_sg_caps - get DMAC SG transfer capabilities
> + * @chan: target DMA channel
> + * @addr_width: address width of the DMA transfer
> + * @maxburst: maximum DMA transfer burst size
> + *
> + * Get SG transfer capabilities for a specified channel. If the dmaengine
> + * driver does not implement SG transfer capabilities then NULL is
> + * returned.
> + */
> +static inline struct dma_slave_sg_caps
> +*dma_get_slave_sg_caps(struct dma_chan *chan,
> +		       enum dma_slave_buswidth addr_width,
> +		       u32 maxburst)
> +{
> +	if (chan->device->device_slave_sg_caps)
> +		return chan->device->device_slave_sg_caps(chan,
> +							  addr_width,
> +							  maxburst);
> +
> +	return NULL;
> +}
> +
>  enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
>  #ifdef CONFIG_DMA_ENGINE
>  enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
> -- 
> 1.7.9.5
>
Matt Porter - March 6, 2013, 7:58 p.m.
On Tue, Feb 12, 2013 at 05:08:44PM +0000, Vinod Koul wrote:
> On Mon, Feb 04, 2013 at 02:47:02PM -0500, Matt Porter wrote:
> > Add a dmaengine API to retrieve slave SG transfer capabilities.
> > 
> > The API is optionally implemented by dmaengine drivers and when
> > unimplemented will return a NULL pointer. A client driver using
> > this API provides the required dma channel, address width, and
> > burst size of the transfer. dma_get_slave_sg_caps() returns an
> > SG caps structure with the maximum number and size of SG segments
> > that the given channel can handle.
> Okay this sounds much better :-)
> 
> few points though:
> - you added API for caps, but is actually calculating for given configuration
>   the max allowed range. IMHO that is not caps, perhaps renaming to get_max_sg
>   /some_better_name would be more apt.

I went with get_slave_sg_limits(), seemed pretty descriptive. Just
posted v4 with that change.

> - Still I like the idea of caps, but it should give H/W support capablity. If
>   you want to add that, pls develop on same line...

Ok, seems like a good separate submission.
 
-Matt

Patch

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index d3201e4..5b5b220 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -371,6 +371,19 @@  struct dma_slave_config {
 	unsigned int slave_id;
 };
 
+/* struct dma_slave_sg_caps - expose SG transfer capability of a
+ * channel.
+ *
+ * @max_seg_nr: maximum number of SG segments supported on a SG/SLAVE
+ *	    channel (0 for no maximum or not a SG/SLAVE channel)
+ * @max_seg_len: maximum length of SG segments supported on a SG/SLAVE
+ *	     channel (0 for no maximum or not a SG/SLAVE channel)
+ */
+struct dma_slave_sg_caps {
+	u32 max_seg_nr;
+	u32 max_seg_len;
+};
+
 static inline const char *dma_chan_name(struct dma_chan *chan)
 {
 	return dev_name(&chan->dev->device);
@@ -534,6 +547,7 @@  struct dma_tx_state {
  *	struct with auxiliary transfer status information, otherwise the call
  *	will just return a simple status code
  * @device_issue_pending: push pending transactions to hardware
+ * @device_slave_sg_caps: return the slave SG capabilities
  */
 struct dma_device {
 
@@ -602,6 +616,9 @@  struct dma_device {
 					    dma_cookie_t cookie,
 					    struct dma_tx_state *txstate);
 	void (*device_issue_pending)(struct dma_chan *chan);
+	struct dma_slave_sg_caps *(*device_slave_sg_caps)(
+		struct dma_chan *chan, enum dma_slave_buswidth addr_width,
+		u32 maxburst);
 };
 
 static inline int dmaengine_device_control(struct dma_chan *chan,
@@ -969,6 +986,29 @@  dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used,
 	}
 }
 
+/**
+ * dma_get_slave_sg_caps - get DMAC SG transfer capabilities
+ * @chan: target DMA channel
+ * @addr_width: address width of the DMA transfer
+ * @maxburst: maximum DMA transfer burst size
+ *
+ * Get SG transfer capabilities for a specified channel. If the dmaengine
+ * driver does not implement SG transfer capabilities then NULL is
+ * returned.
+ */
+static inline struct dma_slave_sg_caps
+*dma_get_slave_sg_caps(struct dma_chan *chan,
+		       enum dma_slave_buswidth addr_width,
+		       u32 maxburst)
+{
+	if (chan->device->device_slave_sg_caps)
+		return chan->device->device_slave_sg_caps(chan,
+							  addr_width,
+							  maxburst);
+
+	return NULL;
+}
+
 enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
 #ifdef CONFIG_DMA_ENGINE
 enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);