On Tue, Aug 30, 2011 at 10:50 PM, Deng-Cheng Zhu <dczhu@xxxxxxxx> wrote: > Hi, Bjorn > > > Thanks for your constructive review. > >> One logistical issue here is that the first patch touches several >> architectures at once, which puts Jesse in a bit of a pinch. If he >> applies it, there's always the possibility that an arch patch will >> conflict with it, which makes merging harder. > > In case the conflicts happen, the effort to resolve them should be > trivial (a matter of an extra NULL argument), I suppose. Also, the odds > of other incoming arch patches making a reference to pci_create_bus() > should not be great. > >> It might be easier if, instead of changing the pci_create_bus() >> interface, you added a new one (it could call pci_create_bus() then >> replace the resources, so the implementation could still be mostly >> shared.) We already have a plethora of "create bus" methods >> (pci_create_bus(), pci_scan_bus_parented(), pci_scan_bus()), but if >> you added a pci_create_root_bus() or something similar, maybe we could >> try to converge on it and obsolete the others. >> >> Then the first patch would touch only the PCI core, and the second >> would touch only MIPS, which would make merging more straightforward. >> > > Hmm.. Adding a wrapper of pci_create_bus() does leave other > architectures alone for this merging. But before all of them converge on > it (a long way to go), the wrapper is adding naming confusion to the > PCI core. Personally I think the current low-level transparent change to > pci_create_bus() is appropriate enough. Does anybody have comments? > > > Deng-Cheng Just to be clear, I'm fine with it either way, as long as Jesse and the arch maintainers are OK with it.
