Re: [PATCH] 2.4.9-ac5: drivers/sound/trident.c

Alan Cox (alan@lxorguk.ukuu.org.uk)
Sun, 2 Sep 2001 13:59:38 +0100 (BST)


>
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&card->lock, flags);
> portDat = cyber_inidx(CYBER_PORT_AUDIO, CYBER_IDX_AUDIO_ENABLE);
> /* enable, if it was disabled */
> if( (portDat & CYBER_BMSK_AUENZ) != CYBER_BMSK_AUENZ_ENABLE ) {
> @@ -729,7 +730,7 @@
> cyber_outidx( CYBER_PORT_AUDIO, 0xb3, 0x06 );
> cyber_outidx( CYBER_PORT_AUDIO, 0xbf, 0x00 );
> }
> - restore_flags(flags);
> + spin_unlock_irqrestore(&card->lock, flags);

This one is wrong. What is probably not obvious on first reading is that
the cyber_out* code actually configures the chipset so needs to be locked
against other chipset configurations (current cli/sti based therefore)

> + spin_lock_irqsave(&card->lock, flags);
> dmabuf->swptr = dmabuf->hwptr = 0;
> dmabuf->count = dmabuf->total_bytes = 0;
> + spin_unlock_irqrestore(&card->lock, flags);
> }
> if (file->f_mode & FMODE_READ) {
> stop_adc(state);
> synchronize_irq();
> dmabuf->ready = 0;
> + spin_lock_irqsave(&card->lock, flags);
> dmabuf->swptr = dmabuf->hwptr = 0;
> dmabuf->count = dmabuf->total_bytes = 0;
> + spin_unlock_irqrestore(&card->lock, flags);
> }

Possibly needed yes. We have however stopped the adc before we read it
and we hold the semaphore so I think its ok on this card

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/