[PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Shawn Guo]

The series adds a snd_dmaengine_generic_pcm_open() for users that
adopt generic DMA binding and helplers.

Vinod,

AS per the request from Mark, I add the first patch to mark name
parameter of dmaengine helpers as const, so that clients can mark
const on their side.

We need to have these two patches on a topic branch, as I've heard
a few people converting their ASoC driver needing the patches.  Can
you ack the first patch, so that Mark can maintain them in a branch
for anyone who needs it to pull?

Shawn Guo (2):
  dmaengine: add const for name parameter
  ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

 drivers/dma/dmaengine.c       |    2 +-
 drivers/dma/of-dma.c          |    6 +++---
 include/linux/dmaengine.h     |    7 ++++---
 include/linux/of_dma.h        |    2 +-
 include/sound/dmaengine_pcm.h |    2 ++
 sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 50 insertions(+), 8 deletions(-)

-- 
1.7.9.5

[PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Shawn Guo]

The series adds a snd_dmaengine_generic_pcm_open() for users that
adopt generic DMA binding and helplers.

Vinod,

AS per the request from Mark, I add the first patch to mark name
parameter of dmaengine helpers as const, so that clients can mark
const on their side.

We need to have these two patches on a topic branch, as I've heard
a few people converting their ASoC driver needing the patches.  Can
you ack the first patch, so that Mark can maintain them in a branch
for anyone who needs it to pull?

Shawn Guo (2):
  dmaengine: add const for name parameter
  ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

 drivers/dma/dmaengine.c       |    2 +-
 drivers/dma/of-dma.c          |    6 +++---
 include/linux/dmaengine.h     |    7 ++++---
 include/linux/of_dma.h        |    2 +-
 include/sound/dmaengine_pcm.h |    2 ++
 sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 50 insertions(+), 8 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] dmaengine: add const for name parameter

[Shawn Guo]

Mark name parameter of generic device tree helpers as const, as it
shouldn't be changed anyway.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/dma/dmaengine.c   |    2 +-
 drivers/dma/of-dma.c      |    6 +++---
 include/linux/dmaengine.h |    7 ++++---
 include/linux/of_dma.h    |    2 +-
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index b2728d6..2cbfefe 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -555,7 +555,7 @@ EXPORT_SYMBOL_GPL(__dma_request_channel);
  * @dev:	pointer to client device structure
  * @name:	slave channel name
  */
-struct dma_chan *dma_request_slave_channel(struct device *dev, char *name)
+struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name)
 {
 	/* If device-tree is present get slave info from here */
 	if (dev->of_node)
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 69d04d2..6036cd0 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -172,8 +172,8 @@ EXPORT_SYMBOL_GPL(of_dma_controller_free);
  * specifiers, matches the name provided. Returns 0 if the name matches and
  * a valid pointer to the DMA specifier is found. Otherwise returns -ENODEV.
  */
-static int of_dma_match_channel(struct device_node *np, char *name, int index,
-				struct of_phandle_args *dma_spec)
+static int of_dma_match_channel(struct device_node *np, const char *name,
+				int index, struct of_phandle_args *dma_spec)
 {
 	const char *s;
 
@@ -198,7 +198,7 @@ static int of_dma_match_channel(struct device_node *np, char *name, int index,
  * Returns pointer to appropriate dma channel on success or NULL on error.
  */
 struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
-					      char *name)
+					      const char *name)
 {
 	struct of_phandle_args	dma_spec;
 	struct of_dma		*ofdma;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 91ac8da..68b4252 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -968,7 +968,8 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
 enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
 void dma_issue_pending_all(void);
 struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param);
-struct dma_chan *dma_request_slave_channel(struct device *dev, char *name);
+struct dma_chan *dma_request_slave_channel(struct device *dev,
+					   const char *name);
 void dma_release_channel(struct dma_chan *chan);
 #else
 static inline enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx)
@@ -984,7 +985,7 @@ static inline struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask,
 	return NULL;
 }
 static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
-							 char *name)
+							 const char *name)
 {
 	return NULL;
 }
@@ -1007,7 +1008,7 @@ struct dma_chan *net_dma_find_channel(void);
 static inline struct dma_chan
 *__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
 				  void *fn_param, struct device *dev,
-				  char *name)
+				  const char *name)
 {
 	struct dma_chan *chan;
 
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index d15073e..0c4a82e 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -40,7 +40,7 @@ extern int of_dma_controller_register(struct device_node *np,
 		void *data);
 extern int of_dma_controller_free(struct device_node *np);
 extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
-						     char *name);
+						     const char *name);
 extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 		struct of_dma *ofdma);
 #else
-- 
1.7.9.5

[PATCH 1/2] dmaengine: add const for name parameter

[Shawn Guo]

Mark name parameter of generic device tree helpers as const, as it
shouldn't be changed anyway.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/dma/dmaengine.c   |    2 +-
 drivers/dma/of-dma.c      |    6 +++---
 include/linux/dmaengine.h |    7 ++++---
 include/linux/of_dma.h    |    2 +-
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index b2728d6..2cbfefe 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -555,7 +555,7 @@ EXPORT_SYMBOL_GPL(__dma_request_channel);
  * @dev:	pointer to client device structure
  * @name:	slave channel name
  */
-struct dma_chan *dma_request_slave_channel(struct device *dev, char *name)
+struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name)
 {
 	/* If device-tree is present get slave info from here */
 	if (dev->of_node)
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 69d04d2..6036cd0 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -172,8 +172,8 @@ EXPORT_SYMBOL_GPL(of_dma_controller_free);
  * specifiers, matches the name provided. Returns 0 if the name matches and
  * a valid pointer to the DMA specifier is found. Otherwise returns -ENODEV.
  */
-static int of_dma_match_channel(struct device_node *np, char *name, int index,
-				struct of_phandle_args *dma_spec)
+static int of_dma_match_channel(struct device_node *np, const char *name,
+				int index, struct of_phandle_args *dma_spec)
 {
 	const char *s;
 
@@ -198,7 +198,7 @@ static int of_dma_match_channel(struct device_node *np, char *name, int index,
  * Returns pointer to appropriate dma channel on success or NULL on error.
  */
 struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
-					      char *name)
+					      const char *name)
 {
 	struct of_phandle_args	dma_spec;
 	struct of_dma		*ofdma;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 91ac8da..68b4252 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -968,7 +968,8 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
 enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
 void dma_issue_pending_all(void);
 struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param);
-struct dma_chan *dma_request_slave_channel(struct device *dev, char *name);
+struct dma_chan *dma_request_slave_channel(struct device *dev,
+					   const char *name);
 void dma_release_channel(struct dma_chan *chan);
 #else
 static inline enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx)
@@ -984,7 +985,7 @@ static inline struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask,
 	return NULL;
 }
 static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
-							 char *name)
+							 const char *name)
 {
 	return NULL;
 }
@@ -1007,7 +1008,7 @@ struct dma_chan *net_dma_find_channel(void);
 static inline struct dma_chan
 *__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
 				  void *fn_param, struct device *dev,
-				  char *name)
+				  const char *name)
 {
 	struct dma_chan *chan;
 
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index d15073e..0c4a82e 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -40,7 +40,7 @@ extern int of_dma_controller_register(struct device_node *np,
 		void *data);
 extern int of_dma_controller_free(struct device_node *np);
 extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
-						     char *name);
+						     const char *name);
 extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 		struct of_dma *ofdma);
 #else
-- 
1.7.9.5

[PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Shawn Guo]

With generic DMA device tree binding and helper function
dma_request_slave_channel() in place, dmaengine_pcm should support
that in requesting DMA channel for users that support generic DMA
device tree binding.

Add a new API snd_dmaengine_generic_pcm_open() as the variant of
snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
and slave channel name as arguments and calls generic DMA helper
dma_request_slave_channel() to request DMA channel from dmaengine.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: alsa-devel@alsa-project.org
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 include/sound/dmaengine_pcm.h |    2 ++
 sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
index b877334..452df15 100644
--- a/include/sound/dmaengine_pcm.h
+++ b/include/sound/dmaengine_pcm.h
@@ -43,6 +43,8 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
 
 int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
 	dma_filter_fn filter_fn, void *filter_data);
+int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
+				   struct device *dev, const char *name);
 int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
 
 struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 111b7d92..970eb2b 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -304,6 +304,45 @@ int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
 EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open);
 
 /**
+ * snd_dmaengine_generic_pcm_open - Open a dmaengine based PCM substream
+ * @substream: PCM substream
+ * @dev: pointer to DMA client device structure
+ * @name: DMA slave channel name
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ *
+ * This function is a variant of snd_dmaengine_pcm_open(). It takes different
+ * parameters and calls generic DMA helper dma_request_slave_channel() to
+ * request a DMA channel from dmaengine.
+ */
+int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
+				   struct device *dev, const char *name)
+{
+	struct dmaengine_pcm_runtime_data *prtd;
+	int ret;
+
+	ret = snd_pcm_hw_constraint_integer(substream->runtime,
+					    SNDRV_PCM_HW_PARAM_PERIODS);
+	if (ret < 0)
+		return ret;
+
+	prtd = kzalloc(sizeof(*prtd), GFP_KERNEL);
+	if (!prtd)
+		return -ENOMEM;
+
+	prtd->dma_chan = dma_request_slave_channel(dev, name);
+	if (!prtd->dma_chan) {
+		kfree(prtd);
+		return -ENXIO;
+	}
+
+	substream->runtime->private_data = prtd;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_generic_pcm_open);
+
+/**
  * snd_dmaengine_pcm_close - Close a dmaengine based PCM substream
  * @substream: PCM substream
  */
-- 
1.7.9.5

[PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Shawn Guo]

With generic DMA device tree binding and helper function
dma_request_slave_channel() in place, dmaengine_pcm should support
that in requesting DMA channel for users that support generic DMA
device tree binding.

Add a new API snd_dmaengine_generic_pcm_open() as the variant of
snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
and slave channel name as arguments and calls generic DMA helper
dma_request_slave_channel() to request DMA channel from dmaengine.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: alsa-devel at alsa-project.org
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 include/sound/dmaengine_pcm.h |    2 ++
 sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
index b877334..452df15 100644
--- a/include/sound/dmaengine_pcm.h
+++ b/include/sound/dmaengine_pcm.h
@@ -43,6 +43,8 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
 
 int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
 	dma_filter_fn filter_fn, void *filter_data);
