All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Add AVR32 ALSA drivers
@ 2009-02-04 11:48 Hans-Christian Egtvedt
  2009-02-04 11:48 ` [PATCH 1/3] Add AVR32 ALSA drivers directory Hans-Christian Egtvedt
  0 siblings, 1 reply; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 11:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Hans-Christian Egtvedt

From: Hans-Christian Egtvedt <hcegtvedt@atmel.com>

This series adds support for the ABDAC and AC97C peripherals on AVR32 devices.

They also need the DW DMAC cyclic DMA API patch to work[1]. The patch set is
made against kernel v2.6.29-rc3. A review would be greatly appreciated,
checkpatch.pl gives no warnings and usage works as expected on hardware.

Adding these drivers to the queue for the upstream kernel should wait until the
cyclic DMA API for the DW DMAC driver is also in the queue for upstream.

1: http://avr32linux.org/archives/kernel/2009-February/002088.html
   http://avr32linux.org/archives/kernel/2009-February/002087.html

-- 
Best regards,
Hans-Christian Egtvedt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/3] Add AVR32 ALSA drivers directory.
  2009-02-04 11:48 [PATCH 0/3] Add AVR32 ALSA drivers Hans-Christian Egtvedt
@ 2009-02-04 11:48 ` Hans-Christian Egtvedt
  2009-02-04 11:48   ` [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC Hans-Christian Egtvedt
  2009-02-04 12:02   ` [PATCH 1/3] Add AVR32 ALSA drivers directory Takashi Iwai
  0 siblings, 2 replies; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 11:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Haavard Skinnemoen, Hans-Christian Egtvedt

From: Hans-Christian Egtvedt <hcegtvedt@atmel.com>

Signed-off-by: Hans-Christian Egtvedt <hcegtvedt@atmel.com>
Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 sound/Kconfig  |    2 ++
 sound/Makefile |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/sound/Kconfig b/sound/Kconfig
index 200aca1..0a5fbb3 100644
--- a/sound/Kconfig
+++ b/sound/Kconfig
@@ -60,6 +60,8 @@ source "sound/aoa/Kconfig"
 
 source "sound/arm/Kconfig"
 
+source "sound/avr32/Kconfig"
+
 source "sound/spi/Kconfig"
 
 source "sound/mips/Kconfig"
diff --git a/sound/Makefile b/sound/Makefile
index c76d707..a52b236 100644
--- a/sound/Makefile
+++ b/sound/Makefile
@@ -6,7 +6,7 @@ obj-$(CONFIG_SOUND_PRIME) += sound_firmware.o
 obj-$(CONFIG_SOUND_PRIME) += oss/
 obj-$(CONFIG_DMASOUND) += oss/
 obj-$(CONFIG_SND) += core/ i2c/ drivers/ isa/ pci/ ppc/ arm/ sh/ synth/ usb/ \
-	sparc/ spi/ parisc/ pcmcia/ mips/ soc/
+	sparc/ spi/ parisc/ pcmcia/ mips/ soc/ avr32/
 obj-$(CONFIG_SND_AOA) += aoa/
 
 # This one must be compilable even if sound is configured out
