[PATCH 0/5] PCI scan interfaces clean up

[PATCH 0/5] PCI scan interfaces clean up

[Yijing Wang]

Yijing Wang (5):
  PCI: Rip out pci_bus_add_devices() from pci_scan_bus()
  PCI: Introduce pci_scan_bus_simple()
  PCI: Use pci_scan_bus_simple() in simple scene.
  PCI: Remove pci_scan_bus() and pci_scan_bus_parented()
  PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

 arch/alpha/kernel/pci.c           |    2 ++
 arch/alpha/kernel/sys_nautilus.c  |    3 ++-
 arch/frv/mb93090-mb00/pci-vdk.c   |    6 ++++--
 arch/ia64/sn/kernel/io_init.c     |    1 +
 arch/m68k/coldfire/pci.c          |    3 ++-
 arch/microblaze/pci/pci-common.c  |    1 +
 arch/mips/pci/pci.c               |    1 +
 arch/mn10300/unit-asb2305/pci.c   |    5 ++++-
 arch/s390/pci/pci.c               |    2 +-
 arch/sh/drivers/pci/pci.c         |    1 +
 arch/sparc/kernel/leon_pci.c      |    1 +
 arch/sparc/kernel/pcic.c          |    5 ++++-
 arch/tile/kernel/pci.c            |    2 ++
 arch/tile/kernel/pci_gx.c         |    2 ++
 arch/unicore32/kernel/pci.c       |   15 +++++----------
 arch/x86/pci/common.c             |    1 +
 arch/xtensa/kernel/pci.c          |    2 ++
 drivers/pci/host/pci-xgene.c      |    1 +
 drivers/pci/hotplug/ibmphp_core.c |    7 +++++--
 drivers/pci/probe.c               |   26 ++------------------------
 drivers/pci/xen-pcifront.c        |    2 +-
 include/linux/pci.h               |    3 +--
 22 files changed, 46 insertions(+), 46 deletions(-)

[PATCH 0/5] PCI scan interfaces clean up

[Yijing Wang]

Yijing Wang (5):
  PCI: Rip out pci_bus_add_devices() from pci_scan_bus()
  PCI: Introduce pci_scan_bus_simple()
  PCI: Use pci_scan_bus_simple() in simple scene.
  PCI: Remove pci_scan_bus() and pci_scan_bus_parented()
  PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

 arch/alpha/kernel/pci.c           |    2 ++
 arch/alpha/kernel/sys_nautilus.c  |    3 ++-
 arch/frv/mb93090-mb00/pci-vdk.c   |    6 ++++--
 arch/ia64/sn/kernel/io_init.c     |    1 +
 arch/m68k/coldfire/pci.c          |    3 ++-
 arch/microblaze/pci/pci-common.c  |    1 +
 arch/mips/pci/pci.c               |    1 +
 arch/mn10300/unit-asb2305/pci.c   |    5 ++++-
 arch/s390/pci/pci.c               |    2 +-
 arch/sh/drivers/pci/pci.c         |    1 +
 arch/sparc/kernel/leon_pci.c      |    1 +
 arch/sparc/kernel/pcic.c          |    5 ++++-
 arch/tile/kernel/pci.c            |    2 ++
 arch/tile/kernel/pci_gx.c         |    2 ++
 arch/unicore32/kernel/pci.c       |   15 +++++----------
 arch/x86/pci/common.c             |    1 +
 arch/xtensa/kernel/pci.c          |    2 ++
 drivers/pci/host/pci-xgene.c      |    1 +
 drivers/pci/hotplug/ibmphp_core.c |    7 +++++--
 drivers/pci/probe.c               |   26 ++------------------------
 drivers/pci/xen-pcifront.c        |    2 +-
 include/linux/pci.h               |    3 +--
 22 files changed, 46 insertions(+), 46 deletions(-)

[PATCH 0/5] PCI scan interfaces clean up

[Yijing Wang]

Yijing Wang (5):
  PCI: Rip out pci_bus_add_devices() from pci_scan_bus()
  PCI: Introduce pci_scan_bus_simple()
  PCI: Use pci_scan_bus_simple() in simple scene.
  PCI: Remove pci_scan_bus() and pci_scan_bus_parented()
  PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

 arch/alpha/kernel/pci.c           |    2 ++
 arch/alpha/kernel/sys_nautilus.c  |    3 ++-
 arch/frv/mb93090-mb00/pci-vdk.c   |    6 ++++--
 arch/ia64/sn/kernel/io_init.c     |    1 +
 arch/m68k/coldfire/pci.c          |    3 ++-
 arch/microblaze/pci/pci-common.c  |    1 +
 arch/mips/pci/pci.c               |    1 +
 arch/mn10300/unit-asb2305/pci.c   |    5 ++++-
 arch/s390/pci/pci.c               |    2 +-
 arch/sh/drivers/pci/pci.c         |    1 +
 arch/sparc/kernel/leon_pci.c      |    1 +
 arch/sparc/kernel/pcic.c          |    5 ++++-
 arch/tile/kernel/pci.c            |    2 ++
 arch/tile/kernel/pci_gx.c         |    2 ++
 arch/unicore32/kernel/pci.c       |   15 +++++----------
 arch/x86/pci/common.c             |    1 +
 arch/xtensa/kernel/pci.c          |    2 ++
 drivers/pci/host/pci-xgene.c      |    1 +
 drivers/pci/hotplug/ibmphp_core.c |    7 +++++--
 drivers/pci/probe.c               |   26 ++------------------------
 drivers/pci/xen-pcifront.c        |    2 +-
 include/linux/pci.h               |    3 +--
 22 files changed, 46 insertions(+), 46 deletions(-)

[PATCH 2/5] PCI: Introduce pci_scan_bus_simple()

[Yijing Wang]

Now pci_scan_bus() and pci_scan_bus_parented()
are almost the same except parent device.
There is no need to use two similar scan interfaces.
Introduce pci_scan_bus_simple() which is same
as pci_scan_bus_parented(). And remove pci_scan_bus()
and pci_scan_bus_parented() in later patch.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/probe.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 5c5e01e..a8d90d4 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2089,6 +2089,24 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 }
 EXPORT_SYMBOL(pci_scan_root_bus);
 
+struct pci_bus *pci_scan_bus_simple(struct device *parent,
+		int bus, struct pci_ops *ops, void *sysdata)
+{
+	LIST_HEAD(resources);
+	struct pci_bus *b;
+
+	pci_add_resource(&resources, &ioport_resource);
+	pci_add_resource(&resources, &iomem_resource);
+	pci_add_resource(&resources, &busn_resource);
+	b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
+	if (b)
+		pci_scan_child_bus(b);
+	else
+		pci_free_resource_list(&resources);
+	return b;
+}
+EXPORT_SYMBOL(pci_scan_bus_simple);
+
 /* Deprecated; use pci_scan_root_bus() instead */
 struct pci_bus *pci_scan_bus_parented(struct device *parent,
 		int bus, struct pci_ops *ops, void *sysdata)
-- 
1.7.1

[PATCH 2/5] PCI: Introduce pci_scan_bus_simple()

[Yijing Wang]

Now pci_scan_bus() and pci_scan_bus_parented()
are almost the same except parent device.
There is no need to use two similar scan interfaces.
Introduce pci_scan_bus_simple() which is same
as pci_scan_bus_parented(). And remove pci_scan_bus()
and pci_scan_bus_parented() in later patch.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/probe.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 5c5e01e..a8d90d4 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2089,6 +2089,24 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 }
 EXPORT_SYMBOL(pci_scan_root_bus);
 
+struct pci_bus *pci_scan_bus_simple(struct device *parent,
+		int bus, struct pci_ops *ops, void *sysdata)
+{
+	LIST_HEAD(resources);
+	struct pci_bus *b;
+
+	pci_add_resource(&resources, &ioport_resource);
+	pci_add_resource(&resources, &iomem_resource);
+	pci_add_resource(&resources, &busn_resource);
+	b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
+	if (b)
+		pci_scan_child_bus(b);
+	else
+		pci_free_resource_list(&resources);
+	return b;
+}
+EXPORT_SYMBOL(pci_scan_bus_simple);
+
 /* Deprecated; use pci_scan_root_bus() instead */
 struct pci_bus *pci_scan_bus_parented(struct device *parent,
 		int bus, struct pci_ops *ops, void *sysdata)
-- 
1.7.1

[PATCH 2/5] PCI: Introduce pci_scan_bus_simple()

[Yijing Wang]

Now pci_scan_bus() and pci_scan_bus_parented()
are almost the same except parent device.
There is no need to use two similar scan interfaces.
Introduce pci_scan_bus_simple() which is same
as pci_scan_bus_parented(). And remove pci_scan_bus()
and pci_scan_bus_parented() in later patch.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/probe.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 5c5e01e..a8d90d4 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2089,6 +2089,24 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 }
 EXPORT_SYMBOL(pci_scan_root_bus);
 
+struct pci_bus *pci_scan_bus_simple(struct device *parent,
+		int bus, struct pci_ops *ops, void *sysdata)
+{
+	LIST_HEAD(resources);
+	struct pci_bus *b;
+
+	pci_add_resource(&resources, &ioport_resource);
+	pci_add_resource(&resources, &iomem_resource);
+	pci_add_resource(&resources, &busn_resource);
+	b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
+	if (b)
+		pci_scan_child_bus(b);
+	else
+		pci_free_resource_list(&resources);
+	return b;
+}
+EXPORT_SYMBOL(pci_scan_bus_simple);
+
 /* Deprecated; use pci_scan_root_bus() instead */
 struct pci_bus *pci_scan_bus_parented(struct device *parent,
 		int bus, struct pci_ops *ops, void *sysdata)
-- 
1.7.1

[PATCH 4/5] PCI: Remove pci_scan_bus() and pci_scan_bus_parented()

[Yijing Wang]

Clean out pci_scan_bus() and pci_scan_bus_parented().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/probe.c |   38 --------------------------------------
 include/linux/pci.h |    3 ---
 2 files changed, 0 insertions(+), 41 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a8d90d4..79775b5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2107,44 +2107,6 @@ struct pci_bus *pci_scan_bus_simple(struct device *parent,
 }
 EXPORT_SYMBOL(pci_scan_bus_simple);
 
-/* Deprecated; use pci_scan_root_bus() instead */
-struct pci_bus *pci_scan_bus_parented(struct device *parent,
-		int bus, struct pci_ops *ops, void *sysdata)
-{
-	LIST_HEAD(resources);
-	struct pci_bus *b;
-
-	pci_add_resource(&resources, &ioport_resource);
-	pci_add_resource(&resources, &iomem_resource);
-	pci_add_resource(&resources, &busn_resource);
-	b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
-	if (b)
-		pci_scan_child_bus(b);
-	else
-		pci_free_resource_list(&resources);
-	return b;
-}
-EXPORT_SYMBOL(pci_scan_bus_parented);
-
-struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
-					void *sysdata)
-{
-	LIST_HEAD(resources);
-	struct pci_bus *b;
-
-	pci_add_resource(&resources, &ioport_resource);
-	pci_add_resource(&resources, &iomem_resource);
-	pci_add_resource(&resources, &busn_resource);
-	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
-	if (b) {
-		pci_scan_child_bus(b);
-	} else {
-		pci_free_resource_list(&resources);
-	}
-	return b;
-}
-EXPORT_SYMBOL(pci_scan_bus);
-
 /**
  * pci_rescan_bus_bridge_resize - scan a PCI bus for devices.
  * @bridge: PCI bridge for the bus to scan
diff --git a/include/linux/pci.h b/include/linux/pci.h
index befefbd..9ebc908 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -767,11 +767,8 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
 void pcibios_scan_specific_bus(int busn);
 struct pci_bus *pci_find_bus(int domain, int busnr);
 void pci_bus_add_devices(const struct pci_bus *bus);
-struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
-				      struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_scan_bus_simple(struct device *parent, int bus,
 				      struct pci_ops *ops, void *sysdata);
-struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
 				    struct list_head *resources);
-- 
1.7.1

[PATCH 4/5] PCI: Remove pci_scan_bus() and pci_scan_bus_parented()

[Yijing Wang]

Clean out pci_scan_bus() and pci_scan_bus_parented().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/probe.c |   38 --------------------------------------
 include/linux/pci.h |    3 ---
 2 files changed, 0 insertions(+), 41 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a8d90d4..79775b5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2107,44 +2107,6 @@ struct pci_bus *pci_scan_bus_simple(struct device *parent,
 }
 EXPORT_SYMBOL(pci_scan_bus_simple);
 
-/* Deprecated; use pci_scan_root_bus() instead */
-struct pci_bus *pci_scan_bus_parented(struct device *parent,
-		int bus, struct pci_ops *ops, void *sysdata)
-{
-	LIST_HEAD(resources);
-	struct pci_bus *b;
-
-	pci_add_resource(&resources, &ioport_resource);
-	pci_add_resource(&resources, &iomem_resource);
-	pci_add_resource(&resources, &busn_resource);
-	b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
-	if (b)
-		pci_scan_child_bus(b);
-	else
-		pci_free_resource_list(&resources);
-	return b;
-}
-EXPORT_SYMBOL(pci_scan_bus_parented);
-
-struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
-					void *sysdata)
-{
-	LIST_HEAD(resources);
-	struct pci_bus *b;
-
-	pci_add_resource(&resources, &ioport_resource);
-	pci_add_resource(&resources, &iomem_resource);
-	pci_add_resource(&resources, &busn_resource);
-	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
-	if (b) {
-		pci_scan_child_bus(b);
-	} else {
-		pci_free_resource_list(&resources);
-	}
-	return b;
-}
-EXPORT_SYMBOL(pci_scan_bus);
-
 /**
  * pci_rescan_bus_bridge_resize - scan a PCI bus for devices.
  * @bridge: PCI bridge for the bus to scan
diff --git a/include/linux/pci.h b/include/linux/pci.h
index befefbd..9ebc908 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -767,11 +767,8 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
 void pcibios_scan_specific_bus(int busn);
 struct pci_bus *pci_find_bus(int domain, int busnr);
 void pci_bus_add_devices(const struct pci_bus *bus);
-struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
-				      struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_scan_bus_simple(struct device *parent, int bus,
 				      struct pci_ops *ops, void *sysdata);
-struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
 				    struct list_head *resources);
-- 
1.7.1

[PATCH 4/5] PCI: Remove pci_scan_bus() and pci_scan_bus_parented()

[Yijing Wang]

Clean out pci_scan_bus() and pci_scan_bus_parented().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/probe.c |   38 --------------------------------------
 include/linux/pci.h |    3 ---
 2 files changed, 0 insertions(+), 41 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a8d90d4..79775b5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2107,44 +2107,6 @@ struct pci_bus *pci_scan_bus_simple(struct device *parent,
 }
 EXPORT_SYMBOL(pci_scan_bus_simple);
 
-/* Deprecated; use pci_scan_root_bus() instead */
-struct pci_bus *pci_scan_bus_parented(struct device *parent,
-		int bus, struct pci_ops *ops, void *sysdata)
-{
-	LIST_HEAD(resources);
-	struct pci_bus *b;
-
-	pci_add_resource(&resources, &ioport_resource);
-	pci_add_resource(&resources, &iomem_resource);
-	pci_add_resource(&resources, &busn_resource);
-	b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
-	if (b)
-		pci_scan_child_bus(b);
-	else
-		pci_free_resource_list(&resources);
-	return b;
-}
-EXPORT_SYMBOL(pci_scan_bus_parented);
-
-struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
-					void *sysdata)
-{
-	LIST_HEAD(resources);
-	struct pci_bus *b;
-
-	pci_add_resource(&resources, &ioport_resource);
-	pci_add_resource(&resources, &iomem_resource);
-	pci_add_resource(&resources, &busn_resource);
-	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
-	if (b) {
-		pci_scan_child_bus(b);
-	} else {
-		pci_free_resource_list(&resources);
-	}
-	return b;
-}
-EXPORT_SYMBOL(pci_scan_bus);
-
 /**
  * pci_rescan_bus_bridge_resize - scan a PCI bus for devices.
  * @bridge: PCI bridge for the bus to scan
diff --git a/include/linux/pci.h b/include/linux/pci.h
index befefbd..9ebc908 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -767,11 +767,8 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
 void pcibios_scan_specific_bus(int busn);
 struct pci_bus *pci_find_bus(int domain, int busnr);
 void pci_bus_add_devices(const struct pci_bus *bus);
-struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
-				      struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_scan_bus_simple(struct device *parent, int bus,
 				      struct pci_ops *ops, void *sysdata);
-struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
 				    struct list_head *resources);
-- 
1.7.1

[PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene.

[Yijing Wang]

Use pci_scan_bus_simple() to scan pci buses with
the default IO/MEM/BUS resources. Use it instead
of pci_scan_bus() and pci_scan_bus_parented().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 arch/alpha/kernel/sys_nautilus.c  |    2 +-
 arch/m68k/coldfire/pci.c          |    2 +-
 arch/sparc/kernel/pcic.c          |    3 ++-
 arch/unicore32/kernel/pci.c       |    4 ++--
 drivers/pci/hotplug/ibmphp_core.c |    3 ++-
 drivers/pci/xen-pcifront.c        |    2 +-
 include/linux/pci.h               |    2 ++
 7 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nautilus.c
index 4ae4a40..a4f190b 100644
--- a/arch/alpha/kernel/sys_nautilus.c
+++ b/arch/alpha/kernel/sys_nautilus.c
@@ -206,7 +206,7 @@ nautilus_init_pci(void)
 	unsigned long memtop = max_low_pfn << PAGE_SHIFT;
 
 	/* Scan our single hose.  */
-	bus = pci_scan_bus(0, alpha_mv.pci_ops, hose);
+	bus = pci_scan_bus_simple(NULL, 0, alpha_mv.pci_ops, hose);
 	hose->bus = bus;
 	pcibios_claim_one_bus(bus);
 
diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
index d45f087..9f423d5 100644
--- a/arch/m68k/coldfire/pci.c
+++ b/arch/m68k/coldfire/pci.c
@@ -312,7 +312,7 @@ static int __init mcf_pci_init(void)
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	schedule_timeout(msecs_to_jiffies(200));
 
