On Wed, Jun 29, 2011 at 08:00:18PM +0200, Nicolas Ferre wrote:
+static const struct snd_pcm_hardware atmel_pcm_dma_hardware = {
- .info = SNDRV_PCM_INFO_MMAP |
SNDRV_PCM_INFO_MMAP_VALID |
The naming here looks a bit undescriptive but I'm not sure what a better name would be - obviously both DMA controllers are DMA controllers. It feels like it would be more sensible to just write a separate DMA driver as I'm not sure how much code actually ends up being shared here but it looks pretty small from the diff...
+static bool filter(struct dma_chan *chan, void *slave) +{
- struct at_dma_slave *sl = slave;
Odd whitespace here.
+static int atmel_pcm_dma_alloc(struct snd_pcm_substream *substream,
- struct snd_pcm_hw_params *params)
+{
- struct snd_pcm_runtime *runtime = substream->runtime;
- struct atmel_runtime_data *prtd = runtime->private_data;
- struct ssc_device *ssc = prtd->params->ssc;
- struct at_dma_slave *sdata = NULL;
- if (ssc->pdev)
sdata = ssc->pdev->dev.platform_data;
Why wouldn't we have a device, and why is there a separate copy of the device?
- if (!prtd->dma_chan) {
pr_err("atmel-pcm: ""DMA channel not available, unable to use SSC-audio\n");return -EBUSY;
dev_err() and don't split strings over lines.
- switch (prtd->params->data_xfer_size) {
- case 1:
buswidth = DMA_SLAVE_BUSWIDTH_1_BYTE;break;- case 2:
buswidth = DMA_SLAVE_BUSWIDTH_2_BYTES;break;- case 4:
buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES;break;- default:
return;- }
It strikes me that this mapping of bytes to DMA_SLAVE_ is likely to be quite common and might be nice as a helper in dmaengine.
I'd also expect to see something complain (even if just with BUG()) if we hit the default case.