+int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
+				   struct device *dev, const char *name);
 int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
 
 struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 111b7d92..970eb2b 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -304,6 +304,45 @@ int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
 EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open);
 
 /**
+ * snd_dmaengine_generic_pcm_open - Open a dmaengine based PCM substream
+ * @substream: PCM substream
+ * @dev: pointer to DMA client device structure
+ * @name: DMA slave channel name
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ *
+ * This function is a variant of snd_dmaengine_pcm_open(). It takes different
+ * parameters and calls generic DMA helper dma_request_slave_channel() to
+ * request a DMA channel from dmaengine.
+ */
+int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
+				   struct device *dev, const char *name)
+{
+	struct dmaengine_pcm_runtime_data *prtd;
+	int ret;
+
+	ret = snd_pcm_hw_constraint_integer(substream->runtime,
+					    SNDRV_PCM_HW_PARAM_PERIODS);
+	if (ret < 0)
+		return ret;
+
+	prtd = kzalloc(sizeof(*prtd), GFP_KERNEL);
+	if (!prtd)
+		return -ENOMEM;
+
+	prtd->dma_chan = dma_request_slave_channel(dev, name);
+	if (!prtd->dma_chan) {
+		kfree(prtd);
+		return -ENXIO;
+	}
+
+	substream->runtime->private_data = prtd;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_generic_pcm_open);
+
+/**
  * snd_dmaengine_pcm_close - Close a dmaengine based PCM substream
  * @substream: PCM substream
  */
-- 
1.7.9.5

Re: [PATCH 1/2] dmaengine: add const for name parameter

[Markus Pargmann]

Hi,

I already sent a nearly identical patch:
https://patchwork.kernel.org/patch/2179321/

Regards,

Markus

On Fri, Mar 15, 2013 at 11:36:40AM +0800, Shawn Guo wrote:
> Mark name parameter of generic device tree helpers as const, as it
> shouldn't be changed anyway.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/dma/dmaengine.c   |    2 +-
>  drivers/dma/of-dma.c      |    6 +++---
>  include/linux/dmaengine.h |    7 ++++---
>  include/linux/of_dma.h    |    2 +-
>  4 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index b2728d6..2cbfefe 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -555,7 +555,7 @@ EXPORT_SYMBOL_GPL(__dma_request_channel);
>   * @dev:	pointer to client device structure
>   * @name:	slave channel name
>   */
> -struct dma_chan *dma_request_slave_channel(struct device *dev, char *name)
> +struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name)
>  {
>  	/* If device-tree is present get slave info from here */
>  	if (dev->of_node)
> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> index 69d04d2..6036cd0 100644
> --- a/drivers/dma/of-dma.c
> +++ b/drivers/dma/of-dma.c
> @@ -172,8 +172,8 @@ EXPORT_SYMBOL_GPL(of_dma_controller_free);
>   * specifiers, matches the name provided. Returns 0 if the name matches and
>   * a valid pointer to the DMA specifier is found. Otherwise returns -ENODEV.
>   */
> -static int of_dma_match_channel(struct device_node *np, char *name, int index,
> -				struct of_phandle_args *dma_spec)
> +static int of_dma_match_channel(struct device_node *np, const char *name,
> +				int index, struct of_phandle_args *dma_spec)
>  {
>  	const char *s;
>  
> @@ -198,7 +198,7 @@ static int of_dma_match_channel(struct device_node *np, char *name, int index,
>   * Returns pointer to appropriate dma channel on success or NULL on error.
>   */
>  struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> -					      char *name)
> +					      const char *name)
>  {
>  	struct of_phandle_args	dma_spec;
>  	struct of_dma		*ofdma;
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 91ac8da..68b4252 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -968,7 +968,8 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
>  enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
>  void dma_issue_pending_all(void);
>  struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param);
> -struct dma_chan *dma_request_slave_channel(struct device *dev, char *name);
> +struct dma_chan *dma_request_slave_channel(struct device *dev,
> +					   const char *name);
>  void dma_release_channel(struct dma_chan *chan);
>  #else
>  static inline enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx)
> @@ -984,7 +985,7 @@ static inline struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask,
>  	return NULL;
>  }
>  static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
> -							 char *name)
> +							 const char *name)
>  {
>  	return NULL;
>  }
> @@ -1007,7 +1008,7 @@ struct dma_chan *net_dma_find_channel(void);
>  static inline struct dma_chan
>  *__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
>  				  void *fn_param, struct device *dev,
> -				  char *name)
> +				  const char *name)
>  {
>  	struct dma_chan *chan;
>  
> diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> index d15073e..0c4a82e 100644
> --- a/include/linux/of_dma.h
> +++ b/include/linux/of_dma.h
> @@ -40,7 +40,7 @@ extern int of_dma_controller_register(struct device_node *np,
>  		void *data);
>  extern int of_dma_controller_free(struct device_node *np);
>  extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> -						     char *name);
> +						     const char *name);
>  extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
>  		struct of_dma *ofdma);
>  #else
> -- 
> 1.7.9.5
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/2] dmaengine: add const for name parameter

[Markus Pargmann]

Hi,

I already sent a nearly identical patch:
https://patchwork.kernel.org/patch/2179321/

Regards,

Markus

On Fri, Mar 15, 2013 at 11:36:40AM +0800, Shawn Guo wrote:
> Mark name parameter of generic device tree helpers as const, as it
> shouldn't be changed anyway.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/dma/dmaengine.c   |    2 +-
>  drivers/dma/of-dma.c      |    6 +++---
>  include/linux/dmaengine.h |    7 ++++---
>  include/linux/of_dma.h    |    2 +-
>  4 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index b2728d6..2cbfefe 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -555,7 +555,7 @@ EXPORT_SYMBOL_GPL(__dma_request_channel);
>   * @dev:	pointer to client device structure
>   * @name:	slave channel name
>   */
> -struct dma_chan *dma_request_slave_channel(struct device *dev, char *name)
> +struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name)
>  {
>  	/* If device-tree is present get slave info from here */
>  	if (dev->of_node)
> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> index 69d04d2..6036cd0 100644
> --- a/drivers/dma/of-dma.c
> +++ b/drivers/dma/of-dma.c
> @@ -172,8 +172,8 @@ EXPORT_SYMBOL_GPL(of_dma_controller_free);
>   * specifiers, matches the name provided. Returns 0 if the name matches and
>   * a valid pointer to the DMA specifier is found. Otherwise returns -ENODEV.
>   */
> -static int of_dma_match_channel(struct device_node *np, char *name, int index,
> -				struct of_phandle_args *dma_spec)
> +static int of_dma_match_channel(struct device_node *np, const char *name,
> +				int index, struct of_phandle_args *dma_spec)
>  {
>  	const char *s;
>  
> @@ -198,7 +198,7 @@ static int of_dma_match_channel(struct device_node *np, char *name, int index,
>   * Returns pointer to appropriate dma channel on success or NULL on error.
>   */
>  struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> -					      char *name)
> +					      const char *name)
>  {
>  	struct of_phandle_args	dma_spec;
>  	struct of_dma		*ofdma;
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 91ac8da..68b4252 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -968,7 +968,8 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
>  enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
>  void dma_issue_pending_all(void);
>  struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param);
> -struct dma_chan *dma_request_slave_channel(struct device *dev, char *name);
> +struct dma_chan *dma_request_slave_channel(struct device *dev,
> +					   const char *name);
>  void dma_release_channel(struct dma_chan *chan);
>  #else
>  static inline enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx)
> @@ -984,7 +985,7 @@ static inline struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask,
>  	return NULL;
>  }
>  static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
> -							 char *name)
> +							 const char *name)
>  {
>  	return NULL;
>  }
> @@ -1007,7 +1008,7 @@ struct dma_chan *net_dma_find_channel(void);
>  static inline struct dma_chan
>  *__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
>  				  void *fn_param, struct device *dev,
> -				  char *name)
> +				  const char *name)
>  {
>  	struct dma_chan *chan;
>  
> diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> index d15073e..0c4a82e 100644
> --- a/include/linux/of_dma.h
> +++ b/include/linux/of_dma.h
> @@ -40,7 +40,7 @@ extern int of_dma_controller_register(struct device_node *np,
>  		void *data);
>  extern int of_dma_controller_free(struct device_node *np);
>  extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> -						     char *name);
> +						     const char *name);
>  extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
>  		struct of_dma *ofdma);
>  #else
> -- 
> 1.7.9.5
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Sebastien Guiriec]

Test on OMAP with patches
  http://www.spinics.net/lists/linux-omap/msg87893.html

Tested-by: Sebastien Guiriec <s-guiriec@ti.com>

On 03/15/2013 04:36 AM, Shawn Guo wrote:
> With generic DMA device tree binding and helper function
> dma_request_slave_channel() in place, dmaengine_pcm should support
> that in requesting DMA channel for users that support generic DMA
> device tree binding.
>
> Add a new API snd_dmaengine_generic_pcm_open() as the variant of
> snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
> and slave channel name as arguments and calls generic DMA helper
> dma_request_slave_channel() to request DMA channel from dmaengine.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: alsa-devel@alsa-project.org
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   include/sound/dmaengine_pcm.h |    2 ++
>   sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+)
>
> diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
> index b877334..452df15 100644
> --- a/include/sound/dmaengine_pcm.h
> +++ b/include/sound/dmaengine_pcm.h
> @@ -43,6 +43,8 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
>
>   int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>   	dma_filter_fn filter_fn, void *filter_data);
> +int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
> +				   struct device *dev, const char *name);
>   int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
>
>   struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> index 111b7d92..970eb2b 100644
> --- a/sound/soc/soc-dmaengine-pcm.c
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -304,6 +304,45 @@ int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>   EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open);
>
>   /**
> + * snd_dmaengine_generic_pcm_open - Open a dmaengine based PCM substream
> + * @substream: PCM substream
> + * @dev: pointer to DMA client device structure
> + * @name: DMA slave channel name
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + *
> + * This function is a variant of snd_dmaengine_pcm_open(). It takes different
> + * parameters and calls generic DMA helper dma_request_slave_channel() to
> + * request a DMA channel from dmaengine.
> + */
> +int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
> +				   struct device *dev, const char *name)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd;
> +	int ret;
> +
> +	ret = snd_pcm_hw_constraint_integer(substream->runtime,
> +					    SNDRV_PCM_HW_PARAM_PERIODS);
> +	if (ret < 0)
> +		return ret;
> +
> +	prtd = kzalloc(sizeof(*prtd), GFP_KERNEL);
> +	if (!prtd)
> +		return -ENOMEM;
> +
> +	prtd->dma_chan = dma_request_slave_channel(dev, name);
> +	if (!prtd->dma_chan) {
> +		kfree(prtd);
> +		return -ENXIO;
> +	}
> +
> +	substream->runtime->private_data = prtd;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_generic_pcm_open);
> +
> +/**
>    * snd_dmaengine_pcm_close - Close a dmaengine based PCM substream
>    * @substream: PCM substream
>    */
>

[PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Sebastien Guiriec]

Test on OMAP with patches
  http://www.spinics.net/lists/linux-omap/msg87893.html

Tested-by: Sebastien Guiriec <s-guiriec@ti.com>

On 03/15/2013 04:36 AM, Shawn Guo wrote:
> With generic DMA device tree binding and helper function
> dma_request_slave_channel() in place, dmaengine_pcm should support
> that in requesting DMA channel for users that support generic DMA
> device tree binding.
>
> Add a new API snd_dmaengine_generic_pcm_open() as the variant of
> snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
> and slave channel name as arguments and calls generic DMA helper
> dma_request_slave_channel() to request DMA channel from dmaengine.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: alsa-devel at alsa-project.org
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   include/sound/dmaengine_pcm.h |    2 ++
>   sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+)
>
> diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
> index b877334..452df15 100644
> --- a/include/sound/dmaengine_pcm.h
> +++ b/include/sound/dmaengine_pcm.h
> @@ -43,6 +43,8 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
>
>   int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>   	dma_filter_fn filter_fn, void *filter_data);
> +int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
> +				   struct device *dev, const char *name);
>   int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
>
>   struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> index 111b7d92..970eb2b 100644
> --- a/sound/soc/soc-dmaengine-pcm.c
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -304,6 +304,45 @@ int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>   EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open);
>
>   /**
> + * snd_dmaengine_generic_pcm_open - Open a dmaengine based PCM substream
> + * @substream: PCM substream
> + * @dev: pointer to DMA client device structure
> + * @name: DMA slave channel name
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + *
> + * This function is a variant of snd_dmaengine_pcm_open(). It takes different
> + * parameters and calls generic DMA helper dma_request_slave_channel() to
> + * request a DMA channel from dmaengine.
> + */
> +int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
> +				   struct device *dev, const char *name)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd;
> +	int ret;
> +
> +	ret = snd_pcm_hw_constraint_integer(substream->runtime,
> +					    SNDRV_PCM_HW_PARAM_PERIODS);
> +	if (ret < 0)
> +		return ret;
> +
> +	prtd = kzalloc(sizeof(*prtd), GFP_KERNEL);
> +	if (!prtd)
> +		return -ENOMEM;
> +
> +	prtd->dma_chan = dma_request_slave_channel(dev, name);
> +	if (!prtd->dma_chan) {
> +		kfree(prtd);
> +		return -ENXIO;
> +	}
> +
> +	substream->runtime->private_data = prtd;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_generic_pcm_open);
> +
> +/**
>    * snd_dmaengine_pcm_close - Close a dmaengine based PCM substream
>    * @substream: PCM substream
>    */
>

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Shawn Guo]

On Fri, Mar 15, 2013 at 11:36:39AM +0800, Shawn Guo wrote:
> The series adds a snd_dmaengine_generic_pcm_open() for users that
> adopt generic DMA binding and helplers.
> 
> Vinod,
> 
Ping?

Shawn

> AS per the request from Mark, I add the first patch to mark name
> parameter of dmaengine helpers as const, so that clients can mark
> const on their side.
> 
> We need to have these two patches on a topic branch, as I've heard
> a few people converting their ASoC driver needing the patches.  Can
> you ack the first patch, so that Mark can maintain them in a branch
> for anyone who needs it to pull?
> 
> Shawn Guo (2):
>   dmaengine: add const for name parameter
>   ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()
> 
>  drivers/dma/dmaengine.c       |    2 +-
>  drivers/dma/of-dma.c          |    6 +++---
>  include/linux/dmaengine.h     |    7 ++++---
>  include/linux/of_dma.h        |    2 +-
>  include/sound/dmaengine_pcm.h |    2 ++
>  sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
>  6 files changed, 50 insertions(+), 8 deletions(-)
> 
> -- 
> 1.7.9.5
> 
> 

[PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Shawn Guo]

On Fri, Mar 15, 2013 at 11:36:39AM +0800, Shawn Guo wrote:
> The series adds a snd_dmaengine_generic_pcm_open() for users that
> adopt generic DMA binding and helplers.
> 
> Vinod,
> 
Ping?

Shawn

> AS per the request from Mark, I add the first patch to mark name
> parameter of dmaengine helpers as const, so that clients can mark
> const on their side.
> 
> We need to have these two patches on a topic branch, as I've heard
> a few people converting their ASoC driver needing the patches.  Can
> you ack the first patch, so that Mark can maintain them in a branch
> for anyone who needs it to pull?
> 
> Shawn Guo (2):
>   dmaengine: add const for name parameter
>   ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()
> 
>  drivers/dma/dmaengine.c       |    2 +-
>  drivers/dma/of-dma.c          |    6 +++---
>  include/linux/dmaengine.h     |    7 ++++---
>  include/linux/of_dma.h        |    2 +-
>  include/sound/dmaengine_pcm.h |    2 ++
>  sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
>  6 files changed, 50 insertions(+), 8 deletions(-)
> 
> -- 
> 1.7.9.5
> 
> 

Re: [PATCH 1/2] dmaengine: add const for name parameter

[Vinod Koul]

On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> Hi,
> 
> I already sent a nearly identical patch:
> https://patchwork.kernel.org/patch/2179321/
I applied that one a while ago so Markus can you check my tree and see if
anything is remaining, looks most of cases are covered

> 
> Regards,
> 
> Markus
> 
> On Fri, Mar 15, 2013 at 11:36:40AM +0800, Shawn Guo wrote:
> > Mark name parameter of generic device tree helpers as const, as it
> > shouldn't be changed anyway.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> >  drivers/dma/dmaengine.c   |    2 +-
> >  drivers/dma/of-dma.c      |    6 +++---
> >  include/linux/dmaengine.h |    7 ++++---
> >  include/linux/of_dma.h    |    2 +-
> >  4 files changed, 9 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > index b2728d6..2cbfefe 100644
> > --- a/drivers/dma/dmaengine.c
> > +++ b/drivers/dma/dmaengine.c
> > @@ -555,7 +555,7 @@ EXPORT_SYMBOL_GPL(__dma_request_channel);
> >   * @dev:	pointer to client device structure
> >   * @name:	slave channel name
> >   */
> > -struct dma_chan *dma_request_slave_channel(struct device *dev, char *name)
> > +struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name)
> >  {
> >  	/* If device-tree is present get slave info from here */
> >  	if (dev->of_node)
> > diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> > index 69d04d2..6036cd0 100644
> > --- a/drivers/dma/of-dma.c
> > +++ b/drivers/dma/of-dma.c
> > @@ -172,8 +172,8 @@ EXPORT_SYMBOL_GPL(of_dma_controller_free);
> >   * specifiers, matches the name provided. Returns 0 if the name matches and
> >   * a valid pointer to the DMA specifier is found. Otherwise returns -ENODEV.
> >   */
> > -static int of_dma_match_channel(struct device_node *np, char *name, int index,
> > -				struct of_phandle_args *dma_spec)
> > +static int of_dma_match_channel(struct device_node *np, const char *name,
> > +				int index, struct of_phandle_args *dma_spec)
> >  {
> >  	const char *s;
> >  
> > @@ -198,7 +198,7 @@ static int of_dma_match_channel(struct device_node *np, char *name, int index,
> >   * Returns pointer to appropriate dma channel on success or NULL on error.
> >   */
> >  struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> > -					      char *name)
> > +					      const char *name)
> >  {
> >  	struct of_phandle_args	dma_spec;
> >  	struct of_dma		*ofdma;
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 91ac8da..68b4252 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -968,7 +968,8 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
> >  enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
> >  void dma_issue_pending_all(void);
> >  struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param);
> > -struct dma_chan *dma_request_slave_channel(struct device *dev, char *name);
> > +struct dma_chan *dma_request_slave_channel(struct device *dev,
> > +					   const char *name);
> >  void dma_release_channel(struct dma_chan *chan);
> >  #else
> >  static inline enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx)
> > @@ -984,7 +985,7 @@ static inline struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask,
> >  	return NULL;
> >  }
> >  static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
> > -							 char *name)
> > +							 const char *name)
> >  {
> >  	return NULL;
> >  }
> > @@ -1007,7 +1008,7 @@ struct dma_chan *net_dma_find_channel(void);
> >  static inline struct dma_chan
> >  *__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
> >  				  void *fn_param, struct device *dev,
> > -				  char *name)
> > +				  const char *name)
> >  {
> >  	struct dma_chan *chan;
> >  
> > diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> > index d15073e..0c4a82e 100644
> > --- a/include/linux/of_dma.h
> > +++ b/include/linux/of_dma.h
> > @@ -40,7 +40,7 @@ extern int of_dma_controller_register(struct device_node *np,
> >  		void *data);
> >  extern int of_dma_controller_free(struct device_node *np);
> >  extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> > -						     char *name);
> > +						     const char *name);
> >  extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
> >  		struct of_dma *ofdma);
> >  #else
> > -- 
> > 1.7.9.5
> > 
> > 
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/2] dmaengine: add const for name parameter

[Vinod Koul]

On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> Hi,
> 
> I already sent a nearly identical patch:
> https://patchwork.kernel.org/patch/2179321/
I applied that one a while ago so Markus can you check my tree and see if
anything is remaining, looks most of cases are covered

