Date:   Wed, 1 Dec 2021 14:24:33 -0600
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     Sergio Paracuellos <sergio.paracuellos@gmail.com>
Cc:     linux-pci@vger.kernel.org, linux-mips@vger.kernel.org,
        tsbogend@alpha.franken.de, john@phrozen.org,
        lorenzo.pieralisi@arm.com, bhelgaas@google.com, arnd@arndb.de,
        linux-kernel@vger.kernel.org,
        Thierry Reding <thierry.reding@gmail.com>
Subject: Re: [PATCH 1/5] PCI: let 'pcibios_root_bridge_prepare()' access to
 'bridge->windows'
Message-ID: <20211201202433.GA2837547@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211119232017.GA1981034@bhelgaas>
Precedence: bulk

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@gmail.com>
> > ---
> >  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).

I think this would be material for v5.17.

> >  	bus->sysdata = bridge->sysdata;
> >  	bus->ops = bridge->ops;
> >  	bus->number = bus->busn_res.start = bridge->busnr;
> > @@ -925,6 +923,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
> >  	if (err)
> >  		goto free;
> >  
> > +	/* Temporarily move resources off the list */
> > +	list_splice_init(&bridge->windows, &resources);
> >  	err = device_add(&bridge->dev);
> >  	if (err) {
> >  		put_device(&bridge->dev);
> > -- 
> > 2.33.0
> > 
> 
> [1] https://git.kernel.org/linus/37d6a0a6f470