[alsa-devel] [PATCH 0/7] snd-atmel-ac97c: misc fixes and error reporting
This patch series contains six minor fixes and one enhancment of the snd-atmel-ac97c driver. It is based on the for-next branch in sound-2.6 git repo.
The enhancment is enabling interrupts for error reporting, this might be useful if you have sound troubles (glitches) on a stressed system.
This patch will remove traces of channel B registers, since they are not used by the AC97C driver. Channel B might be used for other purposes.
The driver also adds channel status bits TXEMPTY and OVRUN and a AC97C_CH_MASK macro to ease clearing a channel settings.
Signed-off-by: Hans-Christian Egtvedt hans-christian.egtvedt@atmel.com --- sound/atmel/ac97c.h | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/sound/atmel/ac97c.h b/sound/atmel/ac97c.h index c17bd58..ecbba50 100644 --- a/sound/atmel/ac97c.h +++ b/sound/atmel/ac97c.h @@ -1,5 +1,5 @@ /* - * Register definitions for the Atmel AC97C controller + * Register definitions for Atmel AC97C * * Copyright (C) 2005-2009 Atmel Corporation * @@ -17,10 +17,6 @@ #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 @@ -46,8 +42,10 @@ #define AC97C_MR_VRA (1 << 2)
#define AC97C_CSR_TXRDY (1 << 0) +#define AC97C_CSR_TXEMPTY (1 << 1) #define AC97C_CSR_UNRUN (1 << 2) #define AC97C_CSR_RXRDY (1 << 4) +#define AC97C_CSR_OVRUN (1 << 5) #define AC97C_CSR_ENDTX (1 << 10) #define AC97C_CSR_ENDRX (1 << 14)
@@ -61,11 +59,15 @@ #define AC97C_CMR_DMAEN (1 << 22)
#define AC97C_SR_CAEVT (1 << 3) +#define AC97C_SR_COEVT (1 << 2) +#define AC97C_SR_WKUP (1 << 1) +#define AC97C_SR_SOF (1 << 0)
+#define AC97C_CH_MASK(slot) \ + (0x7 << (3 * (AC97_SLOT_##slot - 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_ATMEL_AC97C_H */
Signed-off-by: Hans-Christian Egtvedt hans-christian.egtvedt@atmel.com --- sound/atmel/ac97c.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c index dd72e00..21be9c9 100644 --- a/sound/atmel/ac97c.c +++ b/sound/atmel/ac97c.c @@ -312,7 +312,6 @@ static int atmel_ac97c_playback_prepare(struct snd_pcm_substream *substream) default: /* TODO: support more than two channels */ return -EINVAL; - break; } ac97c_writel(chip, OCA, word);
@@ -374,7 +373,6 @@ static int atmel_ac97c_capture_prepare(struct snd_pcm_substream *substream) default: /* TODO: support more than two channels */ return -EINVAL; - break; } ac97c_writel(chip, ICA, word);
This patch will take care not to overwrite OCA and ICA registers when assigning input and output channels. It will also make sure the registers are at a known state when enabling a channel and clean up properly in case of an error.
Signed-off-by: Hans-Christian Egtvedt hans-christian.egtvedt@atmel.com --- sound/atmel/ac97c.c | 23 ++++++++++++++++++----- 1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c index 21be9c9..4e8f66d 100644 --- a/sound/atmel/ac97c.c +++ b/sound/atmel/ac97c.c @@ -1,5 +1,5 @@ /* - * Driver for the Atmel AC97C controller + * Driver for Atmel AC97C * * Copyright (C) 2005-2009 Atmel Corporation * @@ -10,6 +10,7 @@ #include <linux/clk.h> #include <linux/delay.h> #include <linux/bitmap.h> +#include <linux/device.h> #include <linux/dmaengine.h> #include <linux/dma-mapping.h> #include <linux/init.h> @@ -297,9 +298,11 @@ 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; + unsigned long word = ac97c_readl(chip, OCA); int retval;
+ word &= ~(AC97C_CH_MASK(PCM_LEFT) | AC97C_CH_MASK(PCM_RIGHT)); + /* assign channels to AC97C channel A */ switch (runtime->channels) { case 1: @@ -323,9 +326,13 @@ static int atmel_ac97c_playback_prepare(struct snd_pcm_substream *substream) word |= AC97C_CMR_CEM_LITTLE; break; case SNDRV_PCM_FORMAT_S16_BE: /* fall through */ - default: word &= ~(AC97C_CMR_CEM_LITTLE); break; + default: + word = ac97c_readl(chip, OCA); + word &= ~(AC97C_CH_MASK(PCM_LEFT) | AC97C_CH_MASK(PCM_RIGHT)); + ac97c_writel(chip, OCA, word); + return -EINVAL; }
ac97c_writel(chip, CAMR, word); @@ -358,9 +365,11 @@ 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; + unsigned long word = ac97c_readl(chip, ICA); int retval;
+ word &= ~(AC97C_CH_MASK(PCM_LEFT) | AC97C_CH_MASK(PCM_RIGHT)); + /* assign channels to AC97C channel A */ switch (runtime->channels) { case 1: @@ -384,9 +393,13 @@ static int atmel_ac97c_capture_prepare(struct snd_pcm_substream *substream) word |= AC97C_CMR_CEM_LITTLE; break; case SNDRV_PCM_FORMAT_S16_BE: /* fall through */ - default: word &= ~(AC97C_CMR_CEM_LITTLE); break; + default: + word = ac97c_readl(chip, ICA); + word &= ~(AC97C_CH_MASK(PCM_LEFT) | AC97C_CH_MASK(PCM_RIGHT)); + ac97c_writel(chip, ICA, word); + return -EINVAL; }
ac97c_writel(chip, CAMR, word);
This patch will set a proper maximum bytes for the buffer, which is: channels * bytes per sample * maximum periods * maximum bytes per period.
Signed-off-by: Hans-Christian Egtvedt hans-christian.egtvedt@atmel.com --- sound/atmel/ac97c.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c index 4e8f66d..9748225 100644 --- a/sound/atmel/ac97c.c +++ b/sound/atmel/ac97c.c @@ -151,7 +151,7 @@ static struct snd_pcm_hardware atmel_ac97c_hw = { .rate_max = 48000, .channels_min = 1, .channels_max = 2, - .buffer_bytes_max = 64 * 4096, + .buffer_bytes_max = 2 * 2 * 64 * 2048, .period_bytes_min = 4096, .period_bytes_max = 4096, .periods_min = 4,
This patch will enable interrupts from AC97C and report about error conditions that occurs.
On channel A both overrun and underrun will be enabled depending if playback and/or capture are enabled. On the control channel the overrun interrupt is enabled.
Signed-off-by: Hans-Christian Egtvedt hans-christian.egtvedt@atmel.com --- sound/atmel/ac97c.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 76 insertions(+), 1 deletions(-)
diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c index 9748225..7c7521b 100644 --- a/sound/atmel/ac97c.c +++ b/sound/atmel/ac97c.c @@ -66,6 +66,7 @@ struct atmel_ac97c { /* Serialize access to opened variable */ spinlock_t lock; void __iomem *regs; + int irq; int opened; int reset_pin; }; @@ -335,8 +336,16 @@ static int atmel_ac97c_playback_prepare(struct snd_pcm_substream *substream) return -EINVAL; }
+ /* Enable underrun interrupt on channel A */ + word |= AC97C_CSR_UNRUN; + ac97c_writel(chip, CAMR, word);
+ /* Enable channel A event interrupt */ + word = ac97c_readl(chip, IMR); + word |= AC97C_SR_CAEVT; + ac97c_writel(chip, IER, word); + /* set variable rate if needed */ if (runtime->rate != 48000) { word = ac97c_readl(chip, MR); @@ -402,8 +411,16 @@ static int atmel_ac97c_capture_prepare(struct snd_pcm_substream *substream) return -EINVAL; }
+ /* Enable overrun interrupt on channel A */ + word |= AC97C_CSR_OVRUN; + ac97c_writel(chip, CAMR, word);
+ /* Enable channel A event interrupt */ + word = ac97c_readl(chip, IMR); + word |= AC97C_SR_CAEVT; + ac97c_writel(chip, IER, word); + /* set variable rate if needed */ if (runtime->rate != 48000) { word = ac97c_readl(chip, MR); @@ -554,6 +571,43 @@ static struct snd_pcm_ops atmel_ac97_capture_ops = { .pointer = atmel_ac97c_capture_pointer, };
+static irqreturn_t atmel_ac97c_interrupt(int irq, void *dev) +{ + struct atmel_ac97c *chip = (struct atmel_ac97c *)dev; + irqreturn_t retval = IRQ_NONE; + u32 sr = ac97c_readl(chip, SR); + u32 casr = ac97c_readl(chip, CASR); + u32 cosr = ac97c_readl(chip, COSR); + + if (sr & AC97C_SR_CAEVT) { + dev_info(&chip->pdev->dev, "channel A event%s%s%s%s%s%s\n", + casr & AC97C_CSR_OVRUN ? " OVRUN" : "", + casr & AC97C_CSR_RXRDY ? " RXRDY" : "", + casr & AC97C_CSR_UNRUN ? " UNRUN" : "", + casr & AC97C_CSR_TXEMPTY ? " TXEMPTY" : "", + casr & AC97C_CSR_TXRDY ? " TXRDY" : "", + !casr ? " NONE" : ""); + retval = IRQ_HANDLED; + } + + if (sr & AC97C_SR_COEVT) { + dev_info(&chip->pdev->dev, "codec channel event%s%s%s%s%s\n", + cosr & AC97C_CSR_OVRUN ? " OVRUN" : "", + cosr & AC97C_CSR_RXRDY ? " RXRDY" : "", + cosr & AC97C_CSR_TXEMPTY ? " TXEMPTY" : "", + cosr & AC97C_CSR_TXRDY ? " TXRDY" : "", + !cosr ? " NONE" : ""); + retval = IRQ_HANDLED; + } + + if (retval == IRQ_NONE) { + dev_err(&chip->pdev->dev, "spurious interrupt sr 0x%08x " + "casr 0x%08x cosr 0x%08x\n", sr, casr, cosr); + } + + return retval; +} + static int __devinit atmel_ac97c_pcm_new(struct atmel_ac97c *chip) { struct snd_pcm *pcm; @@ -701,6 +755,7 @@ static int __devinit atmel_ac97c_probe(struct platform_device *pdev) .read = atmel_ac97c_read, }; int retval; + int irq;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!regs) { @@ -714,6 +769,12 @@ static int __devinit atmel_ac97c_probe(struct platform_device *pdev) return -ENXIO; }
+ irq = platform_get_irq(pdev, 0); + if (irq < 0) { + dev_dbg(&pdev->dev, "could not get irq\n"); + return -ENXIO; + } + pclk = clk_get(&pdev->dev, "pclk"); if (IS_ERR(pclk)) { dev_dbg(&pdev->dev, "no peripheral clock\n"); @@ -730,6 +791,13 @@ static int __devinit atmel_ac97c_probe(struct platform_device *pdev)
chip = get_chip(card);
+ retval = request_irq(irq, atmel_ac97c_interrupt, 0, "AC97C", chip); + if (retval) { + dev_dbg(&pdev->dev, "unable to request irq %d\n", irq); + goto err_request_irq; + } + chip->irq = irq; + spin_lock_init(&chip->lock);
strcpy(card->driver, "Atmel AC97C"); @@ -758,6 +826,10 @@ static int __devinit atmel_ac97c_probe(struct platform_device *pdev)
snd_card_set_dev(card, &pdev->dev);
+ /* Enable overrun interrupt from codec channel */ + ac97c_writel(chip, COMR, AC97C_CSR_OVRUN); + ac97c_writel(chip, IER, ac97c_readl(chip, IMR) | AC97C_SR_COEVT); + retval = snd_ac97_bus(card, 0, &ops, chip, &chip->ac97_bus); if (retval) { dev_dbg(&pdev->dev, "could not register on ac97 bus\n"); @@ -820,7 +892,7 @@ static int __devinit atmel_ac97c_probe(struct platform_device *pdev) retval = snd_card_register(card); if (retval) { dev_dbg(&pdev->dev, "could not register sound card\n"); - goto err_ac97_bus; + goto err_dma; }
platform_set_drvdata(pdev, card); @@ -847,6 +919,8 @@ err_ac97_bus:
iounmap(chip->regs); err_ioremap: + free_irq(irq, chip); +err_request_irq: snd_card_free(card); err_snd_card_new: clk_disable(pclk); @@ -898,6 +972,7 @@ static int __devexit atmel_ac97c_remove(struct platform_device *pdev) clk_disable(chip->pclk); clk_put(chip->pclk); iounmap(chip->regs); + free_irq(chip->irq, chip);
if (test_bit(DMA_RX_CHAN_PRESENT, &chip->flags)) dma_release_channel(chip->dma.rx_chan);
This patch will enable the AC97C before resetting the external codec, leaving the AC97C disabled will result in floating I/O lines that can affect the reset procedure.
Signed-off-by: Hans-Christian Egtvedt hans-christian.egtvedt@atmel.com --- sound/atmel/ac97c.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c index 7c7521b..75100b0 100644 --- a/sound/atmel/ac97c.c +++ b/sound/atmel/ac97c.c @@ -730,17 +730,17 @@ static bool filter(struct dma_chan *chan, void *slave)
static void atmel_ac97c_reset(struct atmel_ac97c *chip) { - ac97c_writel(chip, MR, AC97C_MR_WRST); + ac97c_writel(chip, MR, 0); + ac97c_writel(chip, MR, AC97C_MR_ENA); + ac97c_writel(chip, CAMR, 0); + ac97c_writel(chip, COMR, 0);
if (gpio_is_valid(chip->reset_pin)) { gpio_set_value(chip->reset_pin, 0); /* AC97 v2.2 specifications says minimum 1 us. */ - udelay(10); + udelay(2); 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) @@ -826,6 +826,8 @@ static int __devinit atmel_ac97c_probe(struct platform_device *pdev)
snd_card_set_dev(card, &pdev->dev);
+ atmel_ac97c_reset(chip); + /* Enable overrun interrupt from codec channel */ ac97c_writel(chip, COMR, AC97C_CSR_OVRUN); ac97c_writel(chip, IER, ac97c_readl(chip, IMR) | AC97C_SR_COEVT); @@ -836,8 +838,6 @@ static int __devinit atmel_ac97c_probe(struct platform_device *pdev) 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");
This patch will set the channel A and control channel mode register to zero before disabling the AC97C peripheral.
Signed-off-by: Hans-Christian Egtvedt hans-christian.egtvedt@atmel.com --- sound/atmel/ac97c.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c index 75100b0..c917306 100644 --- a/sound/atmel/ac97c.c +++ b/sound/atmel/ac97c.c @@ -969,6 +969,10 @@ static int __devexit atmel_ac97c_remove(struct platform_device *pdev) if (gpio_is_valid(chip->reset_pin)) gpio_free(chip->reset_pin);
+ ac97c_writel(chip, CAMR, 0); + ac97c_writel(chip, COMR, 0); + ac97c_writel(chip, MR, 0); + clk_disable(chip->pclk); clk_put(chip->pclk); iounmap(chip->regs);
On Tue, 24 Mar 2009 13:47:43 +0100 Hans-Christian Egtvedt hans-christian.egtvedt@atmel.com wrote:
Hi,
This patch series contains six minor fixes and one enhancment of the snd-atmel-ac97c driver. It is based on the for-next branch in sound-2.6 git repo.
Any news about this patch series? It would be great if it could make it in with this merge window.
<snipp>
On Wed, Apr 01, 2009 at 03:50:06PM +0200, Hans-Christian Egtvedt wrote:
Any news about this patch series? It would be great if it could make it in with this merge window.
Takashi is away at the minute - I'm not entirely sure when he's due back. If he doesn't notice the patches in the backlog when he returns it'd be worth resubmitting with a CC to him (this is a good idea in general when submitting ALSA patches).
On Wed, 1 Apr 2009 16:37:24 +0100 Mark Brown broonie@sirena.org.uk wrote:
On Wed, Apr 01, 2009 at 03:50:06PM +0200, Hans-Christian Egtvedt wrote:
Any news about this patch series? It would be great if it could make it in with this merge window.
Takashi is away at the minute - I'm not entirely sure when he's due back. If he doesn't notice the patches in the backlog when he returns it'd be worth resubmitting with a CC to him (this is a good idea in general when submitting ALSA patches).
Okay, thanks, I think I'll do a resubmit then with Takashi on CC, since I am leaving for holidays later today.
participants (2)
-
Hans-Christian Egtvedt -
Mark Brown