[RFC] dmaengine: add dma_slave_get_caps api

[RFC] dmaengine: add dma_slave_get_caps api

[Vinod Koul]

add new device callback .device_slave_caps api which can be used by clients to
query the dma channel capablties before they program the channel. This can help
is removing errors during the channel programming. Also add helper
dma_slave_get_caps API

This patch folds the work done by Matt earlier
https://patchwork.kernel.org/patch/2094891/

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/linux/dmaengine.h |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 96d3e4a..f259f27 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -371,6 +371,37 @@ struct dma_slave_config {
 	unsigned int slave_id;
 };
 
+/* struct dma_slave_caps - expose capabilities of a slave channel only
+ *
+ * @src_addr_widths: bit mask of src addr widths the channel supports
+ * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
+ * @src_burst_lengths: bit mask of src slave burst lengths supported
+ * @dstn_burst_lengths: bit mask of dstn slave burst lengths supported
+ * @directions: bit mask of slave direction the channel supported
+ * 	since the enum dma_transfer_direction is not defined as bits for each
+ * 	type of direction, the dma controller should fill (1 << <TYPE>) and same
+ * 	should be checked by controller as well
+ * @cmd_pause: true, if pause and thereby resume is supported
+ * @cmd_terminate: true, if terminate cmd is supported
+ *
+ * @max_seg_nr: maximum number of SG segments supported
+ * 	0 for no maximum
+ * @max_seg_len: maximum length of a SG segment supported
+ * 	0 for no maximum
+ */
+struct dma_slave_caps {
+	u32 src_addr_widths;
+	u32 dstn_addr_widths;
+	u32 src_burst_lengths;
+	u32 dstn_burst_lengths;
+	u32 directions;
+	bool cmd_pause;
+	bool cmd_terminate;
+
+	u32 max_sg_nr;
+	u32 max_sg_len;
+};
+
 static inline const char *dma_chan_name(struct dma_chan *chan)
 {
 	return dev_name(&chan->dev->device);
@@ -534,6 +565,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_caps: return the slave channel capabilities
  */
 struct dma_device {
 
@@ -602,6 +634,7 @@ struct dma_device {
 					    dma_cookie_t cookie,
 					    struct dma_tx_state *txstate);
 	void (*device_issue_pending)(struct dma_chan *chan);
+	struct dma_slave_caps *(*device_slave_caps)(struct dma_chan *chan);
 };
 
 static inline int dmaengine_device_control(struct dma_chan *chan,
@@ -675,6 +708,18 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma(
 	return chan->device->device_prep_interleaved_dma(chan, xt, flags);
 }
 
+static inline struct dma_slave_caps *dma_get_slave_caps(struct dma_chan *chan)
+{
+	/* check if the channel supports slave transactions */
+	if (!test_bit(DMA_SLAVE, chan->device->cap_mask.bits))
+		return NULL;
+
+	if (chan->device->device_slave_caps)
+		return chan->device->device_slave_caps(chan);
+
+	return NULL;
+}
+
 static inline int dmaengine_terminate_all(struct dma_chan *chan)
 {
 	return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
-- 
1.7.0.4

Re: [RFC] dmaengine: add dma_slave_get_caps api

[Lars-Peter Clausen]

On 07/08/2013 10:54 AM, Vinod Koul wrote:
> add new device callback .device_slave_caps api which can be used by clients to
> query the dma channel capablties before they program the channel. This can help
> is removing errors during the channel programming. Also add helper
> dma_slave_get_caps API
> 
> This patch folds the work done by Matt earlier
> https://patchwork.kernel.org/patch/2094891/
> 

Hi,

Thanks for taking care of this.

> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  include/linux/dmaengine.h |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 45 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 96d3e4a..f259f27 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -371,6 +371,37 @@ struct dma_slave_config {
>  	unsigned int slave_id;
>  };
>  
> +/* struct dma_slave_caps - expose capabilities of a slave channel only
> + *
> + * @src_addr_widths: bit mask of src addr widths the channel supports
> + * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
> + * @src_burst_lengths: bit mask of src slave burst lengths supported
> + * @dstn_burst_lengths: bit mask of dstn slave burst lengths supported

I'm not sure if we need the burst_lengths fields. For one we can only
express a max burst length up 32. And usually it is fine if the dma
controller does not support the burst length requested by the slave driver,
since this only specifies the maximum and the dma controller driver can
choose a value below this limit. E.g. if the max burst length is set to 16
it is still fine if the controller only supports a burst length of 8.

> + * @directions: bit mask of slave direction the channel supported
> + * 	since the enum dma_transfer_direction is not defined as bits for each
> + * 	type of direction, the dma controller should fill (1 << <TYPE>) and same
> + * 	should be checked by controller as well
> + * @cmd_pause: true, if pause and thereby resume is supported
> + * @cmd_terminate: true, if terminate cmd is supported
> + *
> + * @max_seg_nr: maximum number of SG segments supported
> + * 	0 for no maximum
> + * @max_seg_len: maximum length of a SG segment supported
> + * 	0 for no maximum
> + */
> +struct dma_slave_caps {
> +	u32 src_addr_widths;
> +	u32 dstn_addr_widths;
> +	u32 src_burst_lengths;
> +	u32 dstn_burst_lengths;
> +	u32 directions;
> +	bool cmd_pause;
> +	bool cmd_terminate;
> +
> +	u32 max_sg_nr;
> +	u32 max_sg_len;
> +};
> +
>  static inline const char *dma_chan_name(struct dma_chan *chan)
>  {
>  	return dev_name(&chan->dev->device);
> @@ -534,6 +565,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_caps: return the slave channel capabilities
>   */
>  struct dma_device {
>  
> @@ -602,6 +634,7 @@ struct dma_device {
>  					    dma_cookie_t cookie,
>  					    struct dma_tx_state *txstate);
>  	void (*device_issue_pending)(struct dma_chan *chan);
> +	struct dma_slave_caps *(*device_slave_caps)(struct dma_chan *chan);
>  };
>  
>  static inline int dmaengine_device_control(struct dma_chan *chan,
> @@ -675,6 +708,18 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma(
>  	return chan->device->device_prep_interleaved_dma(chan, xt, flags);
>  }
>  
> +static inline struct dma_slave_caps *dma_get_slave_caps(struct dma_chan *chan)

Same comment as for Matt's patch. The caller of the dma_get_slave_caps()
should pass in a pointer to a dma_slave_caps struct which the dma driver
will then fill in. This makes it much clearer to the caller what the
lifetime of the struct is.

> +{
> +	/* check if the channel supports slave transactions */
> +	if (!test_bit(DMA_SLAVE, chan->device->cap_mask.bits))
> +		return NULL;
> +
> +	if (chan->device->device_slave_caps)
> +		return chan->device->device_slave_caps(chan);
> +
> +	return NULL;
> +}

Re: [RFC] dmaengine: add dma_slave_get_caps api

[Vinod Koul]

On Mon, Jul 08, 2013 at 02:01:35PM +0200, Lars-Peter Clausen wrote:
> On 07/08/2013 10:54 AM, Vinod Koul wrote:
> > +/* struct dma_slave_caps - expose capabilities of a slave channel only
> > + *
> > + * @src_addr_widths: bit mask of src addr widths the channel supports
> > + * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
> > + * @src_burst_lengths: bit mask of src slave burst lengths supported
> > + * @dstn_burst_lengths: bit mask of dstn slave burst lengths supported
> 
> I'm not sure if we need the burst_lengths fields. For one we can only
> express a max burst length up 32. And usually it is fine if the dma
> controller does not support the burst length requested by the slave driver,
> since this only specifies the maximum and the dma controller driver can
> choose a value below this limit. E.g. if the max burst length is set to 16
> it is still fine if the controller only supports a burst length of 8.
well how are you picking up which one to use?
The idea is that you query and match that with system and client to get best
throughput. If you have IP block which over generations change capablities you
can runtime query and then program the channel smartly!

> > +static inline struct dma_slave_caps *dma_get_slave_caps(struct dma_chan *chan)
> 
> Same comment as for Matt's patch. The caller of the dma_get_slave_caps()
> should pass in a pointer to a dma_slave_caps struct which the dma driver
> will then fill in. This makes it much clearer to the caller what the
> lifetime of the struct is.
Yup i agree, will update...

~Vinod
-- 

Re: [RFC] dmaengine: add dma_slave_get_caps api

[Lars-Peter Clausen]

On 07/08/2013 03:40 PM, Vinod Koul wrote:
> On Mon, Jul 08, 2013 at 02:01:35PM +0200, Lars-Peter Clausen wrote:
>> On 07/08/2013 10:54 AM, Vinod Koul wrote:
>>> +/* struct dma_slave_caps - expose capabilities of a slave channel only
>>> + *
>>> + * @src_addr_widths: bit mask of src addr widths the channel supports
>>> + * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
>>> + * @src_burst_lengths: bit mask of src slave burst lengths supported
>>> + * @dstn_burst_lengths: bit mask of dstn slave burst lengths supported
>>
>> I'm not sure if we need the burst_lengths fields. For one we can only
>> express a max burst length up 32. And usually it is fine if the dma
>> controller does not support the burst length requested by the slave driver,
>> since this only specifies the maximum and the dma controller driver can
>> choose a value below this limit. E.g. if the max burst length is set to 16
>> it is still fine if the controller only supports a burst length of 8.
> well how are you picking up which one to use?
> The idea is that you query and match that with system and client to get best
> throughput. If you have IP block which over generations change capablities you
> can runtime query and then program the channel smartly!

The client would always request the largest burst size it can support, and
the dma master would then select a burst size equal or smaller to it
depending on what it can support.

E.g. if the client has an internal FIFO with room for 16 samples and it
sends a request once the FIFO is half empty, max_burst would be set to 8. If
the master supports burst sizes of 8 sample it would send 8 samples, if it
only supported burst sizes of 4 it might send either one burst of 4 or two of 4.

- Lars

Re: [RFC] dmaengine: add dma_slave_get_caps api

[Vinod Koul]

On Mon, Jul 08, 2013 at 04:28:29PM +0200, Lars-Peter Clausen wrote:
> On 07/08/2013 03:40 PM, Vinod Koul wrote:
> > On Mon, Jul 08, 2013 at 02:01:35PM +0200, Lars-Peter Clausen wrote:
> >> On 07/08/2013 10:54 AM, Vinod Koul wrote:
> >>> +/* struct dma_slave_caps - expose capabilities of a slave channel only
> >>> + *
> >>> + * @src_addr_widths: bit mask of src addr widths the channel supports
> >>> + * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
> >>> + * @src_burst_lengths: bit mask of src slave burst lengths supported
> >>> + * @dstn_burst_lengths: bit mask of dstn slave burst lengths supported
> >>
> >> I'm not sure if we need the burst_lengths fields. For one we can only
> >> express a max burst length up 32. And usually it is fine if the dma
> >> controller does not support the burst length requested by the slave driver,
> >> since this only specifies the maximum and the dma controller driver can
> >> choose a value below this limit. E.g. if the max burst length is set to 16
> >> it is still fine if the controller only supports a burst length of 8.
> > well how are you picking up which one to use?
> > The idea is that you query and match that with system and client to get best
> > throughput. If you have IP block which over generations change capablities you
> > can runtime query and then program the channel smartly!
> 
> The client would always request the largest burst size it can support, and
> the dma master would then select a burst size equal or smaller to it
> depending on what it can support.
not really, most dmac drivers will return an error when they find a value is not
in the supported list. And that is precisely what we would like to prevent.
If you look at current model, it mostly program value which we know is already
a capablity and works model.

> E.g. if the client has an internal FIFO with room for 16 samples and it
> sends a request once the FIFO is half empty, max_burst would be set to 8. If
> the master supports burst sizes of 8 sample it would send 8 samples, if it
> only supported burst sizes of 4 it might send either one burst of 4 or two of 4.

~Vinod
-- 

Re: [RFC] dmaengine: add dma_slave_get_caps api

[Lars-Peter Clausen]

On 07/08/2013 04:03 PM, Vinod Koul wrote:
> On Mon, Jul 08, 2013 at 04:28:29PM +0200, Lars-Peter Clausen wrote:
>> On 07/08/2013 03:40 PM, Vinod Koul wrote:
>>> On Mon, Jul 08, 2013 at 02:01:35PM +0200, Lars-Peter Clausen wrote:
>>>> On 07/08/2013 10:54 AM, Vinod Koul wrote:
>>>>> +/* struct dma_slave_caps - expose capabilities of a slave channel only
>>>>> + *
>>>>> + * @src_addr_widths: bit mask of src addr widths the channel supports
>>>>> + * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
>>>>> + * @src_burst_lengths: bit mask of src slave burst lengths supported
>>>>> + * @dstn_burst_lengths: bit mask of dstn slave burst lengths supported
>>>>
>>>> I'm not sure if we need the burst_lengths fields. For one we can only
>>>> express a max burst length up 32. And usually it is fine if the dma
>>>> controller does not support the burst length requested by the slave driver,
>>>> since this only specifies the maximum and the dma controller driver can
>>>> choose a value below this limit. E.g. if the max burst length is set to 16
>>>> it is still fine if the controller only supports a burst length of 8.
>>> well how are you picking up which one to use?
>>> The idea is that you query and match that with system and client to get best
>>> throughput. If you have IP block which over generations change capablities you
>>> can runtime query and then program the channel smartly!
>>
>> The client would always request the largest burst size it can support, and
>> the dma master would then select a burst size equal or smaller to it
>> depending on what it can support.
> not really, most dmac drivers will return an error when they find a value is not
> in the supported list. And that is precisely what we would like to prevent.
> If you look at current model, it mostly program value which we know is already
> a capablity and works model.

The majority of the DMA drivers will either accept any maxburst value (which
is probably broken) or round down. The only ones which accept only a fixed
set of burst sizes seem to be mmp_pdma, mmp_tdma and sa11x0-dma.

- Lars

[PATCH] dmaengine: add dma_slave_get_caps api

[Vinod Koul]

add new device callback .device_slave_caps api which can be used by clients to
query the dma channel capablties before they program the channel. This can help
is removing errors during the channel programming. Also add helper
dma_slave_get_caps API

This patch folds the work done by Matt earlier
https://patchwork.kernel.org/patch/2094891/

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
--
this version
	changes api to include struct dma_slave_caps as arg
	removed burst lengths, can be added when we have users
--

 include/linux/dmaengine.h |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index cb286b1..f247660 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -370,6 +370,37 @@ struct dma_slave_config {
 	unsigned int slave_id;
 };
 
+/* struct dma_slave_caps - expose capabilities of a slave channel only
+ *
+ * @src_addr_widths: bit mask of src addr widths the channel supports
+ * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
+ * @src_burst_lengths: bit mask of src slave burst lengths supported
+ * @dstn_burst_lengths: bit mask of dstn slave burst lengths supported
+ * @directions: bit mask of slave direction the channel supported
+ * 	since the enum dma_transfer_direction is not defined as bits for each
+ * 	type of direction, the dma controller should fill (1 << <TYPE>) and same
+ * 	should be checked by controller as well
+ * @cmd_pause: true, if pause and thereby resume is supported
+ * @cmd_terminate: true, if terminate cmd is supported
+ *
+ * @max_seg_nr: maximum number of SG segments supported
+ * 	0 for no maximum
+ * @max_seg_len: maximum length of a SG segment supported
+ * 	0 for no maximum
+ */
+struct dma_slave_caps {
+	u32 src_addr_widths;
+	u32 dstn_addr_widths;
+	u32 src_burst_lengths;
+	u32 dstn_burst_lengths;
+	u32 directions;
+	bool cmd_pause;
+	bool cmd_terminate;
+
+	u32 max_sg_nr;
+	u32 max_sg_len;
+};
+
 static inline const char *dma_chan_name(struct dma_chan *chan)
 {
 	return dev_name(&chan->dev->device);
@@ -532,6 +563,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_caps: return the slave channel capabilities
  */
 struct dma_device {
 
@@ -597,6 +629,7 @@ struct dma_device {
 					    dma_cookie_t cookie,
 					    struct dma_tx_state *txstate);
 	void (*device_issue_pending)(struct dma_chan *chan);
+	struct dma_slave_caps *(*device_slave_caps)(struct dma_chan *chan);
 };
 
 static inline int dmaengine_device_control(struct dma_chan *chan,
@@ -670,6 +703,18 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma(
 	return chan->device->device_prep_interleaved_dma(chan, xt, flags);
 }
 
+static inline struct dma_slave_caps *dma_get_slave_caps(struct dma_chan *chan)
+{
+	/* check if the channel supports slave transactions */
+	if (!test_bit(DMA_SLAVE, chan->device->cap_mask.bits))
+		return NULL;
+
+	if (chan->device->device_slave_caps)
+		return chan->device->device_slave_caps(chan);
+
+	return NULL;
+}
+
 static inline int dmaengine_terminate_all(struct dma_chan *chan)
 {
 	return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
-- 
1.7.0.4

Re: [PATCH] dmaengine: add dma_slave_get_caps api

[Lars-Peter Clausen]

On 07/15/2013 11:57 AM, Vinod Koul wrote:
> add new device callback .device_slave_caps api which can be used by clients to
> query the dma channel capablties before they program the channel. This can help
> is removing errors during the channel programming. Also add helper
> dma_slave_get_caps API
> 
> This patch folds the work done by Matt earlier
> https://patchwork.kernel.org/patch/2094891/
> 
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> --
> this version
> 	changes api to include struct dma_slave_caps as arg
> 	removed burst lengths, can be added when we have users

Hi,

it looks like you did send out v1 by accident.

- Lars

> --
> 
>  include/linux/dmaengine.h |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 45 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index cb286b1..f247660 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -370,6 +370,37 @@ struct dma_slave_config {
>  	unsigned int slave_id;
>  };
>  
> +/* struct dma_slave_caps - expose capabilities of a slave channel only
> + *
> + * @src_addr_widths: bit mask of src addr widths the channel supports
> + * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
> + * @src_burst_lengths: bit mask of src slave burst lengths supported
> + * @dstn_burst_lengths: bit mask of dstn slave burst lengths supported
> + * @directions: bit mask of slave direction the channel supported
> + * 	since the enum dma_transfer_direction is not defined as bits for each
> + * 	type of direction, the dma controller should fill (1 << <TYPE>) and same
> + * 	should be checked by controller as well
> + * @cmd_pause: true, if pause and thereby resume is supported
> + * @cmd_terminate: true, if terminate cmd is supported
> + *
> + * @max_seg_nr: maximum number of SG segments supported
> + * 	0 for no maximum
> + * @max_seg_len: maximum length of a SG segment supported
> + * 	0 for no maximum
> + */
> +struct dma_slave_caps {
> +	u32 src_addr_widths;
> +	u32 dstn_addr_widths;
> +	u32 src_burst_lengths;
> +	u32 dstn_burst_lengths;
> +	u32 directions;
> +	bool cmd_pause;
> +	bool cmd_terminate;
> +
> +	u32 max_sg_nr;
> +	u32 max_sg_len;
> +};
> +
>  static inline const char *dma_chan_name(struct dma_chan *chan)
>  {
>  	return dev_name(&chan->dev->device);
> @@ -532,6 +563,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_caps: return the slave channel capabilities
>   */
>  struct dma_device {
>  
> @@ -597,6 +629,7 @@ struct dma_device {
>  					    dma_cookie_t cookie,
>  					    struct dma_tx_state *txstate);
>  	void (*device_issue_pending)(struct dma_chan *chan);
> +	struct dma_slave_caps *(*device_slave_caps)(struct dma_chan *chan);
>  };
>  
>  static inline int dmaengine_device_control(struct dma_chan *chan,
> @@ -670,6 +703,18 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma(
>  	return chan->device->device_prep_interleaved_dma(chan, xt, flags);
>  }
>  
> +static inline struct dma_slave_caps *dma_get_slave_caps(struct dma_chan *chan)
> +{
> +	/* check if the channel supports slave transactions */
> +	if (!test_bit(DMA_SLAVE, chan->device->cap_mask.bits))
> +		return NULL;
> +
> +	if (chan->device->device_slave_caps)
> +		return chan->device->device_slave_caps(chan);
> +
> +	return NULL;
> +}
> +
>  static inline int dmaengine_terminate_all(struct dma_chan *chan)
>  {
>  	return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
> 

Re: [PATCH] dmaengine: add dma_slave_get_caps api

[Vinod Koul]

On Mon, Jul 15, 2013 at 12:52:19PM +0200, Lars-Peter Clausen wrote:
> On 07/15/2013 11:57 AM, Vinod Koul wrote:
> > add new device callback .device_slave_caps api which can be used by clients to
> > query the dma channel capablties before they program the channel. This can help
> > is removing errors during the channel programming. Also add helper
> > dma_slave_get_caps API
> > 
> > This patch folds the work done by Matt earlier
> > https://patchwork.kernel.org/patch/2094891/
> > 
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > --
> > this version
> > 	changes api to include struct dma_slave_caps as arg
> > 	removed burst lengths, can be added when we have users
> 
> Hi,
> 
> it looks like you did send out v1 by accident.
Yes but problem wasnt sending wrong patch but generating patch without
comitting!

resent now...

I am planning to keep this on a branch which I wont rebase. That way users
(mostly ASoC for now), can merge this and get drivers converted.

~Vinod

[PATCH] dmaengine: add dma_slave_get_caps api

[Vinod Koul]

add new device callback .device_slave_caps api which can be used by clients to
query the dma channel capablties before they program the channel. This can help
is removing errors during the channel programming. Also add helper
dma_slave_get_caps API

This patch folds the work done by Matt earlier
https://patchwork.kernel.org/patch/2094891/

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
this version
	changes api to include struct dma_slave_caps as arg
	removed burst lengths, can be added when we have users
--

 include/linux/dmaengine.h |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index cb286b1..3582593 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -370,6 +370,33 @@ struct dma_slave_config {
 	unsigned int slave_id;
 };
 
+/* struct dma_slave_caps - expose capabilities of a slave channel only
+ *
+ * @src_addr_widths: bit mask of src addr widths the channel supports
+ * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
+ * @directions: bit mask of slave direction the channel supported
+ * 	since the enum dma_transfer_direction is not defined as bits for each
+ * 	type of direction, the dma controller should fill (1 << <TYPE>) and same
+ * 	should be checked by controller as well
+ * @cmd_pause: true, if pause and thereby resume is supported
+ * @cmd_terminate: true, if terminate cmd is supported
+ *
+ * @max_seg_nr: maximum number of SG segments supported
+ * 	0 for no maximum
+ * @max_seg_len: maximum length of a SG segment supported
+ * 	0 for no maximum
+ */
+struct dma_slave_caps {
+	u32 src_addr_widths;
+	u32 dstn_addr_widths;
+	u32 directions;
+	bool cmd_pause;
+	bool cmd_terminate;
+
+	u32 max_sg_nr;
+	u32 max_sg_len;
+};
+
 static inline const char *dma_chan_name(struct dma_chan *chan)
 {
 	return dev_name(&chan->dev->device);
@@ -532,6 +559,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_caps: return the slave channel capabilities
  */
 struct dma_device {
 
@@ -597,6 +625,7 @@ struct dma_device {
 					    dma_cookie_t cookie,
 					    struct dma_tx_state *txstate);
 	void (*device_issue_pending)(struct dma_chan *chan);
+	int (*device_slave_caps)(struct dma_chan *chan, struct dma_slave_caps *caps);
 };
 
 static inline int dmaengine_device_control(struct dma_chan *chan,
@@ -670,6 +699,21 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma(
 	return chan->device->device_prep_interleaved_dma(chan, xt, flags);
 }
 
+static inline int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
+{
+	if (!chan || !caps)
+		return -EINVAL;
+
+	/* check if the channel supports slave transactions */
+	if (!test_bit(DMA_SLAVE, chan->device->cap_mask.bits))
+		return -ENXIO;
+
+	if (chan->device->device_slave_caps)
+		return chan->device->device_slave_caps(chan, caps);
+
+	return -ENXIO;
+}
+
 static inline int dmaengine_terminate_all(struct dma_chan *chan)
 {
 	return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
-- 
1.7.0.4

Re: [PATCH] dmaengine: add dma_slave_get_caps api

[Lars-Peter Clausen]

On 07/15/2013 12:21 PM, Vinod Koul wrote:
> add new device callback .device_slave_caps api which can be used by clients to
> query the dma channel capablties before they program the channel. This can help
> is removing errors during the channel programming. Also add helper
> dma_slave_get_caps API
> 
> This patch folds the work done by Matt earlier
> https://patchwork.kernel.org/patch/2094891/
> 
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>

Thanks, looks good to me. I already had some changes to the dmaengine PCM
driver based on Matt's patch, I'll rework them on top of this one.

- Lars

> ---
> this version
> 	changes api to include struct dma_slave_caps as arg
> 	removed burst lengths, can be added when we have users
> --
> 
>  include/linux/dmaengine.h |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 44 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index cb286b1..3582593 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -370,6 +370,33 @@ struct dma_slave_config {
>  	unsigned int slave_id;
>  };
>  
> +/* struct dma_slave_caps - expose capabilities of a slave channel only
> + *
> + * @src_addr_widths: bit mask of src addr widths the channel supports
> + * @dstn_addr_widths: bit mask of dstn addr widths the channel supports
> + * @directions: bit mask of slave direction the channel supported
> + * 	since the enum dma_transfer_direction is not defined as bits for each
> + * 	type of direction, the dma controller should fill (1 << <TYPE>) and same
> + * 	should be checked by controller as well
> + * @cmd_pause: true, if pause and thereby resume is supported
> + * @cmd_terminate: true, if terminate cmd is supported
> + *
> + * @max_seg_nr: maximum number of SG segments supported
> + * 	0 for no maximum
> + * @max_seg_len: maximum length of a SG segment supported
> + * 	0 for no maximum
> + */
> +struct dma_slave_caps {
> +	u32 src_addr_widths;
> +	u32 dstn_addr_widths;
> +	u32 directions;
> +	bool cmd_pause;
> +	bool cmd_terminate;
> +
> +	u32 max_sg_nr;
> +	u32 max_sg_len;
> +};
> +
>  static inline const char *dma_chan_name(struct dma_chan *chan)
>  {
>  	return dev_name(&chan->dev->device);
> @@ -532,6 +559,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_caps: return the slave channel capabilities
>   */
>  struct dma_device {
>  
> @@ -597,6 +625,7 @@ struct dma_device {
>  					    dma_cookie_t cookie,
>  					    struct dma_tx_state *txstate);
>  	void (*device_issue_pending)(struct dma_chan *chan);
> +	int (*device_slave_caps)(struct dma_chan *chan, struct dma_slave_caps *caps);
>  };
>  
>  static inline int dmaengine_device_control(struct dma_chan *chan,
> @@ -670,6 +699,21 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma(
>  	return chan->device->device_prep_interleaved_dma(chan, xt, flags);
>  }
>  
> +static inline int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
> +{
> +	if (!chan || !caps)
> +		return -EINVAL;
> +
> +	/* check if the channel supports slave transactions */
> +	if (!test_bit(DMA_SLAVE, chan->device->cap_mask.bits))
> +		return -ENXIO;
> +
> +	if (chan->device->device_slave_caps)
> +		return chan->device->device_slave_caps(chan, caps);
> +
> +	return -ENXIO;
> +}
> +
>  static inline int dmaengine_terminate_all(struct dma_chan *chan)
>  {
>  	return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
> 

Re: [PATCH] dmaengine: add dma_slave_get_caps api

[Lars-Peter Clausen]

On 07/15/2013 12:21 PM, Vinod Koul wrote:
[...]
> + * @max_seg_nr: maximum number of SG segments supported
> + * 	0 for no maximum
> + * @max_seg_len: maximum length of a SG segment supported
> + * 	0 for no maximum
[...]
> +	u32 max_sg_nr;
> +	u32 max_sg_len;

seg vs. sg

Re: [PATCH] dmaengine: add dma_slave_get_caps api

[Vinod Koul]

On Mon, Jul 15, 2013 at 05:51:34PM +0200, Lars-Peter Clausen wrote:
> On 07/15/2013 12:21 PM, Vinod Koul wrote:
> [...]
> > + * @max_seg_nr: maximum number of SG segments supported
> > + * 	0 for no maximum
> > + * @max_seg_len: maximum length of a SG segment supported
> > + * 	0 for no maximum
> [...]
> > +	u32 max_sg_nr;
> > +	u32 max_sg_len;
> 
> seg vs. sg
Thanks, fixed and applied now to topic/api_caps

--
~Vinod