-- 
1.5.6.3

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC
  2009-02-04 11:48 ` [PATCH 1/3] Add AVR32 ALSA drivers directory Hans-Christian Egtvedt
@ 2009-02-04 11:48   ` Hans-Christian Egtvedt
  2009-02-04 11:48     ` [PATCH 3/3] Add ALSA driver for Atmel AC97 controller Hans-Christian Egtvedt
  2009-02-04 12:00     ` [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC Takashi Iwai
  2009-02-04 12:02   ` [PATCH 1/3] Add AVR32 ALSA drivers directory Takashi Iwai
  1 sibling, 2 replies; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 11:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Hans-Christian Egtvedt

This patch adds ALSA support for the Audio Bistream DAC found on Atmel
AVR32 devices.

Tested on ATSTK1006 + ATSTK1000 with a class D amplifier stage.

Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
---
 sound/avr32/Kconfig  |   11 +
 sound/avr32/Makefile |    3 +
 sound/avr32/abdac.c  |  597 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 611 insertions(+), 0 deletions(-)
 create mode 100644 sound/avr32/Kconfig
 create mode 100644 sound/avr32/Makefile
 create mode 100644 sound/avr32/abdac.c

diff --git a/sound/avr32/Kconfig b/sound/avr32/Kconfig
new file mode 100644
index 0000000..6ac06ff
--- /dev/null
+++ b/sound/avr32/Kconfig
@@ -0,0 +1,11 @@
+menu "AVR32 devices"
+	depends on AVR32
+
+config SND_AT32_ABDAC
+	tristate "Atmel AVR32 Audio Bitstream DAC (ABDAC) support"
+	select SND_PCM
+	depends on DW_DMAC
+	help
+	  ALSA sound driver for the Atmel AVR32 Audio Bitstream DAC (ABDAC).
+
+endmenu
diff --git a/sound/avr32/Makefile b/sound/avr32/Makefile
new file mode 100644
index 0000000..fa9ac7f
--- /dev/null
+++ b/sound/avr32/Makefile
@@ -0,0 +1,3 @@
+snd-at32-abdac-objs		:= abdac.o
+
+obj-$(CONFIG_SND_AT32_ABDAC)	+= snd-at32-abdac.o
diff --git a/sound/avr32/abdac.c b/sound/avr32/abdac.c
new file mode 100644
index 0000000..8e5ba9b
--- /dev/null
+++ b/sound/avr32/abdac.c
@@ -0,0 +1,597 @@
+/*
+ * Driver for the Atmel AVR32 on-chip Audio Bitstream DAC (ABDAC)
+ *
+ * Copyright (C) 2006-2009 Atmel Corporation
+ *
+ * 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.
+ */
+#include <linux/clk.h>
+#include <linux/bitmap.h>
+#include <linux/dw_dmac.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+
+#include <sound/core.h>
+#include <sound/initval.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+
+#include <mach/board.h>
+
+/* DAC register offsets */
+#define DAC_DATA                                0x0000
+#define DAC_CTRL                                0x0008
+#define DAC_INT_MASK                            0x000c
+#define DAC_INT_EN                              0x0010
+#define DAC_INT_DIS                             0x0014
+#define DAC_INT_CLR                             0x0018
+#define DAC_INT_STATUS                          0x001c
+
+/* Bitfields in CTRL */
+#define DAC_SWAP_OFFSET                         30
+#define DAC_SWAP_SIZE                           1
+#define DAC_EN_OFFSET                           31
+#define DAC_EN_SIZE                             1
+
+/* Bitfields in INT_MASK/INT_EN/INT_DIS/INT_STATUS/INT_CLR */
+#define DAC_UNDERRUN_OFFSET                     28
+#define DAC_UNDERRUN_SIZE                       1
+#define DAC_TX_READY_OFFSET                     29
+#define DAC_TX_READY_SIZE                       1
+
+/* Bit manipulation macros */
+#define DAC_BIT(name)					\
+	(1 << DAC_##name##_OFFSET)
+#define DAC_BF(name, value)				\
+	(((value) & ((1 << DAC_##name##_SIZE) - 1))	\
+	 << DAC_##name##_OFFSET)
+#define DAC_BFEXT(name, value)				\
+	(((value) >> DAC_##name##_OFFSET)		\
+	 & ((1 << DAC_##name##_SIZE) - 1))
+#define DAC_BFINS(name, value, old)			\
+	(((old) & ~(((1 << DAC_##name##_SIZE) - 1)	\
+		    << DAC_##name##_OFFSET))		\
+	 | DAC_BF(name, value))
+
+/* Register access macros */
+#define dac_readl(port, reg)				\
+	__raw_readl((port)->regs + DAC_##reg)
+#define dac_writel(port, reg, value)			\
+	__raw_writel((value), (port)->regs + DAC_##reg)
+
+/*
+ * ABDAC supports a maximum of 6 different rates from a generic clock. The
+ * generic clock has a power of two divider, which gives 6 steps from 192 kHz
+ * to 5112 Hz.
+ */
+#define MAX_NUM_RATES	6
+/* ALSA seems to use rates between 192000 Hz and 5112 Hz. */
+#define RATE_MAX	192000
+#define RATE_MIN	5112
+
+enum {
+	DMA_READY = 0,
+	DMA_STOP,
+};
+
+struct at32_abdac_dma {
+	struct dma_chan		*chan;
+	struct dw_cyclic_desc	*cdesc;
+};
+
+struct at32_abdac {
+	struct clk				*pclk;
+	struct clk				*sample_clk;
+	struct platform_device			*pdev;
+	struct at32_abdac_dma			dma;
+
+	struct snd_pcm_hw_constraint_list	constraints_rates;
+	struct snd_pcm_substream		*substream;
+	struct snd_card				*card;
+	struct snd_pcm				*pcm;
+
+	void __iomem				*regs;
+	unsigned long				flags;
+	unsigned int				rates[MAX_NUM_RATES];
+	unsigned int				rates_num;
+	int					irq;
+};
+
+#define get_dac(card) ((struct at32_abdac *)(card)->private_data)
+
+/* This function is called by the DMA driver. */
+static void at32_abdac_dma_period_done(void *arg)
+{
+	struct at32_abdac *dac = arg;
+	if (test_bit(DMA_STOP, &dac->flags))
+		return;
+	snd_pcm_period_elapsed(dac->substream);
+}
+
+static int at32_abdac_prepare_dma(struct at32_abdac *dac,
+		struct snd_pcm_substream *substream,
+		enum dma_data_direction direction)
+{
+	struct dma_chan			*chan = dac->dma.chan;
+	struct dw_cyclic_desc		*cdesc;
+	struct snd_pcm_runtime		*runtime = substream->runtime;
+	unsigned long			buffer_len, period_len;
+
+	/*
+	 * We don't do DMA on "complex" transfers, i.e. with
+	 * non-halfword-aligned buffers or lengths.
+	 */
+	if (runtime->dma_addr & 1 || runtime->buffer_size & 1) {
+		dev_dbg(&dac->pdev->dev, "too complex transfer\n");
+		return -EINVAL;
+	}
+
+	buffer_len = frames_to_bytes(runtime, runtime->buffer_size);
+	period_len = frames_to_bytes(runtime, runtime->period_size);
+
+	cdesc = dw_dma_cyclic_prep(chan, runtime->dma_addr, buffer_len,
+			period_len, DMA_TO_DEVICE);
+	if (IS_ERR(cdesc)) {
+		dev_dbg(&dac->pdev->dev, "could not prepare cyclic DMA\n");
+		return PTR_ERR(cdesc);
+	}
+
+	cdesc->period_callback = at32_abdac_dma_period_done;
+	cdesc->period_callback_param = dac;
+
+	dac->dma.cdesc = cdesc;
+
+	set_bit(DMA_READY, &dac->flags);
+
+	return 0;
+}
+
+static struct snd_pcm_hardware at32_abdac_hw = {
+	.info			= (SNDRV_PCM_INFO_MMAP
+				  | SNDRV_PCM_INFO_MMAP_VALID
+				  | SNDRV_PCM_INFO_INTERLEAVED
+				  | SNDRV_PCM_INFO_BLOCK_TRANSFER
+				  | SNDRV_PCM_INFO_RESUME
+				  | SNDRV_PCM_INFO_PAUSE),
+	.formats		= (SNDRV_PCM_FMTBIT_S16_BE),
+	.rates			= (SNDRV_PCM_RATE_KNOT),
+	.rate_min		= RATE_MIN,
+	.rate_max		= RATE_MAX,
+	.channels_min		= 2,
+	.channels_max		= 2,
+	.buffer_bytes_max	= 64 * 4096,
+	.period_bytes_min	= 4096,
+	.period_bytes_max	= 4096,
+	.periods_min		= 4,
+	.periods_max		= 64,
+};
+
+static int at32_abdac_open(struct snd_pcm_substream *substream)
+{
+	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
+
+	dac->substream = substream;
+	at32_abdac_hw.rate_max = dac->rates[dac->rates_num - 1];
+	at32_abdac_hw.rate_min = dac->rates[0];
+	substream->runtime->hw = at32_abdac_hw;
+
+	return snd_pcm_hw_constraint_list(substream->runtime, 0,
+			SNDRV_PCM_HW_PARAM_RATE, &dac->constraints_rates);
+}
+
+static int at32_abdac_close(struct snd_pcm_substream *substream)
+{
+	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
+	clear_bit(DMA_STOP, &dac->flags);
+	dac->substream = NULL;
+	return 0;
+}
+
+static int at32_abdac_hw_params(struct snd_pcm_substream *substream,
+		struct snd_pcm_hw_params *hw_params)
+{
+	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
+	int retval;
+
+	retval = snd_pcm_lib_malloc_pages(substream,
+			params_buffer_bytes(hw_params));
+	if (retval)
+		return retval;
+
+	if (test_and_clear_bit(DMA_READY, &dac->flags))
+		dw_dma_cyclic_free(dac->dma.chan);
+
+	return at32_abdac_prepare_dma(dac, substream, DMA_TO_DEVICE);
+}
+
+static int at32_abdac_hw_free(struct snd_pcm_substream *substream)
+{
+	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
+	if (test_and_clear_bit(DMA_READY, &dac->flags))
+		dw_dma_cyclic_free(dac->dma.chan);
+	return snd_pcm_lib_free_pages(substream);
+}
+
+static int at32_abdac_prepare(struct snd_pcm_substream *substream)
+{
+	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
+	return clk_set_rate(dac->sample_clk, 256 * substream->runtime->rate);
+}
+
+static int at32_abdac_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
+	int retval = -EINVAL;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: /* fall through */
+	case SNDRV_PCM_TRIGGER_RESUME: /* fall through */
+	case SNDRV_PCM_TRIGGER_START:
+		clk_enable(dac->sample_clk);
+		retval = dw_dma_cyclic_start(dac->dma.chan);
+		if (retval)
+			goto out;
+		dac_writel(dac, CTRL, DAC_BIT(EN));
+		break;
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: /* fall through */
+	case SNDRV_PCM_TRIGGER_SUSPEND: /* fall through */
+	case SNDRV_PCM_TRIGGER_STOP:
+		set_bit(DMA_STOP, &dac->flags);
+		dw_dma_cyclic_stop(dac->dma.chan);
+		dac_writel(dac, DATA, 0);
+		dac_writel(dac, CTRL, 0);
+		clk_disable(dac->sample_clk);
+		break;
+	default:
+		break;
+	}
+out:
+	return retval;
+}
+
+static snd_pcm_uframes_t
+at32_abdac_pointer(struct snd_pcm_substream *substream)
+{
+	struct at32_abdac	*dac = snd_pcm_substream_chip(substream);
+	struct snd_pcm_runtime	*runtime = substream->runtime;
+	snd_pcm_uframes_t	frames;
+	unsigned long		bytes;
+
+	bytes = dw_dma_get_src_addr(dac->dma.chan);
+	bytes -= runtime->dma_addr;
+
+	frames = bytes_to_frames(runtime, bytes);
+	if (frames >= runtime->buffer_size)
+		frames -= runtime->buffer_size;
+
+	return frames;
+}
+
+static irqreturn_t abdac_interrupt(int irq, void *dev_id)
+{
+	struct at32_abdac *dac = dev_id;
+	u32 status;
+
+	status = dac_readl(dac, INT_STATUS);
+	if (status & DAC_BIT(UNDERRUN)) {
+		dev_err(&dac->pdev->dev, "underrun detected\n");
+		dac_writel(dac, INT_CLR, DAC_BIT(UNDERRUN));
+	} else {
+		dev_err(&dac->pdev->dev, "spurious interrupt (status=0x%x)\n",
+			status);
+		dac_writel(dac, INT_CLR, status);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct snd_pcm_ops at32_abdac_ops = {
+	.open		= at32_abdac_open,
+	.close		= at32_abdac_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= at32_abdac_hw_params,
+	.hw_free	= at32_abdac_hw_free,
+	.prepare	= at32_abdac_prepare,
+	.trigger	= at32_abdac_trigger,
+	.pointer	= at32_abdac_pointer,
+};
+
+static int __devinit at32_abdac_pcm_new(struct at32_abdac *dac)
+{
+	struct snd_pcm *pcm;
+	int retval;
+
+	retval = snd_pcm_new(dac->card, dac->card->shortname,
+			dac->pdev->id, 1, 0, &pcm);
+	if (retval)
+		return retval;
+
+	strcpy(pcm->name, dac->card->shortname);
+	pcm->private_data = dac;
+	pcm->info_flags = 0;
+	dac->pcm = pcm;
+
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &at32_abdac_ops);
+
+	retval = snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV,
+			&dac->pdev->dev, at32_abdac_hw.buffer_bytes_max,
+			at32_abdac_hw.buffer_bytes_max);
+
+	return retval;
+}
+
+static bool filter(struct dma_chan *chan, void *slave)
+{
+	struct dw_dma_slave *dws = slave;
+
+	if (dws->dma_dev == chan->device->dev) {
+		chan->private = dws;
+		return true;
+	} else
+		return false;
+}
+
+static int set_sample_rates(struct at32_abdac *dac)
+{
+	long new_rate = RATE_MAX;
+	int retval = -EINVAL;
+	int index = 0;
+
+	/* we start at 192 kHz and work our way down to 5112 Hz */
+	while (new_rate >= RATE_MIN && index < (MAX_NUM_RATES + 1)) {
+		new_rate = clk_round_rate(dac->sample_clk, 256 * new_rate);
+		if (new_rate < 0)
+			break;
+		/* make sure we are below the ABDAC clock */
+		if (new_rate <= clk_get_rate(dac->pclk)) {
+			dac->rates[index] = new_rate / 256;
+			index++;
+		}
+		/* divide by 256 and then by two to get next rate */
+		new_rate /= 256 * 2;
+	}
+
+	if (index) {
+		int i;
+
+		/* reverse array, smallest go first */
+		for (i = 0; i < (index / 2); i++) {
+			unsigned int tmp = dac->rates[index - 1 - i];
+			dac->rates[index - 1 - i] = dac->rates[i];
+			dac->rates[i] = tmp;
+		}
+
+		dac->constraints_rates.count = index;
+		dac->constraints_rates.list = dac->rates;
+		dac->constraints_rates.mask = 0;
+		dac->rates_num = index;
+
+		retval = 0;
+	}
+
+	return retval;
+}
+
+static int __devinit at32_abdac_probe(struct platform_device *pdev)
+{
+	struct snd_card		*card;
+	struct at32_abdac	*dac;
+	struct resource		*regs;
+	struct at32_abdac_pdata	*pdata;
+	struct clk		*pclk;
+	struct clk		*sample_clk;
+	int			retval;
+	int			irq;
+
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!regs) {
+		dev_dbg(&pdev->dev, "no memory resource\n");
+		return -ENXIO;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_dbg(&pdev->dev, "could not get IRQ number\n");
+		return irq;
+	}
+
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		dev_dbg(&pdev->dev, "no platform data\n");
+		return -ENXIO;
+	}
+
+	pclk = clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(pclk)) {
+		dev_dbg(&pdev->dev, "no peripheral clock\n");
+		return PTR_ERR(pclk);
+	}
+	sample_clk = clk_get(&pdev->dev, "sample_clk");
+	if (IS_ERR(pclk)) {
+		dev_dbg(&pdev->dev, "no sample clock\n");
+		retval = PTR_ERR(pclk);
+		goto out_put_pclk;
+	}
+	clk_enable(pclk);
+
+	retval = -ENOMEM;
+	card = snd_card_new(SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
+			THIS_MODULE, sizeof(struct at32_abdac));
+	if (!card) {
+		dev_dbg(&pdev->dev, "could not allocate memory\n");
+		goto out_put_sample_clk;
+	}
+
+	dac = get_dac(card);
+
+	dac->irq = irq;
+	dac->card = card;
+	dac->pclk = pclk;
+	dac->sample_clk = sample_clk;
+	dac->pdev = pdev;
+
+	retval = set_sample_rates(dac);
+	if (retval < 0) {
+		dev_dbg(&pdev->dev, "could not set supported rates\n");
+		goto out_free_card;
+	}
+
+	dac->regs = ioremap(regs->start, regs->end - regs->start + 1);
+	if (!dac->regs) {
+		dev_dbg(&pdev->dev, "could not remap register memory\n");
+		goto out_free_card;
+	}
+
+	/* make sure the DAC is silent and disabled */
+	dac_writel(dac, DATA, 0);
+	dac_writel(dac, CTRL, 0);
+
+	retval = request_irq(irq, abdac_interrupt, 0, "dac", dac);
+	if (retval) {
+		dev_dbg(&pdev->dev, "could not request irq\n");
+		goto out_unmap_regs;
+	}
+
+	snd_card_set_dev(card, &pdev->dev);
+
+	if (pdata->dws.dma_dev) {
+		struct dw_dma_slave *dws = &pdata->dws;
+		dma_cap_mask_t mask;
+
+		dws->tx_reg = regs->start + DAC_DATA;
+
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+
+		dac->dma.chan = dma_request_channel(mask, filter, dws);
+	}
+	if (!pdata->dws.dma_dev || !dac->dma.chan) {
+		dev_dbg(&pdev->dev, "DMA not available\n");
+		retval = -ENODEV;
+		goto out_unset_card_dev;
+	}
+
+	strcpy(card->driver, "AVR32 ABDAC");
+	strcpy(card->shortname, "AVR32 ABDAC");
+	sprintf(card->longname, "Atmel AVR32 Audio Bitstream DAC");
+
+	retval = at32_abdac_pcm_new(dac);
+	if (retval) {
+		dev_dbg(&pdev->dev, "could not register ac97 pcm device\n");
+		goto out_release_dma;
+	}
+
+	retval = snd_card_register(card);
+	if (retval) {
+		dev_dbg(&pdev->dev, "could not register sound card\n");
+		goto out_release_dma;
+	}
+
+	platform_set_drvdata(pdev, card);
+
+	dev_info(&pdev->dev, "Atmel AVR32 ABDAC at 0x%p using %s\n",
+			dac->regs, dac->dma.chan->dev->device.bus_id);
+
+	return retval;
+
+out_release_dma:
+	dma_release_channel(dac->dma.chan);
+	dac->dma.chan = NULL;
+out_unset_card_dev:
+	snd_card_set_dev(card, NULL);
+	free_irq(irq, dac);
+out_unmap_regs:
+	iounmap(dac->regs);
+out_free_card:
+	snd_card_free(card);
+out_put_sample_clk:
+	clk_put(sample_clk);
+	clk_disable(pclk);
+out_put_pclk:
+	clk_put(pclk);
+	return retval;
+}
+
+#ifdef CONFIG_PM
+static int at32_abdac_suspend(struct platform_device *pdev, pm_message_t msg)
+{
+	struct snd_card *card = platform_get_drvdata(pdev);
+	struct at32_abdac *dac = card->private_data;
+
+	dw_dma_cyclic_stop(dac->dma.chan);
+	clk_disable(dac->sample_clk);
+	clk_disable(dac->pclk);
+
+	return 0;
+}
+
+static int at32_abdac_resume(struct platform_device *pdev)
+{
+	struct snd_card *card = platform_get_drvdata(pdev);
+	struct at32_abdac *dac = card->private_data;
+
+	clk_enable(dac->pclk);
+	clk_enable(dac->sample_clk);
+	if (test_bit(DMA_READY, &dac->flags))
+		dw_dma_cyclic_start(dac->dma.chan);
+
+	return 0;
+}
+#else
+#define at32_abdac_suspend NULL
+#define at32_abdac_resume NULL
+#endif
+
+static int __devexit at32_abdac_remove(struct platform_device *pdev)
+{
+	struct snd_card *card = platform_get_drvdata(pdev);
+	struct at32_abdac *dac = get_dac(card);
+
+	clk_put(dac->sample_clk);
+	clk_disable(dac->pclk);
+	clk_put(dac->pclk);
+
+	dma_release_channel(dac->dma.chan);
+	dac->dma.chan = NULL;
+	snd_card_set_dev(card, NULL);
+	iounmap(dac->regs);
+	free_irq(dac->irq, dac);
+	snd_card_free(card);
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver at32_abdac_driver = {
+	.remove		= __devexit_p(at32_abdac_remove),
+	.driver		= {
+		.name	= "at32_abdac",
+	},
+	.suspend	= at32_abdac_suspend,
+	.resume		= at32_abdac_resume,
+};
+
+static int __init at32_abdac_init(void)
+{
+	return platform_driver_probe(&at32_abdac_driver,
+			at32_abdac_probe);
+}
+module_init(at32_abdac_init);
+
+static void __exit at32_abdac_exit(void)
+{
+	platform_driver_unregister(&at32_abdac_driver);
+}
+module_exit(at32_abdac_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Driver for AVR32 Audio Bitstream DAC (ABDAC)");
+MODULE_AUTHOR("Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>");
-- 
1.5.6.3

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 11:48   ` [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC Hans-Christian Egtvedt
@ 2009-02-04 11:48     ` Hans-Christian Egtvedt
  2009-02-04 12:00       ` Mark Brown
                         ` (2 more replies)
  2009-02-04 12:00     ` [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC Takashi Iwai
  1 sibling, 3 replies; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 11:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Hans-Christian Egtvedt

This patch adds ALSA support for the AC97 controller found on Atmel
AVR32 devices.

Tested on ATSTK1006 + ATSTK1000 with a development board with a AC97
codec.

Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
---
 include/sound/atmel-ac97c.h |   40 ++
 sound/avr32/Kconfig         |    8 +
 sound/avr32/Makefile        |    2 +
 sound/avr32/ac97c.c         |  994 +++++++++++++++++++++++++++++++++++++++++++
 sound/avr32/ac97c.h         |   71 +++
 5 files changed, 1115 insertions(+), 0 deletions(-)
 create mode 100644 include/sound/atmel-ac97c.h
 create mode 100644 sound/avr32/ac97c.c
 create mode 100644 sound/avr32/ac97c.h

diff --git a/include/sound/atmel-ac97c.h b/include/sound/atmel-ac97c.h
new file mode 100644
index 0000000..d5daddd
--- /dev/null
+++ b/include/sound/atmel-ac97c.h
@@ -0,0 +1,40 @@
+/*
+ * Driver for the Atmel AC97C controller
+ *
+ * Copyright (C) 2005-2009 Atmel Corporation
+ *
+ * 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.
+ */
+#ifndef __INCLUDE_SOUND_AT32_AC97C_H
+#define __INCLUDE_SOUND_AT32_AC97C_H
+
+#include <linux/dw_dmac.h>
+
+#define AC97C_CAPTURE	0x01
+#define AC97C_PLAYBACK	0x02
+#define AC97C_BOTH	(AC97C_CAPTURE | AC97C_PLAYBACK)
+
+/**
+ * struct atmel_ac97c_pdata - board specific AC97C configuration
+ * @rx_dws: DMA slave interface to use in sound capture.
+ * @tx_dws: DMA slave interface to use in sound playback.
+ * @reset_pin: GPIO pin wired to the reset input on the external AC97 codec,
+ *             optional to use, set to -ENODEV if not in use. AC97 layer will
+ *             try to do a software reset of the external codec anyway.
+ * @flags: Flags for which directions should be enabled.
+ *
+ * If the user do not want to use a DMA channel for playback or capture, i.e.
+ * only one feature is required on the board. The slave for playback or capture
+ * can be set to NULL. The AC97C driver will take use of this when setting up
+ * the sound streams.
+ */
+struct ac97c_platform_data {
+	struct dw_dma_slave	rx_dws;
+	struct dw_dma_slave	tx_dws;
+	unsigned int		flags;
+	int			reset_pin;
+};
+
+#endif /* __INCLUDE_SOUND_AT32_AC97C_H */
diff --git a/sound/avr32/Kconfig b/sound/avr32/Kconfig
index 6ac06ff..82ec27f 100644
--- a/sound/avr32/Kconfig
+++ b/sound/avr32/Kconfig
@@ -8,4 +8,12 @@ config SND_AT32_ABDAC
 	help
 	  ALSA sound driver for the Atmel AVR32 Audio Bitstream DAC (ABDAC).
 
+config SND_AT32_AC97C
+	tristate "Atmel AVR32 AC97 Controller Driver"
+	select SND_PCM
+	select SND_AC97_CODEC
+	depends on DW_DMAC
+	help
+	  ALSA sound driver for the Atmel AVR32 AC97 controller.
+
 endmenu
diff --git a/sound/avr32/Makefile b/sound/avr32/Makefile
index fa9ac7f..4018adc 100644
--- a/sound/avr32/Makefile
+++ b/sound/avr32/Makefile
@@ -1,3 +1,5 @@
 snd-at32-abdac-objs		:= abdac.o
+snd-at32-ac97c-objs		:= ac97c.o
 
 obj-$(CONFIG_SND_AT32_ABDAC)	+= snd-at32-abdac.o
+obj-$(CONFIG_SND_AT32_AC97C)	+= snd-at32-ac97c.o
diff --git a/sound/avr32/ac97c.c b/sound/avr32/ac97c.c
new file mode 100644
index 0000000..218d666
--- /dev/null
+++ b/sound/avr32/ac97c.c
@@ -0,0 +1,994 @@
+/*
+ * Driver for the Atmel AC97C controller
+ *
+ * Copyright (C) 2005-2009 Atmel Corporation
+ *
+ * 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.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/bitmap.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/mutex.h>
+#include <linux/gpio.h>
+#include <linux/io.h>
+
+#include <sound/core.h>
+#include <sound/initval.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/ac97_codec.h>
+#include <sound/atmel-ac97c.h>
+#include <sound/memalloc.h>
+
+#include <linux/dw_dmac.h>
+
+#include "ac97c.h"
+
+enum {
+	DMA_TX_READY = 0,
+	DMA_TX_STOP,
+	DMA_RX_READY,
+	DMA_RX_STOP,
+	DMA_TX_CHAN_PRESENT,
+	DMA_RX_CHAN_PRESENT,
+};
+
+/* Serialize access to opened variable */
+static DEFINE_MUTEX(opened_mutex);
+
+struct atmel_ac97c_dma {
+	struct dma_chan			*rx_chan;
+	struct dma_chan			*tx_chan;
+};
+
+struct atmel_ac97c {
+	struct clk			*pclk;
+	struct platform_device		*pdev;
+	struct atmel_ac97c_dma		dma;
+
+	struct snd_pcm_substream	*playback_substream;
+	struct snd_pcm_substream	*capture_substream;
+	struct snd_card			*card;
+	struct snd_pcm			*pcm;
+	struct snd_ac97			*ac97;
+	struct snd_ac97_bus		*ac97_bus;
+
+	u64				cur_format;
+	unsigned int			cur_rate;
+	unsigned long			flags;
+	/* Serialize access to opened variable */
+	spinlock_t			lock;
+	void __iomem			*regs;
+	int				opened;
+	int				reset_pin;
+};
+
+#define get_chip(card) ((struct atmel_ac97c *)(card)->private_data)
+
+#define ac97c_writel(chip, reg, val)			\
+	__raw_writel((val), (chip)->regs + AC97C_##reg)
+#define ac97c_readl(chip, reg)				\
+	__raw_readl((chip)->regs + AC97C_##reg)
+
+/* This function is called by the DMA driver. */
+static void atmel_ac97c_dma_playback_period_done(void *arg)
+{
+	struct atmel_ac97c	*chip = arg;
+
+	if (test_bit(DMA_TX_STOP, &chip->flags))
+		return;
+	snd_pcm_period_elapsed(chip->playback_substream);
+}
+
+static void atmel_ac97c_dma_capture_period_done(void *arg)
+{
+	struct atmel_ac97c	*chip = arg;
+
+	if (test_bit(DMA_RX_STOP, &chip->flags))
+		return;
+	snd_pcm_period_elapsed(chip->capture_substream);
+}
+
+static int atmel_ac97c_prepare_dma(struct atmel_ac97c *chip,
+		struct snd_pcm_substream *substream,
+		enum dma_data_direction direction)
+{
+	struct dma_chan			*chan;
+	struct dw_cyclic_desc		*cdesc;
+	struct snd_pcm_runtime		*runtime = substream->runtime;
+	unsigned long			buffer_len, period_len;
+
+	/*
+	 * We don't do DMA on "complex" transfers, i.e. with
+	 * non-halfword-aligned buffers or lengths.
+	 */
+	if (runtime->dma_addr & 1 || runtime->buffer_size & 1) {
+		dev_dbg(&chip->pdev->dev, "too complex transfer\n");
+		return -EINVAL;
+	}
+
+	if (direction == DMA_TO_DEVICE)
+		chan = chip->dma.tx_chan;
+	else
+		chan = chip->dma.rx_chan;
+
+	buffer_len = frames_to_bytes(runtime, runtime->buffer_size);
+	period_len = frames_to_bytes(runtime, runtime->period_size);
+
+	cdesc = dw_dma_cyclic_prep(chan, runtime->dma_addr, buffer_len,
+			period_len, direction);
+	if (IS_ERR(cdesc)) {
+		dev_dbg(&chip->pdev->dev, "could not prepare cyclic DMA\n");
+		return PTR_ERR(cdesc);
+	}
+
+	if (direction == DMA_TO_DEVICE) {
+		cdesc->period_callback = atmel_ac97c_dma_playback_period_done;
+		set_bit(DMA_TX_READY, &chip->flags);
+	} else {
+		cdesc->period_callback = atmel_ac97c_dma_capture_period_done;
+		set_bit(DMA_RX_READY, &chip->flags);
+	}
+
+	cdesc->period_callback_param = chip;
+
+	return 0;
+}
+
+static struct snd_pcm_hardware atmel_ac97c_playback_hw = {
+	.info			= (SNDRV_PCM_INFO_MMAP
+				  | SNDRV_PCM_INFO_MMAP_VALID
+				  | SNDRV_PCM_INFO_INTERLEAVED
+				  | SNDRV_PCM_INFO_BLOCK_TRANSFER
+				  | SNDRV_PCM_INFO_JOINT_DUPLEX
+				  | SNDRV_PCM_INFO_BLOCK_TRANSFER
+				  | SNDRV_PCM_INFO_RESUME
+				  | SNDRV_PCM_INFO_PAUSE),
+	.formats		= (SNDRV_PCM_FMTBIT_S16_BE
+				  | SNDRV_PCM_FMTBIT_S16_LE),
+	.rates			= (SNDRV_PCM_RATE_CONTINUOUS),
+	.rate_min		= 4000,
+	.rate_max		= 48000,
+	.channels_min		= 1,
+	.channels_max		= 2,
+	.buffer_bytes_max	= 64 * 4096,
+	.period_bytes_min	= 4096,
+	.period_bytes_max	= 4096,
+	.periods_min		= 4,
+	.periods_max		= 64,
+};
+
+static struct snd_pcm_hardware atmel_ac97c_capture_hw = {
+	.info			= (SNDRV_PCM_INFO_MMAP
+				  | SNDRV_PCM_INFO_MMAP_VALID
+				  | SNDRV_PCM_INFO_INTERLEAVED
+				  | SNDRV_PCM_INFO_BLOCK_TRANSFER
+				  | SNDRV_PCM_INFO_JOINT_DUPLEX
+				  | SNDRV_PCM_INFO_BLOCK_TRANSFER
+				  | SNDRV_PCM_INFO_RESUME
+				  | SNDRV_PCM_INFO_PAUSE),
+	.formats		= (SNDRV_PCM_FMTBIT_S16_BE
+				  | SNDRV_PCM_FMTBIT_S16_LE),
+	.rates			= (SNDRV_PCM_RATE_CONTINUOUS),
+	.rate_min		= 4000,
+	.rate_max		= 48000,
+	.channels_min		= 1,
+	.channels_max		= 2,
+	.buffer_bytes_max	= 64 * 4096,
+	.period_bytes_min	= 4096,
+	.period_bytes_max	= 4096,
+	.periods_min		= 4,
+	.periods_max		= 64,
+};
+
+static int atmel_ac97c_playback_open(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	mutex_lock(&opened_mutex);
+	chip->opened++;
+	runtime->hw = atmel_ac97c_playback_hw;
+	if (chip->cur_rate) {
+		runtime->hw.rate_min = chip->cur_rate;
+		runtime->hw.rate_max = chip->cur_rate;
+	}
+	if (chip->cur_format)
+		runtime->hw.formats = (1ULL << chip->cur_format);
+	mutex_unlock(&opened_mutex);
+	chip->playback_substream = substream;
+	return 0;
+}
+
+static int atmel_ac97c_capture_open(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	mutex_lock(&opened_mutex);
+	chip->opened++;
+	runtime->hw = atmel_ac97c_capture_hw;
+	if (chip->cur_rate) {
+		runtime->hw.rate_min = chip->cur_rate;
+		runtime->hw.rate_max = chip->cur_rate;
+	}
+	if (chip->cur_format)
+		runtime->hw.formats = (1ULL << chip->cur_format);
+	mutex_unlock(&opened_mutex);
+	chip->capture_substream = substream;
+	return 0;
+}
+
+static int atmel_ac97c_playback_close(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+
+	mutex_lock(&opened_mutex);
+	chip->opened--;
+	if (!chip->opened) {
+		chip->cur_rate = 0;
+		chip->cur_format = 0;
+	}
+	mutex_unlock(&opened_mutex);
+
+	clear_bit(DMA_TX_STOP, &chip->flags);
+	chip->playback_substream = NULL;
+
+	return 0;
+}
+
+static int atmel_ac97c_capture_close(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+
+	mutex_lock(&opened_mutex);
+	chip->opened--;
+	if (!chip->opened) {
+		chip->cur_rate = 0;
+		chip->cur_format = 0;
+	}
+	mutex_unlock(&opened_mutex);
+
+	clear_bit(DMA_RX_STOP, &chip->flags);
+	chip->capture_substream = NULL;
+
+	return 0;
+}
+
+static int atmel_ac97c_playback_hw_params(struct snd_pcm_substream *substream,
+		struct snd_pcm_hw_params *hw_params)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	int retval;
+
+	retval = snd_pcm_lib_malloc_pages(substream,
+					params_buffer_bytes(hw_params));
+	if (retval)
+		return retval;
+
+	/* Set restrictions to params */
+	mutex_lock(&opened_mutex);
+	chip->cur_rate = params_rate(hw_params);
+	chip->cur_format = params_format(hw_params);
+	mutex_unlock(&opened_mutex);
+
+	if (test_and_clear_bit(DMA_TX_READY, &chip->flags))
+		dw_dma_cyclic_free(chip->dma.tx_chan);
+
+	return atmel_ac97c_prepare_dma(chip, substream, DMA_TO_DEVICE);
+}
+
+static int atmel_ac97c_capture_hw_params(struct snd_pcm_substream *substream,
+		struct snd_pcm_hw_params *hw_params)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	int retval;
+
+	retval = snd_pcm_lib_malloc_pages(substream,
+					params_buffer_bytes(hw_params));
+	if (retval)
+		return retval;
+
+	/* Set restrictions to params */
+	mutex_lock(&opened_mutex);
+	chip->cur_rate = params_rate(hw_params);
+	chip->cur_format = params_format(hw_params);
+	mutex_unlock(&opened_mutex);
+
+	if (test_and_clear_bit(DMA_RX_READY, &chip->flags))
+		dw_dma_cyclic_free(chip->dma.rx_chan);
+
+	return atmel_ac97c_prepare_dma(chip, substream, DMA_FROM_DEVICE);
+}
+
+static int atmel_ac97c_playback_hw_free(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	if (test_and_clear_bit(DMA_TX_READY, &chip->flags))
+		dw_dma_cyclic_free(chip->dma.tx_chan);
+	return snd_pcm_lib_free_pages(substream);
+}
+
+static int atmel_ac97c_capture_hw_free(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	if (test_and_clear_bit(DMA_RX_READY, &chip->flags))
+		dw_dma_cyclic_free(chip->dma.rx_chan);
+	return snd_pcm_lib_free_pages(substream);
+}
+
+static int atmel_ac97c_playback_prepare(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	unsigned long word = 0;
+	int retval;
+
+	/* assign channels to AC97C channel A */
+	switch (runtime->channels) {
+	case 1:
+		word |= AC97C_CH_ASSIGN(PCM_LEFT, A);
+		break;
+	case 2:
+		word |= AC97C_CH_ASSIGN(PCM_LEFT, A)
+			| AC97C_CH_ASSIGN(PCM_RIGHT, A);
+		break;
+	default:
+		/* TODO: support more than two channels */
+		return -EINVAL;
+		break;
+	}
+	ac97c_writel(chip, OCA, word);
+
+	/* configure sample format and size */
+	word = AC97C_CMR_DMAEN | AC97C_CMR_SIZE_16;
+
+	switch (runtime->format) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		word |= AC97C_CMR_CEM_LITTLE;
+		break;
+	case SNDRV_PCM_FORMAT_S16_BE: /* fall through */
+	default:
+		word &= ~(AC97C_CMR_CEM_LITTLE);
+		break;
+	}
+
+	ac97c_writel(chip, CAMR, word);
+
+	/* set variable rate if needed */
+	if (runtime->rate != 48000) {
+		word = ac97c_readl(chip, MR);
+		word |= AC97C_MR_VRA;
+		ac97c_writel(chip, MR, word);
+	} else {
+		word = ac97c_readl(chip, MR);
+		word &= ~(AC97C_MR_VRA);
+		ac97c_writel(chip, MR, word);
+	}
+
+	retval = snd_ac97_set_rate(chip->ac97, AC97_PCM_FRONT_DAC_RATE,
+			runtime->rate);
+	if (retval)
+		dev_dbg(&chip->pdev->dev, "could not set rate %d Hz\n",
+				runtime->rate);
+	return retval;
+}
+
+static int atmel_ac97c_capture_prepare(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	unsigned long word = 0;
+	int retval;
+
+	/* assign channels to AC97C channel A */
+	switch (runtime->channels) {
+	case 1:
+		word |= AC97C_CH_ASSIGN(PCM_LEFT, A);
+		break;
+	case 2:
+		word |= AC97C_CH_ASSIGN(PCM_LEFT, A)
+			| AC97C_CH_ASSIGN(PCM_RIGHT, A);
+		break;
+	default:
+		/* TODO: support more than two channels */
+		return -EINVAL;
+		break;
+	}
+	ac97c_writel(chip, ICA, word);
+
+	/* configure sample format and size */
+	word = AC97C_CMR_DMAEN | AC97C_CMR_SIZE_16;
+
+	switch (runtime->format) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		word |= AC97C_CMR_CEM_LITTLE;
+		break;
+	case SNDRV_PCM_FORMAT_S16_BE: /* fall through */
+	default:
+		word &= ~(AC97C_CMR_CEM_LITTLE);
+		break;
+	}
+
+	ac97c_writel(chip, CAMR, word);
+
+	/* set variable rate if needed */
+	if (runtime->rate != 48000) {
+		word = ac97c_readl(chip, MR);
+		word |= AC97C_MR_VRA;
+		ac97c_writel(chip, MR, word);
+	} else {
+		word = ac97c_readl(chip, MR);
+		word &= ~(AC97C_MR_VRA);
+		ac97c_writel(chip, MR, word);
+	}
+
+	retval = snd_ac97_set_rate(chip->ac97, AC97_PCM_LR_ADC_RATE,
+			runtime->rate);
+	if (retval)
+		dev_dbg(&chip->pdev->dev, "could not set rate %d Hz\n",
+				runtime->rate);
+	return retval;
+}
+
+static int
+atmel_ac97c_playback_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	unsigned long camr;
+	int retval = 0;
+
+	camr = ac97c_readl(chip, CAMR);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: /* fall through */
+	case SNDRV_PCM_TRIGGER_RESUME: /* fall through */
+	case SNDRV_PCM_TRIGGER_START:
+		retval = dw_dma_cyclic_start(chip->dma.tx_chan);
+		if (retval)
+			goto out;
+		camr |= AC97C_CMR_CENA;
+		break;
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: /* fall through */
+	case SNDRV_PCM_TRIGGER_SUSPEND: /* fall through */
+	case SNDRV_PCM_TRIGGER_STOP:
+		set_bit(DMA_TX_STOP, &chip->flags);
+		dw_dma_cyclic_stop(chip->dma.tx_chan);
+		if (chip->opened <= 1)
+			camr &= ~AC97C_CMR_CENA;
+		break;
+	default:
+		retval = -EINVAL;
+		goto out;
+	}
+
+	ac97c_writel(chip, CAMR, camr);
+out:
+	return retval;
+}
+
+static int
+atmel_ac97c_capture_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
+	unsigned long camr;
+	int retval = 0;
+
+	camr = ac97c_readl(chip, CAMR);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: /* fall through */
+	case SNDRV_PCM_TRIGGER_RESUME: /* fall through */
+	case SNDRV_PCM_TRIGGER_START:
+		dw_dma_cyclic_start(chip->dma.rx_chan);
+		if (retval)
+			goto out;
+		camr |= AC97C_CMR_CENA;
+		break;
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: /* fall through */
+	case SNDRV_PCM_TRIGGER_SUSPEND: /* fall through */
+	case SNDRV_PCM_TRIGGER_STOP:
+		set_bit(DMA_RX_STOP, &chip->flags);
+		dw_dma_cyclic_stop(chip->dma.rx_chan);
+		if (chip->opened <= 1)
+			camr &= ~AC97C_CMR_CENA;
+		break;
+	default:
+		retval = -EINVAL;
+		break;
+	}
+
+	ac97c_writel(chip, CAMR, camr);
+out:
+	return retval;
+}
+
+static snd_pcm_uframes_t
+atmel_ac97c_playback_pointer(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c	*chip = snd_pcm_substream_chip(substream);
+	struct snd_pcm_runtime	*runtime = substream->runtime;
+	snd_pcm_uframes_t	frames;
+	unsigned long		bytes;
+
+	bytes = dw_dma_get_src_addr(chip->dma.tx_chan);
+	bytes -= runtime->dma_addr;
+
+	frames = bytes_to_frames(runtime, bytes);
+	if (frames >= runtime->buffer_size)
+		frames -= runtime->buffer_size;
+	return frames;
+}
+
+static snd_pcm_uframes_t
+atmel_ac97c_capture_pointer(struct snd_pcm_substream *substream)
+{
+	struct atmel_ac97c	*chip = snd_pcm_substream_chip(substream);
+	struct snd_pcm_runtime	*runtime = substream->runtime;
+	snd_pcm_uframes_t	frames;
+	unsigned long		bytes;
+
+	bytes = dw_dma_get_dst_addr(chip->dma.rx_chan);
+	bytes -= runtime->dma_addr;
+
+	frames = bytes_to_frames(runtime, bytes);
+	if (frames >= runtime->buffer_size)
+		frames -= runtime->buffer_size;
+	return frames;
+}
+
+static struct snd_pcm_ops atmel_ac97_playback_ops = {
+	.open		= atmel_ac97c_playback_open,
+	.close		= atmel_ac97c_playback_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= atmel_ac97c_playback_hw_params,
+	.hw_free	= atmel_ac97c_playback_hw_free,
+	.prepare	= atmel_ac97c_playback_prepare,
+	.trigger	= atmel_ac97c_playback_trigger,
+	.pointer	= atmel_ac97c_playback_pointer,
+};
+
+static struct snd_pcm_ops atmel_ac97_capture_ops = {
+	.open		= atmel_ac97c_capture_open,
+	.close		= atmel_ac97c_capture_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= atmel_ac97c_capture_hw_params,
+	.hw_free	= atmel_ac97c_capture_hw_free,
+	.prepare	= atmel_ac97c_capture_prepare,
+	.trigger	= atmel_ac97c_capture_trigger,
+	.pointer	= atmel_ac97c_capture_pointer,
+};
+
+static struct ac97_pcm atmel_ac97_pcm_defs[] __devinitdata = {
+	/* Playback */
+	{
+		.exclusive = 1,
+		.r = { {
+			.slots = ((1 << AC97_SLOT_PCM_LEFT)
+					| (1 << AC97_SLOT_PCM_RIGHT)
+					| (1 << AC97_SLOT_PCM_CENTER)
+					| (1 << AC97_SLOT_PCM_SLEFT)
+					| (1 << AC97_SLOT_PCM_SRIGHT)
+					| (1 << AC97_SLOT_LFE)),
+		} }
+	},
+	/* PCM in */
+	{
+		.stream = 1,
+		.exclusive = 1,
+		.r = { {
+			.slots = ((1 << AC97_SLOT_PCM_LEFT)
+					| (1 << AC97_SLOT_PCM_RIGHT)),
+		} }
+	},
+	/* Mic in */
+	{
+		.stream = 1,
+		.exclusive = 1,
+		.r = { {
+			.slots = (1 << AC97_SLOT_MIC),
+		} }
+	},
+};
+
+static int __devinit atmel_ac97c_pcm_new(struct atmel_ac97c *chip)
+{
+	struct snd_pcm *pcm;
+	int capture = test_bit(DMA_RX_CHAN_PRESENT, &chip->flags);
+	int playback = test_bit(DMA_TX_CHAN_PRESENT, &chip->flags);
+	int err;
+
+	err = snd_ac97_pcm_assign(chip->ac97_bus,
+			ARRAY_SIZE(atmel_ac97_pcm_defs),
+			atmel_ac97_pcm_defs);
+	if (err)
+		return err;
+
+	err = snd_pcm_new(chip->card, "Atmel AC97C", chip->pdev->id,
+			playback, capture, &pcm);
+	if (err)
+		return err;
+
+	if (capture)
+		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE,
+				&atmel_ac97_capture_ops);
+
+	if (playback)
+		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK,
+				&atmel_ac97_playback_ops);
+
+	snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV,
+			&chip->pdev->dev,
+			128 * 1024, 128 * 1024);
+
+	pcm->private_data = chip;
+	pcm->info_flags = 0;
+	strcpy(pcm->name, "Atmel AC97C");
+	chip->pcm = pcm;
+
+	return 0;
+}
+
+static int atmel_ac97c_mixer_new(struct atmel_ac97c *chip)
+{
+	struct snd_ac97_template template;
+	memset(&template, 0, sizeof(template));
+	template.private_data = chip;
+	return snd_ac97_mixer(chip->ac97_bus, &template, &chip->ac97);
+}
+
+static void atmel_ac97c_write(struct snd_ac97 *ac97, unsigned short reg,
+		unsigned short val)
+{
+	struct atmel_ac97c *chip = get_chip(ac97);
+	unsigned long word;
+	int timeout = 40;
+
+	word = (reg & 0x7f) << 16 | val;
+
+	do {
+		if (ac97c_readl(chip, COSR) & AC97C_CSR_TXRDY) {
+			ac97c_writel(chip, COTHR, word);
+			return;
+		}
+		udelay(1);
+	} while (--timeout);
+
+	dev_dbg(&chip->pdev->dev, "codec write timeout\n");
+}
+
+static unsigned short atmel_ac97c_read(struct snd_ac97 *ac97,
+		unsigned short reg)
+{
+	struct atmel_ac97c *chip = get_chip(ac97);
+	unsigned long word;
+	int timeout = 40;
+	int write = 10;
+
+	word = (0x80 | (reg & 0x7f)) << 16;
+
+	if ((ac97c_readl(chip, COSR) & AC97C_CSR_RXRDY) != 0)
+		ac97c_readl(chip, CORHR);
+
+retry_write:
+	timeout = 40;
+
+	do {
+		if ((ac97c_readl(chip, COSR) & AC97C_CSR_TXRDY) != 0) {
+			ac97c_writel(chip, COTHR, word);
+			goto read_reg;
+		}
+		udelay(10);
+	} while (--timeout);
+
+	if (!--write)
+		goto timed_out;
+	goto retry_write;
+
+read_reg:
+	do {
+		if ((ac97c_readl(chip, COSR) & AC97C_CSR_RXRDY) != 0) {
+			unsigned short val = ac97c_readl(chip, CORHR);
+			return val;
+		}
+		udelay(10);
+	} while (--timeout);
+
+	if (!--write)
+		goto timed_out;
+	goto retry_write;
+
+timed_out:
+	dev_dbg(&chip->pdev->dev, "codec read timeout\n");
+	return 0xffff;
+}
+
+static bool filter(struct dma_chan *chan, void *slave)
+{
+	struct dw_dma_slave *dws = slave;
+
+	if (dws->dma_dev == chan->device->dev) {
+		chan->private = dws;
+		return true;
+	} else
+		return false;
+}
+
+static void atmel_ac97c_reset(struct atmel_ac97c *chip)
+{
+	ac97c_writel(chip, MR, AC97C_MR_WRST);
+
+	if (gpio_is_valid(chip->reset_pin)) {
+		gpio_set_value(chip->reset_pin, 0);
+		/* AC97 v2.2 specifications says minimum 1 us. */
+		udelay(10);
+		gpio_set_value(chip->reset_pin, 1);
+	}
+
+	udelay(1);
+	ac97c_writel(chip, MR, AC97C_MR_ENA);
+}
+
+static int __devinit atmel_ac97c_probe(struct platform_device *pdev)
+{
+	struct snd_card			*card;
+	struct atmel_ac97c		*chip;
+	struct resource			*regs;
+	struct ac97c_platform_data	*pdata;
+	struct clk			*pclk;
+	static struct snd_ac97_bus_ops	ops = {
+		.write	= atmel_ac97c_write,
+		.read	= atmel_ac97c_read,
+	};
+	int				retval;
+
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!regs) {
+		dev_dbg(&pdev->dev, "no memory resource\n");
+		return -ENXIO;
+	}
+
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		dev_dbg(&pdev->dev, "no platform data\n");
+		return -ENXIO;
+	}
+
+	pclk = clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(pclk)) {
+		dev_dbg(&pdev->dev, "no peripheral clock\n");
+		return PTR_ERR(pclk);
+	}
+	clk_enable(pclk);
+
+	mutex_init(&opened_mutex);
+
+	retval = -ENOMEM;
+	card = snd_card_new(SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
+			THIS_MODULE, sizeof(struct atmel_ac97c));
+	if (!card) {
+		dev_dbg(&pdev->dev, "could not allocate memory\n");
+		goto err_snd_card_new;
+	}
+
+	chip = get_chip(card);
+
+	spin_lock_init(&chip->lock);
+
+	chip->card = card;
+	chip->pclk = pclk;
+	chip->pdev = pdev;
+	chip->regs = ioremap(regs->start, regs->end - regs->start + 1);
+
+	if (!chip->regs) {
+		dev_dbg(&pdev->dev, "could not remap register memory\n");
+		goto err_ioremap;
+	}
+
+	if (gpio_is_valid(pdata->reset_pin)) {
+		if (gpio_request(pdata->reset_pin, chip->card->shortname)) {
+			dev_dbg(&pdev->dev, "reset pin not available\n");
+			chip->reset_pin = -ENODEV;
+		} else {
+			gpio_direction_output(pdata->reset_pin, 1);
+			chip->reset_pin = pdata->reset_pin;
+		}
+	}
+
+	snd_card_set_dev(card, &pdev->dev);
+
+	retval = snd_ac97_bus(card, 0, &ops, chip, &chip->ac97_bus);
+	if (retval) {
+		dev_dbg(&pdev->dev, "could not register on ac97 bus\n");
+		goto err_ac97_bus;
+	}
+
+	atmel_ac97c_reset(chip);
+
+	retval = atmel_ac97c_mixer_new(chip);
+	if (retval) {
+		dev_dbg(&pdev->dev, "could not register ac97 mixer\n");
+		goto err_ac97_bus;
+	}
+
+	if (pdata->rx_dws.dma_dev) {
+		struct dw_dma_slave *dws = &pdata->rx_dws;
+		dma_cap_mask_t mask;
+
+		dws->rx_reg = regs->start + AC97C_CARHR + 2;
+
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+
+		chip->dma.rx_chan = dma_request_channel(mask, filter, dws);
+
+		dev_info(&chip->pdev->dev, "using %s for DMA RX\n",
+					chip->dma.rx_chan->dev->device.bus_id);
+		set_bit(DMA_RX_CHAN_PRESENT, &chip->flags);
+	}
+
+	if (pdata->tx_dws.dma_dev) {
+		struct dw_dma_slave *dws = &pdata->tx_dws;
+		dma_cap_mask_t mask;
+
+		dws->tx_reg = regs->start + AC97C_CATHR + 2;
+
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+
+		chip->dma.tx_chan = dma_request_channel(mask, filter, dws);
+
+		dev_info(&chip->pdev->dev, "using %s for DMA TX\n",
+					chip->dma.tx_chan->dev->device.bus_id);
+		set_bit(DMA_TX_CHAN_PRESENT, &chip->flags);
+	}
+
+	if (!test_bit(DMA_RX_CHAN_PRESENT, &chip->flags) &&
+			!test_bit(DMA_TX_CHAN_PRESENT, &chip->flags)) {
+		dev_dbg(&pdev->dev, "DMA not available\n");
+		retval = -ENODEV;
+		goto err_dma;
+	}
+
+	retval = atmel_ac97c_pcm_new(chip);
+	if (retval) {
+		dev_dbg(&pdev->dev, "could not register ac97 pcm device\n");
+		goto err_dma;
+	}
+
+	strcpy(card->driver, "Atmel AC97C");
+	strcpy(card->shortname, "Atmel AC97C");
+	sprintf(card->longname, "Atmel AC97 Controller");
+
+	retval = snd_card_register(card);
+	if (retval) {
+		dev_dbg(&pdev->dev, "could not register sound card\n");
+		goto err_ac97_bus;
+	}
+
+	platform_set_drvdata(pdev, card);
+
+	dev_info(&pdev->dev, "Atmel AC97 controller at 0x%p\n",
+			chip->regs);
+
+	return retval;
+
+err_dma:
+	if (test_bit(DMA_RX_CHAN_PRESENT, &chip->flags))
+		dma_release_channel(chip->dma.rx_chan);
+	if (test_bit(DMA_TX_CHAN_PRESENT, &chip->flags))
+		dma_release_channel(chip->dma.tx_chan);
+	clear_bit(DMA_RX_CHAN_PRESENT, &chip->flags);
+	clear_bit(DMA_TX_CHAN_PRESENT, &chip->flags);
+	chip->dma.rx_chan = NULL;
+	chip->dma.tx_chan = NULL;
+err_ac97_bus:
+	snd_card_set_dev(card, NULL);
+
+	if (gpio_is_valid(chip->reset_pin))
+		gpio_free(chip->reset_pin);
+
+	iounmap(chip->regs);
+err_ioremap:
+	snd_card_free(card);
+err_snd_card_new:
+	clk_disable(pclk);
+	clk_put(pclk);
+	return retval;
+}
+
+#ifdef CONFIG_PM
+static int atmel_ac97c_suspend(struct platform_device *pdev, pm_message_t msg)
+{
+	struct snd_card *card = platform_get_drvdata(pdev);
+	struct atmel_ac97c *chip = card->private_data;
+
+	if (test_bit(DMA_RX_READY, &chip->flags))
+		dw_dma_cyclic_stop(chip->dma.rx_chan);
+	if (test_bit(DMA_TX_READY, &chip->flags))
+		dw_dma_cyclic_stop(chip->dma.tx_chan);
+	clk_disable(chip->pclk);
+
+	return 0;
+}
+
+static int atmel_ac97c_resume(struct platform_device *pdev)
+{
+	struct snd_card *card = platform_get_drvdata(pdev);
+	struct atmel_ac97c *chip = card->private_data;
+
+	clk_enable(chip->pclk);
+	if (test_bit(DMA_RX_READY, &chip->flags))
+		dw_dma_cyclic_start(chip->dma.rx_chan);
+	if (test_bit(DMA_TX_READY, &chip->flags))
+		dw_dma_cyclic_start(chip->dma.tx_chan);
+
+	return 0;
+}
+#else
+#define atmel_ac97c_suspend NULL
+#define atmel_ac97c_resume NULL
+#endif
+
+static int __devexit atmel_ac97c_remove(struct platform_device *pdev)
+{
+	struct snd_card *card = platform_get_drvdata(pdev);
+	struct atmel_ac97c *chip = get_chip(card);
+
+	if (gpio_is_valid(chip->reset_pin))
+		gpio_free(chip->reset_pin);
+
+	clk_disable(chip->pclk);
+	clk_put(chip->pclk);
+	iounmap(chip->regs);
+
+	if (test_bit(DMA_RX_CHAN_PRESENT, &chip->flags))
+		dma_release_channel(chip->dma.rx_chan);
+	if (test_bit(DMA_TX_CHAN_PRESENT, &chip->flags))
+		dma_release_channel(chip->dma.tx_chan);
+	clear_bit(DMA_RX_CHAN_PRESENT, &chip->flags);
+	clear_bit(DMA_TX_CHAN_PRESENT, &chip->flags);
+	chip->dma.rx_chan = NULL;
+	chip->dma.tx_chan = NULL;
+
+	snd_card_set_dev(card, NULL);
+	snd_card_free(card);
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver atmel_ac97c_driver = {
+	.remove		= __devexit_p(atmel_ac97c_remove),
+	.driver		= {
+		.name	= "atmel_ac97c",
+	},
+	.suspend	= atmel_ac97c_suspend,
+	.resume		= atmel_ac97c_resume,
+};
+
+static int __init atmel_ac97c_init(void)
+{
+	return platform_driver_probe(&atmel_ac97c_driver,
+			atmel_ac97c_probe);
+}
+module_init(atmel_ac97c_init);
+
+static void __exit atmel_ac97c_exit(void)
+{
+	platform_driver_unregister(&atmel_ac97c_driver);
+}
+module_exit(atmel_ac97c_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Driver for Atmel AC97 Controller");
+MODULE_AUTHOR("Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>");
diff --git a/sound/avr32/ac97c.h b/sound/avr32/ac97c.h
new file mode 100644
index 0000000..ac49a4d
--- /dev/null
+++ b/sound/avr32/ac97c.h
@@ -0,0 +1,71 @@
+/*
+ * Register definitions for the Atmel AC97C Controller.
+ *
+ * Copyright (C) 2005-2009 Atmel Corporation
+ *
+ * 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.
+ */
+#ifndef __SOUND_AVR32_AC97C_H
+#define __SOUND_AVR32_AC97C_H
+
+#define AC97C_MR		0x08
+#define AC97C_ICA		0x10
+#define AC97C_OCA		0x14
+#define AC97C_CARHR		0x20
+#define AC97C_CATHR		0x24
+#define AC97C_CASR		0x28
+#define AC97C_CAMR		0x2c
+#define AC97C_CBRHR		0x30
+#define AC97C_CBTHR		0x34
+#define AC97C_CBSR		0x38
+#define AC97C_CBMR		0x3c
+#define AC97C_CORHR		0x40
+#define AC97C_COTHR		0x44
+#define AC97C_COSR		0x48
+#define AC97C_COMR		0x4c
+#define AC97C_SR		0x50
+#define AC97C_IER		0x54
+#define AC97C_IDR		0x58
+#define AC97C_IMR		0x5c
+#define AC97C_VERSION		0xfc
+
+#define AC97C_CATPR		PDC_TPR
+#define AC97C_CATCR		PDC_TCR
+#define AC97C_CATNPR		PDC_TNPR
+#define AC97C_CATNCR		PDC_TNCR
+#define AC97C_CARPR		PDC_RPR
+#define AC97C_CARCR		PDC_RCR
+#define AC97C_CARNPR		PDC_RNPR
+#define AC97C_CARNCR		PDC_RNCR
+#define AC97C_PTCR		PDC_PTCR
+
+#define AC97C_MR_ENA		(1 << 0)
+#define AC97C_MR_WRST		(1 << 1)
+#define AC97C_MR_VRA		(1 << 2)
+
+#define AC97C_CSR_TXRDY		(1 << 0)
+#define AC97C_CSR_UNRUN		(1 << 2)
+#define AC97C_CSR_RXRDY		(1 << 4)
+#define AC97C_CSR_ENDTX		(1 << 10)
+#define AC97C_CSR_ENDRX		(1 << 14)
+
+#define AC97C_CMR_SIZE_20	(0 << 16)
+#define AC97C_CMR_SIZE_18	(1 << 16)
+#define AC97C_CMR_SIZE_16	(2 << 16)
+#define AC97C_CMR_SIZE_10	(3 << 16)
+#define AC97C_CMR_CEM_LITTLE	(1 << 18)
+#define AC97C_CMR_CEM_BIG	(0 << 18)
+#define AC97C_CMR_CENA		(1 << 21)
+#define AC97C_CMR_DMAEN		(1 << 22)
+
+#define AC97C_SR_CAEVT		(1 << 3)
+
+#define AC97C_CH_ASSIGN(slot, channel)					\
+	(AC97C_CHANNEL_##channel << (3 * (AC97_SLOT_##slot - 3)))
+#define AC97C_CHANNEL_NONE	0x0
+#define AC97C_CHANNEL_A		0x1
+#define AC97C_CHANNEL_B		0x2
+
+#endif /* __SOUND_AVR32_AC97C_H */
-- 
1.5.6.3

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC
  2009-02-04 11:48   ` [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC Hans-Christian Egtvedt
  2009-02-04 11:48     ` [PATCH 3/3] Add ALSA driver for Atmel AC97 controller Hans-Christian Egtvedt
@ 2009-02-04 12:00     ` Takashi Iwai
  2009-02-04 12:18       ` Hans-Christian Egtvedt
  1 sibling, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2009-02-04 12:00 UTC (permalink / raw)
  To: Hans-Christian Egtvedt; +Cc: alsa-devel

At Wed,  4 Feb 2009 12:48:33 +0100,
Hans-Christian Egtvedt wrote:
> 
> +static int at32_abdac_hw_params(struct snd_pcm_substream *substream,
> +		struct snd_pcm_hw_params *hw_params)
> +{
> +	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
> +	int retval;
> +
> +	retval = snd_pcm_lib_malloc_pages(substream,
> +			params_buffer_bytes(hw_params));
> +	if (retval)
> +		return retval;

snd_pcm_lib_malloc_pages() returns 1 if the buffer is changed
(e.g. reallocation).  So, this check should be if (retval < 0).

> +static int at32_abdac_trigger(struct snd_pcm_substream *substream, int cmd)
> +{
> +	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
> +	int retval = -EINVAL;
> +
> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: /* fall through */
> +	case SNDRV_PCM_TRIGGER_RESUME: /* fall through */
> +	case SNDRV_PCM_TRIGGER_START:
> +		clk_enable(dac->sample_clk);
> +		retval = dw_dma_cyclic_start(dac->dma.chan);
> +		if (retval)
> +			goto out;
> +		dac_writel(dac, CTRL, DAC_BIT(EN));
> +		break;
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: /* fall through */
> +	case SNDRV_PCM_TRIGGER_SUSPEND: /* fall through */
> +	case SNDRV_PCM_TRIGGER_STOP:
> +		set_bit(DMA_STOP, &dac->flags);
> +		dw_dma_cyclic_stop(dac->dma.chan);
> +		dac_writel(dac, DATA, 0);
> +		dac_writel(dac, CTRL, 0);
> +		clk_disable(dac->sample_clk);
> +		break;

No retval is set for these cases?

> +static int __devinit at32_abdac_probe(struct platform_device *pdev)
(snip)
> +	retval = -ENOMEM;
> +	card = snd_card_new(SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
> +			THIS_MODULE, sizeof(struct at32_abdac));

The recent version has a newer API, snd_card_create().  This function
gives the error code.  Please use the new function instead.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 11:48     ` [PATCH 3/3] Add ALSA driver for Atmel AC97 controller Hans-Christian Egtvedt
@ 2009-02-04 12:00       ` Mark Brown
  2009-02-04 12:15         ` Hans-Christian Egtvedt
  2009-02-04 12:09       ` Takashi Iwai
  2009-02-05  7:14       ` Hans-Christian Egtvedt
  2 siblings, 1 reply; 24+ messages in thread
From: Mark Brown @ 2009-02-04 12:00 UTC (permalink / raw)
  To: Hans-Christian Egtvedt; +Cc: Sedji Gaouaou, alsa-devel

On Wed, Feb 04, 2009 at 12:48:34PM +0100, Hans-Christian Egtvedt wrote:
> This patch adds ALSA support for the AC97 controller found on Atmel
> AVR32 devices.

> Tested on ATSTK1006 + ATSTK1000 with a development board with a AC97
> codec.

> Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>

Just glancing over the code here this looks very like the code for the
AT91 AC97 controller which Sedji (CCed) has posted in the past.  Is it
possible to share the same driver for both chips, like we're doing with
the ASoC drivers for the two architectures?  It'd make life easier if
that were possible and I know there's a lot of people interested in the
AT91 support for mainline.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] Add AVR32 ALSA drivers directory.
  2009-02-04 11:48 ` [PATCH 1/3] Add AVR32 ALSA drivers directory Hans-Christian Egtvedt
  2009-02-04 11:48   ` [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC Hans-Christian Egtvedt
@ 2009-02-04 12:02   ` Takashi Iwai
  2009-02-04 12:11     ` Hans-Christian Egtvedt
  1 sibling, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2009-02-04 12:02 UTC (permalink / raw)
  To: Hans-Christian Egtvedt
  Cc: alsa-devel, Haavard Skinnemoen, Hans-Christian Egtvedt

At Wed,  4 Feb 2009 12:48:32 +0100,
Hans-Christian Egtvedt wrote:
> 
> From: Hans-Christian Egtvedt <hcegtvedt@atmel.com>
> 
> Signed-off-by: Hans-Christian Egtvedt <hcegtvedt@atmel.com>
> Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>

Usually this kind of change comes as the last one, in order to avoid
the build error.


Takashi

> ---
>  sound/Kconfig  |    2 ++
>  sound/Makefile |    2 +-
>  2 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/Kconfig b/sound/Kconfig
> index 200aca1..0a5fbb3 100644
> --- a/sound/Kconfig
> +++ b/sound/Kconfig
> @@ -60,6 +60,8 @@ source "sound/aoa/Kconfig"
>  
>  source "sound/arm/Kconfig"
>  
> +source "sound/avr32/Kconfig"
> +
>  source "sound/spi/Kconfig"
>  
>  source "sound/mips/Kconfig"
> diff --git a/sound/Makefile b/sound/Makefile
> index c76d707..a52b236 100644
> --- a/sound/Makefile
> +++ b/sound/Makefile
> @@ -6,7 +6,7 @@ obj-$(CONFIG_SOUND_PRIME) += sound_firmware.o
>  obj-$(CONFIG_SOUND_PRIME) += oss/
>  obj-$(CONFIG_DMASOUND) += oss/
>  obj-$(CONFIG_SND) += core/ i2c/ drivers/ isa/ pci/ ppc/ arm/ sh/ synth/ usb/ \
> -	sparc/ spi/ parisc/ pcmcia/ mips/ soc/
> +	sparc/ spi/ parisc/ pcmcia/ mips/ soc/ avr32/
>  obj-$(CONFIG_SND_AOA) += aoa/
>  
>  # This one must be compilable even if sound is configured out
> -- 
> 1.5.6.3
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 11:48     ` [PATCH 3/3] Add ALSA driver for Atmel AC97 controller Hans-Christian Egtvedt
  2009-02-04 12:00       ` Mark Brown
@ 2009-02-04 12:09       ` Takashi Iwai
  2009-02-04 12:16         ` Hans-Christian Egtvedt
  2009-02-04 15:02         ` Hans-Christian Egtvedt
  2009-02-05  7:14       ` Hans-Christian Egtvedt
  2 siblings, 2 replies; 24+ messages in thread
From: Takashi Iwai @ 2009-02-04 12:09 UTC (permalink / raw)
  To: Hans-Christian Egtvedt; +Cc: alsa-devel

At Wed,  4 Feb 2009 12:48:34 +0100,
Hans-Christian Egtvedt wrote:
> 
> This patch adds ALSA support for the AC97 controller found on Atmel
> AVR32 devices.
> 
> Tested on ATSTK1006 + ATSTK1000 with a development board with a AC97
> codec.
> 
> Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
> ---
>  include/sound/atmel-ac97c.h |   40 ++
>  sound/avr32/Kconfig         |    8 +
>  sound/avr32/Makefile        |    2 +
>  sound/avr32/ac97c.c         |  994 +++++++++++++++++++++++++++++++++++++++++++
>  sound/avr32/ac97c.h         |   71 +++
>  5 files changed, 1115 insertions(+), 0 deletions(-)
>  create mode 100644 include/sound/atmel-ac97c.h
>  create mode 100644 sound/avr32/ac97c.c
>  create mode 100644 sound/avr32/ac97c.h
> 
> diff --git a/include/sound/atmel-ac97c.h b/include/sound/atmel-ac97c.h
> new file mode 100644
> index 0000000..d5daddd
> --- /dev/null
> +++ b/include/sound/atmel-ac97c.h

Is it needed to be in include/sound directory?
If it's just for a local driver, better to put in the local directory.

> diff --git a/sound/avr32/ac97c.c b/sound/avr32/ac97c.c
> new file mode 100644
> index 0000000..218d666
> --- /dev/null
> +++ b/sound/avr32/ac97c.c
> @@ -0,0 +1,994 @@
...
> +static int atmel_ac97c_playback_hw_params(struct snd_pcm_substream *substream,
> +		struct snd_pcm_hw_params *hw_params)
> +{
> +	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
> +	int retval;
> +
> +	retval = snd_pcm_lib_malloc_pages(substream,
> +					params_buffer_bytes(hw_params));
> +	if (retval)
> +		return retval;

Should be a negative-check.

> +static int atmel_ac97c_capture_hw_params(struct snd_pcm_substream *substream,
> +		struct snd_pcm_hw_params *hw_params)
> +{
> +	struct atmel_ac97c *chip = snd_pcm_substream_chip(substream);
> +	int retval;
> +
> +	retval = snd_pcm_lib_malloc_pages(substream,
> +					params_buffer_bytes(hw_params));
> +	if (retval)
> +		return retval;

Ditto.

> +static int __devinit atmel_ac97c_probe(struct platform_device *pdev)
...
> +	mutex_init(&opened_mutex);

This is superfluous.

> +
> +	retval = -ENOMEM;
> +	card = snd_card_new(SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
> +			THIS_MODULE, sizeof(struct atmel_ac97c));

Use snd_card_create().


thanks,

Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] Add AVR32 ALSA drivers directory.
  2009-02-04 12:02   ` [PATCH 1/3] Add AVR32 ALSA drivers directory Takashi Iwai
@ 2009-02-04 12:11     ` Hans-Christian Egtvedt
  0 siblings, 0 replies; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 12:11 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, Haavard Skinnemoen, Hans-Christian Egtvedt

On Wed, 04 Feb 2009 13:02:13 +0100
Takashi Iwai <tiwai@suse.de> wrote:

> At Wed,  4 Feb 2009 12:48:32 +0100,
> Hans-Christian Egtvedt wrote:
> > 
> > From: Hans-Christian Egtvedt <hcegtvedt@atmel.com>
> > 
> > Signed-off-by: Hans-Christian Egtvedt <hcegtvedt@atmel.com>
> > Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
> 
> Usually this kind of change comes as the last one, in order to avoid
> the build error.
> 

Ah, yes, of course. I will put this last after brushing up the drivers
from feedback.

-- 
Best regards,
Hans-Christian Egtvedt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 12:00       ` Mark Brown
@ 2009-02-04 12:15         ` Hans-Christian Egtvedt
  2009-02-04 12:51           ` Sedji Gaouaou
  0 siblings, 1 reply; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 12:15 UTC (permalink / raw)
  To: Mark Brown; +Cc: Sedji Gaouaou, alsa-devel

On Wed, 4 Feb 2009 12:00:52 +0000
Mark Brown <broonie@sirena.org.uk> wrote:

> On Wed, Feb 04, 2009 at 12:48:34PM +0100, Hans-Christian Egtvedt
> wrote:
> > This patch adds ALSA support for the AC97 controller found on Atmel
> > AVR32 devices.
> 
> > Tested on ATSTK1006 + ATSTK1000 with a development board with a AC97
> > codec.
> 
> > Signed-off-by: Hans-Christian Egtvedt
> > <hans-christian.egtvedt@atmel.com>
> 
> Just glancing over the code here this looks very like the code for the
> AT91 AC97 controller which Sedji (CCed) has posted in the past.  Is it
> possible to share the same driver for both chips, like we're doing
> with the ASoC drivers for the two architectures?  It'd make life
> easier if that were possible and I know there's a lot of people
> interested in the AT91 support for mainline.

Yes, it should be possible to share the AC97C driver between AT91 and
AVR32. Although this AC97C driver uses the DesignWare DMAC controller
(drivers/dma/dw_dmac.c).

I do not remember which DMA interface AT91 devices uses.

Should the driver go into sound/drivers then? And renamed to
atmel-ac97c.{c,h} for example.

-- 
Best regards,
Hans-Christian Egtvedt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 12:09       ` Takashi Iwai
@ 2009-02-04 12:16         ` Hans-Christian Egtvedt
  2009-02-04 15:02         ` Hans-Christian Egtvedt
  1 sibling, 0 replies; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 12:16 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On Wed, 04 Feb 2009 13:09:37 +0100
Takashi Iwai <tiwai@suse.de> wrote:

> At Wed,  4 Feb 2009 12:48:34 +0100,
> Hans-Christian Egtvedt wrote:

<snipp>

> > diff --git a/include/sound/atmel-ac97c.h
> > b/include/sound/atmel-ac97c.h new file mode 100644
> > index 0000000..d5daddd
> > --- /dev/null
> > +++ b/include/sound/atmel-ac97c.h
> 
> Is it needed to be in include/sound directory?
> If it's just for a local driver, better to put in the local directory.
> 

Yes, the header must be available for the boards so they can see the
platform data struct. This is how the DMA slave and GPIO for reset line
is passed to the AC97C driver.

> > diff --git a/sound/avr32/ac97c.c b/sound/avr32/ac97c.c
> > new file mode 100644
> > index 0000000..218d666
> > --- /dev/null
> > +++ b/sound/avr32/ac97c.c
> > @@ -0,0 +1,994 @@
> ...
> > +static int atmel_ac97c_playback_hw_params(struct snd_pcm_substream
> > *substream,
> > +		struct snd_pcm_hw_params *hw_params)
> > +{
> > +	struct atmel_ac97c *chip =
> > snd_pcm_substream_chip(substream);
> > +	int retval;
> > +
> > +	retval = snd_pcm_lib_malloc_pages(substream,
> > +
> > params_buffer_bytes(hw_params));
> > +	if (retval)
> > +		return retval;
> 
> Should be a negative-check.
> 

OK

> > +static int atmel_ac97c_capture_hw_params(struct snd_pcm_substream
> > *substream,
> > +		struct snd_pcm_hw_params *hw_params)
> > +{
> > +	struct atmel_ac97c *chip =
> > snd_pcm_substream_chip(substream);
> > +	int retval;
> > +
> > +	retval = snd_pcm_lib_malloc_pages(substream,
> > +
> > params_buffer_bytes(hw_params));
> > +	if (retval)
> > +		return retval;
> 
> Ditto.
> 

OK

> > +static int __devinit atmel_ac97c_probe(struct platform_device
> > *pdev)
> ...
> > +	mutex_init(&opened_mutex);
> 
> This is superfluous.
> 
> > +
> > +	retval = -ENOMEM;
> > +	card = snd_card_new(SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
> > +			THIS_MODULE, sizeof(struct atmel_ac97c));
> 
> Use snd_card_create().
> 

OK, I'll convert to using this for both drivers.

-- 
Best regards,
Hans-Christian Egtvedt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC
  2009-02-04 12:00     ` [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC Takashi Iwai
@ 2009-02-04 12:18       ` Hans-Christian Egtvedt
  2009-02-04 12:52         ` Takashi Iwai
  0 siblings, 1 reply; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 12:18 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On Wed, 04 Feb 2009 13:00:42 +0100
Takashi Iwai <tiwai@suse.de> wrote:

> At Wed,  4 Feb 2009 12:48:33 +0100,
> Hans-Christian Egtvedt wrote:
> > 
> > +static int at32_abdac_hw_params(struct snd_pcm_substream
> > *substream,
> > +		struct snd_pcm_hw_params *hw_params)
> > +{
> > +	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
> > +	int retval;
> > +
> > +	retval = snd_pcm_lib_malloc_pages(substream,
> > +			params_buffer_bytes(hw_params));
> > +	if (retval)
> > +		return retval;
> 
> snd_pcm_lib_malloc_pages() returns 1 if the buffer is changed
> (e.g. reallocation).  So, this check should be if (retval < 0).
> 

Ah, okay, I'll fix this.

> > +static int at32_abdac_trigger(struct snd_pcm_substream *substream,
> > int cmd) +{
> > +	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
> > +	int retval = -EINVAL;
> > +
> > +	switch (cmd) {
> > +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: /* fall through */
> > +	case SNDRV_PCM_TRIGGER_RESUME: /* fall through */
> > +	case SNDRV_PCM_TRIGGER_START:
> > +		clk_enable(dac->sample_clk);
> > +		retval = dw_dma_cyclic_start(dac->dma.chan);
> > +		if (retval)
> > +			goto out;
> > +		dac_writel(dac, CTRL, DAC_BIT(EN));
> > +		break;
> > +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: /* fall through */
> > +	case SNDRV_PCM_TRIGGER_SUSPEND: /* fall through */
> > +	case SNDRV_PCM_TRIGGER_STOP:
> > +		set_bit(DMA_STOP, &dac->flags);
> > +		dw_dma_cyclic_stop(dac->dma.chan);
> > +		dac_writel(dac, DATA, 0);
> > +		dac_writel(dac, CTRL, 0);
> > +		clk_disable(dac->sample_clk);
> > +		break;
> 
> No retval is set for these cases?
> 

Nope, they are all void return functions and will always complete
successfully.

> > +static int __devinit at32_abdac_probe(struct platform_device *pdev)
> (snip)
> > +	retval = -ENOMEM;
> > +	card = snd_card_new(SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
> > +			THIS_MODULE, sizeof(struct at32_abdac));
> 
> The recent version has a newer API, snd_card_create().  This function
> gives the error code.  Please use the new function instead.
> 

Okay, no problem, will redo with the new API.

-- 
Best regards,
Hans-Christian Egtvedt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 12:15         ` Hans-Christian Egtvedt
@ 2009-02-04 12:51           ` Sedji Gaouaou
  2009-02-04 13:01             ` Mark Brown
  2009-02-04 13:03             ` Hans-Christian Egtvedt
  0 siblings, 2 replies; 24+ messages in thread
From: Sedji Gaouaou @ 2009-02-04 12:51 UTC (permalink / raw)
  To: Hans-Christian Egtvedt; +Cc: alsa-devel, Mark Brown

Hi,
Hans-Christian Egtvedt a écrit :
> Yes, it should be possible to share the AC97C driver between AT91 and
> AVR32. Although this AC97C driver uses the DesignWare DMAC controller
> (drivers/dma/dw_dmac.c).
> 
> I do not remember which DMA interface AT91 devices uses.
> 
> Should the driver go into sound/drivers then? And renamed to
> atmel-ac97c.{c,h} for example.
> 
Actually there are some differences, AT91 does not use a DMA, we use a 
specific IP(PDC) to transer the data...

Maybe we could share the same code we some #define in it?

Regards,
Sedji

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC
  2009-02-04 12:18       ` Hans-Christian Egtvedt
@ 2009-02-04 12:52         ` Takashi Iwai
  2009-02-04 12:58           ` Hans-Christian Egtvedt
  0 siblings, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2009-02-04 12:52 UTC (permalink / raw)
  To: Hans-Christian Egtvedt; +Cc: alsa-devel

At Wed, 4 Feb 2009 13:18:35 +0100,
Hans-Christian Egtvedt wrote:
> 
> > > +static int at32_abdac_trigger(struct snd_pcm_substream *substream,
> > > int cmd) +{
> > > +	struct at32_abdac *dac = snd_pcm_substream_chip(substream);
> > > +	int retval = -EINVAL;
> > > +
> > > +	switch (cmd) {
> > > +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: /* fall through */
> > > +	case SNDRV_PCM_TRIGGER_RESUME: /* fall through */
> > > +	case SNDRV_PCM_TRIGGER_START:
> > > +		clk_enable(dac->sample_clk);
> > > +		retval = dw_dma_cyclic_start(dac->dma.chan);
> > > +		if (retval)
> > > +			goto out;
> > > +		dac_writel(dac, CTRL, DAC_BIT(EN));
> > > +		break;
> > > +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: /* fall through */
> > > +	case SNDRV_PCM_TRIGGER_SUSPEND: /* fall through */
> > > +	case SNDRV_PCM_TRIGGER_STOP:
> > > +		set_bit(DMA_STOP, &dac->flags);
> > > +		dw_dma_cyclic_stop(dac->dma.chan);
> > > +		dac_writel(dac, DATA, 0);
> > > +		dac_writel(dac, CTRL, 0);
> > > +		clk_disable(dac->sample_clk);
> > > +		break;
> > 
> > No retval is set for these cases?
> > 
> 
> Nope, they are all void return functions and will always complete
> successfully.

I meant that retval is kept -EINVAL in the cases for STOP, etc,
although the call is successful.


Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC
  2009-02-04 12:52         ` Takashi Iwai
@ 2009-02-04 12:58           ` Hans-Christian Egtvedt
  0 siblings, 0 replies; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 12:58 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On Wed, 04 Feb 2009 13:52:06 +0100
Takashi Iwai <tiwai@suse.de> wrote:

> At Wed, 4 Feb 2009 13:18:35 +0100,
> Hans-Christian Egtvedt wrote:
> > 
> > > > +static int at32_abdac_trigger(struct snd_pcm_substream
> > > > *substream, int cmd) +{
> > > > +	struct at32_abdac *dac =
> > > > snd_pcm_substream_chip(substream);
> > > > +	int retval = -EINVAL;
> > > > +
> > > > +	switch (cmd) {
> > > > +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: /* fall through
> > > > */
> > > > +	case SNDRV_PCM_TRIGGER_RESUME: /* fall through */
> > > > +	case SNDRV_PCM_TRIGGER_START:
> > > > +		clk_enable(dac->sample_clk);
> > > > +		retval = dw_dma_cyclic_start(dac->dma.chan);
> > > > +		if (retval)
> > > > +			goto out;
> > > > +		dac_writel(dac, CTRL, DAC_BIT(EN));
> > > > +		break;
> > > > +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: /* fall through */
> > > > +	case SNDRV_PCM_TRIGGER_SUSPEND: /* fall through */
> > > > +	case SNDRV_PCM_TRIGGER_STOP:
> > > > +		set_bit(DMA_STOP, &dac->flags);
> > > > +		dw_dma_cyclic_stop(dac->dma.chan);
> > > > +		dac_writel(dac, DATA, 0);
> > > > +		dac_writel(dac, CTRL, 0);
> > > > +		clk_disable(dac->sample_clk);
> > > > +		break;
> > > 
> > > No retval is set for these cases?
> > > 
> > 
> > Nope, they are all void return functions and will always complete
> > successfully.
> 
> I meant that retval is kept -EINVAL in the cases for STOP, etc,
> although the call is successful.
> 

Ops, my bad, will fix.

Thanks.

-- 
Best regards,
Hans-Christian Egtvedt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 12:51           ` Sedji Gaouaou
@ 2009-02-04 13:01             ` Mark Brown
  2009-02-04 13:03             ` Hans-Christian Egtvedt
  1 sibling, 0 replies; 24+ messages in thread
From: Mark Brown @ 2009-02-04 13:01 UTC (permalink / raw)
  To: Sedji Gaouaou; +Cc: alsa-devel, Hans-Christian Egtvedt

On Wed, Feb 04, 2009 at 01:51:48PM +0100, Sedji Gaouaou wrote:

> Maybe we could share the same code we some #define in it?

Or have separate files for the DMA on each platform (might be worth
having a look at how the PXA AC97 stuff is split up - it does share all
the DMA code but it's split out from the AC97 controller code.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 12:51           ` Sedji Gaouaou
  2009-02-04 13:01             ` Mark Brown
@ 2009-02-04 13:03             ` Hans-Christian Egtvedt
  2009-02-04 13:11               ` Sedji Gaouaou
  1 sibling, 1 reply; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 13:03 UTC (permalink / raw)
  To: Sedji Gaouaou; +Cc: alsa-devel, Mark Brown

On Wed, 04 Feb 2009 13:51:48 +0100
Sedji Gaouaou <sedji.gaouaou@atmel.com> wrote:

> Hi,
> Hans-Christian Egtvedt a écrit :
> > Yes, it should be possible to share the AC97C driver between AT91
> > and AVR32. Although this AC97C driver uses the DesignWare DMAC
> > controller (drivers/dma/dw_dmac.c).
> > 
> > I do not remember which DMA interface AT91 devices uses.
> > 
> > Should the driver go into sound/drivers then? And renamed to
> > atmel-ac97c.{c,h} for example.
> > 
> Actually there are some differences, AT91 does not use a DMA, we use
> a specific IP(PDC) to transer the data...
> 

PDC is still DMA ;) I guess AT91 uses the "old" PDC and not PDCA?

> Maybe we could share the same code we some #define in it?
> 

Yes, it should be simple to add an additional #define in kconfig.

config SND_ATMEL_AC97C_PDC
	bool
	depends on ARCH_AT91

And then have some ifdefs/else around the DMA stuff, much like the
atmel-mci driver does today.

-- 
Best regards,
Hans-Christian Egtvedt
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 13:03             ` Hans-Christian Egtvedt
@ 2009-02-04 13:11               ` Sedji Gaouaou
  2009-02-04 13:21                 ` Hans-Christian Egtvedt
  0 siblings, 1 reply; 24+ messages in thread
From: Sedji Gaouaou @ 2009-02-04 13:11 UTC (permalink / raw)
  To: Hans-Christian Egtvedt; +Cc: alsa-devel, Mark Brown

Hans-Christian Egtvedt a écrit :
>> Actually there are some differences, AT91 does not use a DMA, we use
>> a specific IP(PDC) to transer the data...
>>
> 
> PDC is still DMA ;) I guess AT91 uses the "old" PDC and not PDCA?
> 
I don't know about any PDCA...so yes I guess we are using the old one...
>> Maybe we could share the same code we some #define in it?
>>
> 
> Yes, it should be simple to add an additional #define in kconfig.
> 
> config SND_ATMEL_AC97C_PDC
> 	bool
> 	depends on ARCH_AT91
> 
> And then have some ifdefs/else around the DMA stuff, much like the
> atmel-mci driver does today.
> 
I don't know which solution is the dest: ifdef or as Mark's one...


Regards,
Sedji

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 13:11               ` Sedji Gaouaou
@ 2009-02-04 13:21                 ` Hans-Christian Egtvedt
  2009-02-04 13:44                   ` Takashi Iwai
  0 siblings, 1 reply; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 13:21 UTC (permalink / raw)
  To: Sedji Gaouaou; +Cc: alsa-devel, Mark Brown

On Wed, 04 Feb 2009 14:11:38 +0100
Sedji Gaouaou <sedji.gaouaou@atmel.com> wrote:

> Hans-Christian Egtvedt a écrit :
> >> Actually there are some differences, AT91 does not use a DMA, we
> >> use a specific IP(PDC) to transer the data...
> >>
> > 
> > PDC is still DMA ;) I guess AT91 uses the "old" PDC and not PDCA?
> > 
> I don't know about any PDCA...so yes I guess we are using the old
> one...
>

PDCA is a centralized version of PDC, it might be we only use it on
AVR32 devices for the time being.

> >> Maybe we could share the same code we some #define in it?
> >>
> > 
> > Yes, it should be simple to add an additional #define in kconfig.
> > 
> > config SND_ATMEL_AC97C_PDC
> > 	bool
> > 	depends on ARCH_AT91
> > 
> > And then have some ifdefs/else around the DMA stuff, much like the
> > atmel-mci driver does today.
> > 
> I don't know which solution is the dest: ifdef or as Mark's one...
> 

The PDC DMA is very simple, so will not add a lot of additional
bits'n'bytes. So IMHO I think going for all in one source would be the
best.

Should I move the AC97C driver to sound/drivers instead then, and then
leave it open for you to implement the extra parts for AT91 devices?

-- 
Best regards,
Hans-Christian Egtvedt
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 13:21                 ` Hans-Christian Egtvedt
@ 2009-02-04 13:44                   ` Takashi Iwai
  2009-02-04 14:28                     ` Sedji Gaouaou
  0 siblings, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2009-02-04 13:44 UTC (permalink / raw)
  To: Hans-Christian Egtvedt; +Cc: Sedji Gaouaou, alsa-devel, Mark Brown

At Wed, 4 Feb 2009 14:21:30 +0100,
Hans-Christian Egtvedt wrote:
> 
> On Wed, 04 Feb 2009 14:11:38 +0100
> Sedji Gaouaou <sedji.gaouaou@atmel.com> wrote:
> 
> > Hans-Christian Egtvedt a écrit :
> > >> Actually there are some differences, AT91 does not use a DMA, we
> > >> use a specific IP(PDC) to transer the data...
> > >>
> > > 
> > > PDC is still DMA ;) I guess AT91 uses the "old" PDC and not PDCA?
> > > 
> > I don't know about any PDCA...so yes I guess we are using the old
> > one...
> >
> 
> PDCA is a centralized version of PDC, it might be we only use it on
> AVR32 devices for the time being.
> 
> > >> Maybe we could share the same code we some #define in it?
> > >>
> > > 
> > > Yes, it should be simple to add an additional #define in kconfig.
> > > 
> > > config SND_ATMEL_AC97C_PDC
> > > 	bool
> > > 	depends on ARCH_AT91
> > > 
> > > And then have some ifdefs/else around the DMA stuff, much like the
> > > atmel-mci driver does today.
> > > 
> > I don't know which solution is the dest: ifdef or as Mark's one...
> > 
> 
> The PDC DMA is very simple, so will not add a lot of additional
> bits'n'bytes. So IMHO I think going for all in one source would be the
> best.
> 
> Should I move the AC97C driver to sound/drivers instead then, and then
> leave it open for you to implement the extra parts for AT91 devices?

Well, in general, sound/drivers is for generic drivers that aren't
specific to certain architecture.  I think it's OK to put it in
sound/atmel.


Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 13:44                   ` Takashi Iwai
@ 2009-02-04 14:28                     ` Sedji Gaouaou
  0 siblings, 0 replies; 24+ messages in thread
From: Sedji Gaouaou @ 2009-02-04 14:28 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, Hans-Christian Egtvedt, Mark Brown

Takashi Iwai a écrit :
> At Wed, 4 Feb 2009 14:21:30 +0100,
> Hans-Christian Egtvedt wrote:
>> Should I move the AC97C driver to sound/drivers instead then, and then
>> leave it open for you to implement the extra parts for AT91 devices?
> 
> Well, in general, sound/drivers is for generic drivers that aren't
> specific to certain architecture.  I think it's OK to put it in
> sound/atmel.
> 
> 
> Takashi
> 
So I should make a patch to apply on the top of your patch?
I can do that but it will not be this week because I am quite busy 
currently...

Sedji

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 12:09       ` Takashi Iwai
  2009-02-04 12:16         ` Hans-Christian Egtvedt
@ 2009-02-04 15:02         ` Hans-Christian Egtvedt
  2009-02-04 15:08           ` Takashi Iwai
  1 sibling, 1 reply; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-04 15:02 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On Wed, 04 Feb 2009 13:09:37 +0100
Takashi Iwai <tiwai@suse.de> wrote:

> At Wed,  4 Feb 2009 12:48:34 +0100,
> Hans-Christian Egtvedt wrote:

<snipp>

> > +static int atmel_ac97c_playback_hw_params(struct snd_pcm_substream
> > *substream,
> > +		struct snd_pcm_hw_params *hw_params)
> > +{
> > +	struct atmel_ac97c *chip =
> > snd_pcm_substream_chip(substream);
> > +	int retval;
> > +
> > +	retval = snd_pcm_lib_malloc_pages(substream,
> > +
> > params_buffer_bytes(hw_params));
> > +	if (retval)
> > +		return retval;
> 
> Should be a negative-check.
> 

For some reason if I only check for < 0, then the buffer is not
configured properly. Seems like the preallocation goes wrong for some
reason.

Any pointers to why?

I do snd_pcm_new(), then snd_pcm_set_ops(), then
snd_pcm_lib_preallocate_pages_for_all().

<snipp>

-- 
Best regards,
Hans-Christian Egtvedt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 15:02         ` Hans-Christian Egtvedt
@ 2009-02-04 15:08           ` Takashi Iwai
  0 siblings, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2009-02-04 15:08 UTC (permalink / raw)
  To: Hans-Christian Egtvedt; +Cc: alsa-devel

At Wed, 4 Feb 2009 16:02:14 +0100,
Hans-Christian Egtvedt wrote:
> 
> On Wed, 04 Feb 2009 13:09:37 +0100
> Takashi Iwai <tiwai@suse.de> wrote:
> 
> > At Wed,  4 Feb 2009 12:48:34 +0100,
> > Hans-Christian Egtvedt wrote:
> 
> <snipp>
> 
> > > +static int atmel_ac97c_playback_hw_params(struct snd_pcm_substream
> > > *substream,
> > > +		struct snd_pcm_hw_params *hw_params)
> > > +{
> > > +	struct atmel_ac97c *chip =
> > > snd_pcm_substream_chip(substream);
> > > +	int retval;
> > > +
> > > +	retval = snd_pcm_lib_malloc_pages(substream,
> > > +
> > > params_buffer_bytes(hw_params));
> > > +	if (retval)
> > > +		return retval;
> > 
> > Should be a negative-check.
> > 
> 
> For some reason if I only check for < 0, then the buffer is not
> configured properly. Seems like the preallocation goes wrong for some
> reason.
> 
> Any pointers to why?

Could it be due to multiple hw_params calls?
You can check the sequence of PCM callbacks.

Oh wait, is it an error from atmel_ac97c_prepare_dma()?
Looking through the code, it uses runtime->buffer_size and
runtime->period_size.  These aren't set yet inside hw_params.
I think this setup can be done better in prepare callback.


Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] Add ALSA driver for Atmel AC97 controller
  2009-02-04 11:48     ` [PATCH 3/3] Add ALSA driver for Atmel AC97 controller Hans-Christian Egtvedt
  2009-02-04 12:00       ` Mark Brown
  2009-02-04 12:09       ` Takashi Iwai
@ 2009-02-05  7:14       ` Hans-Christian Egtvedt
  2 siblings, 0 replies; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2009-02-05  7:14 UTC (permalink / raw)
  To: Hans-Christian Egtvedt; +Cc: alsa-devel

On Wed,  4 Feb 2009 12:48:34 +0100
Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> wrote:

> This patch adds ALSA support for the AC97 controller found on Atmel
> AVR32 devices.
> 
> Tested on ATSTK1006 + ATSTK1000 with a development board with a AC97
> codec.
> 

<snipp most of patch>

> +static int __devinit atmel_ac97c_pcm_new(struct atmel_ac97c *chip)
> +{
> +	struct snd_pcm *pcm;
> +	int capture = test_bit(DMA_RX_CHAN_PRESENT, &chip->flags);
> +	int playback = test_bit(DMA_TX_CHAN_PRESENT, &chip->flags);
> +	int err;
> +
> +	err = snd_ac97_pcm_assign(chip->ac97_bus,
> +			ARRAY_SIZE(atmel_ac97_pcm_defs),
> +			atmel_ac97_pcm_defs);

Is this call really needed? snd_ac97_pcm_assign() seems to have few
users in ALSA. I do not really get what this does, it is just something
that's always been in the driver.

<snipp>

-- 
Best regards,
Hans-Christian Egtvedt

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2009-02-05  7:14 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-04 11:48 [PATCH 0/3] Add AVR32 ALSA drivers Hans-Christian Egtvedt
2009-02-04 11:48 ` [PATCH 1/3] Add AVR32 ALSA drivers directory Hans-Christian Egtvedt
2009-02-04 11:48   ` [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC Hans-Christian Egtvedt
2009-02-04 11:48     ` [PATCH 3/3] Add ALSA driver for Atmel AC97 controller Hans-Christian Egtvedt
2009-02-04 12:00       ` Mark Brown
2009-02-04 12:15         ` Hans-Christian Egtvedt
2009-02-04 12:51           ` Sedji Gaouaou
2009-02-04 13:01             ` Mark Brown
2009-02-04 13:03             ` Hans-Christian Egtvedt
2009-02-04 13:11               ` Sedji Gaouaou
2009-02-04 13:21                 ` Hans-Christian Egtvedt
2009-02-04 13:44                   ` Takashi Iwai
2009-02-04 14:28                     ` Sedji Gaouaou
2009-02-04 12:09       ` Takashi Iwai
2009-02-04 12:16         ` Hans-Christian Egtvedt
2009-02-04 15:02         ` Hans-Christian Egtvedt
2009-02-04 15:08           ` Takashi Iwai
2009-02-05  7:14       ` Hans-Christian Egtvedt
2009-02-04 12:00     ` [PATCH 2/3] Add ALSA driver for Atmel Audio Bitstream DAC Takashi Iwai
2009-02-04 12:18       ` Hans-Christian Egtvedt
2009-02-04 12:52         ` Takashi Iwai
2009-02-04 12:58           ` Hans-Christian Egtvedt
2009-02-04 12:02   ` [PATCH 1/3] Add AVR32 ALSA drivers directory Takashi Iwai
2009-02-04 12:11     ` Hans-Christian Egtvedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.