On Thu, 08 Jun 2017 01:35:29 PDT (-0700), Arnd Bergmann wrote: > On Thu, Jun 8, 2017 at 10:12 AM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: >> On Wed, Jun 07, 2017 at 09:19:49AM +0200, Geert Uytterhoeven wrote: >>> CC pci folks >> >> Ok, replying with pci folks in Cc then :) >> >> Weak symbols have (rightly) gotten a bad reputation, so maybe >> we should approach this without them. > > Agreed, I would almost never recommend using a __weak symbol, > but they are already widely used in the PCI subsystem, so I suggested > using them here for consistency. > > We have a struct pci_host_bridge now, and we should eventually > move most of the 38 pci specific weak per-architecture symbols > into per-host driver callbacks, but that I think that would be too much > to ask for when adding an architecture port. I agree :) >> It seems we have a large number of emptry pcibios_fixup_bus calls >> alreayd, so I think we should simply have the architectures >> that do define it define a Kconfig or header symbol and not call >> it at all otherwise. > > I would argue that most of them should not be per-architecture > in the first place, the current state is mostly an artifact of the > times when each architecture had just one PCI implementation. > > The ones that have multiple implementations (arm, powerpc, ...) > tend to actually override the weak functions with their own > per-host multiplexers again. > >> For the ones that exist as lot just seem to call pci_read_bridge_bases >> and/or pcibios_fixup_device_resources in one form or another, >> and I wonder why we even need the arch indirection for that. >> >> Similarly for pcibios_align_resource: a lot of architetures seem >> to have a noop, and the once that don't mostly seem copy and >> paste code, so we should again have a symbol for architectures >> to opt into it, and we probably should have a generic helper >> for the VGA window mirroring code instead of duplicating it multiple >> times. > > I now remember that we already have a host_bridge->align_resource > callback pointer, so the generic function should definitely try > to use that. > > We could just use the version from arch/mips/pci/pci-generic.c > and remove that in the process. I'm not sure about why mips calls > pci_read_bridge_bases() in pcibios_fixup_bus() though, or whether > this make sense to put in the generic version. I'm splitting this off into another patch set and sending it to a bunch of PCI people.
