[RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer

[RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer

[Lorenzo Pieralisi]

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 <lorenzo.pieralisi@arm.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Tony Luck <tony.luck@intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Koichi Yasutake <yasutake.koichi@jp.panasonic.com>
Cc: Chris Zankel <chris@zankel.net>
---
 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/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 82f738e..cded02c 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -242,12 +242,7 @@ pci_restore_srm_config(void)
 
 void pcibios_fixup_bus(struct pci_bus *bus)
 {
-	struct pci_dev *dev = bus->self;
-
-	if (pci_has_flag(PCI_PROBE_ONLY) && dev &&
- 		   (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
- 		pci_read_bridge_bases(bus);
-	} 
+	struct pci_dev *dev;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		pdev_save_srm_config(dev);
diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index f211839..f9c86c4 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -294,8 +294,6 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 	printk("### PCIBIOS_FIXUP_BUS(%d)\n",bus->number);
 #endif
 
-	pci_read_bridge_bases(bus);
-
 	if (bus->number == 0) {
 		struct pci_dev *dev;
 		list_for_each_entry(dev, &bus->devices, bus_list) {
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index d4e162d..14d06f8 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -527,7 +527,6 @@ void pcibios_fixup_bus(struct pci_bus *b)
 	struct pci_dev *dev;
 
 	if (b->self) {
-		pci_read_bridge_bases(b);
 		pcibios_fixup_bridge_resources(b->self);
 	}
 	list_for_each_entry(dev, &b->devices, bus_list)
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index ae838ed..6b8b752 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -863,14 +863,7 @@ void pcibios_setup_bus_devices(struct pci_bus *bus)
 
 void pcibios_fixup_bus(struct pci_bus *bus)
 {
-	/* When called from the generic PCI probe, read PCI<->PCI bridge
-	 * bases. This is -not- called when generating the PCI tree from
-	 * the OF device-tree.
-	 */
-	if (bus->self != NULL)
-		pci_read_bridge_bases(bus);
-
-	/* Now fixup the bus bus */
+	/* Fixup the bus */
 	pcibios_setup_bus_self(bus);
 
 	/* Now fixup devices on that bus */
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index b8a0bf5..c6996cf 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -311,12 +311,6 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
 
 void pcibios_fixup_bus(struct pci_bus *bus)
 {
-	struct pci_dev *dev = bus->self;
-
-	if (pci_has_flag(PCI_PROBE_ONLY) && dev &&
-	    (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
-		pci_read_bridge_bases(bus);
-	}
 }
 
 EXPORT_SYMBOL(PCIBIOS_MIN_IO);
diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
index 3dfe2d3..deaa893 100644
--- a/arch/mn10300/unit-asb2305/pci.c
+++ b/arch/mn10300/unit-asb2305/pci.c
@@ -324,7 +324,6 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 	struct pci_dev *dev;
 
 	if (bus->self) {
-		pci_read_bridge_bases(bus);
 		pcibios_fixup_bridge_resources(bus->self);
 	}
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 0d05406..722dd5f 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1043,13 +1043,7 @@ void pcibios_set_master(struct pci_dev *dev)
 
 void pcibios_fixup_bus(struct pci_bus *bus)
 {
-	/* When called from the generic PCI probe, read PCI<->PCI bridge
-	 * bases. This is -not- called when generating the PCI tree from
-	 * the OF device-tree.
-	 */
-	pci_read_bridge_bases(bus);
-
-	/* Now fixup the bus bus */
+	/* Fixup the bus */
 	pcibios_setup_bus_self(bus);
 
 	/* Now fixup devices on that bus */
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 8fd6f44..3bff244 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -166,7 +166,6 @@ void pcibios_fixup_bus(struct pci_bus *b)
 {
 	struct pci_dev *dev;
 
-	pci_read_bridge_bases(b);
 	list_for_each_entry(dev, &b->devices, bus_list)
 		pcibios_fixup_device_resources(dev);
 }
diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
index b848cc3..d27b4dc 100644
--- a/arch/xtensa/kernel/pci.c
+++ b/arch/xtensa/kernel/pci.c
@@ -210,10 +210,6 @@ subsys_initcall(pcibios_init);
 
 void pcibios_fixup_bus(struct pci_bus *bus)
 {
-	if (bus->parent) {
-		/* This is a subordinate bridge */
-		pci_read_bridge_bases(bus);
-	}
 }
 
 void pcibios_set_master(struct pci_dev *dev)
diff --git a/drivers/parisc/dino.c b/drivers/parisc/dino.c
index a0580af..baec33c 100644
--- a/drivers/parisc/dino.c
+++ b/drivers/parisc/dino.c
@@ -560,9 +560,6 @@ dino_fixup_bus(struct pci_bus *bus)
 	} else if (bus->parent) {
 		int i;
 
-		pci_read_bridge_bases(bus);
-
-
 		for(i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
 			if((bus->self->resource[i].flags & 
 			    (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index dceb9dd..901e1a3 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -693,7 +693,6 @@ lba_fixup_bus(struct pci_bus *bus)
 	if (bus->parent) {
 		int i;
 		/* PCI-PCI Bridge */
-		pci_read_bridge_bases(bus);
 		for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++)
 			pci_claim_bridge_resource(bus->self, i);
 	} else {
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;
-- 
2.2.1

Re: [RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer

[Will Deacon]

Hi Lorenzo,

On Thu, May 21, 2015 at 02:14:25PM +0100, 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.

[...]

> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index 82f738e..cded02c 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -242,12 +242,7 @@ pci_restore_srm_config(void)
>  
>  void pcibios_fixup_bus(struct pci_bus *bus)
>  {
> -	struct pci_dev *dev = bus->self;
> -
> -	if (pci_has_flag(PCI_PROBE_ONLY) && dev &&
> - 		   (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
> - 		pci_read_bridge_bases(bus);
> -	} 

I appreciate you're basically moving this code ...

> 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;

... to here, but shouldn't we dispense with the class check and use
pci_is_bridge instead? pci_read_bridge_bases is making an assumption
that the header type is BRIDGE, so that seems like the more sensible
check to me.

Also, with these changes, can we now make pci_read_bridge_bases static
(as it has an implicit rdering requirement on the device having been
scanned)?

Will

Re: [RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer

[Lorenzo Pieralisi]

On Tue, May 26, 2015 at 01:57:59PM +0100, Will Deacon wrote:
> Hi Lorenzo,
> 
> On Thu, May 21, 2015 at 02:14:25PM +0100, 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.
> 
> [...]
> 
> > diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> > index 82f738e..cded02c 100644
> > --- a/arch/alpha/kernel/pci.c
> > +++ b/arch/alpha/kernel/pci.c
> > @@ -242,12 +242,7 @@ pci_restore_srm_config(void)
> >  
> >  void pcibios_fixup_bus(struct pci_bus *bus)
> >  {
> > -	struct pci_dev *dev = bus->self;
> > -
> > -	if (pci_has_flag(PCI_PROBE_ONLY) && dev &&
> > - 		   (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
> > - 		pci_read_bridge_bases(bus);
> > -	} 
> 
> I appreciate you're basically moving this code ...
> 
> > 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;
> 
> ... to here, but shouldn't we dispense with the class check and use
> pci_is_bridge instead? pci_read_bridge_bases is making an assumption
> that the header type is BRIDGE, so that seems like the more sensible
> check to me.

Yes I thought about that before posting. I added the class check to make
sure it covers the current checks carried out on all architectures
I am patching (ie pci_is_bridge() would allow reading bridge bases
for a cardbus header and I do not think that's correct as the code
stands).

I am happy to refactor the check once we have some coverage for all
archs.

> Also, with these changes, can we now make pci_read_bridge_bases static
> (as it has an implicit rdering requirement on the device having been
> scanned)?

As far as I understand the code, SPARC still needs it in arch specific
code that scans the PCI bus, so I could not make it static with this
set, I removed the call from all pcibios_fixup_bus implementations
that were calling it but that's where I had to stop.

Thanks for having a look,
Lorenzo

Re: [RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer

[Suravee Suthikulanit]

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 <lorenzo.pieralisi@arm.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: James E.J. Bottomley <jejb@parisc-linux.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Michal Simek <monstr@monstr.eu>
> Cc: Koichi Yasutake <yasutake.koichi@jp.panasonic.com>
> Cc: Chris Zankel <chris@zankel.net>
> ---
>   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?

Thanks,
Suravee

Re: [RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer

[Lorenzo Pieralisi]

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 <lorenzo.pieralisi@arm.com>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: James E.J. Bottomley <jejb@parisc-linux.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: David Howells <dhowells@redhat.com>
> > Cc: Russell King <linux@arm.linux.org.uk>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Michal Simek <monstr@monstr.eu>
> > Cc: Koichi Yasutake <yasutake.koichi@jp.panasonic.com>
> > Cc: Chris Zankel <chris@zankel.net>
> > ---
> >   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

Re: [RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer

[Suravee Suthikulanit]

On 5/27/2015 12:54 PM, Lorenzo Pieralisi wrote:
>>> 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,

That seems reasonable. I test putting it here in pci_scan_bridge(),
and it works.

....
	child = pci_find_bus(pci_domain_nr(bus), secondary);
	if (!child) {
		child = pci_add_new_bus(bus, dev, secondary);
		if (!child)
			goto out;
		child->primary = primary;
		pci_bus_insert_busn_res(child, secondary, subordinate);
		child->bridge_ctl = bctl;
		pci_read_bridge_bases(child);
	}

	cmax = pci_scan_child_bus(child);
.....

Thanks,
Suravee