> 
> Regards,
> 
> Markus
> 
> On Fri, Mar 15, 2013 at 11:36:40AM +0800, Shawn Guo wrote:
> > Mark name parameter of generic device tree helpers as const, as it
> > shouldn't be changed anyway.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> >  drivers/dma/dmaengine.c   |    2 +-
> >  drivers/dma/of-dma.c      |    6 +++---
> >  include/linux/dmaengine.h |    7 ++++---
> >  include/linux/of_dma.h    |    2 +-
> >  4 files changed, 9 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > index b2728d6..2cbfefe 100644
> > --- a/drivers/dma/dmaengine.c
> > +++ b/drivers/dma/dmaengine.c
> > @@ -555,7 +555,7 @@ EXPORT_SYMBOL_GPL(__dma_request_channel);
> >   * @dev:	pointer to client device structure
> >   * @name:	slave channel name
> >   */
> > -struct dma_chan *dma_request_slave_channel(struct device *dev, char *name)
> > +struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name)
> >  {
> >  	/* If device-tree is present get slave info from here */
> >  	if (dev->of_node)
> > diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> > index 69d04d2..6036cd0 100644
> > --- a/drivers/dma/of-dma.c
> > +++ b/drivers/dma/of-dma.c
> > @@ -172,8 +172,8 @@ EXPORT_SYMBOL_GPL(of_dma_controller_free);
> >   * specifiers, matches the name provided. Returns 0 if the name matches and
> >   * a valid pointer to the DMA specifier is found. Otherwise returns -ENODEV.
> >   */
> > -static int of_dma_match_channel(struct device_node *np, char *name, int index,
> > -				struct of_phandle_args *dma_spec)
> > +static int of_dma_match_channel(struct device_node *np, const char *name,
> > +				int index, struct of_phandle_args *dma_spec)
> >  {
> >  	const char *s;
> >  
> > @@ -198,7 +198,7 @@ static int of_dma_match_channel(struct device_node *np, char *name, int index,
> >   * Returns pointer to appropriate dma channel on success or NULL on error.
> >   */
> >  struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> > -					      char *name)
> > +					      const char *name)
> >  {
> >  	struct of_phandle_args	dma_spec;
> >  	struct of_dma		*ofdma;
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 91ac8da..68b4252 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -968,7 +968,8 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
> >  enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
> >  void dma_issue_pending_all(void);
> >  struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param);
> > -struct dma_chan *dma_request_slave_channel(struct device *dev, char *name);
> > +struct dma_chan *dma_request_slave_channel(struct device *dev,
> > +					   const char *name);
> >  void dma_release_channel(struct dma_chan *chan);
> >  #else
> >  static inline enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx)
> > @@ -984,7 +985,7 @@ static inline struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask,
> >  	return NULL;
> >  }
> >  static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
> > -							 char *name)
> > +							 const char *name)
> >  {
> >  	return NULL;
> >  }
> > @@ -1007,7 +1008,7 @@ struct dma_chan *net_dma_find_channel(void);
> >  static inline struct dma_chan
> >  *__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
> >  				  void *fn_param, struct device *dev,
> > -				  char *name)
> > +				  const char *name)
> >  {
> >  	struct dma_chan *chan;
> >  
> > diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> > index d15073e..0c4a82e 100644
> > --- a/include/linux/of_dma.h
> > +++ b/include/linux/of_dma.h
> > @@ -40,7 +40,7 @@ extern int of_dma_controller_register(struct device_node *np,
> >  		void *data);
> >  extern int of_dma_controller_free(struct device_node *np);
> >  extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> > -						     char *name);
> > +						     const char *name);
> >  extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
> >  		struct of_dma *ofdma);
> >  #else
> > -- 
> > 1.7.9.5
> > 
> > 
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Vinod Koul]

On Fri, Mar 15, 2013 at 11:36:41AM +0800, Shawn Guo wrote:
> With generic DMA device tree binding and helper function
> dma_request_slave_channel() in place, dmaengine_pcm should support
> that in requesting DMA channel for users that support generic DMA
> device tree binding.
> 
> Add a new API snd_dmaengine_generic_pcm_open() as the variant of
> snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
> and slave channel name as arguments and calls generic DMA helper
> dma_request_slave_channel() to request DMA channel from dmaengine.
NAK

This is why we have dma_request_slave_channel_compat() API. You dont need to
write two open handlers here, just pass all the arguments and if DT is set it
will allocate a channel using dma_request_slave_channel() otherwise will do
dma_request_channel which is what you need.

> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: alsa-devel@alsa-project.org
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/sound/dmaengine_pcm.h |    2 ++
>  sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
> index b877334..452df15 100644
> --- a/include/sound/dmaengine_pcm.h
> +++ b/include/sound/dmaengine_pcm.h
> @@ -43,6 +43,8 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
>  
>  int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>  	dma_filter_fn filter_fn, void *filter_data);
> +int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
> +				   struct device *dev, const char *name);
>  int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
>  
>  struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> index 111b7d92..970eb2b 100644
> --- a/sound/soc/soc-dmaengine-pcm.c
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -304,6 +304,45 @@ int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>  EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open);
>  
>  /**
> + * snd_dmaengine_generic_pcm_open - Open a dmaengine based PCM substream
> + * @substream: PCM substream
> + * @dev: pointer to DMA client device structure
> + * @name: DMA slave channel name
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + *
> + * This function is a variant of snd_dmaengine_pcm_open(). It takes different
> + * parameters and calls generic DMA helper dma_request_slave_channel() to
> + * request a DMA channel from dmaengine.
> + */
> +int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
> +				   struct device *dev, const char *name)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd;
> +	int ret;
> +
> +	ret = snd_pcm_hw_constraint_integer(substream->runtime,
> +					    SNDRV_PCM_HW_PARAM_PERIODS);
> +	if (ret < 0)
> +		return ret;
> +
> +	prtd = kzalloc(sizeof(*prtd), GFP_KERNEL);
> +	if (!prtd)
> +		return -ENOMEM;
> +
> +	prtd->dma_chan = dma_request_slave_channel(dev, name);
> +	if (!prtd->dma_chan) {
> +		kfree(prtd);
> +		return -ENXIO;
> +	}
> +
> +	substream->runtime->private_data = prtd;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_generic_pcm_open);
> +
> +/**
>   * snd_dmaengine_pcm_close - Close a dmaengine based PCM substream
>   * @substream: PCM substream
>   */
> -- 
> 1.7.9.5
> 
> 

[PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Vinod Koul]

On Fri, Mar 15, 2013 at 11:36:41AM +0800, Shawn Guo wrote:
> With generic DMA device tree binding and helper function
> dma_request_slave_channel() in place, dmaengine_pcm should support
> that in requesting DMA channel for users that support generic DMA
> device tree binding.
> 
> Add a new API snd_dmaengine_generic_pcm_open() as the variant of
> snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
> and slave channel name as arguments and calls generic DMA helper
> dma_request_slave_channel() to request DMA channel from dmaengine.
NAK

This is why we have dma_request_slave_channel_compat() API. You dont need to
write two open handlers here, just pass all the arguments and if DT is set it
will allocate a channel using dma_request_slave_channel() otherwise will do
dma_request_channel which is what you need.

> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: alsa-devel at alsa-project.org
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/sound/dmaengine_pcm.h |    2 ++
>  sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
> index b877334..452df15 100644
> --- a/include/sound/dmaengine_pcm.h
> +++ b/include/sound/dmaengine_pcm.h
> @@ -43,6 +43,8 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
>  
>  int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>  	dma_filter_fn filter_fn, void *filter_data);
> +int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
> +				   struct device *dev, const char *name);
>  int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
>  
>  struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> index 111b7d92..970eb2b 100644
> --- a/sound/soc/soc-dmaengine-pcm.c
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -304,6 +304,45 @@ int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>  EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open);
>  
>  /**
> + * snd_dmaengine_generic_pcm_open - Open a dmaengine based PCM substream
> + * @substream: PCM substream
> + * @dev: pointer to DMA client device structure
> + * @name: DMA slave channel name
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + *
> + * This function is a variant of snd_dmaengine_pcm_open(). It takes different
> + * parameters and calls generic DMA helper dma_request_slave_channel() to
> + * request a DMA channel from dmaengine.
> + */
> +int snd_dmaengine_generic_pcm_open(struct snd_pcm_substream *substream,
> +				   struct device *dev, const char *name)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd;
> +	int ret;
> +
> +	ret = snd_pcm_hw_constraint_integer(substream->runtime,
> +					    SNDRV_PCM_HW_PARAM_PERIODS);
> +	if (ret < 0)
> +		return ret;
> +
> +	prtd = kzalloc(sizeof(*prtd), GFP_KERNEL);
> +	if (!prtd)
> +		return -ENOMEM;
> +
> +	prtd->dma_chan = dma_request_slave_channel(dev, name);
> +	if (!prtd->dma_chan) {
> +		kfree(prtd);
> +		return -ENXIO;
> +	}
> +
> +	substream->runtime->private_data = prtd;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_generic_pcm_open);
> +
> +/**
>   * snd_dmaengine_pcm_close - Close a dmaengine based PCM substream
>   * @substream: PCM substream
>   */
> -- 
> 1.7.9.5
> 
> 

Re: [PATCH 1/2] dmaengine: add const for name parameter

[Vinod Koul]

On Thu, Mar 21, 2013 at 02:10:08PM +0100, Markus Pargmann wrote:
> Hi,
> 
> On Thu, Mar 21, 2013 at 03:17:53PM +0530, Vinod Koul wrote:
> > On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> > > Hi,
> > > 
> > > I already sent a nearly identical patch:
> > > https://patchwork.kernel.org/patch/2179321/
> > I applied that one a while ago so Markus can you check my tree and see if
> > anything is remaining, looks most of cases are covered
> 
> I looked in your slave-dma tree but can't find the patch there. The
> names are not defined as constant in that tree.
Please check now, I had applied patches since morning but hadnt pushed out.
Pushed now...
> 
> Regards
> 
> Markus
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/2] dmaengine: add const for name parameter

[Vinod Koul]

On Thu, Mar 21, 2013 at 02:10:08PM +0100, Markus Pargmann wrote:
> Hi,
> 
> On Thu, Mar 21, 2013 at 03:17:53PM +0530, Vinod Koul wrote:
> > On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> > > Hi,
> > > 
> > > I already sent a nearly identical patch:
> > > https://patchwork.kernel.org/patch/2179321/
> > I applied that one a while ago so Markus can you check my tree and see if
> > anything is remaining, looks most of cases are covered
> 
> I looked in your slave-dma tree but can't find the patch there. The
> names are not defined as constant in that tree.
Please check now, I had applied patches since morning but hadnt pushed out.
Pushed now...
> 
> Regards
> 
> Markus
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/2] dmaengine: add const for name parameter

[Markus Pargmann]

Hi,

On Thu, Mar 21, 2013 at 03:17:53PM +0530, Vinod Koul wrote:
> On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> > Hi,
> > 
> > I already sent a nearly identical patch:
> > https://patchwork.kernel.org/patch/2179321/
> I applied that one a while ago so Markus can you check my tree and see if
> anything is remaining, looks most of cases are covered

I looked in your slave-dma tree but can't find the patch there. The
names are not defined as constant in that tree.

Regards

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/2] dmaengine: add const for name parameter

[Markus Pargmann]

Hi,

On Thu, Mar 21, 2013 at 03:17:53PM +0530, Vinod Koul wrote:
> On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> > Hi,
> > 
> > I already sent a nearly identical patch:
> > https://patchwork.kernel.org/patch/2179321/
> I applied that one a while ago so Markus can you check my tree and see if
> anything is remaining, looks most of cases are covered

I looked in your slave-dma tree but can't find the patch there. The
names are not defined as constant in that tree.

Regards

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH] DMA: of: const name fixup

[Markus Pargmann]

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---

Sorry, this one was actually missing in the first patch I sent.

Regards,

Markus

 include/linux/dmaengine.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 274071c..f30b096 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1007,7 +1007,7 @@ struct dma_chan *net_dma_find_channel(void);
 static inline struct dma_chan
 *__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
 				  void *fn_param, struct device *dev,
