At Fri, 6 Jul 2007 10:46:59 +0100,
Adrian McMenamin wrote:
>
> On 06/07/07, Takashi Iwai <[EMAIL PROTECTED]> 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)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
