On Tue, 23 Aug 2011, Felipe Balbi wrote: > > There's no doubt that the patch is messy. > > > > This is because it tries to cater to two different approaches > > simultaneously: the old (use the PCI device directly) and the new > > (interpose a fake platform device below the PCI device). It would be > > 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 platform_device correspond to? Look, in general, things that bridge between two different kinds of bus get represented in the device model as two different device structures, one for each bus type. Thus, a USB mass-storage device bridges a USB bus with a (trivial) SCSI bus, and it is represented by the usb-storage interface and the SCSI host. Similarly, a USB controller bridges a PCI (or platform) bus with a USB bus, so it should be represented by a PCI (or platform) device and a USB root hub. Not a PCI device plus a fake platform device plus a root hub. > If it was the best > implementation, then we can discuss but the fact that we have TWO > devices on such PCIe cards is irrefutable. Same goes for EHCI/OHCI/UHCI > cores and those need to be changed too. What do you mean you have TWO devices on such PCIe cards? 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 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. > > Personally, I think it's more than a little odd to have the fake > > platform device in the middle like that. It also runs the risk of > > messing up existing userspace programs that do power management on the > > controller device; they might easily contain references to paths like > > /sys/bus/usb/devices/usb3/../power/. Certainly I have done things like > > that in the past. > > This would mean those userspace programs are actually broken if they > have hard-coded paths instead of trying to find the correct devices. In this case, the device the program is looking for is the parent of usb3. The path I gave is the correct way to find that parent; no searching is needed. > All in all, we are trying to prevent that big pile of ifdeferry we have > on e.g. ehci-hcd.c. I'm well aware of all that. I'm asking if there isn't a different way to accomplish the same result. > > For example, maybe the core parts of xhci-hcd could be made into a > > standalone module, containing routines that would be called by separate > > driver modules (one for xhci-pci and others for various platform > > variants). There would be a disadvantage, because now two modules > > would have to be loaded to manage an xHCI controller, but overall it > > would be cleaner. > > 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 because hcd-pci.c provides functions that it can use. > All XHCI needs is a memory base and an IRQ > number (well, quirks, PM, etc) and that can be easily passed by the "bus > integration context" which I'm calling PCI glue/bridge on this mail. It can just as easily be passed by a PCI device driver with no need for a platform-bus glue layer. > > And of course, the same approach could be applied to the other joint > > PCI/platform HCDs. > > you're only thinking about PCI and for embedded platforms we don't have > PCI. Most will have some OCP, or AXI/AHB bus and if we don't decouple > PCI from the host stack we will have the current ifdef mess we're into. You don't seem to understand that I am proposing a way to do just that: decouple PCI from the host stack. It's just that I'm proposing a _different_ way from what Sebastian posted. 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
