At Tue, 06 Jun 2006 13:17:27 +0200,
Johannes Berg wrote:
>
> On Fri, 2006-06-02 at 16:23 +0200, Takashi Iwai wrote:
> > > + 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).
> > > + /* 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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/alsa-devel
