Re: [PATCH v2 1/2] ALSA: Add SoC on-chip internal memory support for DMA buffer allocation

[Nicolin Chen <b42378@freescale.com>]

On Wed, Oct 16, 2013 at 11:54:38AM +0200, Takashi Iwai wrote:
> At Wed, 16 Oct 2013 17:34:45 +0800,
> Nicolin Chen wrote:
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -3201,12 +3201,18 @@ int snd_pcm_lib_default_mmap(struct snd_pcm_substream *substream,
> >  	area->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
> >  #ifdef ARCH_HAS_DMA_MMAP_COHERENT
> >  	if (!substream->ops->page &&
> > -	    substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV)
> > +	    substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV) {
> >  		return dma_mmap_coherent(substream->dma_buffer.dev.dev,
> >  					 area,
> >  					 substream->runtime->dma_area,
> >  					 substream->runtime->dma_addr,
> >  					 area->vm_end - area->vm_start);
> > +	} else if (substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV_IRAM) {
> > +		area->vm_page_prot = pgprot_writecombine(area->vm_page_prot);
> > +		return remap_pfn_range(area, area->vm_start,
> > +				substream->dma_buffer.addr >> PAGE_SHIFT,
> > +				area->vm_end - area->vm_start, area->vm_page_prot);
> > +	}
> 
> Here is a wrong place to put.  The handling of SNDRV_DMA_TYPE_IRAM
> must be irrelevant with the existence of dma_mmap_coherent(), thus it
> should be outside the ifdef block.
> 
> In addition, you need the check whether SNDRV_DMA_TYPE_DEV_IRAM !=
> SNDRV_DMA_TYPE_DEV.

Thank you, Iwai-san.
I'll fix it in v3.

> 
> 
> thanks,
> 
> Takashi
>