[alsa-devel] [PATCH] ASoC: omap-pcm: Lower the dma coherent mask to 32bits

[Takashi Iwai]

At Thu, 5 Dec 2013 21:07:07 +0000,
Russell King - ARM Linux wrote:
> 
> On Thu, Dec 05, 2013 at 09:11:50PM +0100, Takashi Iwai wrote:
> > Does the check of dma_to_pfn(dev, ~0) is really necessary?
> 
> Of course.
> 
> > In get_coherent_dma_mask(), it checks further
> > 
> > 		if (dma_to_pfn(dev, mask) < max_dma_pfn) {
> > 
> > and since mask > ~0, this check should suffice, I think.
> 
> dma_to_pfn() takes a dma_addr_t as the second argument.  At this point,
> we've ascertained that 'mask' is larger than dma_addr_t, so the above
> will truncate the mask.

Ah, I missed that point...

> The whole point of this is to protect the following code which does
> exactly that from this truncation.

OK, I understand that part.  Thanks for clarification!


But, it's still unclear why only get_coherent_dma_mask() takes max_pfn
into account for the check of dma_to_pfn(dev, mask).

In dma_supported(), 

	unsigned long limit;
	limit = dma_to_pfn(dev, mask);
	if (limit < arm_dma_pfn_limit)
		return 0;

while in get_coherent_dma_mask(),

	unsigned long max_dma_pfn;
	max_dma_pfn = min(max_pfn, arm_dma_pfn_limit);
	if (dma_to_pfn(dev, mask) < max_dma_pfn)
		return 0;

So, the current code looks to me that the results from
dma_set_coherent_mask() and the actual allocation may conflict.

If adding warnings is the purpose of the open code, we can create a
function like __dma_supported(struct device *dev, u64 mask, bool warning)
and call it from the both places.


Takashi