From: Greg KH on
On Wed, Apr 07, 2010 at 03:13:11PM -0400, Alan Stern wrote:
> On Wed, 7 Apr 2010, Takashi Iwai wrote:
>
> > > Ok, I'll write some dummies for usb_malloc() and usb_zalloc() which
> > > will just call kmalloc() with GFP_DMA32 for now.
> >
> > Can't we provide only zalloc() variant? Zero'ing doesn't cost much,
> > and the buffer allocation shouldn't be called too often.
>
> Linus specifically requested us to avoid using kzalloc in usbfs. I
> can't find the message in the email archives, but Greg KH should be
> able to confirm it.
>
> As long as we're imitating kmalloc for one use, we might as well make
> it available to all.
>
> > > And while at it,
> > > usb_alloc_buffer() will be renamed to usb_alloc_consistent().
> >
> > Most of recent functions are named with "coherent".
>
> Yes, the terminology got a little confused between the PCI and DMA
> realms. I agree, "coherent" is better.
>
> BTW, although some EHCI controllers may support 64-bit DMA, the driver
> contains this:
>
> if (HCC_64BIT_ADDR(hcc_params)) {
> ehci_writel(ehci, 0, &ehci->regs->segment);
> #if 0
> // this is deeply broken on almost all architectures
> if (!dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64)))
> ehci_info(ehci, "enabled 64bit DMA\n");
> #endif
> }
>
> I don't know if the comment is still true, but until the "#if 0" is
> removed, ehci-hcd won't make use of 64-bit DMA.

I think someone tried to remove it recently, but I wouldn't let them :)

What a mess, hopefully xhci will just take over and save the world from
this whole thing...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
On Wed, 7 Apr 2010, Robert Hancock wrote:

> >> The problem is appearantly the way the transfer buffer is allocated in
> >> the drivers. In the snd-usb-caiaq driver, I used kzalloc() to get memory
> >> which works fine on 32bit systems. On x86_64, however, it seems that
> >> kzalloc() hands out memory beyond the 32bit addressable boundary, which
> >> the DMA controller of the 32bit PCI-connected EHCI controller is unable
> >> to write to or read from. Am I correct on this conclusion?
> >
> > That seems like the right answer. You are correct that an EHCI
> > controller capable only of 32-bit memory accesses would not be able to
> > use a buffer above the 4 GB line.

> AFAIK, the driver shouldn't have to worry about this at all. When the
> buffer gets DMA-mapped for the controller, the DMA mapping code should
> see that the device has a 32-bit DMA mask and either bounce or IOMMU-map
> the memory so that it appears below 4GB.

That's true. It would of course be more efficient for the buffer to be
allocated below 4 GB, but it should work okay either way. Daniel, do
you have any idea why it fails?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Daniel Mack on
On Wed, Apr 07, 2010 at 10:10:17PM -0400, Alan Stern wrote:
> On Wed, 7 Apr 2010, Robert Hancock wrote:
>
> > >> The problem is appearantly the way the transfer buffer is allocated in
> > >> the drivers. In the snd-usb-caiaq driver, I used kzalloc() to get memory
> > >> which works fine on 32bit systems. On x86_64, however, it seems that
> > >> kzalloc() hands out memory beyond the 32bit addressable boundary, which
> > >> the DMA controller of the 32bit PCI-connected EHCI controller is unable
> > >> to write to or read from. Am I correct on this conclusion?
> > >
> > > That seems like the right answer. You are correct that an EHCI
> > > controller capable only of 32-bit memory accesses would not be able to
> > > use a buffer above the 4 GB line.
>
> > AFAIK, the driver shouldn't have to worry about this at all. When the
> > buffer gets DMA-mapped for the controller, the DMA mapping code should
> > see that the device has a 32-bit DMA mask and either bounce or IOMMU-map
> > the memory so that it appears below 4GB.
>
> That's true. It would of course be more efficient for the buffer to be
> allocated below 4 GB, but it should work okay either way. Daniel, do
> you have any idea why it fails?

No, and I can't do real tests as I lack a 64bit machine. I'll do some
more investigation later today, but for now the only explanation I have
is that not the remapped DMA buffer is used eventually by the EHCI code
but the physical address of the original buffer.

It would of course be best to fix the whole problem at this level, if
possible.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Oliver Neukum on
Am Mittwoch, 7. April 2010 17:35:51 schrieb Daniel Mack:
> > Alan, any objection to just using usb_buffer_alloc() for every driver?
> > Or is that too much overhead?
>
> FWIW, most drivers I've seen in the past hours use a wild mix of
> kmalloc(), kzalloc(), kcalloc() and usb_buffer_alloc(). That should
> really be unified.

kmalloc() & friends != usb_buffer_alloc(). They do different things.
It makes no sense to unify them. If you really need an ordinary
buffer DMA will surely work on, this needs a third primitive.

Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Daniel Mack on
On Thu, Apr 08, 2010 at 08:09:11AM +0200, Oliver Neukum wrote:
> Am Mittwoch, 7. April 2010 17:35:51 schrieb Daniel Mack:
> > > Alan, any objection to just using usb_buffer_alloc() for every driver?
> > > Or is that too much overhead?
> >
> > FWIW, most drivers I've seen in the past hours use a wild mix of
> > kmalloc(), kzalloc(), kcalloc() and usb_buffer_alloc(). That should
> > really be unified.
>
> kmalloc() & friends != usb_buffer_alloc(). They do different things.

I know. I just believe that many developers used usb_buffer_alloc() even
though they don't really need coherent DMA memory. The function's name
is misleading, and copy'n paste does the rest.

> It makes no sense to unify them. If you really need an ordinary
> buffer DMA will surely work on, this needs a third primitive.

I think it will help a lot to rename usb_buffer_alloc() in the first
place and then reconsider where coherent memory is really needed.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/