-				  char *name)
+				  const char *name)
 {
 	struct dma_chan *chan;
 
-- 
1.8.1.5

[PATCH] DMA: of: const name fixup

[Markus Pargmann]

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---

Sorry, this one was actually missing in the first patch I sent.

Regards,

Markus

 include/linux/dmaengine.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 274071c..f30b096 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1007,7 +1007,7 @@ struct dma_chan *net_dma_find_channel(void);
 static inline struct dma_chan
 *__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
 				  void *fn_param, struct device *dev,
-				  char *name)
+				  const char *name)
 {
 	struct dma_chan *chan;
 
-- 
1.8.1.5

Re: [PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Shawn Guo]

On Thu, Mar 21, 2013 at 03:27:18PM +0530, Vinod Koul wrote:
> On Fri, Mar 15, 2013 at 11:36:41AM +0800, Shawn Guo wrote:
> > With generic DMA device tree binding and helper function
> > dma_request_slave_channel() in place, dmaengine_pcm should support
> > that in requesting DMA channel for users that support generic DMA
> > device tree binding.
> > 
> > Add a new API snd_dmaengine_generic_pcm_open() as the variant of
> > snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
> > and slave channel name as arguments and calls generic DMA helper
> > dma_request_slave_channel() to request DMA channel from dmaengine.
> NAK
> 
> This is why we have dma_request_slave_channel_compat() API. You dont need to
> write two open handlers here, just pass all the arguments and if DT is set it
> will allocate a channel using dma_request_slave_channel() otherwise will do
> dma_request_channel which is what you need.

I do not quite follow your comment.  Let me try to understand it.  Are
you suggesting that instead of the current call path:

	snd_dmaengine_pcm_open()
		dmaengine_pcm_request_channel()
			dma_request_channel()

we should change it as below?

	snd_dmaengine_pcm_open()
		dmaengine_pcm_request_channel()
			dma_request_slave_channel_compat()

If that's the case, you are fundamentally requesting to change the
fingerprint of API snd_dmaengine_pcm_open() from the existing:

int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
			   dma_filter_fn filter_fn, void *filter_data)

to something like:

int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
			   dma_filter_fn filter_fn, void *filter_data,
			   struct device *dev, const char *name)

So every single user of snd_dmaengine_pcm_open() needs to adapt to the
interface change.

Is my understanding correct?  Or have I misunderstood your comment?

Shawn

[PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Shawn Guo]

On Thu, Mar 21, 2013 at 03:27:18PM +0530, Vinod Koul wrote:
> On Fri, Mar 15, 2013 at 11:36:41AM +0800, Shawn Guo wrote:
> > With generic DMA device tree binding and helper function
> > dma_request_slave_channel() in place, dmaengine_pcm should support
> > that in requesting DMA channel for users that support generic DMA
> > device tree binding.
> > 
> > Add a new API snd_dmaengine_generic_pcm_open() as the variant of
> > snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
> > and slave channel name as arguments and calls generic DMA helper
> > dma_request_slave_channel() to request DMA channel from dmaengine.
> NAK
> 
> This is why we have dma_request_slave_channel_compat() API. You dont need to
> write two open handlers here, just pass all the arguments and if DT is set it
> will allocate a channel using dma_request_slave_channel() otherwise will do
> dma_request_channel which is what you need.

I do not quite follow your comment.  Let me try to understand it.  Are
you suggesting that instead of the current call path:

	snd_dmaengine_pcm_open()
		dmaengine_pcm_request_channel()
			dma_request_channel()

we should change it as below?

	snd_dmaengine_pcm_open()
		dmaengine_pcm_request_channel()
			dma_request_slave_channel_compat()

If that's the case, you are fundamentally requesting to change the
fingerprint of API snd_dmaengine_pcm_open() from the existing:

int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
			   dma_filter_fn filter_fn, void *filter_data)

to something like:

int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
			   dma_filter_fn filter_fn, void *filter_data,
			   struct device *dev, const char *name)

So every single user of snd_dmaengine_pcm_open() needs to adapt to the
interface change.

Is my understanding correct?  Or have I misunderstood your comment?

Shawn

Re: [PATCH 1/2] dmaengine: add const for name parameter

[Shawn Guo]

Vinod,

On Thu, Mar 21, 2013 at 03:17:53PM +0530, Vinod Koul wrote:
> On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> > Hi,
> > 
> > I already sent a nearly identical patch:
> > https://patchwork.kernel.org/patch/2179321/
> I applied that one a while ago so Markus can you check my tree and see if
> anything is remaining, looks most of cases are covered
> 
Can you maintain that patch on a topic branch, so that we can ask Mark
to pull it as dependency for the soc-dmaengine-pcm patch that support
generic DMA binding users?

Shawn

[PATCH 1/2] dmaengine: add const for name parameter

[Shawn Guo]

Vinod,

On Thu, Mar 21, 2013 at 03:17:53PM +0530, Vinod Koul wrote:
> On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> > Hi,
> > 
> > I already sent a nearly identical patch:
> > https://patchwork.kernel.org/patch/2179321/
> I applied that one a while ago so Markus can you check my tree and see if
> anything is remaining, looks most of cases are covered
> 
Can you maintain that patch on a topic branch, so that we can ask Mark
to pull it as dependency for the soc-dmaengine-pcm patch that support
generic DMA binding users?

Shawn

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Lars-Peter Clausen]

On 03/21/2013 03:39 AM, Shawn Guo wrote:
> On Fri, Mar 15, 2013 at 11:36:39AM +0800, Shawn Guo wrote:
>> The series adds a snd_dmaengine_generic_pcm_open() for users that
>> adopt generic DMA binding and helplers.
>>
>> Vinod,
>>
> Ping?
> 

Hm, I only saw this series today would have been good to be on Cc. I've been
working on something very similar. My series goes a bit further though, it
implements an (almost generic) dmaengine based PCM driver using the of
bindings. So you need almost no platform code. The only things that are
platform specific at the moment is the pcm_hardware struct, but I'd like to
replace that in the future with something that queries the pcm hardware
parameter like max_period from the DMA engine driver. And another bit that is
still driver specific is a callback that fills the dma_slave_config struct.

In my series the channels are requested at probe time, so it is possible to
handle -EPROBE_DEFER properly and also we can allocate the audio buffers with
the dma device instead of the sound device, so stupid hacks like

    card->dev->dma_mask = &dma_mask;
    card->dev->coherent_dma_mask = DMA_BIT_MASK(32);

anymore. I'll try to post the series tomorrow.

- Lars


> Shawn
> 
>> AS per the request from Mark, I add the first patch to mark name
>> parameter of dmaengine helpers as const, so that clients can mark
>> const on their side.
>>
>> We need to have these two patches on a topic branch, as I've heard
>> a few people converting their ASoC driver needing the patches.  Can
>> you ack the first patch, so that Mark can maintain them in a branch
>> for anyone who needs it to pull?
>>
>> Shawn Guo (2):
>>   dmaengine: add const for name parameter
>>   ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()
>>
>>  drivers/dma/dmaengine.c       |    2 +-
>>  drivers/dma/of-dma.c          |    6 +++---
>>  include/linux/dmaengine.h     |    7 ++++---
>>  include/linux/of_dma.h        |    2 +-
>>  include/sound/dmaengine_pcm.h |    2 ++
>>  sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
>>  6 files changed, 50 insertions(+), 8 deletions(-)
>>
>> -- 
>> 1.7.9.5
>>
>>
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Lars-Peter Clausen]

On 03/21/2013 03:39 AM, Shawn Guo wrote:
> On Fri, Mar 15, 2013 at 11:36:39AM +0800, Shawn Guo wrote:
>> The series adds a snd_dmaengine_generic_pcm_open() for users that
>> adopt generic DMA binding and helplers.
>>
>> Vinod,
>>
> Ping?
> 

Hm, I only saw this series today would have been good to be on Cc. I've been
working on something very similar. My series goes a bit further though, it
implements an (almost generic) dmaengine based PCM driver using the of
bindings. So you need almost no platform code. The only things that are
platform specific at the moment is the pcm_hardware struct, but I'd like to
replace that in the future with something that queries the pcm hardware
parameter like max_period from the DMA engine driver. And another bit that is
still driver specific is a callback that fills the dma_slave_config struct.

In my series the channels are requested at probe time, so it is possible to
handle -EPROBE_DEFER properly and also we can allocate the audio buffers with
the dma device instead of the sound device, so stupid hacks like

    card->dev->dma_mask = &dma_mask;
    card->dev->coherent_dma_mask = DMA_BIT_MASK(32);

anymore. I'll try to post the series tomorrow.

- Lars


> Shawn
> 
>> AS per the request from Mark, I add the first patch to mark name
>> parameter of dmaengine helpers as const, so that clients can mark
>> const on their side.
>>
>> We need to have these two patches on a topic branch, as I've heard
>> a few people converting their ASoC driver needing the patches.  Can
>> you ack the first patch, so that Mark can maintain them in a branch
>> for anyone who needs it to pull?
>>
>> Shawn Guo (2):
>>   dmaengine: add const for name parameter
>>   ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()
>>
>>  drivers/dma/dmaengine.c       |    2 +-
>>  drivers/dma/of-dma.c          |    6 +++---
>>  include/linux/dmaengine.h     |    7 ++++---
>>  include/linux/of_dma.h        |    2 +-
>>  include/sound/dmaengine_pcm.h |    2 ++
>>  sound/soc/soc-dmaengine-pcm.c |   39 +++++++++++++++++++++++++++++++++++++++
>>  6 files changed, 50 insertions(+), 8 deletions(-)
>>
>> -- 
>> 1.7.9.5
>>
>>
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1275 bytes --]

On Thu, Mar 21, 2013 at 04:06:27PM +0100, Lars-Peter Clausen wrote:

> Hm, I only saw this series today would have been good to be on Cc. I've been
> working on something very similar. My series goes a bit further though, it
> implements an (almost generic) dmaengine based PCM driver using the of
> bindings. So you need almost no platform code. The only things that are
> platform specific at the moment is the pcm_hardware struct, but I'd like to
> replace that in the future with something that queries the pcm hardware
> parameter like max_period from the DMA engine driver. And another bit that is
> still driver specific is a callback that fills the dma_slave_config struct.

FWIW it might be worth looking at the one rmk wrote but has never wanted
to submit for whatever reason.

> In my series the channels are requested at probe time, so it is possible to
> handle -EPROBE_DEFER properly and also we can allocate the audio buffers with
> the dma device instead of the sound device, so stupid hacks like

>     card->dev->dma_mask = &dma_mask;
>     card->dev->coherent_dma_mask = DMA_BIT_MASK(32);

