On 06/07/2013 01:54 AM, Sarah Sharp wrote:
On Thu, Jun 06, 2013 at 09:37:51PM +0300, Andy Shevchenko wrote:
On Thu, Jun 6, 2013 at 5:21 PM, Xenia Ragiadakou<burzalod...@gmail.com>  wrote:
This patch adds a check on whether the host machine
supports the xHC DMA address mask and sets the DMA
mask for coherent DMA address allocation via an
explicit call to dma_set_coherent_mask().

According to DMA-API-HOWTO, if coherent DMA address
mask has not been set explicitly via dma_set_coherent_mask(),
and the driver calls dma_alloc_coherent() or
dma_pool_create() to allocate consistent DMA memory
blocks, the consistent DMA mapping interface will
return by default DMA addresses which are 32-bit
addressable.

Hence, if 64-bit DMA mapping is supported, it
is appropriate to call dma_set_coherent_mask()
with DMA_BIT_MASK(64) to take advantage of it.

Also, according to DMA-API-HOWTO, dma_set_coherent_mask()
is guaranteed to set successfully the same or a smaller
mask as dma_set_mask().

It looks for me overcomplicated.

Why?

We have *dma_mask and dma_coherent mask in the struct device.
First question, who is allocating memory for dma_mask?

The xHCI driver is allocating memory for the host hardware structures,
and the USB core is using the DMA mask of the host controller in order
to move USB buffers into bounce buffers as necessary.

The xHCI driver allocates memory from both DMA pools and with kmalloc.
We need both calls to dma_set_coherent_mask() and
dma_set_coherent_mask() to get 64-bit DMA addresses for both types of
memory.

Second, in case of dma_mask == NULL, dma_set_mask fails with -EIO. It
doesn't mean we have no support of this one. How do you handle this
case?

When would the dma_mask be NULL?  Note that xHCI PCI hosts *must* be
able to support DMA.

I think it's pretty simple to set dma_coherent_mask and then apply its
address to the dma_mask, because you set the same values anyway.

I'm not quite understanding what you want to do.  (I'll blame lack of
sleep, sorry.)  Can you write some pseudo code for me?


Andy explained his concern in more detail to me irl.

The beef is that the dma_mask is a pointer while dma_coherent_mask is a u64 value in struct device, and dma_set_mask() fails if dev->dma_mask doesn't point anywhere.

This is not a problem for PCI enumerated devices because PCI probe sets a default 32bit mask and makes the dma_mask of all new PCI devices point to this value.

For non-pci enumerated devices the device dma_mask may be NULL and dma_set_mask() may fail even if dma is supported on the host machine.

for example ehci-platform.c solves this by adding:

if (!dev->dev.dma_mask)
    dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
if (!dev->dev.coherent_dma_mask)
    dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to