diff mbox

[v4,1/3] dmaengine: add dma_get_slave_sg_limits()

Message ID 1362599767-11292-2-git-send-email-mporter@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matt Porter March 6, 2013, 7:56 p.m. UTC
Add a dmaengine API to retrieve slave SG transfer limits.

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_limits() returns an
SG limits 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 |   39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

Comments

Lars-Peter Clausen March 11, 2013, 4:57 p.m. UTC | #1
[...]
>   *	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_limits: return the slave SG capabilities
>   */
>  struct dma_device {
>  
> @@ -602,6 +615,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_limits *(*device_slave_sg_limits)(
> +		struct dma_chan *chan, enum dma_slave_buswidth addr_width,
> +		u32 maxburst);

In my opinion it is better to pass in a pointer to a dma_slave_sg_limits
struct and let the driver fill it. Instead of passing back a pointer to an
internal structure. It is kind of problematic because you never really know
when the user is done using the struct and you don't know when it is safe to
free or reuse it. E.g. in your implementation for the edma driver if the
function is called with different parameters for the same channel, the
previous result will also be overwritten.

- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vinod Koul March 21, 2013, 6:43 a.m. UTC | #2
On Wed, Mar 06, 2013 at 02:56:05PM -0500, Matt Porter wrote:
> Add a dmaengine API to retrieve slave SG transfer limits.
> 
> 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_limits() returns an
> SG limits structure with the maximum number and size of SG segments
> that the given channel can handle.
> 
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
> +static inline struct dma_slave_sg_limits
> +*dma_get_slave_sg_limits(struct dma_chan *chan,
> +		       enum dma_slave_buswidth addr_width,
> +		       u32 maxburst)
> +{
> +	if (chan->device->device_slave_sg_limits)
> +		return chan->device->device_slave_sg_limits(chan,
> +							  addr_width,
> +							  maxburst);
Hi Matt,

Sorry for delayed reply, this series was sent just before i went for vacation :)

I agree with Lars, that returning pointer maynot be good idea. So this bit needs
work. Also the readblity of above is bad by complying to 80char limit. I would
make it easier to read

--
~Vinod
> +
> +	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);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 91ac8da..a4a6aac 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -371,6 +371,18 @@  struct dma_slave_config {
 	unsigned int slave_id;
 };
 
+/* struct dma_slave_sg_limits - expose SG transfer limits 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_limits {
+	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 +546,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_limits: return the slave SG capabilities
  */
 struct dma_device {
 
@@ -602,6 +615,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_limits *(*device_slave_sg_limits)(
+		struct dma_chan *chan, enum dma_slave_buswidth addr_width,
+		u32 maxburst);
 };
 
 static inline int dmaengine_device_control(struct dma_chan *chan,
@@ -963,6 +979,29 @@  dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used,
 	}
 }
 
+/**
+ * dma_get_slave_sg_limits - 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_limits
+*dma_get_slave_sg_limits(struct dma_chan *chan,
+		       enum dma_slave_buswidth addr_width,
+		       u32 maxburst)
+{
+	if (chan->device->device_slave_sg_limits)
+		return chan->device->device_slave_sg_limits(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);