Re: [Alsa-devel] [RFC 4/8] snd-aoa: add i2sbus

At Tue, 06 Jun 2006 13:17:27 +0200,
Johannes Berg wrote:

On Fri, 2006-06-02 at 16:23 +0200, Takashi Iwai wrote:

quoted

quoted

+	if (I2S_CLOCK_SPEED_18MHz % rate == 0) {
+		if ((I2S_CLOCK_SPEED_18MHz / rate) % mclk == 0) {

Equivalent with "I2S_CLOCK_SPEED_18MHZ % (rate * mclk) == 0" ?

Yeah, I guess, never really thought about that, just wrote it down the
way I thought to do it :) That said, I think it's more readable if
written that way, do you want me to change it regardless?

I found a single if is more readable (and good for compiler).

quoted

quoted

+	/* well, we really should support scatter/gather DMA */
+	/* FIXME FIXME FIXME: If this fails, we BUG() when the alsa layer
+	 * later tries to allocate memory. Apparently we should be setting
+	 * some device pointer for that ...
+	 */
+	snd_pcm_lib_preallocate_pages_for_all(
+		dev->pcm, SNDRV_DMA_TYPE_DEV,
+		snd_dma_pci_data(macio_get_pci_dev(i2sdev->macio)),
+		64 * 1024, 64 * 1024);

Is the comment true?  Yes, you have to set the device pointer via
snd_pcm_lib_preallocate*().  But it must be OK even if preallocate
fails.

Hah, I don't know actually, I didn't know you set the pointer using this
function, when I wrote the comment I just had forgotten the preallocate
call!
Does that mean that _preallocate_pages_for_all() has the side effect of
setting the pointer? If so, imho that's pretty bad.

No, the only requirement is that you have to call snd_pcm_lib_malloc()
with proper type and assigned device pointer if you use
snd_pcm_lib_malloc() function.  (If not called, you've got an error
when compiled with debug option.)


Takashi