[PATCH v2 00/16] Clean up drivers/pci/remove.c

[PATCH v2 00/16] Clean up drivers/pci/remove.c

[Bjorn Helgaas]

This started as a simple conversion of list_for_each() to
list_for_each_entry(), so I could remove the pci_dev_b() helper.

In the process, I noticed that drivers/pci/remove.c is getting a little
crufty, so I reworked it to make it more understandable.  This is a long
series of small patches, so it might be easiest to start by looking at the
end product, so you have some idea where I'm heading.

Here's the final version of remove.c:
http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=blob;f=drivers/pci/remove.c/pci/remove.c;h=4f9ca9162895edd8216fda36e055949541d44012;hb=refs/heads/pci/bjorn-cleanup-remove

This is based on v3.6-rc1 and replaces the 6-patch series I posted on Aug 16.

I've booted this on x86, but I don't have facilities to test the
interesting hotplug changes, so any review and/or testing is welcome.

---

Bjorn Helgaas (16):
      PCI: acpiphp: Stop disabling bridges on remove
      PCI: acpiphp: Use common pci_stop_and_remove_bus_device()
      pcmcia: Use common pci_stop_and_remove_bus_device()
      PCI: Don't export stop_bus_device and remove_bus_device interfaces
      PCI: Remove pci_stop_and_remove_behind_bridge()
      PCI: Use list_for_each_entry() for bus->devices traversal
      PCI: Fold stop and remove helpers into their callers
      PCI: Stop and remove devices in one pass
      PCI: Remove unused, commented-out, code
      PCI: Rename local variables to conventional names
      PCI: Leave normal LIST_POISON in deleted list entries
      frv/PCI: Use list_for_each_entry() for bus->devices traversal
      parisc/PCI: Enable PERR/SERR on all devices
      parisc/PCI: Use list_for_each_entry() for bus->devices traversal
      sgi-agp: Use list_for_each_entry() for bus->devices traversal
      PCI: Remove unused pci_dev_b()


 arch/frv/mb93090-mb00/pci-vdk.c    |    4 -
 drivers/char/agp/sgi-agp.c         |    5 +
 drivers/parisc/dino.c              |    6 +-
 drivers/parisc/lba_pci.c           |    7 +-
 drivers/pci/hotplug/acpiphp_glue.c |   46 -------------
 drivers/pci/proc.c                 |   19 -----
 drivers/pci/remove.c               |  131 ++++++------------------------------
 drivers/pci/rom.c                  |   59 ----------------
 drivers/pci/search.c               |    6 +-
 drivers/pcmcia/cardbus.c           |    7 +-
 include/linux/pci.h                |    4 -
 11 files changed, 37 insertions(+), 257 deletions(-)

[PATCH v2 01/16] PCI: acpiphp: Stop disabling bridges on remove

[Bjorn Helgaas]

acpiphp_disable_slot() turns off power to the slot immediately after
calling disable_device(), so there's no point in disabling any bridges
below the slot: we're about to turn them off anyway.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/hotplug/acpiphp_glue.c |   15 ---------------
 1 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index ad6fd66..c25291c 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -869,17 +869,6 @@ static int __ref enable_device(struct acpiphp_slot *slot)
 	return retval;
 }
 
-static void disable_bridges(struct pci_bus *bus)
-{
-	struct pci_dev *dev;
-	list_for_each_entry(dev, &bus->devices, bus_list) {
-		if (dev->subordinate) {
-			disable_bridges(dev->subordinate);
-			pci_disable_device(dev);
-		}
-	}
-}
-
 /* return first device in slot, acquiring a reference on it */
 static struct pci_dev *dev_in_slot(struct acpiphp_slot *slot)
 {
@@ -932,10 +921,6 @@ static int disable_device(struct acpiphp_slot *slot)
 	 */
 	while ((pdev = dev_in_slot(slot))) {
 		pci_stop_bus_device(pdev);
-		if (pdev->subordinate) {
-			disable_bridges(pdev->subordinate);
-			pci_disable_device(pdev);
-		}
 		__pci_remove_bus_device(pdev);
 		pci_dev_put(pdev);
 	}

[PATCH v2 02/16] PCI: acpiphp: Use common pci_stop_and_remove_bus_device()

[Bjorn Helgaas]

Use pci_stop_and_remove_bus_device() like most other hotplug drivers
rather than stopping and removing separately.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/hotplug/acpiphp_glue.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index c25291c..b5d798e 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -920,8 +920,7 @@ static int disable_device(struct acpiphp_slot *slot)
 	 * here.
 	 */
 	while ((pdev = dev_in_slot(slot))) {
-		pci_stop_bus_device(pdev);
-		__pci_remove_bus_device(pdev);
+		pci_stop_and_remove_bus_device(pdev);
 		pci_dev_put(pdev);
 	}
 

[PATCH v2 03/16] pcmcia: Use common pci_stop_and_remove_bus_device()

[Bjorn Helgaas]

Use pci_stop_and_remove_bus_device() like most other hotplug drivers
rather than the special-purpose "behind_bridge" variant.  This just
means we have to iterate through all the devices downstream of the
bridge ourselves, which is the same thing pci_stop_behind_bridge()
did.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pcmcia/cardbus.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pcmcia/cardbus.c b/drivers/pcmcia/cardbus.c
index 24caeaf..94f87b5 100644
--- a/drivers/pcmcia/cardbus.c
+++ b/drivers/pcmcia/cardbus.c
@@ -106,7 +106,10 @@ int __ref cb_alloc(struct pcmcia_socket *s)
 void cb_free(struct pcmcia_socket *s)
 {
 	struct pci_dev *bridge = s->cb_dev;
+	struct pci_bus *bus = bridge->subordinate;
+	struct pci_dev *dev, *tmp;
 
-	if (bridge)
-		pci_stop_and_remove_behind_bridge(bridge);
+	if (bus)
+		list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list)
+			pci_stop_and_remove_bus_device(dev);
 }

[PATCH v2 04/16] PCI: Don't export stop_bus_device and remove_bus_device interfaces

[Bjorn Helgaas]

The acpiphp hotplug driver was the only user of pci_stop_bus_device() and
__pci_remove_bus_device(), and it now uses pci_stop_and_remove_bus_device()
instead, so stop exposing these interfaces.

This removes these exported symbols:

    __pci_remove_bus_device
    pci_stop_bus_device

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/remove.c |    8 ++++----
 include/linux/pci.h  |    2 --
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 04a4861..534377f 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -79,6 +79,8 @@ void pci_remove_bus(struct pci_bus *pci_bus)
 EXPORT_SYMBOL(pci_remove_bus);
 
 static void __pci_remove_behind_bridge(struct pci_dev *dev);
+static void pci_stop_bus_device(struct pci_dev *dev);
+
 /**
  * pci_stop_and_remove_bus_device - remove a PCI device and any children
  * @dev: the device to remove
@@ -91,7 +93,7 @@ static void __pci_remove_behind_bridge(struct pci_dev *dev);
  * device lists, remove the /proc entry, and notify userspace
  * (/sbin/hotplug).
  */
