[patch] i810 audio driver bugfix and SiS7012 support

From: Thomas Gschwind (tom@infosys.tuwien.ac.at)
Date: Tue Jan 01 2002 - 17:43:10 EST

Hello *!

I have found a serious bug and a couple of smaller ones in the i810
audio driver (kernel version 2.4.17). However, I found the bugs by
reading the source code and doing some experimentations with my
SiS7012 (an i810 clone) mainboard. Hopefully, somebody using the i810
audio driver can confirm or deny the bugs below (especially the first

I have also attached a patch that corrects these problems and adds
support for the SiS7012 (now, playback and recording!). It applies to
linux/drivers/sound/i810_audio.c of a 2.4.17 kernel. Any i810 or
SiS7012 owners, please tell me whether the patch works on your system
or whether it adds a different problem. Please mail me directly since
I am not on the kernel mailing list. THANKS!

The bugs:

* The first bug is that i810_read does not set the current state of
  the task to running and does not remove the wait queue if a signal
  occurs. As a side effect the system _crashes_ at the next read.
  This is more serious since it allows any user with read access to
  /dev/dsp to crash the system. Just try
  $ sox -t ossdsp /dev/dsp x.wav
  and press Ctrl-C (I used sox-12.17.1-4 on a RedHat-7.2 system).
  FIX: Replace return ret with goto done in a couple of places.

* A signed/unsigned comparison in i810_read
  (cnt>(dmabuf->dmasize-swptr)) that results in true if cnt<0. This
  bug is everything but serious since it only prepends about 2/3s of
  silence to the recording.
  FIX: Reordering of cnt computation.

* Sometimes a DMA buffer overrun can occur in drain_dac. This one
  only seems to be a spurious one. Probably, the timing on a i810
  system is more perfect than on my K7S5A. Anyway, it is cleaner to
  use interruptible_sleep_on_timeout than schedule_timeout to be less
  dependent on a perfect time. Especially since the wait queue was
  already declared but never used.
  FIX: interruptible_sleep_on_timeout instead of schedule_timeout

* SNDCTL_DSP_GETISPACE returns the size of the DMA buffer minus
  the number of bytes in the buffer. i810_read, however, only allows
  the ring buffer to grow up to dmabuf->dmasize-dmabuf->fragsize.
  Hence, this IOCTL returns to many bytes. [This was likely a mixup
  FIX: Subtract dmabuf->fragsize from the available bytes.

* SNDCTL_DSP_GETOSPACE returns dmabuf->fragsize to little bytes
  available since i810_write used the full DMA buffer. Fortunately,
  this is not a real problem.
  FIX: The SiS7012 chipset does not like the full DMA buffer to be
  used. Hence, I changed i810_write to use only
  dmabuf->dmasize-dmabuf->fragsize bytes of the ring buffer.

Changes for SiS7012 support:

* SR and PICB registers are swapped.
  FIX: Added some ifs at various places.

* DMA transfer is counted in bytes and not in 16bit-words.
  FIX: Conditional * 2 in prog_dmabuf, Conditional / 2 in get_dma_address


PS: The patch can also be found at

Thomas Gschwind                      Email: tom@infosys.tuwien.ac.at
Technische Universität Wien
Argentinierstraße 8/E1841            Tel: +43 (1) 58801 ext. 18412
A-1040 Wien, Austria, Europe         Fax: +43 (1) 58801 ext. 18491

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

This archive was generated by hypermail 2b29 : Mon Jan 07 2002 - 21:00:15 EST