> anymore. I'll try to post the series tomorrow.

This is the main thing his code has got that the library hasn't, it's
certainly the only issue he keeps mentioning.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Mark Brown]

On Thu, Mar 21, 2013 at 04:06:27PM +0100, Lars-Peter Clausen wrote:

> Hm, I only saw this series today would have been good to be on Cc. I've been
> working on something very similar. My series goes a bit further though, it
> implements an (almost generic) dmaengine based PCM driver using the of
> bindings. So you need almost no platform code. The only things that are
> platform specific at the moment is the pcm_hardware struct, but I'd like to
> replace that in the future with something that queries the pcm hardware
> parameter like max_period from the DMA engine driver. And another bit that is
> still driver specific is a callback that fills the dma_slave_config struct.

FWIW it might be worth looking at the one rmk wrote but has never wanted
to submit for whatever reason.

> In my series the channels are requested at probe time, so it is possible to
> handle -EPROBE_DEFER properly and also we can allocate the audio buffers with
> the dma device instead of the sound device, so stupid hacks like

>     card->dev->dma_mask = &dma_mask;
>     card->dev->coherent_dma_mask = DMA_BIT_MASK(32);

> anymore. I'll try to post the series tomorrow.

This is the main thing his code has got that the library hasn't, it's
certainly the only issue he keeps mentioning.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130321/13b047e0/attachment.sig>

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Lars-Peter Clausen]

On 03/21/2013 04:22 PM, Mark Brown wrote:
> On Thu, Mar 21, 2013 at 04:06:27PM +0100, Lars-Peter Clausen wrote:
> 
>> Hm, I only saw this series today would have been good to be on Cc. I've been
>> working on something very similar. My series goes a bit further though, it
>> implements an (almost generic) dmaengine based PCM driver using the of
>> bindings. So you need almost no platform code. The only things that are
>> platform specific at the moment is the pcm_hardware struct, but I'd like to
>> replace that in the future with something that queries the pcm hardware
>> parameter like max_period from the DMA engine driver. And another bit that is
>> still driver specific is a callback that fills the dma_slave_config struct.
> 
> FWIW it might be worth looking at the one rmk wrote but has never wanted
> to submit for whatever reason.

Ideally I'd like to eventually move this 'fill in slave config' callback to the
DAI driver, since this is almost always DAI specific data, like for example the
FIFO address, the burst length, the bus width, etc. But I'd like to do that in
a second step. The first step should already help us to get rid of a large
portion of redundant code we see in PCM drivers today.

> 
>> In my series the channels are requested at probe time, so it is possible to
>> handle -EPROBE_DEFER properly and also we can allocate the audio buffers with
>> the dma device instead of the sound device, so stupid hacks like
> 
>>     card->dev->dma_mask = &dma_mask;
>>     card->dev->coherent_dma_mask = DMA_BIT_MASK(32);
> 
>> anymore. I'll try to post the series tomorrow.
> 
> This is the main thing his code has got that the library hasn't, it's
> certainly the only issue he keeps mentioning.

Yes, he definitely deserves credit for this, I probably wouldn't have
implemented it, if he hadn't point this out.

The problem with the current library is that we don't know yet which dmaengine
device is going to be used by the time we pre-allocate the audio buffers, since
the DMA filter parameters often are provided by the DAI driver. When using
devicetree on the other hand the handle to the dmaengine comes from the
devicetree itself and so it is available at the time we probe the PCM driver.

There are some other drivers which don't really depend on runtime filter
parameters, e.g. tegra, which doesn't use a filter function. So tegra can also
easily make use of this new generic dmaengine based PCM driver.

- Lars

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Lars-Peter Clausen]

On 03/21/2013 04:22 PM, Mark Brown wrote:
> On Thu, Mar 21, 2013 at 04:06:27PM +0100, Lars-Peter Clausen wrote:
> 
>> Hm, I only saw this series today would have been good to be on Cc. I've been
>> working on something very similar. My series goes a bit further though, it
>> implements an (almost generic) dmaengine based PCM driver using the of
>> bindings. So you need almost no platform code. The only things that are
>> platform specific at the moment is the pcm_hardware struct, but I'd like to
>> replace that in the future with something that queries the pcm hardware
>> parameter like max_period from the DMA engine driver. And another bit that is
>> still driver specific is a callback that fills the dma_slave_config struct.
> 
> FWIW it might be worth looking at the one rmk wrote but has never wanted
> to submit for whatever reason.

Ideally I'd like to eventually move this 'fill in slave config' callback to the
DAI driver, since this is almost always DAI specific data, like for example the
FIFO address, the burst length, the bus width, etc. But I'd like to do that in
a second step. The first step should already help us to get rid of a large
portion of redundant code we see in PCM drivers today.

> 
>> In my series the channels are requested at probe time, so it is possible to
>> handle -EPROBE_DEFER properly and also we can allocate the audio buffers with
>> the dma device instead of the sound device, so stupid hacks like
> 
>>     card->dev->dma_mask = &dma_mask;
>>     card->dev->coherent_dma_mask = DMA_BIT_MASK(32);
> 
>> anymore. I'll try to post the series tomorrow.
> 
> This is the main thing his code has got that the library hasn't, it's
> certainly the only issue he keeps mentioning.

Yes, he definitely deserves credit for this, I probably wouldn't have
implemented it, if he hadn't point this out.

The problem with the current library is that we don't know yet which dmaengine
device is going to be used by the time we pre-allocate the audio buffers, since
the DMA filter parameters often are provided by the DAI driver. When using
devicetree on the other hand the handle to the dmaengine comes from the
devicetree itself and so it is available at the time we probe the PCM driver.

There are some other drivers which don't really depend on runtime filter
parameters, e.g. tegra, which doesn't use a filter function. So tegra can also
easily make use of this new generic dmaengine based PCM driver.

- Lars

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 579 bytes --]

On Thu, Mar 21, 2013 at 05:26:19PM +0100, Lars-Peter Clausen wrote:

> a second step. The first step should already help us to get rid of a large
> portion of redundant code we see in PCM drivers today.

Indeed, factor everything we can out into common code and then improve
the common code in a single place.

> There are some other drivers which don't really depend on runtime filter
> parameters, e.g. tegra, which doesn't use a filter function. So tegra can also
> easily make use of this new generic dmaengine based PCM driver.

Tegra's DT only these days anyway of course.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Mark Brown]

On Thu, Mar 21, 2013 at 05:26:19PM +0100, Lars-Peter Clausen wrote:

> a second step. The first step should already help us to get rid of a large
> portion of redundant code we see in PCM drivers today.

Indeed, factor everything we can out into common code and then improve
the common code in a single place.

> There are some other drivers which don't really depend on runtime filter
> parameters, e.g. tegra, which doesn't use a filter function. So tegra can also
> easily make use of this new generic dmaengine based PCM driver.

Tegra's DT only these days anyway of course.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130321/2d359b73/attachment.sig>

Re: [PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Sebastien Guiriec]

On 03/21/2013 03:53 PM, Shawn Guo wrote:
> On Thu, Mar 21, 2013 at 03:27:18PM +0530, Vinod Koul wrote:
>> On Fri, Mar 15, 2013 at 11:36:41AM +0800, Shawn Guo wrote:
>>> With generic DMA device tree binding and helper function
>>> dma_request_slave_channel() in place, dmaengine_pcm should support
>>> that in requesting DMA channel for users that support generic DMA
>>> device tree binding.
>>>
>>> Add a new API snd_dmaengine_generic_pcm_open() as the variant of
>>> snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
>>> and slave channel name as arguments and calls generic DMA helper
>>> dma_request_slave_channel() to request DMA channel from dmaengine.
>> NAK
>>
>> This is why we have dma_request_slave_channel_compat() API. You dont need to
>> write two open handlers here, just pass all the arguments and if DT is set it
>> will allocate a channel using dma_request_slave_channel() otherwise will do
>> dma_request_channel which is what you need.
>
> I do not quite follow your comment.  Let me try to understand it.  Are
> you suggesting that instead of the current call path:
>
> 	snd_dmaengine_pcm_open()
> 		dmaengine_pcm_request_channel()
> 			dma_request_channel()
>
> we should change it as below?
>
> 	snd_dmaengine_pcm_open()
> 		dmaengine_pcm_request_channel()
> 			dma_request_slave_channel_compat()
>
> If that's the case, you are fundamentally requesting to change the
> fingerprint of API snd_dmaengine_pcm_open() from the existing:
>
> int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
> 			   dma_filter_fn filter_fn, void *filter_data)
>
> to something like:
>
> int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
> 			   dma_filter_fn filter_fn, void *filter_data,
> 			   struct device *dev, const char *name)
>
> So every single user of snd_dmaengine_pcm_open() needs to adapt to the
> interface change.
>
> Is my understanding correct?  Or have I misunderstood your comment?

This is what has been done on OMAP for other IPs. I have tested this 
solution for Audio before you submit your first series and it is working.

Now I will wait Lars serie to check again on OMAP and let Mark comment 
on the best approach. Whereas update current API like you propose or 
wait Lars series and move on according to the different comments done on 
open function. As OMAP audio DMA binding are depending on the chosen 
solution.

Sebastien.
>
> Shawn
>
>

[PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Sebastien Guiriec]

On 03/21/2013 03:53 PM, Shawn Guo wrote:
> On Thu, Mar 21, 2013 at 03:27:18PM +0530, Vinod Koul wrote:
>> On Fri, Mar 15, 2013 at 11:36:41AM +0800, Shawn Guo wrote:
>>> With generic DMA device tree binding and helper function
>>> dma_request_slave_channel() in place, dmaengine_pcm should support
>>> that in requesting DMA channel for users that support generic DMA
>>> device tree binding.
>>>
>>> Add a new API snd_dmaengine_generic_pcm_open() as the variant of
>>> snd_dmaengine_pcm_open().  It takes DMA client struct device pointer
>>> and slave channel name as arguments and calls generic DMA helper
>>> dma_request_slave_channel() to request DMA channel from dmaengine.
>> NAK
>>
>> This is why we have dma_request_slave_channel_compat() API. You dont need to
>> write two open handlers here, just pass all the arguments and if DT is set it
>> will allocate a channel using dma_request_slave_channel() otherwise will do
>> dma_request_channel which is what you need.
>
> I do not quite follow your comment.  Let me try to understand it.  Are
> you suggesting that instead of the current call path:
>
> 	snd_dmaengine_pcm_open()
> 		dmaengine_pcm_request_channel()
> 			dma_request_channel()
>
> we should change it as below?
>
> 	snd_dmaengine_pcm_open()
> 		dmaengine_pcm_request_channel()
> 			dma_request_slave_channel_compat()
>
> If that's the case, you are fundamentally requesting to change the
> fingerprint of API snd_dmaengine_pcm_open() from the existing:
>
> int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
> 			   dma_filter_fn filter_fn, void *filter_data)
>
> to something like:
>
> int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
> 			   dma_filter_fn filter_fn, void *filter_data,
> 			   struct device *dev, const char *name)
>
> So every single user of snd_dmaengine_pcm_open() needs to adapt to the
> interface change.
>
> Is my understanding correct?  Or have I misunderstood your comment?

