[+cc Guenter from other thread: https://lore.kernel.org/r/20211129015909.GA921717@xxxxxxxxxxxx] On Wed, Dec 01, 2021 at 09:56:22PM +0100, Sergio Paracuellos wrote: > On Wed, Dec 1, 2021 at 9:24 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > On Fri, Nov 19, 2021 at 05:20:17PM -0600, Bjorn Helgaas wrote: > > > [+cc Thierry] > > > > > > In subject, > > > > > > PCI: Let pcibios_root_bridge_prepare() access bridge->windows > > > > > > On Mon, Nov 15, 2021 at 08:08:05AM +0100, Sergio Paracuellos wrote: > > > > When function 'pci_register_host_bridge()' is called, 'bridge->windows' are > > > > already available. However this windows are being moved temporarily from > > > > there. To let 'pcibios_root_bridge_prepare()' to have access to this windows > > > > move this windows movement after call this function. This is interesting for > > > > MIPS ralink mt7621 platform to be able to properly set I/O coherence units > > > > with this information and avoid custom MIPs code in generic PCIe controller > > > > drivers. > > > > > > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx> > > > > --- > > > > drivers/pci/probe.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > > > index 087d3658f75c..372a70efccc6 100644 > > > > --- a/drivers/pci/probe.c > > > > +++ b/drivers/pci/probe.c > > > > @@ -898,8 +898,6 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) > > > > > > > > bridge->bus = bus; > > > > > > > > - /* Temporarily move resources off the list */ > > > > - list_splice_init(&bridge->windows, &resources); > > > > > > Arnd added this with 37d6a0a6f470 ("PCI: Add > > > pci_register_host_bridge() interface") [1]. > > > > > > I can't remember why this was done, but we did go to some trouble to > > > move things around, so there must have been a good reason. > > > > > > Arnd or Thierry, do you remember? > > > > Nobody seems to remember, so I think we should go ahead and make this > > change after the usual due diligence (audit the code between the old > > site and the new site to look for any uses of bridge->windows). > > It seems any user of the pcibios_root_bridge_prepare() does nothing > with 'bridge->windows'. But I don't get the point of passing around a > complete bridge pointer if windows are temporarily removed from there. > That is an incomplete bridge and after parsing windows from dts are > supposed to be there... What do you mean with 'audit the code between > the old and new site'? I mean "look at all the code that is run between the old site and the new site to make sure that none of that code depends on bridge->windows being temporarily emptied." > > I think this would be material for v5.17. > > Do you prefer me to parse dts again inside > pcibios_root_bridge_prepare() for ralink mt7621?. Not real sense since > 'windows' should be already there, but it would be a way to get this > patchset added for v5.16. Something like (not tested yet but it should > work): This is definitely too big for v5.16, regardless of which way you go. For v5.16, the only thing that's practical is to avoid building as a module. It'd be *nice* if it could be built as a module, but it is not a requirement. Bjorn
