On Wed, May 20, 2015 at 02:02:52PM +0100, Bjorn Helgaas wrote: [...] > >> @@ -48,6 +48,11 @@ int pcibios_add_device(struct pci_dev *dev) > >> > >> dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); > >> > >> + if (pci_has_flag(PCI_PROBE_ONLY) && > > > > Does it really matter if PCI_PROBE_ONLY is set ? > > > >> + !pci_is_root_bus(dev->bus) && > > > > This check is useless, since pci_read_bridge_bases checks that already. > > > >> + !pci_bridge_bases_is_read(dev->bus)) > >> + pci_read_bridge_bases(dev->bus); > >> + > > > > Ok. Most of the archs do that in pcibios_fixup_bus, I would like to > > understand: > > > > 1) Should we do it on PCI_PROBE_ONLY only > > 2) Can we move this to generic code - ie pci_scan_child_bus (I guess answer > > is no, because there are corner cases I am not aware of) > > In my opinion, we should call pci_read_bridge_bases() in all cases, > regardless of PCI_PROBE_ONLY. pci_read_bridge_bases() doesn't > *change* anything in the hardware; it only reads what's there. (It > should attempt writes to learn whether I/O and prefetchable memory > apertures are implemented, but those should be done as in > pci_bridge_check_ranges(), where the original value is restored.) > > I also think this should be done in generic code, since there's > nothing architecture-specific about bridge operation. > > I've been hoping to get rid of pcibios_fixup_bus() completely for > years, and doing pci_read_bridge_bases() in generic code would be a > big step. I put together a patch to move it to pci_scan_child_bus, and to remove it from almost all archs pcibios_fixup_bus implementations, let's see how it goes. > No doubt there are corner cases we'll trip over, but I'm not aware of them yet. > Let's find out :), posting tomorrow. Thanks, Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html