[alsa-devel] [PATCH v2 00/12] ARM: mxs: move to generic DMA device tree binding
The series converts mxs-dma and its clients to generic DMA device tree binding/helper.
I published a branch below to ease people who is willing to help testing the series.
git://git.linaro.org/people/shawnguo/linux-2.6.git devel/mxs-dma-dt
Changes since v1: - Remove the use of of_dma_filter_info which should not be needed by a customized xlate function - Correct dma-names to have it mean the channel function in client devices - Add a new dmaengine_pcm API snd_dmaengine_generic_pcm_open() for users that support generic DMA device tree binding - Remove the old fsl,<module>-dma-channel properties from device tree
Subsystem maintainers,
I need your ACKs to have the series go via arm-soc as a whole if the patches look good to you.
Thanks, Shawn
Shawn Guo (12): ARM: dts: add generic DMA device tree binding for mxs-dma dma: mxs-dma: use devm_* managed functions dma: mxs-dma: move to generic device tree binding mmc: mxs-mmc: move to use generic DMA helper spi: mxs-spi: move to use generic DMA helper i2c: i2c-mxs: move to use generic DMA helper mtd: gpmi: move to use generic DMA helper serial: mxs-auart: move to use generic DMA helper ASoC: dmaengine_pcm: add snd_dmaengine_generic_pcm_open() ASoC: mxs: move to use generic DMA helper dma: mxs-dma: remove code left from generic DMA binding conversion ARM: dts: remove old DMA binding data from client nodes
.../devicetree/bindings/dma/fsl-mxs-dma.txt | 49 +++++++- Documentation/devicetree/bindings/i2c/i2c-mxs.txt | 12 +- Documentation/devicetree/bindings/mmc/mxs-mmc.txt | 12 +- .../devicetree/bindings/mtd/gpmi-nand.txt | 17 +-- .../devicetree/bindings/sound/mxs-saif.txt | 18 +-- Documentation/devicetree/bindings/spi/mxs-spi.txt | 12 +- .../bindings/tty/serial/fsl-mxs-auart.txt | 16 ++- arch/arm/boot/dts/imx23.dtsi | 57 +++++++-- arch/arm/boot/dts/imx28.dtsi | 104 +++++++++++----- arch/arm/boot/dts/imx6qdl.dtsi | 12 +- drivers/dma/mxs-dma.c | 128 +++++++++++--------- drivers/i2c/busses/i2c-mxs.c | 40 +----- drivers/mmc/host/mxs-mmc.c | 48 +------- drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 51 +------- drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 3 +- drivers/spi/spi-mxs.c | 60 ++------- drivers/tty/serial/mxs-auart.c | 52 +------- include/linux/fsl/mxs-dma.h | 20 --- include/linux/spi/mxs-spi.h | 4 +- include/sound/dmaengine_pcm.h | 2 + sound/soc/mxs/mxs-pcm.c | 42 +------ sound/soc/mxs/mxs-pcm.h | 5 - sound/soc/mxs/mxs-saif.c | 28 +---- sound/soc/mxs/mxs-saif.h | 1 - sound/soc/soc-dmaengine-pcm.c | 39 ++++++ 25 files changed, 368 insertions(+), 464 deletions(-) delete mode 100644 include/linux/fsl/mxs-dma.h
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 --- Mark,
Please maintain a topic branch for the patch if it looks good to you. It should be useful for any dmaengine_pcm user that is moving to generic DMA device tree binding.
Thanks, Shawn
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 */
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 --- Changes since v2: - Drop 'const' from argument 'name' to fix the warning below.
sound/soc/soc-dmaengine-pcm.c: In function 'snd_dmaengine_generic_pcm_open': sound/soc/soc-dmaengine-pcm.c:333:2: warning: passing argument 2 of 'dma_request_slave_channel' discards 'const' qualifier from pointer target type [enabled by default] include/linux/dmaengine.h:971:18: note: expected 'char *' but argument is of type 'const char *'
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..394272b 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, 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..e9d9897 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, 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 */
On Tue, Mar 05, 2013 at 10:37:27PM +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.
This purpetuates the brain-dead behaviour of the existing ASoC DMA engine layer, which makes it unsuitable for platforms with special DMA memory requirements.
The problem is that the DMA mask to be used for allocating DMA-able memory is the DMA engine struct device, not the struct device associated with the ASoC device.
I got this right in my ASoC generic DMA engine layer. Converting this layer is far from trivial though, and as my test platform has now become my entire network firewall, I'm not doing any testing on that anymore.
On Wed, Mar 06, 2013 at 05:13:33PM +0000, Russell King - ARM Linux wrote:
This purpetuates the brain-dead behaviour of the existing ASoC DMA engine layer, which makes it unsuitable for platforms with special DMA memory requirements.
The problem is that the DMA mask to be used for allocating DMA-able memory is the DMA engine struct device, not the struct device associated with the ASoC device.
I got this right in my ASoC generic DMA engine layer. Converting this layer is far from trivial though, and as my test platform has now become my entire network firewall, I'm not doing any testing on that anymore.
Could you go into more detail here please? Looking at the code I'm not seeing any allocations done by the library code at all, the allocations are all done by the individual platform DMA drivers so I don't see anything stopping them doing what they need.
On Thu, Mar 07, 2013 at 10:33:19AM +0800, Mark Brown wrote:
On Wed, Mar 06, 2013 at 05:13:33PM +0000, Russell King - ARM Linux wrote:
This purpetuates the brain-dead behaviour of the existing ASoC DMA engine layer, which makes it unsuitable for platforms with special DMA memory requirements.
The problem is that the DMA mask to be used for allocating DMA-able memory is the DMA engine struct device, not the struct device associated with the ASoC device.
I got this right in my ASoC generic DMA engine layer. Converting this layer is far from trivial though, and as my test platform has now become my entire network firewall, I'm not doing any testing on that anymore.
Could you go into more detail here please? Looking at the code I'm not seeing any allocations done by the library code at all, the allocations are all done by the individual platform DMA drivers so I don't see anything stopping them doing what they need.
I don't know what else you require apart from the description above. Isn't it rather obvious that you can't preallocate the ALSA buffer against the DMA engine device if you can only obtain the DMA engine device in the open function?
Notice in the code below where the DMA engine is obtained in pcm_new and the buffer is preallocated against the DMA engine struct device. That's exactly what I'm talking about.
/* * Generic ASoC DMA engine backend * * Copyright (C) 2012 Russell King * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. * * We expect the DMA engine to give accurate residue information, * returning the number of bytes left to complete to the requested * cookie. We also expect the DMA engine to be able to resume * submitted descriptors after a suspend/resume cycle. */ #include <linux/dma-mapping.h> #include <linux/dmaengine.h> #include <linux/module.h> #include <linux/slab.h> #include <linux/spinlock.h>
#include <sound/core.h> #include <sound/soc-dmaengine.h> #include <sound/soc.h> #include <sound/pcm_params.h>
#define BUFFER_SIZE_MAX 65536 #define PERIOD_SIZE_MIN 32 #define PERIOD_SIZE_MAX 16384 #define PERIODS_MIN 2 #define PERIODS_MAX 256
struct buf_info { struct scatterlist sg; dma_cookie_t cookie; };
struct soc_dma_chan { const struct soc_dma_config *conf; spinlock_t lock; struct dma_chan *chan; struct dma_slave_config cfg; enum dma_transfer_direction dir; unsigned nr_periods; unsigned sg_index; unsigned stopped; unsigned cyclic; dma_cookie_t cyclic_cookie; struct buf_info buf[PERIODS_MAX]; };
struct soc_dma_info { struct soc_dma_chan *chan[2]; };
static const struct snd_pcm_hardware soc_dmaengine_hardware = { .info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME, .period_bytes_min = PERIOD_SIZE_MIN, .period_bytes_max = PERIOD_SIZE_MAX, .periods_min = PERIODS_MIN, .periods_max = PERIODS_MAX, .buffer_bytes_max = BUFFER_SIZE_MAX, };
static int soc_dmaengine_submit(struct snd_pcm_substream *substream, struct soc_dma_chan *dma);
static void soc_dmaengine_callback(void *data) { struct snd_pcm_substream *substream = data; struct soc_dma_chan *dma = substream->runtime->private_data; int ret;
snd_pcm_period_elapsed(substream);
if (!dma->stopped && !dma->cyclic) { spin_lock(&dma->lock); ret = soc_dmaengine_submit(substream, dma); spin_unlock(&dma->lock);
if (ret == 0) dma_async_issue_pending(dma->chan); else pr_err("%s: failed to submit next DMA buffer\n", __func__); } }
static int soc_dmaengine_submit(struct snd_pcm_substream *substream, struct soc_dma_chan *dma) { struct dma_async_tx_descriptor *tx; struct dma_chan *ch = dma->chan; struct buf_info *buf; unsigned sg_index;
sg_index = dma->sg_index;
buf = &dma->buf[sg_index];
tx = dmaengine_prep_slave_sg(ch, &buf->sg, 1, dma->dir, DMA_PREP_INTERRUPT | DMA_CTRL_ACK); if (tx) { tx->callback = soc_dmaengine_callback; tx->callback_param = substream;
buf->cookie = dmaengine_submit(tx);
sg_index++; if (sg_index >= dma->nr_periods) sg_index = 0; dma->sg_index = sg_index; }
return tx ? 0 : -ENOMEM; }
static int soc_dmaengine_start(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct soc_dma_chan *dma = runtime->private_data; unsigned long flags; unsigned i; int ret = 0;
if (!dma->cyclic) { spin_lock_irqsave(&dma->lock, flags); for (i = 0; i < dma->nr_periods; i++) { ret = soc_dmaengine_submit(substream, dma); if (ret) break; } spin_unlock_irqrestore(&dma->lock, flags); if (ret == 0) { dma->stopped = 0; dma_async_issue_pending(dma->chan); } else { dma->stopped = 1; dmaengine_terminate_all(dma->chan); } } else { struct dma_async_tx_descriptor *tx;
tx = dmaengine_prep_dma_cyclic(dma->chan, runtime->dma_addr, snd_pcm_lib_buffer_bytes(substream), snd_pcm_lib_period_bytes(substream), dma->dir, DMA_CTRL_ACK | DMA_PREP_INTERRUPT); if (tx) { tx->callback = soc_dmaengine_callback; tx->callback_param = substream;
dma->cyclic_cookie = dmaengine_submit(tx); dma_async_issue_pending(dma->chan); } }
return ret; }
static int soc_dmaengine_stop(struct snd_pcm_substream *substream) { struct soc_dma_chan *dma = substream->runtime->private_data;
dma->stopped = 1; dmaengine_terminate_all(dma->chan);
return 0; }
static int soc_dmaengine_open(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_pcm_runtime *rtd = substream->private_data; struct soc_dma_info *info = snd_soc_platform_get_drvdata(rtd->platform); struct soc_dma_chan *dma = info->chan[substream->stream]; int ret = 0;
if (!dma) return -EINVAL;
runtime->hw = soc_dmaengine_hardware; runtime->hw.fifo_size = dma->conf->fifo_size;
if (dma->conf->align) { /* * FIXME: Ideally, there should be some way to query * this from the DMA engine itself. * * It would also be helpful to know the maximum size * a scatterlist entry can be to set the period size. */ ret = snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES, dma->conf->align); if (ret) goto err;
ret = snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_BYTES, dma->conf->align); if (ret) goto err; }
runtime->private_data = dma;
err: return ret; }
static int soc_dmaengine_close(struct snd_pcm_substream *substream) { return 0; }
static int soc_dmaengine_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { int ret = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params));
return ret < 0 ? ret : 0; }
static int soc_dmaengine_prepare(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct soc_dma_chan *dma = runtime->private_data; size_t buf_size = snd_pcm_lib_buffer_bytes(substream); size_t period = snd_pcm_lib_period_bytes(substream); dma_addr_t addr = runtime->dma_addr; unsigned i;
/* Create an array of sg entries, one for each period */ for (i = 0; i < PERIODS_MAX && buf_size; i++) { BUG_ON(buf_size < period);
sg_dma_address(&dma->buf[i].sg) = addr; sg_dma_len(&dma->buf[i].sg) = period;
addr += period; buf_size -= period; }
if (buf_size) { pr_err("DMA buffer size not a multiple of the period size: residue=%zu\n", buf_size); return -EINVAL; }
dma->nr_periods = i; dma->sg_index = 0;
return 0; }
static int soc_dmaengine_trigger(struct snd_pcm_substream *substream, int cmd) { struct snd_pcm_runtime *runtime = substream->runtime; struct soc_dma_chan *dma = runtime->private_data; int ret;
switch (cmd) { case SNDRV_PCM_TRIGGER_START: ret = soc_dmaengine_start(substream); break;
case SNDRV_PCM_TRIGGER_STOP: ret = soc_dmaengine_stop(substream); break;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_SUSPEND: ret = dmaengine_pause(dma->chan); break;
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: case SNDRV_PCM_TRIGGER_RESUME: ret = dmaengine_resume(dma->chan); break;
default: ret = -EINVAL; }
return ret; }
static snd_pcm_uframes_t soc_dmaengine_pointer(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct soc_dma_chan *dma = runtime->private_data; struct dma_chan *ch = dma->chan; struct dma_tx_state state; unsigned bytes;
if (dma->cyclic) { ch->device->device_tx_status(ch, dma->cyclic_cookie, &state);
bytes = snd_pcm_lib_buffer_bytes(substream) - state.residue; } else { unsigned index, pos; size_t period = snd_pcm_lib_period_bytes(substream); unsigned long flags; enum dma_status status; dma_cookie_t cookie;
/* * Get the next-to-be-submitted index, and the last submitted * cookie value. We use this to obtain the DMA engine state. */ spin_lock_irqsave(&dma->lock, flags); index = dma->sg_index; do { cookie = dma->buf[index].cookie; status = ch->device->device_tx_status(ch, cookie, &state); if (status == DMA_SUCCESS) { index++; if (index > dma->nr_periods) index = 0; } } while (status == DMA_SUCCESS); spin_unlock_irqrestore(&dma->lock, flags);
/* The end of the current DMA buffer */ pos = (index + 1) * period; /* Now take off the residue */ bytes = pos - state.residue; /* And correct for wrap-around */ if (bytes >= period * dma->nr_periods) bytes -= period * dma->nr_periods; }
return bytes_to_frames(runtime, bytes); }
static int soc_dmaengine_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_dma_buffer *buf = runtime->dma_buffer_p;
return dma_mmap_writecombine(buf->dev.dev, vma, runtime->dma_area, runtime->dma_addr, runtime->dma_bytes); }
static struct snd_pcm_ops soc_dmaengine_ops = { .open = soc_dmaengine_open, .close = soc_dmaengine_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = soc_dmaengine_hw_params, .hw_free = snd_pcm_lib_free_pages, .prepare = soc_dmaengine_prepare, .trigger = soc_dmaengine_trigger, .pointer = soc_dmaengine_pointer, .mmap = soc_dmaengine_mmap, };
static struct soc_dma_chan *soc_dmaengine_alloc(void) { struct soc_dma_chan *dma = kzalloc(sizeof(*dma), GFP_KERNEL); unsigned i;
if (dma) { spin_lock_init(&dma->lock); for (i = 0; i < PERIODS_MAX; i++) sg_init_table(&dma->buf[i].sg, 1); } return dma; }
static struct dma_chan *soc_dmaengine_get(enum dma_transaction_type type, struct soc_dma_config *cfg) { dma_cap_mask_t m;
dma_cap_zero(m); dma_cap_set(type, m); return dma_request_channel(m, cfg->filter, cfg->data); }
static int soc_dmaengine_request(struct soc_dma_chan *dma, struct soc_dma_config *cfg, unsigned stream) { int ret;
dma->conf = cfg; dma->chan = soc_dmaengine_get(DMA_CYCLIC, cfg); if (!dma->chan) dma->chan = soc_dmaengine_get(DMA_SLAVE, cfg); else dma->cyclic = 1; if (!dma->chan) { ret = -ENXIO; goto err_dma_req; }
if (stream == SNDRV_PCM_STREAM_PLAYBACK) { dma->dir = DMA_MEM_TO_DEV; dma->cfg.direction = DMA_MEM_TO_DEV; dma->cfg.dst_addr = cfg->reg; dma->cfg.dst_addr_width = cfg->width; dma->cfg.dst_maxburst = cfg->maxburst; } else { dma->dir = DMA_DEV_TO_MEM; dma->cfg.direction = DMA_DEV_TO_MEM; dma->cfg.src_addr = cfg->reg; dma->cfg.src_addr_width = cfg->width; dma->cfg.src_maxburst = cfg->maxburst; }
ret = dmaengine_slave_config(dma->chan, &dma->cfg); if (ret) goto err_dma_cfg;
return 0;
err_dma_cfg: dma_release_channel(dma->chan); dma->chan = NULL; err_dma_req: return ret; }
static void soc_dmaengine_release(struct soc_dma_chan *dma) { if (dma && dma->chan) dma_release_channel(dma->chan); kfree(dma); }
static int soc_dmaengine_preallocate_buffer(struct snd_pcm *pcm, unsigned stream, struct soc_dma_chan *dma) { struct snd_pcm_substream *substream = pcm->streams[stream].substream; int ret = 0;
if (substream) { struct snd_dma_buffer *buf = &substream->dma_buffer;
buf->dev.type = SNDRV_DMA_TYPE_DEV; buf->dev.dev = dma->chan->device->dev; buf->private_data = NULL; buf->area = dma_alloc_writecombine(buf->dev.dev, BUFFER_SIZE_MAX, &buf->addr, GFP_KERNEL); if (!buf->area) return -ENOMEM;
buf->bytes = BUFFER_SIZE_MAX; } return ret; }
static int soc_dmaengine_pcm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_pcm *pcm = rtd->pcm; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct soc_dma_info *info; unsigned stream; int ret = 0;
if (!cpu_dai) return -EINVAL;
if (!cpu_dai->playback_dma_data && !cpu_dai->capture_dma_data) { pr_err("soc_dmaengine: %s has no cpu_dai DMA data - incorrect probe ordering?\n", rtd->card->name); return -EINVAL; }
info = kzalloc(sizeof(*info), GFP_KERNEL); if (!info) return -ENOMEM;
for (stream = 0; stream < ARRAY_SIZE(pcm->streams); stream++) { struct soc_dma_config *cfg = NULL; struct soc_dma_chan *dma;
if (stream == SNDRV_PCM_STREAM_PLAYBACK) cfg = cpu_dai->playback_dma_data; else if (stream == SNDRV_PCM_STREAM_CAPTURE) cfg = cpu_dai->capture_dma_data;
if (!cfg) continue;
info->chan[stream] = dma = soc_dmaengine_alloc(); if (!dma) { ret = -ENOMEM; break; }
ret = soc_dmaengine_request(dma, cfg, stream); if (ret) return ret;
ret = soc_dmaengine_preallocate_buffer(pcm, stream, dma); if (ret) break; }
if (ret) { for (stream = 0; stream < ARRAY_SIZE(info->chan); stream++) soc_dmaengine_release(info->chan[stream]); kfree(info); } else { snd_soc_platform_set_drvdata(rtd->platform, info); }
return ret; }
/* * Use write-combining memory here: the standard ALSA * snd_free_dev_pages() doesn't support this. */ static void soc_dmaengine_pcm_free(struct snd_pcm *pcm) { unsigned stream;
for (stream = 0; stream < ARRAY_SIZE(pcm->streams); stream++) { struct snd_pcm_substream *substream = pcm->streams[stream].substream; struct snd_dma_buffer *buf;
if (!substream) continue; buf = &substream->dma_buffer; if (!buf->area) continue;
if (buf->dev.type == SNDRV_DMA_TYPE_DEV) dma_free_writecombine(buf->dev.dev, buf->bytes, buf->area, buf->addr); else snd_dma_free_pages(buf); buf->area = NULL; } }
/* * This is annoying - we can't have symetry with .pcm_new because .pcm_free * is called after the runtime information has been removed. It would be * better if we could find somewhere else to store our soc_dma_info pointer. */ static int soc_dmaengine_plat_remove(struct snd_soc_platform *platform) { struct soc_dma_info *info = snd_soc_platform_get_drvdata(platform); unsigned stream;
for (stream = 0; stream < ARRAY_SIZE(info->chan); stream++) soc_dmaengine_release(info->chan[stream]); kfree(info);
return 0; }
static struct snd_soc_platform_driver soc_dmaengine_platform = { .remove = soc_dmaengine_plat_remove, .pcm_new = soc_dmaengine_pcm_new, .pcm_free = soc_dmaengine_pcm_free, .ops = &soc_dmaengine_ops, /* Wait until the cpu_dai has been probed */ .probe_order = SND_SOC_COMP_ORDER_LATE, };
static int soc_dmaengine_probe(struct platform_device *pdev) { return snd_soc_register_platform(&pdev->dev, &soc_dmaengine_platform); }
static int soc_dmaengine_remove(struct platform_device *pdev) { snd_soc_unregister_platform(&pdev->dev); return 0; }
static struct platform_driver soc_dmaengine_driver = { .driver = { .name = "soc-dmaengine", .owner = THIS_MODULE, }, .probe = soc_dmaengine_probe, .remove = soc_dmaengine_remove, };
module_platform_driver(soc_dmaengine_driver);
MODULE_AUTHOR("Russell King"); MODULE_DESCRIPTION("ALSA SoC DMA engine driver"); MODULE_LICENSE("GPL v2"); MODULE_ALIAS("platform:soc-dmaengine");
On Thu, Mar 07, 2013 at 09:18:04AM +0000, Russell King - ARM Linux wrote:
On Thu, Mar 07, 2013 at 10:33:19AM +0800, Mark Brown wrote:
Could you go into more detail here please? Looking at the code I'm not seeing any allocations done by the library code at all, the allocations are all done by the individual platform DMA drivers so I don't see anything stopping them doing what they need.
I don't know what else you require apart from the description above. Isn't it rather obvious that you can't preallocate the ALSA buffer against the DMA engine device if you can only obtain the DMA engine device in the open function?
The bit I'm missing is why this is particularly hard to change, it doesn't seem like a massive refactoring and there's not many users.
On Thu, Mar 07, 2013 at 05:31:23PM +0800, Mark Brown wrote:
On Thu, Mar 07, 2013 at 09:18:04AM +0000, Russell King - ARM Linux wrote:
On Thu, Mar 07, 2013 at 10:33:19AM +0800, Mark Brown wrote:
Could you go into more detail here please? Looking at the code I'm not seeing any allocations done by the library code at all, the allocations are all done by the individual platform DMA drivers so I don't see anything stopping them doing what they need.
I don't know what else you require apart from the description above. Isn't it rather obvious that you can't preallocate the ALSA buffer against the DMA engine device if you can only obtain the DMA engine device in the open function?
The bit I'm missing is why this is particularly hard to change, it doesn't seem like a massive refactoring and there's not many users.
Well, it requires the thing to be reworked along with everyone who uses it, specifically snd_dmaengine_pcm_open() and snd_dmaengine_pcm_close().
Now, I could use your excuse that you've given me in the past: "I don't have much of that hardware so I can't test the changes, so I'm not going to touch this code evar again!" (That's basically what you said about the AC'97 struct device stuff.) You can't have it both ways and always shovel what you don't like onto other people.
So... I'll just go back to quietly sitting on my SA11x0 ASoC support and totally ignoring mainline with it because of obstructive maintainers. :)
On Thu, Mar 07, 2013 at 11:20:06AM +0000, Russell King - ARM Linux wrote:
On Thu, Mar 07, 2013 at 05:31:23PM +0800, Mark Brown wrote:
The bit I'm missing is why this is particularly hard to change, it doesn't seem like a massive refactoring and there's not many users.
Well, it requires the thing to be reworked along with everyone who uses it, specifically snd_dmaengine_pcm_open() and snd_dmaengine_pcm_close().
Oh, OK. That doesn't seem like a big deal really - it's certainly not a throw the thing out and start over job. It sounded like you'd identifed some new issue you'd not mentioned rather than just the same issue, it wasn't clear to me that it was the same issue.
Now, I could use your excuse that you've given me in the past: "I don't have much of that hardware so I can't test the changes, so I'm not going to touch this code evar again!" (That's basically what you said about
I'm not particularly asking you to fix this yourself except in that it seems like it's an important issue for you. If anything something like this patch ought to make things marginally easier to deal with by factoring out a very small bit of the code.
the AC'97 struct device stuff.) You can't have it both ways and always shovel what you don't like onto other people.
I'm aware of the issue, as are the people who've worked on the code. Speaking personally I just happen to disagree with you about the urgency here - it's not like it's the only problem we've got and the practical effects are limited to a subset of mostly older hardware which generally doesn't use dmaeengine in the first place. I imagine that a similar thing is true for everyone else.
There's plenty of other hardware that doesn't work right now, another pressing example I can think of is devices that subdivide an audio interface into multiple unrelated streams of audio, you aren't alone in having hardware that needs the frameworks improving in order to achieve basic functionality.
With the generic DMA device tree helper supported by mxs-dma driver, we now only need to call snd_dmaengine_generic_pcm_open() to have dmaengine_pcm request DMA channel from dmaengine by calling generic DMA helper dma_request_slave_channel().
Signed-off-by: Shawn Guo shawn.guo@linaro.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: alsa-devel@alsa-project.org --- .../devicetree/bindings/sound/mxs-saif.txt | 18 +++++---- sound/soc/mxs/mxs-pcm.c | 42 ++------------------ sound/soc/mxs/mxs-pcm.h | 5 --- sound/soc/mxs/mxs-saif.c | 28 +------------ sound/soc/mxs/mxs-saif.h | 1 - 5 files changed, 15 insertions(+), 79 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/mxs-saif.txt b/Documentation/devicetree/bindings/sound/mxs-saif.txt index c37ba61..89b3821 100644 --- a/Documentation/devicetree/bindings/sound/mxs-saif.txt +++ b/Documentation/devicetree/bindings/sound/mxs-saif.txt @@ -3,8 +3,11 @@ Required properties: - compatible: Should be "fsl,<chip>-saif" - reg: Should contain registers location and length -- interrupts: Should contain ERROR and DMA interrupts -- fsl,saif-dma-channel: APBX DMA channel for the SAIF +- interrupts: Should contain ERROR interrupt number +- dmas: DMA specifier, consisting of a phandle to DMA controller node + and SAIF DMA channel ID. + Refer to dma.txt and fsl-mxs-dma.txt for details. +- dma-names: Must be "rx-tx".
Optional properties: - fsl,saif-master: phandle to the master SAIF. It's only required for @@ -23,14 +26,15 @@ aliases { saif0: saif@80042000 { compatible = "fsl,imx28-saif"; reg = <0x80042000 2000>; - interrupts = <59 80>; - fsl,saif-dma-channel = <4>; + interrupts = <59>; + dmas = <&dma_apbx 4>; + dma-names = "rx-tx"; };
saif1: saif@80046000 { compatible = "fsl,imx28-saif"; reg = <0x80046000 2000>; - interrupts = <58 81>; - fsl,saif-dma-channel = <5>; - fsl,saif-master = <&saif0>; + interrupts = <58>; + dmas = <&dma_apbx 5>; + dma-names = "rx-tx"; }; diff --git a/sound/soc/mxs/mxs-pcm.c b/sound/soc/mxs/mxs-pcm.c index 564b5b6..10b20ed 100644 --- a/sound/soc/mxs/mxs-pcm.c +++ b/sound/soc/mxs/mxs-pcm.c @@ -28,7 +28,6 @@ #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/dmaengine.h> -#include <linux/fsl/mxs-dma.h>
#include <sound/core.h> #include <sound/initval.h> @@ -39,11 +38,6 @@
#include "mxs-pcm.h"
-struct mxs_pcm_dma_data { - struct mxs_dma_data dma_data; - struct mxs_pcm_dma_params *dma_params; -}; - static struct snd_pcm_hardware snd_mxs_hardware = { .info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | @@ -64,22 +58,6 @@ static struct snd_pcm_hardware snd_mxs_hardware = {
};
-static bool filter(struct dma_chan *chan, void *param) -{ - struct mxs_pcm_dma_data *pcm_dma_data = param; - struct mxs_pcm_dma_params *dma_params = pcm_dma_data->dma_params; - - if (!mxs_dma_is_apbx(chan)) - return false; - - if (chan->chan_id != dma_params->chan_num) - return false; - - chan->private = &pcm_dma_data->dma_data; - - return true; -} - static int snd_mxs_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -91,35 +69,21 @@ static int snd_mxs_pcm_hw_params(struct snd_pcm_substream *substream, static int snd_mxs_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct mxs_pcm_dma_data *pcm_dma_data; int ret;
- pcm_dma_data = kzalloc(sizeof(*pcm_dma_data), GFP_KERNEL); - if (pcm_dma_data == NULL) - return -ENOMEM; - - pcm_dma_data->dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); - pcm_dma_data->dma_data.chan_irq = pcm_dma_data->dma_params->chan_irq; - - ret = snd_dmaengine_pcm_open(substream, filter, pcm_dma_data); - if (ret) { - kfree(pcm_dma_data); + ret = snd_dmaengine_generic_pcm_open(substream, rtd->cpu_dai->dev, + "rx-tx"); + if (ret) return ret; - }
snd_soc_set_runtime_hwparams(substream, &snd_mxs_hardware);
- snd_dmaengine_pcm_set_data(substream, pcm_dma_data); - return 0; }
static int snd_mxs_close(struct snd_pcm_substream *substream) { - struct mxs_pcm_dma_data *pcm_dma_data = snd_dmaengine_pcm_get_data(substream); - snd_dmaengine_pcm_close(substream); - kfree(pcm_dma_data);
return 0; } diff --git a/sound/soc/mxs/mxs-pcm.h b/sound/soc/mxs/mxs-pcm.h index 35ba2ca..bc685b6 100644 --- a/sound/soc/mxs/mxs-pcm.h +++ b/sound/soc/mxs/mxs-pcm.h @@ -19,11 +19,6 @@ #ifndef _MXS_PCM_H #define _MXS_PCM_H
-struct mxs_pcm_dma_params { - int chan_irq; - int chan_num; -}; - int mxs_pcm_platform_register(struct device *dev); void mxs_pcm_platform_unregister(struct device *dev);
diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c index 3a2aa1d..0c70903 100644 --- a/sound/soc/mxs/mxs-saif.c +++ b/sound/soc/mxs/mxs-saif.c @@ -26,7 +26,6 @@ #include <linux/clk.h> #include <linux/delay.h> #include <linux/time.h> -#include <linux/fsl/mxs-dma.h> #include <linux/pinctrl/consumer.h> #include <sound/core.h> #include <sound/pcm.h> @@ -369,7 +368,6 @@ static int mxs_saif_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai) { struct mxs_saif *saif = snd_soc_dai_get_drvdata(cpu_dai); - snd_soc_dai_set_dma_data(cpu_dai, substream, &saif->dma_param);
/* clear error status to 0 for each re-open */ saif->fifo_underrun = 0; @@ -659,7 +657,7 @@ static irqreturn_t mxs_saif_irq(int irq, void *dev_id) static int mxs_saif_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; - struct resource *iores, *dmares; + struct resource *iores; struct mxs_saif *saif; struct pinctrl *pinctrl; int ret = 0; @@ -721,22 +719,6 @@ static int mxs_saif_probe(struct platform_device *pdev) if (IS_ERR(saif->base)) return PTR_ERR(saif->base);
- dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0); - if (!dmares) { - /* - * TODO: This is a temporary solution and should be changed - * to use generic DMA binding later when the helplers get in. - */ - ret = of_property_read_u32(np, "fsl,saif-dma-channel", - &saif->dma_param.chan_num); - if (ret) { - dev_err(&pdev->dev, "failed to get dma channel\n"); - return ret; - } - } else { - saif->dma_param.chan_num = dmares->start; - } - saif->irq = platform_get_irq(pdev, 0); if (saif->irq < 0) { ret = saif->irq; @@ -753,14 +735,6 @@ static int mxs_saif_probe(struct platform_device *pdev) return ret; }
- saif->dma_param.chan_irq = platform_get_irq(pdev, 1); - if (saif->dma_param.chan_irq < 0) { - ret = saif->dma_param.chan_irq; - dev_err(&pdev->dev, "failed to get dma irq resource: %d\n", - ret); - return ret; - } - platform_set_drvdata(pdev, saif);
ret = snd_soc_register_dai(&pdev->dev, &mxs_saif_dai); diff --git a/sound/soc/mxs/mxs-saif.h b/sound/soc/mxs/mxs-saif.h index 3cb342e..53eaa4b 100644 --- a/sound/soc/mxs/mxs-saif.h +++ b/sound/soc/mxs/mxs-saif.h @@ -117,7 +117,6 @@ struct mxs_saif { unsigned int mclk_in_use; void __iomem *base; int irq; - struct mxs_pcm_dma_params dma_param; unsigned int id; unsigned int master_id; unsigned int cur_rate;
On Tuesday 05 March 2013, Shawn Guo wrote:
The series converts mxs-dma and its clients to generic DMA device tree binding/helper.
I published a branch below to ease people who is willing to help testing the series.
Looks all good, aside from the bug I found in the first patch.
Reviewed-by: Arnd Bergmann arnd@arndb.de
participants (4)
-
Arnd Bergmann -
Mark Brown -
Russell King - ARM Linux -
Shawn Guo