-	rootbus = pci_scan_bus(0, &mcf_pci_ops, NULL);
+	rootbus = pci_scan_bus_simple(NULL, 0, &mcf_pci_ops, NULL);
 	rootbus->resource[0] = &mcf_pci_io;
 	rootbus->resource[1] = &mcf_pci_mem;
 
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 7a82fe2..9665e03 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -390,7 +390,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pcic *pcic)
 {
 	struct linux_pbm_info *pbm = &pcic->pbm;
 
-	pbm->pci_bus = pci_scan_bus(pbm->pci_first_busno, &pcic_ops, pbm);
+	pbm->pci_bus = pci_scan_bus_simple(NULL,
+			pbm->pci_first_busno, &pcic_ops, pbm);
 	if (pbm->pci_bus)
 		pci_bus_add_devices(pbm->pci_bus);
 #if 0 /* deadwood transplanted from sparc64 */
diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index 3d82024..f290a3d 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -258,8 +258,8 @@ static int __init pci_common_init(void)
 
 	pci_puv3_preinit();
 
-	puv3_bus = pci_scan_bus(0, &pci_puv3_ops, NULL);
-
+	puv3_bus = pci_scan_bus_simple(NULL, 0,
+			&pci_puv3_ops, NULL);
 	if (!puv3_bus)
 		panic("PCI: unable to scan bus!");
 
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index db0f2e8..9190cb1 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -767,7 +767,8 @@ static u8 bus_structure_fixup(u8 busno)
 					(l != 0x0000) && (l != 0xffff)) {
 			debug("%s - Inside bus_structure_fixup()\n",
 							__func__);
-			b = pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			b = pci_scan_bus_simple(NULL, busno,
+					ibmphp_pci_bus->ops, NULL);
 			if (b)
 				pci_bus_add_devices(b);
 			break;
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index b1ffebe..364aaf4 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -474,7 +474,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
 
 	pci_lock_rescan_remove();
 
-	b = pci_scan_bus_parented(&pdev->xdev->dev, bus,
+	b = pci_scan_bus_simple(&pdev->xdev->dev, bus,
 				  &pcifront_bus_ops, sd);
 	if (!b) {
 		dev_err(&pdev->xdev->dev,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 90a5fde..befefbd 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -769,6 +769,8 @@ struct pci_bus *pci_find_bus(int domain, int busnr);
 void pci_bus_add_devices(const struct pci_bus *bus);
 struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
 				      struct pci_ops *ops, void *sysdata);
+struct pci_bus *pci_scan_bus_simple(struct device *parent, int bus,
+				      struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
-- 
1.7.1

[PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene.

[Yijing Wang]

Use pci_scan_bus_simple() to scan pci buses with
the default IO/MEM/BUS resources. Use it instead
of pci_scan_bus() and pci_scan_bus_parented().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 arch/alpha/kernel/sys_nautilus.c  |    2 +-
 arch/m68k/coldfire/pci.c          |    2 +-
 arch/sparc/kernel/pcic.c          |    3 ++-
 arch/unicore32/kernel/pci.c       |    4 ++--
 drivers/pci/hotplug/ibmphp_core.c |    3 ++-
 drivers/pci/xen-pcifront.c        |    2 +-
 include/linux/pci.h               |    2 ++
 7 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nautilus.c
index 4ae4a40..a4f190b 100644
--- a/arch/alpha/kernel/sys_nautilus.c
+++ b/arch/alpha/kernel/sys_nautilus.c
@@ -206,7 +206,7 @@ nautilus_init_pci(void)
 	unsigned long memtop = max_low_pfn << PAGE_SHIFT;
 
 	/* Scan our single hose.  */
-	bus = pci_scan_bus(0, alpha_mv.pci_ops, hose);
+	bus = pci_scan_bus_simple(NULL, 0, alpha_mv.pci_ops, hose);
 	hose->bus = bus;
 	pcibios_claim_one_bus(bus);
 
diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
index d45f087..9f423d5 100644
--- a/arch/m68k/coldfire/pci.c
+++ b/arch/m68k/coldfire/pci.c
@@ -312,7 +312,7 @@ static int __init mcf_pci_init(void)
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	schedule_timeout(msecs_to_jiffies(200));
 
-	rootbus = pci_scan_bus(0, &mcf_pci_ops, NULL);
+	rootbus = pci_scan_bus_simple(NULL, 0, &mcf_pci_ops, NULL);
 	rootbus->resource[0] = &mcf_pci_io;
 	rootbus->resource[1] = &mcf_pci_mem;
 
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 7a82fe2..9665e03 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -390,7 +390,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pcic *pcic)
 {
 	struct linux_pbm_info *pbm = &pcic->pbm;
 
-	pbm->pci_bus = pci_scan_bus(pbm->pci_first_busno, &pcic_ops, pbm);
+	pbm->pci_bus = pci_scan_bus_simple(NULL,
+			pbm->pci_first_busno, &pcic_ops, pbm);
 	if (pbm->pci_bus)
 		pci_bus_add_devices(pbm->pci_bus);
 #if 0 /* deadwood transplanted from sparc64 */
diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index 3d82024..f290a3d 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -258,8 +258,8 @@ static int __init pci_common_init(void)
 
 	pci_puv3_preinit();
 
-	puv3_bus = pci_scan_bus(0, &pci_puv3_ops, NULL);
-
+	puv3_bus = pci_scan_bus_simple(NULL, 0,
+			&pci_puv3_ops, NULL);
 	if (!puv3_bus)
 		panic("PCI: unable to scan bus!");
 
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index db0f2e8..9190cb1 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -767,7 +767,8 @@ static u8 bus_structure_fixup(u8 busno)
 					(l != 0x0000) && (l != 0xffff)) {
 			debug("%s - Inside bus_structure_fixup()\n",
 							__func__);
-			b = pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			b = pci_scan_bus_simple(NULL, busno,
+					ibmphp_pci_bus->ops, NULL);
 			if (b)
 				pci_bus_add_devices(b);
 			break;
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index b1ffebe..364aaf4 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -474,7 +474,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
 
 	pci_lock_rescan_remove();
 
-	b = pci_scan_bus_parented(&pdev->xdev->dev, bus,
+	b = pci_scan_bus_simple(&pdev->xdev->dev, bus,
 				  &pcifront_bus_ops, sd);
 	if (!b) {
 		dev_err(&pdev->xdev->dev,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 90a5fde..befefbd 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -769,6 +769,8 @@ struct pci_bus *pci_find_bus(int domain, int busnr);
 void pci_bus_add_devices(const struct pci_bus *bus);
 struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
 				      struct pci_ops *ops, void *sysdata);
+struct pci_bus *pci_scan_bus_simple(struct device *parent, int bus,
+				      struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
-- 
1.7.1

[PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene.

[Yijing Wang]

Use pci_scan_bus_simple() to scan pci buses with
the default IO/MEM/BUS resources. Use it instead
of pci_scan_bus() and pci_scan_bus_parented().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 arch/alpha/kernel/sys_nautilus.c  |    2 +-
 arch/m68k/coldfire/pci.c          |    2 +-
 arch/sparc/kernel/pcic.c          |    3 ++-
 arch/unicore32/kernel/pci.c       |    4 ++--
 drivers/pci/hotplug/ibmphp_core.c |    3 ++-
 drivers/pci/xen-pcifront.c        |    2 +-
 include/linux/pci.h               |    2 ++
 7 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nautilus.c
index 4ae4a40..a4f190b 100644
--- a/arch/alpha/kernel/sys_nautilus.c
+++ b/arch/alpha/kernel/sys_nautilus.c
@@ -206,7 +206,7 @@ nautilus_init_pci(void)
 	unsigned long memtop = max_low_pfn << PAGE_SHIFT;
 
 	/* Scan our single hose.  */
-	bus = pci_scan_bus(0, alpha_mv.pci_ops, hose);
+	bus = pci_scan_bus_simple(NULL, 0, alpha_mv.pci_ops, hose);
 	hose->bus = bus;
 	pcibios_claim_one_bus(bus);
 
diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
index d45f087..9f423d5 100644
--- a/arch/m68k/coldfire/pci.c
+++ b/arch/m68k/coldfire/pci.c
@@ -312,7 +312,7 @@ static int __init mcf_pci_init(void)
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	schedule_timeout(msecs_to_jiffies(200));
 
-	rootbus = pci_scan_bus(0, &mcf_pci_ops, NULL);
+	rootbus = pci_scan_bus_simple(NULL, 0, &mcf_pci_ops, NULL);
 	rootbus->resource[0] = &mcf_pci_io;
 	rootbus->resource[1] = &mcf_pci_mem;
 
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 7a82fe2..9665e03 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -390,7 +390,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pcic *pcic)
 {
 	struct linux_pbm_info *pbm = &pcic->pbm;
 
-	pbm->pci_bus = pci_scan_bus(pbm->pci_first_busno, &pcic_ops, pbm);
+	pbm->pci_bus = pci_scan_bus_simple(NULL,
+			pbm->pci_first_busno, &pcic_ops, pbm);
 	if (pbm->pci_bus)
 		pci_bus_add_devices(pbm->pci_bus);
 #if 0 /* deadwood transplanted from sparc64 */
diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index 3d82024..f290a3d 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -258,8 +258,8 @@ static int __init pci_common_init(void)
 
 	pci_puv3_preinit();
 
-	puv3_bus = pci_scan_bus(0, &pci_puv3_ops, NULL);
-
+	puv3_bus = pci_scan_bus_simple(NULL, 0,
+			&pci_puv3_ops, NULL);
 	if (!puv3_bus)
 		panic("PCI: unable to scan bus!");
 
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index db0f2e8..9190cb1 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -767,7 +767,8 @@ static u8 bus_structure_fixup(u8 busno)
 					(l != 0x0000) && (l != 0xffff)) {
 			debug("%s - Inside bus_structure_fixup()\n",
 							__func__);
-			b = pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			b = pci_scan_bus_simple(NULL, busno,
+					ibmphp_pci_bus->ops, NULL);
 			if (b)
 				pci_bus_add_devices(b);
 			break;
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index b1ffebe..364aaf4 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -474,7 +474,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
 
 	pci_lock_rescan_remove();
 
-	b = pci_scan_bus_parented(&pdev->xdev->dev, bus,
+	b = pci_scan_bus_simple(&pdev->xdev->dev, bus,
 				  &pcifront_bus_ops, sd);
 	if (!b) {
 		dev_err(&pdev->xdev->dev,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 90a5fde..befefbd 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -769,6 +769,8 @@ struct pci_bus *pci_find_bus(int domain, int busnr);
 void pci_bus_add_devices(const struct pci_bus *bus);
 struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
 				      struct pci_ops *ops, void *sysdata);
+struct pci_bus *pci_scan_bus_simple(struct device *parent, int bus,
+				      struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
-- 
1.7.1

Re: [PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene.

[Arnd Bergmann]

On Wednesday 19 November 2014 15:32:47 Yijing Wang wrote:
> Use pci_scan_bus_simple() to scan pci buses with
> the default IO/MEM/BUS resources. Use it instead
> of pci_scan_bus() and pci_scan_bus_parented().
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> 

I think we should replace pci_scan_bus_parented() with something better
by making the xen code use pci_scan_root_bus() (see below for a draft
patch). After that we no longer need the 'parent' argument here and we
can just keep using the pci_scan_bus() function for the legacy users,
or rename it if that helps.

	Arnd


8<---

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 116ca3746adb..45a4843a608a 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -448,6 +448,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
 	struct pci_bus *b;
 	struct pcifront_sd *sd = NULL;
 	struct pci_bus_entry *bus_entry = NULL;
+	LIST_HEAD(resources);
 	int err = 0;
 
 #ifndef CONFIG_PCI_DOMAINS
@@ -470,17 +471,20 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
 		err = -ENOMEM;
 		goto err_out;
 	}
+	pci_add_resource(&resources, &ioport_resource);
+	pci_add_resource(&resources, &iomem_resource);
 	pcifront_init_sd(sd, domain, bus, pdev);
 
 	pci_lock_rescan_remove();
 
-	b = pci_scan_bus_parented(&pdev->xdev->dev, bus,
-				  &pcifront_bus_ops, sd);
+	b = pci_scan_root_bus(&pdev->xdev->dev, bus, &pcifront_bus_ops, sd,
+			      &resources);
 	if (!b) {
 		dev_err(&pdev->xdev->dev,
 			"Error creating PCI Frontend Bus!\n");
 		err = -ENOMEM;
 		pci_unlock_rescan_remove();
+		pci_free_resource_list(&resources);
 		goto err_out;
 	}
 

Re: [PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene.

[Arnd Bergmann]

On Wednesday 19 November 2014 15:32:47 Yijing Wang wrote:
> Use pci_scan_bus_simple() to scan pci buses with
> the default IO/MEM/BUS resources. Use it instead
> of pci_scan_bus() and pci_scan_bus_parented().
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> 

I think we should replace pci_scan_bus_parented() with something better
by making the xen code use pci_scan_root_bus() (see below for a draft
patch). After that we no longer need the 'parent' argument here and we
can just keep using the pci_scan_bus() function for the legacy users,
or rename it if that helps.

	Arnd


8<---

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 116ca3746adb..45a4843a608a 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -448,6 +448,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
 	struct pci_bus *b;
 	struct pcifront_sd *sd = NULL;
 	struct pci_bus_entry *bus_entry = NULL;
+	LIST_HEAD(resources);
 	int err = 0;
 
 #ifndef CONFIG_PCI_DOMAINS
@@ -470,17 +471,20 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
 		err = -ENOMEM;
 		goto err_out;
 	}
+	pci_add_resource(&resources, &ioport_resource);
+	pci_add_resource(&resources, &iomem_resource);
 	pcifront_init_sd(sd, domain, bus, pdev);
 
 	pci_lock_rescan_remove();
 
-	b = pci_scan_bus_parented(&pdev->xdev->dev, bus,
-				  &pcifront_bus_ops, sd);
+	b = pci_scan_root_bus(&pdev->xdev->dev, bus, &pcifront_bus_ops, sd,
+			      &resources);
 	if (!b) {
 		dev_err(&pdev->xdev->dev,
 			"Error creating PCI Frontend Bus!\n");
 		err = -ENOMEM;
 		pci_unlock_rescan_remove();
+		pci_free_resource_list(&resources);
 		goto err_out;
 	}
 

[PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene.

[Arnd Bergmann]

On Wednesday 19 November 2014 15:32:47 Yijing Wang wrote:
> Use pci_scan_bus_simple() to scan pci buses with
> the default IO/MEM/BUS resources. Use it instead
> of pci_scan_bus() and pci_scan_bus_parented().
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> 

I think we should replace pci_scan_bus_parented() with something better
by making the xen code use pci_scan_root_bus() (see below for a draft
patch). After that we no longer need the 'parent' argument here and we
can just keep using the pci_scan_bus() function for the legacy users,
or rename it if that helps.

	Arnd


8<---

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 116ca3746adb..45a4843a608a 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -448,6 +448,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
 	struct pci_bus *b;
 	struct pcifront_sd *sd = NULL;
 	struct pci_bus_entry *bus_entry = NULL;
+	LIST_HEAD(resources);
 	int err = 0;
 
 #ifndef CONFIG_PCI_DOMAINS
@@ -470,17 +471,20 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
 		err = -ENOMEM;
 		goto err_out;
 	}
+	pci_add_resource(&resources, &ioport_resource);
+	pci_add_resource(&resources, &iomem_resource);
 	pcifront_init_sd(sd, domain, bus, pdev);
 
 	pci_lock_rescan_remove();
 
-	b = pci_scan_bus_parented(&pdev->xdev->dev, bus,
-				  &pcifront_bus_ops, sd);
+	b = pci_scan_root_bus(&pdev->xdev->dev, bus, &pcifront_bus_ops, sd,
+			      &resources);
 	if (!b) {
 		dev_err(&pdev->xdev->dev,
 			"Error creating PCI Frontend Bus!\n");
 		err = -ENOMEM;
 		pci_unlock_rescan_remove();
+		pci_free_resource_list(&resources);
 		goto err_out;
 	}
 

Re: [PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene.

[Yijing Wang]

On 2014/11/19 20:28, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 15:32:47 Yijing Wang wrote:
>> Use pci_scan_bus_simple() to scan pci buses with
>> the default IO/MEM/BUS resources. Use it instead
>> of pci_scan_bus() and pci_scan_bus_parented().
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>>
> 
> I think we should replace pci_scan_bus_parented() with something better
> by making the xen code use pci_scan_root_bus() (see below for a draft
> patch). After that we no longer need the 'parent' argument here and we
> can just keep using the pci_scan_bus() function for the legacy users,
> or rename it if that helps.

It make sense to me, and we could rename pci_scan_bus() to pci_scan_bus_legacy().
Thanks, I will add this to my series,


> 
> 	Arnd
> 
> 
> 8<---
> 
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 116ca3746adb..45a4843a608a 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -448,6 +448,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
>  	struct pci_bus *b;
>  	struct pcifront_sd *sd = NULL;
>  	struct pci_bus_entry *bus_entry = NULL;
> +	LIST_HEAD(resources);
>  	int err = 0;
>  
>  #ifndef CONFIG_PCI_DOMAINS
> @@ -470,17 +471,20 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
>  		err = -ENOMEM;
>  		goto err_out;
>  	}
> +	pci_add_resource(&resources, &ioport_resource);
> +	pci_add_resource(&resources, &iomem_resource);
>  	pcifront_init_sd(sd, domain, bus, pdev);
>  
>  	pci_lock_rescan_remove();
>  
> -	b = pci_scan_bus_parented(&pdev->xdev->dev, bus,
> -				  &pcifront_bus_ops, sd);
> +	b = pci_scan_root_bus(&pdev->xdev->dev, bus, &pcifront_bus_ops, sd,
> +			      &resources);
>  	if (!b) {
>  		dev_err(&pdev->xdev->dev,
>  			"Error creating PCI Frontend Bus!\n");
>  		err = -ENOMEM;
>  		pci_unlock_rescan_remove();
> +		pci_free_resource_list(&resources);
>  		goto err_out;
>  	}
>  
> 
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene.

[Yijing Wang]

On 2014/11/19 20:28, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 15:32:47 Yijing Wang wrote:
>> Use pci_scan_bus_simple() to scan pci buses with
>> the default IO/MEM/BUS resources. Use it instead
>> of pci_scan_bus() and pci_scan_bus_parented().
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>>
> 
> I think we should replace pci_scan_bus_parented() with something better
> by making the xen code use pci_scan_root_bus() (see below for a draft
> patch). After that we no longer need the 'parent' argument here and we
> can just keep using the pci_scan_bus() function for the legacy users,
> or rename it if that helps.

It make sense to me, and we could rename pci_scan_bus() to pci_scan_bus_legacy().
Thanks, I will add this to my series,


> 
> 	Arnd
> 
> 
> 8<---
> 
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 116ca3746adb..45a4843a608a 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -448,6 +448,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
>  	struct pci_bus *b;
>  	struct pcifront_sd *sd = NULL;
>  	struct pci_bus_entry *bus_entry = NULL;
> +	LIST_HEAD(resources);
>  	int err = 0;
>  
>  #ifndef CONFIG_PCI_DOMAINS
> @@ -470,17 +471,20 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
>  		err = -ENOMEM;
>  		goto err_out;
>  	}
> +	pci_add_resource(&resources, &ioport_resource);
> +	pci_add_resource(&resources, &iomem_resource);
>  	pcifront_init_sd(sd, domain, bus, pdev);
>  
>  	pci_lock_rescan_remove();
>  
> -	b = pci_scan_bus_parented(&pdev->xdev->dev, bus,
> -				  &pcifront_bus_ops, sd);
> +	b = pci_scan_root_bus(&pdev->xdev->dev, bus, &pcifront_bus_ops, sd,
> +			      &resources);
>  	if (!b) {
>  		dev_err(&pdev->xdev->dev,
>  			"Error creating PCI Frontend Bus!\n");
>  		err = -ENOMEM;
>  		pci_unlock_rescan_remove();
> +		pci_free_resource_list(&resources);
>  		goto err_out;
>  	}
>  
> 
> 
> .
> 


-- 
Thanks!
Yijing

[PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene.

[Yijing Wang]

On 2014/11/19 20:28, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 15:32:47 Yijing Wang wrote:
>> Use pci_scan_bus_simple() to scan pci buses with
>> the default IO/MEM/BUS resources. Use it instead
>> of pci_scan_bus() and pci_scan_bus_parented().
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>>
> 
> I think we should replace pci_scan_bus_parented() with something better
> by making the xen code use pci_scan_root_bus() (see below for a draft
> patch). After that we no longer need the 'parent' argument here and we
> can just keep using the pci_scan_bus() function for the legacy users,
> or rename it if that helps.

It make sense to me, and we could rename pci_scan_bus() to pci_scan_bus_legacy().
Thanks, I will add this to my series,


> 
> 	Arnd
> 
> 
> 8<---
> 
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 116ca3746adb..45a4843a608a 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -448,6 +448,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
>  	struct pci_bus *b;
>  	struct pcifront_sd *sd = NULL;
>  	struct pci_bus_entry *bus_entry = NULL;
> +	LIST_HEAD(resources);
>  	int err = 0;
>  
>  #ifndef CONFIG_PCI_DOMAINS
> @@ -470,17 +471,20 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
>  		err = -ENOMEM;
>  		goto err_out;
>  	}
> +	pci_add_resource(&resources, &ioport_resource);
> +	pci_add_resource(&resources, &iomem_resource);
>  	pcifront_init_sd(sd, domain, bus, pdev);
>  
>  	pci_lock_rescan_remove();
>  
> -	b = pci_scan_bus_parented(&pdev->xdev->dev, bus,
> -				  &pcifront_bus_ops, sd);
> +	b = pci_scan_root_bus(&pdev->xdev->dev, bus, &pcifront_bus_ops, sd,
> +			      &resources);
>  	if (!b) {
>  		dev_err(&pdev->xdev->dev,
>  			"Error creating PCI Frontend Bus!\n");
>  		err = -ENOMEM;
>  		pci_unlock_rescan_remove();
> +		pci_free_resource_list(&resources);
>  		goto err_out;
>  	}
>  
> 
> 
> .
> 


-- 
Thanks!
Yijing

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

Just like pci_scan_bus(), we also should rip out
pci_bus_add_devices() from pci_scan_root_bus().
Lots platforms first call pci_scan_root_bus(), but
after that, they call pci_bus_size_bridges() and
pci_bus_assign_resources(). Place pci_bus_add_devices()
in pci_scan_root_bus() hurts PCI scan logic.
Should no functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: linux-alpha@vger.kernel.org
CC: linux-ia64@vger.kernel.org
CC: linux-mips@linux-mips.org
CC: linux-xtensa@linux-xtensa.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-sh@vger.kernel.org
CC: sparclinux@vger.kernel.org
---
 arch/alpha/kernel/pci.c          |    2 ++
 arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
 arch/ia64/sn/kernel/io_init.c    |    1 +
 arch/microblaze/pci/pci-common.c |    1 +
 arch/mips/pci/pci.c              |    1 +
 arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
 arch/s390/pci/pci.c              |    2 +-
 arch/sh/drivers/pci/pci.c        |    1 +
 arch/sparc/kernel/leon_pci.c     |    1 +
 arch/tile/kernel/pci.c           |    2 ++
 arch/tile/kernel/pci_gx.c        |    2 ++
 arch/x86/pci/common.c            |    1 +
 arch/xtensa/kernel/pci.c         |    2 ++
 drivers/pci/host/pci-xgene.c     |    1 +
 drivers/pci/probe.c              |    1 -
 15 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 076c35c..97f9730 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -334,6 +334,8 @@ common_init_pci(void)
 
 		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
 					hose, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		hose->bus = bus;
 		hose->need_domain_info = need_domain_info;
 		next_busno = bus->busn_res.end + 1;
diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index efa5d65..2b36044 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 
 int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	struct pci_ops *dir = NULL;
 	LIST_HEAD(resources);
 
@@ -383,12 +384,13 @@ int __init pcibios_init(void)
 	printk("PCI: Probing PCI hardware\n");
 	pci_add_resource(&resources, &pci_ioport_resource);
 	pci_add_resource(&resources, &pci_iomem_resource);
-	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
-
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
index 0b5ce82..63b43a6 100644
--- a/arch/ia64/sn/kernel/io_init.c
+++ b/arch/ia64/sn/kernel/io_init.c
@@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
 		kfree(res);
 		kfree(controller);
 	}
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 9037914..651e25d 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
 	hose->bus = bus;
 
 	hose->last_busno = bus->busn_res.end;
+	pci_bus_add_devices(bus);
 }
 
 static int __init pcibios_init(void)
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 1bf60b1..f083688 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
 		if (!pci_has_flag(PCI_PROBE_ONLY)) {
 			pci_bus_size_bridges(bus);
 			pci_bus_assign_resources(bus);
+			pci_bus_add_devices(bus);
 		}
 	}
 }
diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
index 6b4339f..860aa35 100644
--- a/arch/mn10300/unit-asb2305/pci.c
+++ b/arch/mn10300/unit-asb2305/pci.c
@@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
  */
 static int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	resource_size_t io_offset, mem_offset;
 	LIST_HEAD(resources);
 
@@ -376,11 +377,13 @@ static int __init pcibios_init(void)
 
 	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
 	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
-	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 552b990..98247f8 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
 		zpci_cleanup_bus_resources(zdev);
 		return -EIO;
 	}
-
+	pci_bus_add_devices(zdev->bus);
 	zdev->bus->max_bus_speed = zdev->max_bus_speed;
 	return 0;
 }
diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index 1bc09ee..efc1051 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
 
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
+		pci_bus_add_devices(bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 899b720..2971076 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
 
 		/* Assign devices with resources */
 		pci_assign_unassigned_resources();
+		pci_bus_add_devices(root_bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 1f80a88..007466e 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -308,6 +308,8 @@ int __init pcibios_init(void)
 			pci_add_resource(&resources, &iomem_resource);
 			bus = pci_scan_root_bus(NULL, 0, controller->ops,
 						controller, &resources);
+			if (bus)
+				pci_bus_add_devices(bus);
 			controller->root_bus = bus;
 			controller->last_busno = bus->busn_res.end;
 		}
diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
index e39f9c5..3f8fb2c 100644
--- a/arch/tile/kernel/pci_gx.c
+++ b/arch/tile/kernel/pci_gx.c
@@ -889,6 +889,8 @@ int __init pcibios_init(void)
 		controller->first_busno = next_busno;
 		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
 					controller, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		controller->root_bus = bus;
 		next_busno = bus->busn_res.end + 1;
 	}
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 7b20bcc..300d39e 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
 		pci_free_resource_list(&resources);
 		kfree(sd);
 	}
+	pci_bus_add_devices(bus);
 }
 
 void __init pcibios_set_cache_line_size(void)
diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
index 5b34033..f2ae64e 100644
--- a/arch/xtensa/kernel/pci.c
+++ b/arch/xtensa/kernel/pci.c
@@ -185,6 +185,8 @@ static int __init pcibios_init(void)
 		pci_controller_apertures(pci_ctrl, &resources);
 		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
 					pci_ctrl->ops, pci_ctrl, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		pci_ctrl->bus = bus;
 		pci_ctrl->last_busno = bus->busn_res.end;
 		if (next_busno <= pci_ctrl->last_busno)
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 9ecabfa..c17afe5 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 	if (!bus)
 		return -ENOMEM;
 
+	pci_bus_add_devices(bus);
 	platform_set_drvdata(pdev, port);
 	return 0;
 }
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 79775b5..2ab5a40 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 	if (!found)
 		pci_bus_update_busn_res_end(b, max);
 
-	pci_bus_add_devices(b);
 	return b;
 }
 EXPORT_SYMBOL(pci_scan_root_bus);
-- 
1.7.1

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

Just like pci_scan_bus(), we also should rip out
pci_bus_add_devices() from pci_scan_root_bus().
Lots platforms first call pci_scan_root_bus(), but
after that, they call pci_bus_size_bridges() and
pci_bus_assign_resources(). Place pci_bus_add_devices()
in pci_scan_root_bus() hurts PCI scan logic.
Should no functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: linux-alpha at vger.kernel.org
CC: linux-ia64 at vger.kernel.org
CC: linux-mips at linux-mips.org
CC: linux-xtensa at linux-xtensa.org
CC: linux-arm-kernel at lists.infradead.org
CC: linux-sh at vger.kernel.org
CC: sparclinux at vger.kernel.org
---
 arch/alpha/kernel/pci.c          |    2 ++
 arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
 arch/ia64/sn/kernel/io_init.c    |    1 +
 arch/microblaze/pci/pci-common.c |    1 +
 arch/mips/pci/pci.c              |    1 +
 arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
 arch/s390/pci/pci.c              |    2 +-
 arch/sh/drivers/pci/pci.c        |    1 +
 arch/sparc/kernel/leon_pci.c     |    1 +
 arch/tile/kernel/pci.c           |    2 ++
 arch/tile/kernel/pci_gx.c        |    2 ++
 arch/x86/pci/common.c            |    1 +
 arch/xtensa/kernel/pci.c         |    2 ++
 drivers/pci/host/pci-xgene.c     |    1 +
 drivers/pci/probe.c              |    1 -
 15 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 076c35c..97f9730 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -334,6 +334,8 @@ common_init_pci(void)
 
 		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
 					hose, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		hose->bus = bus;
 		hose->need_domain_info = need_domain_info;
 		next_busno = bus->busn_res.end + 1;
diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index efa5d65..2b36044 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 
 int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	struct pci_ops *dir = NULL;
 	LIST_HEAD(resources);
 
@@ -383,12 +384,13 @@ int __init pcibios_init(void)
 	printk("PCI: Probing PCI hardware\n");
 	pci_add_resource(&resources, &pci_ioport_resource);
 	pci_add_resource(&resources, &pci_iomem_resource);
-	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
-
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
index 0b5ce82..63b43a6 100644
--- a/arch/ia64/sn/kernel/io_init.c
+++ b/arch/ia64/sn/kernel/io_init.c
@@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
 		kfree(res);
 		kfree(controller);
 	}
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 9037914..651e25d 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
 	hose->bus = bus;
 
 	hose->last_busno = bus->busn_res.end;
