On 10/30/2014 02:05 PM, Arnd Bergmann wrote:

On Thursday 30 October 2014 13:16:28 Mark Langsdorf wrote:

quoted

-       /* Initialize dma_mask and coherent_dma_mask to 32-bits */
-       ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
-       if (ret)
-               return ret;
+       /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
+       if (sizeof(dma_addr_t) < 8 ||
+               dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
+               ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+               if (ret)
+                       return ret;
+       }
         if (!pdev->dev.dma_mask)
                 pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
         else
-               dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+               dma_set_mask(&pdev->dev, pdev->dev.coherent_dma_mask);

         hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
         if (!hcd)

The logic here seems wrong: if dma_set_mask is successful, you
can rely on on dma_set_coherent_mask suceeding as well, but
not the other way round.

That's the order in the existing driver. Would you prefer I
switch it to:
	if (sizeof(dma_addr_t) < 8 ||
		dma_set_mask(&pdev->dev, DMA_BIT_MASK(64))) {
			ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
			if (ret)
				return ret;
	}
	dma_set_coherent_mask(&pdev->dev, pdev->dev.dma_mask);

or based on the comment below:
	ret = dma_set_mask(&pdev->dev, pdev->dev.dma_mask);
	if (ret)
		return ret;
	dma_set_coherent_mask(&pdev->dev, pdev->dev.dma_mask);

I prefer this version but I don't know if it would work.

Also, we should no longer need to worry about the case where
pdev->dev.dma_mask is NULL, as this now gets initialized from
the DT setup.

I'm running this on a system with ACPI enabled and no DT. Does
that make a difference?

--Mark Langsdorf