Make hot unplugging of PCI buses work

Hi,

Linus - this patch is for discussion, NOT for applying unless you have zero problems with it since it actively breaks existing hotplug PCI.

PCI Split-bridge "docking station". It's basically a PCI bus with VGA, serial, keyboard, mouse, parport in a box, connected to a cardbus card. Other models include a PC-type case with PCI slots connected to a cardbus card.

To put it another way, hotplug PCI bus. In my case, I have the following bus structure:

---[00]-+-00.0 Digital Equipment Corporation StrongARM DC21285

        +-01.0  Cirrus Logic PD 6833 PCMCIA/CardBus Ctrlr
           \-[01]---00.0  MOBILITY Electronics: Unknown device 0120
                     \-[02]--+-00.0  MOBILITY Electronics: Unknown device 0121
                             +-00.1  MOBILITY Electronics: Unknown device 0122
                             +-00.2  MOBILITY Electronics: Unknown device 0123
                             +-00.3  MOBILITY Electronics: Unknown device 0124
                             +-04.0  ATI Technologies Inc Rage XL
                             +-08.0  Realtek Semiconductor Co., Ltd. RTL-8139
                             \-0c.0  OPTi Inc. 82C861
        +-01.1  Cirrus Logic PD 6833 PCMCIA/CardBus Ctrlr
        +-02.0  OPTi Inc. 82C861
        \-03.0  Advanced Micro Devices [AMD] 79c978 [HomePNA]

And without the device plugged in:

---[00]-+-00.0  Digital Equipment Corporation StrongARM DC21285
        +-01.0  Cirrus Logic PD 6833 PCMCIA/CardBus Ctrlr
        +-01.1  Cirrus Logic PD 6833 PCMCIA/CardBus Ctrlr
        +-02.0  OPTi Inc. 82C861
        \-03.0  Advanced Micro Devices [AMD] 79c978 [HomePNA]

Currently, we do not have a way to tell the PCI subsystem "this device has gone, as have all its children" without manually walking the device tree. I suspect real docking stations will require this same functionality.

The following patch adds support for this to the PCI layer - there are two entry points:

- pci_remove_bus_device(dev)
This removes one device from its parent bus, and any children.
- pci_remove_all_bus_devices(bus)
This removes all devices from a bus, including any children.

The Cardbus layer will use pci_remove_all_bus_devices() when a cardbus card is removed. The drivers do not have an option whether to obey the removal request or not - you've yanked the cardbus device from the socket and the devices are gone.

pci_remove_bus_device() is intended where you have a set of hot- pluggable devices on a bus and you can remove them individually. It is intended that the existing hotplug layer should use this if they need to handle power issues associated with each PCI device socket individually on their hotplugging bus(es).

Furthermore, I propose that pci_remove_device() shall disappear - and this devices makes it so (thereby breaking existing hotplug drivers.)

This patch does not contain any of the other changes to allow Linux to deal with PCI buses hanging off a cardbus card.

@@ -278,8 +278,7 @@
  * Delete the device structure from the device lists,
  * remove the /proc entry, and notify userspace (/sbin/hotplug).
  */
-void
-pci_remove_device(struct pci_dev *dev)
+static void pci_remove_device(struct pci_dev *dev)
 {
 	device_unregister(&dev->dev);
 	list_del(&dev->bus_list);

@@ -290,7 +289,69 @@
 #endif
 }
 
