Hi,

On Fri, Jun 07, 2013 at 10:19:23AM -0700, Sarah Sharp wrote:
> Adding Felipe.
> 
> Xenia, Felipe is the USB "gadget" maintainer, and also added code for
> non-PCI xHCI hosts.  Felipe, Xenia is my summer intern through the FOSS
> Outreach Program for Women: http://kernelnewbies.org/OPWIntro

Hey, cool Sarah :-)

Welcome Xenia :-)

> Felipe, we're discussing the fact that the xHCI host currently does not
> set up 64-bit coherent DMA, so we get 64-bit addresses for USB buffers,
> but not xHCI data structures.  Xenia has a patch to fix it, which can be
> found here: http://marc.info/?l=linux-usb&m=137052855612729&w=2
> 
> Andy thinks the patch has issues with non-PCI xHCI hosts.

Looking at the patch, I don't see why it would have any issues, unless
some HW engineer flipped the wrong bits in hcc_params, but that would be
a silicon bug anyway.

I don't have access to anything to test right now (on vacations, last
day :-( ) but I'll see if anyone from TI can help testing that patch.

Maybe Kishon can help us out  here ? Kishon, could you spend a few
minutes testing Xenia's patch on OMAP5 to make sure nothing goes wrong
with that platform.

Again, I can't see how it would fail.

> On Fri, Jun 07, 2013 at 03:00:47PM +0300, Mathias Nyman wrote:
> > 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);
> 
> So it sounds like xhci_plat_setup() in xhci-plat.c needs to do something
> similar before it calls xhci_gen_setup().  Felipe, do you agree?

(now, after reading the whole thing...)

I'll agree with that :)

> It's funny that the code worked before.  Perhaps setting the dma_mask
> pointer should be a separate patch for stable?

sounds like a good plan to me :-)

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to