[alsa-devel] [PATCH 2/4] ASoC: SAMSUNG: Add I2S0 internal dma driver

[Jassi Brar]

On Thu, Jun 9, 2011 at 1:39 PM, Sangbeom Kim <sbkim73 at samsung.com> wrote:
> I2S in Exynos4 and S5PC110(S5PV210) has a internal dma.
> It can be used low power audio mode and 2nd channel transfer.

For my convenience, could you please tell how does it differ from my
original implementation?  Most things look same, except for a few
variables.


> @@ -16,6 +17,7 @@ obj-$(CONFIG_SND_S3C_I2SV2_SOC) += snd-soc-s3c-i2s-v2.o
>  obj-$(CONFIG_SND_SAMSUNG_SPDIF) += snd-soc-samsung-spdif.o
>  obj-$(CONFIG_SND_SAMSUNG_PCM) += snd-soc-pcm.o
>  obj-$(CONFIG_SND_SAMSUNG_I2S) += snd-soc-i2s.o
> +obj-$(CONFIG_SND_SAMSUNG_I2S) += snd-soc-idma.o

Please check that building only for s3c64xx doesn't break by this.



> +
> +static struct idma_info {
> +       spinlock_t      lock;
> +       void             __iomem  *regs;
> +       int             trigger_stat;
The role of trigger_stat is not necessary.

> +
> +       /* Start address0 of I2S internal DMA operation. */
> +       val = readl(idma.regs + I2SSTR0);
Why read when you immediately overwrite it ?
> +       val = LP_TXBUFF_ADDR;
> +       writel(val, idma.regs + I2SSTR0);
> +


> +static int idma_hw_params(struct snd_pcm_substream *substream,
> +                               struct snd_pcm_hw_params *params)
> +{
> +       struct snd_pcm_runtime *runtime = substream->runtime;
> +       struct idma_ctrl *prtd = substream->runtime->private_data;
> +
> +       pr_debug("Entered %s\n", __func__);
can we have all the pr_debug's converted to dev_debug ?


> +       snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
> +       runtime->dma_bytes = params_buffer_bytes(params);
> +       memset(runtime->dma_area, 0, runtime->dma_bytes);
Is it really needed ?

> +
> +       if (iisahb & AHB_LVL0INT)
> +               val = AHB_CLRLVL0INT;
> +       else
> +               val = 0;
  val = (iisahb & AHB_LVL0INT) ? AHB_CLRLVL0INT : 0;     looks better

> +
> +       if (val) {
> +               iisahb |= val;
> +               writel(iisahb, idma.regs + I2SAHB);
> +
> +               addr = readl(idma.regs + I2SLVL0ADDR);
> +               addr += prtd->periodsz;
> +
> +               if (addr >= prtd->end)
> +                       addr = LP_TXBUFF_ADDR;
This will break if ring buffer is not a multiple of period size.
Either set the constraint in open or do modulo operation here.

> +
> +static int idma_close(struct snd_pcm_substream *substream)
> +{
> +       struct snd_pcm_runtime *runtime = substream->runtime;
> +       struct idma_ctrl *prtd = runtime->private_data;
> +
> +       pr_debug("Entered %s, prtd = %p\n", __func__, prtd);
> +
> +       free_irq(IRQ_I2S0, prtd);
> +
> +       if (!prtd)
> +               pr_err("idma_close called with prtd == NULL\n");
> +
> +       kfree(prtd);

Also       runtime->private_data = NULL;

> diff --git a/sound/soc/samsung/idma.h b/sound/soc/samsung/idma.h
> new file mode 100644
> index 0000000..2b0ac37
> --- /dev/null
> +++ b/sound/soc/samsung/idma.h
> @@ -0,0 +1,28 @@
> +/*
> + * idma.h  --  I2S0's Internal Dma driver
> + *
> + * Copyright (c) 2010 Samsung Electronics Co. Ltd
Copyright 2011 ?

> +/* idma_state */
> +#define LPAM_DMA_STOP    0
> +#define LPAM_DMA_START   1
These are internal to idma.c, please move them there.