+/**
+ * pci_remove_bus_device - remove a PCI device and any children
+ * @dev: the device to remove
+ *
+ * Remove a PCI device from the device lists, informing the drivers
+ * that the device has been removed.  We also remove any subordinate
+ * buses and children in a depth-first manner.
+ */
+void pci_remove_bus_device(struct pci_dev *dev)
+{
+	if (dev->subordinate) {
+		struct pci_bus *b = dev->subordinate;
+
+		/*
+		 * Remove all devices on this bus.
+		 */
+		pci_remove_all_bus_devices(b);
+
+		/*
+		 * This bus must have now have no devices.
+		 */
+		BUG_ON(!list_empty(&b->children));
+		BUG_ON(!list_empty(&b->devices));
+
+		/*
+		 * Remove bus from parent bus list and free.
+		 */
+		list_del(&b->node);
+		kfree(b);
+		dev->subordinate = NULL;
+	}
+
+	/*
+	 * Now remove this device.  This tells drivers that
+	 * this device is going away, unlinks it from the
+	 * device chains and removes any sysfs or procfs files.
+	 */
+	pci_remove_device(dev);
+
+	kfree(dev);
+}
+
+/**
+ * pci_remove_all_bus_devices - remove all devices on a PCI bus
+ * @bus: parent bus of devices to remove
+ *
+ * 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_remove_all_bus_devices(struct pci_bus *bus)
+{
+	struct list_head *l, *n;
+
+	list_for_each_safe(l, n, &bus->devices) {
+		struct pci_dev *dev = pci_dev_b(l);
+
+		pci_remove_bus_device(dev);
+	}
+}
+
 #ifdef CONFIG_HOTPLUG
 EXPORT_SYMBOL(pci_insert_device);
-EXPORT_SYMBOL(pci_remove_device);
+EXPORT_SYMBOL(pci_remove_bus_device);
+EXPORT_SYMBOL(pci_remove_all_bus_devices);
 #endif

@@ -645,7 +645,8 @@
 int pci_register_driver(struct pci_driver *);
 void pci_unregister_driver(struct pci_driver *);
 void pci_insert_device(struct pci_dev *, struct pci_bus *);
-void pci_remove_device(struct pci_dev *);
+void pci_remove_bus_device(struct pci_dev *);
+void pci_remove_all_bus_devices(struct pci_bus *);
 struct pci_driver *pci_dev_driver(const struct pci_dev *);
 const struct pci_device_id *pci_match_device(const struct pci_device_id *ids, const struct pci_dev *dev);
 unsigned int pci_do_scan_bus(struct pci_bus *bus);

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Does this docking station change PCI chassis ids?

Well, I definitely want it, and you should add Alan to the cc list since he apparently even _has_ one of these devices.

I don't have any objections to the patch, but I won't apply it until the otehr PCI people have had a chance to weigh in on it.

Can't you just fix up the current users to use "pci_remove_bus_device()". The breakage seems a bit spiteful ;)

Linus

Alan kindly sent one of these beasts to me, which renewed my interest in this area. I'll forward stuff so far.

I'd like to hear Gregs comments first - Greg knows the hotplugging code better than me. It appears to have its own way of decending some of the PCI buses, but it seems unclear why it needs to supervise removal of devices which are downstream from the hotplug slot.

As far as inserting this device, there needs to be a fair number of other to the PCI layer to make stuff work sanely. Currently, in order, we:

1. discovering all devices
2. once all devices have been initialised, registering each device with sysfs and thereby letting the drivers know.
3. apply any fixups needed
4. initialise any resources that need initialising

The drivers quite rightfully moan, and it isn't a pretty sight.

IMO, what we should be doing, in order, is:

1. discovering all devices
2. apply any fixups needed
3. initialise any resources that need initialising
4. once all devices have been initialised, registering each device with sysfs and thereby letting the drivers know.

We need to wait until everything is setup for step 4 because we may (and do in the case of this split-bridge) need to program PCI-PCI bridges before the devices become accessible.

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

There is an additional catch with the resources to be careful of. Some environments require we register/unregister additional IRQ routing tables. Thats a minor problem but does affect things like IBM thinkpad 600 docking station. Thats a transparent bridge with devices that appear and vanish stuck behind it. Similar problems - it needs

pci_attach_behind_bridge()
pci_destroy_behind_bridge()

functionality just like the cardbus toys

If you're talking the Mobility things, I certainly wouldn't mind to have one ;-) - this stuff isn't available here in Moscow, I did check...

Ditto.

At a glance I agree with your patch.

Ivan.

Having beaten out something roughly similiar for the cPCI hotplug code, I have a couple of comments:
1) The description of pci_remove_bus_device says "informing the drivers that the device has been removed", yet unless I'm missing some sysfs wrinkle, no call will be made to an attached driver's remove callback.
2) The recursive bus handling in pci_remove_bus_device should probably call pci_proc_detach_bus and potentially should also update the parent bridge's subordinate field. The latter is something that I'm doing at the moment in the cPCI hotplug code, but have just recently decided needs further investigation, as my solution does not handle multiple remove and insert cycles with different boards (e.g. it's easy to end up with overlapping ranges behind different bridges).

The current device removal code in all of the PCI hotplug drivers are based to varying degrees on sets of callbacks driven by the pci_visit_* family of functions, and will hence need varying amounts of rework to be able to just call pci_remove_bus_device instead. My cPCI hotplug driver and the ACPI based driver are likely the easiest to change over, since they attempt none of the more sophisticated resource management tricks that the Compaq and IBM drivers do.

Scott

Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: [email protected]

pci_remove_all_bus_devices => pci_remove_bus_device =>
pci_remove_device => device_unregister => device_del =>
bus_remove_device => device_release_driver => driver->remove

Good catch - I'll create a new patch for Monday.

Yes - I think this is something we may consider when sorting out the insertion.

However, whether x86 PCs will survive bus renumbering or not remains to be seen. We currently try to leave as much of the configuration intact from the BIOS.

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Okay, cool. My cursory browsing of drivers/base/core.c missed this, my apologies.

Your previous example wouldn't, but depending on how the BIOS assigns bus numbers at boot time, I could see some docking station hardware suffering from the problems I see on cPCI. For example, if the BIOS does not leave a hole in the bus numbers for the docking station's bridge, and there happens to be another bridge on the laptop, then you would have the same overlapping range problem after docking that I see after hot-inserting a peripheral card with a bridge.

I suspect I'm going to have to try and find out sooner or later, as all of the x86 based cPCI system boards that I've seen use either Phoenix or Award BIOSes that started their lives in desktop PCs.

Scott

Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: [email protected]

Note that I made cardbus bus numbering _ignore_ the BIOS-setup numbering even on PC's, exactly because of issues like this - trying to keep the original BIOS numbering just won't work if the BIOS sets the wrong numbers (I saw a BIOS that had happily assigned the _same_ PCI bus number to both cardbus functions, whee).

I think we can (and should) make all hotpluggable PCI bridges use that same cardbus logic.

The real problematic case I see is if there are transparent hotplug bridges, with some devices just magically appear and disappear from a part of a bus because of some invisible bridge. I don't know if such things exist or even _can_ exist, but the perverse nature of PC hardware makes me suspect they do.

Linus

If you mean the "max +=3" CardBus stuff in pci_scan_bridge, I can't see anything that simple working on a lot of the CompactPCI hardware I've seen, which tends to have multiple on-board bridges (Serverworks chipset boards especially). Reserving bus numbers would therefore mean doing a subsequent renumbering of the other bus(es). I think even your CardBus workaround would suffer in the case of a laptop which for some reason had another bridge (e.g. we might soon see laptops with both CardBus and Newcard bridges).

One approach that might work well for the CardBus situation, and might suffice for now for cPCI, might be to assign the hotplug bridge's subordinate bus and its children bus numbers starting at the maximum busnr
+ 1 after the initial pci_scan_bus has been done. If there is more than one hotplug bridge, each gets an equal share of the remaining bus numbers. On a cPCI system that has several cards with their own bridges, such a scheme is potentially wasteful of bus numbers, but at least the renumbering would be contained to buses that should be able to handle it. For cPCI, some form of DMI-based system would also be required to identify the hotplug bridges, but I've been considering such a beast for resource reservation anyways.

All of the cPCI bus extenders that I've googled so far seem to use relatively normal bridges, so I'm thinking (also hoping) that the scenario you describe is not possible. It does seem feasible, however, that one could build a pretty deep bus hierarchy in a 21-slot cPCI chassis, which could pose a sizing problem for automatic reservation schemes. Here's hoping no one crawls out of the woodwork after 2.6.0 with either beast...

Scott

Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: [email protected]

It gets called after device_unregister() is called on the pci device, through the driver core, and is actually done in the pci_device_remove() function in driver/pci/pci-driver.c

Hm, I just realized that the pci hotplug drivers were calling the remove function twice (once on their own, and then through sysfs), this patch will fix that.

It shouldn't be that tough to convert the Compaq and IBM drivers, I'll work on it tonight and test on hardware tomorrow.

thanks,

greg k-h

Yeah, the pci hotplug driver all implemented their own code to do this, it's much smarter to put this in the core like you just did.

I like the patch, nice job. If you don't mind, I'll add it to my tree, fix up all of the pci hotplug drivers, and then send all of that off to Linus. Is that ok?

thanks,

greg k-h

Here's the updated patch:

- Scott spotted a leak my handling of procfs wrt buses.
- I've also killed pci_remove_device() entirely - it is now inlined.
- After one of Alan's mails, I decided that pci_remove_behind_bridge() is a much better name than pci_remove_all_bus_devices()

@@ -10,6 +10,7 @@
 #define DBG(x...)
 #endif
 
+static void pci_free_resources(struct pci_dev *dev);
 
 #ifdef CONFIG_HOTPLUG
 int pci_hotplug (struct device *dev, char **envp, int num_envp,

@@ -224,7 +225,13 @@
 	if (pci_is_dev_in_use(dev)) {
 		return -EBUSY;
 	}
-	pci_remove_device(dev);
+	device_unregister(&dev->dev);
+	list_del(&dev->bus_list);
+	list_del(&dev->global_list);
+	pci_free_resources(dev);
+#ifdef CONFIG_PROC_FS
+	pci_proc_detach_device(dev);
+#endif
 	return 0;
 }
 EXPORT_SYMBOL(pci_remove_device_safe);

@@ -272,15 +279,33 @@
 }
 
 /**
- * pci_remove_device - remove a pci device
+ * pci_remove_bus_device - remove a PCI device and any children
  * @dev: the device to remove
  *
- * Delete the device structure from the device lists,
- * remove the /proc entry, and notify userspace (/sbin/hotplug).
+ * Remove a PCI device from the device lists, informing the drivers
+ * that the device has been removed.  We also remove any subordinate
+ * buses and children in a depth-first manner.
+ *
+ * For each device we remove, delete the device structure from the
+ * device lists, remove the /proc entry, and notify userspace
+ * (/sbin/hotplug).
  */