This is what has been done on OMAP for other IPs. I have tested this 
solution for Audio before you submit your first series and it is working.

Now I will wait Lars serie to check again on OMAP and let Mark comment 
on the best approach. Whereas update current API like you propose or 
wait Lars series and move on according to the different comments done on 
open function. As OMAP audio DMA binding are depending on the chosen 
solution.

Sebastien.
>
> Shawn
>
>

Re: [PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Shawn Guo]

On Fri, Mar 22, 2013 at 09:07:21AM +0100, Sebastien Guiriec wrote:
> Now I will wait Lars serie to check again on OMAP and let Mark
> comment on the best approach. Whereas update current API like you
> propose or wait Lars series and move on according to the different
> comments done on open function. As OMAP audio DMA binding are
> depending on the chosen solution.

This the case for everyone else who is moving their audio driver to
generic DMA bindings.  For mxs platform that I'm working on, audio
becomes the only one not moving to generic DMA bindings.

Shawn

[PATCH 2/2] ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open()

[Shawn Guo]

On Fri, Mar 22, 2013 at 09:07:21AM +0100, Sebastien Guiriec wrote:
> Now I will wait Lars serie to check again on OMAP and let Mark
> comment on the best approach. Whereas update current API like you
> propose or wait Lars series and move on according to the different
> comments done on open function. As OMAP audio DMA binding are
> depending on the chosen solution.

This the case for everyone else who is moving their audio driver to
generic DMA bindings.  For mxs platform that I'm working on, audio
becomes the only one not moving to generic DMA bindings.

Shawn

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Russell King - ARM Linux]

On Thu, Mar 21, 2013 at 04:22:06PM +0100, Mark Brown wrote:
> On Thu, Mar 21, 2013 at 04:06:27PM +0100, Lars-Peter Clausen wrote:
> 
> > Hm, I only saw this series today would have been good to be on Cc. I've been
> > working on something very similar. My series goes a bit further though, it
> > implements an (almost generic) dmaengine based PCM driver using the of
> > bindings. So you need almost no platform code. The only things that are
> > platform specific at the moment is the pcm_hardware struct, but I'd like to
> > replace that in the future with something that queries the pcm hardware
> > parameter like max_period from the DMA engine driver. And another bit that is
> > still driver specific is a callback that fills the dma_slave_config struct.
> 
> FWIW it might be worth looking at the one rmk wrote but has never wanted
> to submit for whatever reason.

Err no, stop twisting the facts.  I know nothing is ever your fault.  You
rejected it because it was providing support for non-cyclic supporting
DMA engine drivers.

I've since added support to it for cyclic DMA engines, but I've retained
the non-cyclic support in it because I don't see why I should remove it
when it works for me, especially given the difficulties with getting
anything in sound/soc changed once its been merged.

Plus, as I've already said to you, I no longer develop and test it because
the platform I was using is now doing service as my firewall, and you'll
forgive me for not wanting to take the whole of *.arm.linux.org.uk
offline to mess around with ASoC stuff.  But that's not to say I don't
care about the issue.

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Russell King - ARM Linux]

On Thu, Mar 21, 2013 at 04:22:06PM +0100, Mark Brown wrote:
> On Thu, Mar 21, 2013 at 04:06:27PM +0100, Lars-Peter Clausen wrote:
> 
> > Hm, I only saw this series today would have been good to be on Cc. I've been
> > working on something very similar. My series goes a bit further though, it
> > implements an (almost generic) dmaengine based PCM driver using the of
> > bindings. So you need almost no platform code. The only things that are
> > platform specific at the moment is the pcm_hardware struct, but I'd like to
> > replace that in the future with something that queries the pcm hardware
> > parameter like max_period from the DMA engine driver. And another bit that is
> > still driver specific is a callback that fills the dma_slave_config struct.
> 
> FWIW it might be worth looking at the one rmk wrote but has never wanted
> to submit for whatever reason.

Err no, stop twisting the facts.  I know nothing is ever your fault.  You
rejected it because it was providing support for non-cyclic supporting
DMA engine drivers.

I've since added support to it for cyclic DMA engines, but I've retained
the non-cyclic support in it because I don't see why I should remove it
when it works for me, especially given the difficulties with getting
anything in sound/soc changed once its been merged.

Plus, as I've already said to you, I no longer develop and test it because
the platform I was using is now doing service as my firewall, and you'll
forgive me for not wanting to take the whole of *.arm.linux.org.uk
offline to mess around with ASoC stuff.  But that's not to say I don't
care about the issue.

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1815 bytes --]

On Fri, Mar 22, 2013 at 11:17:04AM +0000, Russell King - ARM Linux wrote:
> On Thu, Mar 21, 2013 at 04:22:06PM +0100, Mark Brown wrote:

> > FWIW it might be worth looking at the one rmk wrote but has never wanted
> > to submit for whatever reason.

> Err no, stop twisting the facts.  I know nothing is ever your fault.  You
> rejected it because it was providing support for non-cyclic supporting
> DMA engine drivers.

That might well have been one of the reasons now you mention it, I was
just remembering that the code has never been posted to the list, I've
only ever viewed it in your git trees.  IIRC it was only mentioned on
the list after the currently merged library was posted and you weren't
keen on submitting at that point due as you weren't happy with the
platform code using due to that code having to jump through hoops to
support some unusual hardware design decisions.

> I've since added support to it for cyclic DMA engines, but I've retained
> the non-cyclic support in it because I don't see why I should remove it
> when it works for me, especially given the difficulties with getting

We need support for both cyclic and non-cyclic devices, one doesn't
preclude the other and both kinds of hardware exist.  Cyclic is much
more common and generally more desirable but that doesn't mean that
hardware designers always provide it.

> anything in sound/soc changed once its been merged.

I'm sorry, what difficulties are those?

> Plus, as I've already said to you, I no longer develop and test it because
> the platform I was using is now doing service as my firewall, and you'll
> forgive me for not wanting to take the whole of *.arm.linux.org.uk
> offline to mess around with ASoC stuff.  But that's not to say I don't
> care about the issue.

Indeed, I understand why you've stopped now.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Mark Brown]

On Fri, Mar 22, 2013 at 11:17:04AM +0000, Russell King - ARM Linux wrote:
> On Thu, Mar 21, 2013 at 04:22:06PM +0100, Mark Brown wrote:

> > FWIW it might be worth looking at the one rmk wrote but has never wanted
> > to submit for whatever reason.

> Err no, stop twisting the facts.  I know nothing is ever your fault.  You
> rejected it because it was providing support for non-cyclic supporting
> DMA engine drivers.

That might well have been one of the reasons now you mention it, I was
just remembering that the code has never been posted to the list, I've
only ever viewed it in your git trees.  IIRC it was only mentioned on
the list after the currently merged library was posted and you weren't
keen on submitting at that point due as you weren't happy with the
platform code using due to that code having to jump through hoops to
support some unusual hardware design decisions.

> I've since added support to it for cyclic DMA engines, but I've retained
> the non-cyclic support in it because I don't see why I should remove it
> when it works for me, especially given the difficulties with getting

We need support for both cyclic and non-cyclic devices, one doesn't
preclude the other and both kinds of hardware exist.  Cyclic is much
more common and generally more desirable but that doesn't mean that
hardware designers always provide it.

> anything in sound/soc changed once its been merged.

I'm sorry, what difficulties are those?

> Plus, as I've already said to you, I no longer develop and test it because
> the platform I was using is now doing service as my firewall, and you'll
> forgive me for not wanting to take the whole of *.arm.linux.org.uk
> offline to mess around with ASoC stuff.  But that's not to say I don't
> care about the issue.

Indeed, I understand why you've stopped now.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130322/9919c1d8/attachment.sig>

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Russell King - ARM Linux]

On Thu, Mar 21, 2013 at 05:26:19PM +0100, Lars-Peter Clausen wrote:
> The problem with the current library is that we don't know yet which
> dmaengine device is going to be used by the time we pre-allocate the
> audio buffers, since the DMA filter parameters often are provided by
> the DAI driver.

That's not really a problem.  The CPU DAI can provide the data via
the playback_dma_data and capture_dma_data pointers, which the ASoC
DMA engine driver can pick up on when it initializes.  This data can
contain everything that the ASoC DMA engine driver needs.

Where my driver falls down is if you need to reconfigure the DMA engine
later - it doesn't provide an API for that, but that's trivially easy
to augment the API.

If you want to look at what I did for sa11x0 Assabet platforms, it's
still all here:

http://ftp.arm.linux.org.uk/git/gitweb.cgi?p=linux-arm.git;a=shortlog;h=2f03d47

As you will see from the dates, it's over a year old now - the only
thing I do with it is occasionally rebase it to bring it up to a more
modern kernel.

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Russell King - ARM Linux]

On Thu, Mar 21, 2013 at 05:26:19PM +0100, Lars-Peter Clausen wrote:
> The problem with the current library is that we don't know yet which
> dmaengine device is going to be used by the time we pre-allocate the
> audio buffers, since the DMA filter parameters often are provided by
> the DAI driver.

That's not really a problem.  The CPU DAI can provide the data via
the playback_dma_data and capture_dma_data pointers, which the ASoC
DMA engine driver can pick up on when it initializes.  This data can
contain everything that the ASoC DMA engine driver needs.

Where my driver falls down is if you need to reconfigure the DMA engine
later - it doesn't provide an API for that, but that's trivially easy
to augment the API.

If you want to look at what I did for sa11x0 Assabet platforms, it's
still all here:

http://ftp.arm.linux.org.uk/git/gitweb.cgi?p=linux-arm.git;a=shortlog;h=2f03d47

As you will see from the dates, it's over a year old now - the only
thing I do with it is occasionally rebase it to bring it up to a more
modern kernel.

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Lars-Peter Clausen]

On 03/22/2013 12:30 PM, Russell King - ARM Linux wrote:
> On Thu, Mar 21, 2013 at 05:26:19PM +0100, Lars-Peter Clausen wrote:
>> The problem with the current library is that we don't know yet which
>> dmaengine device is going to be used by the time we pre-allocate the
>> audio buffers, since the DMA filter parameters often are provided by
>> the DAI driver.
> 
> That's not really a problem.  The CPU DAI can provide the data via
> the playback_dma_data and capture_dma_data pointers, which the ASoC
> DMA engine driver can pick up on when it initializes.  This data can
> contain everything that the ASoC DMA engine driver needs.