+	pci_bus_add_devices(bus);
 }
 
 static int __init pcibios_init(void)
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 1bf60b1..f083688 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
 		if (!pci_has_flag(PCI_PROBE_ONLY)) {
 			pci_bus_size_bridges(bus);
 			pci_bus_assign_resources(bus);
+			pci_bus_add_devices(bus);
 		}
 	}
 }
diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
index 6b4339f..860aa35 100644
--- a/arch/mn10300/unit-asb2305/pci.c
+++ b/arch/mn10300/unit-asb2305/pci.c
@@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
  */
 static int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	resource_size_t io_offset, mem_offset;
 	LIST_HEAD(resources);
 
@@ -376,11 +377,13 @@ static int __init pcibios_init(void)
 
 	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
 	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
-	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 552b990..98247f8 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
 		zpci_cleanup_bus_resources(zdev);
 		return -EIO;
 	}
-
+	pci_bus_add_devices(zdev->bus);
 	zdev->bus->max_bus_speed = zdev->max_bus_speed;
 	return 0;
 }
diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index 1bc09ee..efc1051 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
 
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
+		pci_bus_add_devices(bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 899b720..2971076 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
 
 		/* Assign devices with resources */
 		pci_assign_unassigned_resources();
+		pci_bus_add_devices(root_bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 1f80a88..007466e 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -308,6 +308,8 @@ int __init pcibios_init(void)
 			pci_add_resource(&resources, &iomem_resource);
 			bus = pci_scan_root_bus(NULL, 0, controller->ops,
 						controller, &resources);
+			if (bus)
+				pci_bus_add_devices(bus);
 			controller->root_bus = bus;
 			controller->last_busno = bus->busn_res.end;
 		}
diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
index e39f9c5..3f8fb2c 100644
--- a/arch/tile/kernel/pci_gx.c
+++ b/arch/tile/kernel/pci_gx.c
@@ -889,6 +889,8 @@ int __init pcibios_init(void)
 		controller->first_busno = next_busno;
 		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
 					controller, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		controller->root_bus = bus;
 		next_busno = bus->busn_res.end + 1;
 	}
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 7b20bcc..300d39e 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
 		pci_free_resource_list(&resources);
 		kfree(sd);
 	}
+	pci_bus_add_devices(bus);
 }
 
 void __init pcibios_set_cache_line_size(void)
diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
index 5b34033..f2ae64e 100644
--- a/arch/xtensa/kernel/pci.c
+++ b/arch/xtensa/kernel/pci.c
@@ -185,6 +185,8 @@ static int __init pcibios_init(void)
 		pci_controller_apertures(pci_ctrl, &resources);
 		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
 					pci_ctrl->ops, pci_ctrl, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		pci_ctrl->bus = bus;
 		pci_ctrl->last_busno = bus->busn_res.end;
 		if (next_busno <= pci_ctrl->last_busno)
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 9ecabfa..c17afe5 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 	if (!bus)
 		return -ENOMEM;
 
+	pci_bus_add_devices(bus);
 	platform_set_drvdata(pdev, port);
 	return 0;
 }
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 79775b5..2ab5a40 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 	if (!found)
 		pci_bus_update_busn_res_end(b, max);
 
-	pci_bus_add_devices(b);
 	return b;
 }
 EXPORT_SYMBOL(pci_scan_root_bus);
-- 
1.7.1

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

Just like pci_scan_bus(), we also should rip out
pci_bus_add_devices() from pci_scan_root_bus().
Lots platforms first call pci_scan_root_bus(), but
after that, they call pci_bus_size_bridges() and
pci_bus_assign_resources(). Place pci_bus_add_devices()
in pci_scan_root_bus() hurts PCI scan logic.
Should no functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: linux-alpha@vger.kernel.org
CC: linux-ia64@vger.kernel.org
CC: linux-mips@linux-mips.org
CC: linux-xtensa@linux-xtensa.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-sh@vger.kernel.org
CC: sparclinux@vger.kernel.org
---
 arch/alpha/kernel/pci.c          |    2 ++
 arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
 arch/ia64/sn/kernel/io_init.c    |    1 +
 arch/microblaze/pci/pci-common.c |    1 +
 arch/mips/pci/pci.c              |    1 +
 arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
 arch/s390/pci/pci.c              |    2 +-
 arch/sh/drivers/pci/pci.c        |    1 +
 arch/sparc/kernel/leon_pci.c     |    1 +
 arch/tile/kernel/pci.c           |    2 ++
 arch/tile/kernel/pci_gx.c        |    2 ++
 arch/x86/pci/common.c            |    1 +
 arch/xtensa/kernel/pci.c         |    2 ++
 drivers/pci/host/pci-xgene.c     |    1 +
 drivers/pci/probe.c              |    1 -
 15 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 076c35c..97f9730 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -334,6 +334,8 @@ common_init_pci(void)
 
 		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
 					hose, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		hose->bus = bus;
 		hose->need_domain_info = need_domain_info;
 		next_busno = bus->busn_res.end + 1;
diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index efa5d65..2b36044 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 
 int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	struct pci_ops *dir = NULL;
 	LIST_HEAD(resources);
 
@@ -383,12 +384,13 @@ int __init pcibios_init(void)
 	printk("PCI: Probing PCI hardware\n");
 	pci_add_resource(&resources, &pci_ioport_resource);
 	pci_add_resource(&resources, &pci_iomem_resource);
-	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
-
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
index 0b5ce82..63b43a6 100644
--- a/arch/ia64/sn/kernel/io_init.c
+++ b/arch/ia64/sn/kernel/io_init.c
@@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
 		kfree(res);
 		kfree(controller);
 	}
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 9037914..651e25d 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
 	hose->bus = bus;
 
 	hose->last_busno = bus->busn_res.end;
+	pci_bus_add_devices(bus);
 }
 
 static int __init pcibios_init(void)
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 1bf60b1..f083688 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
 		if (!pci_has_flag(PCI_PROBE_ONLY)) {
 			pci_bus_size_bridges(bus);
 			pci_bus_assign_resources(bus);
+			pci_bus_add_devices(bus);
 		}
 	}
 }
diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
index 6b4339f..860aa35 100644
--- a/arch/mn10300/unit-asb2305/pci.c
+++ b/arch/mn10300/unit-asb2305/pci.c
@@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
  */
 static int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	resource_size_t io_offset, mem_offset;
 	LIST_HEAD(resources);
 
@@ -376,11 +377,13 @@ static int __init pcibios_init(void)
 
 	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
 	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
-	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 552b990..98247f8 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
 		zpci_cleanup_bus_resources(zdev);
 		return -EIO;
 	}
-
+	pci_bus_add_devices(zdev->bus);
 	zdev->bus->max_bus_speed = zdev->max_bus_speed;
 	return 0;
 }
diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index 1bc09ee..efc1051 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
 
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
+		pci_bus_add_devices(bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 899b720..2971076 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
 
 		/* Assign devices with resources */
 		pci_assign_unassigned_resources();
+		pci_bus_add_devices(root_bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 1f80a88..007466e 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -308,6 +308,8 @@ int __init pcibios_init(void)
 			pci_add_resource(&resources, &iomem_resource);
 			bus = pci_scan_root_bus(NULL, 0, controller->ops,
 						controller, &resources);
+			if (bus)
+				pci_bus_add_devices(bus);
 			controller->root_bus = bus;
 			controller->last_busno = bus->busn_res.end;
 		}
diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
index e39f9c5..3f8fb2c 100644
--- a/arch/tile/kernel/pci_gx.c
+++ b/arch/tile/kernel/pci_gx.c
@@ -889,6 +889,8 @@ int __init pcibios_init(void)
 		controller->first_busno = next_busno;
 		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
 					controller, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		controller->root_bus = bus;
 		next_busno = bus->busn_res.end + 1;
 	}
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 7b20bcc..300d39e 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
 		pci_free_resource_list(&resources);
 		kfree(sd);
 	}
+	pci_bus_add_devices(bus);
 }
 
 void __init pcibios_set_cache_line_size(void)
diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
index 5b34033..f2ae64e 100644
--- a/arch/xtensa/kernel/pci.c
+++ b/arch/xtensa/kernel/pci.c
@@ -185,6 +185,8 @@ static int __init pcibios_init(void)
 		pci_controller_apertures(pci_ctrl, &resources);
 		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
 					pci_ctrl->ops, pci_ctrl, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		pci_ctrl->bus = bus;
 		pci_ctrl->last_busno = bus->busn_res.end;
 		if (next_busno <= pci_ctrl->last_busno)
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 9ecabfa..c17afe5 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 	if (!bus)
 		return -ENOMEM;
 
+	pci_bus_add_devices(bus);
 	platform_set_drvdata(pdev, port);
 	return 0;
 }
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 79775b5..2ab5a40 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 	if (!found)
 		pci_bus_update_busn_res_end(b, max);
 
-	pci_bus_add_devices(b);
 	return b;
 }
 EXPORT_SYMBOL(pci_scan_root_bus);
-- 
1.7.1

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

Just like pci_scan_bus(), we also should rip out
pci_bus_add_devices() from pci_scan_root_bus().
Lots platforms first call pci_scan_root_bus(), but
after that, they call pci_bus_size_bridges() and
pci_bus_assign_resources(). Place pci_bus_add_devices()
in pci_scan_root_bus() hurts PCI scan logic.
Should no functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: linux-alpha@vger.kernel.org
CC: linux-ia64@vger.kernel.org
CC: linux-mips@linux-mips.org
CC: linux-xtensa@linux-xtensa.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-sh@vger.kernel.org
CC: sparclinux@vger.kernel.org
---
 arch/alpha/kernel/pci.c          |    2 ++
 arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
 arch/ia64/sn/kernel/io_init.c    |    1 +
 arch/microblaze/pci/pci-common.c |    1 +
 arch/mips/pci/pci.c              |    1 +
 arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
 arch/s390/pci/pci.c              |    2 +-
 arch/sh/drivers/pci/pci.c        |    1 +
 arch/sparc/kernel/leon_pci.c     |    1 +
 arch/tile/kernel/pci.c           |    2 ++
 arch/tile/kernel/pci_gx.c        |    2 ++
 arch/x86/pci/common.c            |    1 +
 arch/xtensa/kernel/pci.c         |    2 ++
 drivers/pci/host/pci-xgene.c     |    1 +
 drivers/pci/probe.c              |    1 -
 15 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 076c35c..97f9730 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -334,6 +334,8 @@ common_init_pci(void)
 
 		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
 					hose, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		hose->bus = bus;
 		hose->need_domain_info = need_domain_info;
 		next_busno = bus->busn_res.end + 1;
diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index efa5d65..2b36044 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 
 int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	struct pci_ops *dir = NULL;
 	LIST_HEAD(resources);
 
@@ -383,12 +384,13 @@ int __init pcibios_init(void)
 	printk("PCI: Probing PCI hardware\n");
 	pci_add_resource(&resources, &pci_ioport_resource);
 	pci_add_resource(&resources, &pci_iomem_resource);
-	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
-
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
index 0b5ce82..63b43a6 100644
--- a/arch/ia64/sn/kernel/io_init.c
+++ b/arch/ia64/sn/kernel/io_init.c
@@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
 		kfree(res);
 		kfree(controller);
 	}
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 9037914..651e25d 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
 	hose->bus = bus;
 
 	hose->last_busno = bus->busn_res.end;
+	pci_bus_add_devices(bus);
 }
 
 static int __init pcibios_init(void)
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 1bf60b1..f083688 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
 		if (!pci_has_flag(PCI_PROBE_ONLY)) {
 			pci_bus_size_bridges(bus);
 			pci_bus_assign_resources(bus);
+			pci_bus_add_devices(bus);
 		}
 	}
 }
diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
index 6b4339f..860aa35 100644
--- a/arch/mn10300/unit-asb2305/pci.c
+++ b/arch/mn10300/unit-asb2305/pci.c
@@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
  */
 static int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	resource_size_t io_offset, mem_offset;
 	LIST_HEAD(resources);
 
@@ -376,11 +377,13 @@ static int __init pcibios_init(void)
 
 	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
 	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
-	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 552b990..98247f8 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
 		zpci_cleanup_bus_resources(zdev);
 		return -EIO;
 	}
-
+	pci_bus_add_devices(zdev->bus);
 	zdev->bus->max_bus_speed = zdev->max_bus_speed;
 	return 0;
 }
diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index 1bc09ee..efc1051 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
 
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
+		pci_bus_add_devices(bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 899b720..2971076 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
 
 		/* Assign devices with resources */
 		pci_assign_unassigned_resources();
+		pci_bus_add_devices(root_bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 1f80a88..007466e 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -308,6 +308,8 @@ int __init pcibios_init(void)
 			pci_add_resource(&resources, &iomem_resource);
 			bus = pci_scan_root_bus(NULL, 0, controller->ops,
 						controller, &resources);
+			if (bus)
+				pci_bus_add_devices(bus);
 			controller->root_bus = bus;
 			controller->last_busno = bus->busn_res.end;
 		}
diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
index e39f9c5..3f8fb2c 100644
--- a/arch/tile/kernel/pci_gx.c
+++ b/arch/tile/kernel/pci_gx.c
@@ -889,6 +889,8 @@ int __init pcibios_init(void)
 		controller->first_busno = next_busno;
 		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
 					controller, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		controller->root_bus = bus;
 		next_busno = bus->busn_res.end + 1;
 	}
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 7b20bcc..300d39e 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
 		pci_free_resource_list(&resources);
 		kfree(sd);
 	}
+	pci_bus_add_devices(bus);
 }
 
 void __init pcibios_set_cache_line_size(void)
diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
index 5b34033..f2ae64e 100644
--- a/arch/xtensa/kernel/pci.c
+++ b/arch/xtensa/kernel/pci.c
@@ -185,6 +185,8 @@ static int __init pcibios_init(void)
 		pci_controller_apertures(pci_ctrl, &resources);
 		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
 					pci_ctrl->ops, pci_ctrl, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		pci_ctrl->bus = bus;
 		pci_ctrl->last_busno = bus->busn_res.end;
 		if (next_busno <= pci_ctrl->last_busno)
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 9ecabfa..c17afe5 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 	if (!bus)
 		return -ENOMEM;
 
+	pci_bus_add_devices(bus);
 	platform_set_drvdata(pdev, port);
 	return 0;
 }
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 79775b5..2ab5a40 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 	if (!found)
 		pci_bus_update_busn_res_end(b, max);
 
-	pci_bus_add_devices(b);
 	return b;
 }
 EXPORT_SYMBOL(pci_scan_root_bus);
-- 
1.7.1

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

Just like pci_scan_bus(), we also should rip out
pci_bus_add_devices() from pci_scan_root_bus().
Lots platforms first call pci_scan_root_bus(), but
after that, they call pci_bus_size_bridges() and
pci_bus_assign_resources(). Place pci_bus_add_devices()
in pci_scan_root_bus() hurts PCI scan logic.
Should no functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: linux-alpha@vger.kernel.org
CC: linux-ia64@vger.kernel.org
CC: linux-mips@linux-mips.org
CC: linux-xtensa@linux-xtensa.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-sh@vger.kernel.org
CC: sparclinux@vger.kernel.org
---
 arch/alpha/kernel/pci.c          |    2 ++
 arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
 arch/ia64/sn/kernel/io_init.c    |    1 +
 arch/microblaze/pci/pci-common.c |    1 +
 arch/mips/pci/pci.c              |    1 +
 arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
 arch/s390/pci/pci.c              |    2 +-
 arch/sh/drivers/pci/pci.c        |    1 +
 arch/sparc/kernel/leon_pci.c     |    1 +
 arch/tile/kernel/pci.c           |    2 ++
 arch/tile/kernel/pci_gx.c        |    2 ++
 arch/x86/pci/common.c            |    1 +
 arch/xtensa/kernel/pci.c         |    2 ++
 drivers/pci/host/pci-xgene.c     |    1 +
 drivers/pci/probe.c              |    1 -
 15 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 076c35c..97f9730 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -334,6 +334,8 @@ common_init_pci(void)
 
 		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
 					hose, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		hose->bus = bus;
 		hose->need_domain_info = need_domain_info;
 		next_busno = bus->busn_res.end + 1;
diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index efa5d65..2b36044 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 
 int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	struct pci_ops *dir = NULL;
 	LIST_HEAD(resources);
 