-void
-pci_remove_device(struct pci_dev *dev)
+void pci_remove_bus_device(struct pci_dev *dev)
 {
+	if (dev->subordinate) {
+		struct pci_bus *b = dev->subordinate;
+
+		pci_remove_behind_bridge(dev);
+
+#ifdef CONFIG_PROC_FS
+		pci_proc_detach_bus(b);
+#endif
+
+		list_del(&b->node);
+		kfree(b);
+		dev->subordinate = NULL;
+	}
+
 	device_unregister(&dev->dev);
 	list_del(&dev->bus_list);
 	list_del(&dev->global_list);

@@ -288,9 +313,33 @@
 #ifdef CONFIG_PROC_FS
 	pci_proc_detach_device(dev);
 #endif
+
+	kfree(dev);
+}
+
+/**
+ * pci_remove_behind_bridge - 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_remove_behind_bridge(struct pci_dev *dev)
+{
+	struct list_head *l, *n;
+
+	if (dev->subordinate) {
+		list_for_each_safe(l, n, &dev->subordinate->devices) {
+			struct pci_dev *dev = pci_dev_b(l);
+
+			pci_remove_bus_device(dev);
+		}
+	}
 }
 
 #ifdef CONFIG_HOTPLUG
 EXPORT_SYMBOL(pci_insert_device);
-EXPORT_SYMBOL(pci_remove_device);
+EXPORT_SYMBOL(pci_remove_bus_device);
+EXPORT_SYMBOL(pci_remove_behind_bridge);
 #endif

@@ -645,7 +645,8 @@
 int pci_register_driver(struct pci_driver *);
 void pci_unregister_driver(struct pci_driver *);
 void pci_insert_device(struct pci_dev *, struct pci_bus *);
-void pci_remove_device(struct pci_dev *);
+void pci_remove_bus_device(struct pci_dev *);
+void pci_remove_behind_bridge(struct pci_dev *);
 struct pci_driver *pci_dev_driver(const struct pci_dev *);
 const struct pci_device_id *pci_match_device(const struct pci_device_id *ids, const struct pci_dev *dev);
 unsigned int pci_do_scan_bus(struct pci_bus *bus);

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html