-void __pci_remove_bus_device(struct pci_dev *dev)
+static void __pci_remove_bus_device(struct pci_dev *dev)
 {
 	if (dev->subordinate) {
 		struct pci_bus *b = dev->subordinate;
@@ -103,7 +105,6 @@ void __pci_remove_bus_device(struct pci_dev *dev)
 
 	pci_destroy_dev(dev);
 }
-EXPORT_SYMBOL(__pci_remove_bus_device);
 
 void pci_stop_and_remove_bus_device(struct pci_dev *dev)
 {
@@ -170,7 +171,7 @@ static void pci_stop_bus_devices(struct pci_bus *bus)
  * and so on). This also stop any subordinate buses and children in a
  * depth-first manner.
  */
-void pci_stop_bus_device(struct pci_dev *dev)
+static void pci_stop_bus_device(struct pci_dev *dev)
 {
 	if (dev->subordinate)
 		pci_stop_bus_devices(dev->subordinate);
@@ -180,4 +181,3 @@ void pci_stop_bus_device(struct pci_dev *dev)
 
 EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
 EXPORT_SYMBOL(pci_stop_and_remove_behind_bridge);
-EXPORT_SYMBOL_GPL(pci_stop_bus_device);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5faa831..54b5b2b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -734,9 +734,7 @@ u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);
 extern struct pci_dev *pci_dev_get(struct pci_dev *dev);
 extern void pci_dev_put(struct pci_dev *dev);
 extern void pci_remove_bus(struct pci_bus *b);
-extern void __pci_remove_bus_device(struct pci_dev *dev);
 extern void pci_stop_and_remove_bus_device(struct pci_dev *dev);
-extern void pci_stop_bus_device(struct pci_dev *dev);
 void pci_setup_cardbus(struct pci_bus *bus);
 extern void pci_sort_breadthfirst(void);
 #define dev_is_pci(d) ((d)->bus == &pci_bus_type)

[PATCH v2 05/16] PCI: Remove pci_stop_and_remove_behind_bridge()

[Bjorn Helgaas]

The PCMCIA CardBus driver was the only user of
pci_stop_and_remove_behind_bridge(), and it now uses
pci_stop_and_remove_bus_device() instead, so remove this interface.

This removes exported symbol pci_stop_and_remove_behind_bridge.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/remove.c |   25 -------------------------
 include/linux/pci.h  |    1 -
 2 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 534377f..b18dc2e 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -121,30 +121,6 @@ static void __pci_remove_behind_bridge(struct pci_dev *dev)
 			__pci_remove_bus_device(pci_dev_b(l));
 }
 
-static void pci_stop_behind_bridge(struct pci_dev *dev)
-{
-	struct list_head *l, *n;
-
-	if (dev->subordinate)
-		list_for_each_safe(l, n, &dev->subordinate->devices)
-			pci_stop_bus_device(pci_dev_b(l));
-}
-
-/**
- * pci_stop_and_remove_behind_bridge - stop and remove all devices behind
- *					 a PCI bridge
- * @dev: PCI bridge device
- *
- * Remove all devices on the bus, except for the parent bridge.
- * This also removes any child buses, and any devices they may
- * contain in a depth-first manner.
- */
-void pci_stop_and_remove_behind_bridge(struct pci_dev *dev)
-{
-	pci_stop_behind_bridge(dev);
-	__pci_remove_behind_bridge(dev);
-}
-
 static void pci_stop_bus_devices(struct pci_bus *bus)
 {
 	struct list_head *l, *n;
@@ -180,4 +156,3 @@ static void pci_stop_bus_device(struct pci_dev *dev)
 }
 
 EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
-EXPORT_SYMBOL(pci_stop_and_remove_behind_bridge);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 54b5b2b..1dce47c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1011,7 +1011,6 @@ void pci_unregister_driver(struct pci_driver *dev);
 	module_driver(__pci_driver, pci_register_driver, \
 		       pci_unregister_driver)
 
-void pci_stop_and_remove_behind_bridge(struct pci_dev *dev);
 struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
 int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int vendor, unsigned int device,

[PATCH v2 06/16] PCI: Use list_for_each_entry() for bus->devices traversal

[Bjorn Helgaas]

Replace list_for_each() + pci_dev_b() with the simpler
list_for_each_entry().

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/remove.c |   13 ++++++-------
 drivers/pci/search.c |    6 ++----
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index b18dc2e..f17a027 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -114,16 +114,17 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
 
 static void __pci_remove_behind_bridge(struct pci_dev *dev)
 {
-	struct list_head *l, *n;
+	struct pci_dev *child, *tmp;
 
 	if (dev->subordinate)
-		list_for_each_safe(l, n, &dev->subordinate->devices)
-			__pci_remove_bus_device(pci_dev_b(l));
+		list_for_each_entry_safe(child, tmp,
+					 &dev->subordinate->devices, bus_list)
+			__pci_remove_bus_device(child);
 }
 
 static void pci_stop_bus_devices(struct pci_bus *bus)
 {
-	struct list_head *l, *n;
+	struct pci_dev *dev, *tmp;
 
 	/*
 	 * VFs could be removed by pci_stop_and_remove_bus_device() in the
@@ -133,10 +134,8 @@ static void pci_stop_bus_devices(struct pci_bus *bus)
 	 * We can iterate the list backwards to get prev valid PF instead
 	 *  of removed VF.
 	 */
-	list_for_each_prev_safe(l, n, &bus->devices) {
-		struct pci_dev *dev = pci_dev_b(l);
+	list_for_each_entry_safe_reverse(dev, tmp, &bus->devices, bus_list)
 		pci_stop_bus_device(dev);
-	}
 }
 
 /**
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 993d4a0..f56b237 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -130,16 +130,14 @@ pci_find_next_bus(const struct pci_bus *from)
  * decrement the reference count by calling pci_dev_put().
  * If no device is found, %NULL is returned.
  */
-struct pci_dev * pci_get_slot(struct pci_bus *bus, unsigned int devfn)
+struct pci_dev *pci_get_slot(struct pci_bus *bus, unsigned int devfn)
 {
-	struct list_head *tmp;
 	struct pci_dev *dev;
 
 	WARN_ON(in_interrupt());
 	down_read(&pci_bus_sem);
 
-	list_for_each(tmp, &bus->devices) {
-		dev = pci_dev_b(tmp);
+	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (dev->devfn == devfn)
 			goto out;
 	}

[PATCH v2 07/16] PCI: Fold stop and remove helpers into their callers

[Bjorn Helgaas]

pci_stop_bus_devices() is only two lines of code and is only called by
pci_stop_bus_device(), so I think it's easier to read if we just fold it
into the caller.  Similarly for __pci_remove_behind_bridge().

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/remove.c |   53 +++++++++++++++++++-------------------------------
 1 files changed, 20 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index f17a027..30d002e 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -78,7 +78,6 @@ void pci_remove_bus(struct pci_bus *pci_bus)
 }
 EXPORT_SYMBOL(pci_remove_bus);
 
-static void __pci_remove_behind_bridge(struct pci_dev *dev);
 static void pci_stop_bus_device(struct pci_dev *dev);
 
 /**
@@ -95,11 +94,14 @@ static void pci_stop_bus_device(struct pci_dev *dev);
  */
 static void __pci_remove_bus_device(struct pci_dev *dev)
 {
-	if (dev->subordinate) {
-		struct pci_bus *b = dev->subordinate;
+	struct pci_bus *bus = dev->subordinate;
+	struct pci_dev *child, *tmp;
+
+	if (bus) {
+		list_for_each_entry_safe(child, tmp, &bus->devices, bus_list)
+			__pci_remove_bus_device(child);
 
-		__pci_remove_behind_bridge(dev);
-		pci_remove_bus(b);
+		pci_remove_bus(bus);
 		dev->subordinate = NULL;
 	}
 
@@ -112,32 +114,6 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
 	__pci_remove_bus_device(dev);
 }
 
-static void __pci_remove_behind_bridge(struct pci_dev *dev)
-{
-	struct pci_dev *child, *tmp;
-
-	if (dev->subordinate)
-		list_for_each_entry_safe(child, tmp,
-					 &dev->subordinate->devices, bus_list)
-			__pci_remove_bus_device(child);
-}
-
-static void pci_stop_bus_devices(struct pci_bus *bus)
-{
-	struct pci_dev *dev, *tmp;
-
-	/*
-	 * VFs could be removed by pci_stop_and_remove_bus_device() in the
-	 *  pci_stop_bus_devices() code path for PF.
-	 *  aka, bus->devices get updated in the process.
-	 * but VFs are inserted after PFs when SRIOV is enabled for PF,
-	 * We can iterate the list backwards to get prev valid PF instead
-	 *  of removed VF.
-	 */
-	list_for_each_entry_safe_reverse(dev, tmp, &bus->devices, bus_list)
-		pci_stop_bus_device(dev);
-}
-
 /**
  * pci_stop_bus_device - stop a PCI device and any children
  * @dev: the device to stop
@@ -148,8 +124,19 @@ static void pci_stop_bus_devices(struct pci_bus *bus)
  */
 static void pci_stop_bus_device(struct pci_dev *dev)
 {
-	if (dev->subordinate)
-		pci_stop_bus_devices(dev->subordinate);
+	struct pci_bus *bus = dev->subordinate;
+	struct pci_dev *child, *tmp;
+
+	/*
+	 * Removing an SR-IOV PF device removes all the associated VFs,
+	 * which will update the bus->devices list and confuse the
+	 * iterator.  Therefore, iterate in reverse so we remove the VFs
+	 * first, then the PF.
+	 */
+	if (bus)
+		list_for_each_entry_safe_reverse(child, tmp,
+						 &bus->devices, bus_list)
+			pci_stop_bus_device(child);
 
 	pci_stop_dev(dev);
 }

[PATCH v2 08/16] PCI: Stop and remove devices in one pass

[Bjorn Helgaas]

Previously, when we removed a PCI device, we made two passes over the
hierarchy rooted at the device.  In the first pass, we stopped all
the devices, and in the second, we removed them.

This patch combines the two passes into one so that we remove a device as
soon as it and all its children have been stopped.

Note that we previously stopped devices in reverse order and removed them
in forward order.  Now we stop and remove them in reverse order.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/remove.c |   42 +++++++-----------------------------------
 1 files changed, 7 insertions(+), 35 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 30d002e..3828104 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -78,8 +78,6 @@ void pci_remove_bus(struct pci_bus *pci_bus)
 }
 EXPORT_SYMBOL(pci_remove_bus);
 
-static void pci_stop_bus_device(struct pci_dev *dev);
-
 /**
  * pci_stop_and_remove_bus_device - remove a PCI device and any children
  * @dev: the device to remove
@@ -92,38 +90,8 @@ static void pci_stop_bus_device(struct pci_dev *dev);
  * device lists, remove the /proc entry, and notify userspace
  * (/sbin/hotplug).
  */
-static void __pci_remove_bus_device(struct pci_dev *dev)
-{
-	struct pci_bus *bus = dev->subordinate;
-	struct pci_dev *child, *tmp;
-
-	if (bus) {
-		list_for_each_entry_safe(child, tmp, &bus->devices, bus_list)
-			__pci_remove_bus_device(child);
-
-		pci_remove_bus(bus);
-		dev->subordinate = NULL;
-	}
-
-	pci_destroy_dev(dev);
-}
-
 void pci_stop_and_remove_bus_device(struct pci_dev *dev)
 {
-	pci_stop_bus_device(dev);
-	__pci_remove_bus_device(dev);
-}
-
-/**
- * pci_stop_bus_device - stop a PCI device and any children
- * @dev: the device to stop
- *
- * Stop a PCI device (detach the driver, remove from the global list
- * and so on). This also stop any subordinate buses and children in a
- * depth-first manner.
- */
-static void pci_stop_bus_device(struct pci_dev *dev)
-{
 	struct pci_bus *bus = dev->subordinate;
 	struct pci_dev *child, *tmp;
 
@@ -133,12 +101,16 @@ static void pci_stop_bus_device(struct pci_dev *dev)
 	 * iterator.  Therefore, iterate in reverse so we remove the VFs
 	 * first, then the PF.
 	 */
-	if (bus)
+	if (bus) {
 		list_for_each_entry_safe_reverse(child, tmp,
 						 &bus->devices, bus_list)
-			pci_stop_bus_device(child);
+			pci_stop_and_remove_bus_device(child);
+
+		pci_remove_bus(bus);
+		dev->subordinate = NULL;
+	}
 
 	pci_stop_dev(dev);
+	pci_destroy_dev(dev);
 }
-
 EXPORT_SYMBOL(pci_stop_and_remove_bus_device);

[PATCH v2 09/16] PCI: Remove unused, commented-out, code

[Bjorn Helgaas]

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/hotplug/acpiphp_glue.c |   28 -----------------
 drivers/pci/proc.c                 |   19 ------------
 drivers/pci/remove.c               |   19 ------------
 drivers/pci/rom.c                  |   59 ------------------------------------
 4 files changed, 0 insertions(+), 125 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index b5d798e..7be4ca5 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1461,34 +1461,6 @@ int __init acpiphp_get_num_slots(void)
 }
 
 
-#if 0
-/**
- * acpiphp_for_each_slot - call function for each slot
- * @fn: callback function
- * @data: context to be passed to callback function
- */
-static int acpiphp_for_each_slot(acpiphp_callback fn, void *data)
-{
-	struct list_head *node;
-	struct acpiphp_bridge *bridge;
-	struct acpiphp_slot *slot;
-	int retval = 0;
-
-	list_for_each (node, &bridge_list) {
-		bridge = (struct acpiphp_bridge *)node;
-		for (slot = bridge->slots; slot; slot = slot->next) {
-			retval = fn(slot, data);
-			if (!retval)
-				goto err_exit;
-		}
-	}
-
- err_exit:
-	return retval;
-}
-#endif
-
-
 /**
  * acpiphp_enable_slot - power on slot
  * @slot: ACPI PHP slot
diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index 27911b5..eb907a8f 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -434,25 +434,6 @@ int pci_proc_detach_device(struct pci_dev *dev)
 	return 0;
 }
 
-#if 0
-int pci_proc_attach_bus(struct pci_bus* bus)
-{
-	struct proc_dir_entry *de = bus->procdir;
-
-	if (!proc_initialized)
-		return -EACCES;
-
-	if (!de) {
-		char name[16];
-		sprintf(name, "%02x", bus->number);
-		de = bus->procdir = proc_mkdir(name, proc_bus_pci_dir);
-		if (!de)
-			return -ENOMEM;
-	}
-	return 0;
-}
-#endif  /*  0  */
-
 int pci_proc_detach_bus(struct pci_bus* bus)
 {
 	struct proc_dir_entry *de = bus->procdir;
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 3828104..44f479f 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -43,25 +43,6 @@ static void pci_destroy_dev(struct pci_dev *dev)
 	pci_dev_put(dev);
 }
 
-/**
- * pci_remove_device_safe - remove an unused hotplug device
- * @dev: the device to remove
- *
- * Delete the device structure from the device lists and 
- * notify userspace (/sbin/hotplug), but only if the device
- * in question is not being used by a driver.
- * Returns 0 on success.
- */
-#if 0
-int pci_remove_device_safe(struct pci_dev *dev)
-{
-	if (pci_dev_driver(dev))
-		return -EBUSY;
-	pci_destroy_dev(dev);
-	return 0;
-}
-#endif  /*  0  */
-
 void pci_remove_bus(struct pci_bus *pci_bus)
 {
 	pci_proc_detach_bus(pci_bus);
diff --git a/drivers/pci/rom.c b/drivers/pci/rom.c
index 48ebdb2..0b3037a 100644
--- a/drivers/pci/rom.c
+++ b/drivers/pci/rom.c
@@ -167,44 +167,6 @@ void __iomem *pci_map_rom(struct pci_dev *pdev, size_t *size)
 	return rom;
 }
 
-#if 0
-/**
- * pci_map_rom_copy - map a PCI ROM to kernel space, create a copy
- * @pdev: pointer to pci device struct
- * @size: pointer to receive size of pci window over ROM
- *
- * Return: kernel virtual pointer to image of ROM
- *
- * Map a PCI ROM into kernel space. If ROM is boot video ROM,
- * the shadow BIOS copy will be returned instead of the
- * actual ROM.
- */
-void __iomem *pci_map_rom_copy(struct pci_dev *pdev, size_t *size)
-{
-	struct resource *res = &pdev->resource[PCI_ROM_RESOURCE];
-	void __iomem *rom;
-
-	rom = pci_map_rom(pdev, size);
-	if (!rom)
-		return NULL;
-
-	if (res->flags & (IORESOURCE_ROM_COPY | IORESOURCE_ROM_SHADOW |
-			  IORESOURCE_ROM_BIOS_COPY))
-		return rom;
-
-	res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
-	if (!res->start)
-		return rom;
-
-	res->end = res->start + *size;
-	memcpy_fromio((void*)(unsigned long)res->start, rom, *size);
-	pci_unmap_rom(pdev, rom);
-	res->flags |= IORESOURCE_ROM_COPY;
-
-	return (void __iomem *)(unsigned long)res->start;
-}
-#endif  /*  0  */
-
 /**
  * pci_unmap_rom - unmap the ROM from kernel space
  * @pdev: pointer to pci device struct
@@ -226,27 +188,6 @@ void pci_unmap_rom(struct pci_dev *pdev, void __iomem *rom)
 		pci_disable_rom(pdev);
 }
 
-#if 0
-/**
- * pci_remove_rom - disable the ROM and remove its sysfs attribute
- * @pdev: pointer to pci device struct
- *
- * Remove the rom file in sysfs and disable ROM decoding.
- */
-void pci_remove_rom(struct pci_dev *pdev)
-{
-	struct resource *res = &pdev->resource[PCI_ROM_RESOURCE];
-
-	if (pci_resource_len(pdev, PCI_ROM_RESOURCE))
-		sysfs_remove_bin_file(&pdev->dev.kobj, pdev->rom_attr);
-	if (!(res->flags & (IORESOURCE_ROM_ENABLE |
-			    IORESOURCE_ROM_SHADOW |
-			    IORESOURCE_ROM_BIOS_COPY |
-			    IORESOURCE_ROM_COPY)))
-		pci_disable_rom(pdev);
-}
-#endif  /*  0  */
-
 /**
  * pci_cleanup_rom - free the ROM copy created by pci_map_rom_copy
  * @pdev: pointer to pci device struct

[PATCH v2 10/16] PCI: Rename local variables to conventional names

[Bjorn Helgaas]

"bus" is the conventional name for a "struct pci_bus *" variable.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/remove.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 44f479f..c01baca 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -43,19 +43,19 @@ static void pci_destroy_dev(struct pci_dev *dev)
 	pci_dev_put(dev);
 }
 
-void pci_remove_bus(struct pci_bus *pci_bus)
+void pci_remove_bus(struct pci_bus *bus)
 {
-	pci_proc_detach_bus(pci_bus);
+	pci_proc_detach_bus(bus);
 
 	down_write(&pci_bus_sem);
-	list_del(&pci_bus->node);
-	pci_bus_release_busn_res(pci_bus);
+	list_del(&bus->node);
+	pci_bus_release_busn_res(bus);
 	up_write(&pci_bus_sem);
-	if (!pci_bus->is_added)
+	if (!bus->is_added)
 		return;
 
-	pci_remove_legacy_files(pci_bus);
-	device_unregister(&pci_bus->dev);
+	pci_remove_legacy_files(bus);
+	device_unregister(&bus->dev);
 }
 EXPORT_SYMBOL(pci_remove_bus);
 

[PATCH v2 11/16] PCI: Leave normal LIST_POISON in deleted list entries

[Bjorn Helgaas]

list_del() already sets next/prev to LIST_POISON1/LIST_POISON2, so we
don't need to do anything special here to prevent further list accesses.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/remove.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index c01baca..4f9ca91 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -32,11 +32,8 @@ static void pci_stop_dev(struct pci_dev *dev)
 
 static void pci_destroy_dev(struct pci_dev *dev)
 {
-	/* Remove the device from the device lists, and prevent any further
-	 * list accesses from this device */
 	down_write(&pci_bus_sem);
 	list_del(&dev->bus_list);
-	dev->bus_list.next = dev->bus_list.prev = NULL;
 	up_write(&pci_bus_sem);
 
 	pci_free_resources(dev);

[PATCH v2 12/16] frv/PCI: Use list_for_each_entry() for bus->devices traversal

[Bjorn Helgaas]

Replace open-coded list traversal with list_for_each_entry().

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: David Howells <dhowells@redhat.com>
---
 arch/frv/mb93090-mb00/pci-vdk.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index d04ed14..71e9bcf 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -330,10 +330,8 @@ void __init pcibios_fixup_bus(struct pci_bus *bus)
 	pci_read_bridge_bases(bus);
 
 	if (bus->number == 0) {
-		struct list_head *ln;
 		struct pci_dev *dev;
-		for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
-			dev = pci_dev_b(ln);
+		list_for_each_entry(dev, &bus->devices, bus_list) {
 			if (dev->devfn == 0) {
 				dev->resource[0].start = 0;
 				dev->resource[0].end = 0;

[PATCH v2 13/16] parisc/PCI: Enable PERR/SERR on all devices

[Bjorn Helgaas]

Previously, we enabled PERR & SERR for the first device on the bus, but
left other devices alone.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: linux-parisc@vger.kernel.org
---
 drivers/parisc/lba_pci.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index 4f9cf24..4ce57c9 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -630,6 +630,7 @@ static void
 lba_fixup_bus(struct pci_bus *bus)
 {
 	struct list_head *ln;
+	struct pci_dev *dev;
 #ifdef FBB_SUPPORT
 	u16 status;
 #endif
@@ -712,8 +713,8 @@ lba_fixup_bus(struct pci_bus *bus)
 
 	list_for_each(ln, &bus->devices) {
 		int i;
-		struct pci_dev *dev = pci_dev_b(ln);
 
+		dev = pci_dev_b(ln);
 		DBG("lba_fixup_bus() %s\n", pci_name(dev));
 
 		/* Virtualize Device/Bridge Resources. */
@@ -771,6 +772,7 @@ lba_fixup_bus(struct pci_bus *bus)
 
 	/* Lastly enable FBB/PERR/SERR on all devices too */
 	list_for_each(ln, &bus->devices) {
+		dev = pci_dev_b(ln);
 		(void) pci_read_config_word(dev, PCI_COMMAND, &status);
 		status |= PCI_COMMAND_PARITY | PCI_COMMAND_SERR | fbb_enable;
 		(void) pci_write_config_word(dev, PCI_COMMAND, status);

[PATCH v2 14/16] parisc/PCI: Use list_for_each_entry() for bus->devices traversal

[Bjorn Helgaas]

Replace list_for_each() + pci_dev_b() with the simpler
list_for_each_entry().

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: linux-parisc@vger.kernel.org
---
 drivers/parisc/dino.c    |    6 ++----
 drivers/parisc/lba_pci.c |    7 ++-----
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/parisc/dino.c b/drivers/parisc/dino.c
index ffddc4f..4581ee0 100644
--- a/drivers/parisc/dino.c
+++ b/drivers/parisc/dino.c
@@ -477,14 +477,12 @@ dino_card_setup(struct pci_bus *bus, void __iomem *base_addr)
 	if (ccio_allocate_resource(dino_dev->hba.dev, res, _8MB,
 				F_EXTEND(0xf0000000UL) | _8MB,
 				F_EXTEND(0xffffffffUL) &~ _8MB, _8MB) < 0) {
-		struct list_head *ln, *tmp_ln;
+		struct pci_dev *dev, *tmp;
 
 		printk(KERN_ERR "Dino: cannot attach bus %s\n",
 		       dev_name(bus->bridge));
 		/* kill the bus, we can't do anything with it */
-		list_for_each_safe(ln, tmp_ln, &bus->devices) {
-			struct pci_dev *dev = pci_dev_b(ln);
-
+		list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list) {
 			list_del(&dev->bus_list);
 		}
 			
diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index 4ce57c9..fdd63a6 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -629,7 +629,6 @@ truncate_pat_collision(struct resource *root, struct resource *new)
 static void
 lba_fixup_bus(struct pci_bus *bus)
 {
-	struct list_head *ln;
 	struct pci_dev *dev;
 #ifdef FBB_SUPPORT
 	u16 status;
@@ -711,10 +710,9 @@ lba_fixup_bus(struct pci_bus *bus)
 
 	}
 
-	list_for_each(ln, &bus->devices) {
+	list_for_each_entry(dev, &bus->devices, bus_list) {
 		int i;
 
-		dev = pci_dev_b(ln);
 		DBG("lba_fixup_bus() %s\n", pci_name(dev));
 
 		/* Virtualize Device/Bridge Resources. */
@@ -771,8 +769,7 @@ lba_fixup_bus(struct pci_bus *bus)
 	}
 
 	/* Lastly enable FBB/PERR/SERR on all devices too */
-	list_for_each(ln, &bus->devices) {
-		dev = pci_dev_b(ln);
+	list_for_each_entry(dev, &bus->devices, bus_list) {
 		(void) pci_read_config_word(dev, PCI_COMMAND, &status);
 		status |= PCI_COMMAND_PARITY | PCI_COMMAND_SERR | fbb_enable;
 		(void) pci_write_config_word(dev, PCI_COMMAND, status);

[PATCH v2 15/16] sgi-agp: Use list_for_each_entry() for bus->devices traversal

[Bjorn Helgaas]

Replace list_for_each() + pci_dev_b() with the simpler
list_for_each_entry().

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: David Airlie <airlied@linux.ie>
---
 drivers/char/agp/sgi-agp.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/char/agp/sgi-agp.c b/drivers/char/agp/sgi-agp.c
index 1920003..3a5af2f 100644
--- a/drivers/char/agp/sgi-agp.c
+++ b/drivers/char/agp/sgi-agp.c
@@ -289,12 +289,11 @@ static int __devinit agp_sgi_init(void)
 
 	j = 0;
 	list_for_each_entry(info, &tioca_list, ca_list) {
-		struct list_head *tmp;
 		if (list_empty(info->ca_devices))
 			continue;
-		list_for_each(tmp, info->ca_devices) {
+		list_for_each_entry(pdev, info->ca_devices, bus_list) {
 			u8 cap_ptr;
-			pdev = pci_dev_b(tmp);
+
 			if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8))
 				continue;
 			cap_ptr = pci_find_capability(pdev, PCI_CAP_ID_AGP);

[PATCH v2 16/16] PCI: Remove unused pci_dev_b()

[Bjorn Helgaas]

All uses of pci_dev_b() have been replaced by list_for_each_entry(), so
remove it.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/linux/pci.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1dce47c..ed47147 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -369,7 +369,6 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
 
 extern struct pci_dev *alloc_pci_dev(void);
 
-#define pci_dev_b(n) list_entry(n, struct pci_dev, bus_list)
 #define	to_pci_dev(n) container_of(n, struct pci_dev, dev)
 #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
 

Re: [PATCH v2 03/16] pcmcia: Use common pci_stop_and_remove_bus_device()

[Yinghai Lu]

On Fri, Aug 17, 2012 at 4:36 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Use pci_stop_and_remove_bus_device() like most other hotplug drivers
> rather than the special-purpose "behind_bridge" variant.  This just
> means we have to iterate through all the devices downstream of the
> bridge ourselves, which is the same thing pci_stop_behind_bridge()
> did.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pcmcia/cardbus.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pcmcia/cardbus.c b/drivers/pcmcia/cardbus.c
> index 24caeaf..94f87b5 100644
> --- a/drivers/pcmcia/cardbus.c
> +++ b/drivers/pcmcia/cardbus.c
> @@ -106,7 +106,10 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>  void cb_free(struct pcmcia_socket *s)
>  {
>         struct pci_dev *bridge = s->cb_dev;
> +       struct pci_bus *bus = bridge->subordinate;
> +       struct pci_dev *dev, *tmp;
>
> -       if (bridge)
> -               pci_stop_and_remove_behind_bridge(bridge);
> +       if (bus)
> +               list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list)
> +                       pci_stop_and_remove_bus_device(dev);
>  }
>

original looks like bridge could be NULL.

Yinghai

Re: [PATCH v2 08/16] PCI: Stop and remove devices in one pass

[Yinghai Lu]

On Fri, Aug 17, 2012 at 4:36 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Previously, when we removed a PCI device, we made two passes over the
> hierarchy rooted at the device.  In the first pass, we stopped all
> the devices, and in the second, we removed them.
>
> This patch combines the two passes into one so that we remove a device as
> soon as it and all its children have been stopped.
>
> Note that we previously stopped devices in reverse order and removed them
> in forward order.  Now we stop and remove them in reverse order.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/remove.c |   42 +++++++-----------------------------------
>  1 files changed, 7 insertions(+), 35 deletions(-)

looks like your cleanup will have some conflict with my pci root bus
hot plug branch.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=blob;f=drivers/pci/remove.c;h=f672731922f8db619fb36db05dbd9c27f3253c19;hb=refs/heads/for-pci-root-bus-hotplug

it add pci_stop_and_remove_bus and it depends on some functions that
you kill in this patch set.

 180 void pci_stop_and_remove_bus(struct pci_bus *bus)
181 {
182         struct pci_host_bridge *host_bridge = NULL;
183         struct pci_dev *pci_bridge = NULL;
184
185         pci_stop_bus_devices(bus);
186
187         if (pci_is_root_bus(bus)) {
188                 host_bridge = to_pci_host_bridge(bus->bridge);
189                 get_device(&host_bridge->dev);
190                 pci_stop_host_bridge(host_bridge);
191         } else
192                 pci_bridge = bus->self;
193
194         __pci_remove_bus_devices(bus);
195
196         pci_remove_bus(bus);
197
198         if (host_bridge) {
199                 host_bridge->bus = NULL;
200                 put_device(&host_bridge->dev);
201         }
202
203         if (pci_bridge)
204                 pci_bridge->subordinate = NULL;
205 }

Yinghai

Re: [PATCH v2 00/16] Clean up drivers/pci/remove.c

[Yijing Wang]

tested-by Yijing Wang <wangyijing@huawei.com>
> This started as a simple conversion of list_for_each() to
> list_for_each_entry(), so I could remove the pci_dev_b() helper.
> 
> In the process, I noticed that drivers/pci/remove.c is getting a little
> crufty, so I reworked it to make it more understandable.  This is a long
> series of small patches, so it might be easiest to start by looking at the
> end product, so you have some idea where I'm heading.
> 
> Here's the final version of remove.c:
> http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=blob;f=drivers/pci/remove.c/pci/remove.c;h=4f9ca9162895edd8216fda36e055949541d44012;hb=refs/heads/pci/bjorn-cleanup-remove
> 
> This is based on v3.6-rc1 and replaces the 6-patch series I posted on Aug 16.
> 
> I've booted this on x86, but I don't have facilities to test the
> interesting hotplug changes, so any review and/or testing is welcome.
> 
> ---
> 
> Bjorn Helgaas (16):
>       PCI: acpiphp: Stop disabling bridges on remove
>       PCI: acpiphp: Use common pci_stop_and_remove_bus_device()
>       pcmcia: Use common pci_stop_and_remove_bus_device()
>       PCI: Don't export stop_bus_device and remove_bus_device interfaces
>       PCI: Remove pci_stop_and_remove_behind_bridge()
>       PCI: Use list_for_each_entry() for bus->devices traversal
>       PCI: Fold stop and remove helpers into their callers
>       PCI: Stop and remove devices in one pass
>       PCI: Remove unused, commented-out, code
>       PCI: Rename local variables to conventional names
>       PCI: Leave normal LIST_POISON in deleted list entries
>       frv/PCI: Use list_for_each_entry() for bus->devices traversal
>       parisc/PCI: Enable PERR/SERR on all devices
>       parisc/PCI: Use list_for_each_entry() for bus->devices traversal
>       sgi-agp: Use list_for_each_entry() for bus->devices traversal
>       PCI: Remove unused pci_dev_b()
> 
> 
>  arch/frv/mb93090-mb00/pci-vdk.c    |    4 -
>  drivers/char/agp/sgi-agp.c         |    5 +
>  drivers/parisc/dino.c              |    6 +-
>  drivers/parisc/lba_pci.c           |    7 +-
>  drivers/pci/hotplug/acpiphp_glue.c |   46 -------------
>  drivers/pci/proc.c                 |   19 -----
>  drivers/pci/remove.c               |  131 ++++++------------------------------
>  drivers/pci/rom.c                  |   59 ----------------
>  drivers/pci/search.c               |    6 +-
>  drivers/pcmcia/cardbus.c           |    7 +-
>  include/linux/pci.h                |    4 -
>  11 files changed, 37 insertions(+), 257 deletions(-)
> --
> 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 v2 03/16] pcmcia: Use common pci_stop_and_remove_bus_device()

[Bjorn Helgaas]

On Fri, Aug 17, 2012 at 5:58 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Aug 17, 2012 at 4:36 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> Use pci_stop_and_remove_bus_device() like most other hotplug drivers
>> rather than the special-purpose "behind_bridge" variant.  This just
>> means we have to iterate through all the devices downstream of the
>> bridge ourselves, which is the same thing pci_stop_behind_bridge()
>> did.
>>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>>  drivers/pcmcia/cardbus.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pcmcia/cardbus.c b/drivers/pcmcia/cardbus.c
>> index 24caeaf..94f87b5 100644
>> --- a/drivers/pcmcia/cardbus.c
>> +++ b/drivers/pcmcia/cardbus.c
>> @@ -106,7 +106,10 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>>  void cb_free(struct pcmcia_socket *s)
>>  {
>>         struct pci_dev *bridge = s->cb_dev;
>> +       struct pci_bus *bus = bridge->subordinate;
>> +       struct pci_dev *dev, *tmp;
>>
>> -       if (bridge)
>> -               pci_stop_and_remove_behind_bridge(bridge);
>> +       if (bus)
>> +               list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list)
>> +                       pci_stop_and_remove_bus_device(dev);
>>  }
>>
>
> original looks like bridge could be NULL.

You're right, thanks!  I fixed that.

Re: [PATCH v2 08/16] PCI: Stop and remove devices in one pass

[Bjorn Helgaas]

On Fri, Aug 17, 2012 at 7:21 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Aug 17, 2012 at 4:36 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> Previously, when we removed a PCI device, we made two passes over the
>> hierarchy rooted at the device.  In the first pass, we stopped all
>> the devices, and in the second, we removed them.
>>
>> This patch combines the two passes into one so that we remove a device as
>> soon as it and all its children have been stopped.
>>
>> Note that we previously stopped devices in reverse order and removed them
>> in forward order.  Now we stop and remove them in reverse order.
>>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>>  drivers/pci/remove.c |   42 +++++++-----------------------------------
>>  1 files changed, 7 insertions(+), 35 deletions(-)
>
> looks like your cleanup will have some conflict with my pci root bus
> hot plug branch.

Yes, indeed, they have serious conflicts.

> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=blob;f=drivers/pci/remove.c;h=f672731922f8db619fb36db05dbd9c27f3253c19;hb=refs/heads/for-pci-root-bus-hotplug
>
> it add pci_stop_and_remove_bus and it depends on some functions that
> you kill in this patch set.

Do you have any suggestions on how to proceed?  In my opinion,
remove.c is a bit of a rat's nest right now.  It took me quite a long
time to sort out what's going on, but when you finally get down to the
bottom, it's actually very simple.  So I'd really like to clean it up
and expose that simple structure before we throw more stuff into it.

After my cleanup, pci_stop_and_remove_bus_device() and
pci_remove_bus() are the only exported interfaces left.  How hard
would it be to implement host bridge remove on top of that?

I think it's OK (and even preferable) if the host bridge-related code
can be segregated by itself rather than being sprinkled around through
lots of PCI code.  So maybe the pci_is_root_bus()  and
pci_stop_host_bridge() stuff below could be in some sort of host
bridge interface that just calls the code in remove.c.

>  180 void pci_stop_and_remove_bus(struct pci_bus *bus)
> 181 {
> 182         struct pci_host_bridge *host_bridge = NULL;
> 183         struct pci_dev *pci_bridge = NULL;
> 184
> 185         pci_stop_bus_devices(bus);
> 186
> 187         if (pci_is_root_bus(bus)) {
> 188                 host_bridge = to_pci_host_bridge(bus->bridge);
> 189                 get_device(&host_bridge->dev);
> 190                 pci_stop_host_bridge(host_bridge);
> 191         } else
> 192                 pci_bridge = bus->self;
> 193
> 194         __pci_remove_bus_devices(bus);
> 195
> 196         pci_remove_bus(bus);
> 197
> 198         if (host_bridge) {
> 199                 host_bridge->bus = NULL;
> 200                 put_device(&host_bridge->dev);
> 201         }
> 202
> 203         if (pci_bridge)
> 204                 pci_bridge->subordinate = NULL;
> 205 }
>
> Yinghai

Re: [PATCH v2 00/16] Clean up drivers/pci/remove.c

[Bjorn Helgaas]

On Sun, Aug 19, 2012 at 10:58 PM, Yijing Wang <wangyijing@huawei.com> wrote:
> tested-by Yijing Wang <wangyijing@huawei.com>

Great, thanks for trying this out!  Can you give me any details on
what you tested (what sort of machine, which hotplug driver, PCIe
button/LED style hotplug or ExpressCard style, etc?)

>> This started as a simple conversion of list_for_each() to
>> list_for_each_entry(), so I could remove the pci_dev_b() helper.
>>
>> In the process, I noticed that drivers/pci/remove.c is getting a little
>> crufty, so I reworked it to make it more understandable.  This is a long
>> series of small patches, so it might be easiest to start by looking at the
>> end product, so you have some idea where I'm heading.
>>
>> Here's the final version of remove.c:
>> http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=blob;f=drivers/pci/remove.c/pci/remove.c;h=4f9ca9162895edd8216fda36e055949541d44012;hb=refs/heads/pci/bjorn-cleanup-remove
>>
>> This is based on v3.6-rc1 and replaces the 6-patch series I posted on Aug 16.
>>
>> I've booted this on x86, but I don't have facilities to test the
>> interesting hotplug changes, so any review and/or testing is welcome.
>>
>> ---
>>
>> Bjorn Helgaas (16):
>>       PCI: acpiphp: Stop disabling bridges on remove
>>       PCI: acpiphp: Use common pci_stop_and_remove_bus_device()
>>       pcmcia: Use common pci_stop_and_remove_bus_device()
>>       PCI: Don't export stop_bus_device and remove_bus_device interfaces
>>       PCI: Remove pci_stop_and_remove_behind_bridge()
>>       PCI: Use list_for_each_entry() for bus->devices traversal
>>       PCI: Fold stop and remove helpers into their callers
>>       PCI: Stop and remove devices in one pass
>>       PCI: Remove unused, commented-out, code
>>       PCI: Rename local variables to conventional names
>>       PCI: Leave normal LIST_POISON in deleted list entries
>>       frv/PCI: Use list_for_each_entry() for bus->devices traversal
>>       parisc/PCI: Enable PERR/SERR on all devices
>>       parisc/PCI: Use list_for_each_entry() for bus->devices traversal
>>       sgi-agp: Use list_for_each_entry() for bus->devices traversal
>>       PCI: Remove unused pci_dev_b()
>>
>>
>>  arch/frv/mb93090-mb00/pci-vdk.c    |    4 -
>>  drivers/char/agp/sgi-agp.c         |    5 +
>>  drivers/parisc/dino.c              |    6 +-
>>  drivers/parisc/lba_pci.c           |    7 +-
>>  drivers/pci/hotplug/acpiphp_glue.c |   46 -------------
>>  drivers/pci/proc.c                 |   19 -----
>>  drivers/pci/remove.c               |  131 ++++++------------------------------
>>  drivers/pci/rom.c                  |   59 ----------------
>>  drivers/pci/search.c               |    6 +-
>>  drivers/pcmcia/cardbus.c           |    7 +-
>>  include/linux/pci.h                |    4 -
>>  11 files changed, 37 insertions(+), 257 deletions(-)
>> --
>> 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
>
> --
> 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 v2 00/16] Clean up drivers/pci/remove.c

[Yijing Wang]

On 2012/8/20 23:40, Bjorn Helgaas wrote:
> On Sun, Aug 19, 2012 at 10:58 PM, Yijing Wang <wangyijing@huawei.com> wrote:
>> tested-by Yijing Wang <wangyijing@huawei.com>
> 
> Great, thanks for trying this out!  Can you give me any details on
> what you tested (what sort of machine, which hotplug driver, PCIe
> button/LED style hotplug or ExpressCard style, etc?)
> 

Hi Bjorn,
   I tested these patches in IA_64, and use acpiphp driver to do the hot-plug test.
The hotplug action was triggered via sysfs interface(/sys/bus/pci/slots/xxx/).
The target pcie devices were fibre channel card and Intel 82576 card.As bellow:

0000:40:07.0 root port supports hotplug by acpiphp.
hot plug 0000:46:00.0 and its child devices and buses.

-+-[0000:40]-+-00.0-[0000:41]--
 |           +-01.0-[0000:42]--+-00.0  Intel Corporation 82576 Gigabit Network Connection
 |           |                 \-00.1  Intel Corporation 82576 Gigabit Network Connection
 |           +-03.0-[0000:43]----00.0  LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS
 |           +-04.0-[0000:44]--
 |           +-05.0-[0000:45]--
 |           +-07.0-[0000:46-49]----00.0-[0000:47-49]--+-02.0-[0000:48]--+-00.0  Intel Corporation 82576 Gigabit Network Connection
 |           |                                         |                 \-00.1  Intel Corporation 82576 Gigabit Network Connection
 |           |                                         \-04.0-[0000:49]--+-00.0  Intel Corporation 82576 Gigabit Network Connection
 |           |                                                           \-00.1  Intel Corporation 82576 Gigabit Network Connection

after hot remove
-+-[0000:40]-+-00.0-[0000:41]--
 |           +-01.0-[0000:42]--+-00.0  Intel Corporation 82576 Gigabit Network Connection
 |           |                 \-00.1  Intel Corporation 82576 Gigabit Network Connection
 |           +-03.0-[0000:43]----00.0  LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS
 |           +-04.0-[0000:44]--
 |           +-05.0-[0000:45]--
 |           +-07.0-[0000:46-49]--


>>> This started as a simple conversion of list_for_each() to
>>> list_for_each_entry(), so I could remove the pci_dev_b() helper.
>>>
>>> In the process, I noticed that drivers/pci/remove.c is getting a little
>>> crufty, so I reworked it to make it more understandable.  This is a long
>>> series of small patches, so it might be easiest to start by looking at the
>>> end product, so you have some idea where I'm heading.



-- 
Thanks!
Yijing

Re: [PATCH v2 08/16] PCI: Stop and remove devices in one pass

[Yinghai Lu]

On Mon, Aug 20, 2012 at 8:27 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Aug 17, 2012 at 7:21 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Aug 17, 2012 at 4:36 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> Previously, when we removed a PCI device, we made two passes over the
>>> hierarchy rooted at the device.  In the first pass, we stopped all
>>> the devices, and in the second, we removed them.
>>>
>>> This patch combines the two passes into one so that we remove a device as
>>> soon as it and all its children have been stopped.
>>>
>>> Note that we previously stopped devices in reverse order and removed them
>>> in forward order.  Now we stop and remove them in reverse order.
>>>
>>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>> ---
>>>  drivers/pci/remove.c |   42 +++++++-----------------------------------
>>>  1 files changed, 7 insertions(+), 35 deletions(-)
>>
>> looks like your cleanup will have some conflict with my pci root bus
>> hot plug branch.
>
> Yes, indeed, they have serious conflicts.
>
>> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=blob;f=drivers/pci/remove.c;h=f672731922f8db619fb36db05dbd9c27f3253c19;hb=refs/heads/for-pci-root-bus-hotplug
>>
>> it add pci_stop_and_remove_bus and it depends on some functions that
>> you kill in this patch set.
>
> Do you have any suggestions on how to proceed?  In my opinion,
> remove.c is a bit of a rat's nest right now.  It took me quite a long
> time to sort out what's going on, but when you finally get down to the
> bottom, it's actually very simple.  So I'd really like to clean it up
> and expose that simple structure before we throw more stuff into it.
>
> After my cleanup, pci_stop_and_remove_bus_device() and
> pci_remove_bus() are the only exported interfaces left.  How hard
> would it be to implement host bridge remove on top of that?
>
> I think it's OK (and even preferable) if the host bridge-related code
> can be segregated by itself rather than being sprinkled around through
> lots of PCI code.  So maybe the pci_is_root_bus()  and
> pci_stop_host_bridge() stuff below could be in some sort of host
> bridge interface that just calls the code in remove.c.

Yes, your change is quite clean.
Please go ahead to push your changes for 3.7 and my Acked-by.

I would try to rebase pci-root-bus hot plug branch according to the
output of discussion of pci mini-summit.

Thanks

Yinghai

Re: [PATCH v2 00/16] Clean up drivers/pci/remove.c

[Bjorn Helgaas]

On Mon, Aug 20, 2012 at 8:45 PM, Yijing Wang <wangyijing@huawei.com> wrote:
> On 2012/8/20 23:40, Bjorn Helgaas wrote:
>> On Sun, Aug 19, 2012 at 10:58 PM, Yijing Wang <wangyijing@huawei.com> wrote:
>>> tested-by Yijing Wang <wangyijing@huawei.com>
>>
>> Great, thanks for trying this out!  Can you give me any details on
>> what you tested (what sort of machine, which hotplug driver, PCIe
>> button/LED style hotplug or ExpressCard style, etc?)
>>
>
> Hi Bjorn,
>    I tested these patches in IA_64, and use acpiphp driver to do the hot-plug test.
> The hotplug action was triggered via sysfs interface(/sys/bus/pci/slots/xxx/).
> The target pcie devices were fibre channel card and Intel 82576 card.As bellow:

Thanks, that's perfect!  It's especially good to test acpiphp because
that was one of the "not completely obvious" changes.

I added your Tested-by: to the acpiphp and the PCI core patches; let
me know if that's not what you intended.

> 0000:40:07.0 root port supports hotplug by acpiphp.
> hot plug 0000:46:00.0 and its child devices and buses.
>
> -+-[0000:40]-+-00.0-[0000:41]--
>  |           +-01.0-[0000:42]--+-00.0  Intel Corporation 82576 Gigabit Network Connection
>  |           |                 \-00.1  Intel Corporation 82576 Gigabit Network Connection
>  |           +-03.0-[0000:43]----00.0  LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS
>  |           +-04.0-[0000:44]--
>  |           +-05.0-[0000:45]--
>  |           +-07.0-[0000:46-49]----00.0-[0000:47-49]--+-02.0-[0000:48]--+-00.0  Intel Corporation 82576 Gigabit Network Connection
>  |           |                                         |                 \-00.1  Intel Corporation 82576 Gigabit Network Connection
>  |           |                                         \-04.0-[0000:49]--+-00.0  Intel Corporation 82576 Gigabit Network Connection
>  |           |                                                           \-00.1  Intel Corporation 82576 Gigabit Network Connection
>
> after hot remove
> -+-[0000:40]-+-00.0-[0000:41]--
>  |           +-01.0-[0000:42]--+-00.0  Intel Corporation 82576 Gigabit Network Connection
>  |           |                 \-00.1  Intel Corporation 82576 Gigabit Network Connection
>  |           +-03.0-[0000:43]----00.0  LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS
>  |           +-04.0-[0000:44]--
>  |           +-05.0-[0000:45]--
>  |           +-07.0-[0000:46-49]--
>
>
>>>> This started as a simple conversion of list_for_each() to
>>>> list_for_each_entry(), so I could remove the pci_dev_b() helper.
>>>>
>>>> In the process, I noticed that drivers/pci/remove.c is getting a little
>>>> crufty, so I reworked it to make it more understandable.  This is a long
>>>> series of small patches, so it might be easiest to start by looking at the
>>>> end product, so you have some idea where I'm heading.
>
>
>
> --
> Thanks!
> Yijing
>
> --
> 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 v2 08/16] PCI: Stop and remove devices in one pass

[Bjorn Helgaas]

On Mon, Aug 20, 2012 at 10:39 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Aug 20, 2012 at 8:27 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Fri, Aug 17, 2012 at 7:21 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Fri, Aug 17, 2012 at 4:36 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>>> Previously, when we removed a PCI device, we made two passes over the
>>>> hierarchy rooted at the device.  In the first pass, we stopped all
>>>> the devices, and in the second, we removed them.
>>>>
>>>> This patch combines the two passes into one so that we remove a device as
>>>> soon as it and all its children have been stopped.
>>>>
>>>> Note that we previously stopped devices in reverse order and removed them
>>>> in forward order.  Now we stop and remove them in reverse order.
>>>>
>>>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>>> ---
>>>>  drivers/pci/remove.c |   42 +++++++-----------------------------------
>>>>  1 files changed, 7 insertions(+), 35 deletions(-)
>>>
>>> looks like your cleanup will have some conflict with my pci root bus
>>> hot plug branch.
>>
>> Yes, indeed, they have serious conflicts.
>>
>>> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=blob;f=drivers/pci/remove.c;h=f672731922f8db619fb36db05dbd9c27f3253c19;hb=refs/heads/for-pci-root-bus-hotplug
>>>
>>> it add pci_stop_and_remove_bus and it depends on some functions that
>>> you kill in this patch set.
>>
>> Do you have any suggestions on how to proceed?  In my opinion,
>> remove.c is a bit of a rat's nest right now.  It took me quite a long
>> time to sort out what's going on, but when you finally get down to the
>> bottom, it's actually very simple.  So I'd really like to clean it up
>> and expose that simple structure before we throw more stuff into it.
>>
>> After my cleanup, pci_stop_and_remove_bus_device() and
>> pci_remove_bus() are the only exported interfaces left.  How hard
>> would it be to implement host bridge remove on top of that?
>>
>> I think it's OK (and even preferable) if the host bridge-related code
>> can be segregated by itself rather than being sprinkled around through
>> lots of PCI code.  So maybe the pci_is_root_bus()  and
>> pci_stop_host_bridge() stuff below could be in some sort of host
>> bridge interface that just calls the code in remove.c.
>
> Yes, your change is quite clean.
> Please go ahead to push your changes for 3.7 and my Acked-by.
>
> I would try to rebase pci-root-bus hot plug branch according to the
> output of discussion of pci mini-summit.

I fixed the pcmcia bridge check you noted, added your acks and
Yijing's tested-by, and pushed the updated branch to
http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=shortlog;h=refs/heads/pci/bjorn-cleanup-remove

I plan to put it in my next branch tomorrow or Friday.

Bjorn

Re: [PATCH v2 00/16] Clean up drivers/pci/remove.c

[Bjorn Helgaas]

On Fri, Aug 17, 2012 at 5:35 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> This started as a simple conversion of list_for_each() to
> list_for_each_entry(), so I could remove the pci_dev_b() helper.
>
> In the process, I noticed that drivers/pci/remove.c is getting a little
> crufty, so I reworked it to make it more understandable.  This is a long
> series of small patches, so it might be easiest to start by looking at the
> end product, so you have some idea where I'm heading.

I applied this series (with the pcmcia fix suggested by Yinghai) to my
"next" branch.

> Here's the final version of remove.c:
> http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=blob;f=drivers/pci/remove.c/pci/remove.c;h=4f9ca9162895edd8216fda36e055949541d44012;hb=refs/heads/pci/bjorn-cleanup-remove
>
> This is based on v3.6-rc1 and replaces the 6-patch series I posted on Aug 16.
>
> I've booted this on x86, but I don't have facilities to test the
> interesting hotplug changes, so any review and/or testing is welcome.
>
> ---
>
> Bjorn Helgaas (16):
>       PCI: acpiphp: Stop disabling bridges on remove
>       PCI: acpiphp: Use common pci_stop_and_remove_bus_device()
>       pcmcia: Use common pci_stop_and_remove_bus_device()
>       PCI: Don't export stop_bus_device and remove_bus_device interfaces
>       PCI: Remove pci_stop_and_remove_behind_bridge()
>       PCI: Use list_for_each_entry() for bus->devices traversal
>       PCI: Fold stop and remove helpers into their callers
>       PCI: Stop and remove devices in one pass
>       PCI: Remove unused, commented-out, code
>       PCI: Rename local variables to conventional names
>       PCI: Leave normal LIST_POISON in deleted list entries
>       frv/PCI: Use list_for_each_entry() for bus->devices traversal
>       parisc/PCI: Enable PERR/SERR on all devices
>       parisc/PCI: Use list_for_each_entry() for bus->devices traversal
>       sgi-agp: Use list_for_each_entry() for bus->devices traversal
>       PCI: Remove unused pci_dev_b()
>
>
>  arch/frv/mb93090-mb00/pci-vdk.c    |    4 -
>  drivers/char/agp/sgi-agp.c         |    5 +
>  drivers/parisc/dino.c              |    6 +-
>  drivers/parisc/lba_pci.c           |    7 +-
>  drivers/pci/hotplug/acpiphp_glue.c |   46 -------------
>  drivers/pci/proc.c                 |   19 -----
>  drivers/pci/remove.c               |  131 ++++++------------------------------
>  drivers/pci/rom.c                  |   59 ----------------
>  drivers/pci/search.c               |    6 +-
>  drivers/pcmcia/cardbus.c           |    7 +-
>  include/linux/pci.h                |    4 -
>  11 files changed, 37 insertions(+), 257 deletions(-)