@@ -383,12 +384,13 @@ int __init pcibios_init(void)
 	printk("PCI: Probing PCI hardware\n");
 	pci_add_resource(&resources, &pci_ioport_resource);
 	pci_add_resource(&resources, &pci_iomem_resource);
-	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
-
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
index 0b5ce82..63b43a6 100644
--- a/arch/ia64/sn/kernel/io_init.c
+++ b/arch/ia64/sn/kernel/io_init.c
@@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
 		kfree(res);
 		kfree(controller);
 	}
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 9037914..651e25d 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
 	hose->bus = bus;
 
 	hose->last_busno = bus->busn_res.end;
+	pci_bus_add_devices(bus);
 }
 
 static int __init pcibios_init(void)
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 1bf60b1..f083688 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
 		if (!pci_has_flag(PCI_PROBE_ONLY)) {
 			pci_bus_size_bridges(bus);
 			pci_bus_assign_resources(bus);
+			pci_bus_add_devices(bus);
 		}
 	}
 }
diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
index 6b4339f..860aa35 100644
--- a/arch/mn10300/unit-asb2305/pci.c
+++ b/arch/mn10300/unit-asb2305/pci.c
@@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
  */
 static int __init pcibios_init(void)
 {
+	struct pci_bus *bus;
 	resource_size_t io_offset, mem_offset;
 	LIST_HEAD(resources);
 
@@ -376,11 +377,13 @@ static int __init pcibios_init(void)
 
 	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
 	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
-	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
+	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
 
 	pcibios_irq_init();
 	pcibios_fixup_irqs();
 	pcibios_resource_survey();
+	if (bus)
+		pci_bus_add_devices(bus);
 	return 0;
 }
 
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 552b990..98247f8 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
 		zpci_cleanup_bus_resources(zdev);
 		return -EIO;
 	}
-
+	pci_bus_add_devices(zdev->bus);
 	zdev->bus->max_bus_speed = zdev->max_bus_speed;
 	return 0;
 }
diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index 1bc09ee..efc1051 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
 
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
+		pci_bus_add_devices(bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 899b720..2971076 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
 
 		/* Assign devices with resources */
 		pci_assign_unassigned_resources();
+		pci_bus_add_devices(root_bus);
 	} else {
 		pci_free_resource_list(&resources);
 	}
diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 1f80a88..007466e 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -308,6 +308,8 @@ int __init pcibios_init(void)
 			pci_add_resource(&resources, &iomem_resource);
 			bus = pci_scan_root_bus(NULL, 0, controller->ops,
 						controller, &resources);
+			if (bus)
+				pci_bus_add_devices(bus);
 			controller->root_bus = bus;
 			controller->last_busno = bus->busn_res.end;
 		}
diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
index e39f9c5..3f8fb2c 100644
--- a/arch/tile/kernel/pci_gx.c
+++ b/arch/tile/kernel/pci_gx.c
@@ -889,6 +889,8 @@ int __init pcibios_init(void)
 		controller->first_busno = next_busno;
 		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
 					controller, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		controller->root_bus = bus;
 		next_busno = bus->busn_res.end + 1;
 	}
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 7b20bcc..300d39e 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
 		pci_free_resource_list(&resources);
 		kfree(sd);
 	}
+	pci_bus_add_devices(bus);
 }
 
 void __init pcibios_set_cache_line_size(void)
diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
index 5b34033..f2ae64e 100644
--- a/arch/xtensa/kernel/pci.c
+++ b/arch/xtensa/kernel/pci.c
@@ -185,6 +185,8 @@ static int __init pcibios_init(void)
 		pci_controller_apertures(pci_ctrl, &resources);
 		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
 					pci_ctrl->ops, pci_ctrl, &resources);
+		if (bus)
+			pci_bus_add_devices(bus);
 		pci_ctrl->bus = bus;
 		pci_ctrl->last_busno = bus->busn_res.end;
 		if (next_busno <= pci_ctrl->last_busno)
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 9ecabfa..c17afe5 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 	if (!bus)
 		return -ENOMEM;
 
+	pci_bus_add_devices(bus);
 	platform_set_drvdata(pdev, port);
 	return 0;
 }
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 79775b5..2ab5a40 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 	if (!found)
 		pci_bus_update_busn_res_end(b, max);
 
-	pci_bus_add_devices(b);
 	return b;
 }
 EXPORT_SYMBOL(pci_scan_root_bus);
-- 
1.7.1

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Lorenzo Pieralisi]

On Wed, Nov 19, 2014 at 07:32:49AM +0000, Yijing Wang wrote:
> Just like pci_scan_bus(), we also should rip out
> pci_bus_add_devices() from pci_scan_root_bus().
> Lots platforms first call pci_scan_root_bus(), but
> after that, they call pci_bus_size_bridges() and
> pci_bus_assign_resources(). Place pci_bus_add_devices()
> in pci_scan_root_bus() hurts PCI scan logic.
> Should no functional change.

I agree with the patch, I do not agree it provides no functional change.
You are actually fixing an issue on systems where the root bus
is scanned at device_initcall time and pci drivers may have already been
registered, so they end up being probed with possibly unassigned
resources as you correctly mention in the log.

You could move the sequence:

if (!pci_has_flag(PCI_PROBE_ONLY)) {
	pci_bus_size_bridges();
	pci_bus_assign_resources();
}

in pci_scan_root_bus() instead of taking pci_bus_add_devices() out ?

Would this break any code ? Or add a helper function:

pci_scan_root_bus(..., bool assign) ?

arm64 xgene code has already been fixed by reshuffling the scanning code
sequence.

I think we should have a look at eg drivers/pci/host/pcie-xilinx.c, I
think there is a possible initcall ordering issue there (drivers being probed
with unassigned resources).

Lorenzo

> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> CC: linux-alpha@vger.kernel.org
> CC: linux-ia64@vger.kernel.org
> CC: linux-mips@linux-mips.org
> CC: linux-xtensa@linux-xtensa.org
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-sh@vger.kernel.org
> CC: sparclinux@vger.kernel.org
> ---
>  arch/alpha/kernel/pci.c          |    2 ++
>  arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
>  arch/ia64/sn/kernel/io_init.c    |    1 +
>  arch/microblaze/pci/pci-common.c |    1 +
>  arch/mips/pci/pci.c              |    1 +
>  arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
>  arch/s390/pci/pci.c              |    2 +-
>  arch/sh/drivers/pci/pci.c        |    1 +
>  arch/sparc/kernel/leon_pci.c     |    1 +
>  arch/tile/kernel/pci.c           |    2 ++
>  arch/tile/kernel/pci_gx.c        |    2 ++
>  arch/x86/pci/common.c            |    1 +
>  arch/xtensa/kernel/pci.c         |    2 ++
>  drivers/pci/host/pci-xgene.c     |    1 +
>  drivers/pci/probe.c              |    1 -
>  15 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index 076c35c..97f9730 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -334,6 +334,8 @@ common_init_pci(void)
>  
>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>  					hose, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		hose->bus = bus;
>  		hose->need_domain_info = need_domain_info;
>  		next_busno = bus->busn_res.end + 1;
> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
> index efa5d65..2b36044 100644
> --- a/arch/frv/mb93090-mb00/pci-vdk.c
> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
> @@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>  
>  int __init pcibios_init(void)
>  {
> +	struct pci_bus *bus;
>  	struct pci_ops *dir = NULL;
>  	LIST_HEAD(resources);
>  
> @@ -383,12 +384,13 @@ int __init pcibios_init(void)
>  	printk("PCI: Probing PCI hardware\n");
>  	pci_add_resource(&resources, &pci_ioport_resource);
>  	pci_add_resource(&resources, &pci_iomem_resource);
> -	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
> +	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>  
>  	pcibios_irq_init();
>  	pcibios_fixup_irqs();
>  	pcibios_resource_survey();
> -
> +	if (bus)
> +		pci_bus_add_devices(bus);
>  	return 0;
>  }
>  
> diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
> index 0b5ce82..63b43a6 100644
> --- a/arch/ia64/sn/kernel/io_init.c
> +++ b/arch/ia64/sn/kernel/io_init.c
> @@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
>  		kfree(res);
>  		kfree(controller);
>  	}
> +	pci_bus_add_devices(bus);
>  }
>  
>  /*
> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> index 9037914..651e25d 100644
> --- a/arch/microblaze/pci/pci-common.c
> +++ b/arch/microblaze/pci/pci-common.c
> @@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
>  	hose->bus = bus;
>  
>  	hose->last_busno = bus->busn_res.end;
> +	pci_bus_add_devices(bus);
>  }
>  
>  static int __init pcibios_init(void)
> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> index 1bf60b1..f083688 100644
> --- a/arch/mips/pci/pci.c
> +++ b/arch/mips/pci/pci.c
> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>  			pci_bus_size_bridges(bus);
>  			pci_bus_assign_resources(bus);
> +			pci_bus_add_devices(bus);
>  		}
>  	}
>  }
> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
> index 6b4339f..860aa35 100644
> --- a/arch/mn10300/unit-asb2305/pci.c
> +++ b/arch/mn10300/unit-asb2305/pci.c
> @@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>   */
>  static int __init pcibios_init(void)
>  {
> +	struct pci_bus *bus;
>  	resource_size_t io_offset, mem_offset;
>  	LIST_HEAD(resources);
>  
> @@ -376,11 +377,13 @@ static int __init pcibios_init(void)
>  
>  	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
>  	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
> -	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
> +	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>  
>  	pcibios_irq_init();
>  	pcibios_fixup_irqs();
>  	pcibios_resource_survey();
> +	if (bus)
> +		pci_bus_add_devices(bus);
>  	return 0;
>  }
>  
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 552b990..98247f8 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
>  		zpci_cleanup_bus_resources(zdev);
>  		return -EIO;
>  	}
> -
> +	pci_bus_add_devices(zdev->bus);
>  	zdev->bus->max_bus_speed = zdev->max_bus_speed;
>  	return 0;
>  }
> diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
> index 1bc09ee..efc1051 100644
> --- a/arch/sh/drivers/pci/pci.c
> +++ b/arch/sh/drivers/pci/pci.c
> @@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
>  
>  		pci_bus_size_bridges(bus);
>  		pci_bus_assign_resources(bus);
> +		pci_bus_add_devices(bus);
>  	} else {
>  		pci_free_resource_list(&resources);
>  	}
> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
> index 899b720..2971076 100644
> --- a/arch/sparc/kernel/leon_pci.c
> +++ b/arch/sparc/kernel/leon_pci.c
> @@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
>  
>  		/* Assign devices with resources */
>  		pci_assign_unassigned_resources();
> +		pci_bus_add_devices(root_bus);
>  	} else {
>  		pci_free_resource_list(&resources);
>  	}
> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index 1f80a88..007466e 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>  			pci_add_resource(&resources, &iomem_resource);
>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>  						controller, &resources);
> +			if (bus)
> +				pci_bus_add_devices(bus);
>  			controller->root_bus = bus;
>  			controller->last_busno = bus->busn_res.end;
>  		}
> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
> index e39f9c5..3f8fb2c 100644
> --- a/arch/tile/kernel/pci_gx.c
> +++ b/arch/tile/kernel/pci_gx.c
> @@ -889,6 +889,8 @@ int __init pcibios_init(void)
>  		controller->first_busno = next_busno;
>  		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
>  					controller, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		controller->root_bus = bus;
>  		next_busno = bus->busn_res.end + 1;
>  	}
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 7b20bcc..300d39e 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
>  		pci_free_resource_list(&resources);
>  		kfree(sd);
>  	}
> +	pci_bus_add_devices(bus);
>  }
>  
>  void __init pcibios_set_cache_line_size(void)
> diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
> index 5b34033..f2ae64e 100644
> --- a/arch/xtensa/kernel/pci.c
> +++ b/arch/xtensa/kernel/pci.c
> @@ -185,6 +185,8 @@ static int __init pcibios_init(void)
>  		pci_controller_apertures(pci_ctrl, &resources);
>  		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
>  					pci_ctrl->ops, pci_ctrl, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		pci_ctrl->bus = bus;
>  		pci_ctrl->last_busno = bus->busn_res.end;
>  		if (next_busno <= pci_ctrl->last_busno)
> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> index 9ecabfa..c17afe5 100644
> --- a/drivers/pci/host/pci-xgene.c
> +++ b/drivers/pci/host/pci-xgene.c
> @@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
>  	if (!bus)
>  		return -ENOMEM;
>  
> +	pci_bus_add_devices(bus);
>  	platform_set_drvdata(pdev, port);
>  	return 0;
>  }
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 79775b5..2ab5a40 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>  	if (!found)
>  		pci_bus_update_busn_res_end(b, max);
>  
> -	pci_bus_add_devices(b);
>  	return b;
>  }
>  EXPORT_SYMBOL(pci_scan_root_bus);
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Lorenzo Pieralisi]

On Wed, Nov 19, 2014 at 07:32:49AM +0000, Yijing Wang wrote:
> Just like pci_scan_bus(), we also should rip out
> pci_bus_add_devices() from pci_scan_root_bus().
> Lots platforms first call pci_scan_root_bus(), but
> after that, they call pci_bus_size_bridges() and
> pci_bus_assign_resources(). Place pci_bus_add_devices()
> in pci_scan_root_bus() hurts PCI scan logic.
> Should no functional change.

I agree with the patch, I do not agree it provides no functional change.
You are actually fixing an issue on systems where the root bus
is scanned at device_initcall time and pci drivers may have already been
registered, so they end up being probed with possibly unassigned
resources as you correctly mention in the log.

You could move the sequence:

if (!pci_has_flag(PCI_PROBE_ONLY)) {
	pci_bus_size_bridges();
	pci_bus_assign_resources();
}

in pci_scan_root_bus() instead of taking pci_bus_add_devices() out ?

Would this break any code ? Or add a helper function:

pci_scan_root_bus(..., bool assign) ?

arm64 xgene code has already been fixed by reshuffling the scanning code
sequence.

I think we should have a look at eg drivers/pci/host/pcie-xilinx.c, I
think there is a possible initcall ordering issue there (drivers being probed
with unassigned resources).

Lorenzo

> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> CC: linux-alpha at vger.kernel.org
> CC: linux-ia64 at vger.kernel.org
> CC: linux-mips at linux-mips.org
> CC: linux-xtensa at linux-xtensa.org
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-sh at vger.kernel.org
> CC: sparclinux at vger.kernel.org
> ---
>  arch/alpha/kernel/pci.c          |    2 ++
>  arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
>  arch/ia64/sn/kernel/io_init.c    |    1 +
>  arch/microblaze/pci/pci-common.c |    1 +
>  arch/mips/pci/pci.c              |    1 +
>  arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
>  arch/s390/pci/pci.c              |    2 +-
>  arch/sh/drivers/pci/pci.c        |    1 +
>  arch/sparc/kernel/leon_pci.c     |    1 +
>  arch/tile/kernel/pci.c           |    2 ++
>  arch/tile/kernel/pci_gx.c        |    2 ++
>  arch/x86/pci/common.c            |    1 +
>  arch/xtensa/kernel/pci.c         |    2 ++
>  drivers/pci/host/pci-xgene.c     |    1 +
>  drivers/pci/probe.c              |    1 -
>  15 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index 076c35c..97f9730 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -334,6 +334,8 @@ common_init_pci(void)
>  
>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>  					hose, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		hose->bus = bus;
>  		hose->need_domain_info = need_domain_info;
>  		next_busno = bus->busn_res.end + 1;
> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
> index efa5d65..2b36044 100644
> --- a/arch/frv/mb93090-mb00/pci-vdk.c
> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
> @@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>  
>  int __init pcibios_init(void)
>  {
> +	struct pci_bus *bus;
>  	struct pci_ops *dir = NULL;
>  	LIST_HEAD(resources);
>  
> @@ -383,12 +384,13 @@ int __init pcibios_init(void)
>  	printk("PCI: Probing PCI hardware\n");
>  	pci_add_resource(&resources, &pci_ioport_resource);
>  	pci_add_resource(&resources, &pci_iomem_resource);
> -	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
> +	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>  
>  	pcibios_irq_init();
>  	pcibios_fixup_irqs();
>  	pcibios_resource_survey();
> -
> +	if (bus)
> +		pci_bus_add_devices(bus);
>  	return 0;
>  }
>  
> diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
> index 0b5ce82..63b43a6 100644
> --- a/arch/ia64/sn/kernel/io_init.c
> +++ b/arch/ia64/sn/kernel/io_init.c
> @@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
>  		kfree(res);
>  		kfree(controller);
>  	}
> +	pci_bus_add_devices(bus);
>  }
>  
>  /*
> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> index 9037914..651e25d 100644
> --- a/arch/microblaze/pci/pci-common.c
> +++ b/arch/microblaze/pci/pci-common.c
> @@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
>  	hose->bus = bus;
>  
>  	hose->last_busno = bus->busn_res.end;
> +	pci_bus_add_devices(bus);
>  }
>  
>  static int __init pcibios_init(void)
> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> index 1bf60b1..f083688 100644
> --- a/arch/mips/pci/pci.c
> +++ b/arch/mips/pci/pci.c
> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>  			pci_bus_size_bridges(bus);
>  			pci_bus_assign_resources(bus);
> +			pci_bus_add_devices(bus);
>  		}
>  	}
>  }
> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
> index 6b4339f..860aa35 100644
> --- a/arch/mn10300/unit-asb2305/pci.c
> +++ b/arch/mn10300/unit-asb2305/pci.c
> @@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>   */
>  static int __init pcibios_init(void)
>  {
> +	struct pci_bus *bus;
>  	resource_size_t io_offset, mem_offset;
>  	LIST_HEAD(resources);
>  
> @@ -376,11 +377,13 @@ static int __init pcibios_init(void)
>  
>  	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
>  	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
> -	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
> +	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>  
>  	pcibios_irq_init();
>  	pcibios_fixup_irqs();
>  	pcibios_resource_survey();
> +	if (bus)
> +		pci_bus_add_devices(bus);
>  	return 0;
>  }
>  
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 552b990..98247f8 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
>  		zpci_cleanup_bus_resources(zdev);
>  		return -EIO;
>  	}
> -
> +	pci_bus_add_devices(zdev->bus);
>  	zdev->bus->max_bus_speed = zdev->max_bus_speed;
>  	return 0;
>  }
> diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
> index 1bc09ee..efc1051 100644
> --- a/arch/sh/drivers/pci/pci.c
> +++ b/arch/sh/drivers/pci/pci.c
> @@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
>  
>  		pci_bus_size_bridges(bus);
>  		pci_bus_assign_resources(bus);
> +		pci_bus_add_devices(bus);
>  	} else {
>  		pci_free_resource_list(&resources);
>  	}
> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
> index 899b720..2971076 100644
> --- a/arch/sparc/kernel/leon_pci.c
> +++ b/arch/sparc/kernel/leon_pci.c
> @@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
>  
>  		/* Assign devices with resources */
>  		pci_assign_unassigned_resources();
> +		pci_bus_add_devices(root_bus);
>  	} else {
>  		pci_free_resource_list(&resources);
>  	}
> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index 1f80a88..007466e 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>  			pci_add_resource(&resources, &iomem_resource);
>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>  						controller, &resources);
> +			if (bus)
> +				pci_bus_add_devices(bus);
>  			controller->root_bus = bus;
>  			controller->last_busno = bus->busn_res.end;
>  		}
> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
> index e39f9c5..3f8fb2c 100644
> --- a/arch/tile/kernel/pci_gx.c
> +++ b/arch/tile/kernel/pci_gx.c
> @@ -889,6 +889,8 @@ int __init pcibios_init(void)
>  		controller->first_busno = next_busno;
>  		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
>  					controller, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		controller->root_bus = bus;
>  		next_busno = bus->busn_res.end + 1;
>  	}
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 7b20bcc..300d39e 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
>  		pci_free_resource_list(&resources);
>  		kfree(sd);
>  	}
> +	pci_bus_add_devices(bus);
>  }
>  
>  void __init pcibios_set_cache_line_size(void)
> diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
> index 5b34033..f2ae64e 100644
> --- a/arch/xtensa/kernel/pci.c
> +++ b/arch/xtensa/kernel/pci.c
> @@ -185,6 +185,8 @@ static int __init pcibios_init(void)
>  		pci_controller_apertures(pci_ctrl, &resources);
>  		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
>  					pci_ctrl->ops, pci_ctrl, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		pci_ctrl->bus = bus;
>  		pci_ctrl->last_busno = bus->busn_res.end;
>  		if (next_busno <= pci_ctrl->last_busno)
> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> index 9ecabfa..c17afe5 100644
> --- a/drivers/pci/host/pci-xgene.c
> +++ b/drivers/pci/host/pci-xgene.c
> @@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
>  	if (!bus)
>  		return -ENOMEM;
>  
> +	pci_bus_add_devices(bus);
>  	platform_set_drvdata(pdev, port);
>  	return 0;
>  }
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 79775b5..2ab5a40 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>  	if (!found)
>  		pci_bus_update_busn_res_end(b, max);
>  
> -	pci_bus_add_devices(b);
>  	return b;
>  }
>  EXPORT_SYMBOL(pci_scan_root_bus);
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Lorenzo Pieralisi]

On Wed, Nov 19, 2014 at 07:32:49AM +0000, Yijing Wang wrote:
> Just like pci_scan_bus(), we also should rip out
> pci_bus_add_devices() from pci_scan_root_bus().
> Lots platforms first call pci_scan_root_bus(), but
> after that, they call pci_bus_size_bridges() and
> pci_bus_assign_resources(). Place pci_bus_add_devices()
> in pci_scan_root_bus() hurts PCI scan logic.
> Should no functional change.

