Commit 7b666653 authored by Sergei Shtylylov's avatar Sergei Shtylylov Committed by Linus Torvalds

[PATCH] Au1550 AC'97 OSS driver spinlock fixes

We have found some issues with Au1550 AC'97 OSS driver in 2.6
(sound/oss/au1550_ac97.c), though it also should concern 2.4 driver
(drivers/sound/au1550_psc.c).

start_dac() grabs a spinlock already held by its caller, au1550_write().
This doesn't show up with the standard UP spinlock impelmentation but when
the different one (mutex based) is in use, a lockup happens.

And the interrupt handlers also didn't grab the spinlock -- that's OK in
the usual kernel but not when the IRQ handlers are threaded.  So, they're
grabbing the spinlock now (as every correct interrupt handler should do).
Signed-off-by: default avatarKonstantin Baidarov <kbaidarov@ru.mvista.com>
Signed-off-by: default avatarSergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 2f40fb72
...@@ -578,17 +578,15 @@ set_recv_slots(int num_channels) ...@@ -578,17 +578,15 @@ set_recv_slots(int num_channels)
} while ((stat & PSC_AC97STAT_DR) == 0); } while ((stat & PSC_AC97STAT_DR) == 0);
} }
/* Hold spinlock for both start_dac() and start_adc() calls */
static void static void
start_dac(struct au1550_state *s) start_dac(struct au1550_state *s)
{ {
struct dmabuf *db = &s->dma_dac; struct dmabuf *db = &s->dma_dac;
unsigned long flags;
if (!db->stopped) if (!db->stopped)
return; return;
spin_lock_irqsave(&s->lock, flags);
set_xmit_slots(db->num_channels); set_xmit_slots(db->num_channels);
au_writel(PSC_AC97PCR_TC, PSC_AC97PCR); au_writel(PSC_AC97PCR_TC, PSC_AC97PCR);
au_sync(); au_sync();
...@@ -598,8 +596,6 @@ start_dac(struct au1550_state *s) ...@@ -598,8 +596,6 @@ start_dac(struct au1550_state *s)
au1xxx_dbdma_start(db->dmanr); au1xxx_dbdma_start(db->dmanr);
db->stopped = 0; db->stopped = 0;
spin_unlock_irqrestore(&s->lock, flags);
} }
static void static void
...@@ -718,7 +714,6 @@ prog_dmabuf_dac(struct au1550_state *s) ...@@ -718,7 +714,6 @@ prog_dmabuf_dac(struct au1550_state *s)
} }
/* hold spinlock for the following */
static void static void
dac_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs) dac_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs)
{ {
...@@ -726,6 +721,8 @@ dac_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs) ...@@ -726,6 +721,8 @@ dac_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs)
struct dmabuf *db = &s->dma_dac; struct dmabuf *db = &s->dma_dac;
u32 ac97c_stat; u32 ac97c_stat;
spin_lock(&s->lock);
ac97c_stat = au_readl(PSC_AC97STAT); ac97c_stat = au_readl(PSC_AC97STAT);
if (ac97c_stat & (AC97C_XU | AC97C_XO | AC97C_TE)) if (ac97c_stat & (AC97C_XU | AC97C_XO | AC97C_TE))
pr_debug("AC97C status = 0x%08x\n", ac97c_stat); pr_debug("AC97C status = 0x%08x\n", ac97c_stat);
...@@ -747,6 +744,8 @@ dac_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs) ...@@ -747,6 +744,8 @@ dac_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs)
/* wake up anybody listening */ /* wake up anybody listening */
if (waitqueue_active(&db->wait)) if (waitqueue_active(&db->wait))
wake_up(&db->wait); wake_up(&db->wait);
spin_unlock(&s->lock);
} }
...@@ -758,6 +757,8 @@ adc_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs) ...@@ -758,6 +757,8 @@ adc_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs)
u32 obytes; u32 obytes;
char *obuf; char *obuf;
spin_lock(&s->lock);
/* Pull the buffer from the dma queue. /* Pull the buffer from the dma queue.
*/ */
au1xxx_dbdma_get_dest(dp->dmanr, (void *)(&obuf), &obytes); au1xxx_dbdma_get_dest(dp->dmanr, (void *)(&obuf), &obytes);
...@@ -765,6 +766,7 @@ adc_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs) ...@@ -765,6 +766,7 @@ adc_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs)
if ((dp->count + obytes) > dp->dmasize) { if ((dp->count + obytes) > dp->dmasize) {
/* Overrun. Stop ADC and log the error /* Overrun. Stop ADC and log the error
*/ */
spin_unlock(&s->lock);
stop_adc(s); stop_adc(s);
dp->error++; dp->error++;
err("adc overrun"); err("adc overrun");
...@@ -787,6 +789,7 @@ adc_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs) ...@@ -787,6 +789,7 @@ adc_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs)
if (waitqueue_active(&dp->wait)) if (waitqueue_active(&dp->wait))
wake_up(&dp->wait); wake_up(&dp->wait);
spin_unlock(&s->lock);
} }
static loff_t static loff_t
...@@ -1048,9 +1051,9 @@ au1550_read(struct file *file, char *buffer, size_t count, loff_t *ppos) ...@@ -1048,9 +1051,9 @@ au1550_read(struct file *file, char *buffer, size_t count, loff_t *ppos)
/* wait for samples in ADC dma buffer /* wait for samples in ADC dma buffer
*/ */
do { do {
spin_lock_irqsave(&s->lock, flags);
if (db->stopped) if (db->stopped)
start_adc(s); start_adc(s);
spin_lock_irqsave(&s->lock, flags);
avail = db->count; avail = db->count;
if (avail <= 0) if (avail <= 0)
__set_current_state(TASK_INTERRUPTIBLE); __set_current_state(TASK_INTERRUPTIBLE);
...@@ -1570,15 +1573,19 @@ au1550_ioctl(struct inode *inode, struct file *file, unsigned int cmd, ...@@ -1570,15 +1573,19 @@ au1550_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
if (get_user(val, (int *) arg)) if (get_user(val, (int *) arg))
return -EFAULT; return -EFAULT;
if (file->f_mode & FMODE_READ) { if (file->f_mode & FMODE_READ) {
if (val & PCM_ENABLE_INPUT) if (val & PCM_ENABLE_INPUT) {
spin_lock_irqsave(&s->lock, flags);
start_adc(s); start_adc(s);
else spin_unlock_irqrestore(&s->lock, flags);
} else
stop_adc(s); stop_adc(s);
} }
if (file->f_mode & FMODE_WRITE) { if (file->f_mode & FMODE_WRITE) {
if (val & PCM_ENABLE_OUTPUT) if (val & PCM_ENABLE_OUTPUT) {
spin_lock_irqsave(&s->lock, flags);
start_dac(s); start_dac(s);
else spin_unlock_irqrestore(&s->lock, flags);
} else
stop_dac(s); stop_dac(s);
} }
return 0; return 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment