[RFC PATCH 00/10] PCI core learns 'hotplug'

[RFC PATCH 00/10] PCI core learns 'hotplug'

[Alex Chiang]

A while ago, Darrick Wong posted a patch for fakephp that kicked off
some controversy:

	http://thread.gmane.org/gmane.linux.kernel/761944

The issue was that I broke the fakephp interface back in the 2.6.27
timeframe. After some discussion on the lists, Trent Piepho sent some
patches, and I proposed a solution incorporating those patches.

This is my first cut at making everyone happy. In summary, it:

	- introduces /sys/bus/pci/devices/.../remove for function level
	  hot-remove
	
	- introduces /sys/bus/pci/devices/.../rescan to rescan the PCI
	  hierarchy, starting at that device and descending to all children

	- introduces /sys/bus/pci/rescan to rescan the entire PCI hierarchy

	- restores the pre-2.6.27 fakephp interface for userspace compatability

Some notes:

	- I did review Rolf Eike Beer's dummyphp patch at his request. It
	  won't work for the same reasons that we cannot fix the existing
	  fakephp driver -- both use the PCI hotplug API, namely
	  pci_hp_register(), which enforces the semantic of "only one
	  slots/ entry per physical slot", which clearly does not allow
	  multiple entries for multi-function devices. One side note, Trent's
	  new fakephp driver is also much smaller and cleaner, but the
	  real reason is dummyphp uses the PCI hotplug API.

	  I hope this reasoning is fair, but if folks disagree, I'm happy
	  to discuss

	- I've been testing this patchset on my ia64 machines, which Linus
	  has called "an insane mess of PCI bridges"[1], and it seems to
	  work well. I'm just starting to test on some x86 machines, and
	  have been noticing some issues with BAR collisions, so this is
	  definitely a work-in-progress.

	- Also, you might be wondering, "how does this patchset interact
	  with existing hotplug drivers, like acpiphp or pciehp?" The answer
	  is, "poorly". :(

	  If you use the new PCI core removal/rescan and then try to modify
	  the slot using acpiphp, you get an oops. My impression is that
	  this behavior is the same as pre-2.6.27, where you could have
	  loaded fakephp and acpiphp, removed the device with fakephp,
	  and encountered an oops with acpiphp.

	  2.6.27 enforced the rule of "one hotplug driver per slot", at the
	  cost of the fakephp breakage (which started this whole discussion).

	  So, I'm not sure what to do about this. The way that we remove
	  devices today, using pci_remove_bus_device() doesn't lend itself
	  to safety very well, since it will just start removing devices
	  from the bus without checking anything.

	  Maybe we need some other API, or maybe we just live with the
	  limitation of, "if you use PCI core hotplug, don't use the
	  other hotplug drivers and vice versa".

	- Finally, a note to Jesse, even if this patchset is rejected,
	  I'd recommend taking the ASPM fix, as that is just a pure bugfix.
	  It could probably go into .29.

Thanks. Comments welcome.

/ac

1: http://lkml.org/lkml/2008/12/13/196
---

Alex Chiang (9):
      PCI: more whitespace cleanups
      PCI Hotplug: schedule fakephp for feature removal
      PCI Hotplug: rename legacy_fakephp to fakephp
      PCI: Introduce /sys/bus/pci/devices/.../rescan
      PCI: Introduce /sys/bus/pci/rescan
      PCI: Introduce /sys/bus/pci/devices/.../remove
      PCI: properly clean up ASPM link state on device remove
      PCI: always scan child buses
      PCI: don't scan existing devices

Trent Piepho (1):
      PCI Hotplug: restore fakephp interface with complete reimplementation


 Documentation/ABI/testing/sysfs-bus-pci    |   27 ++
 Documentation/feature-removal-schedule.txt |   32 ++
 drivers/pci/hotplug/fakephp.c              |  438 +++++++---------------------
 drivers/pci/pci-sysfs.c                    |   98 ++++++
 drivers/pci/pcie/aspm.c                    |    4 
 drivers/pci/probe.c                        |   68 +++-
 drivers/pci/remove.c                       |    4 
 7 files changed, 303 insertions(+), 368 deletions(-)

[RFC PATCH 01/10] PCI: don't scan existing devices

[Alex Chiang]

pci_scan_slot is supposed to add newly discovered devices to
pci_bus->devices, but doesn't check to see if the device has
already been added. This can cause problems if we ever want
to use this interface to rescan the PCI bus.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/pci/probe.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 55ec44a..a9e17e2 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1035,6 +1035,17 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
 	for (func = 0; func < 8; func++, devfn++) {
 		struct pci_dev *dev;
 
+		dev = pci_get_slot(bus, devfn);
+		if (dev) {
+			if (dev->multifunction) {
+				pci_dev_put(dev);
+				continue;
+			} else {
+				pci_dev_put(dev);
+				break;
+			}
+		}
+
 		dev = pci_scan_single_device(bus, devfn);
 		if (dev) {
 			nr++;

[RFC PATCH 02/10] PCI: always scan child buses

[Alex Chiang]

While scanning bridges, we stop our scan if we encounter a bus
that we've seen before, to work around some buggy chipsets. This
is a good idea, but prevents us from fully scanning the PCI bus
at a future time (to find newly hot-added devices, for example).

Change the logic so that we skip _re-adding_ an existing bus
that we've seen before, but also allow the scan to descend to
all child buses.

Now that we're potentially scanning our child buses again, we
also need to be sure not to attempt re-initializing their BARs
so we avoid that.

This patch lays the groundwork to allow the user to issue a
rescan of the PCI bus at any time.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/pci/probe.c |   33 +++++++++++++++++++--------------
 1 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a9e17e2..9cf810e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -511,21 +511,21 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 
 		/*
 		 * If we already got to this bus through a different bridge,
-		 * ignore it.  This can happen with the i450NX chipset.
+		 * don't re-add it. This can happen with the i450NX chipset.
+		 *
+		 * However, we continue to descend down the hierarchy and
+		 * scan remaining child buses.
 		 */
-		if (pci_find_bus(pci_domain_nr(bus), busnr)) {
-			dev_info(&dev->dev, "bus %04x:%02x already known\n",
-				 pci_domain_nr(bus), busnr);
-			goto out;
+		child = pci_find_bus(pci_domain_nr(bus), busnr);
+		if (!child) {
+			child = pci_add_new_bus(bus, dev, busnr);
+			if (!child)
+				goto out;
+			child->primary = buses & 0xFF;
+			child->subordinate = (buses >> 16) & 0xFF;
+			child->bridge_ctl = bctl;
 		}
 
-		child = pci_add_new_bus(bus, dev, busnr);
-		if (!child)
-			goto out;
-		child->primary = buses & 0xFF;
-		child->subordinate = (buses >> 16) & 0xFF;
-		child->bridge_ctl = bctl;
-
 		cmax = pci_scan_child_bus(child);
 		if (cmax > max)
 			max = cmax;
@@ -1089,8 +1089,13 @@ unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
 	 * After performing arch-dependent fixup of the bus, look behind
 	 * all PCI-to-PCI bridges on this bus.
 	 */
-	pr_debug("PCI: Fixups for bus %04x:%02x\n", pci_domain_nr(bus), bus->number);
-	pcibios_fixup_bus(bus);
+	if (!bus->is_added) {
+		pr_debug("PCI: Fixups for bus %04x:%02x\n", pci_domain_nr(bus), bus->number);
+		pcibios_fixup_bus(bus);
+		if (bus->parent == NULL)
+			bus->is_added = 1;
+	}
+
 	for (pass=0; pass < 2; pass++)
 		list_for_each_entry(dev, &bus->devices, bus_list) {
 			if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||

[RFC PATCH 03/10] PCI: properly clean up ASPM link state on device remove

[Alex Chiang]

We only want to disable ASPM when the last function is removed from
the parent's device list. We determine this by checking to see if
the parent's device list is completely empty.

Unfortunately, we never hit that code because the parent is considered
an upstream port, and never had an ASPM link_state associated with it.

The early check for !link_state causes us to return early, we never
discover that our device list is empty, and thus we never remove the
downstream ports' link_state nodes.

Instead of checking to see if the parent's device list is empty, we can
check to see if we are the last device on the list, and if so, then we
know that we can clean up properly.

Cc: Shaohua Li <shaohua.li@intel.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/pci/pcie/aspm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 586b6f7..b0367f1 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -718,9 +718,9 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 
 	/*
 	 * All PCIe functions are in one slot, remove one function will remove
-	 * the the whole slot, so just wait
+	 * the whole slot, so just wait until we are the last function left.
 	 */
-	if (!list_empty(&parent->subordinate->devices))
+	if (!list_is_last(&pdev->bus_list, &parent->subordinate->devices))
 		goto out;
 
 	/* All functions are removed, so just disable ASPM for the link */

Re: [RFC PATCH 03/10] PCI: properly clean up ASPM link state on device remove

[Jesse Barnes]

On Wednesday, January 28, 2009 1:59 pm Alex Chiang wrote:
> We only want to disable ASPM when the last function is removed from
> the parent's device list. We determine this by checking to see if
> the parent's device list is completely empty.
>
> Unfortunately, we never hit that code because the parent is considered
> an upstream port, and never had an ASPM link_state associated with it.
>
> The early check for !link_state causes us to return early, we never
> discover that our device list is empty, and thus we never remove the
> downstream ports' link_state nodes.
>
> Instead of checking to see if the parent's device list is empty, we can
> check to see if we are the last device on the list, and if so, then we
> know that we can clean up properly.
>
> Cc: Shaohua Li <shaohua.li@intel.com>
> Signed-off-by: Alex Chiang <achiang@hp.com>

Applied to my for-linus branch, thanks.  Let me know if you have a problem 
with it (fast) Shaohua! :)

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFC PATCH 03/10] PCI: properly clean up ASPM link state on device remove

[Alex Chiang]

* Jesse Barnes <jbarnes@virtuousgeek.org>:
> On Wednesday, January 28, 2009 1:59 pm Alex Chiang wrote:
> > We only want to disable ASPM when the last function is removed from
> > the parent's device list. We determine this by checking to see if
> > the parent's device list is completely empty.
> >
> > Unfortunately, we never hit that code because the parent is considered
> > an upstream port, and never had an ASPM link_state associated with it.
> >
> > The early check for !link_state causes us to return early, we never
> > discover that our device list is empty, and thus we never remove the
> > downstream ports' link_state nodes.
> >
> > Instead of checking to see if the parent's device list is empty, we can
> > check to see if we are the last device on the list, and if so, then we
> > know that we can clean up properly.
> >
> > Cc: Shaohua Li <shaohua.li@intel.com>
> > Signed-off-by: Alex Chiang <achiang@hp.com>
> 
> Applied to my for-linus branch, thanks.  Let me know if you have a problem 
> with it (fast) Shaohua! :)

Thanks Jesse.

I'm cc'ing stable@kernel.org as well (which I should have done in
the first place). This fixes a bug that existed since ASPM
support was added (7d715a6c1), and git name-rev tells me that
we've had it since 2.6.26.

So this patch is a candidate for 2.6.27 and 2.6.28 stable
branches.

Thanks.

/ac

Re: [stable] [RFC PATCH 03/10] PCI: properly clean up ASPM link state on device remove

[Greg KH]

On Wed, Feb 04, 2009 at 06:13:52PM -0700, Alex Chiang wrote:
> * Jesse Barnes <jbarnes@virtuousgeek.org>:
> > On Wednesday, January 28, 2009 1:59 pm Alex Chiang wrote:
> > > We only want to disable ASPM when the last function is removed from
> > > the parent's device list. We determine this by checking to see if
> > > the parent's device list is completely empty.
> > >
> > > Unfortunately, we never hit that code because the parent is considered
> > > an upstream port, and never had an ASPM link_state associated with it.
> > >
> > > The early check for !link_state causes us to return early, we never
> > > discover that our device list is empty, and thus we never remove the
> > > downstream ports' link_state nodes.
> > >
> > > Instead of checking to see if the parent's device list is empty, we can
> > > check to see if we are the last device on the list, and if so, then we
> > > know that we can clean up properly.
> > >
> > > Cc: Shaohua Li <shaohua.li@intel.com>
> > > Signed-off-by: Alex Chiang <achiang@hp.com>
> > 
> > Applied to my for-linus branch, thanks.  Let me know if you have a problem 
> > with it (fast) Shaohua! :)
> 
> Thanks Jesse.
> 
> I'm cc'ing stable@kernel.org as well (which I should have done in
> the first place). This fixes a bug that existed since ASPM
> support was added (7d715a6c1), and git name-rev tells me that
> we've had it since 2.6.26.
> 
> So this patch is a candidate for 2.6.27 and 2.6.28 stable
> branches.

Now queued up.

thanks,

greg k-h

[RFC PATCH 04/10] PCI: Introduce /sys/bus/pci/devices/.../remove

[Alex Chiang]

This patch adds an attribute named "remove" to a PCI device's sysfs
directory.  Writing a non-zero value to this attribute will remove the PCI
device and any of its children.

Code was originally written by Trent Piepho, with a small cleanup and
additional documentation from me.

Cc: Trent Piepho <xyzzy@speakeasy.org>
Cc: djwong@us.ibm.com
Reviewed-by: James Cameron <qz@hp.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 Documentation/ABI/testing/sysfs-bus-pci |    8 ++++++++
 drivers/pci/pci-sysfs.c                 |   30 ++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index ceddcff..faa21de 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -1,3 +1,11 @@
+What:		/sys/bus/pci/devices/.../remove
+Date:		January 2009
+Contact:	Linux PCI developers <linux-pci@vger.kernel.org>
+Description:
+		Writing a non-zero value to this attribute will
+		hot-remove the PCI device and any of its children.
+		Depends on CONFIG_HOTPLUG.
+
 What:		/sys/bus/pci/devices/.../vpd
 Date:		February 2008
 Contact:	Ben Hutchings <bhutchings@solarflare.com>
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index db7ec14..603a32d 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -219,6 +219,33 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+#ifdef CONFIG_HOTPLUG
+static void remove_callback(void *data)
+{
+	pci_remove_bus_device((struct pci_dev *)data);
+}
+
+static ssize_t
+remove_store(struct device *dev, struct device_attribute *dummy,
+	     const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (val)
+		sysfs_schedule_callback(&dev->kobj, remove_callback, pdev,
+		                        THIS_MODULE);
+
+	return count;
+}
+#endif
+
 struct device_attribute pci_dev_attrs[] = {
 	__ATTR_RO(resource),
 	__ATTR_RO(vendor),
@@ -237,6 +264,9 @@ struct device_attribute pci_dev_attrs[] = {
 	__ATTR(broken_parity_status,(S_IRUGO|S_IWUSR),
 		broken_parity_status_show,broken_parity_status_store),
 	__ATTR(msi_bus, 0644, msi_bus_show, msi_bus_store),
+#ifdef CONFIG_HOTPLUG
+	__ATTR(remove, S_IWUSR, NULL, remove_store),
+#endif
 	__ATTR_NULL,
 };
 

[RFC PATCH 05/10] PCI: Introduce /sys/bus/pci/rescan

[Alex Chiang]

This interface allows the user to force a rescan of all PCI buses
in system, and rediscover devices that have been removed earlier.

Cc: Trent Piepho <xyzzy@speakeasy.org>
Cc: djwong@us.ibm.com
Reviewed-by: James Cameron <qz@hp.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 Documentation/ABI/testing/sysfs-bus-pci |    9 +++++++++
 drivers/pci/pci-sysfs.c                 |   28 ++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index faa21de..8c0abc7 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -1,3 +1,12 @@
+What:		/sys/bus/pci/rescan
+Date:		January 2009
+Contact:	Linux PCI developers <linux-pci@vger.kernel.org>
+Description:
+		Writing a non-zero value to this attribute will
+		force a rescan of all PCI buses in the system, and
+		re-discover previously removed devices.
+		Depends on CONFIG_HOTPLUG.
+
 What:		/sys/bus/pci/devices/.../remove
 Date:		January 2009
 Contact:	Linux PCI developers <linux-pci@vger.kernel.org>
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 603a32d..c300242 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -986,12 +986,40 @@ void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
 	}
 }
 
+#ifdef CONFIG_HOTPLUG
+static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf, size_t count)
+{
+	unsigned long val;
+	struct pci_bus *b = NULL;
+
+	if (strict_strtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (val)
+		while ((b = pci_find_next_bus(b)) != NULL) {
+			pci_scan_child_bus(b);
+			pci_bus_add_devices(b);
+		}
+
+	return count;
+}
+static BUS_ATTR(rescan, S_IWUSR, NULL, bus_rescan_store);
+#endif
+
 static int __init pci_sysfs_init(void)
 {
 	struct pci_dev *pdev = NULL;
 	int retval;
 
 	sysfs_initialized = 1;
+#ifdef CONFIG_HOTPLUG
+	retval = bus_create_file(&pci_bus_type, &bus_attr_rescan);
+	if (retval)
+		return retval;
+#endif
 	for_each_pci_dev(pdev) {
 		retval = pci_create_sysfs_dev_files(pdev);
 		if (retval) {

[RFC PATCH 06/10] PCI: Introduce /sys/bus/pci/devices/.../rescan

[Alex Chiang]

This interface allows the user to force a rescan of the device's
parent bus and all subordinate buses, and rediscover devices removed
earlier from this part of the device tree.

Cc: Trent Piepho <xyzzy@speakeasy.org>
Cc: djwong@us.ibm.com
Reviewed-by: James Cameron <qz@hp.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 Documentation/ABI/testing/sysfs-bus-pci |   10 ++++++++++
 drivers/pci/pci-sysfs.c                 |   22 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 8c0abc7..f59e963 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -15,6 +15,16 @@ Description:
 		hot-remove the PCI device and any of its children.
 		Depends on CONFIG_HOTPLUG.
 
+What:		/sys/bus/pci/devices/.../rescan
+Date:		January 2009
+Contact:	Linux PCI developers <linux-pci@vger.kernel.org>
+Description:
+		Writing a non-zero value to this attribute will
+		force a rescan of the device's parent bus and all
+		child buses, and re-discover devices removed earlier
+		from this part of the device tree.
+		Depends on CONFIG_HOTPLUG.
+
 What:		/sys/bus/pci/devices/.../vpd
 Date:		February 2008
 Contact:	Ben Hutchings <bhutchings@solarflare.com>
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index c300242..dcbc8bc 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -220,6 +220,27 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
 }
 
 #ifdef CONFIG_HOTPLUG
+static ssize_t
+dev_rescan_store(struct device *dev, struct device_attribute *attr,
+	      const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (val) {
+		pci_scan_child_bus(pdev->bus);
+		pci_bus_add_devices(pdev->bus);
+	}
+
+	return count;
+}
+
 static void remove_callback(void *data)
 {
 	pci_remove_bus_device((struct pci_dev *)data);
@@ -266,6 +287,7 @@ struct device_attribute pci_dev_attrs[] = {
 	__ATTR(msi_bus, 0644, msi_bus_show, msi_bus_store),
 #ifdef CONFIG_HOTPLUG
 	__ATTR(remove, S_IWUSR, NULL, remove_store),
+	__ATTR(rescan, S_IWUSR, NULL, dev_rescan_store),
 #endif
 	__ATTR_NULL,
 };

[RFC PATCH 07/10] PCI Hotplug: restore fakephp interface with complete reimplementation

[Alex Chiang]

From: Trent Piepho <xyzzy@speakeasy.org>

A complete re-implementation of fakephp is necessary if it is to
present its former interface (pre-2.6.27, when it broke). The
reason is that PCI hotplug drivers call pci_hp_register(), which
enforces the rule that only one /sys/bus/pci/slots/ file may be
created per physical slot.

The change breaks the old fakephp's assumption that it could
create a file per function. So we re-implement fakephp to avoid
using the standard PCI hotplug API so that we can restore the old
fakephp user interface.

It puts entries in /sys/bus/pci/slots with the names of all PCI
devices/functions, exactly symmetrical to what is shown in
/sys/bus/pci/devices. Each slots/ entry has a "power" attribute,
which works the same way as the fakephp driver's power attribute
has worked.

There are a few improvements over old fakephp, which couldn't handle
PCI devices being added or removed via a means outside of
fakephp's knowledge.  If a device was added another way, old fakephp
didn't notice and didn't create the fake slot for it.  If a
device was removed another way, old fakephp didn't delete the fake
slot for it (and accessing the stale slot caused an oops).

The new implementation overcomes these limitations. As a
consequence, removing a bridge with other devices behind it now
works as well, which is something else old fakephp couldn't do
previously.

This duplicates a tiny bit of the code in the PCI core that does
this same function.  Re-using that code ends up being more
complex than duplicating it, and it makes code in the PCI core
more ugly just to support this legacy fakephp interface
compatibility layer.

Reviewed-by: James Cameron <qz@hp.com>
Signed-off-by: Trent Piepho <xyzzy@speakeasy.org>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/pci/hotplug/Makefile         |    2 
 drivers/pci/hotplug/fakephp.c        |  385 ----------------------------------
 drivers/pci/hotplug/legacy_fakephp.c |  163 ++++++++++++++
 3 files changed, 164 insertions(+), 386 deletions(-)
 delete mode 100644 drivers/pci/hotplug/fakephp.c
 create mode 100644 drivers/pci/hotplug/legacy_fakephp.c

diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
index e31fb91..5a86728 100644
--- a/drivers/pci/hotplug/Makefile
+++ b/drivers/pci/hotplug/Makefile
@@ -16,7 +16,7 @@ obj-$(CONFIG_HOTPLUG_PCI_RPA_DLPAR)	+= rpadlpar_io.o
 obj-$(CONFIG_HOTPLUG_PCI_SGI)		+= sgi_hotplug.o
 
 # Link this last so it doesn't claim devices that have a real hotplug driver
-obj-$(CONFIG_HOTPLUG_PCI_FAKE)		+= fakephp.o
+obj-$(CONFIG_HOTPLUG_PCI_FAKE)		+= legacy_fakephp.o
 
 pci_hotplug-objs	:=	pci_hotplug_core.o
 
diff --git a/drivers/pci/hotplug/fakephp.c b/drivers/pci/hotplug/fakephp.c
deleted file mode 100644
index b0e7de9..0000000
--- a/drivers/pci/hotplug/fakephp.c
+++ /dev/null
@@ -1,385 +0,0 @@
-/*
- * Fake PCI Hot Plug Controller Driver
- *
- * Copyright (C) 2003 Greg Kroah-Hartman <greg@kroah.com>
- * Copyright (C) 2003 IBM Corp.
- * Copyright (C) 2003 Rolf Eike Beer <eike-kernel@sf-tec.de>
- *
- * Based on ideas and code from:
- * 	Vladimir Kondratiev <vladimir.kondratiev@intel.com>
- *	Rolf Eike Beer <eike-kernel@sf-tec.de>
- *
- * All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, version 2 of the License.
- *
- * Send feedback to <greg@kroah.com>
- */
-
-/*
- *
- * This driver will "emulate" removing PCI devices from the system.  If
- * the "power" file is written to with "0" then the specified PCI device
- * will be completely removed from the kernel.
- *
- * WARNING, this does NOT turn off the power to the PCI device.  This is
- * a "logical" removal, not a physical or electrical removal.
- *
- * Use this module at your own risk, you have been warned!
- *
- * Enabling PCI devices is left as an exercise for the reader...
- *
- */
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/pci_hotplug.h>
-#include <linux/init.h>
-#include <linux/string.h>
-#include <linux/slab.h>
-#include <linux/workqueue.h>
-#include "../pci.h"
-
-#if !defined(MODULE)
-	#define MY_NAME	"fakephp"
-#else
-	#define MY_NAME	THIS_MODULE->name
-#endif
-
-#define dbg(format, arg...)					\
-	do {							\
-		if (debug)					\
-			printk(KERN_DEBUG "%s: " format,	\
-				MY_NAME , ## arg); 		\
-	} while (0)
-#define err(format, arg...) printk(KERN_ERR "%s: " format, MY_NAME , ## arg)
-#define info(format, arg...) printk(KERN_INFO "%s: " format, MY_NAME , ## arg)
-
-#define DRIVER_AUTHOR	"Greg Kroah-Hartman <greg@kroah.com>"
-#define DRIVER_DESC	"Fake PCI Hot Plug Controller Driver"
-
-struct dummy_slot {
-	struct list_head node;
-	struct hotplug_slot *slot;
-	struct pci_dev *dev;
-	struct work_struct remove_work;
-	unsigned long removed;
-};
-
-static int debug;
-static int dup_slots;
-static LIST_HEAD(slot_list);
-static struct workqueue_struct *dummyphp_wq;
-
-static void pci_rescan_worker(struct work_struct *work);
-static DECLARE_WORK(pci_rescan_work, pci_rescan_worker);
-
-static int enable_slot (struct hotplug_slot *slot);
-static int disable_slot (struct hotplug_slot *slot);
-
-static struct hotplug_slot_ops dummy_hotplug_slot_ops = {
-	.owner			= THIS_MODULE,
-	.enable_slot		= enable_slot,
-	.disable_slot		= disable_slot,
-};
-
-static void dummy_release(struct hotplug_slot *slot)
-{
-	struct dummy_slot *dslot = slot->private;
-
-	list_del(&dslot->node);
-	kfree(dslot->slot->info);
-	kfree(dslot->slot);
-	pci_dev_put(dslot->dev);
-	kfree(dslot);
-}
-
-#define SLOT_NAME_SIZE	8
-
-static int add_slot(struct pci_dev *dev)
-{
-	struct dummy_slot *dslot;
-	struct hotplug_slot *slot;
-	char name[SLOT_NAME_SIZE];
-	int retval = -ENOMEM;
-	static int count = 1;
-
-	slot = kzalloc(sizeof(struct hotplug_slot), GFP_KERNEL);
-	if (!slot)
-		goto error;
-
-	slot->info = kzalloc(sizeof(struct hotplug_slot_info), GFP_KERNEL);
-	if (!slot->info)
-		goto error_slot;
-
-	slot->info->power_status = 1;
-	slot->info->max_bus_speed = PCI_SPEED_UNKNOWN;
-	slot->info->cur_bus_speed = PCI_SPEED_UNKNOWN;
-
-	dslot = kzalloc(sizeof(struct dummy_slot), GFP_KERNEL);
-	if (!dslot)
-		goto error_info;
-
-	if (dup_slots)
-		snprintf(name, SLOT_NAME_SIZE, "fake");
-	else
-		snprintf(name, SLOT_NAME_SIZE, "fake%d", count++);
-	dbg("slot->name = %s\n", name);
-	slot->ops = &dummy_hotplug_slot_ops;
-	slot->release = &dummy_release;
-	slot->private = dslot;
-
-	retval = pci_hp_register(slot, dev->bus, PCI_SLOT(dev->devfn), name);
-	if (retval) {
-		err("pci_hp_register failed with error %d\n", retval);
-		goto error_dslot;
-	}
-
-	dbg("slot->name = %s\n", hotplug_slot_name(slot));
-	dslot->slot = slot;
-	dslot->dev = pci_dev_get(dev);
-	list_add (&dslot->node, &slot_list);
-	return retval;
-
-error_dslot:
-	kfree(dslot);
-error_info:
-	kfree(slot->info);
-error_slot:
-	kfree(slot);
-error:
-	return retval;
-}
-
-static int __init pci_scan_buses(void)
-{
-	struct pci_dev *dev = NULL;
-	int lastslot = 0;
-
-	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
-		if (PCI_FUNC(dev->devfn) > 0 &&
-				lastslot == PCI_SLOT(dev->devfn))
-			continue;
-		lastslot = PCI_SLOT(dev->devfn);
-		add_slot(dev);
-	}
-
-	return 0;
-}
-
-static void remove_slot(struct dummy_slot *dslot)
-{
-	int retval;
-
-	dbg("removing slot %s\n", hotplug_slot_name(dslot->slot));
-	retval = pci_hp_deregister(dslot->slot);
-	if (retval)
-		err("Problem unregistering a slot %s\n",
-			hotplug_slot_name(dslot->slot));
-}
-
-/* called from the single-threaded workqueue handler to remove a slot */
-static void remove_slot_worker(struct work_struct *work)
-{
-	struct dummy_slot *dslot =
-		container_of(work, struct dummy_slot, remove_work);
-	remove_slot(dslot);
-}
-
-/**
- * pci_rescan_slot - Rescan slot
- * @temp: Device template. Should be set: bus and devfn.
- *
- * Tries hard not to re-enable already existing devices;
- * also handles scanning of subfunctions.
- */
-static void pci_rescan_slot(struct pci_dev *temp)
-{
-	struct pci_bus *bus = temp->bus;
-	struct pci_dev *dev;
-	int func;
-	int retval;
-	u8 hdr_type;
-
-	if (!pci_read_config_byte(temp, PCI_HEADER_TYPE, &hdr_type)) {
-		temp->hdr_type = hdr_type & 0x7f;
-		if ((dev = pci_get_slot(bus, temp->devfn)) != NULL)
-			pci_dev_put(dev);
-		else {
-			dev = pci_scan_single_device(bus, temp->devfn);
-			if (dev) {
-				dbg("New device on %s function %x:%x\n",
-					bus->name, temp->devfn >> 3,
-					temp->devfn & 7);
-				retval = pci_bus_add_device(dev);
-				if (retval)
-					dev_err(&dev->dev, "error adding "
-						"device, continuing.\n");
-				else
-					add_slot(dev);
-			}
-		}
-		/* multifunction device? */
-		if (!(hdr_type & 0x80))
-			return;
-
-		/* continue scanning for other functions */
-		for (func = 1, temp->devfn++; func < 8; func++, temp->devfn++) {
-			if (pci_read_config_byte(temp, PCI_HEADER_TYPE, &hdr_type))
-				continue;
-			temp->hdr_type = hdr_type & 0x7f;
-
-			if ((dev = pci_get_slot(bus, temp->devfn)) != NULL)
-				pci_dev_put(dev);
-			else {
-				dev = pci_scan_single_device(bus, temp->devfn);
-				if (dev) {
-					dbg("New device on %s function %x:%x\n",
-						bus->name, temp->devfn >> 3,
-						temp->devfn & 7);
-					retval = pci_bus_add_device(dev);
-					if (retval)
-						dev_err(&dev->dev, "error adding "
-							"device, continuing.\n");
-					else
-						add_slot(dev);
-				}
-			}
-		}
-	}
-}
-
-
-/**
- * pci_rescan_bus - Rescan PCI bus
- * @bus: the PCI bus to rescan
- *
- * Call pci_rescan_slot for each possible function of the bus.
- */
-static void pci_rescan_bus(const struct pci_bus *bus)
-{
-	unsigned int devfn;
-	struct pci_dev *dev;
-	dev = alloc_pci_dev();
-	if (!dev)
-		return;
-
-	dev->bus = (struct pci_bus*)bus;
-	dev->sysdata = bus->sysdata;
-	for (devfn = 0; devfn < 0x100; devfn += 8) {
-		dev->devfn = devfn;
-		pci_rescan_slot(dev);
-	}
-	kfree(dev);
-}
-
-/* recursively scan all buses */
-static void pci_rescan_buses(const struct list_head *list)
-{
-	const struct list_head *l;
-	list_for_each(l,list) {
-		const struct pci_bus *b = pci_bus_b(l);
-		pci_rescan_bus(b);
-		pci_rescan_buses(&b->children);
-	}
-}
-
-/* initiate rescan of all pci buses */
-static inline void pci_rescan(void) {
-	pci_rescan_buses(&pci_root_buses);
-}
-
-/* called from the single-threaded workqueue handler to rescan all pci buses */
-static void pci_rescan_worker(struct work_struct *work)
-{
-	pci_rescan();
-}
-
-static int enable_slot(struct hotplug_slot *hotplug_slot)
-{
-	/* mis-use enable_slot for rescanning of the pci bus */
-	cancel_work_sync(&pci_rescan_work);
-	queue_work(dummyphp_wq, &pci_rescan_work);
-	return 0;
-}
-
-static int disable_slot(struct hotplug_slot *slot)
-{
-	struct dummy_slot *dslot;
-	struct pci_dev *dev;
-	int func;
-
-	if (!slot)
-		return -ENODEV;
-	dslot = slot->private;
-
-	dbg("%s - physical_slot = %s\n", __func__, hotplug_slot_name(slot));
-
-	for (func = 7; func >= 0; func--) {
-		dev = pci_get_slot(dslot->dev->bus, dslot->dev->devfn + func);
-		if (!dev)
-			continue;
-
-		if (test_and_set_bit(0, &dslot->removed)) {
-			dbg("Slot already scheduled for removal\n");
-			pci_dev_put(dev);
-			return -ENODEV;
-		}
-
-		/* remove the device from the pci core */
-		pci_remove_bus_device(dev);
-
-		/* queue work item to blow away this sysfs entry and other
-		 * parts.
-		 */
-		INIT_WORK(&dslot->remove_work, remove_slot_worker);
-		queue_work(dummyphp_wq, &dslot->remove_work);
-
-		pci_dev_put(dev);
-	}
-	return 0;
-}
-
-static void cleanup_slots (void)
-{
-	struct list_head *tmp;
-	struct list_head *next;
-	struct dummy_slot *dslot;
-
-	destroy_workqueue(dummyphp_wq);
-	list_for_each_safe (tmp, next, &slot_list) {
-		dslot = list_entry (tmp, struct dummy_slot, node);
-		remove_slot(dslot);
-	}
-	
-}
-
-static int __init dummyphp_init(void)
-{
-	info(DRIVER_DESC "\n");
-
-	dummyphp_wq = create_singlethread_workqueue(MY_NAME);
-	if (!dummyphp_wq)
-		return -ENOMEM;
-
-	return pci_scan_buses();
-}
-
-
-static void __exit dummyphp_exit(void)
-{
-	cleanup_slots();
-}
-
-module_init(dummyphp_init);
-module_exit(dummyphp_exit);
-
-MODULE_AUTHOR(DRIVER_AUTHOR);
-MODULE_DESCRIPTION(DRIVER_DESC);
-MODULE_LICENSE("GPL");
-module_param(debug, bool, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(debug, "Debugging mode enabled or not");
-module_param(dup_slots, bool, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(dup_slots, "Force duplicate slot names for debugging");
diff --git a/drivers/pci/hotplug/legacy_fakephp.c b/drivers/pci/hotplug/legacy_fakephp.c
new file mode 100644
index 0000000..185cb67
--- /dev/null
+++ b/drivers/pci/hotplug/legacy_fakephp.c
@@ -0,0 +1,163 @@
+/* Works like the fakephp driver used to, except a little better.
+ *
+ * - It's possible to remove devices with subordinate busses.
+ * - New PCI devices that appear via any method, not just a fakephp triggered
+ *   rescan, will be noticed.
+ * - Devices that are removed via any method, not just a fakephp triggered
+ *   removal, will also be noticed.
+ *
+ * Uses nothing from the pci-hotplug subsystem.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/list.h>
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include "../pci.h"
+
+struct legacy_slot {
+	struct kobject		kobj;
+	struct pci_dev		*dev;
+	struct list_head	list;
+};
+
+static LIST_HEAD(legacy_list);
+
+static ssize_t legacy_show(struct kobject *kobj, struct attribute *attr,
+			   char *buf)
+{
+	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
+	strcpy(buf, "1\n");
+	return 2;
+}
+
+static void remove_callback(void *data)
+{
+	pci_remove_bus_device((struct pci_dev *)data);
+}
+
+static ssize_t legacy_store(struct kobject *kobj, struct attribute *attr,
+			    const char *buf, size_t len)
+{
+	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (val) {
+		pci_scan_child_bus(slot->dev->bus);
+		pci_bus_add_devices(slot->dev->bus);
+	} else
+		sysfs_schedule_callback(&slot->dev->dev.kobj, remove_callback,
+		                        slot->dev, THIS_MODULE);
+	return len;
+}
+
+static struct attribute *legacy_attrs[] = {
+	&(struct attribute){ .name = "power", .mode = 0644 },
+	NULL,
+};
+
+static void legacy_release(struct kobject *kobj)
+{
+	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
+
+	pci_dev_put(slot->dev);
+	kfree(slot);
+}
+
+static struct kobj_type legacy_ktype = {
+	.sysfs_ops = &(struct sysfs_ops){
+		.store = legacy_store, .show = legacy_show
+	},
+	.release = &legacy_release,
+	.default_attrs = legacy_attrs,
+};
+
+static int legacy_add_slot(struct pci_dev *pdev)
+{
+	struct legacy_slot *slot = kzalloc(sizeof(*slot), GFP_KERNEL);
+
+	if (!slot)
+		return -ENOMEM;
+
+	if (kobject_init_and_add(&slot->kobj, &legacy_ktype,
+				 &pci_slots_kset->kobj, "%s",
+				 pdev->dev.bus_id)) {
+		dev_warn(&pdev->dev, "Failed to created legacy fake slot\n");
+		return -EINVAL;
+	}
+	slot->dev = pci_dev_get(pdev);
+
+	list_add(&slot->list, &legacy_list);
+
+	return 0;
+}
+
+static int legacy_notify(struct notifier_block *nb,
+			 unsigned long action, void *data)
+{
+        struct pci_dev *pdev = to_pci_dev(data);
+
+	if (action == BUS_NOTIFY_ADD_DEVICE) {
+		legacy_add_slot(pdev);
+	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
+		struct legacy_slot *slot;
+
+		list_for_each_entry(slot, &legacy_list, list)
+			if (slot->dev == pdev)
+				goto found;
+
+		dev_warn(&pdev->dev, "Missing legacy fake slot?");
+		return -ENODEV;
+found:
+		kobject_del(&slot->kobj);
+		list_del(&slot->list);
+		kobject_put(&slot->kobj);
+	}
+
+	return 0;
+}
+
+static struct notifier_block legacy_notifier = {
+	.notifier_call = legacy_notify
+};
+
+static int __init init_legacy(void)
+{
+	struct pci_dev *pdev = NULL;
+
+	/* Add existing devices */
+	while ((pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pdev)))
+		legacy_add_slot(pdev);
+
+	/* Be alerted of any new ones */
+	bus_register_notifier(&pci_bus_type, &legacy_notifier);
+	return 0;
+}
+module_init(init_legacy);
+
+static void __exit remove_legacy(void)
+{
+	struct legacy_slot *slot, *tmp;
+
+	bus_unregister_notifier(&pci_bus_type, &legacy_notifier);
+
+	list_for_each_entry_safe(slot, tmp, &legacy_list, list) {
+		list_del(&slot->list);
+		kobject_del(&slot->kobj);
+		kobject_put(&slot->kobj);
+	}
+}
+module_exit(remove_legacy);
+
+
+MODULE_AUTHOR("Trent Piepho <xyzzy@speakeasy.org>");
+MODULE_DESCRIPTION("Legacy version of the fakephp interface");
+MODULE_LICENSE("GPL");

[RFC PATCH 08/10] PCI Hotplug: rename legacy_fakephp to fakephp

[Alex Chiang]

We wanted to replace fakephp wholesale, so rename legacy_fakephp back
to fakephp. Yes, this is a silly commit, but it produces a much easier
patch to read and review.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/pci/hotplug/Makefile         |    2 
 drivers/pci/hotplug/fakephp.c        |  163 ++++++++++++++++++++++++++++++++++
 drivers/pci/hotplug/legacy_fakephp.c |  163 ----------------------------------
 3 files changed, 164 insertions(+), 164 deletions(-)
 create mode 100644 drivers/pci/hotplug/fakephp.c
 delete mode 100644 drivers/pci/hotplug/legacy_fakephp.c

diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
index 5a86728..e31fb91 100644
--- a/drivers/pci/hotplug/Makefile
+++ b/drivers/pci/hotplug/Makefile
@@ -16,7 +16,7 @@ obj-$(CONFIG_HOTPLUG_PCI_RPA_DLPAR)	+= rpadlpar_io.o
 obj-$(CONFIG_HOTPLUG_PCI_SGI)		+= sgi_hotplug.o
 
 # Link this last so it doesn't claim devices that have a real hotplug driver
-obj-$(CONFIG_HOTPLUG_PCI_FAKE)		+= legacy_fakephp.o
+obj-$(CONFIG_HOTPLUG_PCI_FAKE)		+= fakephp.o
 
 pci_hotplug-objs	:=	pci_hotplug_core.o
 
diff --git a/drivers/pci/hotplug/fakephp.c b/drivers/pci/hotplug/fakephp.c
new file mode 100644
index 0000000..185cb67
--- /dev/null
+++ b/drivers/pci/hotplug/fakephp.c
@@ -0,0 +1,163 @@
+/* Works like the fakephp driver used to, except a little better.
+ *
+ * - It's possible to remove devices with subordinate busses.
+ * - New PCI devices that appear via any method, not just a fakephp triggered
+ *   rescan, will be noticed.
+ * - Devices that are removed via any method, not just a fakephp triggered
+ *   removal, will also be noticed.
+ *
+ * Uses nothing from the pci-hotplug subsystem.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/list.h>
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include "../pci.h"
+
+struct legacy_slot {
+	struct kobject		kobj;
+	struct pci_dev		*dev;
+	struct list_head	list;
+};
+
+static LIST_HEAD(legacy_list);
+
+static ssize_t legacy_show(struct kobject *kobj, struct attribute *attr,
+			   char *buf)
+{
+	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
+	strcpy(buf, "1\n");
+	return 2;
+}
+
+static void remove_callback(void *data)
+{
+	pci_remove_bus_device((struct pci_dev *)data);
+}
+
+static ssize_t legacy_store(struct kobject *kobj, struct attribute *attr,
+			    const char *buf, size_t len)
+{
+	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (val) {
+		pci_scan_child_bus(slot->dev->bus);
+		pci_bus_add_devices(slot->dev->bus);
+	} else
+		sysfs_schedule_callback(&slot->dev->dev.kobj, remove_callback,
+		                        slot->dev, THIS_MODULE);
+	return len;
+}
+
+static struct attribute *legacy_attrs[] = {
+	&(struct attribute){ .name = "power", .mode = 0644 },
+	NULL,
+};
+
+static void legacy_release(struct kobject *kobj)
+{
+	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
+
+	pci_dev_put(slot->dev);
+	kfree(slot);
+}
+
+static struct kobj_type legacy_ktype = {
+	.sysfs_ops = &(struct sysfs_ops){
+		.store = legacy_store, .show = legacy_show
+	},
+	.release = &legacy_release,
+	.default_attrs = legacy_attrs,
+};
+
+static int legacy_add_slot(struct pci_dev *pdev)
+{
+	struct legacy_slot *slot = kzalloc(sizeof(*slot), GFP_KERNEL);
+
+	if (!slot)
+		return -ENOMEM;
+
+	if (kobject_init_and_add(&slot->kobj, &legacy_ktype,
+				 &pci_slots_kset->kobj, "%s",
+				 pdev->dev.bus_id)) {
+		dev_warn(&pdev->dev, "Failed to created legacy fake slot\n");
+		return -EINVAL;
+	}
+	slot->dev = pci_dev_get(pdev);
+
+	list_add(&slot->list, &legacy_list);
+
+	return 0;
+}
+
+static int legacy_notify(struct notifier_block *nb,
+			 unsigned long action, void *data)
+{
+        struct pci_dev *pdev = to_pci_dev(data);
+
+	if (action == BUS_NOTIFY_ADD_DEVICE) {
+		legacy_add_slot(pdev);
+	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
+		struct legacy_slot *slot;
+
+		list_for_each_entry(slot, &legacy_list, list)
+			if (slot->dev == pdev)
+				goto found;
+
+		dev_warn(&pdev->dev, "Missing legacy fake slot?");
+		return -ENODEV;
+found:
+		kobject_del(&slot->kobj);
+		list_del(&slot->list);
+		kobject_put(&slot->kobj);
+	}
+
+	return 0;
+}
+
+static struct notifier_block legacy_notifier = {
+	.notifier_call = legacy_notify
+};
+
+static int __init init_legacy(void)
+{
+	struct pci_dev *pdev = NULL;
+
+	/* Add existing devices */
+	while ((pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pdev)))
+		legacy_add_slot(pdev);
+
+	/* Be alerted of any new ones */
+	bus_register_notifier(&pci_bus_type, &legacy_notifier);
+	return 0;
+}
+module_init(init_legacy);
+
+static void __exit remove_legacy(void)
+{
+	struct legacy_slot *slot, *tmp;
+
+	bus_unregister_notifier(&pci_bus_type, &legacy_notifier);
+
+	list_for_each_entry_safe(slot, tmp, &legacy_list, list) {
+		list_del(&slot->list);
+		kobject_del(&slot->kobj);
+		kobject_put(&slot->kobj);
+	}
+}
+module_exit(remove_legacy);
+
+
+MODULE_AUTHOR("Trent Piepho <xyzzy@speakeasy.org>");
+MODULE_DESCRIPTION("Legacy version of the fakephp interface");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pci/hotplug/legacy_fakephp.c b/drivers/pci/hotplug/legacy_fakephp.c
deleted file mode 100644
index 185cb67..0000000
--- a/drivers/pci/hotplug/legacy_fakephp.c
+++ /dev/null
@@ -1,163 +0,0 @@
-/* Works like the fakephp driver used to, except a little better.
- *
- * - It's possible to remove devices with subordinate busses.
- * - New PCI devices that appear via any method, not just a fakephp triggered
- *   rescan, will be noticed.
- * - Devices that are removed via any method, not just a fakephp triggered
- *   removal, will also be noticed.
- *
- * Uses nothing from the pci-hotplug subsystem.
- *
- */
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/types.h>
-#include <linux/list.h>
-#include <linux/kobject.h>
-#include <linux/sysfs.h>
-#include <linux/init.h>
-#include <linux/pci.h>
-#include "../pci.h"
-
-struct legacy_slot {
-	struct kobject		kobj;
-	struct pci_dev		*dev;
-	struct list_head	list;
-};
-
-static LIST_HEAD(legacy_list);
-
-static ssize_t legacy_show(struct kobject *kobj, struct attribute *attr,
-			   char *buf)
-{
-	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
-	strcpy(buf, "1\n");
-	return 2;
-}
-
-static void remove_callback(void *data)
-{
-	pci_remove_bus_device((struct pci_dev *)data);
-}
-
-static ssize_t legacy_store(struct kobject *kobj, struct attribute *attr,
-			    const char *buf, size_t len)
-{
-	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
-	unsigned long val;
-
-	if (strict_strtoul(buf, 0, &val) < 0)
-		return -EINVAL;
-
-	if (val) {
-		pci_scan_child_bus(slot->dev->bus);
-		pci_bus_add_devices(slot->dev->bus);
-	} else
-		sysfs_schedule_callback(&slot->dev->dev.kobj, remove_callback,
-		                        slot->dev, THIS_MODULE);
-	return len;
-}
-
-static struct attribute *legacy_attrs[] = {
-	&(struct attribute){ .name = "power", .mode = 0644 },
-	NULL,
-};
-
-static void legacy_release(struct kobject *kobj)
-{
-	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
-
-	pci_dev_put(slot->dev);
-	kfree(slot);
-}
-
-static struct kobj_type legacy_ktype = {
-	.sysfs_ops = &(struct sysfs_ops){
-		.store = legacy_store, .show = legacy_show
-	},
-	.release = &legacy_release,
-	.default_attrs = legacy_attrs,
-};
-
-static int legacy_add_slot(struct pci_dev *pdev)
-{
-	struct legacy_slot *slot = kzalloc(sizeof(*slot), GFP_KERNEL);
-
-	if (!slot)
-		return -ENOMEM;
-
-	if (kobject_init_and_add(&slot->kobj, &legacy_ktype,
-				 &pci_slots_kset->kobj, "%s",
-				 pdev->dev.bus_id)) {
-		dev_warn(&pdev->dev, "Failed to created legacy fake slot\n");
-		return -EINVAL;
-	}
-	slot->dev = pci_dev_get(pdev);
-
-	list_add(&slot->list, &legacy_list);
-
-	return 0;
-}
-
-static int legacy_notify(struct notifier_block *nb,
-			 unsigned long action, void *data)
-{
-        struct pci_dev *pdev = to_pci_dev(data);
-
-	if (action == BUS_NOTIFY_ADD_DEVICE) {
-		legacy_add_slot(pdev);
-	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
-		struct legacy_slot *slot;
-
-		list_for_each_entry(slot, &legacy_list, list)
-			if (slot->dev == pdev)
-				goto found;
-
-		dev_warn(&pdev->dev, "Missing legacy fake slot?");
-		return -ENODEV;
-found:
-		kobject_del(&slot->kobj);
-		list_del(&slot->list);
-		kobject_put(&slot->kobj);
-	}
-
-	return 0;
-}
-
-static struct notifier_block legacy_notifier = {
-	.notifier_call = legacy_notify
-};
-
-static int __init init_legacy(void)
-{
-	struct pci_dev *pdev = NULL;
-
-	/* Add existing devices */
-	while ((pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pdev)))
-		legacy_add_slot(pdev);
-
-	/* Be alerted of any new ones */
-	bus_register_notifier(&pci_bus_type, &legacy_notifier);
-	return 0;
-}
-module_init(init_legacy);
-
-static void __exit remove_legacy(void)
-{
-	struct legacy_slot *slot, *tmp;
-
-	bus_unregister_notifier(&pci_bus_type, &legacy_notifier);
-
-	list_for_each_entry_safe(slot, tmp, &legacy_list, list) {
-		list_del(&slot->list);
-		kobject_del(&slot->kobj);
-		kobject_put(&slot->kobj);
-	}
-}
-module_exit(remove_legacy);
-
-
-MODULE_AUTHOR("Trent Piepho <xyzzy@speakeasy.org>");
-MODULE_DESCRIPTION("Legacy version of the fakephp interface");
-MODULE_LICENSE("GPL");

[RFC PATCH 09/10] PCI Hotplug: schedule fakephp for feature removal

[Alex Chiang]

Now that the PCI core is capable of function-level remove and rescan
as well as bus-level rescan, there's no functional need to keep fakephp
anymore.

We keep it around for userspace compatibility reasons, schedule removal
in three years.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 Documentation/feature-removal-schedule.txt |   32 ++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 5ddbe35..fc3505b 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -335,3 +335,35 @@ Why:	In 2.6.18 the Secmark concept was introduced to replace the "compat_net"
 	Secmark, it is time to deprecate the older mechanism and start the
 	process of removing the old code.
 Who:	Paul Moore <paul.moore@hp.com>
+
+---------------------------
+
+What:	fakephp and associated sysfs files in /sys/bus/pci/slots/
+When:	2011
+Why:	In 2.6.27, the semantics of /sys/bus/pci/slots was redefined to
+	represent a machine's physical PCI slots. The change in semantics
+	had userspace implications, as the hotplug core no longer allowed
+	drivers to create multiple sysfs files per physical slot (required
+	for multi-function devices, e.g.). fakephp was seen as a developer's
+	tool only, and its interface changed. Too late, we learned that
+	there were some users of the fakephp interface.
+
+	In 2.6.30, the original fakephp interface was restored. At the same
+	time, the PCI core gained the ability that fakephp provided, namely
+	function-level hot-remove and hot-add.
+
+	Since the PCI core now provides the same functionality, exposed in:
+
+		/sys/bus/pci/rescan
+		/sys/bus/pci/devices/.../remove
+		/sys/bus/pci/devices/.../rescan
+
+	there is no functional reason to maintain fakephp as well.
+
+	We will keep the existing module so that 'modprobe fakephp' will
+	present the old /sys/bus/pci/slots/... interface for compatibility,
+	but users are urged to migrate their applications to the API above.
+
+	After a reasonable transition period, we will remove the legacy
+	fakephp interface.
+Who:	Alex Chiang <achiang@hp.com>

[RFC PATCH 10/10] PCI: more whitespace cleanups

[Alex Chiang]

Clean up some stray whitespace issues.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/pci/pci-sysfs.c |   18 +++++++++---------
 drivers/pci/probe.c     |   24 +++++++++++++-----------
 drivers/pci/remove.c    |    4 ++--
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index dcbc8bc..adc6b86 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -10,7 +10,7 @@
  *
  * File attributes for PCI devices
  *
- * Modeled after usb's driverfs.c 
+ * Modeled after usb's driverfs.c
  *
  */
 
@@ -70,7 +70,7 @@ static ssize_t broken_parity_status_store(struct device *dev,
 
 static ssize_t local_cpus_show(struct device *dev,
 			struct device_attribute *attr, char *buf)
-{		
+{
 	const struct cpumask *mask;
 	int len;
 
@@ -380,13 +380,13 @@ pci_write_config(struct kobject *kobj, struct bin_attribute *bin_attr,
 		size = dev->cfg_size - off;
 		count = size;
 	}
-	
+
 	if ((off & 1) && size) {
 		pci_user_write_config_byte(dev, off, data[off - init_off]);
 		off++;
 		size--;
 	}
-	
+
 	if ((off & 3) && size > 2) {
 		u16 val = data[off - init_off];
 		val |= (u16) data[off - init_off + 1] << 8;
@@ -404,7 +404,7 @@ pci_write_config(struct kobject *kobj, struct bin_attribute *bin_attr,
 		off += 4;
 		size -= 4;
 	}
-	
+
 	if (size >= 2) {
 		u16 val = data[off - init_off];
 		val |= (u16) data[off - init_off + 1] << 8;
@@ -818,21 +818,21 @@ pci_read_rom(struct kobject *kobj, struct bin_attribute *bin_attr,
 
 	if (!pdev->rom_attr_enabled)
 		return -EINVAL;
-	
+
 	rom = pci_map_rom(pdev, &size);	/* size starts out as PCI window size */
 	if (!rom)
 		return 0;
-		
+
 	if (off >= size)
 		count = 0;
 	else {
 		if (off + count > size)
 			count = size - off;
-		
+
 		memcpy_fromio(buf, rom + off, count);
 	}
 	pci_unmap_rom(pdev, rom);
-		
+
 	return count;
 }
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 9cf810e..7399d4e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -494,7 +494,7 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 	}
 
 	/* Disable MasterAbortMode during probing to avoid reporting
-	   of bus errors (in some architectures) */ 
+	   of bus errors (in some architectures) */
 	pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &bctl);
 	pci_write_config_word(dev, PCI_BRIDGE_CONTROL,
 			      bctl & ~PCI_BRIDGE_CTL_MASTER_ABORT);
@@ -570,7 +570,7 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 			buses &= ~0xff000000;
 			buses |= CARDBUS_LATENCY_TIMER << 24;
 		}
-			
+
 		/*
 		 * We need to blast all three values with a single write.
 		 */
@@ -680,7 +680,7 @@ static void pci_read_irq(struct pci_dev *dev)
  * pci_setup_device - fill in class and map information of a device
  * @dev: the device structure to fill
  *
- * Initialize the device structure with information about the device's 
+ * Initialize the device structure with information about the device's
  * vendor,class,memory and IO-space addresses,IRQ lines etc.
  * Called at initialisation of the PCI subsystem and by CardBus services.
  * Returns 0 on success and -1 if unknown type of device (not normal, bridge
@@ -720,8 +720,8 @@ static int pci_setup_device(struct pci_dev * dev)
 		pci_read_config_word(dev, PCI_SUBSYSTEM_ID, &dev->subsystem_device);
 
 		/*
-		 *	Do the ugly legacy mode stuff here rather than broken chip
-		 *	quirk code. Legacy mode ATA controllers have fixed
+		 *	Do the ugly legacy mode stuff here rather than broken
+		 *	chip quirk code. Legacy mode ATA controllers have fixed
 		 *	addresses. These are not always echoed in BAR0-3, and
 		 *	BAR0-3 in a few cases contain junk!
 		 */
@@ -752,7 +752,7 @@ static int pci_setup_device(struct pci_dev * dev)
 			goto bad;
 		/* The PCI-to-PCI bridge spec requires that subtractive
 		   decoding (i.e. transparent) bridge must have programming
-		   interface code of 0x01. */ 
+		   interface code of 0x01. */
 		pci_read_irq(dev);
 		dev->transparent = ((dev->class & 0xff) == 1);
 		pci_read_bases(dev, 2, PCI_ROM_ADDRESS1);
@@ -1051,14 +1051,14 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
 			nr++;
 
 			/*
-		 	 * If this is a single function device,
-		 	 * don't scan past the first function.
-		 	 */
+			 * If this is a single function device,
+			 * don't scan past the first function.
+			 */
 			if (!dev->multifunction) {
 				if (func > 0) {
 					dev->multifunction = 1;
 				} else {
- 					break;
+					break;
 				}
 			}
 		} else {
@@ -1140,7 +1140,9 @@ struct pci_bus * pci_create_bus(struct device *parent,
 	b->ops = ops;
 
 	if (pci_find_bus(pci_domain_nr(b), bus)) {
-		/* If we already got to this bus through a different bridge, ignore it */
+		/* If we already got to this bus through a different bridge,
+		 * ignore it
+		 */
 		pr_debug("PCI: Bus %04x:%02x already known\n", pci_domain_nr(b), bus);
 		goto err_out;
 	}
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 042e089..ba58128 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -7,7 +7,7 @@ static void pci_free_resources(struct pci_dev *dev)
 {
 	int i;
 
- 	msi_remove_pci_irq_vectors(dev);
+	msi_remove_pci_irq_vectors(dev);
 
 	pci_cleanup_rom(dev);
 	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
@@ -49,7 +49,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
  * pci_remove_device_safe - remove an unused hotplug device
  * @dev: the device to remove
  *
- * Delete the device structure from the device lists and 
+ * 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.

Re: [RFC PATCH 00/10] PCI core learns 'hotplug'

[Trent Piepho]

On Wed, 28 Jan 2009, Alex Chiang wrote:
> A while ago, Darrick Wong posted a patch for fakephp that kicked off
> some controversy:
>
> 	http://thread.gmane.org/gmane.linux.kernel/761944
>
> The issue was that I broke the fakephp interface back in the 2.6.27
> timeframe. After some discussion on the lists, Trent Piepho sent some
> patches, and I proposed a solution incorporating those patches.
>
> This is my first cut at making everyone happy. In summary, it:
>
> 	- introduces /sys/bus/pci/devices/.../remove for function level
> 	  hot-remove
>
> 	- introduces /sys/bus/pci/devices/.../rescan to rescan the PCI
> 	  hierarchy, starting at that device and descending to all children
>
> 	- introduces /sys/bus/pci/rescan to rescan the entire PCI hierarchy
>
> 	- restores the pre-2.6.27 fakephp interface for userspace compatability

I also continued to work on my patches, but then my reasons for caring
about PCI hotplug disappeared due to the current economic climate.

I updated my "remove" patch to include documentation.  I created a patch
that added "/sys/bus/pci/scan", but not the per-device version.  And I
updated my new fakephp driver to support rescanning.

Everything worked, but when a bridge was rescanned there would be annoying
warning messages.  I never got around to figuring about what to do about
that.  It seems like the code that assigns bridge resources wasn't intended
to handle bridges that already had resources assigned to them, though it
does work.

Maybe your series can use my latest patches for removal and legacy_fakephp?
It sounds like your patches for rescanning do more than mine.


> 	- I've been testing this patchset on my ia64 machines, which Linus
> 	  has called "an insane mess of PCI bridges"[1], and it seems to
> 	  work well. I'm just starting to test on some x86 machines, and
> 	  have been noticing some issues with BAR collisions, so this is
> 	  definitely a work-in-progress.

Does it not work, or is it just warnings?  I didn't have any problems with
resources ending up unassigned, but I did get warnings.  I think there was
also an issue with removed and rescanned devices' resources' ->parent
pointers not being the same as they were before removal.  Which doesn't
seem to matter any, but made me feel like the code wasn't right yet.

> 	  If you use the new PCI core removal/rescan and then try to modify
> 	  the slot using acpiphp, you get an oops. My impression is that
> 	  this behavior is the same as pre-2.6.27, where you could have
> 	  loaded fakephp and acpiphp, removed the device with fakephp,
> 	  and encountered an oops with acpiphp.

I came to the same conclusion.  fakephp or acpiphp will oops if you use the
other to remove a pci device.  The drivers just aren't designed to handle a
pci device being removed out from under them.

> 	  So, I'm not sure what to do about this. The way that we remove
> 	  devices today, using pci_remove_bus_device() doesn't lend itself
> 	  to safety very well, since it will just start removing devices
> 	  from the bus without checking anything.
>
> 	  Maybe we need some other API, or maybe we just live with the
> 	  limitation of, "if you use PCI core hotplug, don't use the
> 	  other hotplug drivers and vice versa".

My new fakephp driver seems to handle this ok, maybe other php drivers
could do the same thing?

[PATCH 1/3] PCI: Method for removing PCI devices

[Trent Piepho]

This patch adds an attribute named "remove" to a PCI device's sysfs
directory.  Writing a non-zero value to this attribute will remove the PCI
device and any children of it.
---
 Documentation/filesystems/sysfs-pci.txt |    9 +++++++++
 drivers/pci/pci-sysfs.c                 |   26 ++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/Documentation/filesystems/sysfs-pci.txt b/Documentation/filesystems/sysfs-pci.txt
index 68ef488..54e014f 100644
--- a/Documentation/filesystems/sysfs-pci.txt
+++ b/Documentation/filesystems/sysfs-pci.txt
@@ -11,6 +11,7 @@ that support it.  For example, a given bus might look like this:
      |   |-- device
      |   |-- irq
      |   |-- local_cpus
+     |   |-- remove
      |   |-- resource
      |   |-- resource0
      |   |-- resource1
@@ -34,6 +35,7 @@ files, each with their own function.
        device		   PCI device (ascii, ro)
        irq		   IRQ number (ascii, ro)
        local_cpus	   nearby CPU mask (cpumask, ro)
+       remove		   remove device from kernel's list (ascii, wo)
        resource		   PCI resource host addresses (ascii, ro)
        resource0..N	   PCI resource N, if present (binary, mmap)
        resource0_wc..N_wc  PCI WC map resource N, if prefetchable (binary, mmap)
@@ -44,6 +46,7 @@ files, each with their own function.
 
   ro - read only file
   rw - file is readable and writable
+  wo - write only file
   mmap - file is mmapable
   ascii - file contains ascii text
   binary - file contains binary data
@@ -62,6 +65,12 @@ ROM file, if available.  It's disabled by default, however, so applications
 should write the string "1" to the file to enable it before attempting a read
 call, and disable it following the access by writing "0" to the file.
 
+The 'remove' file is used to remove the PCI device, by writing a non-zero
+integer to the file.  This does not involve any kind of hot-plug functionality,
+e.g. powering off the device.  The device is removed from the kernel's list of
+PCI devices, the sysfs directory for it removed, and the device will be removed
+from any drivers attached to it.
+
 Accessing legacy resources through sysfs
 ----------------------------------------
 
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 3ddee7f..d422f37 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -219,6 +219,31 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static void remove_callback(void *data)
+{
+	pci_remove_bus_device((struct pci_dev *)data);
+}
+
+static ssize_t
+remove_store(struct device *dev, struct device_attribute *dummy,
+	     const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (val)
+		sysfs_schedule_callback(&dev->kobj, remove_callback, pdev,
+		                        THIS_MODULE);
+
+	return count;
+}
+
 struct device_attribute pci_dev_attrs[] = {
 	__ATTR_RO(resource),
 	__ATTR_RO(vendor),
@@ -237,6 +262,7 @@ struct device_attribute pci_dev_attrs[] = {
 	__ATTR(broken_parity_status,(S_IRUGO|S_IWUSR),
 		broken_parity_status_show,broken_parity_status_store),
 	__ATTR(msi_bus, 0644, msi_bus_show, msi_bus_store),
+	__ATTR(remove, S_IWUSR, NULL, remove_store),
 	__ATTR_NULL,
 };
 
-- 
1.5.4.3

Re: [PATCH 1/3] PCI: Method for removing PCI devices

[Alex Chiang]

* Trent Piepho <xyzzy@speakeasy.org>:
> This patch adds an attribute named "remove" to a PCI device's sysfs
> directory.  Writing a non-zero value to this attribute will remove the PCI
> device and any children of it.
> ---
>  Documentation/filesystems/sysfs-pci.txt |    9 +++++++++
>  drivers/pci/pci-sysfs.c                 |   26 ++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 0 deletions(-)
 
I've fold this almost verbatim into my patch series, changing the
patch title, adding ABI documentation, and protecting the remove
calls with #ifdef CONFIG_HOTPLUG:

Otherwise, it's untouched.

Author: Trent Piepho <xyzzy@speakeasy.org>
Date:   Tue Feb 10 15:02:54 2009 -0700

    PCI: Introduce /sys/bus/pci/devices/.../remove
    
    This patch adds an attribute named "remove" to a PCI device's sysfs
    directory.  Writing a non-zero value to this attribute will remove the PCI
    device and any children of it.
    
    [achiang@hp.com: document in Documentation/ABI]
    [achiang@hp.com: add #ifdef CONFIG_HOTPLUG]
    Cc: djwong@us.ibm.com
    Signed-off-by: Trent Piepho <xyzzy@speakeasy.org>
    Signed-off-by: Alex Chiang <achiang@hp.com>

Thanks.

/ac

> diff --git a/Documentation/filesystems/sysfs-pci.txt b/Documentation/filesystems/sysfs-pci.txt
> index 68ef488..54e014f 100644
> --- a/Documentation/filesystems/sysfs-pci.txt
> +++ b/Documentation/filesystems/sysfs-pci.txt
> @@ -11,6 +11,7 @@ that support it.  For example, a given bus might look like this:
>       |   |-- device
>       |   |-- irq
>       |   |-- local_cpus
> +     |   |-- remove
>       |   |-- resource
>       |   |-- resource0
>       |   |-- resource1
> @@ -34,6 +35,7 @@ files, each with their own function.
>         device		   PCI device (ascii, ro)
>         irq		   IRQ number (ascii, ro)
>         local_cpus	   nearby CPU mask (cpumask, ro)
> +       remove		   remove device from kernel's list (ascii, wo)
>         resource		   PCI resource host addresses (ascii, ro)
>         resource0..N	   PCI resource N, if present (binary, mmap)
>         resource0_wc..N_wc  PCI WC map resource N, if prefetchable (binary, mmap)
> @@ -44,6 +46,7 @@ files, each with their own function.
>  
>    ro - read only file
>    rw - file is readable and writable
> +  wo - write only file
>    mmap - file is mmapable
>    ascii - file contains ascii text
>    binary - file contains binary data
> @@ -62,6 +65,12 @@ ROM file, if available.  It's disabled by default, however, so applications
>  should write the string "1" to the file to enable it before attempting a read
>  call, and disable it following the access by writing "0" to the file.
>  
> +The 'remove' file is used to remove the PCI device, by writing a non-zero
> +integer to the file.  This does not involve any kind of hot-plug functionality,
> +e.g. powering off the device.  The device is removed from the kernel's list of
> +PCI devices, the sysfs directory for it removed, and the device will be removed
> +from any drivers attached to it.
> +
>  Accessing legacy resources through sysfs
>  ----------------------------------------
>  
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 3ddee7f..d422f37 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -219,6 +219,31 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
>  	return count;
>  }
>  
> +static void remove_callback(void *data)
> +{
> +	pci_remove_bus_device((struct pci_dev *)data);
> +}
> +
> +static ssize_t
> +remove_store(struct device *dev, struct device_attribute *dummy,
> +	     const char *buf, size_t count)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	unsigned long val;
> +
> +	if (strict_strtoul(buf, 0, &val) < 0)
> +		return -EINVAL;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
> +	if (val)
> +		sysfs_schedule_callback(&dev->kobj, remove_callback, pdev,
> +		                        THIS_MODULE);
> +
> +	return count;
> +}
> +
>  struct device_attribute pci_dev_attrs[] = {
>  	__ATTR_RO(resource),
>  	__ATTR_RO(vendor),
> @@ -237,6 +262,7 @@ struct device_attribute pci_dev_attrs[] = {
>  	__ATTR(broken_parity_status,(S_IRUGO|S_IWUSR),
>  		broken_parity_status_show,broken_parity_status_store),
>  	__ATTR(msi_bus, 0644, msi_bus_show, msi_bus_store),
> +	__ATTR(remove, S_IWUSR, NULL, remove_store),
>  	__ATTR_NULL,
>  };
>  
> -- 
> 1.5.4.3
> 

[PATCH 2/3] PCI: Add ability to rescan PCI busses

[Trent Piepho]

This adds a bus attribute named "scan" to the PCI bus, which will
appear in sysfs at "/sys/bus/pci/scan".  Writing to this file will
trigger a rescan of all PCI busses.

To do this pci_scan_single_device() is modified to first check if the
device to be scanned is already known to Linux.  In which case it just
returns the existing device.  The old behavior was to create a new
device anyway that would conflict with the existing one, causing all
manner of Bad Things to happen.

pci_scan_slot() has been rewritten to be less complex and will now
return the number of *new* devices found.  It didn't used to work to
call pci_scan_slot() on a previously scanned slot, so returning all
devices found was effectivly the same as returning new devices.
---
 drivers/pci/pci-driver.c |    1 +
 drivers/pci/pci-sysfs.c  |   14 +++++++++++
 drivers/pci/pci.h        |    6 +++++
 drivers/pci/probe.c      |   55 +++++++++++++++++++++++++++------------------
 4 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 2bec651..fcfd445 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -847,6 +847,7 @@ struct bus_type pci_bus_type = {
 	.remove		= pci_device_remove,
 	.shutdown	= pci_device_shutdown,
 	.dev_attrs	= pci_dev_attrs,
+	.bus_attrs	= pci_bus_attrs,
 	.pm		= PCI_PM_OPS_PTR,
 };
 
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d422f37..0781ab7 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -266,6 +266,20 @@ struct device_attribute pci_dev_attrs[] = {
 	__ATTR_NULL,
 };
 
+#ifdef CONFIG_HOTPLUG
+static ssize_t __ref scan(struct bus_type *type, const char *buf, size_t count)
+{
+	pci_rescan_busses();
+
+	return count;
+}
+
+struct bus_attribute pci_bus_attrs[] = {
+	__ATTR(scan, S_IWUSR, NULL, scan),
+	__ATTR_NULL
+};
+#endif
+
 static ssize_t
 pci_read_config(struct kobject *kobj, struct bin_attribute *bin_attr,
 		char *buf, loff_t off, size_t count)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 65deed8..49d7bb4 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -87,6 +87,7 @@ static inline int pci_proc_detach_bus(struct pci_bus *bus) { return 0; }
 
 /* Functions for PCI Hotplug drivers to use */
 extern unsigned int pci_do_scan_bus(struct pci_bus *bus);
+extern void pci_rescan_busses(void) __devinit;
 
 #ifdef HAVE_PCI_LEGACY
 extern void pci_create_legacy_files(struct pci_bus *bus);
@@ -128,6 +129,11 @@ extern int pcie_mch_quirk;
 extern struct device_attribute pci_dev_attrs[];
 extern struct device_attribute dev_attr_cpuaffinity;
 extern struct device_attribute dev_attr_cpulistaffinity;
+#ifdef CONFIG_HOTPLUG
+extern struct bus_attribute pci_bus_attrs[];
+#else
+#define pci_bus_attrs NULL
+#endif
 
 /**
  * pci_match_one_device - Tell if a PCI device structure has a matching
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 251c2b1..6e6172d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1006,6 +1006,13 @@ struct pci_dev *__ref pci_scan_single_device(struct pci_bus *bus, int devfn)
 {
 	struct pci_dev *dev;
 
+	/* Check to see if the device is already known */
+	dev = pci_get_slot(bus, devfn);
+	if (dev) {
+		pci_dev_put(dev);
+		return dev;
+	}
+
 	dev = pci_scan_device(bus, devfn);
 	if (!dev)
 		return NULL;
@@ -1024,35 +1031,26 @@ EXPORT_SYMBOL(pci_scan_single_device);
  * Scan a PCI slot on the specified PCI bus for devices, adding
  * discovered devices to the @bus->devices list.  New devices
  * will not have is_added set.
+ *
+ * Returns the number of new devices found.
  */
 int pci_scan_slot(struct pci_bus *bus, int devfn)
 {
-	int func, nr = 0;
-	int scan_all_fns;
-
-	scan_all_fns = pcibios_scan_all_fns(bus, devfn);
-
-	for (func = 0; func < 8; func++, devfn++) {
-		struct pci_dev *dev;
+	int fn, nr = 0;
+	struct pci_dev *dev;
 
-		dev = pci_scan_single_device(bus, devfn);
-		if (dev) {
+	if ((dev = pci_scan_single_device(bus, devfn)))
+		if (!dev->is_added)	/* new device? */
 			nr++;
 
-			/*
-		 	 * If this is a single function device,
-		 	 * don't scan past the first function.
-		 	 */
-			if (!dev->multifunction) {
-				if (func > 0) {
-					dev->multifunction = 1;
-				} else {
- 					break;
-				}
+	if ((dev && dev->multifunction) ||
+	    (!dev && pcibios_scan_all_fns(bus, devfn))) {
+		for (fn = 1; fn < 8; fn++) {
+			if ((dev = pci_scan_single_device(bus, devfn + fn))) {
+				if (!dev->is_added)
+					nr++;
+				dev->multifunction = 1;
 			}
-		} else {
-			if (func == 0 && !scan_all_fns)
-				break;
 		}
 	}
 
@@ -1192,11 +1190,24 @@ struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent,
 }
 EXPORT_SYMBOL(pci_scan_bus_parented);
 
+/* Does a recursive rescan of all PCI busses. */
+void __devinit pci_rescan_busses(void)
+{
+	struct pci_bus *bus = NULL;
+
+	while ((bus = pci_find_next_bus(bus)) != NULL) {
+		pci_scan_child_bus(bus);
+		pci_bus_assign_resources(bus);
+		pci_bus_add_devices(bus);
+	}
+}
+
 #ifdef CONFIG_HOTPLUG
 EXPORT_SYMBOL(pci_add_new_bus);
 EXPORT_SYMBOL(pci_scan_slot);
 EXPORT_SYMBOL(pci_scan_bridge);
 EXPORT_SYMBOL_GPL(pci_scan_child_bus);
+EXPORT_SYMBOL_GPL(pci_rescan_busses);
 #endif
 
 static int __init pci_sort_bf_cmp(const struct device *d_a, const struct device *d_b)
-- 
1.5.4.3

Re: [PATCH 2/3] PCI: Add ability to rescan PCI busses

[Alex Chiang]

I will definitely take some ideas from this patch and incorporate
them into mine.

* Trent Piepho <xyzzy@speakeasy.org>:
> This adds a bus attribute named "scan" to the PCI bus, which will
> appear in sysfs at "/sys/bus/pci/scan".  Writing to this file will
> trigger a rescan of all PCI busses.

I prefer the name "rescan".

> To do this pci_scan_single_device() is modified to first check if the
> device to be scanned is already known to Linux.  In which case it just
> returns the existing device.  The old behavior was to create a new
> device anyway that would conflict with the existing one, causing all
> manner of Bad Things to happen.
> 
> pci_scan_slot() has been rewritten to be less complex and will now
> return the number of *new* devices found.  It didn't used to work to
> call pci_scan_slot() on a previously scanned slot, so returning all
> devices found was effectivly the same as returning new devices.
> ---
>  drivers/pci/pci-driver.c |    1 +
>  drivers/pci/pci-sysfs.c  |   14 +++++++++++
>  drivers/pci/pci.h        |    6 +++++
>  drivers/pci/probe.c      |   55 +++++++++++++++++++++++++++------------------
>  4 files changed, 54 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 2bec651..fcfd445 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -847,6 +847,7 @@ struct bus_type pci_bus_type = {
>  	.remove		= pci_device_remove,
>  	.shutdown	= pci_device_shutdown,
>  	.dev_attrs	= pci_dev_attrs,
> +	.bus_attrs	= pci_bus_attrs,

This is cleaner than my approach, so I'll use your technique.

>  	.pm		= PCI_PM_OPS_PTR,
>  };
>  
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index d422f37..0781ab7 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -266,6 +266,20 @@ struct device_attribute pci_dev_attrs[] = {
>  	__ATTR_NULL,
>  };
>  
> +#ifdef CONFIG_HOTPLUG
> +static ssize_t __ref scan(struct bus_type *type, const char *buf, size_t count)
> +{
> +	pci_rescan_busses();
> +
> +	return count;
> +}
> +
> +struct bus_attribute pci_bus_attrs[] = {
> +	__ATTR(scan, S_IWUSR, NULL, scan),
> +	__ATTR_NULL
> +};
> +#endif
> +
>  static ssize_t
>  pci_read_config(struct kobject *kobj, struct bin_attribute *bin_attr,
>  		char *buf, loff_t off, size_t count)
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 65deed8..49d7bb4 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -87,6 +87,7 @@ static inline int pci_proc_detach_bus(struct pci_bus *bus) { return 0; }
>  
>  /* Functions for PCI Hotplug drivers to use */
>  extern unsigned int pci_do_scan_bus(struct pci_bus *bus);
> +extern void pci_rescan_busses(void) __devinit;
>  
>  #ifdef HAVE_PCI_LEGACY
>  extern void pci_create_legacy_files(struct pci_bus *bus);
> @@ -128,6 +129,11 @@ extern int pcie_mch_quirk;
>  extern struct device_attribute pci_dev_attrs[];
>  extern struct device_attribute dev_attr_cpuaffinity;
>  extern struct device_attribute dev_attr_cpulistaffinity;
> +#ifdef CONFIG_HOTPLUG
> +extern struct bus_attribute pci_bus_attrs[];
> +#else
> +#define pci_bus_attrs NULL
> +#endif
>  
>  /**
>   * pci_match_one_device - Tell if a PCI device structure has a matching
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 251c2b1..6e6172d 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1006,6 +1006,13 @@ struct pci_dev *__ref pci_scan_single_device(struct pci_bus *bus, int devfn)
>  {
>  	struct pci_dev *dev;
>  
> +	/* Check to see if the device is already known */
> +	dev = pci_get_slot(bus, devfn);
> +	if (dev) {
> +		pci_dev_put(dev);
> +		return dev;
> +	}
> +

Above is already a separate patch in my series.

>  	dev = pci_scan_device(bus, devfn);
>  	if (!dev)
>  		return NULL;
> @@ -1024,35 +1031,26 @@ EXPORT_SYMBOL(pci_scan_single_device);
>   * Scan a PCI slot on the specified PCI bus for devices, adding
>   * discovered devices to the @bus->devices list.  New devices
>   * will not have is_added set.
> + *
> + * Returns the number of new devices found.
>   */
>  int pci_scan_slot(struct pci_bus *bus, int devfn)
>  {
> -	int func, nr = 0;
> -	int scan_all_fns;
> -
> -	scan_all_fns = pcibios_scan_all_fns(bus, devfn);
> -
> -	for (func = 0; func < 8; func++, devfn++) {
> -		struct pci_dev *dev;
> +	int fn, nr = 0;
> +	struct pci_dev *dev;
>  
> -		dev = pci_scan_single_device(bus, devfn);
> -		if (dev) {
> +	if ((dev = pci_scan_single_device(bus, devfn)))
> +		if (!dev->is_added)	/* new device? */
>  			nr++;
>  
> -			/*
> -		 	 * If this is a single function device,
> -		 	 * don't scan past the first function.
> -		 	 */
> -			if (!dev->multifunction) {
> -				if (func > 0) {
> -					dev->multifunction = 1;
> -				} else {
> - 					break;
> -				}
> +	if ((dev && dev->multifunction) ||
> +	    (!dev && pcibios_scan_all_fns(bus, devfn))) {
> +		for (fn = 1; fn < 8; fn++) {
> +			if ((dev = pci_scan_single_device(bus, devfn + fn))) {
> +				if (!dev->is_added)
> +					nr++;
> +				dev->multifunction = 1;
>  			}
> -		} else {
> -			if (func == 0 && !scan_all_fns)
> -				break;
>  		}
>  	}

Hm, now this is interesting. I'll have to have a bit more of
a think about this to see if your approach makes more sense.

>  
> @@ -1192,11 +1190,24 @@ struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent,
>  }
>  EXPORT_SYMBOL(pci_scan_bus_parented);
>  
> +/* Does a recursive rescan of all PCI busses. */
> +void __devinit pci_rescan_busses(void)
> +{
> +	struct pci_bus *bus = NULL;
> +
> +	while ((bus = pci_find_next_bus(bus)) != NULL) {
> +		pci_scan_child_bus(bus);
> +		pci_bus_assign_resources(bus);
> +		pci_bus_add_devices(bus);
> +	}
> +}
> +

This is better than what I did too.

If it turns out that I can use the majority of the ideas in your
patch, then I'll just incorporate it into my series and attribute
it properly to you of course.

We still need a bunch of the bug fixes I have to actually allow
us to rescan the entire bus without BAR collisions or
reinitializing devices, etc.

Thanks.

/ac

>  #ifdef CONFIG_HOTPLUG
>  EXPORT_SYMBOL(pci_add_new_bus);
>  EXPORT_SYMBOL(pci_scan_slot);
>  EXPORT_SYMBOL(pci_scan_bridge);
>  EXPORT_SYMBOL_GPL(pci_scan_child_bus);
> +EXPORT_SYMBOL_GPL(pci_rescan_busses);
>  #endif
>  
>  static int __init pci_sort_bf_cmp(const struct device *d_a, const struct device *d_b)
> -- 
> 1.5.4.3
> 

[PATCH 3/3] PCI: Legacy fakephp driver

[Trent Piepho]

This module's interface is compatible with what fakephp's interface has
been.

It puts entries in /sys/bus/pci/slots with the names of all PCI
devices/functions.  Each one has a "power" attribute, which works the same
way as the fakephp driver's power attribute has worked.

There are a few improvements over fakephp, which couldn't handle PCI
devices being added or removed via a means other than fakephp's actions.
If a device is added another way, fakephp doesn't notice it and doesn't
create the fake slot for it.  If a device was removed another way, fakephp
doesn't delete the fake slot for it (and accessing the stale slot will
cause an oops).  These problems are fixed.  As a consequence of this,
removing a bridge with other devices behind it works as well, which is
something else fakephp couldn't do.

This patch duplicates a tiny bit of the code in the PCI core that does this
same function.  Re-using that code ends up being more complex than
duplicating it, and it makes code in the PCI core more ugly just to support
this legacy fakephp interface compatibility layer.
---
 drivers/pci/hotplug/Kconfig          |    5 +
 drivers/pci/hotplug/Makefile         |    1 +
 drivers/pci/hotplug/legacy_fakephp.c |  168 ++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pci/hotplug/legacy_fakephp.c

diff --git a/drivers/pci/hotplug/Kconfig b/drivers/pci/hotplug/Kconfig
index eacfb13..d26bc68 100644
--- a/drivers/pci/hotplug/Kconfig
+++ b/drivers/pci/hotplug/Kconfig
@@ -39,6 +39,11 @@ config HOTPLUG_PCI_FAKE
 
 	  When in doubt, say N.
 
+config HOTPLUG_PCI_LEGACY_FAKE
+	tristate "Legacy Fake PCI Hotplug driver"
+	help
+	  Provides an interface like the fakephp driver used to.
+
 config HOTPLUG_PCI_COMPAQ
 	tristate "Compaq PCI Hotplug driver"
 	depends on X86 && PCI_BIOS && PCI_LEGACY
diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
index 9bdbe1a..9ad009d 100644
--- a/drivers/pci/hotplug/Makefile
+++ b/drivers/pci/hotplug/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_HOTPLUG_PCI_SHPC)		+= shpchp.o
 obj-$(CONFIG_HOTPLUG_PCI_RPA)		+= rpaphp.o
 obj-$(CONFIG_HOTPLUG_PCI_RPA_DLPAR)	+= rpadlpar_io.o
 obj-$(CONFIG_HOTPLUG_PCI_SGI)		+= sgi_hotplug.o
+obj-$(CONFIG_HOTPLUG_PCI_LEGACY_FAKE)	+= legacy_fakephp.o
 
 # Link this last so it doesn't claim devices that have a real hotplug driver
 obj-$(CONFIG_HOTPLUG_PCI_FAKE)		+= fakephp.o
diff --git a/drivers/pci/hotplug/legacy_fakephp.c b/drivers/pci/hotplug/legacy_fakephp.c
new file mode 100644
index 0000000..696b156
--- /dev/null
+++ b/drivers/pci/hotplug/legacy_fakephp.c
@@ -0,0 +1,168 @@
+/* Works like the fakephp driver used to, except a little better.
+ *
+ * - It's possible to remove devices with subordinate busses.
+ * - New PCI devices that appear via any method, not just a fakephp triggered
+ *   rescan, will be noticed.
+ * - Devices that are removed via any method, not just a fakephp triggered
+ *   removal, will also be noticed.
+ *
+ * Uses nothing from the pci-hotplug subsystem.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/list.h>
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include "../pci.h"
+
+struct legacy_slot {
+	struct kobject		kobj;
+	struct pci_dev		*dev;
+	struct list_head	list;
+};
+
+static LIST_HEAD(legacy_list);
+
+static ssize_t legacy_show(struct kobject *kobj, struct attribute *attr,
+			   char *buf)
+{
+	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
+	strcpy(buf, "1\n");
+	return 2;
+}
+
+static void remove_callback(void *data)
+{
+	pci_remove_bus_device((struct pci_dev *)data);
+}
+
+/* pci_rescan_busses is __devinit, which is why this driver requires HOTPLUG to
+ * be on.  */
+static void __ref rescan(void)
+{
+	pci_rescan_busses();
+}
+
+static ssize_t legacy_store(struct kobject *kobj, struct attribute *attr,
+			    const char *buf, size_t len)
+{
+	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (val)
+		rescan();
+	else
+		sysfs_schedule_callback(&slot->dev->dev.kobj, remove_callback,
+		                        slot->dev, THIS_MODULE);
+	return len;
+}
+
+static struct attribute *legacy_attrs[] = {
+	&(struct attribute){ .name = "power", .mode = 0644 },
+	NULL,
+};
+
+static void legacy_release(struct kobject *kobj)
+{
+	struct legacy_slot *slot = container_of(kobj, typeof(*slot), kobj);
+
+	pci_dev_put(slot->dev);
+	kfree(slot);
+}
+
+static struct kobj_type legacy_ktype = {
+	.sysfs_ops = &(struct sysfs_ops){
+		.store = legacy_store, .show = legacy_show
+	},
+	.release = &legacy_release,
+	.default_attrs = legacy_attrs,
+};
+
+static int legacy_add_slot(struct pci_dev *pdev)
+{
+	struct legacy_slot *slot = kzalloc(sizeof(*slot), GFP_KERNEL);
+
+	if (!slot)
+		return -ENOMEM;
+
+	if (kobject_init_and_add(&slot->kobj, &legacy_ktype,
+				 &pci_slots_kset->kobj, "%s",
+				 pdev->dev.bus_id)) {
+		dev_warn(&pdev->dev, "Failed to created legacy fake slot\n");
+		return -EINVAL;
+	}
+	slot->dev = pci_dev_get(pdev);
+
+	list_add(&slot->list, &legacy_list);
+
+	return 0;
+}
+
+static int legacy_notify(struct notifier_block *nb,
+			 unsigned long action, void *data)
+{
+        struct pci_dev *pdev = to_pci_dev(data);
+
+	if (action == BUS_NOTIFY_ADD_DEVICE) {
+		legacy_add_slot(pdev);
+	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
+		struct legacy_slot *slot;
+
+		list_for_each_entry(slot, &legacy_list, list)
+			if (slot->dev == pdev)
+				goto found;
+
+		dev_warn(&pdev->dev, "Missing legacy fake slot?");
+		return -ENODEV;
+found:
+		kobject_del(&slot->kobj);
+		list_del(&slot->list);
+		kobject_put(&slot->kobj);
+	}
+
+	return 0;
+}
+
+static struct notifier_block legacy_notifier = {
+	.notifier_call = legacy_notify
+};
+
+static int __init init_legacy(void)
+{
+	struct pci_dev *pdev = NULL;
+
+	/* Add existing devices */
+	while ((pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pdev)))
+		legacy_add_slot(pdev);
+
+	/* Be alerted of any new ones */
+	bus_register_notifier(&pci_bus_type, &legacy_notifier);
+	return 0;
+}
+module_init(init_legacy);
+
+static void __exit remove_legacy(void)
+{
+	struct legacy_slot *slot, *tmp;
+
+	bus_unregister_notifier(&pci_bus_type, &legacy_notifier);
+
+	list_for_each_entry_safe(slot, tmp, &legacy_list, list) {
+		list_del(&slot->list);
+		kobject_del(&slot->kobj);
+		kobject_put(&slot->kobj);
+	}
+}
+module_exit(remove_legacy);
+
+
+MODULE_AUTHOR("Trent Piepho <xyzzy@speakeasy.org>");
+MODULE_DESCRIPTION("Legacy version of the fakephp interface");
+MODULE_LICENSE("GPL");
-- 
1.5.4.3

Re: [RFC PATCH 00/10] PCI core learns 'hotplug'

[Alex Chiang]

Hi Trent,

Sorry for the very long delay. I've been swamped with other
things. :-/

* Trent Piepho <xyzzy@speakeasy.org>:
> On Wed, 28 Jan 2009, Alex Chiang wrote:
> > A while ago, Darrick Wong posted a patch for fakephp that kicked off
> > some controversy:
> >
> > 	http://thread.gmane.org/gmane.linux.kernel/761944
> >
> > The issue was that I broke the fakephp interface back in the 2.6.27
> > timeframe. After some discussion on the lists, Trent Piepho sent some
> > patches, and I proposed a solution incorporating those patches.
> >
> > This is my first cut at making everyone happy. In summary, it:
> >
> > 	- introduces /sys/bus/pci/devices/.../remove for function level
> > 	  hot-remove
> >
> > 	- introduces /sys/bus/pci/devices/.../rescan to rescan the PCI
> > 	  hierarchy, starting at that device and descending to all children
> >
> > 	- introduces /sys/bus/pci/rescan to rescan the entire PCI hierarchy
> >
> > 	- restores the pre-2.6.27 fakephp interface for userspace compatability
> 
> I also continued to work on my patches, but then my reasons for caring
> about PCI hotplug disappeared due to the current economic climate.

:(

> I updated my "remove" patch to include documentation.  I created a patch
> that added "/sys/bus/pci/scan", but not the per-device version.  And I
> updated my new fakephp driver to support rescanning.
> 
> Everything worked, but when a bridge was rescanned there would be annoying
> warning messages.  I never got around to figuring about what to do about
> that.  It seems like the code that assigns bridge resources wasn't intended
> to handle bridges that already had resources assigned to them, though it
> does work.
> 
> Maybe your series can use my latest patches for removal and legacy_fakephp?
> It sounds like your patches for rescanning do more than mine.

I will incorporate your patch for removal (and replace mine).

I've already incorporated your legacy_fakephp patch (although I
took the liberty of just replacing fakephp wholesale).

When (if? :) I work out the kinks, you'll get authorship credit
for both the above.

> > 	- I've been testing this patchset on my ia64 machines, which Linus
> > 	  has called "an insane mess of PCI bridges"[1], and it seems to
> > 	  work well. I'm just starting to test on some x86 machines, and
> > 	  have been noticing some issues with BAR collisions, so this is
> > 	  definitely a work-in-progress.
> 
> Does it not work, or is it just warnings?  I didn't have any problems with
> resources ending up unassigned, but I did get warnings.  I think there was
> also an issue with removed and rescanned devices' resources' ->parent
> pointers not being the same as they were before removal.  Which doesn't
> seem to matter any, but made me feel like the code wasn't right yet.

I'm actually getting errors on my x86 machine:

pci 0000:04:01.0: BAR 8: bogus alignment [0xfa000000-0xfbffffff] flags 0x200
pci 0000:04:01.0: BAR 9: bogus alignment [0xd1100000-0xd11fffff] flags 0x1201
...
pci 0000:07:00.0: BAR 8: bogus alignment [0xfa000000-0xfbffffff] flags 0x200
pci 0000:07:00.0: BAR 9: bogus alignment [0xd1100000-0xd11fffff] flags 0x1201
...
pcieport-driver 0000:04:01.0: irq 57 for MSI/MSI-X
pcieport-driver 0000:04:01.0: device not available because of BAR 8 [0xfa000000-0xfbffffff] collisions
pcieport-driver: probe of 0000:04:01.0 failed with error -22

Obviously, this needs to be figured out before going into
mainline, and even then, I think it might need some soak time in
-mm...

> > 	  If you use the new PCI core removal/rescan and then try to modify
> > 	  the slot using acpiphp, you get an oops. My impression is that
> > 	  this behavior is the same as pre-2.6.27, where you could have
> > 	  loaded fakephp and acpiphp, removed the device with fakephp,
> > 	  and encountered an oops with acpiphp.
> 
> I came to the same conclusion.  fakephp or acpiphp will oops if you use the
> other to remove a pci device.  The drivers just aren't designed to handle a
> pci device being removed out from under them.
> 
> > 	  So, I'm not sure what to do about this. The way that we remove
> > 	  devices today, using pci_remove_bus_device() doesn't lend itself
> > 	  to safety very well, since it will just start removing devices
> > 	  from the bus without checking anything.
> >
> > 	  Maybe we need some other API, or maybe we just live with the
> > 	  limitation of, "if you use PCI core hotplug, don't use the
> > 	  other hotplug drivers and vice versa".
> 
> My new fakephp driver seems to handle this ok, maybe other php drivers
> could do the same thing?

Yeah, I saw what you did by registering a bus notifier, nice
trick.

I don't think every driver wants to do this, it might be better
to change the hotplug core to require some sort of callback that
gets called when some other driver removes a device.

Thanks.

/ac