On Tue, Jun 14, 2011 at 03:39:30PM +0300, Felipe Balbi wrote:
> Hi Sarah,
>
> following we have two patches for XHCI. The first
> one can be applied now and it's just a Kconfig change.
>
> The second one, I wanted to discuss with you in more
> detail. It's adding a platform_device to xhci.c. I
> want to do that because it will us to add more xhci
> glue/link/name-it-as-you-wish layers without having
> to touch xhci.c and will avoid the ifdeffery mess
> we have on EHCI/OHCI modules.
I'm not familiar with the platform device code, but I would like to
avoid excessive ifdefferies. :)
> The idea is that we will always have (at least I
> think) memory mapped controllers,
I'm pretty sure the xHCI spec requires memory mapped controllers. I
think it may also require interrupts, although I suppose a driver could
poll the event ring if interrupts aren't possible.
> so it doesn't
> really matter if it's PCI or integrated in a SoC
> environment; as long as we long we setup the
> IP integration context, the core driver shouldn't
> really care where it's running.
Do you think platform xHCI drivers in an SOC will need to careful how
much memory they allocate for xHCI structures? For example, the ring
segments could be smaller if necessary, as long as all the segments are
the same size.
> If you need a full
> implementation where I'm using this, you see my
> (still out of tree) dwc3 driver which is the driver
> for the OMAP5 USB3 controller. I have the same IP
> core for OMAP5 and on a PCIe pre-silicon environment
> for development purposes. The PCI glue layer you
> can find at [1], OMAP5 glue layer at [2] and core
> driver at [3].
>
> Anyway, here are the details:
>
> xhci-pci.c allocates a platform_device for xhci.c
> and sets itself as parent of that device. We also
> need to set dma_mask, coherent_dma_mask and dma_parms
> to the same as the parent device. Like so:
>
> pci_set_drvdata(pci, glue);
>
> dma_set_coherent_mask(&xhci->dev, pci->dev.coherent_dma_mask);
>
> xhci->dev.dma_mask = pci->dev.dma_mask;
> xhci->dev.dma_parms = pci->dev.dma_parms;
> xhci->dev.parent = &pci->dev;
> glue->dev = &pci->dev;
> glue->xhci = dwc3;
>
> You also fetch the pci_resources only to struct
> resources for the xhci platform_device, like so:
>
> res[0].start = pci_resource_start(pci, 0);
> res[0].end = pci_resource_end(pci, 0);
> res[0].name = "memory";
> res[0].flags = IORESOURCE_MEM;
>
> res[1].start = pci->irq;
> res[1].name = "irq";
> res[1].flags = IORESOURCE_IRQ;
How does this interact with the MSI/MSI-X setup in xhci.c? If we end up
not using the legacy PCI interrupt, how does that interact with the
platform device code?
>
> ret = platform_device_add_resources(xhci, res, ARRAY_SIZE(res));
> if (ret) {
> dev_err(&pci->dev, "couldn't add resources to xhci device\n");
> goto err;
> }
>
> after that, all you have to do is add the platform_device:
>
> ret = platform_device_add(xhci);
> if (ret) {
> dev_err(&pci->dev, "failed to register xhci device\n");
> goto err;
> }
>
> The main idea, is to be able to add glue layers
> without having to touch the core driver, which
> will make bugs easier to track down and localized
> to particular integration contexts.
Sure, sounds like a good plan.
> It will also avoid us having to provide basically
> the same struct hc_driver every time.
>
> If we need to differentiate a few of the glue
> layers and act differently based on a few small
> details, we can use the driver_data field of
> struct platform_device_id:
>
> static const struct platform_device_id xhci_id_table[] __devinitconst = {
> {
> .name = "xhci-omap",
> .driver_data = &xhci_omap_data,
> {
> .name = "xhci-pci",
> .driver_data = &xhci_pci_data,
> },
> { }, /* Terminating Entry */
> };
>
> But I doubt we will need something like that
> any time soon.
>
> What do you think of this idea ? Do you see any
> terrible drawbacks which I didn't see ? We have
> two drivers using this (MUSB and DWC3) and it
> has been quite good solution thus far.
I'll admit that I don't really know how MUSB works. So there's an MUSB
platform device that can be both an xHCI host controller and a USB3
device controller?
> udev
> will load the correct modules anyway based on
> device creation.
Do you mean udev, the userspace program? And if so, does that mean we
have to wait for userspace bring up before the xHCI driver probes a
device? (A little confused here.) How would that interact with booting
off of a USB device?
Sarah Sharp
> [1] http://git.kernel.org/?p=linux/kernel/git/balbi/usb.git;a=blob;f=drivers/usb/dwc3/dwc3-pci.c;h=dfe114f36fc11c89c024afa44f0af45c2520ff0e;hb=refs/heads/dwc3
>
> [2] http://git.kernel.org/?p=linux/kernel/git/balbi/usb.git;a=blob;f=drivers/usb/dwc3/dwc3-omap.c;h=724962fdaa8924069790e84a90abd1bd08e8a168;hb=refs/heads/dwc3
>
> [3] http://git.kernel.org/?p=linux/kernel/git/balbi/usb.git;a=blob;f=drivers/usb/dwc3/core.c;h=1ba1408638df2c4c307b576d8435b7102cb5fd02;hb=refs/heads/dwc3
--
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