I agree with the patch, I do not agree it provides no functional change.
You are actually fixing an issue on systems where the root bus
is scanned at device_initcall time and pci drivers may have already been
registered, so they end up being probed with possibly unassigned
resources as you correctly mention in the log.

You could move the sequence:

if (!pci_has_flag(PCI_PROBE_ONLY)) {
	pci_bus_size_bridges();
	pci_bus_assign_resources();
}

in pci_scan_root_bus() instead of taking pci_bus_add_devices() out ?

Would this break any code ? Or add a helper function:

pci_scan_root_bus(..., bool assign) ?

arm64 xgene code has already been fixed by reshuffling the scanning code
sequence.

I think we should have a look at eg drivers/pci/host/pcie-xilinx.c, I
think there is a possible initcall ordering issue there (drivers being probed
with unassigned resources).

Lorenzo

> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> CC: linux-alpha@vger.kernel.org
> CC: linux-ia64@vger.kernel.org
> CC: linux-mips@linux-mips.org
> CC: linux-xtensa@linux-xtensa.org
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-sh@vger.kernel.org
> CC: sparclinux@vger.kernel.org
> ---
>  arch/alpha/kernel/pci.c          |    2 ++
>  arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
>  arch/ia64/sn/kernel/io_init.c    |    1 +
>  arch/microblaze/pci/pci-common.c |    1 +
>  arch/mips/pci/pci.c              |    1 +
>  arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
>  arch/s390/pci/pci.c              |    2 +-
>  arch/sh/drivers/pci/pci.c        |    1 +
>  arch/sparc/kernel/leon_pci.c     |    1 +
>  arch/tile/kernel/pci.c           |    2 ++
>  arch/tile/kernel/pci_gx.c        |    2 ++
>  arch/x86/pci/common.c            |    1 +
>  arch/xtensa/kernel/pci.c         |    2 ++
>  drivers/pci/host/pci-xgene.c     |    1 +
>  drivers/pci/probe.c              |    1 -
>  15 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index 076c35c..97f9730 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -334,6 +334,8 @@ common_init_pci(void)
>  
>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>  					hose, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		hose->bus = bus;
>  		hose->need_domain_info = need_domain_info;
>  		next_busno = bus->busn_res.end + 1;
> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
> index efa5d65..2b36044 100644
> --- a/arch/frv/mb93090-mb00/pci-vdk.c
> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
> @@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>  
>  int __init pcibios_init(void)
>  {
> +	struct pci_bus *bus;
>  	struct pci_ops *dir = NULL;
>  	LIST_HEAD(resources);
>  
> @@ -383,12 +384,13 @@ int __init pcibios_init(void)
>  	printk("PCI: Probing PCI hardware\n");
>  	pci_add_resource(&resources, &pci_ioport_resource);
>  	pci_add_resource(&resources, &pci_iomem_resource);
> -	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
> +	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>  
>  	pcibios_irq_init();
>  	pcibios_fixup_irqs();
>  	pcibios_resource_survey();
> -
> +	if (bus)
> +		pci_bus_add_devices(bus);
>  	return 0;
>  }
>  
> diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
> index 0b5ce82..63b43a6 100644
> --- a/arch/ia64/sn/kernel/io_init.c
> +++ b/arch/ia64/sn/kernel/io_init.c
> @@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
>  		kfree(res);
>  		kfree(controller);
>  	}
> +	pci_bus_add_devices(bus);
>  }
>  
>  /*
> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> index 9037914..651e25d 100644
> --- a/arch/microblaze/pci/pci-common.c
> +++ b/arch/microblaze/pci/pci-common.c
> @@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
>  	hose->bus = bus;
>  
>  	hose->last_busno = bus->busn_res.end;
> +	pci_bus_add_devices(bus);
>  }
>  
>  static int __init pcibios_init(void)
> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> index 1bf60b1..f083688 100644
> --- a/arch/mips/pci/pci.c
> +++ b/arch/mips/pci/pci.c
> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>  			pci_bus_size_bridges(bus);
>  			pci_bus_assign_resources(bus);
> +			pci_bus_add_devices(bus);
>  		}
>  	}
>  }
> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
> index 6b4339f..860aa35 100644
> --- a/arch/mn10300/unit-asb2305/pci.c
> +++ b/arch/mn10300/unit-asb2305/pci.c
> @@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>   */
>  static int __init pcibios_init(void)
>  {
> +	struct pci_bus *bus;
>  	resource_size_t io_offset, mem_offset;
>  	LIST_HEAD(resources);
>  
> @@ -376,11 +377,13 @@ static int __init pcibios_init(void)
>  
>  	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
>  	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
> -	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
> +	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>  
>  	pcibios_irq_init();
>  	pcibios_fixup_irqs();
>  	pcibios_resource_survey();
> +	if (bus)
> +		pci_bus_add_devices(bus);
>  	return 0;
>  }
>  
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 552b990..98247f8 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
>  		zpci_cleanup_bus_resources(zdev);
>  		return -EIO;
>  	}
> -
> +	pci_bus_add_devices(zdev->bus);
>  	zdev->bus->max_bus_speed = zdev->max_bus_speed;
>  	return 0;
>  }
> diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
> index 1bc09ee..efc1051 100644
> --- a/arch/sh/drivers/pci/pci.c
> +++ b/arch/sh/drivers/pci/pci.c
> @@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
>  
>  		pci_bus_size_bridges(bus);
>  		pci_bus_assign_resources(bus);
> +		pci_bus_add_devices(bus);
>  	} else {
>  		pci_free_resource_list(&resources);
>  	}
> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
> index 899b720..2971076 100644
> --- a/arch/sparc/kernel/leon_pci.c
> +++ b/arch/sparc/kernel/leon_pci.c
> @@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
>  
>  		/* Assign devices with resources */
>  		pci_assign_unassigned_resources();
> +		pci_bus_add_devices(root_bus);
>  	} else {
>  		pci_free_resource_list(&resources);
>  	}
> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index 1f80a88..007466e 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>  			pci_add_resource(&resources, &iomem_resource);
>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>  						controller, &resources);
> +			if (bus)
> +				pci_bus_add_devices(bus);
>  			controller->root_bus = bus;
>  			controller->last_busno = bus->busn_res.end;
>  		}
> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
> index e39f9c5..3f8fb2c 100644
> --- a/arch/tile/kernel/pci_gx.c
> +++ b/arch/tile/kernel/pci_gx.c
> @@ -889,6 +889,8 @@ int __init pcibios_init(void)
>  		controller->first_busno = next_busno;
>  		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
>  					controller, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		controller->root_bus = bus;
>  		next_busno = bus->busn_res.end + 1;
>  	}
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 7b20bcc..300d39e 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
>  		pci_free_resource_list(&resources);
>  		kfree(sd);
>  	}
> +	pci_bus_add_devices(bus);
>  }
>  
>  void __init pcibios_set_cache_line_size(void)
> diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
> index 5b34033..f2ae64e 100644
> --- a/arch/xtensa/kernel/pci.c
> +++ b/arch/xtensa/kernel/pci.c
> @@ -185,6 +185,8 @@ static int __init pcibios_init(void)
>  		pci_controller_apertures(pci_ctrl, &resources);
>  		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
>  					pci_ctrl->ops, pci_ctrl, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		pci_ctrl->bus = bus;
>  		pci_ctrl->last_busno = bus->busn_res.end;
>  		if (next_busno <= pci_ctrl->last_busno)
> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> index 9ecabfa..c17afe5 100644
> --- a/drivers/pci/host/pci-xgene.c
> +++ b/drivers/pci/host/pci-xgene.c
> @@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
>  	if (!bus)
>  		return -ENOMEM;
>  
> +	pci_bus_add_devices(bus);
>  	platform_set_drvdata(pdev, port);
>  	return 0;
>  }
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 79775b5..2ab5a40 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>  	if (!found)
>  		pci_bus_update_busn_res_end(b, max);
>  
> -	pci_bus_add_devices(b);
>  	return b;
>  }
>  EXPORT_SYMBOL(pci_scan_root_bus);
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Lorenzo Pieralisi]

On Wed, Nov 19, 2014 at 07:32:49AM +0000, Yijing Wang wrote:
> Just like pci_scan_bus(), we also should rip out
> pci_bus_add_devices() from pci_scan_root_bus().
> Lots platforms first call pci_scan_root_bus(), but
> after that, they call pci_bus_size_bridges() and
> pci_bus_assign_resources(). Place pci_bus_add_devices()
> in pci_scan_root_bus() hurts PCI scan logic.
> Should no functional change.

I agree with the patch, I do not agree it provides no functional change.
You are actually fixing an issue on systems where the root bus
is scanned at device_initcall time and pci drivers may have already been
registered, so they end up being probed with possibly unassigned
resources as you correctly mention in the log.

You could move the sequence:

if (!pci_has_flag(PCI_PROBE_ONLY)) {
	pci_bus_size_bridges();
	pci_bus_assign_resources();
}

in pci_scan_root_bus() instead of taking pci_bus_add_devices() out ?

Would this break any code ? Or add a helper function:

pci_scan_root_bus(..., bool assign) ?

arm64 xgene code has already been fixed by reshuffling the scanning code
sequence.

I think we should have a look at eg drivers/pci/host/pcie-xilinx.c, I
think there is a possible initcall ordering issue there (drivers being probed
with unassigned resources).

Lorenzo

> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> CC: linux-alpha@vger.kernel.org
> CC: linux-ia64@vger.kernel.org
> CC: linux-mips@linux-mips.org
> CC: linux-xtensa@linux-xtensa.org
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-sh@vger.kernel.org
> CC: sparclinux@vger.kernel.org
> ---
>  arch/alpha/kernel/pci.c          |    2 ++
>  arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
>  arch/ia64/sn/kernel/io_init.c    |    1 +
>  arch/microblaze/pci/pci-common.c |    1 +
>  arch/mips/pci/pci.c              |    1 +
>  arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
>  arch/s390/pci/pci.c              |    2 +-
>  arch/sh/drivers/pci/pci.c        |    1 +
>  arch/sparc/kernel/leon_pci.c     |    1 +
>  arch/tile/kernel/pci.c           |    2 ++
>  arch/tile/kernel/pci_gx.c        |    2 ++
>  arch/x86/pci/common.c            |    1 +
>  arch/xtensa/kernel/pci.c         |    2 ++
>  drivers/pci/host/pci-xgene.c     |    1 +
>  drivers/pci/probe.c              |    1 -
>  15 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index 076c35c..97f9730 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -334,6 +334,8 @@ common_init_pci(void)
>  
>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>  					hose, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		hose->bus = bus;
>  		hose->need_domain_info = need_domain_info;
>  		next_busno = bus->busn_res.end + 1;
> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
> index efa5d65..2b36044 100644
> --- a/arch/frv/mb93090-mb00/pci-vdk.c
> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
> @@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>  
>  int __init pcibios_init(void)
>  {
> +	struct pci_bus *bus;
>  	struct pci_ops *dir = NULL;
>  	LIST_HEAD(resources);
>  
> @@ -383,12 +384,13 @@ int __init pcibios_init(void)
>  	printk("PCI: Probing PCI hardware\n");
>  	pci_add_resource(&resources, &pci_ioport_resource);
>  	pci_add_resource(&resources, &pci_iomem_resource);
> -	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
> +	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>  
>  	pcibios_irq_init();
>  	pcibios_fixup_irqs();
>  	pcibios_resource_survey();
> -
> +	if (bus)
> +		pci_bus_add_devices(bus);
>  	return 0;
>  }
>  
> diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
> index 0b5ce82..63b43a6 100644
> --- a/arch/ia64/sn/kernel/io_init.c
> +++ b/arch/ia64/sn/kernel/io_init.c
> @@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
>  		kfree(res);
>  		kfree(controller);
>  	}
> +	pci_bus_add_devices(bus);
>  }
>  
>  /*
> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> index 9037914..651e25d 100644
> --- a/arch/microblaze/pci/pci-common.c
> +++ b/arch/microblaze/pci/pci-common.c
> @@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
>  	hose->bus = bus;
>  
>  	hose->last_busno = bus->busn_res.end;
> +	pci_bus_add_devices(bus);
>  }
>  
>  static int __init pcibios_init(void)
> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> index 1bf60b1..f083688 100644
> --- a/arch/mips/pci/pci.c
> +++ b/arch/mips/pci/pci.c
> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>  			pci_bus_size_bridges(bus);
>  			pci_bus_assign_resources(bus);
> +			pci_bus_add_devices(bus);
>  		}
>  	}
>  }
> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
> index 6b4339f..860aa35 100644
> --- a/arch/mn10300/unit-asb2305/pci.c
> +++ b/arch/mn10300/unit-asb2305/pci.c
> @@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>   */
>  static int __init pcibios_init(void)
>  {
> +	struct pci_bus *bus;
>  	resource_size_t io_offset, mem_offset;
>  	LIST_HEAD(resources);
>  
> @@ -376,11 +377,13 @@ static int __init pcibios_init(void)
>  
>  	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
>  	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
> -	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
> +	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>  
>  	pcibios_irq_init();
>  	pcibios_fixup_irqs();
>  	pcibios_resource_survey();
> +	if (bus)
> +		pci_bus_add_devices(bus);
>  	return 0;
>  }
>  
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 552b990..98247f8 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
>  		zpci_cleanup_bus_resources(zdev);
>  		return -EIO;
>  	}
> -
> +	pci_bus_add_devices(zdev->bus);
>  	zdev->bus->max_bus_speed = zdev->max_bus_speed;
>  	return 0;
>  }
> diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
> index 1bc09ee..efc1051 100644
> --- a/arch/sh/drivers/pci/pci.c
> +++ b/arch/sh/drivers/pci/pci.c
> @@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
>  
>  		pci_bus_size_bridges(bus);
>  		pci_bus_assign_resources(bus);
> +		pci_bus_add_devices(bus);
>  	} else {
>  		pci_free_resource_list(&resources);
>  	}
> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
> index 899b720..2971076 100644
> --- a/arch/sparc/kernel/leon_pci.c
> +++ b/arch/sparc/kernel/leon_pci.c
> @@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
>  
>  		/* Assign devices with resources */
>  		pci_assign_unassigned_resources();
> +		pci_bus_add_devices(root_bus);
>  	} else {
>  		pci_free_resource_list(&resources);
>  	}
> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index 1f80a88..007466e 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>  			pci_add_resource(&resources, &iomem_resource);
>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>  						controller, &resources);
> +			if (bus)
> +				pci_bus_add_devices(bus);
>  			controller->root_bus = bus;
>  			controller->last_busno = bus->busn_res.end;
>  		}
> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
> index e39f9c5..3f8fb2c 100644
> --- a/arch/tile/kernel/pci_gx.c
> +++ b/arch/tile/kernel/pci_gx.c
> @@ -889,6 +889,8 @@ int __init pcibios_init(void)
>  		controller->first_busno = next_busno;
>  		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
>  					controller, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		controller->root_bus = bus;
>  		next_busno = bus->busn_res.end + 1;
>  	}
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 7b20bcc..300d39e 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
>  		pci_free_resource_list(&resources);
>  		kfree(sd);
>  	}
> +	pci_bus_add_devices(bus);
>  }
>  
>  void __init pcibios_set_cache_line_size(void)
> diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
> index 5b34033..f2ae64e 100644
> --- a/arch/xtensa/kernel/pci.c
> +++ b/arch/xtensa/kernel/pci.c
> @@ -185,6 +185,8 @@ static int __init pcibios_init(void)
>  		pci_controller_apertures(pci_ctrl, &resources);
>  		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
>  					pci_ctrl->ops, pci_ctrl, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		pci_ctrl->bus = bus;
>  		pci_ctrl->last_busno = bus->busn_res.end;
>  		if (next_busno <= pci_ctrl->last_busno)
> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> index 9ecabfa..c17afe5 100644
> --- a/drivers/pci/host/pci-xgene.c
> +++ b/drivers/pci/host/pci-xgene.c
> @@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
>  	if (!bus)
>  		return -ENOMEM;
>  
> +	pci_bus_add_devices(bus);
>  	platform_set_drvdata(pdev, port);
>  	return 0;
>  }
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 79775b5..2ab5a40 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>  	if (!found)
>  		pci_bus_update_busn_res_end(b, max);
>  
> -	pci_bus_add_devices(b);
>  	return b;
>  }
>  EXPORT_SYMBOL(pci_scan_root_bus);
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

On 2014/11/19 19:49, Lorenzo Pieralisi wrote:
> On Wed, Nov 19, 2014 at 07:32:49AM +0000, Yijing Wang wrote:
>> Just like pci_scan_bus(), we also should rip out
>> pci_bus_add_devices() from pci_scan_root_bus().
>> Lots platforms first call pci_scan_root_bus(), but
>> after that, they call pci_bus_size_bridges() and
>> pci_bus_assign_resources(). Place pci_bus_add_devices()
>> in pci_scan_root_bus() hurts PCI scan logic.
>> Should no functional change.
> 
> I agree with the patch, I do not agree it provides no functional change.
> You are actually fixing an issue on systems where the root bus
> is scanned at device_initcall time and pci drivers may have already been
> registered, so they end up being probed with possibly unassigned
> resources as you correctly mention in the log.
> 
> You could move the sequence:
> 
> if (!pci_has_flag(PCI_PROBE_ONLY)) {
> 	pci_bus_size_bridges();
> 	pci_bus_assign_resources();
> }
> 
> in pci_scan_root_bus() instead of taking pci_bus_add_devices() out ?

No, many drivers need additional setup before resources setup.

> 
> Would this break any code ? Or add a helper function:
> 
> pci_scan_root_bus(..., bool assign) ?

I don't think it's worth, this make scan function involved something not
scan business.

> 
> arm64 xgene code has already been fixed by reshuffling the scanning code
> sequence.
> 
> I think we should have a look at eg drivers/pci/host/pcie-xilinx.c, I
> think there is a possible initcall ordering issue there (drivers being probed
> with unassigned resources).

If pci device drivers(like network driver) are loaded before pcie host driver,
use pci_scan_root_bus() may cause problem, because pci_bus_add_devices() will
try to attach the device driver, but this time pci devices resources may not
be configured properly.

> 
> Lorenzo
> 
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> CC: linux-alpha@vger.kernel.org
>> CC: linux-ia64@vger.kernel.org
>> CC: linux-mips@linux-mips.org
>> CC: linux-xtensa@linux-xtensa.org
>> CC: linux-arm-kernel@lists.infradead.org
>> CC: linux-sh@vger.kernel.org
>> CC: sparclinux@vger.kernel.org
>> ---
>>  arch/alpha/kernel/pci.c          |    2 ++
>>  arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
>>  arch/ia64/sn/kernel/io_init.c    |    1 +
>>  arch/microblaze/pci/pci-common.c |    1 +
>>  arch/mips/pci/pci.c              |    1 +
>>  arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
>>  arch/s390/pci/pci.c              |    2 +-
>>  arch/sh/drivers/pci/pci.c        |    1 +
>>  arch/sparc/kernel/leon_pci.c     |    1 +
>>  arch/tile/kernel/pci.c           |    2 ++
>>  arch/tile/kernel/pci_gx.c        |    2 ++
>>  arch/x86/pci/common.c            |    1 +
>>  arch/xtensa/kernel/pci.c         |    2 ++
>>  drivers/pci/host/pci-xgene.c     |    1 +
>>  drivers/pci/probe.c              |    1 -
>>  15 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
>> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
>> index efa5d65..2b36044 100644
>> --- a/arch/frv/mb93090-mb00/pci-vdk.c
>> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
>> @@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>>  
>>  int __init pcibios_init(void)
>>  {
>> +	struct pci_bus *bus;
>>  	struct pci_ops *dir = NULL;
>>  	LIST_HEAD(resources);
>>  
>> @@ -383,12 +384,13 @@ int __init pcibios_init(void)
>>  	printk("PCI: Probing PCI hardware\n");
>>  	pci_add_resource(&resources, &pci_ioport_resource);
>>  	pci_add_resource(&resources, &pci_iomem_resource);
>> -	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>> +	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>>  
>>  	pcibios_irq_init();
>>  	pcibios_fixup_irqs();
>>  	pcibios_resource_survey();
>> -
>> +	if (bus)
>> +		pci_bus_add_devices(bus);
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
>> index 0b5ce82..63b43a6 100644
>> --- a/arch/ia64/sn/kernel/io_init.c
>> +++ b/arch/ia64/sn/kernel/io_init.c
>> @@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
>>  		kfree(res);
>>  		kfree(controller);
>>  	}
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  /*
>> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
>> index 9037914..651e25d 100644
>> --- a/arch/microblaze/pci/pci-common.c
>> +++ b/arch/microblaze/pci/pci-common.c
>> @@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
>>  	hose->bus = bus;
>>  
>>  	hose->last_busno = bus->busn_res.end;
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  static int __init pcibios_init(void)
>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
>> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
>> index 6b4339f..860aa35 100644
>> --- a/arch/mn10300/unit-asb2305/pci.c
>> +++ b/arch/mn10300/unit-asb2305/pci.c
>> @@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>>   */
>>  static int __init pcibios_init(void)
>>  {
>> +	struct pci_bus *bus;
>>  	resource_size_t io_offset, mem_offset;
>>  	LIST_HEAD(resources);
>>  
>> @@ -376,11 +377,13 @@ static int __init pcibios_init(void)
>>  
>>  	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
>>  	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
>> -	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>> +	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>>  
>>  	pcibios_irq_init();
>>  	pcibios_fixup_irqs();
>>  	pcibios_resource_survey();
>> +	if (bus)
>> +		pci_bus_add_devices(bus);
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
>> index 552b990..98247f8 100644
>> --- a/arch/s390/pci/pci.c
>> +++ b/arch/s390/pci/pci.c
>> @@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
>>  		zpci_cleanup_bus_resources(zdev);
>>  		return -EIO;
>>  	}
>> -
>> +	pci_bus_add_devices(zdev->bus);
>>  	zdev->bus->max_bus_speed = zdev->max_bus_speed;
>>  	return 0;
>>  }
>> diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
>> index 1bc09ee..efc1051 100644
>> --- a/arch/sh/drivers/pci/pci.c
>> +++ b/arch/sh/drivers/pci/pci.c
>> @@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
>>  
>>  		pci_bus_size_bridges(bus);
>>  		pci_bus_assign_resources(bus);
>> +		pci_bus_add_devices(bus);
>>  	} else {
>>  		pci_free_resource_list(&resources);
>>  	}
>> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
>> index 899b720..2971076 100644
>> --- a/arch/sparc/kernel/leon_pci.c
>> +++ b/arch/sparc/kernel/leon_pci.c
>> @@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
>>  
>>  		/* Assign devices with resources */
>>  		pci_assign_unassigned_resources();
>> +		pci_bus_add_devices(root_bus);
>>  	} else {
>>  		pci_free_resource_list(&resources);
>>  	}
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
>> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
>> index e39f9c5..3f8fb2c 100644
>> --- a/arch/tile/kernel/pci_gx.c
>> +++ b/arch/tile/kernel/pci_gx.c
>> @@ -889,6 +889,8 @@ int __init pcibios_init(void)
>>  		controller->first_busno = next_busno;
>>  		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
>>  					controller, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		controller->root_bus = bus;
>>  		next_busno = bus->busn_res.end + 1;
>>  	}
>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>> index 7b20bcc..300d39e 100644
>> --- a/arch/x86/pci/common.c
>> +++ b/arch/x86/pci/common.c
>> @@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
>>  		pci_free_resource_list(&resources);
>>  		kfree(sd);
>>  	}
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  void __init pcibios_set_cache_line_size(void)
>> diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
>> index 5b34033..f2ae64e 100644
>> --- a/arch/xtensa/kernel/pci.c
>> +++ b/arch/xtensa/kernel/pci.c
>> @@ -185,6 +185,8 @@ static int __init pcibios_init(void)
>>  		pci_controller_apertures(pci_ctrl, &resources);
>>  		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
>>  					pci_ctrl->ops, pci_ctrl, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		pci_ctrl->bus = bus;
>>  		pci_ctrl->last_busno = bus->busn_res.end;
>>  		if (next_busno <= pci_ctrl->last_busno)
>> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
>> index 9ecabfa..c17afe5 100644
>> --- a/drivers/pci/host/pci-xgene.c
>> +++ b/drivers/pci/host/pci-xgene.c
>> @@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
>>  	if (!bus)
>>  		return -ENOMEM;
>>  
>> +	pci_bus_add_devices(bus);
>>  	platform_set_drvdata(pdev, port);
>>  	return 0;
>>  }
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 79775b5..2ab5a40 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>>  	if (!found)
>>  		pci_bus_update_busn_res_end(b, max);
>>  
>> -	pci_bus_add_devices(b);
>>  	return b;
>>  }
>>  EXPORT_SYMBOL(pci_scan_root_bus);
>> -- 
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> .
> 