Yes and no. Unfortunately it doesn't work for all drivers. But yes that's
the way I want to move forward for the non dt case. Let the channels be
requested in the pcm_new callback.

> 
> Where my driver falls down is if you need to reconfigure the DMA engine
> later - it doesn't provide an API for that, but that's trivially easy
> to augment the API.
> 
> If you want to look at what I did for sa11x0 Assabet platforms, it's
> still all here:
> 
> http://ftp.arm.linux.org.uk/git/gitweb.cgi?p=linux-arm.git;a=shortlog;h=2f03d47
> 
> As you will see from the dates, it's over a year old now - the only
> thing I do with it is occasionally rebase it to bring it up to a more
> modern kernel.

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Lars-Peter Clausen]

On 03/22/2013 12:30 PM, Russell King - ARM Linux wrote:
> On Thu, Mar 21, 2013 at 05:26:19PM +0100, Lars-Peter Clausen wrote:
>> The problem with the current library is that we don't know yet which
>> dmaengine device is going to be used by the time we pre-allocate the
>> audio buffers, since the DMA filter parameters often are provided by
>> the DAI driver.
> 
> That's not really a problem.  The CPU DAI can provide the data via
> the playback_dma_data and capture_dma_data pointers, which the ASoC
> DMA engine driver can pick up on when it initializes.  This data can
> contain everything that the ASoC DMA engine driver needs.

Yes and no. Unfortunately it doesn't work for all drivers. But yes that's
the way I want to move forward for the non dt case. Let the channels be
requested in the pcm_new callback.

> 
> Where my driver falls down is if you need to reconfigure the DMA engine
> later - it doesn't provide an API for that, but that's trivially easy
> to augment the API.
> 
> If you want to look at what I did for sa11x0 Assabet platforms, it's
> still all here:
> 
> http://ftp.arm.linux.org.uk/git/gitweb.cgi?p=linux-arm.git;a=shortlog;h=2f03d47
> 
> As you will see from the dates, it's over a year old now - the only
> thing I do with it is occasionally rebase it to bring it up to a more
> modern kernel.

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Lars-Peter Clausen]

On 03/22/2013 12:28 PM, Mark Brown wrote:
> On Fri, Mar 22, 2013 at 11:17:04AM +0000, Russell King - ARM Linux wrote:
>> On Thu, Mar 21, 2013 at 04:22:06PM +0100, Mark Brown wrote:
> 
>>> FWIW it might be worth looking at the one rmk wrote but has never wanted
>>> to submit for whatever reason.
> 
>> Err no, stop twisting the facts.  I know nothing is ever your fault.  You
>> rejected it because it was providing support for non-cyclic supporting
>> DMA engine drivers.
> 
> That might well have been one of the reasons now you mention it, I was
> just remembering that the code has never been posted to the list, I've
> only ever viewed it in your git trees.  IIRC it was only mentioned on
> the list after the currently merged library was posted and you weren't
> keen on submitting at that point due as you weren't happy with the
> platform code using due to that code having to jump through hoops to
> support some unusual hardware design decisions.
> 
>> I've since added support to it for cyclic DMA engines, but I've retained
>> the non-cyclic support in it because I don't see why I should remove it
>> when it works for me, especially given the difficulties with getting
> 
> We need support for both cyclic and non-cyclic devices, one doesn't
> preclude the other and both kinds of hardware exist.  Cyclic is much
> more common and generally more desirable but that doesn't mean that
> hardware designers always provide it.


The idea was to only have support for the cyclic dmaengine API in the ASoC
PCM driver and deal with the emulation of cyclic transfers for hardware,
which does not implement it, at the dmaengine layer.

> 
>> anything in sound/soc changed once its been merged.
> 
> I'm sorry, what difficulties are those?
> 
>> Plus, as I've already said to you, I no longer develop and test it because
>> the platform I was using is now doing service as my firewall, and you'll
>> forgive me for not wanting to take the whole of *.arm.linux.org.uk
>> offline to mess around with ASoC stuff.  But that's not to say I don't
>> care about the issue.
> 
> Indeed, I understand why you've stopped now.
> 
> 
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Lars-Peter Clausen]

On 03/22/2013 12:28 PM, Mark Brown wrote:
> On Fri, Mar 22, 2013 at 11:17:04AM +0000, Russell King - ARM Linux wrote:
>> On Thu, Mar 21, 2013 at 04:22:06PM +0100, Mark Brown wrote:
> 
>>> FWIW it might be worth looking at the one rmk wrote but has never wanted
>>> to submit for whatever reason.
> 
>> Err no, stop twisting the facts.  I know nothing is ever your fault.  You
>> rejected it because it was providing support for non-cyclic supporting
>> DMA engine drivers.
> 
> That might well have been one of the reasons now you mention it, I was
> just remembering that the code has never been posted to the list, I've
> only ever viewed it in your git trees.  IIRC it was only mentioned on
> the list after the currently merged library was posted and you weren't
> keen on submitting at that point due as you weren't happy with the
> platform code using due to that code having to jump through hoops to
> support some unusual hardware design decisions.
> 
>> I've since added support to it for cyclic DMA engines, but I've retained
>> the non-cyclic support in it because I don't see why I should remove it
>> when it works for me, especially given the difficulties with getting
> 
> We need support for both cyclic and non-cyclic devices, one doesn't
> preclude the other and both kinds of hardware exist.  Cyclic is much
> more common and generally more desirable but that doesn't mean that
> hardware designers always provide it.


The idea was to only have support for the cyclic dmaengine API in the ASoC
PCM driver and deal with the emulation of cyclic transfers for hardware,
which does not implement it, at the dmaengine layer.

> 
>> anything in sound/soc changed once its been merged.
> 
> I'm sorry, what difficulties are those?
> 
>> Plus, as I've already said to you, I no longer develop and test it because
>> the platform I was using is now doing service as my firewall, and you'll
>> forgive me for not wanting to take the whole of *.arm.linux.org.uk
>> offline to mess around with ASoC stuff.  But that's not to say I don't
>> care about the issue.
> 
> Indeed, I understand why you've stopped now.
> 
> 
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 821 bytes --]

On Fri, Mar 22, 2013 at 12:42:44PM +0100, Lars-Peter Clausen wrote:
> On 03/22/2013 12:28 PM, Mark Brown wrote:

> > We need support for both cyclic and non-cyclic devices, one doesn't
> > preclude the other and both kinds of hardware exist.  Cyclic is much
> > more common and generally more desirable but that doesn't mean that
> > hardware designers always provide it.

> The idea was to only have support for the cyclic dmaengine API in the ASoC
> PCM driver and deal with the emulation of cyclic transfers for hardware,
> which does not implement it, at the dmaengine layer.

Perhaps I'm misremembering but I thought there was pushback on that on
the basis that there are few if any other users of cyclic DMA so nobody
else cares or wants to deal with the hassle?  A quick grep seems not to
turn up any other users.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[alsa-devel] [PATCH 0/2] ASoC: dmaengine_pcm: support generic DMA binding users

[Mark Brown]

On Fri, Mar 22, 2013 at 12:42:44PM +0100, Lars-Peter Clausen wrote:
> On 03/22/2013 12:28 PM, Mark Brown wrote:

> > We need support for both cyclic and non-cyclic devices, one doesn't
> > preclude the other and both kinds of hardware exist.  Cyclic is much
> > more common and generally more desirable but that doesn't mean that
> > hardware designers always provide it.

> The idea was to only have support for the cyclic dmaengine API in the ASoC
> PCM driver and deal with the emulation of cyclic transfers for hardware,
> which does not implement it, at the dmaengine layer.

Perhaps I'm misremembering but I thought there was pushback on that on
the basis that there are few if any other users of cyclic DMA so nobody
else cares or wants to deal with the hassle?  A quick grep seems not to
turn up any other users.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130322/e46380c3/attachment.sig>

Re: [PATCH 1/2] dmaengine: add const for name parameter

[Vinod Koul]

On Thu, Mar 21, 2013 at 10:57:30PM +0800, Shawn Guo wrote:
> Vinod,
> 
> On Thu, Mar 21, 2013 at 03:17:53PM +0530, Vinod Koul wrote:
> > On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> > > Hi,
> > > 
> > > I already sent a nearly identical patch:
> > > https://patchwork.kernel.org/patch/2179321/
> > I applied that one a while ago so Markus can you check my tree and see if
> > anything is remaining, looks most of cases are covered
> > 
> Can you maintain that patch on a topic branch, so that we can ask Mark
> to pull it as dependency for the soc-dmaengine-pcm patch that support
> generic DMA binding users?
Looks like i never replied to this one, althoug I have pushed a branch

Mark,

You can pull commit a0c43a3 from topic/of or the entire branch into your tree

--
Thanks
~Vinod

[PATCH 1/2] dmaengine: add const for name parameter

[Vinod Koul]

On Thu, Mar 21, 2013 at 10:57:30PM +0800, Shawn Guo wrote:
> Vinod,
> 
> On Thu, Mar 21, 2013 at 03:17:53PM +0530, Vinod Koul wrote:
> > On Fri, Mar 15, 2013 at 09:53:46AM +0100, Markus Pargmann wrote:
> > > Hi,
> > > 
> > > I already sent a nearly identical patch:
> > > https://patchwork.kernel.org/patch/2179321/
> > I applied that one a while ago so Markus can you check my tree and see if
> > anything is remaining, looks most of cases are covered
> > 
> Can you maintain that patch on a topic branch, so that we can ask Mark
> to pull it as dependency for the soc-dmaengine-pcm patch that support
> generic DMA binding users?
Looks like i never replied to this one, althoug I have pushed a branch

Mark,

You can pull commit a0c43a3 from topic/of or the entire branch into your tree

--
Thanks
~Vinod

Re: [PATCH 1/2] dmaengine: add const for name parameter

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 296 bytes --]

On Tue, Apr 02, 2013 at 11:21:28PM +0530, Vinod Koul wrote:

> You can pull commit a0c43a3 from topic/of or the entire branch into your tree

It's just a warning fix so I'm not too concerned about it - so long as
everything is resolved by the time the merge window is over that ought
to be fine.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 1/2] dmaengine: add const for name parameter

[Mark Brown]

On Tue, Apr 02, 2013 at 11:21:28PM +0530, Vinod Koul wrote:

> You can pull commit a0c43a3 from topic/of or the entire branch into your tree

It's just a warning fix so I'm not too concerned about it - so long as
everything is resolved by the time the merge window is over that ought
to be fine.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130402/dd0f7d35/attachment.sig>