On 10/30/2014 04:05 PM, Arnd Bergmann wrote:
On Thursday 30 October 2014 15:09:33 Mark Langsdorf wrote:
On 10/30/2014 02:05 PM, Arnd Bergmann wrote:
On Thursday 30 October 2014 13:16:28 Mark Langsdorf wrote:
- /* 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.
You should not access pdev->dev.dma_mask here, that gets set
by the platform code. You should be able to just use
dma_set_mask_and_coherent to set both.
So:
if (sizeof(dma_addr_t) < 8 ||
dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
if (ret)
return ret;
}
This doesn't actually work for me. I experimented a bit on the
hardware and I always fail if I don't set the coherent mask
first.
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?
I don't know how the DMA mask gets initialized on ACPI, I assume it
doesn't at the moment, but that is something that should be fixed
in the ACPI code, not worked around in the driver.
You should definitely make sure that this also works with DT, as
I don't think it's possible to support X-Gene with ACPI. I know
that Al Stone has experimented with it in the past, but he never
came back with any results, so I assume the experiment failed.
I'm running my test code on an X-Gene with ACPI. Al Stone, Mark
Salter, and I got it working.
--Mark Langsdorf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
