From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 27 May 2015 18:54:47 +0100 From: Lorenzo Pieralisi To: Suravee Suthikulanit Cc: "linux-kernel@vger.kernel.org" , "linux-pci@vger.kernel.org" , Ralf Baechle , "James E.J. Bottomley" , Michael Ellerman , Bjorn Helgaas , Richard Henderson , Benjamin Herrenschmidt , David Howells , Russell King , Tony Luck , "David S. Miller" , Ingo Molnar , Michal Simek , "yasutake.koichi@jp.panasonic.com" , Chris Zankel , Arnd Bergmann , Krzysztof Halasa , Phil Edworthy , Jason Gunthorpe , Jingoo Han , Lucas Stach , Simon Horman , Minghuan Lian , Murali Karicheri , Tanmay Inamdar , Kishon Vijay Abraham I , Thierry Reding , Thomas Petazzoni , Will Deacon , Jayachandran C Subject: Re: [RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer Message-ID: <20150527175447.GA7914@red-moon> References: <1432214067-2752-1-git-send-email-lorenzo.pieralisi@arm.com> <5565F40C.8010004@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <5565F40C.8010004@amd.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: On Wed, May 27, 2015 at 05:42:52PM +0100, Suravee Suthikulanit wrote: > Hi Lorenzo, > > Sorry for late reply. > > On 5/21/2015 8:14 AM, Lorenzo Pieralisi wrote: > > When a PCI bus is scanned, upon PCI bridge detection the kernel > > has to read the bridge registers to set-up its resources so that > > the PCI resource hierarchy can be validated properly. > > > > Most if not all architectures read PCI bridge registers in the > > pcibios_fixup_bus hook, that is called by the PCI generic layer > > whenever a PCI bus is scanned. > > > > Since pci_read_bridge_bases is an arch agnostic operation (and it > > is carried out on all architectures) it can be moved to the generic > > PCI layer in order to consolidate code and remove the respective > > calls from the architectures back-ends. > > > > The PCI_PROBE_ONLY flag is not checked before calling > > pci_read_bridge_buses in the generic layer since reading the bridge > > bases is not related to resources assignment; this implies that it > > can be carried out safely on PCI_PROBE_ONLY systems too and should > > not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY > > flag before reading the bridge bases. > > > > Signed-off-by: Lorenzo Pieralisi > > Cc: Ralf Baechle > > Cc: James E.J. Bottomley > > Cc: Michael Ellerman > > Cc: Bjorn Helgaas > > Cc: Richard Henderson > > Cc: Benjamin Herrenschmidt > > Cc: David Howells > > Cc: Russell King > > Cc: Tony Luck > > Cc: David S. Miller > > Cc: Ingo Molnar > > Cc: Michal Simek > > Cc: Koichi Yasutake > > Cc: Chris Zankel > > --- > > arch/alpha/kernel/pci.c | 7 +------ > > arch/frv/mb93090-mb00/pci-vdk.c | 2 -- > > arch/ia64/pci/pci.c | 1 - > > arch/microblaze/pci/pci-common.c | 9 +-------- > > arch/mips/pci/pci.c | 6 ------ > > arch/mn10300/unit-asb2305/pci.c | 1 - > > arch/powerpc/kernel/pci-common.c | 8 +------- > > arch/x86/pci/common.c | 1 - > > arch/xtensa/kernel/pci.c | 4 ---- > > drivers/parisc/dino.c | 3 --- > > drivers/parisc/lba_pci.c | 1 - > > drivers/pci/probe.c | 11 ++++++++++- > > 12 files changed, 13 insertions(+), 41 deletions(-) > > > > [.....] > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index 062fee6..335d9f2 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -453,7 +453,11 @@ void pci_read_bridge_bases(struct pci_bus *child) > > struct resource *res; > > int i; > > > > - if (pci_is_root_bus(child)) /* It's a host bus, nothing to read */ > > + /* > > + * If it is not a PCI bridge there is nothing to read > > + */ > > + if (pci_is_root_bus(child) || !dev || > > + !((dev->class >> 8) == PCI_CLASS_BRIDGE_PCI)) > > return; > > > > dev_info(&dev->dev, "PCI bridge to %pR%s\n", > > @@ -1878,6 +1882,11 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus) > > * all PCI-to-PCI bridges on this bus. > > */ > > if (!bus->is_added) { > > + /* > > + * Read and initialize bridge resources. > > + */ > > + pci_read_bridge_bases(bus); > > + > > dev_dbg(&bus->dev, "fixups for bus\n"); > > pcibios_fixup_bus(bus); > > bus->is_added = 1; > > > > So, I have tested the patch on ARM64 system w/ PROBE_ONLY mode, and > noticed that we are calling pci_read_bridge_bases() after adding the > devices on the slots. This is not soon enough since the downstream > devices still failing to claim resources. > > However, do you think we can move pci_read_bridge_bases() before the > pci_scan_slot() loop? Right, how about moving it to pci_scan_bridge() before calling the respective pci_scan_child_bus() ? I think it belongs there anyway. Thanks, Lorenzo