At Fri, 6 Jul 2007 10:46:59 +0100, Adrian McMenamin wrote:
On 06/07/07, Takashi Iwai tiwai@suse.de wrote:
At Thu, 5 Jul 2007 23:07:44 +0100, Adrian McMenamin wrote:
@@ -402,17 +396,10 @@ static int snd_aicapcm_pcm_trigger(struct snd_pcm_substream *substream, int cmd) {
struct snd_card_aica *dreamcastcard; switch (cmd) { case SNDRV_PCM_TRIGGER_START: spu_begin_dma(substream); break;case SNDRV_PCM_TRIGGER_STOP:dreamcastcard = substream->pcm->private_data;if (dreamcastcard->timer.data)del_timer(&dreamcastcard->timer);aica_chn_halt();break; default: return -EINVAL; }Is this a correct change? Then you'd have no control for stopping the stream.
The shutdown is all done in the close() now, partly because snd_aicapcm_pcm_trigger cannot sleep and close() can. Is there a pressing reason to put code in snd_aicapcm_pcm_trigger?
I guess you shoud call del_timer() there at least. Otherwise the timer handler (aica_period_elapsed()) might be called even if the stream is not really running.
The below is some coding style fixes, BTW. I'm not sure whether queue_work() is intended in the block if "if (dma_check == 0)" or not.
Takashi
--- aica.c.old 2007-07-06 11:51:18.000000000 +0200 +++ aica.c 2007-07-06 11:52:10.000000000 +0200 @@ -256,7 +256,8 @@ buffer_size = frames_to_bytes(runtime, runtime->buffer_size); if (runtime->channels > 1) dreamcastcard->channel->flags |= 0x01; - aica_dma_transfer(runtime->channels, buffer_size, dreamcastcard->substream); + aica_dma_transfer(runtime->channels, buffer_size, + dreamcastcard->substream); startup_aica(dreamcastcard); dreamcastcard->clicks = buffer_size / (AICA_PERIOD_SIZE * runtime->channels); @@ -268,9 +269,7 @@ snd_pcm_period_elapsed(dreamcastcard->substream); dreamcastcard->clicks++; if (unlikely(dreamcastcard->clicks >= AICA_PERIOD_NUMBER)) - { dreamcastcard->clicks %= AICA_PERIOD_NUMBER; - } mod_timer(&dreamcastcard->timer, jiffies + 1); } } @@ -300,8 +299,7 @@ dreamcastcard->current_period = play_period; if (unlikely(dreamcastcard->dma_check == 0)) dreamcastcard->dma_check = 1; - queue_work(aica_queue, &(dreamcastcard->spu_dma_work)); - + queue_work(aica_queue, &(dreamcastcard->spu_dma_work)); }
static void spu_begin_dma(struct snd_pcm_substream *substream) @@ -311,7 +309,7 @@ struct snd_pcm_runtime *runtime; runtime = substream->runtime; dreamcastcard = substream->pcm->private_data; - //get the queue to do the work + /* get the queue to do the work */ queue_work(aica_queue, &(dreamcastcard->spu_dma_work)); /* Timer may already be running */ if (unlikely(dreamcastcard->timer.data)) { @@ -597,7 +595,7 @@ strcpy(dreamcastcard->card->shortname, SND_AICA_DRIVER); strcpy(dreamcastcard->card->longname, "Yamaha AICA Super Intelligent Sound Processor for SEGA Dreamcast"); - //start the worker thread + /* start the worker thread */ INIT_WORK(&(dreamcastcard->spu_dma_work), run_spu_dma); /* Load the PCM 'chip' */ err = snd_aicapcmchip(dreamcastcard, 0); @@ -631,7 +629,8 @@ .probe = snd_aica_probe, .remove = snd_aica_remove, .driver = { - .name = SND_AICA_DRIVER}, + .name = SND_AICA_DRIVER + }, };
static int __init aica_init(void)