Hi, On Tue, Aug 23, 2011 at 05:33:20PM -0400, Alan Stern wrote: > On Tue, 23 Aug 2011, Felipe Balbi wrote: > > > > 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. > > > > Alan, my whole point is that this is hardly an OMAP-only thing. Just > > look into the many different ARM SoCs we have. > > All right, try this instead: Merely because OMAP and a bunch of other > SoC architectures require a bridge device between the PCI and USB > layers, that doesn't mean the Intel implementation should be forced to > use one. that doesn't mean either that Intel couldn't license the same IP the ARM SoCs are licensing. > > The way things go at the level is > > somewhat below (in a very bad pseudo-code): > > > > module xhci_hcd ( > > <list of inputs>, > > <list of outputs>, > > <CPU-related signals>, > > <IRQ line signal>, > > <DMA related signals>, > > <etc etc etc>); > > > > CPU_interface_block(<assign inputs and outputs correctly>); > > real_usb_logic(<assign inputs and outputs correctly>); > > > > so there is a block which handles the BUS interconnection logic. Thet > > CPU_interface_block is decoding the bus protocol. No matter if it's PCI, > > AXI, OCP, AHB, or whatever else, you will have some entity handly the > > integration with the CPU/SoC/Bus. > > So what? Sure, every PCI device has such an entity. Does that mean > every pci_device structure needs to have a platform_device child? why not ? Then we split the Integration logic (PCI, OMAP, FreeScale, Marvel, etc) from the IP driver. > > > 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. > > > > Fine, then let's duplicate. I just thought the whole point was to share > > code where possible to avoid duplicating. The driver tree has been > > adding code at a very fast rate and sooner or later we will be the next > > target wrt to code sharing. > > At this point it's not clear how much code Sebastian really ended up > sharing. I've got a feeling it wasn't very much. And in the process, > he made a mess of hcd-pci.c. Separating the files could easily end up > being better. > > Besides, I'm talking about adding a single file that would handle _all_ > the platform devices. Not a separate file for each architecture or > platform. And I'm talking about not adding a new file at all. After all *hci-hcd are converted, the only bus needing a bridge would PCI (sorry); all others could pass correct resources via devicetree and instantiate xhci-core directly. -- balbi
Attachment:
signature.asc
Description: Digital signature
