At Mon, 07 Nov 2005 18:48:18 -0200,
[email protected] wrote:
>
> From: Ricardo Cerqueira <[email protected]>
>
> - Saa7134-alsa can only be autoloaded after saa7134 is active
> - Applied pertinent changes proposed by the ALSA team
> - dsp_nr replaced by ALSA's index[]
>
> Signed-off-by: Ricardo Cerqueira <[email protected]>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Thanks for the quick rewrite!
> --- hg.old.orig/drivers/media/video/saa7134/saa7134-alsa.c
> +++ hg.old/drivers/media/video/saa7134/saa7134-alsa.c
> @@ -186,30 +171,31 @@ void saa7134_irq_alsa_done(struct saa713
> goto done;
> }
>
> - if (dev->oss.read_count >= dev->oss.blksize * (dev->oss.blocks-2)) {
> - dprintk("irq: overrun [full=%d/%d] - Blocks in %d\n",dev->oss.read_count,
> - dev->oss.bufsize, dev->oss.blocks);
> + if (dev->dmasound.read_count >= dev->dmasound.blksize * (dev->dmasound.blocks-2)) {
> + dprintk("irq: overrun [full=%d/%d] - Blocks in %d\n",dev->dmasound.read_count,
> + dev->dmasound.bufsize, dev->dmasound.blocks);
> + snd_pcm_stop(dev->dmasound.substream,SNDRV_PCM_STATE_XRUN);
> saa7134_dma_stop(dev);
> goto done;
> }
You need spin_unlock(&dev->slock) before calling snd_pcm_stop() since
this lock is used in trigger callback, too. Note that "goto done" may
require the lock again. Also, the succeeding saa7134_dma_stop() is
superfluous.
> @@ -374,13 +376,13 @@ static int snd_card_saa7134_capture_prep
> goto fail2;
>
> /* prepare buffer */
> - if (0 != (err = videobuf_dma_pci_map(dev->pci,&dev->oss.dma)))
> + if (0 != (err = videobuf_dma_pci_map(dev->pci,&dev->dmasound.dma)))
> return err;
> - if (0 != (err = saa7134_pgtable_alloc(dev->pci,&dev->oss.pt)))
> + if (0 != (err = saa7134_pgtable_alloc(dev->pci,&dev->dmasound.pt)))
> goto fail1;
> - if (0 != (err = saa7134_pgtable_build(dev->pci,&dev->oss.pt,
> - dev->oss.dma.sglist,
> - dev->oss.dma.sglen,
> + if (0 != (err = saa7134_pgtable_build(dev->pci,&dev->dmasound.pt,
> + dev->dmasound.dma.sglist,
> + dev->dmasound.dma.sglen,
> 0)))
> goto fail2;
The buffer allocation would be better to be done in hw_params
callback because prepare callback can be frequently called (although
it works in prepare callback, too).
Or, at least, you shoud cache the value in the same stream.
> @@ -818,7 +811,7 @@ static int snd_saa7134_capsrc_put(snd_kc
> chip->capture_source[addr][1] != right;
> chip->capture_source[addr][0] = left;
> chip->capture_source[addr][1] = right;
> - dev->oss.input=addr;
> + dev->dmasound.input=addr;
> spin_unlock_irqrestore(&chip->mixer_lock, flags);
You can use spin_unlock_irq() safely here.
> @@ -973,18 +963,17 @@ int alsa_card_saa7134_create (struct saa
> chip->irq = saadev->pci->irq;
> chip->iobase = pci_resource_start(saadev->pci, 0);
>
> - err = request_irq(chip->pci->irq, saa7134_alsa_irq,
> + err = request_irq(saadev->pci->irq, saa7134_alsa_irq,
> SA_SHIRQ | SA_INTERRUPT, saadev->name, saadev);
Hmm, still I don't see any call of free_irq() in the driver?
> @@ -1010,7 +997,7 @@ int alsa_card_saa7134_create (struct saa
>
> __nodev:
> snd_card_free(card);
> - kfree(card);
> + kfree(chip);
It's not safe to call kfree(chip) here, too. This can be already
released, once after snd_device_new() is succeeded.
In your case, it's easier to use the following style:
err = snd_card_new(index[dev], id[dev], THIS_MODULE,
sizeof(snd_card_saa7134_t));
chip = card->private_data;
card->private_free = snd_saa7134_free;
so that no extra kfree() is needed.
Takashi
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
