At Fri, 5 Oct 2007 00:34:58 -0400,
Dave Dillow wrote:
>
> Basic audio support for the SiS 7019 Audio Accelerator as found in
> the SiS 55x SoC. There is currently no synth or power management
> support at the moment, but the audio playback has been tested with
> several channel and rate combinations, as well as various period
> and buffer configurations.
>
> Signed-off-by: David Dillow <dave@xxxxxxxxxxxxxx>
Thanks for the patch. The code looks almost OK to me.
Below is a very brief review.
> diff --git a/include/sound/sis7019.h b/include/sound/sis7019.h
> new file mode 100644
> index 0000000..d395abb
> --- /dev/null
> +++ b/include/sound/sis7019.h
Any reason to put this to include/sound? It's basically a place for
headers used publicly. If the header is for the driver-only, better
placed in sound/pci or a place where is private.
> diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c
> new file mode 100644
> index 0000000..3a74f64
> --- /dev/null
> +++ b/sound/pci/sis7019.c
(snip)
> +struct sis7019 {
> + long unsigned ioport;
unsigned long is a more common style.
> +static irqreturn_t sis_interrupt(int irq, void *dev)
(snip)
> + spin_lock(&sis->hw_lock);
> + do {
> + status = inl(io + SIS_PISR_A);
> + if (status) {
> + orig = status;
> + voice = sis->voices;
> + while (status) {
> + bit = __ffs(status);
> + status >>= bit + 1;
> + voice += bit;
> + snd_pcm_period_elapsed(voice->substream);
You need to unlock sis->hw_lock before calling
snd_pcm_period_elapsed(). It may call snd_pcm_stop() at XRUN, and
this invokes the trigger callback, which locks hw_lock again.
Simply re-acquire hw_lock after calling snd_pcm_period_elapsed().
> + sis_update_end_offsets(voice);
> + voice++;
> + }
> + outl(orig, io + SIS_PISR_A);
> + }
> +
> + status = inl(io + SIS_PISR_B);
> + if (status) {
> + orig = status;
> + voice = &sis->voices[32];
> + while (status) {
> + bit = __ffs(status);
> + status >>= bit + 1;
> + voice += bit;
> + snd_pcm_period_elapsed(voice->substream);
Ditto.
> + sis_update_end_offsets(voice);
> + voice++;
> + }
> + outl(orig, io + SIS_PISR_B);
> + }
> +
> + status = inl(io + SIS_RISR);
> + if (status) {
> + voice = &sis->capture_voice;
> + if (!voice->timing)
> + snd_pcm_period_elapsed(voice->substream);
Ditto.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