-- 
Thanks!
Yijing

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

On 2014/11/19 19:49, Lorenzo Pieralisi wrote:
> On Wed, Nov 19, 2014 at 07:32:49AM +0000, Yijing Wang wrote:
>> Just like pci_scan_bus(), we also should rip out
>> pci_bus_add_devices() from pci_scan_root_bus().
>> Lots platforms first call pci_scan_root_bus(), but
>> after that, they call pci_bus_size_bridges() and
>> pci_bus_assign_resources(). Place pci_bus_add_devices()
>> in pci_scan_root_bus() hurts PCI scan logic.
>> Should no functional change.
> 
> I agree with the patch, I do not agree it provides no functional change.
> You are actually fixing an issue on systems where the root bus
> is scanned at device_initcall time and pci drivers may have already been
> registered, so they end up being probed with possibly unassigned
> resources as you correctly mention in the log.
> 
> You could move the sequence:
> 
> if (!pci_has_flag(PCI_PROBE_ONLY)) {
> 	pci_bus_size_bridges();
> 	pci_bus_assign_resources();
> }
> 
> in pci_scan_root_bus() instead of taking pci_bus_add_devices() out ?

No, many drivers need additional setup before resources setup.

> 
> Would this break any code ? Or add a helper function:
> 
> pci_scan_root_bus(..., bool assign) ?

I don't think it's worth, this make scan function involved something not
scan business.

> 
> arm64 xgene code has already been fixed by reshuffling the scanning code
> sequence.
> 
> I think we should have a look at eg drivers/pci/host/pcie-xilinx.c, I
> think there is a possible initcall ordering issue there (drivers being probed
> with unassigned resources).

If pci device drivers(like network driver) are loaded before pcie host driver,
use pci_scan_root_bus() may cause problem, because pci_bus_add_devices() will
try to attach the device driver, but this time pci devices resources may not
be configured properly.

> 
> Lorenzo
> 
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> CC: linux-alpha at vger.kernel.org
>> CC: linux-ia64 at vger.kernel.org
>> CC: linux-mips at linux-mips.org
>> CC: linux-xtensa at linux-xtensa.org
>> CC: linux-arm-kernel at lists.infradead.org
>> CC: linux-sh at vger.kernel.org
>> CC: sparclinux at vger.kernel.org
>> ---
>>  arch/alpha/kernel/pci.c          |    2 ++
>>  arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
>>  arch/ia64/sn/kernel/io_init.c    |    1 +
>>  arch/microblaze/pci/pci-common.c |    1 +
>>  arch/mips/pci/pci.c              |    1 +
>>  arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
>>  arch/s390/pci/pci.c              |    2 +-
>>  arch/sh/drivers/pci/pci.c        |    1 +
>>  arch/sparc/kernel/leon_pci.c     |    1 +
>>  arch/tile/kernel/pci.c           |    2 ++
>>  arch/tile/kernel/pci_gx.c        |    2 ++
>>  arch/x86/pci/common.c            |    1 +
>>  arch/xtensa/kernel/pci.c         |    2 ++
>>  drivers/pci/host/pci-xgene.c     |    1 +
>>  drivers/pci/probe.c              |    1 -
>>  15 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
>> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
>> index efa5d65..2b36044 100644
>> --- a/arch/frv/mb93090-mb00/pci-vdk.c
>> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
>> @@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>>  
>>  int __init pcibios_init(void)
>>  {
>> +	struct pci_bus *bus;
>>  	struct pci_ops *dir = NULL;
>>  	LIST_HEAD(resources);
>>  
>> @@ -383,12 +384,13 @@ int __init pcibios_init(void)
>>  	printk("PCI: Probing PCI hardware\n");
>>  	pci_add_resource(&resources, &pci_ioport_resource);
>>  	pci_add_resource(&resources, &pci_iomem_resource);
>> -	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>> +	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>>  
>>  	pcibios_irq_init();
>>  	pcibios_fixup_irqs();
>>  	pcibios_resource_survey();
>> -
>> +	if (bus)
>> +		pci_bus_add_devices(bus);
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
>> index 0b5ce82..63b43a6 100644
>> --- a/arch/ia64/sn/kernel/io_init.c
>> +++ b/arch/ia64/sn/kernel/io_init.c
>> @@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
>>  		kfree(res);
>>  		kfree(controller);
>>  	}
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  /*
>> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
>> index 9037914..651e25d 100644
>> --- a/arch/microblaze/pci/pci-common.c
>> +++ b/arch/microblaze/pci/pci-common.c
>> @@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
>>  	hose->bus = bus;
>>  
>>  	hose->last_busno = bus->busn_res.end;
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  static int __init pcibios_init(void)
>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
>> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
>> index 6b4339f..860aa35 100644
>> --- a/arch/mn10300/unit-asb2305/pci.c
>> +++ b/arch/mn10300/unit-asb2305/pci.c
>> @@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>>   */
>>  static int __init pcibios_init(void)
>>  {
>> +	struct pci_bus *bus;
>>  	resource_size_t io_offset, mem_offset;
>>  	LIST_HEAD(resources);
>>  
>> @@ -376,11 +377,13 @@ static int __init pcibios_init(void)
>>  
>>  	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
>>  	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
>> -	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>> +	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>>  
>>  	pcibios_irq_init();
>>  	pcibios_fixup_irqs();
>>  	pcibios_resource_survey();
>> +	if (bus)
>> +		pci_bus_add_devices(bus);
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
>> index 552b990..98247f8 100644
>> --- a/arch/s390/pci/pci.c
>> +++ b/arch/s390/pci/pci.c
>> @@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
>>  		zpci_cleanup_bus_resources(zdev);
>>  		return -EIO;
>>  	}
>> -
>> +	pci_bus_add_devices(zdev->bus);
>>  	zdev->bus->max_bus_speed = zdev->max_bus_speed;
>>  	return 0;
>>  }
>> diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
>> index 1bc09ee..efc1051 100644
>> --- a/arch/sh/drivers/pci/pci.c
>> +++ b/arch/sh/drivers/pci/pci.c
>> @@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
>>  
>>  		pci_bus_size_bridges(bus);
>>  		pci_bus_assign_resources(bus);
>> +		pci_bus_add_devices(bus);
>>  	} else {
>>  		pci_free_resource_list(&resources);
>>  	}
>> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
>> index 899b720..2971076 100644
>> --- a/arch/sparc/kernel/leon_pci.c
>> +++ b/arch/sparc/kernel/leon_pci.c
>> @@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
>>  
>>  		/* Assign devices with resources */
>>  		pci_assign_unassigned_resources();
>> +		pci_bus_add_devices(root_bus);
>>  	} else {
>>  		pci_free_resource_list(&resources);
>>  	}
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
>> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
>> index e39f9c5..3f8fb2c 100644
>> --- a/arch/tile/kernel/pci_gx.c
>> +++ b/arch/tile/kernel/pci_gx.c
>> @@ -889,6 +889,8 @@ int __init pcibios_init(void)
>>  		controller->first_busno = next_busno;
>>  		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
>>  					controller, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		controller->root_bus = bus;
>>  		next_busno = bus->busn_res.end + 1;
>>  	}
>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>> index 7b20bcc..300d39e 100644
>> --- a/arch/x86/pci/common.c
>> +++ b/arch/x86/pci/common.c
>> @@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
>>  		pci_free_resource_list(&resources);
>>  		kfree(sd);
>>  	}
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  void __init pcibios_set_cache_line_size(void)
>> diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
>> index 5b34033..f2ae64e 100644
>> --- a/arch/xtensa/kernel/pci.c
>> +++ b/arch/xtensa/kernel/pci.c
>> @@ -185,6 +185,8 @@ static int __init pcibios_init(void)
>>  		pci_controller_apertures(pci_ctrl, &resources);
>>  		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
>>  					pci_ctrl->ops, pci_ctrl, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		pci_ctrl->bus = bus;
>>  		pci_ctrl->last_busno = bus->busn_res.end;
>>  		if (next_busno <= pci_ctrl->last_busno)
>> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
>> index 9ecabfa..c17afe5 100644
>> --- a/drivers/pci/host/pci-xgene.c
>> +++ b/drivers/pci/host/pci-xgene.c
>> @@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
>>  	if (!bus)
>>  		return -ENOMEM;
>>  
>> +	pci_bus_add_devices(bus);
>>  	platform_set_drvdata(pdev, port);
>>  	return 0;
>>  }
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 79775b5..2ab5a40 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>>  	if (!found)
>>  		pci_bus_update_busn_res_end(b, max);
>>  
>> -	pci_bus_add_devices(b);
>>  	return b;
>>  }
>>  EXPORT_SYMBOL(pci_scan_root_bus);
>> -- 
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

On 2014/11/19 19:49, Lorenzo Pieralisi wrote:
> On Wed, Nov 19, 2014 at 07:32:49AM +0000, Yijing Wang wrote:
>> Just like pci_scan_bus(), we also should rip out
>> pci_bus_add_devices() from pci_scan_root_bus().
>> Lots platforms first call pci_scan_root_bus(), but
>> after that, they call pci_bus_size_bridges() and
>> pci_bus_assign_resources(). Place pci_bus_add_devices()
>> in pci_scan_root_bus() hurts PCI scan logic.
>> Should no functional change.
> 
> I agree with the patch, I do not agree it provides no functional change.
> You are actually fixing an issue on systems where the root bus
> is scanned at device_initcall time and pci drivers may have already been
> registered, so they end up being probed with possibly unassigned
> resources as you correctly mention in the log.
> 
> You could move the sequence:
> 
> if (!pci_has_flag(PCI_PROBE_ONLY)) {
> 	pci_bus_size_bridges();
> 	pci_bus_assign_resources();
> }
> 
> in pci_scan_root_bus() instead of taking pci_bus_add_devices() out ?

No, many drivers need additional setup before resources setup.

> 
> Would this break any code ? Or add a helper function:
> 
> pci_scan_root_bus(..., bool assign) ?

I don't think it's worth, this make scan function involved something not
scan business.

> 
> arm64 xgene code has already been fixed by reshuffling the scanning code
> sequence.
> 
> I think we should have a look at eg drivers/pci/host/pcie-xilinx.c, I
> think there is a possible initcall ordering issue there (drivers being probed
> with unassigned resources).

If pci device drivers(like network driver) are loaded before pcie host driver,
use pci_scan_root_bus() may cause problem, because pci_bus_add_devices() will
try to attach the device driver, but this time pci devices resources may not
be configured properly.

> 
> Lorenzo
> 
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> CC: linux-alpha@vger.kernel.org
>> CC: linux-ia64@vger.kernel.org
>> CC: linux-mips@linux-mips.org
>> CC: linux-xtensa@linux-xtensa.org
>> CC: linux-arm-kernel@lists.infradead.org
>> CC: linux-sh@vger.kernel.org
>> CC: sparclinux@vger.kernel.org
>> ---
>>  arch/alpha/kernel/pci.c          |    2 ++
>>  arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
>>  arch/ia64/sn/kernel/io_init.c    |    1 +
>>  arch/microblaze/pci/pci-common.c |    1 +
>>  arch/mips/pci/pci.c              |    1 +
>>  arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
>>  arch/s390/pci/pci.c              |    2 +-
>>  arch/sh/drivers/pci/pci.c        |    1 +
>>  arch/sparc/kernel/leon_pci.c     |    1 +
>>  arch/tile/kernel/pci.c           |    2 ++
>>  arch/tile/kernel/pci_gx.c        |    2 ++
>>  arch/x86/pci/common.c            |    1 +
>>  arch/xtensa/kernel/pci.c         |    2 ++
>>  drivers/pci/host/pci-xgene.c     |    1 +
>>  drivers/pci/probe.c              |    1 -
>>  15 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
>> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
>> index efa5d65..2b36044 100644
>> --- a/arch/frv/mb93090-mb00/pci-vdk.c
>> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
>> @@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>>  
>>  int __init pcibios_init(void)
>>  {
>> +	struct pci_bus *bus;
>>  	struct pci_ops *dir = NULL;
>>  	LIST_HEAD(resources);
>>  
>> @@ -383,12 +384,13 @@ int __init pcibios_init(void)
>>  	printk("PCI: Probing PCI hardware\n");
>>  	pci_add_resource(&resources, &pci_ioport_resource);
>>  	pci_add_resource(&resources, &pci_iomem_resource);
>> -	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>> +	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>>  
>>  	pcibios_irq_init();
>>  	pcibios_fixup_irqs();
>>  	pcibios_resource_survey();
>> -
>> +	if (bus)
>> +		pci_bus_add_devices(bus);
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
>> index 0b5ce82..63b43a6 100644
>> --- a/arch/ia64/sn/kernel/io_init.c
>> +++ b/arch/ia64/sn/kernel/io_init.c
>> @@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
>>  		kfree(res);
>>  		kfree(controller);
>>  	}
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  /*
>> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
>> index 9037914..651e25d 100644
>> --- a/arch/microblaze/pci/pci-common.c
>> +++ b/arch/microblaze/pci/pci-common.c
>> @@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
>>  	hose->bus = bus;
>>  
>>  	hose->last_busno = bus->busn_res.end;
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  static int __init pcibios_init(void)
>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
>> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
>> index 6b4339f..860aa35 100644
>> --- a/arch/mn10300/unit-asb2305/pci.c
>> +++ b/arch/mn10300/unit-asb2305/pci.c
>> @@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>>   */
>>  static int __init pcibios_init(void)
>>  {
>> +	struct pci_bus *bus;
>>  	resource_size_t io_offset, mem_offset;
>>  	LIST_HEAD(resources);
>>  
>> @@ -376,11 +377,13 @@ static int __init pcibios_init(void)
>>  
>>  	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
>>  	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
>> -	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>> +	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>>  
>>  	pcibios_irq_init();
>>  	pcibios_fixup_irqs();
>>  	pcibios_resource_survey();
>> +	if (bus)
>> +		pci_bus_add_devices(bus);
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
>> index 552b990..98247f8 100644
>> --- a/arch/s390/pci/pci.c
>> +++ b/arch/s390/pci/pci.c
>> @@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
>>  		zpci_cleanup_bus_resources(zdev);
>>  		return -EIO;
>>  	}
>> -
>> +	pci_bus_add_devices(zdev->bus);
>>  	zdev->bus->max_bus_speed = zdev->max_bus_speed;
>>  	return 0;
>>  }
>> diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
>> index 1bc09ee..efc1051 100644
>> --- a/arch/sh/drivers/pci/pci.c
>> +++ b/arch/sh/drivers/pci/pci.c
>> @@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
>>  
>>  		pci_bus_size_bridges(bus);
>>  		pci_bus_assign_resources(bus);
>> +		pci_bus_add_devices(bus);
>>  	} else {
>>  		pci_free_resource_list(&resources);
>>  	}
>> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
>> index 899b720..2971076 100644
>> --- a/arch/sparc/kernel/leon_pci.c
>> +++ b/arch/sparc/kernel/leon_pci.c
>> @@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
>>  
>>  		/* Assign devices with resources */
>>  		pci_assign_unassigned_resources();
>> +		pci_bus_add_devices(root_bus);
>>  	} else {
>>  		pci_free_resource_list(&resources);
>>  	}
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
>> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
>> index e39f9c5..3f8fb2c 100644
>> --- a/arch/tile/kernel/pci_gx.c
>> +++ b/arch/tile/kernel/pci_gx.c
>> @@ -889,6 +889,8 @@ int __init pcibios_init(void)
>>  		controller->first_busno = next_busno;
>>  		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
>>  					controller, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		controller->root_bus = bus;
>>  		next_busno = bus->busn_res.end + 1;
>>  	}
>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>> index 7b20bcc..300d39e 100644
>> --- a/arch/x86/pci/common.c
>> +++ b/arch/x86/pci/common.c
>> @@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
>>  		pci_free_resource_list(&resources);
>>  		kfree(sd);
>>  	}
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  void __init pcibios_set_cache_line_size(void)
>> diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
>> index 5b34033..f2ae64e 100644
>> --- a/arch/xtensa/kernel/pci.c
>> +++ b/arch/xtensa/kernel/pci.c
>> @@ -185,6 +185,8 @@ static int __init pcibios_init(void)
>>  		pci_controller_apertures(pci_ctrl, &resources);
>>  		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
>>  					pci_ctrl->ops, pci_ctrl, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		pci_ctrl->bus = bus;
>>  		pci_ctrl->last_busno = bus->busn_res.end;
>>  		if (next_busno <= pci_ctrl->last_busno)
>> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
>> index 9ecabfa..c17afe5 100644
>> --- a/drivers/pci/host/pci-xgene.c
>> +++ b/drivers/pci/host/pci-xgene.c
>> @@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
>>  	if (!bus)
>>  		return -ENOMEM;
>>  
>> +	pci_bus_add_devices(bus);
>>  	platform_set_drvdata(pdev, port);
>>  	return 0;
>>  }
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 79775b5..2ab5a40 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>>  	if (!found)
>>  		pci_bus_update_busn_res_end(b, max);
>>  
>> -	pci_bus_add_devices(b);
>>  	return b;
>>  }
>>  EXPORT_SYMBOL(pci_scan_root_bus);
>> -- 
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

On 2014/11/19 19:49, Lorenzo Pieralisi wrote:
> On Wed, Nov 19, 2014 at 07:32:49AM +0000, Yijing Wang wrote:
>> Just like pci_scan_bus(), we also should rip out
>> pci_bus_add_devices() from pci_scan_root_bus().
>> Lots platforms first call pci_scan_root_bus(), but
>> after that, they call pci_bus_size_bridges() and
>> pci_bus_assign_resources(). Place pci_bus_add_devices()
>> in pci_scan_root_bus() hurts PCI scan logic.
>> Should no functional change.
> 
> I agree with the patch, I do not agree it provides no functional change.
> You are actually fixing an issue on systems where the root bus
> is scanned at device_initcall time and pci drivers may have already been
> registered, so they end up being probed with possibly unassigned
> resources as you correctly mention in the log.
> 
> You could move the sequence:
> 
> if (!pci_has_flag(PCI_PROBE_ONLY)) {
> 	pci_bus_size_bridges();
> 	pci_bus_assign_resources();
> }
> 
> in pci_scan_root_bus() instead of taking pci_bus_add_devices() out ?

No, many drivers need additional setup before resources setup.

> 
> Would this break any code ? Or add a helper function:
> 
> pci_scan_root_bus(..., bool assign) ?

