On Tue, 23 Aug 2011, Felipe Balbi wrote: > > > I wouldn't call it fake. You _do_ have an IP core with a PCIe bridge > > > around and that's what the patch tries to model. > > > > I don't understand. The IP core with the PCIe bridge already > > corresponds to a pci_device, doesn't it? So then what does the new > > not really. Take for example the DesignWare USB3 IP driver which I just > sent. The exact same IP can be configured (at RT level) as host only, > DRD and device-only. You'll have to forgive me for not being familiar with all the jargon. I don't remember what DRD is -- dual-role device? Is that like OTG? > The host side is XHCI. For the purpose of $SUBJECT let's talk about a > host-only configuration, so here it goes: > > That particular IP talks AXI. That's how the verilog code was written, What's AXI? (Or does it not really matter?) > still we have TUSB7340 which is a host-only configuration of that IP > (so an XHCI controller) on a PCIe card. Don't you agree that our HW > engineers had to create an AXI-to-PCIe bridge ? Don't you aggree those > are two different devices (on the same die) ? I have no idea what your engineers had to create. As for whether they are two different devices, that's somewhat in the eye of the beholder. Does the kernel need to program or configure the PCIe-to-AXI bridge device in any way? If not, then as far as the kernel is concerned, that device may as well not exist. But if the device does need some configuration, then I agree, it does belong sandwiched between the PCI and USB layers. > > Are you talking about something like the traditional arrangement > > whereby several controllers are bundled together as separate PCI > > functions in the same PCI slot? The way Intel boards have UHCI > > unfortunately not :-( > > > controllers at 00:1d.0, 00:1d.1, and 00:1d.2 plus an EHCI controller at > > 00:1d.7? Each of those PCI functions corresponds to a separate > > pci_device structure, which is the parent of the controller's root hub > > device. There's no need for an intervening platform device. > > see above. Okay. But consider this case for a moment. Merely because the OMAP implementation requires a bridge device between the PCI and USB layers, that doesn't mean the Intel implementation should be forced to use one. As I understand it, Sebastian's patch set would have created a fake platform-bus device even in the Intel case, not corresponding to any real hardware. Was my understanding wrong? > > I'm well aware of all that. I'm asking if there isn't a different way > > to accomplish the same result. > > I can't see any, if you come up with a better approach, I'm willing to > consider :-) That's what I was trying to present. Namely, write an xhci-core module, with routines that can be invoked by various driver modules such as xhci-pci or xhci-omap, and do it in such a way that these routines take a struct device as an argument -- and are agnostic as to whether that struct device is embedded in a struct pci_device or a struct platform_device. Maybe Sebastian's work can be changed to work like this, perhaps even without the need for a separate xhci-core module. That would be fine with me. All I really object to is creating a platform_device structure in cases where it doesn't correspond to any real hardware. > > > I don't think so. I think your approach is inverted. Why should a PCI > > > glue/bridge call into XHCI ? > > > > That's like asking why ehci-hcd should call into usbcore. It does so > > no, it's not. > > > because hcd-pci.c provides functions that it can use. > > There's nothing PCI provides that XHCI needs to know about. I can't see > the need to invert things. You read my reply backward. A PCI glue/bridge would call into XHCI because XHCI provides things that the glue/bridge can use -- namely, an xHCI controller driver. Not the other way around. Besides, PCI _does_ provide things XHCI needs to use. Things like memory-mapped IO addresses, interrupt numbers, and other resources. > > It can just as easily be passed by a PCI device driver with no need for > > a platform-bus glue layer. > > For a PCI-only context, of course. But when we need to add PCI-less SoC > devices, it starts to get funny and we will end up with ehci-hcd.c all > over again. That's okay. There's nothing wrong with adding glue layers when they are appropriate. But they shouldn't be added all the time if they are sometimes inappropriate. Finally, as far as the $SUBJECT patch is concerned, it looks like Sebastian was trying to avoid creating an hcd-platform.c source file by sharing the code in hcd-pci.c. Looking at it more closely, it appears to contain some layering violations. For example, consider where hcd_plat_pci_suspend() calls _suspend_common(). The first argument it passes, dev, points to the platform device whereas it's expected to point to the pci_device. I suspect the code sharing wasn't done correctly. Creating a new file instead might well work out better. The amount of code duplication probably wouldn't be all that large. Alan Stern -- 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
