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@thedillows.org
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