I don't think it's worth, this make scan function involved something not
scan business.

> 
> arm64 xgene code has already been fixed by reshuffling the scanning code
> sequence.
> 
> I think we should have a look at eg drivers/pci/host/pcie-xilinx.c, I
> think there is a possible initcall ordering issue there (drivers being probed
> with unassigned resources).

If pci device drivers(like network driver) are loaded before pcie host driver,
use pci_scan_root_bus() may cause problem, because pci_bus_add_devices() will
try to attach the device driver, but this time pci devices resources may not
be configured properly.

> 
> Lorenzo
> 
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> CC: linux-alpha@vger.kernel.org
>> CC: linux-ia64@vger.kernel.org
>> CC: linux-mips@linux-mips.org
>> CC: linux-xtensa@linux-xtensa.org
>> CC: linux-arm-kernel@lists.infradead.org
>> CC: linux-sh@vger.kernel.org
>> CC: sparclinux@vger.kernel.org
>> ---
>>  arch/alpha/kernel/pci.c          |    2 ++
>>  arch/frv/mb93090-mb00/pci-vdk.c  |    6 ++++--
>>  arch/ia64/sn/kernel/io_init.c    |    1 +
>>  arch/microblaze/pci/pci-common.c |    1 +
>>  arch/mips/pci/pci.c              |    1 +
>>  arch/mn10300/unit-asb2305/pci.c  |    5 ++++-
>>  arch/s390/pci/pci.c              |    2 +-
>>  arch/sh/drivers/pci/pci.c        |    1 +
>>  arch/sparc/kernel/leon_pci.c     |    1 +
>>  arch/tile/kernel/pci.c           |    2 ++
>>  arch/tile/kernel/pci_gx.c        |    2 ++
>>  arch/x86/pci/common.c            |    1 +
>>  arch/xtensa/kernel/pci.c         |    2 ++
>>  drivers/pci/host/pci-xgene.c     |    1 +
>>  drivers/pci/probe.c              |    1 -
>>  15 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
>> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
>> index efa5d65..2b36044 100644
>> --- a/arch/frv/mb93090-mb00/pci-vdk.c
>> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
>> @@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>>  
>>  int __init pcibios_init(void)
>>  {
>> +	struct pci_bus *bus;
>>  	struct pci_ops *dir = NULL;
>>  	LIST_HEAD(resources);
>>  
>> @@ -383,12 +384,13 @@ int __init pcibios_init(void)
>>  	printk("PCI: Probing PCI hardware\n");
>>  	pci_add_resource(&resources, &pci_ioport_resource);
>>  	pci_add_resource(&resources, &pci_iomem_resource);
>> -	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>> +	bus = pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
>>  
>>  	pcibios_irq_init();
>>  	pcibios_fixup_irqs();
>>  	pcibios_resource_survey();
>> -
>> +	if (bus)
>> +		pci_bus_add_devices(bus);
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
>> index 0b5ce82..63b43a6 100644
>> --- a/arch/ia64/sn/kernel/io_init.c
>> +++ b/arch/ia64/sn/kernel/io_init.c
>> @@ -272,6 +272,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
>>  		kfree(res);
>>  		kfree(controller);
>>  	}
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  /*
>> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
>> index 9037914..651e25d 100644
>> --- a/arch/microblaze/pci/pci-common.c
>> +++ b/arch/microblaze/pci/pci-common.c
>> @@ -1346,6 +1346,7 @@ static void pcibios_scan_phb(struct pci_controller *hose)
>>  	hose->bus = bus;
>>  
>>  	hose->last_busno = bus->busn_res.end;
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  static int __init pcibios_init(void)
>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
>> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
>> index 6b4339f..860aa35 100644
>> --- a/arch/mn10300/unit-asb2305/pci.c
>> +++ b/arch/mn10300/unit-asb2305/pci.c
>> @@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>>   */
>>  static int __init pcibios_init(void)
>>  {
>> +	struct pci_bus *bus;
>>  	resource_size_t io_offset, mem_offset;
>>  	LIST_HEAD(resources);
>>  
>> @@ -376,11 +377,13 @@ static int __init pcibios_init(void)
>>  
>>  	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
>>  	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
>> -	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>> +	bus = pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
>>  
>>  	pcibios_irq_init();
>>  	pcibios_fixup_irqs();
>>  	pcibios_resource_survey();
>> +	if (bus)
>> +		pci_bus_add_devices(bus);
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
>> index 552b990..98247f8 100644
>> --- a/arch/s390/pci/pci.c
>> +++ b/arch/s390/pci/pci.c
>> @@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
>>  		zpci_cleanup_bus_resources(zdev);
>>  		return -EIO;
>>  	}
>> -
>> +	pci_bus_add_devices(zdev->bus);
>>  	zdev->bus->max_bus_speed = zdev->max_bus_speed;
>>  	return 0;
>>  }
>> diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
>> index 1bc09ee..efc1051 100644
>> --- a/arch/sh/drivers/pci/pci.c
>> +++ b/arch/sh/drivers/pci/pci.c
>> @@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
>>  
>>  		pci_bus_size_bridges(bus);
>>  		pci_bus_assign_resources(bus);
>> +		pci_bus_add_devices(bus);
>>  	} else {
>>  		pci_free_resource_list(&resources);
>>  	}
>> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
>> index 899b720..2971076 100644
>> --- a/arch/sparc/kernel/leon_pci.c
>> +++ b/arch/sparc/kernel/leon_pci.c
>> @@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
>>  
>>  		/* Assign devices with resources */
>>  		pci_assign_unassigned_resources();
>> +		pci_bus_add_devices(root_bus);
>>  	} else {
>>  		pci_free_resource_list(&resources);
>>  	}
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
>> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
>> index e39f9c5..3f8fb2c 100644
>> --- a/arch/tile/kernel/pci_gx.c
>> +++ b/arch/tile/kernel/pci_gx.c
>> @@ -889,6 +889,8 @@ int __init pcibios_init(void)
>>  		controller->first_busno = next_busno;
>>  		bus = pci_scan_root_bus(NULL, next_busno, controller->ops,
>>  					controller, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		controller->root_bus = bus;
>>  		next_busno = bus->busn_res.end + 1;
>>  	}
>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>> index 7b20bcc..300d39e 100644
>> --- a/arch/x86/pci/common.c
>> +++ b/arch/x86/pci/common.c
>> @@ -475,6 +475,7 @@ void pcibios_scan_root(int busnum)
>>  		pci_free_resource_list(&resources);
>>  		kfree(sd);
>>  	}
>> +	pci_bus_add_devices(bus);
>>  }
>>  
>>  void __init pcibios_set_cache_line_size(void)
>> diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
>> index 5b34033..f2ae64e 100644
>> --- a/arch/xtensa/kernel/pci.c
>> +++ b/arch/xtensa/kernel/pci.c
>> @@ -185,6 +185,8 @@ static int __init pcibios_init(void)
>>  		pci_controller_apertures(pci_ctrl, &resources);
>>  		bus = pci_scan_root_bus(NULL, pci_ctrl->first_busno,
>>  					pci_ctrl->ops, pci_ctrl, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		pci_ctrl->bus = bus;
>>  		pci_ctrl->last_busno = bus->busn_res.end;
>>  		if (next_busno <= pci_ctrl->last_busno)
>> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
>> index 9ecabfa..c17afe5 100644
>> --- a/drivers/pci/host/pci-xgene.c
>> +++ b/drivers/pci/host/pci-xgene.c
>> @@ -635,6 +635,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
>>  	if (!bus)
>>  		return -ENOMEM;
>>  
>> +	pci_bus_add_devices(bus);
>>  	platform_set_drvdata(pdev, port);
>>  	return 0;
>>  }
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 79775b5..2ab5a40 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2084,7 +2084,6 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>>  	if (!found)
>>  		pci_bus_update_busn_res_end(b, max);
>>  
>> -	pci_bus_add_devices(b);
>>  	return b;
>>  }
>>  EXPORT_SYMBOL(pci_scan_root_bus);
>> -- 
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Arnd Bergmann]

On Wednesday 19 November 2014 15:32:49 Yijing Wang wrote:
> Just like pci_scan_bus(), we also should rip out
> pci_bus_add_devices() from pci_scan_root_bus().
> Lots platforms first call pci_scan_root_bus(), but
> after that, they call pci_bus_size_bridges() and
> pci_bus_assign_resources(). Place pci_bus_add_devices()
> in pci_scan_root_bus() hurts PCI scan logic.

I think we really need to wait for Bjorn to comment on this patch,
as I mentioned the idea behind pci_scan_root_bus() was to integrate
the scanning part, which you now undo, though I can still see this
working out in the long run.

> Should no functional change.

But you are moving the pci_bus_add_devices() later in a couple of
places. While this seems entirely reasonable, I would consider
it a functional change.

> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index 076c35c..97f9730 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -334,6 +334,8 @@ common_init_pci(void)
>  
>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>  					hose, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		hose->bus = bus;
>  		hose->need_domain_info = need_domain_info;
>  		next_busno = bus->busn_res.end + 1;

How about making pci_bus_add_devices() handle a NULL argument to save
the if() here and elsewhere?
> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> index 1bf60b1..f083688 100644
> --- a/arch/mips/pci/pci.c
> +++ b/arch/mips/pci/pci.c
> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>  			pci_bus_size_bridges(bus);
>  			pci_bus_assign_resources(bus);
> +			pci_bus_add_devices(bus);
>  		}
>  	}
>  }

This one looks wrong, I think you still want to call pci_bus_add_devices()
even with PCI_PROBE_ONLY set.

> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index 1f80a88..007466e 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>  			pci_add_resource(&resources, &iomem_resource);
>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>  						controller, &resources);
> +			if (bus)
> +				pci_bus_add_devices(bus);
>  			controller->root_bus = bus;
>  			controller->last_busno = bus->busn_res.end;
>  		}

Should the pci_bus_add_devices come after setting the bus numbers here?

	Arnd

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Arnd Bergmann]

On Wednesday 19 November 2014 15:32:49 Yijing Wang wrote:
> Just like pci_scan_bus(), we also should rip out
> pci_bus_add_devices() from pci_scan_root_bus().
> Lots platforms first call pci_scan_root_bus(), but
> after that, they call pci_bus_size_bridges() and
> pci_bus_assign_resources(). Place pci_bus_add_devices()
> in pci_scan_root_bus() hurts PCI scan logic.

I think we really need to wait for Bjorn to comment on this patch,
as I mentioned the idea behind pci_scan_root_bus() was to integrate
the scanning part, which you now undo, though I can still see this
working out in the long run.

> Should no functional change.

But you are moving the pci_bus_add_devices() later in a couple of
places. While this seems entirely reasonable, I would consider
it a functional change.

> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index 076c35c..97f9730 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -334,6 +334,8 @@ common_init_pci(void)
>  
>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>  					hose, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		hose->bus = bus;
>  		hose->need_domain_info = need_domain_info;
>  		next_busno = bus->busn_res.end + 1;

How about making pci_bus_add_devices() handle a NULL argument to save
the if() here and elsewhere?
> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> index 1bf60b1..f083688 100644
> --- a/arch/mips/pci/pci.c
> +++ b/arch/mips/pci/pci.c
> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>  			pci_bus_size_bridges(bus);
>  			pci_bus_assign_resources(bus);
> +			pci_bus_add_devices(bus);
>  		}
>  	}
>  }

This one looks wrong, I think you still want to call pci_bus_add_devices()
even with PCI_PROBE_ONLY set.

> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index 1f80a88..007466e 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>  			pci_add_resource(&resources, &iomem_resource);
>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>  						controller, &resources);
> +			if (bus)
> +				pci_bus_add_devices(bus);
>  			controller->root_bus = bus;
>  			controller->last_busno = bus->busn_res.end;
>  		}

Should the pci_bus_add_devices come after setting the bus numbers here?

	Arnd

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Arnd Bergmann]

On Wednesday 19 November 2014 15:32:49 Yijing Wang wrote:
> Just like pci_scan_bus(), we also should rip out
> pci_bus_add_devices() from pci_scan_root_bus().
> Lots platforms first call pci_scan_root_bus(), but
> after that, they call pci_bus_size_bridges() and
> pci_bus_assign_resources(). Place pci_bus_add_devices()
> in pci_scan_root_bus() hurts PCI scan logic.

I think we really need to wait for Bjorn to comment on this patch,
as I mentioned the idea behind pci_scan_root_bus() was to integrate
the scanning part, which you now undo, though I can still see this
working out in the long run.

> Should no functional change.

But you are moving the pci_bus_add_devices() later in a couple of
places. While this seems entirely reasonable, I would consider
it a functional change.

> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index 076c35c..97f9730 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -334,6 +334,8 @@ common_init_pci(void)
>  
>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>  					hose, &resources);
> +		if (bus)
> +			pci_bus_add_devices(bus);
>  		hose->bus = bus;
>  		hose->need_domain_info = need_domain_info;
>  		next_busno = bus->busn_res.end + 1;

How about making pci_bus_add_devices() handle a NULL argument to save
the if() here and elsewhere?
> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> index 1bf60b1..f083688 100644
> --- a/arch/mips/pci/pci.c
> +++ b/arch/mips/pci/pci.c
> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>  			pci_bus_size_bridges(bus);
>  			pci_bus_assign_resources(bus);
> +			pci_bus_add_devices(bus);
>  		}
>  	}
>  }

This one looks wrong, I think you still want to call pci_bus_add_devices()
even with PCI_PROBE_ONLY set.

> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index 1f80a88..007466e 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>  			pci_add_resource(&resources, &iomem_resource);
>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>  						controller, &resources);
> +			if (bus)
> +				pci_bus_add_devices(bus);
>  			controller->root_bus = bus;
>  			controller->last_busno = bus->busn_res.end;
>  		}

Should the pci_bus_add_devices come after setting the bus numbers here?

	Arnd

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Liviu Dudau]

On Wed, Nov 19, 2014 at 01:11:41PM +0100, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 15:32:49 Yijing Wang wrote:
> > Just like pci_scan_bus(), we also should rip out
> > pci_bus_add_devices() from pci_scan_root_bus().
> > Lots platforms first call pci_scan_root_bus(), but
> > after that, they call pci_bus_size_bridges() and
> > pci_bus_assign_resources(). Place pci_bus_add_devices()
> > in pci_scan_root_bus() hurts PCI scan logic.
> 
> I think we really need to wait for Bjorn to comment on this patch,
> as I mentioned the idea behind pci_scan_root_bus() was to integrate
> the scanning part, which you now undo, though I can still see this
> working out in the long run.
> 
> > Should no functional change.
> 
> But you are moving the pci_bus_add_devices() later in a couple of
> places. While this seems entirely reasonable, I would consider
> it a functional change.
> 
> > diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> > index 076c35c..97f9730 100644
> > --- a/arch/alpha/kernel/pci.c
> > +++ b/arch/alpha/kernel/pci.c
> > @@ -334,6 +334,8 @@ common_init_pci(void)
> >  
> >  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
> >  					hose, &resources);
> > +		if (bus)
> > +			pci_bus_add_devices(bus);
> >  		hose->bus = bus;
> >  		hose->need_domain_info = need_domain_info;
> >  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

Actually, I would stop there if the bus is NULL and start cleaning up. If not,
things are going to blow up three lines down when next_busno is calculated.

Best regards,
Liviu


> > diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> > index 1bf60b1..f083688 100644
> > --- a/arch/mips/pci/pci.c
> > +++ b/arch/mips/pci/pci.c
> > @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
> >  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
> >  			pci_bus_size_bridges(bus);
> >  			pci_bus_assign_resources(bus);
> > +			pci_bus_add_devices(bus);
> >  		}
> >  	}
> >  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.
> 
> > diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> > index 1f80a88..007466e 100644
> > --- a/arch/tile/kernel/pci.c
> > +++ b/arch/tile/kernel/pci.c
> > @@ -308,6 +308,8 @@ int __init pcibios_init(void)
> >  			pci_add_resource(&resources, &iomem_resource);
> >  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
> >  						controller, &resources);
> > +			if (bus)
> > +				pci_bus_add_devices(bus);
> >  			controller->root_bus = bus;
> >  			controller->last_busno = bus->busn_res.end;
> >  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?
> 
> 	Arnd
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Liviu Dudau]

On Wed, Nov 19, 2014 at 01:11:41PM +0100, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 15:32:49 Yijing Wang wrote:
> > Just like pci_scan_bus(), we also should rip out
> > pci_bus_add_devices() from pci_scan_root_bus().
> > Lots platforms first call pci_scan_root_bus(), but
> > after that, they call pci_bus_size_bridges() and
> > pci_bus_assign_resources(). Place pci_bus_add_devices()
> > in pci_scan_root_bus() hurts PCI scan logic.
> 
> I think we really need to wait for Bjorn to comment on this patch,
> as I mentioned the idea behind pci_scan_root_bus() was to integrate
> the scanning part, which you now undo, though I can still see this
> working out in the long run.
> 
> > Should no functional change.
> 
> But you are moving the pci_bus_add_devices() later in a couple of
> places. While this seems entirely reasonable, I would consider
> it a functional change.
> 
> > diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> > index 076c35c..97f9730 100644
> > --- a/arch/alpha/kernel/pci.c
> > +++ b/arch/alpha/kernel/pci.c
> > @@ -334,6 +334,8 @@ common_init_pci(void)
> >  
> >  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
> >  					hose, &resources);
> > +		if (bus)
> > +			pci_bus_add_devices(bus);
> >  		hose->bus = bus;
> >  		hose->need_domain_info = need_domain_info;
> >  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

Actually, I would stop there if the bus is NULL and start cleaning up. If not,
things are going to blow up three lines down when next_busno is calculated.

Best regards,
Liviu


> > diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> > index 1bf60b1..f083688 100644
> > --- a/arch/mips/pci/pci.c
> > +++ b/arch/mips/pci/pci.c
> > @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
> >  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
> >  			pci_bus_size_bridges(bus);
> >  			pci_bus_assign_resources(bus);
> > +			pci_bus_add_devices(bus);
> >  		}
> >  	}
> >  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.
> 
> > diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> > index 1f80a88..007466e 100644
> > --- a/arch/tile/kernel/pci.c
> > +++ b/arch/tile/kernel/pci.c
> > @@ -308,6 +308,8 @@ int __init pcibios_init(void)
> >  			pci_add_resource(&resources, &iomem_resource);
> >  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
> >  						controller, &resources);
> > +			if (bus)
> > +				pci_bus_add_devices(bus);
> >  			controller->root_bus = bus;
> >  			controller->last_busno = bus->busn_res.end;
> >  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?
> 
> 	Arnd
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Liviu Dudau]

On Wed, Nov 19, 2014 at 01:11:41PM +0100, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 15:32:49 Yijing Wang wrote:
> > Just like pci_scan_bus(), we also should rip out
> > pci_bus_add_devices() from pci_scan_root_bus().
> > Lots platforms first call pci_scan_root_bus(), but
> > after that, they call pci_bus_size_bridges() and
> > pci_bus_assign_resources(). Place pci_bus_add_devices()
> > in pci_scan_root_bus() hurts PCI scan logic.
> 
> I think we really need to wait for Bjorn to comment on this patch,
> as I mentioned the idea behind pci_scan_root_bus() was to integrate
> the scanning part, which you now undo, though I can still see this
> working out in the long run.
> 
> > Should no functional change.
> 
> But you are moving the pci_bus_add_devices() later in a couple of
> places. While this seems entirely reasonable, I would consider
> it a functional change.
> 
> > diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> > index 076c35c..97f9730 100644
> > --- a/arch/alpha/kernel/pci.c
> > +++ b/arch/alpha/kernel/pci.c
> > @@ -334,6 +334,8 @@ common_init_pci(void)
> >  
> >  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
> >  					hose, &resources);
> > +		if (bus)
> > +			pci_bus_add_devices(bus);
> >  		hose->bus = bus;
> >  		hose->need_domain_info = need_domain_info;
> >  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

Actually, I would stop there if the bus is NULL and start cleaning up. If not,
things are going to blow up three lines down when next_busno is calculated.

Best regards,
Liviu


> > diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> > index 1bf60b1..f083688 100644
> > --- a/arch/mips/pci/pci.c
> > +++ b/arch/mips/pci/pci.c
> > @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
> >  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
> >  			pci_bus_size_bridges(bus);
> >  			pci_bus_assign_resources(bus);
> > +			pci_bus_add_devices(bus);
> >  		}
> >  	}
> >  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.
> 
> > diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> > index 1f80a88..007466e 100644
> > --- a/arch/tile/kernel/pci.c
> > +++ b/arch/tile/kernel/pci.c
> > @@ -308,6 +308,8 @@ int __init pcibios_init(void)
> >  			pci_add_resource(&resources, &iomem_resource);
> >  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
> >  						controller, &resources);
> > +			if (bus)
> > +				pci_bus_add_devices(bus);
> >  			controller->root_bus = bus;
> >  			controller->last_busno = bus->busn_res.end;
> >  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?
> 
> 	Arnd
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

This make sense, will update it, thanks!

>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.

Yes, this is my mistake, :(

> 
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?

I think it's doesn't matter, but move it backward is ok to me.

> 
> 	Arnd
> 
> .
> 


-- 
Thanks!
Yijing

[PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

This make sense, will update it, thanks!

>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.

Yes, this is my mistake, :(

> 
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?

I think it's doesn't matter, but move it backward is ok to me.

> 
> 	Arnd
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

This make sense, will update it, thanks!

>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.

Yes, this is my mistake, :(

> 
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?

I think it's doesn't matter, but move it backward is ok to me.

> 
> 	Arnd
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()

[Yijing Wang]

>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

This make sense, will update it, thanks!

>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.

Yes, this is my mistake, :(

> 
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?

I think it's doesn't matter, but move it backward is ok to me.

> 
> 	Arnd
> 
> .
> 


-- 
Thanks!
Yijing

[PATCH 1/5] PCI: Rip out pci_bus_add_devices() from pci_scan_bus()

[Yijing Wang]

Pci_bus_add_devices() should not be placed in
pci_scan_bus(). Now pci devices will be added
to driver core once its creation. All the things
left in pci_bus_add_devices() are driver attach
and other trivial things. pci_scan_bus() should
be the function responsible for scanning PCI devices,
not including device driver attached. Other, lots
callers of pci_scan_bus() call pci_bus_size_bridges()
and pci_bus_assign_resources() after pci_scan_bus().
It's not correct, but because pci_scan_bus() always
be called before driver loaded, so maybe no problem
will cause. We should rip out pci_bus_add_devices()
for better code design.
No functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Guan Xuetao <gxt@mprc.pku.edu.cn>
CC: linux-alpha@vger.kernel.org
CC: linux-m68k@lists.linux-m68k.org
CC: sparclinux@vger.kernel.org
---
 arch/alpha/kernel/sys_nautilus.c  |    1 +
 arch/m68k/coldfire/pci.c          |    1 +
 arch/sparc/kernel/pcic.c          |    2 ++
 arch/unicore32/kernel/pci.c       |   11 +++--------
 drivers/pci/hotplug/ibmphp_core.c |    6 ++++--
 drivers/pci/probe.c               |    1 -
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nautilus.c
index 837c0fa..4ae4a40 100644
--- a/arch/alpha/kernel/sys_nautilus.c
+++ b/arch/alpha/kernel/sys_nautilus.c
@@ -253,6 +253,7 @@ nautilus_init_pci(void)
 	   for the root bus, so just clear it. */
 	bus->self = NULL;
 	pci_fixup_irqs(alpha_mv.pci_swizzle, alpha_mv.pci_map_irq);
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
index df96792..d45f087 100644
--- a/arch/m68k/coldfire/pci.c
+++ b/arch/m68k/coldfire/pci.c
@@ -319,6 +319,7 @@ static int __init mcf_pci_init(void)
 	pci_fixup_irqs(pci_common_swizzle, mcf_pci_map_irq);
 	pci_bus_size_bridges(rootbus);
 	pci_bus_assign_resources(rootbus);
+	pci_bus_add_devices(rootbus);
 	return 0;
 }
 
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 6cc78c2..7a82fe2 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -391,6 +391,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pcic *pcic)
 	struct linux_pbm_info *pbm = &pcic->pbm;
 
 	pbm->pci_bus = pci_scan_bus(pbm->pci_first_busno, &pcic_ops, pbm);
+	if (pbm->pci_bus)
+		pci_bus_add_devices(pbm->pci_bus);
 #if 0 /* deadwood transplanted from sparc64 */
 	pci_fill_in_pbm_cookies(pbm->pci_bus, pbm, pbm->prom_node);
 	pci_record_assignments(pbm, pbm->pci_bus);
diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index 374a055..3d82024 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -266,17 +266,12 @@ static int __init pci_common_init(void)
 	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
 
 	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		/*
-		 * Size the bridge windows.
-		 */
+		/* Size the bridge windows. */
 		pci_bus_size_bridges(puv3_bus);
-
-		/*
-		 * Assign resources.
-		 */
+		/* Assign resources. */
 		pci_bus_assign_resources(puv3_bus);
 	}
-
+	pci_bus_add_devices(puv3_bus);
 	return 0;
 }
 subsys_initcall(pci_common_init);
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index 3efaf4c..db0f2e8 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -740,7 +740,7 @@ static void ibm_unconfigure_device(struct pci_func *func)
  */
 static u8 bus_structure_fixup(u8 busno)
 {
-	struct pci_bus *bus;
+	struct pci_bus *bus, *b;
 	struct pci_dev *dev;
 	u16 l;
 
@@ -767,7 +767,9 @@ static u8 bus_structure_fixup(u8 busno)
 					(l != 0x0000) && (l != 0xffff)) {
 			debug("%s - Inside bus_structure_fixup()\n",
 							__func__);
-			pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			b = pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			if (b)
+				pci_bus_add_devices(b);
 			break;
 		}
 	}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index db16678..5c5e01e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2120,7 +2120,6 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
 	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
 	if (b) {
 		pci_scan_child_bus(b);
-		pci_bus_add_devices(b);
 	} else {
 		pci_free_resource_list(&resources);
 	}
-- 
1.7.1

[PATCH 1/5] PCI: Rip out pci_bus_add_devices() from pci_scan_bus()

[Yijing Wang]

Pci_bus_add_devices() should not be placed in
pci_scan_bus(). Now pci devices will be added
to driver core once its creation. All the things
left in pci_bus_add_devices() are driver attach
and other trivial things. pci_scan_bus() should
be the function responsible for scanning PCI devices,
not including device driver attached. Other, lots
callers of pci_scan_bus() call pci_bus_size_bridges()
and pci_bus_assign_resources() after pci_scan_bus().
It's not correct, but because pci_scan_bus() always
be called before driver loaded, so maybe no problem
will cause. We should rip out pci_bus_add_devices()
for better code design.
No functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Guan Xuetao <gxt@mprc.pku.edu.cn>
CC: linux-alpha at vger.kernel.org
CC: linux-m68k at lists.linux-m68k.org
CC: sparclinux at vger.kernel.org
---
 arch/alpha/kernel/sys_nautilus.c  |    1 +
 arch/m68k/coldfire/pci.c          |    1 +
 arch/sparc/kernel/pcic.c          |    2 ++
 arch/unicore32/kernel/pci.c       |   11 +++--------
 drivers/pci/hotplug/ibmphp_core.c |    6 ++++--
 drivers/pci/probe.c               |    1 -
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nautilus.c
index 837c0fa..4ae4a40 100644
--- a/arch/alpha/kernel/sys_nautilus.c
+++ b/arch/alpha/kernel/sys_nautilus.c
@@ -253,6 +253,7 @@ nautilus_init_pci(void)
 	   for the root bus, so just clear it. */
 	bus->self = NULL;
 	pci_fixup_irqs(alpha_mv.pci_swizzle, alpha_mv.pci_map_irq);
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
index df96792..d45f087 100644
--- a/arch/m68k/coldfire/pci.c
+++ b/arch/m68k/coldfire/pci.c
@@ -319,6 +319,7 @@ static int __init mcf_pci_init(void)
 	pci_fixup_irqs(pci_common_swizzle, mcf_pci_map_irq);
 	pci_bus_size_bridges(rootbus);
 	pci_bus_assign_resources(rootbus);
+	pci_bus_add_devices(rootbus);
 	return 0;
 }
 
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 6cc78c2..7a82fe2 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -391,6 +391,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pcic *pcic)
 	struct linux_pbm_info *pbm = &pcic->pbm;
 
 	pbm->pci_bus = pci_scan_bus(pbm->pci_first_busno, &pcic_ops, pbm);
+	if (pbm->pci_bus)
+		pci_bus_add_devices(pbm->pci_bus);
 #if 0 /* deadwood transplanted from sparc64 */
 	pci_fill_in_pbm_cookies(pbm->pci_bus, pbm, pbm->prom_node);
 	pci_record_assignments(pbm, pbm->pci_bus);
diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index 374a055..3d82024 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -266,17 +266,12 @@ static int __init pci_common_init(void)
 	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
 
 	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		/*
-		 * Size the bridge windows.
-		 */
+		/* Size the bridge windows. */
 		pci_bus_size_bridges(puv3_bus);
-
-		/*
-		 * Assign resources.
-		 */
+		/* Assign resources. */
 		pci_bus_assign_resources(puv3_bus);
 	}
-
+	pci_bus_add_devices(puv3_bus);
 	return 0;
 }
 subsys_initcall(pci_common_init);
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index 3efaf4c..db0f2e8 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -740,7 +740,7 @@ static void ibm_unconfigure_device(struct pci_func *func)
  */
 static u8 bus_structure_fixup(u8 busno)
 {
-	struct pci_bus *bus;
+	struct pci_bus *bus, *b;
 	struct pci_dev *dev;
 	u16 l;
 
@@ -767,7 +767,9 @@ static u8 bus_structure_fixup(u8 busno)
 					(l != 0x0000) && (l != 0xffff)) {
 			debug("%s - Inside bus_structure_fixup()\n",
 							__func__);
-			pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			b = pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			if (b)
+				pci_bus_add_devices(b);
 			break;
 		}
 	}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index db16678..5c5e01e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2120,7 +2120,6 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
 	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
 	if (b) {
 		pci_scan_child_bus(b);
-		pci_bus_add_devices(b);
 	} else {
 		pci_free_resource_list(&resources);
 	}
-- 
1.7.1

[PATCH 1/5] PCI: Rip out pci_bus_add_devices() from pci_scan_bus()

[Yijing Wang]

Pci_bus_add_devices() should not be placed in
pci_scan_bus(). Now pci devices will be added
to driver core once its creation. All the things
left in pci_bus_add_devices() are driver attach
and other trivial things. pci_scan_bus() should
be the function responsible for scanning PCI devices,
not including device driver attached. Other, lots
callers of pci_scan_bus() call pci_bus_size_bridges()
and pci_bus_assign_resources() after pci_scan_bus().
It's not correct, but because pci_scan_bus() always
be called before driver loaded, so maybe no problem
will cause. We should rip out pci_bus_add_devices()
for better code design.
No functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Guan Xuetao <gxt@mprc.pku.edu.cn>
CC: linux-alpha@vger.kernel.org
CC: linux-m68k@lists.linux-m68k.org
CC: sparclinux@vger.kernel.org
---
 arch/alpha/kernel/sys_nautilus.c  |    1 +
 arch/m68k/coldfire/pci.c          |    1 +
 arch/sparc/kernel/pcic.c          |    2 ++
 arch/unicore32/kernel/pci.c       |   11 +++--------
 drivers/pci/hotplug/ibmphp_core.c |    6 ++++--
 drivers/pci/probe.c               |    1 -
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nautilus.c
index 837c0fa..4ae4a40 100644
--- a/arch/alpha/kernel/sys_nautilus.c
+++ b/arch/alpha/kernel/sys_nautilus.c
@@ -253,6 +253,7 @@ nautilus_init_pci(void)
 	   for the root bus, so just clear it. */
 	bus->self = NULL;
 	pci_fixup_irqs(alpha_mv.pci_swizzle, alpha_mv.pci_map_irq);
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
index df96792..d45f087 100644
--- a/arch/m68k/coldfire/pci.c
+++ b/arch/m68k/coldfire/pci.c
@@ -319,6 +319,7 @@ static int __init mcf_pci_init(void)
 	pci_fixup_irqs(pci_common_swizzle, mcf_pci_map_irq);
 	pci_bus_size_bridges(rootbus);
 	pci_bus_assign_resources(rootbus);
+	pci_bus_add_devices(rootbus);
 	return 0;
 }
 
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 6cc78c2..7a82fe2 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -391,6 +391,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pcic *pcic)
 	struct linux_pbm_info *pbm = &pcic->pbm;
 
 	pbm->pci_bus = pci_scan_bus(pbm->pci_first_busno, &pcic_ops, pbm);
+	if (pbm->pci_bus)
+		pci_bus_add_devices(pbm->pci_bus);
 #if 0 /* deadwood transplanted from sparc64 */
 	pci_fill_in_pbm_cookies(pbm->pci_bus, pbm, pbm->prom_node);
 	pci_record_assignments(pbm, pbm->pci_bus);
diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index 374a055..3d82024 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -266,17 +266,12 @@ static int __init pci_common_init(void)
 	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
 
 	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		/*
-		 * Size the bridge windows.
-		 */
+		/* Size the bridge windows. */
 		pci_bus_size_bridges(puv3_bus);
-
-		/*
-		 * Assign resources.
-		 */
+		/* Assign resources. */
 		pci_bus_assign_resources(puv3_bus);
 	}
-
+	pci_bus_add_devices(puv3_bus);
 	return 0;
 }
 subsys_initcall(pci_common_init);
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index 3efaf4c..db0f2e8 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -740,7 +740,7 @@ static void ibm_unconfigure_device(struct pci_func *func)
  */
 static u8 bus_structure_fixup(u8 busno)
 {
-	struct pci_bus *bus;
+	struct pci_bus *bus, *b;
 	struct pci_dev *dev;
 	u16 l;
 
@@ -767,7 +767,9 @@ static u8 bus_structure_fixup(u8 busno)
 					(l != 0x0000) && (l != 0xffff)) {
 			debug("%s - Inside bus_structure_fixup()\n",
 							__func__);
-			pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			b = pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			if (b)
+				pci_bus_add_devices(b);
 			break;
 		}
 	}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index db16678..5c5e01e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2120,7 +2120,6 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
 	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
 	if (b) {
 		pci_scan_child_bus(b);
-		pci_bus_add_devices(b);
 	} else {
 		pci_free_resource_list(&resources);
 	}
-- 
1.7.1

[PATCH 1/5] PCI: Rip out pci_bus_add_devices() from pci_scan_bus()

[Yijing Wang]

Pci_bus_add_devices() should not be placed in
pci_scan_bus(). Now pci devices will be added
to driver core once its creation. All the things
left in pci_bus_add_devices() are driver attach
and other trivial things. pci_scan_bus() should
be the function responsible for scanning PCI devices,
not including device driver attached. Other, lots
callers of pci_scan_bus() call pci_bus_size_bridges()
and pci_bus_assign_resources() after pci_scan_bus().
It's not correct, but because pci_scan_bus() always
be called before driver loaded, so maybe no problem
will cause. We should rip out pci_bus_add_devices()
for better code design.
No functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Guan Xuetao <gxt@mprc.pku.edu.cn>
CC: linux-alpha@vger.kernel.org
CC: linux-m68k@lists.linux-m68k.org
CC: sparclinux@vger.kernel.org
---
 arch/alpha/kernel/sys_nautilus.c  |    1 +
 arch/m68k/coldfire/pci.c          |    1 +
 arch/sparc/kernel/pcic.c          |    2 ++
 arch/unicore32/kernel/pci.c       |   11 +++--------
 drivers/pci/hotplug/ibmphp_core.c |    6 ++++--
 drivers/pci/probe.c               |    1 -
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nautilus.c
index 837c0fa..4ae4a40 100644
--- a/arch/alpha/kernel/sys_nautilus.c
+++ b/arch/alpha/kernel/sys_nautilus.c
@@ -253,6 +253,7 @@ nautilus_init_pci(void)
 	   for the root bus, so just clear it. */
 	bus->self = NULL;
 	pci_fixup_irqs(alpha_mv.pci_swizzle, alpha_mv.pci_map_irq);
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
index df96792..d45f087 100644
--- a/arch/m68k/coldfire/pci.c
+++ b/arch/m68k/coldfire/pci.c
@@ -319,6 +319,7 @@ static int __init mcf_pci_init(void)
 	pci_fixup_irqs(pci_common_swizzle, mcf_pci_map_irq);
 	pci_bus_size_bridges(rootbus);
 	pci_bus_assign_resources(rootbus);
+	pci_bus_add_devices(rootbus);
 	return 0;
 }
 
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 6cc78c2..7a82fe2 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -391,6 +391,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pcic *pcic)
 	struct linux_pbm_info *pbm = &pcic->pbm;
 
 	pbm->pci_bus = pci_scan_bus(pbm->pci_first_busno, &pcic_ops, pbm);
+	if (pbm->pci_bus)
+		pci_bus_add_devices(pbm->pci_bus);
 #if 0 /* deadwood transplanted from sparc64 */
 	pci_fill_in_pbm_cookies(pbm->pci_bus, pbm, pbm->prom_node);
 	pci_record_assignments(pbm, pbm->pci_bus);
diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index 374a055..3d82024 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -266,17 +266,12 @@ static int __init pci_common_init(void)
 	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
 
 	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		/*
-		 * Size the bridge windows.
-		 */
+		/* Size the bridge windows. */
 		pci_bus_size_bridges(puv3_bus);
-
-		/*
-		 * Assign resources.
-		 */
+		/* Assign resources. */
 		pci_bus_assign_resources(puv3_bus);
 	}
-
+	pci_bus_add_devices(puv3_bus);
 	return 0;
 }
 subsys_initcall(pci_common_init);
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index 3efaf4c..db0f2e8 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -740,7 +740,7 @@ static void ibm_unconfigure_device(struct pci_func *func)
  */
 static u8 bus_structure_fixup(u8 busno)
 {
-	struct pci_bus *bus;
+	struct pci_bus *bus, *b;
 	struct pci_dev *dev;
 	u16 l;
 
@@ -767,7 +767,9 @@ static u8 bus_structure_fixup(u8 busno)
 					(l != 0x0000) && (l != 0xffff)) {
 			debug("%s - Inside bus_structure_fixup()\n",
 							__func__);
-			pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			b = pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			if (b)
+				pci_bus_add_devices(b);
 			break;
 		}
 	}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index db16678..5c5e01e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2120,7 +2120,6 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
 	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
 	if (b) {
 		pci_scan_child_bus(b);
-		pci_bus_add_devices(b);
 	} else {
 		pci_free_resource_list(&resources);
 	}
-- 
1.7.1

[PATCH 1/5] PCI: Rip out pci_bus_add_devices() from pci_scan_bus()

[Yijing Wang]

Pci_bus_add_devices() should not be placed in
pci_scan_bus(). Now pci devices will be added
to driver core once its creation. All the things
left in pci_bus_add_devices() are driver attach
and other trivial things. pci_scan_bus() should
be the function responsible for scanning PCI devices,
not including device driver attached. Other, lots
callers of pci_scan_bus() call pci_bus_size_bridges()
and pci_bus_assign_resources() after pci_scan_bus().
It's not correct, but because pci_scan_bus() always
be called before driver loaded, so maybe no problem
will cause. We should rip out pci_bus_add_devices()
for better code design.
No functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Guan Xuetao <gxt@mprc.pku.edu.cn>
CC: linux-alpha@vger.kernel.org
CC: linux-m68k@lists.linux-m68k.org
CC: sparclinux@vger.kernel.org
---
 arch/alpha/kernel/sys_nautilus.c  |    1 +
 arch/m68k/coldfire/pci.c          |    1 +
 arch/sparc/kernel/pcic.c          |    2 ++
 arch/unicore32/kernel/pci.c       |   11 +++--------
 drivers/pci/hotplug/ibmphp_core.c |    6 ++++--
 drivers/pci/probe.c               |    1 -
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nautilus.c
index 837c0fa..4ae4a40 100644
--- a/arch/alpha/kernel/sys_nautilus.c
+++ b/arch/alpha/kernel/sys_nautilus.c
@@ -253,6 +253,7 @@ nautilus_init_pci(void)
 	   for the root bus, so just clear it. */
 	bus->self = NULL;
 	pci_fixup_irqs(alpha_mv.pci_swizzle, alpha_mv.pci_map_irq);
+	pci_bus_add_devices(bus);
 }
 
 /*
diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
index df96792..d45f087 100644
--- a/arch/m68k/coldfire/pci.c
+++ b/arch/m68k/coldfire/pci.c
@@ -319,6 +319,7 @@ static int __init mcf_pci_init(void)
 	pci_fixup_irqs(pci_common_swizzle, mcf_pci_map_irq);
 	pci_bus_size_bridges(rootbus);
 	pci_bus_assign_resources(rootbus);
+	pci_bus_add_devices(rootbus);
 	return 0;
 }
 
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 6cc78c2..7a82fe2 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -391,6 +391,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pcic *pcic)
 	struct linux_pbm_info *pbm = &pcic->pbm;
 
 	pbm->pci_bus = pci_scan_bus(pbm->pci_first_busno, &pcic_ops, pbm);
+	if (pbm->pci_bus)
+		pci_bus_add_devices(pbm->pci_bus);
 #if 0 /* deadwood transplanted from sparc64 */
 	pci_fill_in_pbm_cookies(pbm->pci_bus, pbm, pbm->prom_node);
 	pci_record_assignments(pbm, pbm->pci_bus);
diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index 374a055..3d82024 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -266,17 +266,12 @@ static int __init pci_common_init(void)
 	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
 
 	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		/*
-		 * Size the bridge windows.
-		 */
+		/* Size the bridge windows. */
 		pci_bus_size_bridges(puv3_bus);
-
-		/*
-		 * Assign resources.
-		 */
+		/* Assign resources. */
 		pci_bus_assign_resources(puv3_bus);
 	}
-
+	pci_bus_add_devices(puv3_bus);
 	return 0;
 }
 subsys_initcall(pci_common_init);
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index 3efaf4c..db0f2e8 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -740,7 +740,7 @@ static void ibm_unconfigure_device(struct pci_func *func)
  */
 static u8 bus_structure_fixup(u8 busno)
 {
-	struct pci_bus *bus;
+	struct pci_bus *bus, *b;
 	struct pci_dev *dev;
 	u16 l;
 
@@ -767,7 +767,9 @@ static u8 bus_structure_fixup(u8 busno)
 					(l != 0x0000) && (l != 0xffff)) {
 			debug("%s - Inside bus_structure_fixup()\n",
 							__func__);
-			pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			b = pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
+			if (b)
+				pci_bus_add_devices(b);
 			break;
 		}
 	}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index db16678..5c5e01e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2120,7 +2120,6 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
 	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
 	if (b) {
 		pci_scan_child_bus(b);
-		pci_bus_add_devices(b);
 	} else {
 		pci_free_resource_list(&resources);
 	}
-- 
1.7.1