[PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects

[PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects

[Alex Chiang]

Hi Greg, Kenji-san,

This is v10 of my "introduce pci_slot" series, to be considered
for inclusion in linux-next to get more test exposure and shake
out the bugs.

* Greg KH <gregkh@suse.de>:
> Can you work with Kenji to get his approval on this before I
> add this to the tree?  It sounds like there are still some
> outstanding issues getting this to work properly on his
> hardware.  It needs to work without relying on a module not
> being loaded, as the distros are never going to go for that
> kind of solution.

New in v10 is the addition of a DMI list to do the right thing on
Fujitsu hardware, and does not rely on "don't load the module" to
get the correct behavior.

Also, v10 should fix the "pci_slot module changes the sysfs name"
issue that Kenji-san was seeing.

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Alex-san, could you send me the updated patches? I'll Ack the
> patch after adding Fujitsu firmware versions to the DMI list
> and testing it.

Thanks, I'd like to know if the naming issue is fixed for you as
well.

cheers,

/ac

[PATCH 1/3] Construct one fakephp slot per pci slot

[Alex Chiang]

Register one slot per slot, rather than one slot per function.
Change the name of the slot to fake%d instead of the pci address.

Signed-off-by: Alex Chiang <achiang@hp.com>
Signed-off-by: Matthew Wilcox <matthew@wil.cx>
---
 drivers/pci/hotplug/fakephp.c |   85 ++++++++++++++--------------------------
 1 files changed, 30 insertions(+), 55 deletions(-)

diff --git a/drivers/pci/hotplug/fakephp.c b/drivers/pci/hotplug/fakephp.c
index 94b6401..6c14b4d 100644
--- a/drivers/pci/hotplug/fakephp.c
+++ b/drivers/pci/hotplug/fakephp.c
@@ -66,6 +66,7 @@ struct dummy_slot {
 	struct pci_dev *dev;
 	struct work_struct remove_work;
 	unsigned long removed;
+	char name[8];
 };
 
 static int debug;
@@ -100,6 +101,7 @@ static int add_slot(struct pci_dev *dev)
 	struct dummy_slot *dslot;
 	struct hotplug_slot *slot;
 	int retval = -ENOMEM;
+	static int count = 1;
 
 	slot = kzalloc(sizeof(struct hotplug_slot), GFP_KERNEL);
 	if (!slot)
@@ -113,13 +115,14 @@ static int add_slot(struct pci_dev *dev)
 	slot->info->max_bus_speed = PCI_SPEED_UNKNOWN;
 	slot->info->cur_bus_speed = PCI_SPEED_UNKNOWN;
 
-	slot->name = &dev->dev.bus_id[0];
-	dbg("slot->name = %s\n", slot->name);
-
 	dslot = kzalloc(sizeof(struct dummy_slot), GFP_KERNEL);
 	if (!dslot)
 		goto error_info;
 
+	slot->name = dslot->name;
+	snprintf(slot->name, sizeof(dslot->name), "fake%d", count++);
+	dbg("slot->name = %s\n", slot->name);
+
 	slot->ops = &dummy_hotplug_slot_ops;
 	slot->release = &dummy_release;
 	slot->private = dslot;
@@ -148,17 +151,17 @@ error:
 static int __init pci_scan_buses(void)
 {
 	struct pci_dev *dev = NULL;
-	int retval = 0;
+	int lastslot = 0;
 
 	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
-		retval = add_slot(dev);
-		if (retval) {
-			pci_dev_put(dev);
-			break;
-		}
+		if (PCI_FUNC(dev->devfn) > 0 &&
+				lastslot == PCI_SLOT(dev->devfn))
+			continue;
+		lastslot = PCI_SLOT(dev->devfn);
+		add_slot(dev);
 	}
 
-	return retval;
+	return 0;
 }
 
 static void remove_slot(struct dummy_slot *dslot)
@@ -296,23 +299,9 @@ static int enable_slot(struct hotplug_slot *hotplug_slot)
 	return -ENODEV;
 }
 
-/* find the hotplug_slot for the pci_dev */
-static struct hotplug_slot *get_slot_from_dev(struct pci_dev *dev)
-{
-	struct dummy_slot *dslot;
-
-	list_for_each_entry(dslot, &slot_list, node) {
-		if (dslot->dev == dev)
-			return dslot->slot;
-	}
-	return NULL;
-}
-
-
 static int disable_slot(struct hotplug_slot *slot)
 {
 	struct dummy_slot *dslot;
-	struct hotplug_slot *hslot;
 	struct pci_dev *dev;
 	int func;
 
@@ -322,41 +311,27 @@ static int disable_slot(struct hotplug_slot *slot)
 
 	dbg("%s - physical_slot = %s\n", __FUNCTION__, slot->name);
 
-	/* don't disable bridged devices just yet, we can't handle them easily... */
-	if (dslot->dev->subordinate) {
-		err("Can't remove PCI devices with other PCI devices behind it yet.\n");
-		return -ENODEV;
-	}
-	if (test_and_set_bit(0, &dslot->removed)) {
-		dbg("Slot already scheduled for removal\n");
-		return -ENODEV;
-	}
-	/* search for subfunctions and disable them first */
-	if (!(dslot->dev->devfn & 7)) {
-		for (func = 1; func < 8; func++) {
-			dev = pci_get_slot(dslot->dev->bus,
-					dslot->dev->devfn + func);
-			if (dev) {
-				hslot = get_slot_from_dev(dev);
-				if (hslot)
-					disable_slot(hslot);
-				else {
-					err("Hotplug slot not found for subfunction of PCI device\n");
-					return -ENODEV;
-				}
-				pci_dev_put(dev);
-			} else
-				dbg("No device in slot found\n");
+	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");
+			return -ENODEV;
 		}
-	}
 
-	/* remove the device from the pci core */
-	pci_remove_bus_device(dslot->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);
 
-	/* 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);
+		/* blow away this sysfs entry and other parts. */
+		remove_slot(dslot);
 
+		pci_dev_put(dev);
+	}
 	return 0;
 }
 
-- 
1.5.3.1.g1e61

[PATCH 2/3] Introduce pci_slot

[Alex Chiang]

Currently, /sys/bus/pci/slots/ only exposes hotplug attributes
when a hotplug driver is loaded, but PCI slots have attributes
such as address, speed, width, etc. that are not related to
hotplug at all.

Introduce pci_slot as the primary data structure and kobject
model. Hotplug attributes described in hotplug_slot become a
secondary structure associated with the pci_slot.

This patch only creates the infrastructure that allows the
separation of PCI slot attributes and hotplug attributes.
In this patch, the PCI hotplug core remains the only user of this
infrastructure, and thus, /sys/bus/pci/slots/ will still only
become populated when a hotplug driver is loaded.

A later patch in this series will add a second user of this new
infrastructure and demonstrate splitting the task of exposing
pci_slot attributes from hotplug_slot attributes.

  - Make pci_slot the primary sysfs entity. hotplug_slot becomes a
    subsidiary structure.
    o pci_create_slot() creates and registers a slot with the PCI core
    o pci_slot_add_hotplug() gives it hotplug capability

  - Change the prototype of pci_hp_register() to take the bus and
    slot number (on parent bus) as parameters.

  - Remove all the ->get_address methods since this functionality is
    now handled by pci_slot directly.

v8 -> v9:
	Fixed printk output, changed to American spelling of
	"initialization"

v7 -> v8:
	Removed externs from C files.

	Taught sn_hp_destroy and sn_hotplug_slot_register about
	struct pci_slot.

v6 -> v7:
	Refresh to new kobject model.

v5 -> v6:
	Add debugging information.

v4 -> v5:
	Add refcounting for pci_slot objects.

	Return -EBUSY if an hp driver attempts to register a slot
	that is already registered to another driver. Do not consider
	that to be an error condition in acpiphp and pciehp.

v3 -> v4:
	Fixed bug with pciehp and rpaphp registering slots

v2 -> v3:
	Separated slot creation and slot hotplug ability into two
	interfaces. Fixed bugs in pci_destroy_slot(), and now
	properly calling from pci_hp_deregister.

v1 -> v2:
	No change

Signed-off-by: Alex Chiang <achiang@hp.com>
Signed-off-by: Matthew Wilcox <matthew@wil.cx>
---
 drivers/pci/Makefile                    |    2 +-
 drivers/pci/hotplug/acpiphp.h           |    1 -
 drivers/pci/hotplug/acpiphp_core.c      |   25 +---
 drivers/pci/hotplug/acpiphp_glue.c      |   23 +--
 drivers/pci/hotplug/acpiphp_ibm.c       |    6 +-
 drivers/pci/hotplug/cpci_hotplug_core.c |    2 +-
 drivers/pci/hotplug/cpqphp_core.c       |    4 +-
 drivers/pci/hotplug/fakephp.c           |    2 +-
 drivers/pci/hotplug/ibmphp_ebda.c       |    3 +-
 drivers/pci/hotplug/pci_hotplug_core.c  |  244 +++++++++++--------------------
 drivers/pci/hotplug/pciehp_core.c       |   31 ++---
 drivers/pci/hotplug/rpadlpar_sysfs.c    |    5 +-
 drivers/pci/hotplug/rpaphp_slot.c       |    3 +-
 drivers/pci/hotplug/sgi_hotplug.c       |   10 +-
 drivers/pci/hotplug/shpchp_core.c       |   17 +--
 drivers/pci/pci.h                       |   13 ++
 drivers/pci/slot.c                      |  224 ++++++++++++++++++++++++++++
 include/linux/pci.h                     |   17 ++
 include/linux/pci_hotplug.h             |   12 +-
 19 files changed, 394 insertions(+), 250 deletions(-)
 create mode 100644 drivers/pci/slot.c

diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 4d1ce2e..7d63f8c 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the PCI bus specific drivers.
 #
 
-obj-y		+= access.o bus.o probe.o remove.o pci.o quirks.o \
+obj-y		+= access.o bus.o probe.o remove.o pci.o quirks.o slot.o \
 			pci-driver.o search.o pci-sysfs.o rom.o setup-res.o
 obj-$(CONFIG_PROC_FS) += proc.o
 
diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index 7a29164..eecf7cb 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -215,7 +215,6 @@ extern u8 acpiphp_get_power_status (struct acpiphp_slot *slot);
 extern u8 acpiphp_get_attention_status (struct acpiphp_slot *slot);
 extern u8 acpiphp_get_latch_status (struct acpiphp_slot *slot);
 extern u8 acpiphp_get_adapter_status (struct acpiphp_slot *slot);
-extern u32 acpiphp_get_address (struct acpiphp_slot *slot);
 
 /* variables */
 extern int acpiphp_debug;
diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
index 9279d5b..57319e6 100644
--- a/drivers/pci/hotplug/acpiphp_core.c
+++ b/drivers/pci/hotplug/acpiphp_core.c
@@ -70,7 +70,6 @@ static int disable_slot		(struct hotplug_slot *slot);
 static int set_attention_status (struct hotplug_slot *slot, u8 value);
 static int get_power_status	(struct hotplug_slot *slot, u8 *value);
 static int get_attention_status (struct hotplug_slot *slot, u8 *value);
-static int get_address		(struct hotplug_slot *slot, u32 *value);
 static int get_latch_status	(struct hotplug_slot *slot, u8 *value);
 static int get_adapter_status	(struct hotplug_slot *slot, u8 *value);
 
@@ -83,7 +82,6 @@ static struct hotplug_slot_ops acpi_hotplug_slot_ops = {
 	.get_attention_status	= get_attention_status,
 	.get_latch_status	= get_latch_status,
 	.get_adapter_status	= get_adapter_status,
-	.get_address		= get_address,
 };
 
 
@@ -274,23 +272,6 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
 	return 0;
 }
 
-
-/**
- * get_address - get pci address of a slot
- * @hotplug_slot: slot to get status
- * @value: pointer to struct pci_busdev (seg, bus, dev)
- */
-static int get_address(struct hotplug_slot *hotplug_slot, u32 *value)
-{
-	struct slot *slot = hotplug_slot->private;
-
-	dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot->name);
-
-	*value = acpiphp_get_address(slot->acpi_slot);
-
-	return 0;
-}
-
 static int __init init_acpi(void)
 {
 	int retval;
@@ -357,7 +338,11 @@ int acpiphp_register_hotplug_slot(struct acpiphp_slot *acpiphp_slot)
 	acpiphp_slot->slot = slot;
 	snprintf(slot->name, sizeof(slot->name), "%u", slot->acpi_slot->sun);
 
-	retval = pci_hp_register(slot->hotplug_slot);
+	retval = pci_hp_register(slot->hotplug_slot,
+					acpiphp_slot->bridge->pci_bus,
+					acpiphp_slot->device);
+	if (retval == -EBUSY)
+		goto error_hpslot;
 	if (retval) {
 		err("pci_hp_register failed with error %d\n", retval);
 		goto error_hpslot;
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 5e50008..b8039eb 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -258,7 +258,12 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 				bridge->pci_bus->number, slot->device);
 		retval = acpiphp_register_hotplug_slot(slot);
 		if (retval) {
-			warn("acpiphp_register_hotplug_slot failed(err code = 0x%x)\n", retval);
+			if (retval == -EBUSY)
+				warn("Slot %d already registered by another "
+					"hotplug driver\n", slot->sun);
+			else
+				warn("acpiphp_register_hotplug_slot failed "
+					"(err code = 0x%x)\n", retval);
 			goto err_exit;
 		}
 	}
@@ -1867,19 +1872,3 @@ u8 acpiphp_get_adapter_status(struct acpiphp_slot *slot)
 
 	return (sta == 0) ? 0 : 1;
 }
-
-
-/*
- * pci address (seg/bus/dev)
- */
-u32 acpiphp_get_address(struct acpiphp_slot *slot)
-{
-	u32 address;
-	struct pci_bus *pci_bus = slot->bridge->pci_bus;
-
-	address = (pci_domain_nr(pci_bus) << 16) |
-		  (pci_bus->number << 8) |
-		  slot->device;
-
-	return address;
-}
diff --git a/drivers/pci/hotplug/acpiphp_ibm.c b/drivers/pci/hotplug/acpiphp_ibm.c
index b0a22b9..34a708f 100644
--- a/drivers/pci/hotplug/acpiphp_ibm.c
+++ b/drivers/pci/hotplug/acpiphp_ibm.c
@@ -33,8 +33,10 @@
 #include <linux/kobject.h>
 #include <asm/uaccess.h>
 #include <linux/moduleparam.h>
+#include <linux/pci.h>
 
 #include "acpiphp.h"
+#include "../pci.h"
 
 #define DRIVER_VERSION	"1.0.1"
 #define DRIVER_AUTHOR	"Irene Zubarev <zubarev@us.ibm.com>, Vernon Mauery <vernux@us.ibm.com>"
@@ -430,7 +432,7 @@ static int __init ibm_acpiphp_init(void)
 	int retval = 0;
 	acpi_status status;
 	struct acpi_device *device;
-	struct kobject *sysdir = &pci_hotplug_slots_kset->kobj;
+	struct kobject *sysdir = &pci_slots_kset->kobj;
 
 	dbg("%s\n", __FUNCTION__);
 
@@ -477,7 +479,7 @@ init_return:
 static void __exit ibm_acpiphp_exit(void)
 {
 	acpi_status status;
-	struct kobject *sysdir = &pci_hotplug_slots_kset->kobj;
+	struct kobject *sysdir = &pci_slots_kset->kobj;
 
 	dbg("%s\n", __FUNCTION__);
 
diff --git a/drivers/pci/hotplug/cpci_hotplug_core.c b/drivers/pci/hotplug/cpci_hotplug_core.c
index ed4d44e..aa47b80 100644
--- a/drivers/pci/hotplug/cpci_hotplug_core.c
+++ b/drivers/pci/hotplug/cpci_hotplug_core.c
@@ -285,7 +285,7 @@ cpci_hp_register_bus(struct pci_bus *bus, u8 first, u8 last)
 		info->attention_status = cpci_get_attention_status(slot);
 
 		dbg("registering slot %s", slot->hotplug_slot->name);
-		status = pci_hp_register(slot->hotplug_slot);
+		status = pci_hp_register(slot->hotplug_slot, bus, i);
 		if (status) {
 			err("pci_hp_register failed with error %d", status);
 			goto error_name;
diff --git a/drivers/pci/hotplug/cpqphp_core.c b/drivers/pci/hotplug/cpqphp_core.c
index 7417887..2e0392e 100644
--- a/drivers/pci/hotplug/cpqphp_core.c
+++ b/drivers/pci/hotplug/cpqphp_core.c
@@ -434,7 +434,9 @@ static int ctrl_slot_setup(struct controller *ctrl,
 				slot->bus, slot->device,
 				slot->number, ctrl->slot_device_offset,
 				slot_number);
-		result = pci_hp_register(hotplug_slot);
+		result = pci_hp_register(hotplug_slot,
+					 ctrl->pci_dev->subordinate,
+					 slot->device);
 		if (result) {
 			err("pci_hp_register failed with error %d\n", result);
 			goto error_name;
diff --git a/drivers/pci/hotplug/fakephp.c b/drivers/pci/hotplug/fakephp.c
index 6c14b4d..2d84755 100644
--- a/drivers/pci/hotplug/fakephp.c
+++ b/drivers/pci/hotplug/fakephp.c
@@ -127,7 +127,7 @@ static int add_slot(struct pci_dev *dev)
 	slot->release = &dummy_release;
 	slot->private = dslot;
 
-	retval = pci_hp_register(slot);
+	retval = pci_hp_register(slot, dev->bus, PCI_SLOT(dev->devfn));
 	if (retval) {
 		err("pci_hp_register failed with error %d\n", retval);
 		goto error_dslot;
diff --git a/drivers/pci/hotplug/ibmphp_ebda.c b/drivers/pci/hotplug/ibmphp_ebda.c
index bbccde9..17a5cfe 100644
--- a/drivers/pci/hotplug/ibmphp_ebda.c
+++ b/drivers/pci/hotplug/ibmphp_ebda.c
@@ -1001,7 +1001,8 @@ static int __init ebda_rsrc_controller (void)
 		tmp_slot = list_entry (list, struct slot, ibm_slot_list);
 
 		snprintf (tmp_slot->hotplug_slot->name, 30, "%s", create_file_name (tmp_slot));
-		pci_hp_register (tmp_slot->hotplug_slot);
+		pci_hp_register(tmp_slot->hotplug_slot,
+			pci_find_bus(0, tmp_slot->bus), tmp_slot->device);
 	}
 
 	print_ebda_hpc ();
diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index dd59a05..21bbb5e 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -40,6 +40,7 @@
 #include <linux/pci.h>
 #include <linux/pci_hotplug.h>
 #include <asm/uaccess.h>
+#include "../pci.h"
 
 #define MY_NAME	"pci_hotplug"
 
@@ -61,41 +62,6 @@ static int debug;
 
 static LIST_HEAD(pci_hotplug_slot_list);
 
-struct kset *pci_hotplug_slots_kset;
-
-static ssize_t hotplug_slot_attr_show(struct kobject *kobj,
-		struct attribute *attr, char *buf)
-{
-	struct hotplug_slot *slot = to_hotplug_slot(kobj);
-	struct hotplug_slot_attribute *attribute = to_hotplug_attr(attr);
-	return attribute->show ? attribute->show(slot, buf) : -EIO;
-}
-
-static ssize_t hotplug_slot_attr_store(struct kobject *kobj,
-		struct attribute *attr, const char *buf, size_t len)
-{
-	struct hotplug_slot *slot = to_hotplug_slot(kobj);
-	struct hotplug_slot_attribute *attribute = to_hotplug_attr(attr);
-	return attribute->store ? attribute->store(slot, buf, len) : -EIO;
-}
-
-static struct sysfs_ops hotplug_slot_sysfs_ops = {
-	.show = hotplug_slot_attr_show,
-	.store = hotplug_slot_attr_store,
-};
-
-static void hotplug_slot_release(struct kobject *kobj)
-{
-	struct hotplug_slot *slot = to_hotplug_slot(kobj);
-	if (slot->release)
-		slot->release(slot);
-}
-
-static struct kobj_type hotplug_slot_ktype = {
-	.sysfs_ops = &hotplug_slot_sysfs_ops,
-	.release = &hotplug_slot_release,
-};
-
 /* these strings match up with the values in pci_bus_speed */
 static char *pci_bus_speed_strings[] = {
 	"33 MHz PCI",		/* 0x00 */
@@ -149,16 +115,15 @@ GET_STATUS(power_status, u8)
 GET_STATUS(attention_status, u8)
 GET_STATUS(latch_status, u8)
 GET_STATUS(adapter_status, u8)
-GET_STATUS(address, u32)
 GET_STATUS(max_bus_speed, enum pci_bus_speed)
 GET_STATUS(cur_bus_speed, enum pci_bus_speed)
 
-static ssize_t power_read_file (struct hotplug_slot *slot, char *buf)
+static ssize_t power_read_file(struct pci_slot *slot, char *buf)
 {
 	int retval;
 	u8 value;
 
-	retval = get_power_status (slot, &value);
+	retval = get_power_status(slot->hotplug, &value);
 	if (retval)
 		goto exit;
 	retval = sprintf (buf, "%d\n", value);
@@ -166,9 +131,10 @@ exit:
 	return retval;
 }
 
-static ssize_t power_write_file (struct hotplug_slot *slot, const char *buf,
+static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
 		size_t count)
 {
+	struct hotplug_slot *slot = pci_slot->hotplug;
 	unsigned long lpower;
 	u8 power;
 	int retval = 0;
@@ -204,29 +170,30 @@ exit:
 	return count;
 }
 
-static struct hotplug_slot_attribute hotplug_slot_attr_power = {
+static struct pci_slot_attribute hotplug_slot_attr_power = {
 	.attr = {.name = "power", .mode = S_IFREG | S_IRUGO | S_IWUSR},
 	.show = power_read_file,
 	.store = power_write_file
 };
 
-static ssize_t attention_read_file (struct hotplug_slot *slot, char *buf)
+static ssize_t attention_read_file(struct pci_slot *slot, char *buf)
 {
 	int retval;
 	u8 value;
 
-	retval = get_attention_status (slot, &value);
+	retval = get_attention_status(slot->hotplug, &value);
 	if (retval)
 		goto exit;
-	retval = sprintf (buf, "%d\n", value);
+	retval = sprintf(buf, "%d\n", value);
 
 exit:
 	return retval;
 }
 
-static ssize_t attention_write_file (struct hotplug_slot *slot, const char *buf,
+static ssize_t attention_write_file(struct pci_slot *slot, const char *buf,
 		size_t count)
 {
+	struct hotplug_slot_ops *ops = slot->hotplug->ops;
 	unsigned long lattention;
 	u8 attention;
 	int retval = 0;
@@ -235,13 +202,13 @@ static ssize_t attention_write_file (struct hotplug_slot *slot, const char *buf,
 	attention = (u8)(lattention & 0xff);
 	dbg (" - attention = %d\n", attention);
 
-	if (!try_module_get(slot->ops->owner)) {
+	if (!try_module_get(ops->owner)) {
 		retval = -ENODEV;
 		goto exit;
 	}
-	if (slot->ops->set_attention_status)
-		retval = slot->ops->set_attention_status(slot, attention);
-	module_put(slot->ops->owner);
+	if (ops->set_attention_status)
+		retval = ops->set_attention_status(slot->hotplug, attention);
+	module_put(ops->owner);
 
 exit:	
 	if (retval)
@@ -249,18 +216,18 @@ exit:
 	return count;
 }
 
-static struct hotplug_slot_attribute hotplug_slot_attr_attention = {
+static struct pci_slot_attribute hotplug_slot_attr_attention = {
 	.attr = {.name = "attention", .mode = S_IFREG | S_IRUGO | S_IWUSR},
 	.show = attention_read_file,
 	.store = attention_write_file
 };
 
-static ssize_t latch_read_file (struct hotplug_slot *slot, char *buf)
+static ssize_t latch_read_file(struct pci_slot *slot, char *buf)
 {
 	int retval;
 	u8 value;
 
-	retval = get_latch_status (slot, &value);
+	retval = get_latch_status(slot->hotplug, &value);
 	if (retval)
 		goto exit;
 	retval = sprintf (buf, "%d\n", value);
@@ -269,17 +236,17 @@ exit:
 	return retval;
 }
 
-static struct hotplug_slot_attribute hotplug_slot_attr_latch = {
+static struct pci_slot_attribute hotplug_slot_attr_latch = {
 	.attr = {.name = "latch", .mode = S_IFREG | S_IRUGO},
 	.show = latch_read_file,
 };
 
-static ssize_t presence_read_file (struct hotplug_slot *slot, char *buf)
+static ssize_t presence_read_file(struct pci_slot *slot, char *buf)
 {
 	int retval;
 	u8 value;
 
-	retval = get_adapter_status (slot, &value);
+	retval = get_adapter_status(slot->hotplug, &value);
 	if (retval)
 		goto exit;
 	retval = sprintf (buf, "%d\n", value);
@@ -288,42 +255,20 @@ exit:
 	return retval;
 }
 
-static struct hotplug_slot_attribute hotplug_slot_attr_presence = {
+static struct pci_slot_attribute hotplug_slot_attr_presence = {
 	.attr = {.name = "adapter", .mode = S_IFREG | S_IRUGO},
 	.show = presence_read_file,
 };
 
-static ssize_t address_read_file (struct hotplug_slot *slot, char *buf)
-{
-	int retval;
-	u32 address;
-
-	retval = get_address (slot, &address);
-	if (retval)
-		goto exit;
-	retval = sprintf (buf, "%04x:%02x:%02x\n",
-			  (address >> 16) & 0xffff,
-			  (address >> 8) & 0xff,
-			  address & 0xff);
-
-exit:
-	return retval;
-}
-
-static struct hotplug_slot_attribute hotplug_slot_attr_address = {
-	.attr = {.name = "address", .mode = S_IFREG | S_IRUGO},
-	.show = address_read_file,
-};
-
 static char *unknown_speed = "Unknown bus speed";
 
-static ssize_t max_bus_speed_read_file (struct hotplug_slot *slot, char *buf)
+static ssize_t max_bus_speed_read_file(struct pci_slot *slot, char *buf)
 {
 	char *speed_string;
 	int retval;
 	enum pci_bus_speed value;
 	
-	retval = get_max_bus_speed (slot, &value);
+	retval = get_max_bus_speed(slot->hotplug, &value);
 	if (retval)
 		goto exit;
 
@@ -338,18 +283,18 @@ exit:
 	return retval;
 }
 
-static struct hotplug_slot_attribute hotplug_slot_attr_max_bus_speed = {
+static struct pci_slot_attribute hotplug_slot_attr_max_bus_speed = {
 	.attr = {.name = "max_bus_speed", .mode = S_IFREG | S_IRUGO},
 	.show = max_bus_speed_read_file,
 };
 
-static ssize_t cur_bus_speed_read_file (struct hotplug_slot *slot, char *buf)
+static ssize_t cur_bus_speed_read_file(struct pci_slot *slot, char *buf)
 {
 	char *speed_string;
 	int retval;
 	enum pci_bus_speed value;
 
-	retval = get_cur_bus_speed (slot, &value);
+	retval = get_cur_bus_speed(slot->hotplug, &value);
 	if (retval)
 		goto exit;
 
@@ -364,14 +309,15 @@ exit:
 	return retval;
 }
 
-static struct hotplug_slot_attribute hotplug_slot_attr_cur_bus_speed = {
+static struct pci_slot_attribute hotplug_slot_attr_cur_bus_speed = {
 	.attr = {.name = "cur_bus_speed", .mode = S_IFREG | S_IRUGO},
 	.show = cur_bus_speed_read_file,
 };
 
-static ssize_t test_write_file (struct hotplug_slot *slot, const char *buf,
+static ssize_t test_write_file(struct pci_slot *pci_slot, const char *buf,
 		size_t count)
 {
+	struct hotplug_slot *slot = pci_slot->hotplug;
 	unsigned long ltest;
 	u32 test;
 	int retval = 0;
@@ -394,13 +340,14 @@ exit:
 	return count;
 }
 
-static struct hotplug_slot_attribute hotplug_slot_attr_test = {
+static struct pci_slot_attribute hotplug_slot_attr_test = {
 	.attr = {.name = "test", .mode = S_IFREG | S_IRUGO | S_IWUSR},
 	.store = test_write_file
 };
 
-static int has_power_file (struct hotplug_slot *slot)
+static int has_power_file(struct pci_slot *pci_slot)
 {
+	struct hotplug_slot *slot = pci_slot->hotplug;
 	if ((!slot) || (!slot->ops))
 		return -ENODEV;
 	if ((slot->ops->enable_slot) ||
@@ -410,8 +357,9 @@ static int has_power_file (struct hotplug_slot *slot)
 	return -ENOENT;
 }
 
-static int has_attention_file (struct hotplug_slot *slot)
+static int has_attention_file(struct pci_slot *pci_slot)
 {
+	struct hotplug_slot *slot = pci_slot->hotplug;
 	if ((!slot) || (!slot->ops))
 		return -ENODEV;
 	if ((slot->ops->set_attention_status) ||
@@ -420,8 +368,9 @@ static int has_attention_file (struct hotplug_slot *slot)
 	return -ENOENT;
 }
 
-static int has_latch_file (struct hotplug_slot *slot)
+static int has_latch_file(struct pci_slot *pci_slot)
 {
+	struct hotplug_slot *slot = pci_slot->hotplug;
 	if ((!slot) || (!slot->ops))
 		return -ENODEV;
 	if (slot->ops->get_latch_status)
@@ -429,8 +378,9 @@ static int has_latch_file (struct hotplug_slot *slot)
 	return -ENOENT;
 }
 
-static int has_adapter_file (struct hotplug_slot *slot)
+static int has_adapter_file(struct pci_slot *pci_slot)
 {
+	struct hotplug_slot *slot = pci_slot->hotplug;
 	if ((!slot) || (!slot->ops))
 		return -ENODEV;
 	if (slot->ops->get_adapter_status)
@@ -438,17 +388,9 @@ static int has_adapter_file (struct hotplug_slot *slot)
 	return -ENOENT;
 }
 
-static int has_address_file (struct hotplug_slot *slot)
-{
-	if ((!slot) || (!slot->ops))
-		return -ENODEV;
-	if (slot->ops->get_address)
-		return 0;
-	return -ENOENT;
-}
-
-static int has_max_bus_speed_file (struct hotplug_slot *slot)
+static int has_max_bus_speed_file(struct pci_slot *pci_slot)
 {
+	struct hotplug_slot *slot = pci_slot->hotplug;
 	if ((!slot) || (!slot->ops))
 		return -ENODEV;
 	if (slot->ops->get_max_bus_speed)
@@ -456,8 +398,9 @@ static int has_max_bus_speed_file (struct hotplug_slot *slot)
 	return -ENOENT;
 }
 
-static int has_cur_bus_speed_file (struct hotplug_slot *slot)
+static int has_cur_bus_speed_file(struct pci_slot *pci_slot)
 {
+	struct hotplug_slot *slot = pci_slot->hotplug;
 	if ((!slot) || (!slot->ops))
 		return -ENODEV;
 	if (slot->ops->get_cur_bus_speed)
@@ -465,8 +408,9 @@ static int has_cur_bus_speed_file (struct hotplug_slot *slot)
 	return -ENOENT;
 }
 
-static int has_test_file (struct hotplug_slot *slot)
+static int has_test_file(struct pci_slot *pci_slot)
 {
+	struct hotplug_slot *slot = pci_slot->hotplug;
 	if ((!slot) || (!slot->ops))
 		return -ENODEV;
 	if (slot->ops->hardware_test)
@@ -474,7 +418,7 @@ static int has_test_file (struct hotplug_slot *slot)
 	return -ENOENT;
 }
 
-static int fs_add_slot (struct hotplug_slot *slot)
+static int fs_add_slot(struct pci_slot *slot)
 {
 	int retval = 0;
 
@@ -505,13 +449,6 @@ static int fs_add_slot (struct hotplug_slot *slot)
 			goto exit_adapter;
 	}
 
-	if (has_address_file(slot) == 0) {
-		retval = sysfs_create_file(&slot->kobj,
-					   &hotplug_slot_attr_address.attr);
-		if (retval)
-			goto exit_address;
-	}
-
 	if (has_max_bus_speed_file(slot) == 0) {
 		retval = sysfs_create_file(&slot->kobj,
 					   &hotplug_slot_attr_max_bus_speed.attr);
@@ -544,10 +481,6 @@ exit_cur_speed:
 		sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_max_bus_speed.attr);
 
 exit_max_speed:
-	if (has_address_file(slot) == 0)
-		sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_address.attr);
-
-exit_address:
 	if (has_adapter_file(slot) == 0)
 		sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_presence.attr);
 
@@ -567,7 +500,7 @@ exit:
 	return retval;
 }
 
-static void fs_remove_slot (struct hotplug_slot *slot)
+static void fs_remove_slot(struct pci_slot *slot)
 {
 	if (has_power_file(slot) == 0)
 		sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_power.attr);
@@ -581,9 +514,6 @@ static void fs_remove_slot (struct hotplug_slot *slot)
 	if (has_adapter_file(slot) == 0)
 		sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_presence.attr);
 
-	if (has_address_file(slot) == 0)
-		sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_address.attr);
-
 	if (has_max_bus_speed_file(slot) == 0)
 		sysfs_remove_file(&slot->kobj, &hotplug_slot_attr_max_bus_speed.attr);
 
@@ -607,6 +537,12 @@ static struct hotplug_slot *get_slot_from_name (const char *name)
 	return NULL;
 }
 
+static void hotplug_release(struct pci_slot *slot)
+{
+	struct hotplug_slot *hotplug = slot->hotplug;
+	hotplug->release(hotplug);
+}
+
 /**
  * pci_hp_register - register a hotplug_slot with the PCI hotplug subsystem
  * @slot: pointer to the &struct hotplug_slot to register
@@ -616,9 +552,10 @@ static struct hotplug_slot *get_slot_from_name (const char *name)
  *
  * Returns 0 if successful, anything else for an error.
  */
-int pci_hp_register (struct hotplug_slot *slot)
+int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
 {
 	int result;
+	struct pci_slot *pci_slot;
 
 	if (slot == NULL)
 		return -ENODEV;
@@ -630,20 +567,24 @@ int pci_hp_register (struct hotplug_slot *slot)
 		return -EINVAL;
 	}
 
-	/* this can fail if we have already registered a slot with the same name */
-	slot->kobj.kset = pci_hotplug_slots_kset;
-	result = kobject_init_and_add(&slot->kobj, &hotplug_slot_ktype, NULL,
-				      "%s", slot->name);
+	pci_slot = pci_create_slot(bus, slot_nr, slot->name);
+	if (IS_ERR(pci_slot))
+		return PTR_ERR(pci_slot);
+
+	result = pci_slot_add_hotplug(bus, slot_nr, hotplug_release);
 	if (result) {
-		err("Unable to register kobject '%s'", slot->name);
-		return -EINVAL;
+		pci_destroy_slot(pci_slot);
+		return result;
 	}
 
-	list_add (&slot->slot_list, &pci_hotplug_slot_list);
+	slot->pci_slot = pci_slot;
+	pci_slot->hotplug = slot;
+
+	list_add(&slot->slot_list, &pci_hotplug_slot_list);
 
-	result = fs_add_slot (slot);
-	kobject_uevent(&slot->kobj, KOBJ_ADD);
-	dbg ("Added slot %s to the list\n", slot->name);
+	result = fs_add_slot(pci_slot);
+	kobject_uevent(&pci_slot->kobj, KOBJ_ADD);
+	dbg("Added slot %s to the list\n", slot->name);
 	return result;
 }
 
@@ -656,22 +597,24 @@ int pci_hp_register (struct hotplug_slot *slot)
  *
  * Returns 0 if successful, anything else for an error.
  */
-int pci_hp_deregister (struct hotplug_slot *slot)
+int pci_hp_deregister(struct hotplug_slot *hotplug)
 {
 	struct hotplug_slot *temp;
+	struct pci_slot *slot;
 
-	if (slot == NULL)
+	if (!hotplug)
 		return -ENODEV;
 
-	temp = get_slot_from_name (slot->name);
-	if (temp != slot) {
+	temp = get_slot_from_name(hotplug->name);
+	if (temp != hotplug)
 		return -ENODEV;
-	}
-	list_del (&slot->slot_list);
 
-	fs_remove_slot (slot);
-	dbg ("Removed slot %s from the list\n", slot->name);
-	kobject_put(&slot->kobj);
+	list_del(&hotplug->slot_list);
+
+	slot = hotplug->pci_slot;
+	fs_remove_slot(slot);
+	pci_destroy_slot(slot);
+	dbg("Removed slot %s from the list\n", hotplug->name);
 	return 0;
 }
 
@@ -685,13 +628,15 @@ int pci_hp_deregister (struct hotplug_slot *slot)
  *
  * Returns 0 if successful, anything else for an error.
  */
-int __must_check pci_hp_change_slot_info(struct hotplug_slot *slot,
+int __must_check pci_hp_change_slot_info(struct hotplug_slot *hotplug,
 					 struct hotplug_slot_info *info)
 {
-	if ((slot == NULL) || (info == NULL))
+	struct pci_slot *slot;
+	if (!hotplug || !info)
 		return -ENODEV;
+	slot = hotplug->pci_slot;
 
-	memcpy (slot->info, info, sizeof (struct hotplug_slot_info));
+	memcpy(hotplug->info, info, sizeof(struct hotplug_slot_info));
 
 	return 0;
 }
@@ -699,36 +644,22 @@ int __must_check pci_hp_change_slot_info(struct hotplug_slot *slot,
 static int __init pci_hotplug_init (void)
 {
 	int result;
-	struct kset *pci_bus_kset;
 
-	pci_bus_kset = bus_get_kset(&pci_bus_type);
-
-	pci_hotplug_slots_kset = kset_create_and_add("slots", NULL,
-						     &pci_bus_kset->kobj);
-	if (!pci_hotplug_slots_kset) {
-		result = -ENOMEM;
-		err("Register subsys error\n");
-		goto exit;
-	}
 	result = cpci_hotplug_init(debug);
 	if (result) {
 		err ("cpci_hotplug_init with error %d\n", result);
-		goto err_subsys;
+		goto err_cpci;
 	}
 
 	info (DRIVER_DESC " version: " DRIVER_VERSION "\n");
-	goto exit;
 
-err_subsys:
-	kset_unregister(pci_hotplug_slots_kset);
-exit:
+err_cpci:
 	return result;
 }
 
 static void __exit pci_hotplug_exit (void)
 {
 	cpci_hotplug_exit();
-	kset_unregister(pci_hotplug_slots_kset);
 }
 
 module_init(pci_hotplug_init);
@@ -740,7 +671,6 @@ MODULE_LICENSE("GPL");
 module_param(debug, bool, 0644);
 MODULE_PARM_DESC(debug, "Debugging mode enabled or not");
 
-EXPORT_SYMBOL_GPL(pci_hotplug_slots_kset);
 EXPORT_SYMBOL_GPL(pci_hp_register);
 EXPORT_SYMBOL_GPL(pci_hp_deregister);
 EXPORT_SYMBOL_GPL(pci_hp_change_slot_info);
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 5fa4ba0..e6d3d4d 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -69,7 +69,6 @@ static int get_power_status	(struct hotplug_slot *slot, u8 *value);
 static int get_attention_status	(struct hotplug_slot *slot, u8 *value);
 static int get_latch_status	(struct hotplug_slot *slot, u8 *value);
 static int get_adapter_status	(struct hotplug_slot *slot, u8 *value);
-static int get_address		(struct hotplug_slot *slot, u32 *value);
 static int get_max_bus_speed	(struct hotplug_slot *slot, enum pci_bus_speed *value);
 static int get_cur_bus_speed	(struct hotplug_slot *slot, enum pci_bus_speed *value);
 
@@ -82,7 +81,6 @@ static struct hotplug_slot_ops pciehp_hotplug_slot_ops = {
 	.get_attention_status =	get_attention_status,
 	.get_latch_status =	get_latch_status,
 	.get_adapter_status =	get_adapter_status,
-	.get_address =		get_address,
   	.get_max_bus_speed =	get_max_bus_speed,
   	.get_cur_bus_speed =	get_cur_bus_speed,
 };
@@ -245,14 +243,18 @@ static int init_slots(struct controller *ctrl)
 		dbg("Registering bus=%x dev=%x hp_slot=%x sun=%x "
 		    "slot_device_offset=%x\n", slot->bus, slot->device,
 		    slot->hp_slot, slot->number, ctrl->slot_device_offset);
-		retval = pci_hp_register(hotplug_slot);
+		retval = pci_hp_register(hotplug_slot,
+					 ctrl->pci_dev->subordinate,
+					 slot->device);
+		if (retval == -EBUSY)
+			goto error_info;
 		if (retval) {
 			err ("pci_hp_register failed with error %d\n", retval);
 			goto error_info;
 		}
 		/* create additional sysfs entries */
 		if (EMI(ctrl->ctrlcap)) {
-			retval = sysfs_create_file(&hotplug_slot->kobj,
+			retval = sysfs_create_file(&hotplug_slot->pci_slot->kobj,
 				&hotplug_slot_attr_lock.attr);
 			if (retval) {
 				pci_hp_deregister(hotplug_slot);
@@ -285,7 +287,7 @@ static void cleanup_slots(struct controller *ctrl)
 		slot = list_entry(tmp, struct slot, slot_list);
 		list_del(&slot->slot_list);
 		if (EMI(ctrl->ctrlcap))
-			sysfs_remove_file(&slot->hotplug_slot->kobj,
+			sysfs_remove_file(&slot->hotplug_slot->pci_slot->kobj,
 				&hotplug_slot_attr_lock.attr);
 		cancel_delayed_work(&slot->work);
 		flush_scheduled_work();
@@ -387,18 +389,6 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
 	return 0;
 }
 
-static int get_address(struct hotplug_slot *hotplug_slot, u32 *value)
-{
-	struct slot *slot = hotplug_slot->private;
-	struct pci_bus *bus = slot->ctrl->pci_dev->subordinate;
-
-	dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot->name);
-
-	*value = (pci_domain_nr(bus) << 16) | (slot->bus << 8) | slot->device;
-
-	return 0;
-}
-
 static int get_max_bus_speed(struct hotplug_slot *hotplug_slot, enum pci_bus_speed *value)
 {
 	struct slot *slot = hotplug_slot->private;
@@ -460,7 +450,12 @@ static int pciehp_probe(struct pcie_device *dev, const struct pcie_port_service_
 	/* Setup the slot information structures */
 	rc = init_slots(ctrl);
 	if (rc) {
-		err("%s: slot initialization failed\n", PCIE_MODULE_NAME);
+		if (rc == -EBUSY)
+			warn("%s: slot already registered by another "
+				"hotplug driver\n", PCIE_MODULE_NAME);
+		else
+			err("%s: slot initialization failed\n",
+				PCIE_MODULE_NAME);
 		goto err_out_release_ctlr;
 	}
 
diff --git a/drivers/pci/hotplug/rpadlpar_sysfs.c b/drivers/pci/hotplug/rpadlpar_sysfs.c
index e32148a..399b196 100644
--- a/drivers/pci/hotplug/rpadlpar_sysfs.c
+++ b/drivers/pci/hotplug/rpadlpar_sysfs.c
@@ -14,8 +14,10 @@
  */
 #include <linux/kobject.h>
 #include <linux/string.h>
+#include <linux/pci.h>
 #include <linux/pci_hotplug.h>
 #include "rpadlpar.h"
+#include "../pcih."
 
 #define DLPAR_KOBJ_NAME       "control"
 #define ADD_SLOT_ATTR_NAME    "add_slot"
@@ -23,7 +25,6 @@
 
 #define MAX_DRC_NAME_LEN 64
 
-
 static ssize_t add_slot_store(struct kobject *kobj, struct kobj_attribute *attr,
 			      const char *buf, size_t nbytes)
 {
@@ -108,7 +109,7 @@ int dlpar_sysfs_init(void)
 	int error;
 
 	dlpar_kobj = kobject_create_and_add(DLPAR_KOBJ_NAME,
-					    &pci_hotplug_slots_kset->kobj);
+					    &pci_slots_kset->kobj);
 	if (!dlpar_kobj)
 		return -EINVAL;
 
diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c
index 8ad3deb..8e5fff0 100644
--- a/drivers/pci/hotplug/rpaphp_slot.c
+++ b/drivers/pci/hotplug/rpaphp_slot.c
@@ -162,7 +162,8 @@ int rpaphp_register_slot(struct slot *slot)
 		return -EAGAIN;
 	}	
 
-	retval = pci_hp_register(php_slot);
+	retval = pci_hp_register(php_slot, slot->bus,
+				 PCI_SLOT(PCI_DN(slot->dn->child)->devfn));
 	if (retval) {
 		err("pci_hp_register failed with error %d\n", retval);
 		return retval;
diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index ef07c36..8908834 100644
--- a/drivers/pci/hotplug/sgi_hotplug.c
+++ b/drivers/pci/hotplug/sgi_hotplug.c
@@ -197,13 +197,15 @@ static int sn_hp_slot_private_alloc(struct hotplug_slot *bss_hotplug_slot,
 static struct hotplug_slot * sn_hp_destroy(void)
 {
 	struct slot *slot;
+	struct pci_slot *pci_slot;
 	struct hotplug_slot *bss_hotplug_slot = NULL;
 
 	list_for_each_entry(slot, &sn_hp_list, hp_list) {
 		bss_hotplug_slot = slot->hotplug_slot;
+		pci_slot = bss_hotplug_slot->pci_slot;
 		list_del(&((struct slot *)bss_hotplug_slot->private)->
 			 hp_list);
-		sysfs_remove_file(&bss_hotplug_slot->kobj,
+		sysfs_remove_file(&pci_slot->kobj,
 				  &sn_slot_path_attr.attr);
 		break;
 	}
@@ -614,6 +616,7 @@ static void sn_release_slot(struct hotplug_slot *bss_hotplug_slot)
 static int sn_hotplug_slot_register(struct pci_bus *pci_bus)
 {
 	int device;
+	struct pci_slot *pci_slot;
 	struct hotplug_slot *bss_hotplug_slot;
 	int rc = 0;
 
@@ -650,11 +653,12 @@ static int sn_hotplug_slot_register(struct pci_bus *pci_bus)
 		bss_hotplug_slot->ops = &sn_hotplug_slot_ops;
 		bss_hotplug_slot->release = &sn_release_slot;
 
-		rc = pci_hp_register(bss_hotplug_slot);
+		rc = pci_hp_register(bss_hotplug_slot, pci_bus, device);
 		if (rc)
 			goto register_err;
 
-		rc = sysfs_create_file(&bss_hotplug_slot->kobj,
+		pci_slot = bss_hotplug_slot->pci_slot;
+		rc = sysfs_create_file(&pci_slot->kobj,
 				       &sn_slot_path_attr.attr);
 		if (rc)
 			goto register_err;
diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
index 80dec97..22c4d2e 100644
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -65,7 +65,6 @@ static int get_power_status	(struct hotplug_slot *slot, u8 *value);
 static int get_attention_status	(struct hotplug_slot *slot, u8 *value);
 static int get_latch_status	(struct hotplug_slot *slot, u8 *value);
 static int get_adapter_status	(struct hotplug_slot *slot, u8 *value);
-static int get_address		(struct hotplug_slot *slot, u32 *value);
 static int get_max_bus_speed	(struct hotplug_slot *slot, enum pci_bus_speed *value);
 static int get_cur_bus_speed	(struct hotplug_slot *slot, enum pci_bus_speed *value);
 
@@ -78,7 +77,6 @@ static struct hotplug_slot_ops shpchp_hotplug_slot_ops = {
 	.get_attention_status =	get_attention_status,
 	.get_latch_status =	get_latch_status,
 	.get_adapter_status =	get_adapter_status,
-	.get_address =		get_address,
 	.get_max_bus_speed =	get_max_bus_speed,
 	.get_cur_bus_speed =	get_cur_bus_speed,
 };
@@ -152,7 +150,8 @@ static int init_slots(struct controller *ctrl)
 		dbg("Registering bus=%x dev=%x hp_slot=%x sun=%x "
 		    "slot_device_offset=%x\n", slot->bus, slot->device,
 		    slot->hp_slot, slot->number, ctrl->slot_device_offset);
-		retval = pci_hp_register(slot->hotplug_slot);
+		retval = pci_hp_register(slot->hotplug_slot,
+				ctrl->pci_dev->subordinate, slot->device);
 		if (retval) {
 			err("pci_hp_register failed with error %d\n", retval);
 			goto error_info;
@@ -277,18 +276,6 @@ static int get_adapter_status (struct hotplug_slot *hotplug_slot, u8 *value)
 	return 0;
 }
 
-static int get_address (struct hotplug_slot *hotplug_slot, u32 *value)
-{
-	struct slot *slot = get_slot(hotplug_slot);
-	struct pci_bus *bus = slot->ctrl->pci_dev->subordinate;
-
-	dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot->name);
-
-	*value = (pci_domain_nr(bus) << 16) | (slot->bus << 8) | slot->device;
-
-	return 0;
-}
-
 static int get_max_bus_speed (struct hotplug_slot *hotplug_slot, enum pci_bus_speed *value)
 {
 	struct slot *slot = get_slot(hotplug_slot);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index eabeb1f..61bb743 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -87,3 +87,16 @@ pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
 }
 
 struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
+
+/* PCI slot sysfs helper code */
+#define to_pci_slot(s) container_of(s, struct pci_slot, kobj)
+
+extern struct kset *pci_slots_kset;
+
+struct pci_slot_attribute {
+	struct attribute attr;
+	ssize_t (*show)(struct pci_slot *, char *);
+	ssize_t (*store)(struct pci_slot *, const char *, size_t);
+};
+#define to_pci_slot_attr(s) container_of(s, struct pci_slot_attribute, attr)
+
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
new file mode 100644
index 0000000..5de7b7b
--- /dev/null
+++ b/drivers/pci/slot.c
@@ -0,0 +1,224 @@
+/*
+ * drivers/pci/slot.c
+ * Copyright (C) 2006 Matthew Wilcox <matthew@wil.cx>
+ * Copyright (C) 2006,2007 Hewlett-Packard Development Company, L.P.
+ * Copyright (C) 2007 Alex Chiang <achiang@hp.com>
+ */
+
+#include <linux/kobject.h>
+#include <linux/pci.h>
+#include "pci.h"
+
+static int pci_slot_debug;
+#define MY_NAME "slot"
+#define dbg(format, arg...)					\
+	do {							\
+		if (pci_slot_debug)				\
+			printk(KERN_DEBUG "%s: " format,	\
+				MY_NAME , ## arg);		\
+	} while (0)
+
+struct kset *pci_slots_kset;
+EXPORT_SYMBOL_GPL(pci_slots_kset);
+
+static ssize_t pci_slot_attr_show(struct kobject *kobj,
+					struct attribute *attr, char *buf)
+{
+	struct pci_slot *slot = to_pci_slot(kobj);
+	struct pci_slot_attribute *attribute = to_pci_slot_attr(attr);
+	return attribute->show ? attribute->show(slot, buf) : -EIO;
+}
+
+static ssize_t pci_slot_attr_store(struct kobject *kobj,
+			struct attribute *attr, const char *buf, size_t len)
+{
+	struct pci_slot *slot = to_pci_slot(kobj);
+	struct pci_slot_attribute *attribute = to_pci_slot_attr(attr);
+	return attribute->store ? attribute->store(slot, buf, len) : -EIO;
+}
+
+static struct sysfs_ops pci_slot_sysfs_ops = {
+	.show = pci_slot_attr_show,
+	.store = pci_slot_attr_store,
+};
+
+static ssize_t address_read_file(struct pci_slot *slot, char *buf)
+{
+	return sprintf(buf, "%04x:%02x:%02x\n", pci_domain_nr(slot->bus),
+					slot->bus->number, slot->number);
+}
+
+static struct pci_slot_attribute pci_slot_attr_address = {
+	.attr = { .name = "address", .mode = S_IFREG | S_IRUGO },
+	.show = address_read_file,
+};
+
+static void remove_sysfs_files(struct pci_slot *slot)
+{
+	sysfs_remove_file(&slot->kobj, &pci_slot_attr_address.attr);
+}
+
+static int create_sysfs_files(struct pci_slot *slot)
+{
+	int result;
+
+	result = sysfs_create_file(&slot->kobj, &pci_slot_attr_address.attr);
+
+	return result;
+}
+
+static void pci_slot_release(struct kobject *kobj)
+{
+	struct pci_slot **pprev;
+	struct pci_slot *slot = to_pci_slot(kobj);
+
+	dbg("%s: releasing pci_slot on %x:%d\n", __func__,
+		slot->bus->number, slot->number);
+
+	for (pprev = &slot->bus->slot; *pprev; pprev = &(*pprev)->next) {
+		if (*pprev == slot) {
+			*pprev = slot->next;
+			break;
+		}
+	}
+
+	if (slot->release)
+		slot->release(slot);
+
+	remove_sysfs_files(slot);
+	kfree(slot);
+}
+
+static struct kobj_type pci_slot_ktype = {
+	.sysfs_ops = &pci_slot_sysfs_ops,
+	.release = &pci_slot_release,
+};
+
+int pci_slot_add_hotplug(struct pci_bus *parent, int slot_nr,
+			 void (*release)(struct pci_slot *))
+{
+	struct pci_slot *slot;
+	int retval, found;
+
+	retval = found = 0;
+
+	down_write(&pci_bus_sem);
+
+	/* This slot should have already been created, so look for it. If
+	 * we can't find it, return -EEXIST.
+	 */
+	for (slot = parent->slot; slot; slot = slot->next)
+		if (slot->number == slot_nr) {
+			found = 1;
+			break;
+		}
+
+	if (!found) {
+		dbg("%s: slot not found\n", __func__);
+		retval = -EEXIST;
+		goto out;
+	}
+
+	if (slot->release) {
+		dbg("%s: already claimed\n", __func__);
+		retval = -EBUSY;
+		goto out;
+	}
+
+	dbg("%s: adding release function to %x:%d\n",
+		__func__, parent->number, slot_nr);
+	slot->release = release;
+ out:
+	up_write(&pci_bus_sem);
+	return retval;
+}
+EXPORT_SYMBOL_GPL(pci_slot_add_hotplug);
+
+struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
+				 const char *name)
+{
+	struct pci_slot *slot;
+	int err;
+
+	down_write(&pci_bus_sem);
+
+	/* If we've already created this slot, bump refcount and return. */
+	for (slot = parent->slot; slot; slot = slot->next) {
+		if (slot->number == slot_nr) {
+			kobject_get(&slot->kobj);
+			dbg("%s: bumped refcount to %d on %x:%d\n",
+				__func__, slot->kobj.kref.refcount,
+				parent->number, slot_nr);
+			goto out;
+		}
+	}
+
+	slot = kzalloc(sizeof(*slot), GFP_KERNEL);
+	if (!slot) {
+		slot = ERR_PTR(-ENOMEM);
+		goto out;
+	}
+
+	slot->bus = parent;
+	slot->number = slot_nr;
+
+	slot->kobj.kset = pci_slots_kset;
+	err = kobject_init_and_add(&slot->kobj, &pci_slot_ktype, NULL,
+				   "%s", name);
+	if (err) {
+		printk(KERN_ERR "Unable to register kobject %s\n", name);
+		err = -EINVAL;
+		goto err;
+	}
+
+	err = create_sysfs_files(slot);
+	if (err)
+		goto unregister;
+
+	slot->next = parent->slot;
+	parent->slot = slot;
+
+	dbg("%s: created pci_slot on %x:%d\n",
+		__func__, parent->number, slot_nr);
+
+ out:
+	up_write(&pci_bus_sem);
+	return slot;
+
+ unregister:
+	kobject_put(&slot->kobj);
+ err:
+	kfree(slot);
+	slot = ERR_PTR(err);
+	goto out;
+}
+EXPORT_SYMBOL_GPL(pci_create_slot);
+
+int pci_destroy_slot(struct pci_slot *slot)
+{
+	kobject_put(&slot->kobj);
+
+	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
+		slot->kobj.kref.refcount, slot->bus->number, slot->number);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_destroy_slot);
+
+static int pci_slot_init(void)
+{
+	int result = 0;
+	struct kset *pci_bus_kset;
+
+	pci_bus_kset = bus_get_kset(&pci_bus_type);
+
+	pci_slots_kset = kset_create_and_add("slots", NULL,
+						&pci_bus_kset->kobj);
+	if (!pci_slots_kset) {
+		result = -ENOMEM;
+		printk(KERN_ERR "PCI: Slot initialization failure (%d)\n",
+			result);
+	}
+	return result;
+}
+
+subsys_initcall(pci_slot_init);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b7e4b63..6e35cd6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -128,6 +128,16 @@ struct pci_cap_saved_state {
 	u32 data[0];
 };
 
+/* pci_slot represents a physical slot */
+struct pci_slot {
+	struct pci_bus *bus;		/* The bus this slot is on */
+	struct pci_slot *next;		/* Next slot on this bus */
+	struct hotplug_slot *hotplug;	/* Hotplug info (migrate over time) */
+	unsigned char number;		/* PCI_SLOT(pci_dev->devfn) */
+	struct kobject kobj;
+	void (*release)(struct pci_slot *);
+};
+
 /*
  * The pci_dev structure is used to describe PCI devices.
  */
@@ -139,6 +149,7 @@ struct pci_dev {
 
 	void		*sysdata;	/* hook for sys-specific extension */
 	struct proc_dir_entry *procent;	/* device entry in /proc/bus/pci */
+	struct pci_slot	*slot;		/* Physical slot this device is in */
 
 	unsigned int	devfn;		/* encoded device & function index */
 	unsigned short	vendor;
@@ -258,6 +269,7 @@ struct pci_bus {
 	struct list_head children;	/* list of child buses */
 	struct list_head devices;	/* list of devices on this bus */
 	struct pci_dev	*self;		/* bridge device as seen by parent */
+	struct pci_slot	*slot;		/* First physical slot on this bus */
 	struct resource	*resource[PCI_BUS_NUM_RESOURCES];
 					/* address space routed to this bus */
 
@@ -481,6 +493,11 @@ struct pci_bus *pci_create_bus(struct device *parent, int bus,
 			       struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
 				int busnr);
+struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
+				 const char *name);
+int pci_slot_add_hotplug(struct pci_bus *parent, int slot_nr,
+			 void (*release)(struct pci_slot *));
+int pci_destroy_slot(struct pci_slot *slot);
 int pci_scan_slot(struct pci_bus *bus, int devfn);
 struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn);
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus);
diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index 8f67e8f..bb36c59 100644
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -95,9 +95,6 @@ struct hotplug_slot_attribute {
  * @get_adapter_status: Called to get see if an adapter is present in the slot or not.
  *	If this field is NULL, the value passed in the struct hotplug_slot_info
  *	will be used when this value is requested by a user.
- * @get_address: Called to get pci address of a slot.
- *	If this field is NULL, the value passed in the struct hotplug_slot_info
- *	will be used when this value is requested by a user.
  * @get_max_bus_speed: Called to get the max bus speed for a slot.
  *	If this field is NULL, the value passed in the struct hotplug_slot_info
  *	will be used when this value is requested by a user.
@@ -120,7 +117,6 @@ struct hotplug_slot_ops {
 	int (*get_attention_status)	(struct hotplug_slot *slot, u8 *value);
 	int (*get_latch_status)		(struct hotplug_slot *slot, u8 *value);
 	int (*get_adapter_status)	(struct hotplug_slot *slot, u8 *value);
-	int (*get_address)		(struct hotplug_slot *slot, u32 *value);
 	int (*get_max_bus_speed)	(struct hotplug_slot *slot, enum pci_bus_speed *value);
 	int (*get_cur_bus_speed)	(struct hotplug_slot *slot, enum pci_bus_speed *value);
 };
@@ -140,7 +136,6 @@ struct hotplug_slot_info {
 	u8	attention_status;
 	u8	latch_status;
 	u8	adapter_status;
-	u32	address;
 	enum pci_bus_speed	max_bus_speed;
 	enum pci_bus_speed	cur_bus_speed;
 };
@@ -166,15 +161,14 @@ struct hotplug_slot {
 
 	/* Variables below this are for use only by the hotplug pci core. */
 	struct list_head		slot_list;
-	struct kobject			kobj;
+	struct pci_slot			*pci_slot;
 };
 #define to_hotplug_slot(n) container_of(n, struct hotplug_slot, kobj)
 
-extern int pci_hp_register		(struct hotplug_slot *slot);
-extern int pci_hp_deregister		(struct hotplug_slot *slot);
+extern int pci_hp_register(struct hotplug_slot *, struct pci_bus *, int nr);
+extern int pci_hp_deregister(struct hotplug_slot *slot);
 extern int __must_check pci_hp_change_slot_info	(struct hotplug_slot *slot,
 						 struct hotplug_slot_info *info);
-extern struct kset *pci_hotplug_slots_kset;
 
 /* PCI Setting Record (Type 0) */
 struct hpp_type0 {
-- 
1.5.3.1.g1e61

[PATCH 3/3] ACPI PCI slot detection driver

[Alex Chiang]

Detect all physical PCI slots as described by ACPI, and create
entries in /sys/bus/pci/slots/.

Not all physical slots are hotpluggable, and the acpiphp module
does not detect them. Now we know the physical PCI geography of
our system, without caring about hotplug.


v9 -> v10:
	Allow an hp driver to override the pci_slot kobject name.

v8 -> v9:
	Add DMI quirk for Fujitsu machines; eval _STA before _SUN

v6 -> v8:
	No change

v5 -> v6:
	Add debugging information.

v4 -> v5:
	Convert to a tristate module.

	Remove #ifdef CONFIG_ACPI_PCI_SLOT, as struct pci_slot
	objects are properly refcounted, and multiple calls to
	pci_create/destroy_slot work just fine.

	Remove prior -EBUSY changes, as they have been folded
	into the Introduce pci_slot patch.

v3 -> v4:
	Always attempt to call pci_create_slot from pcihp_core to
	cover cases of
		a) user did not say Y to Kconfig option ACPI_PCI_SLOT
		b) native PCIe hotplug driver registering on a
		non-ACPI system.

	Return -EBUSY if an hp driver attempts to register a slot
	that is already registered to another driver. Do not
	consider that to be an error condition in acpiphp and pciehp.

v2 -> v3:
	Add Kconfig option to driver, allowing users to [de]config
	this driver. If configured, take slightly different code
	paths in pci_hp_register and pci_hp_deregister.

v1 -> v2:
	Now recursively discovering p2p bridges and slots
	underneath them. Hopefully, this will prevent us
	from trying to register the same slot multiple times.

Signed-off-by: Alex Chiang <achiang@hp.com>
---
 drivers/acpi/Kconfig                   |    9 +
 drivers/acpi/Makefile                  |    1 +
 drivers/acpi/pci_slot.c                |  360 ++++++++++++++++++++++++++++++++
 drivers/pci/hotplug/pci_hotplug_core.c |   22 ++-
 drivers/pci/hotplug/pciehp_core.c      |    2 +-
 drivers/pci/hotplug/sgi_hotplug.c      |    2 +-
 6 files changed, 393 insertions(+), 3 deletions(-)
 create mode 100644 drivers/acpi/pci_slot.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b4f5e85..4ccb0e7 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -335,6 +335,15 @@ config ACPI_EC
 	  the battery and thermal drivers.  If you are compiling for a 
 	  mobile system, say Y.
 
+config ACPI_PCI_SLOT
+	tristate "PCI slot detection driver"
+	default n
+	help
+	  This driver will attempt to discover all PCI slots in your system,
+	  and creates entries in /sys/bus/pci/slots/. This feature can
+	  help you correlate PCI bus addresses with the physical geography
+	  of your slots. If you are unsure, say N.
+
 config ACPI_POWER
 	bool
 	default y
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 40b0fca..579c29c 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_ACPI_DOCK)		+= dock.o
 obj-$(CONFIG_ACPI_BAY)		+= bay.o
 obj-$(CONFIG_ACPI_VIDEO)	+= video.o
 obj-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o
+obj-$(CONFIG_ACPI_PCI_SLOT)	+= pci_slot.o
 obj-$(CONFIG_ACPI_POWER)	+= power.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-$(CONFIG_ACPI_CONTAINER)	+= container.o
diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
new file mode 100644
index 0000000..4134d04
--- /dev/null
+++ b/drivers/acpi/pci_slot.c
@@ -0,0 +1,360 @@
+/*
+ *  pci_slot.c - ACPI PCI Slot Driver
+ *
+ *  The code here is heavily leveraged from the acpiphp module.
+ *  Thanks to Matthew Wilcox <matthew@wil.cx> for much guidance.
+ *
+ *  Copyright (C) 2007 Alex Chiang <achiang@hp.com>
+ *  Copyright (C) 2007 Hewlett-Packard Development Company, L.P.
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms and conditions of the GNU General Public License,
+ *  version 2, as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/pci.h>
+#include <linux/acpi.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+
+static int debug;
+static int check_sta_before_sun;
+
+#define DRIVER_VERSION 	"0.1"
+#define DRIVER_AUTHOR	"Alex Chiang <achiang@hp.com>"
+#define DRIVER_DESC	"ACPI PCI Slot Detection Driver"
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+MODULE_PARM_DESC(debug, "Debugging mode enabled or not");
+module_param(debug, bool, 0644);
+
+#define _COMPONENT		ACPI_PCI_COMPONENT
+ACPI_MODULE_NAME("pci_slot");
+
+#define MY_NAME "pci_slot"
+#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 dbg(format, arg...)					\
+	do {							\
+		if (debug)					\
+			printk(KERN_DEBUG "%s: " format,	\
+				MY_NAME , ## arg);		\
+	} while (0)
+
+static int acpi_pci_slot_add(acpi_handle handle);
+static void acpi_pci_slot_remove(acpi_handle handle);
+
+static struct acpi_pci_driver acpi_pci_slot_driver = {
+	.add = acpi_pci_slot_add,
+	.remove = acpi_pci_slot_remove,
+};
+
+static int
+check_slot(acpi_handle handle, int *device, unsigned long *sun)
+{
+	int retval = 0;
+	unsigned long adr, sta;
+	acpi_status status;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+
+	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+	dbg("Checking slot on path: %s\n", (char *)buffer.pointer);
+
+	if (check_sta_before_sun) {
+		/* If SxFy doesn't have _STA, we just assume it's there */
+		acpi_evaluate_integer(handle, "_STA", NULL, &sta);
+
+		if (!(sta & ACPI_STA_DEVICE_PRESENT)) {
+			retval = -1;
+			goto out;
+		}
+	}
+
+	status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
+	if (ACPI_FAILURE(status)) {
+		dbg("_ADR returned %d on %s\n", status, (char *)buffer.pointer);
+		retval = -1;
+		goto out;
+	}
+
+	*device = (adr >> 16) & 0xffff;
+
+	/* No _SUN == not a slot == bail */
+	status = acpi_evaluate_integer(handle, "_SUN", NULL, sun);
+	if (ACPI_FAILURE(status)) {
+		dbg("_SUN returned %d on %s\n", status, (char *)buffer.pointer);
+		retval = -1;
+		goto out;
+	}
+
+out:
+	kfree(buffer.pointer);
+	return retval;
+}
+
+/*
+ * unregister_slot
+ *
+ * Called once for each SxFy object in the namespace. Each call to
+ * pci_destroy_slot decrements the refcount on the pci_slot, and
+ * eventually calls kobject_unregister at the appropriate time.
+ */
+static acpi_status
+unregister_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	int device;
+	unsigned long sun;
+	struct pci_slot *slot;
+	struct pci_bus *pci_bus = context;
+
+	if (check_slot(handle, &device, &sun))
+		return AE_OK;
+
+	for (slot = pci_bus->slot; slot; slot = slot->next) {
+		if (slot->number == device)
+			pci_destroy_slot(slot);
+	}
+
+	return AE_OK;
+}
+
+/*
+ * register_slot
+ *
+ * Called once for each SxFy object in the namespace. Don't worry about
+ * calling pci_create_slot multiple times for the same pci_bus:device,
+ * since each subsequent call simply bumps the refcount on the pci_slot.
+ *
+ * The number of calls to pci_destroy_slot from unregister_slot is
+ * symmetrical.
+ */
+static acpi_status
+register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	int device;
+	unsigned long sun;
+	char name[KOBJ_NAME_LEN];
+
+	struct pci_slot *pci_slot;
+	struct pci_bus *pci_bus = context;
+
+	if (check_slot(handle, &device, &sun))
+		return AE_OK;
+
+	snprintf(name, sizeof(name), "%u", (u32)sun);
+	pci_slot = pci_create_slot(pci_bus, device, name);
+	if (IS_ERR(pci_slot))
+		err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
+
+	dbg("pci_slot: %#lx, pci_bus: %x, device: %d, name: %s\n",
+		(uint64_t)pci_slot, pci_bus->number, device, name);
+
+	return AE_OK;
+}
+
+struct p2p_bridge_context {
+	acpi_walk_callback	user_function;
+	struct pci_bus		*pci_bus;
+};
+
+/*
+ * walk_p2p_bridge - discover and walk p2p bridges
+ * @handle: points to an acpi_pci_root
+ * @context: p2p_bridge_context pointer
+ *
+ * Note that when we call ourselves recursively, we pass a different
+ * value of pci_bus in the child_context.
+ */
+static acpi_status
+walk_p2p_bridge(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	int device, function;
+	unsigned long adr;
+	acpi_status status;
+	acpi_handle dummy_handle;
+	acpi_walk_callback user_function;
+
+	struct pci_dev *dev;
+	struct pci_bus *pci_bus;
+	struct p2p_bridge_context child_context;
+	struct p2p_bridge_context *parent_context = context;
+
+	pci_bus = parent_context->pci_bus;
+	user_function = parent_context->user_function;
+
+	status = acpi_get_handle(handle, "_ADR", &dummy_handle);
+	if (ACPI_FAILURE(status))
+		return AE_OK;
+
+	status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
+	if (ACPI_FAILURE(status))
+		return AE_OK;
+
+	device = (adr >> 16) & 0xffff;
+	function = adr & 0xffff;
+
+	dev = pci_get_slot(pci_bus, PCI_DEVFN(device, function));
+	if (!dev || !dev->subordinate)
+		goto out;
+
+	dbg("p2p bridge walk, pci_bus = %x\n", dev->subordinate->number);
+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
+				     user_function, dev->subordinate, NULL);
+	if (ACPI_FAILURE(status))
+		goto out;
+
+	child_context.pci_bus = dev->subordinate;
+	child_context.user_function = user_function;
+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
+				     walk_p2p_bridge, &child_context, NULL);
+out:
+	pci_dev_put(dev);
+	return AE_OK;
+}
+
+#define ACPI_STA_FUNCTIONING            (0x00000008)
+
+/*
+ * walk_root_bridge - generic root bridge walker
+ * @handle: points to an acpi_pci_root
+ * @user_function: user callback for slot objects
+ *
+ * Call user_function for all objects underneath this root bridge.
+ * Walk p2p bridges underneath us and call user_function on those too.
+ */
+static int
+walk_root_bridge(acpi_handle handle, acpi_walk_callback user_function)
+{
+	int seg, bus;
+	unsigned long tmp;
+	acpi_status status;
+	acpi_handle dummy_handle;
+	struct pci_bus *pci_bus;
+	struct p2p_bridge_context context;
+
+	/* If the bridge doesn't have _STA, we assume it is always there */
+	status = acpi_get_handle(handle, "_STA", &dummy_handle);
+	if (ACPI_SUCCESS(status)) {
+		status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
+		if (ACPI_FAILURE(status)) {
+			info("%s: _STA evaluation failure\n", __func__);
+			return 0;
+		}
+		if ((tmp & ACPI_STA_FUNCTIONING) == 0)
+			/* don't register this object */
+			return 0;
+	}
+
+	status = acpi_evaluate_integer(handle, "_SEG", NULL, &tmp);
+	seg = ACPI_SUCCESS(status) ? tmp : 0;
+
+	status = acpi_evaluate_integer(handle, "_BBN", NULL, &tmp);
+	bus = ACPI_SUCCESS(status) ? tmp : 0;
+
+	pci_bus = pci_find_bus(seg, bus);
+	if (!pci_bus)
+		return 0;
+
+	dbg("root bridge walk, pci_bus = %x\n", pci_bus->number);
+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
+				     user_function, pci_bus, NULL);
+	if (ACPI_FAILURE(status))
+		return status;
+
+	context.pci_bus = pci_bus;
+	context.user_function = user_function;
+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
+				     walk_p2p_bridge, &context, NULL);
+	if (ACPI_FAILURE(status))
+		err("%s: walk_p2p_bridge failure - %d\n", __func__, status);
+
+	return status;
+}
+
+/*
+ * acpi_pci_slot_add
+ * @handle: points to an acpi_pci_root
+ */
+static int
+acpi_pci_slot_add(acpi_handle handle)
+{
+	acpi_status status;
+
+	status = walk_root_bridge(handle, register_slot);
+	if (ACPI_FAILURE(status))
+		err("%s: register_slot failure - %d\n", __func__, status);
+
+	return status;
+}
+
+/*
+ * acpi_pci_slot_remove
+ * @handle: points to an acpi_pci_root
+ */
+static void
+acpi_pci_slot_remove(acpi_handle handle)
+{
+	acpi_status status;
+
+	status = walk_root_bridge(handle, unregister_slot);
+	if (ACPI_FAILURE(status))
+		err("%s: unregister_slot failure - %d\n", __func__, status);
+}
+
+#ifdef CONFIG_DMI
+static int do_sta_before_sun(const struct dmi_system_id *d)
+{
+	info("%s detected: will evaluate _STA before calling _SUN\n", d->ident);
+	check_sta_before_sun = 1;
+	return 0;
+}
+
+static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = {
+	/*
+	 * Fujitsu Primequest machines will return 1023 to indicate an
+	 * error if the _SUN method is evaluated on SxFy objects that
+	 * are not present (as indicated by _STA), so for those machines,
+	 * we want to check _STA before evaluating _SUN.
+	 */
+	{
+	 .callback = do_sta_before_sun,
+	 .ident = "Fujitsu Limited Primequest",
+	 .matches = {
+		DMI_MATCH(DMI_BIOS_VENDOR, "Fujitsu"),
+		DMI_MATCH(DMI_BIOS_VERSION, "2.35"),
+		},
+	},
+	{}
+};
+#endif /* CONFIG_DMI */
+
+static int __init
+acpi_pci_slot_init(void)
+{
+	dmi_check_system(acpi_pci_slot_dmi_table);
+	acpi_pci_register_driver(&acpi_pci_slot_driver);
+	return 0;
+}
+
+static void __exit
+acpi_pci_slot_exit(void)
+{
+	acpi_pci_unregister_driver(&acpi_pci_slot_driver);
+}
+
+module_init(acpi_pci_slot_init);
+module_exit(acpi_pci_slot_exit);
diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 21bbb5e..fc12096 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -567,6 +567,11 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
 		return -EINVAL;
 	}
 
+	/*
+	 * No problems if we call this interface from both ACPI_PCI_SLOT
+	 * driver and call it here again. If we've already created the
+	 * pci_slot, the interface will simply bump the refcount.
+	 */
 	pci_slot = pci_create_slot(bus, slot_nr, slot->name);
 	if (IS_ERR(pci_slot))
 		return PTR_ERR(pci_slot);
@@ -580,6 +585,17 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
 	slot->pci_slot = pci_slot;
 	pci_slot->hotplug = slot;
 
+	/*
+	 * Allow pcihp drivers to override the ACPI_PCI_SLOT name.
+	 */
+	if (strcmp(kobject_name(&pci_slot->kobj), slot->name)) {
+		result = kobject_rename(&pci_slot->kobj, slot->name);
+		if (result) {
+			pci_destroy_slot(pci_slot);
+			return result;
+		}
+	}
+
 	list_add(&slot->slot_list, &pci_hotplug_slot_list);
 
 	result = fs_add_slot(pci_slot);
@@ -613,8 +629,12 @@ int pci_hp_deregister(struct hotplug_slot *hotplug)
 
 	slot = hotplug->pci_slot;
 	fs_remove_slot(slot);
-	pci_destroy_slot(slot);
 	dbg("Removed slot %s from the list\n", hotplug->name);
+
+	hotplug_release(slot);
+	slot->release = NULL;
+	pci_destroy_slot(slot);
+
 	return 0;
 }
 
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index e6d3d4d..7561d17 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -249,7 +249,7 @@ static int init_slots(struct controller *ctrl)
 		if (retval == -EBUSY)
 			goto error_info;
 		if (retval) {
-			err ("pci_hp_register failed with error %d\n", retval);
+			err("pci_hp_register failed with error %d\n", retval);
 			goto error_info;
 		}
 		/* create additional sysfs entries */
diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index 8908834..5fd6887 100644
--- a/drivers/pci/hotplug/sgi_hotplug.c
+++ b/drivers/pci/hotplug/sgi_hotplug.c
@@ -668,7 +668,7 @@ static int sn_hotplug_slot_register(struct pci_bus *pci_bus)
 
 register_err:
 	dev_dbg(&pci_bus->self->dev, "bus failed to register with err = %d\n",
-	        rc);
+		rc);
 
 alloc_err:
 	if (rc == -ENOMEM)
-- 
1.5.3.1.g1e61

Re: [PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects

[Matthew Wilcox]

On Tue, Mar 18, 2008 at 03:05:39PM -0600, Alex Chiang wrote:
> Also, v10 should fix the "pci_slot module changes the sysfs name"
> issue that Kenji-san was seeing.

I thought we agreed that the current names are wrong, and we shouldn't
consider this 'different name' an issue.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects

[Alex Chiang]

Hi Willy,

* Matthew Wilcox <matthew@wil.cx>:
> On Tue, Mar 18, 2008 at 03:05:39PM -0600, Alex Chiang wrote:
> > Also, v10 should fix the "pci_slot module changes the sysfs
> > name" issue that Kenji-san was seeing.
> 
> I thought we agreed that the current names are wrong, and we
> shouldn't consider this 'different name' an issue.

I did see some discussion around this, but wasn't actually sure
what we decided on. For v10, the little adjustment I made keeps
the existing userspace names for pciehp, but doesn't affect
shpchp.

[The example that Kenji-san sent out earlier showed that his
pciehp slot names changed depending on the order of module
loading between pciehp and pci_slot; his shpchp slot names did
not change.]

Injecting my thoughts into the exchange between Kenji-san and
Kristen earlier on this subject...

> > * Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> * Kristen Carlson Accardi <kristen.c.accardi@intel.com>:
* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> > >     Though I don't have any specific idea about this,
> > >     folliwings might be candidates.                                                           
> > >                                                                              
> > >     - Override slot names with hotplug driver's slot names                   
> > I think this should be done - if the pci slot driver detects
> > that a hotplug driver is controlling a slot, it should allow
> > that driver to set the name of the slot.                                                     
> 
> Yes, it is what I thought.  But maybe we should try unifying
> slots names first, I think.

This is the tweak I added in v10.

> > >     - Unify slot names among all hotplug drivers                             
> > I'm not sure if we can do this, since slot name might depend
> > on what spec people are implementing.                                                 
> 
> I'm not sure too. But I think we can unify the slot names using
> the combination of chassis number and slot number among the
> controllers which is based on PCI Hot-Plug spec.

I could take this approach instead, but it will definitely change
what userspace sees. I'd like to get an ACK from Kristen before
starting down this path.

Actually, my real preference would be to save it for a future
patch set, while I work on getting this one into the tree. ;)

> > >     - Stop automatic loading of ACPI PCI slot driver                         
> > I think we should definitely implement this one ^^^. 

I don't think that should be happening anyway. On my Fedora Core
8 userspace, pci_slot doesn't get loaded until I modprobe it.

Maybe Kenji-san is building it into his kernel?

/ac

Re: [PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects

[Kenji Kaneshige]

Hi Alex-san,

> I don't think that should be happening anyway. On my Fedora Core
> 8 userspace, pci_slot doesn't get loaded until I modprobe it.
> 
> Maybe Kenji-san is building it into his kernel?

I also noticed that pci_slot doesn't get loaded automatically on
Fedra Core 8 yesterday. When I reported before, I was using other
distribution (Red Hat one). I don't know why.

Thanks,
Kenji Kaneshige


Alex Chiang wrote:
> Hi Willy,
> 
> * Matthew Wilcox <matthew@wil.cx>:
>> On Tue, Mar 18, 2008 at 03:05:39PM -0600, Alex Chiang wrote:
>>> Also, v10 should fix the "pci_slot module changes the sysfs
>>> name" issue that Kenji-san was seeing.
>> I thought we agreed that the current names are wrong, and we
>> shouldn't consider this 'different name' an issue.
> 
> I did see some discussion around this, but wasn't actually sure
> what we decided on. For v10, the little adjustment I made keeps
> the existing userspace names for pciehp, but doesn't affect
> shpchp.
> 
> [The example that Kenji-san sent out earlier showed that his
> pciehp slot names changed depending on the order of module
> loading between pciehp and pci_slot; his shpchp slot names did
> not change.]
> 
> Injecting my thoughts into the exchange between Kenji-san and
> Kristen earlier on this subject...
> 
>>> * Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
>> * Kristen Carlson Accardi <kristen.c.accardi@intel.com>:
> * Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
>>>>     Though I don't have any specific idea about this,
>>>>     folliwings might be candidates.                                                           
>>>>                                                                              
>>>>     - Override slot names with hotplug driver's slot names                   
>>> I think this should be done - if the pci slot driver detects
>>> that a hotplug driver is controlling a slot, it should allow
>>> that driver to set the name of the slot.                                                     
>> Yes, it is what I thought.  But maybe we should try unifying
>> slots names first, I think.
> 
> This is the tweak I added in v10.
> 
>>>>     - Unify slot names among all hotplug drivers                             
>>> I'm not sure if we can do this, since slot name might depend
>>> on what spec people are implementing.                                                 
>> I'm not sure too. But I think we can unify the slot names using
>> the combination of chassis number and slot number among the
>> controllers which is based on PCI Hot-Plug spec.
> 
> I could take this approach instead, but it will definitely change
> what userspace sees. I'd like to get an ACK from Kristen before
> starting down this path.
> 
> Actually, my real preference would be to save it for a future
> patch set, while I work on getting this one into the tree. ;)
> 
>>>>     - Stop automatic loading of ACPI PCI slot driver                         
>>> I think we should definitely implement this one ^^^. 
> 
> I don't think that should be happening anyway. On my Fedora Core
> 8 userspace, pci_slot doesn't get loaded until I modprobe it.
> 
> Maybe Kenji-san is building it into his kernel?
> 
> /ac
> 
> 
> 

Re: [PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects

[Kenji Kaneshige]

Matthew Wilcox wrote:
> On Tue, Mar 18, 2008 at 03:05:39PM -0600, Alex Chiang wrote:
>> Also, v10 should fix the "pci_slot module changes the sysfs name"
>> issue that Kenji-san was seeing.
> 
> I thought we agreed that the current names are wrong, and we shouldn't
> consider this 'different name' an issue.
> 

I also think that the current names are wrong. But even if we fix the
slot name of shpchp/pciehp, it doesn't ensure that slot names are same
among pci_slot.ko and the other hotplug dirvers.

BTW, I'm planning to suggest the patch for shpchp/pciehp to use slot
number as a slot name (as they used to do), and add a module option
to use the combination of bus number and slot number as a slot name
as a workaround for problematic platforms.

Thanks,
Kenji Kaneshige

Re: [PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects

[Kenji Kaneshige]

Hi Alex-san, Greg-san,

I've reviewed and tested the latest version of Alex's "introduce
pci_slot" series with DMI table entry for Fujitsu PRIMEQUEST added
on our PRIMEQUEST server that has both SHPC and PCI-Ex based hotplug
slots. And I confirmed invalid slot ('1023') problem was solved
and slot names were overridden with hotplug drivers as we want.
But on the other hand, I found some bugs and some codes need to
be changed. So I would like Alex-san to fix those. I made patches
for each of those. Could you consider adding or merging my patches
to your patch series? My patches are as follows. Please see the
header of each patch for details. Those are on top of your three
patches.

- [PATCH  1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!)
- [PATCH  2/16] ACPI pci_slot: Fix dmi table for Fujitsu PRIMEQUEST (Not for mainline!)
- [PATCH  3/16][BUG] ACPI pci_slot: Fix _STA evaluation (Not for mainline!)
- [PATCH  4/16][BUG] PCI slot: Add missing semaphore for slot release (Not for mainline!)
- [PATCH  5/16] PCI slot: Use list_head for pci slot list (Not for mainline!)
- [PATCH  6/16][BUG] ACPI pci_slot: Fix slot removal path (Not for mainline!)
- [PATCH  7/16][BUG] PCI slot: Remove compiler warnings (Not for mainline!)
- [PATCH  8/16][BUG] PCI slot: Fix invalid memory access (Not for mainline!)
- [PATCH  9/16] PCI slot: Remove unused slot member from pci_dev (Not for mainline!)
- [PATCH 10/16] PCI slot: Replace dbg with pr_debug (Not for mainline!)
- [PATCH 11/16] PCI slot: Remove useless release handler (Not for mainline!)
- [PATCH 12/16] PCI slot: Use .default_attrs for address file (Not for mainline!)
- [PATCH 13/16] PCI slot: Fix return value of pci_create_slot() (Not for mainline!)
- [PATCH 14/16] PCI slot: Change return value of pci_destroy_slot() (Not for mainline!)
- [PATCH 15/16] PCI slot: Trivial cleanups for slot.c (Not for mainline!)
- [PATCH 16/16][BUG] PCI hotplug core: add missing lock for hotplug slot list (Not for mainline!)

BTW, I've posted other patches to fix the slot name of shpchp/pciehp
to pcihpd-discuss mailing-list. Please see it also.

Thanks,
Kenji Kaneshige




Alex Chiang wrote:
> Hi Greg, Kenji-san,
> 
> This is v10 of my "introduce pci_slot" series, to be considered
> for inclusion in linux-next to get more test exposure and shake
> out the bugs.
> 
> * Greg KH <gregkh@suse.de>:
>> Can you work with Kenji to get his approval on this before I
>> add this to the tree?  It sounds like there are still some
>> outstanding issues getting this to work properly on his
>> hardware.  It needs to work without relying on a module not
>> being loaded, as the distros are never going to go for that
>> kind of solution.
> 
> New in v10 is the addition of a DMI list to do the right thing on
> Fujitsu hardware, and does not rely on "don't load the module" to
> get the correct behavior.
> 
> Also, v10 should fix the "pci_slot module changes the sysfs name"
> issue that Kenji-san was seeing.
> 
> * Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
>> Alex-san, could you send me the updated patches? I'll Ack the
>> patch after adding Fujitsu firmware versions to the DMI list
>> and testing it.
> 
> Thanks, I'd like to know if the naming issue is fixed for you as
> well.
> 
> cheers,
> 
> /ac
> 
> 
> 

[PATCH 1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!)

[Kenji Kaneshige]

Export kobject_rename() to fix the following link error. This happens
when pci_hotplug_core driver is compiled as a kernel module.

ERROR: "kobject_rename" [drivers/pci/hotplug/pci_hotplug.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 lib/kobject.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6.25-rc6/lib/kobject.c
===================================================================
--- linux-2.6.25-rc6.orig/lib/kobject.c
+++ linux-2.6.25-rc6/lib/kobject.c
@@ -456,6 +456,7 @@ out:
 
 	return error;
 }
+EXPORT_SYMBOL_GPL(kobject_rename);
 
 /**
  * kobject_move - move object to another parent

Re: [PATCH 1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!)

[Alex Chiang]

Hi Greg,

This patch looks like a candidate for the current 2.6.25 cycle.
Any chance of getting it in there?

If not, maybe you can push it into linux-next without waiting for
the rest of my pci_slot changes, since this is a fix for the
kobject core, and not necessarily what I'm working on.

Thanks.

/ac

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Export kobject_rename() to fix the following link error. This happens
> when pci_hotplug_core driver is compiled as a kernel module.
> 
> ERROR: "kobject_rename" [drivers/pci/hotplug/pci_hotplug.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2
> make: *** Waiting for unfinished jobs....
> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

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

> 
> ---
>  lib/kobject.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux-2.6.25-rc6/lib/kobject.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/lib/kobject.c
> +++ linux-2.6.25-rc6/lib/kobject.c
> @@ -456,6 +456,7 @@ out:
>  
>  	return error;
>  }
> +EXPORT_SYMBOL_GPL(kobject_rename);
>  
>  /**
>   * kobject_move - move object to another parent
> 
> 

Re: [PATCH 1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!)

[Greg KH]

On Fri, Mar 21, 2008 at 09:56:05AM -0600, Alex Chiang wrote:
> Hi Greg,
> 
> This patch looks like a candidate for the current 2.6.25 cycle.
> Any chance of getting it in there?

Is this a problem today?  I don't see it being one, so when you need
this function exported, we can do it then.  Otherwise people will just
start sending me patches unexporting it as they don't see any in-kernel
users of it :)

So just keep it as part of your patchset, it's easier for everyone that
way.

And it will remind me to verify that you really should be calling
kobject_rename and not just getting the name correctly the first time
around...{hint}

thanks,

greg k-h

Re: [PATCH 1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!)

[Alex Chiang]

* Greg KH <gregkh@suse.de>:
> On Fri, Mar 21, 2008 at 09:56:05AM -0600, Alex Chiang wrote:
> > Hi Greg,
> > 
> > This patch looks like a candidate for the current 2.6.25 cycle.
> > Any chance of getting it in there?
> 
> Is this a problem today?  I don't see it being one, so when you need
> this function exported, we can do it then.  Otherwise people will just
> start sending me patches unexporting it as they don't see any in-kernel
> users of it :)
> 
> So just keep it as part of your patchset, it's easier for everyone that
> way.

Ok, sounds fair to me.

> And it will remind me to verify that you really should be calling
> kobject_rename and not just getting the name correctly the first time
> around...{hint}

I'm pretty sure we really do need to call it, but I appreciate
your future review. ;)

/ac

[PATCH 2/16] ACPI pci_slot: Fix dmi table for Fujitsu PRIMEQUEST (Not for mainline!)

[Kenji Kaneshige]

Fix DMI table entry for Fujitsu PRIMEQUEST.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/acpi/pci_slot.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6.25-rc6/drivers/acpi/pci_slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/acpi/pci_slot.c
+++ linux-2.6.25-rc6/drivers/acpi/pci_slot.c
@@ -332,10 +332,10 @@ static struct dmi_system_id acpi_pci_slo
 	 */
 	{
 	 .callback = do_sta_before_sun,
-	 .ident = "Fujitsu Limited Primequest",
+	 .ident = "Fujitsu PRIMEQUEST",
 	 .matches = {
-		DMI_MATCH(DMI_BIOS_VENDOR, "Fujitsu"),
-		DMI_MATCH(DMI_BIOS_VERSION, "2.35"),
+		DMI_MATCH(DMI_BIOS_VENDOR, "FUJITSU LIMITED"),
+		DMI_MATCH(DMI_BIOS_VERSION, "PRIMEQUEST"),
 		},
 	},
 	{}

Re: [PATCH 2/16] ACPI pci_slot: Fix dmi table for Fujitsu PRIMEQUEST (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Fix DMI table entry for Fujitsu PRIMEQUEST.
> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

Merged, thanks.

/ac

> 
> ---
>  drivers/acpi/pci_slot.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/acpi/pci_slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/acpi/pci_slot.c
> +++ linux-2.6.25-rc6/drivers/acpi/pci_slot.c
> @@ -332,10 +332,10 @@ static struct dmi_system_id acpi_pci_slo
>  	 */
>  	{
>  	 .callback = do_sta_before_sun,
> -	 .ident = "Fujitsu Limited Primequest",
> +	 .ident = "Fujitsu PRIMEQUEST",
>  	 .matches = {
> -		DMI_MATCH(DMI_BIOS_VENDOR, "Fujitsu"),
> -		DMI_MATCH(DMI_BIOS_VERSION, "2.35"),
> +		DMI_MATCH(DMI_BIOS_VENDOR, "FUJITSU LIMITED"),
> +		DMI_MATCH(DMI_BIOS_VERSION, "PRIMEQUEST"),
>  		},
>  	},
>  	{}
> 
> 

[PATCH 3/16][BUG] ACPI pci_slot: Fix _STA evaluation (Not for mainline!)

[Kenji Kaneshige]

Fix the problem that pci slots that doesn't have _STA are not
detected. If the device doesn't have _STA, we must assume it is always
there.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
---
 drivers/acpi/pci_slot.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: linux-2.6.25-rc6/drivers/acpi/pci_slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/acpi/pci_slot.c
+++ linux-2.6.25-rc6/drivers/acpi/pci_slot.c
@@ -76,9 +76,8 @@ check_slot(acpi_handle handle, int *devi
 
 	if (check_sta_before_sun) {
 		/* If SxFy doesn't have _STA, we just assume it's there */
-		acpi_evaluate_integer(handle, "_STA", NULL, &sta);
-
-		if (!(sta & ACPI_STA_DEVICE_PRESENT)) {
+		status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
+		if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_PRESENT)) {
 			retval = -1;
 			goto out;
 		}

Re: [PATCH 3/16][BUG] ACPI pci_slot: Fix _STA evaluation (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Fix the problem that pci slots that doesn't have _STA are not
> detected. If the device doesn't have _STA, we must assume it is always
> there.

Merged, thanks.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> ---
>  drivers/acpi/pci_slot.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/acpi/pci_slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/acpi/pci_slot.c
> +++ linux-2.6.25-rc6/drivers/acpi/pci_slot.c
> @@ -76,9 +76,8 @@ check_slot(acpi_handle handle, int *devi
>  
>  	if (check_sta_before_sun) {
>  		/* If SxFy doesn't have _STA, we just assume it's there */
> -		acpi_evaluate_integer(handle, "_STA", NULL, &sta);
> -
> -		if (!(sta & ACPI_STA_DEVICE_PRESENT)) {
> +		status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
> +		if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_PRESENT)) {
>  			retval = -1;
>  			goto out;
>  		}
> 
> 

[PATCH 4/16][BUG] PCI slot: Add missing semaphore for slot release (Not for mainline!)

[Kenji Kaneshige]

Add missing pci_bus sem into pci_destroy_slot(). There is possible
race condition because current code doesn't have any serialization
in the destroy path.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/slot.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -196,7 +196,9 @@ EXPORT_SYMBOL_GPL(pci_create_slot);
 
 int pci_destroy_slot(struct pci_slot *slot)
 {
+	down_write(&pci_bus_sem);
 	kobject_put(&slot->kobj);
+	up_write(&pci_bus_sem);
 
 	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
 		slot->kobj.kref.refcount, slot->bus->number, slot->number);

Re: [PATCH 4/16][BUG] PCI slot: Add missing semaphore for slot release (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Add missing pci_bus sem into pci_destroy_slot(). There is possible
> race condition because current code doesn't have any serialization
> in the destroy path.

Merged, thanks.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/slot.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -196,7 +196,9 @@ EXPORT_SYMBOL_GPL(pci_create_slot);
>  
>  int pci_destroy_slot(struct pci_slot *slot)
>  {
> +	down_write(&pci_bus_sem);
>  	kobject_put(&slot->kobj);
> +	up_write(&pci_bus_sem);
>  
>  	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
>  		slot->kobj.kref.refcount, slot->bus->number, slot->number);
> 
> 

[PATCH 5/16] PCI slot: Use list_head for pci slot list (Not for mainline!)

[Kenji Kaneshige]

To make code simple, we should use list_head for physical slot list on
the bus.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/acpi/pci_slot.c |    9 ++++++---
 drivers/pci/probe.c     |    1 +
 drivers/pci/slot.c      |   17 ++++++-----------
 include/linux/pci.h     |    4 ++--
 4 files changed, 15 insertions(+), 16 deletions(-)

Index: linux-2.6.25-rc6/drivers/pci/probe.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/probe.c
+++ linux-2.6.25-rc6/drivers/pci/probe.c
@@ -384,6 +384,7 @@ static struct pci_bus * pci_alloc_bus(vo
 		INIT_LIST_HEAD(&b->node);
 		INIT_LIST_HEAD(&b->children);
 		INIT_LIST_HEAD(&b->devices);
+		INIT_LIST_HEAD(&b->slots);
 	}
 	return b;
 }
Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -69,18 +69,12 @@ static int create_sysfs_files(struct pci
 
 static void pci_slot_release(struct kobject *kobj)
 {
-	struct pci_slot **pprev;
 	struct pci_slot *slot = to_pci_slot(kobj);
 
 	dbg("%s: releasing pci_slot on %x:%d\n", __func__,
 		slot->bus->number, slot->number);
 
-	for (pprev = &slot->bus->slot; *pprev; pprev = &(*pprev)->next) {
-		if (*pprev == slot) {
-			*pprev = slot->next;
-			break;
-		}
-	}
+	list_del(&slot->list);
 
 	if (slot->release)
 		slot->release(slot);
@@ -107,11 +101,12 @@ int pci_slot_add_hotplug(struct pci_bus 
 	/* This slot should have already been created, so look for it. If
 	 * we can't find it, return -EEXIST.
 	 */
-	for (slot = parent->slot; slot; slot = slot->next)
+	list_for_each_entry(slot, &parent->slots, list) {
 		if (slot->number == slot_nr) {
 			found = 1;
 			break;
 		}
+	}
 
 	if (!found) {
 		dbg("%s: slot not found\n", __func__);
@@ -143,7 +138,7 @@ struct pci_slot *pci_create_slot(struct 
 	down_write(&pci_bus_sem);
 
 	/* If we've already created this slot, bump refcount and return. */
-	for (slot = parent->slot; slot; slot = slot->next) {
+	list_for_each_entry(slot, &parent->slots, list) {
 		if (slot->number == slot_nr) {
 			kobject_get(&slot->kobj);
 			dbg("%s: bumped refcount to %d on %x:%d\n",
@@ -175,8 +170,8 @@ struct pci_slot *pci_create_slot(struct 
 	if (err)
 		goto unregister;
 
-	slot->next = parent->slot;
-	parent->slot = slot;
+	INIT_LIST_HEAD(&slot->list);
+	list_add(&slot->list, &parent->slots);
 
 	dbg("%s: created pci_slot on %x:%d\n",
 		__func__, parent->number, slot_nr);
Index: linux-2.6.25-rc6/include/linux/pci.h
===================================================================
--- linux-2.6.25-rc6.orig/include/linux/pci.h
+++ linux-2.6.25-rc6/include/linux/pci.h
@@ -131,7 +131,7 @@ struct pci_cap_saved_state {
 /* pci_slot represents a physical slot */
 struct pci_slot {
 	struct pci_bus *bus;		/* The bus this slot is on */
-	struct pci_slot *next;		/* Next slot on this bus */
+	struct list_head list;		/* node in list of slots on this bus */
 	struct hotplug_slot *hotplug;	/* Hotplug info (migrate over time) */
 	unsigned char number;		/* PCI_SLOT(pci_dev->devfn) */
 	struct kobject kobj;
@@ -269,7 +269,7 @@ struct pci_bus {
 	struct list_head children;	/* list of child buses */
 	struct list_head devices;	/* list of devices on this bus */
 	struct pci_dev	*self;		/* bridge device as seen by parent */
-	struct pci_slot	*slot;		/* First physical slot on this bus */
+	struct list_head slots;		/* list of slots on this bus */
 	struct resource	*resource[PCI_BUS_NUM_RESOURCES];
 					/* address space routed to this bus */
 
Index: linux-2.6.25-rc6/drivers/acpi/pci_slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/acpi/pci_slot.c
+++ linux-2.6.25-rc6/drivers/acpi/pci_slot.c
@@ -117,15 +117,18 @@ unregister_slot(acpi_handle handle, u32 
 {
 	int device;
 	unsigned long sun;
-	struct pci_slot *slot;
+	struct pci_slot *slot, *tmp;
 	struct pci_bus *pci_bus = context;
 
 	if (check_slot(handle, &device, &sun))
 		return AE_OK;
 
-	for (slot = pci_bus->slot; slot; slot = slot->next) {
-		if (slot->number == device)
+	/* FIXME - Need pci_bus_sem to be held */
+	list_for_each_entry_safe(slot, tmp, &pci_bus->slots, list) {
+		if (slot->number == device) {
 			pci_destroy_slot(slot);
+			break;
+		}
 	}
 
 	return AE_OK;

Re: [PATCH 5/16] PCI slot: Use list_head for pci slot list (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> To make code simple, we should use list_head for physical slot list on
> the bus.

Merged, thanks.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/acpi/pci_slot.c |    9 ++++++---
>  drivers/pci/probe.c     |    1 +
>  drivers/pci/slot.c      |   17 ++++++-----------
>  include/linux/pci.h     |    4 ++--
>  4 files changed, 15 insertions(+), 16 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/probe.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/probe.c
> +++ linux-2.6.25-rc6/drivers/pci/probe.c
> @@ -384,6 +384,7 @@ static struct pci_bus * pci_alloc_bus(vo
>  		INIT_LIST_HEAD(&b->node);
>  		INIT_LIST_HEAD(&b->children);
>  		INIT_LIST_HEAD(&b->devices);
> +		INIT_LIST_HEAD(&b->slots);
>  	}
>  	return b;
>  }
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -69,18 +69,12 @@ static int create_sysfs_files(struct pci
>  
>  static void pci_slot_release(struct kobject *kobj)
>  {
> -	struct pci_slot **pprev;
>  	struct pci_slot *slot = to_pci_slot(kobj);
>  
>  	dbg("%s: releasing pci_slot on %x:%d\n", __func__,
>  		slot->bus->number, slot->number);
>  
> -	for (pprev = &slot->bus->slot; *pprev; pprev = &(*pprev)->next) {
> -		if (*pprev == slot) {
> -			*pprev = slot->next;
> -			break;
> -		}
> -	}
> +	list_del(&slot->list);
>  
>  	if (slot->release)
>  		slot->release(slot);
> @@ -107,11 +101,12 @@ int pci_slot_add_hotplug(struct pci_bus 
>  	/* This slot should have already been created, so look for it. If
>  	 * we can't find it, return -EEXIST.
>  	 */
> -	for (slot = parent->slot; slot; slot = slot->next)
> +	list_for_each_entry(slot, &parent->slots, list) {
>  		if (slot->number == slot_nr) {
>  			found = 1;
>  			break;
>  		}
> +	}
>  
>  	if (!found) {
>  		dbg("%s: slot not found\n", __func__);
> @@ -143,7 +138,7 @@ struct pci_slot *pci_create_slot(struct 
>  	down_write(&pci_bus_sem);
>  
>  	/* If we've already created this slot, bump refcount and return. */
> -	for (slot = parent->slot; slot; slot = slot->next) {
> +	list_for_each_entry(slot, &parent->slots, list) {
>  		if (slot->number == slot_nr) {
>  			kobject_get(&slot->kobj);
>  			dbg("%s: bumped refcount to %d on %x:%d\n",
> @@ -175,8 +170,8 @@ struct pci_slot *pci_create_slot(struct 
>  	if (err)
>  		goto unregister;
>  
> -	slot->next = parent->slot;
> -	parent->slot = slot;
> +	INIT_LIST_HEAD(&slot->list);
> +	list_add(&slot->list, &parent->slots);
>  
>  	dbg("%s: created pci_slot on %x:%d\n",
>  		__func__, parent->number, slot_nr);
> Index: linux-2.6.25-rc6/include/linux/pci.h
> ===================================================================
> --- linux-2.6.25-rc6.orig/include/linux/pci.h
> +++ linux-2.6.25-rc6/include/linux/pci.h
> @@ -131,7 +131,7 @@ struct pci_cap_saved_state {
>  /* pci_slot represents a physical slot */
>  struct pci_slot {
>  	struct pci_bus *bus;		/* The bus this slot is on */
> -	struct pci_slot *next;		/* Next slot on this bus */
> +	struct list_head list;		/* node in list of slots on this bus */
>  	struct hotplug_slot *hotplug;	/* Hotplug info (migrate over time) */
>  	unsigned char number;		/* PCI_SLOT(pci_dev->devfn) */
>  	struct kobject kobj;
> @@ -269,7 +269,7 @@ struct pci_bus {
>  	struct list_head children;	/* list of child buses */
>  	struct list_head devices;	/* list of devices on this bus */
>  	struct pci_dev	*self;		/* bridge device as seen by parent */
> -	struct pci_slot	*slot;		/* First physical slot on this bus */
> +	struct list_head slots;		/* list of slots on this bus */
>  	struct resource	*resource[PCI_BUS_NUM_RESOURCES];
>  					/* address space routed to this bus */
>  
> Index: linux-2.6.25-rc6/drivers/acpi/pci_slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/acpi/pci_slot.c
> +++ linux-2.6.25-rc6/drivers/acpi/pci_slot.c
> @@ -117,15 +117,18 @@ unregister_slot(acpi_handle handle, u32 
>  {
>  	int device;
>  	unsigned long sun;
> -	struct pci_slot *slot;
> +	struct pci_slot *slot, *tmp;
>  	struct pci_bus *pci_bus = context;
>  
>  	if (check_slot(handle, &device, &sun))
>  		return AE_OK;
>  
> -	for (slot = pci_bus->slot; slot; slot = slot->next) {
> -		if (slot->number == device)
> +	/* FIXME - Need pci_bus_sem to be held */
> +	list_for_each_entry_safe(slot, tmp, &pci_bus->slots, list) {
> +		if (slot->number == device) {
>  			pci_destroy_slot(slot);
> +			break;
> +		}
>  	}
>  
>  	return AE_OK;
> 
> 

[PATCH 6/16][BUG] ACPI pci_slot: Fix slot removal path (Not for mainline!)

[Kenji Kaneshige]

Current ACPI pci slot driver scans ACPI namespace and slot list on
pci_bus structure to find the pci_slot pointer. It is definitely
redundant. In addition, it scans slot list on pci_bus without holding
pci_bus_sem. This patch fixes those problems.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/acpi/pci_slot.c |  104 +++++++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 49 deletions(-)

Index: linux-2.6.25-rc6/drivers/acpi/pci_slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/acpi/pci_slot.c
+++ linux-2.6.25-rc6/drivers/acpi/pci_slot.c
@@ -55,9 +55,17 @@ ACPI_MODULE_NAME("pci_slot");
 				MY_NAME , ## arg);		\
 	} while (0)
 
+struct acpi_pci_slot {
+	acpi_handle root_handle;	/* handle of the root bridge */
+	struct pci_slot *pci_slot;	/* corresponding pci_slot */
+	struct list_head list;		/* node in the list of slots */
+};
+
 static int acpi_pci_slot_add(acpi_handle handle);
 static void acpi_pci_slot_remove(acpi_handle handle);
 
+static LIST_HEAD(slot_list);
+static DEFINE_MUTEX(slot_list_lock);
 static struct acpi_pci_driver acpi_pci_slot_driver = {
 	.add = acpi_pci_slot_add,
 	.remove = acpi_pci_slot_remove,
@@ -105,34 +113,11 @@ out:
 	return retval;
 }
 
-/*
- * unregister_slot
- *
- * Called once for each SxFy object in the namespace. Each call to
- * pci_destroy_slot decrements the refcount on the pci_slot, and
- * eventually calls kobject_unregister at the appropriate time.
- */
-static acpi_status
-unregister_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-	int device;
-	unsigned long sun;
-	struct pci_slot *slot, *tmp;
-	struct pci_bus *pci_bus = context;
-
-	if (check_slot(handle, &device, &sun))
-		return AE_OK;
-
-	/* FIXME - Need pci_bus_sem to be held */
-	list_for_each_entry_safe(slot, tmp, &pci_bus->slots, list) {
-		if (slot->number == device) {
-			pci_destroy_slot(slot);
-			break;
-		}
-	}
-
-	return AE_OK;
-}
+struct callback_args {
+	acpi_walk_callback	user_function;	/* only for walk_p2p_bridge */
+	struct pci_bus		*pci_bus;
+	acpi_handle		root_handle;
+};
 
 /*
  * register_slot
@@ -150,17 +135,33 @@ register_slot(acpi_handle handle, u32 lv
 	int device;
 	unsigned long sun;
 	char name[KOBJ_NAME_LEN];
-
+	struct acpi_pci_slot *slot;
 	struct pci_slot *pci_slot;
-	struct pci_bus *pci_bus = context;
+	struct callback_args *parent_context = context;
+	struct pci_bus *pci_bus = parent_context->pci_bus;
 
 	if (check_slot(handle, &device, &sun))
 		return AE_OK;
 
+	slot = kmalloc(sizeof(*slot), GFP_KERNEL);
+	if (!slot) {
+		err("%s: cannot allocate memory\n", __FUNCTION__);
+		return AE_OK;
+	}
+
 	snprintf(name, sizeof(name), "%u", (u32)sun);
 	pci_slot = pci_create_slot(pci_bus, device, name);
-	if (IS_ERR(pci_slot))
+	if (IS_ERR(pci_slot)) {
 		err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
+		kfree(slot);
+	}
+
+	slot->root_handle = parent_context->root_handle;
+	slot->pci_slot = pci_slot;
+	INIT_LIST_HEAD(&slot->list);
+	mutex_lock(&slot_list_lock);
+	list_add(&slot->list, &slot_list);
+	mutex_unlock(&slot_list_lock);
 
 	dbg("pci_slot: %#lx, pci_bus: %x, device: %d, name: %s\n",
 		(uint64_t)pci_slot, pci_bus->number, device, name);
@@ -168,11 +169,6 @@ register_slot(acpi_handle handle, u32 lv
 	return AE_OK;
 }
 
-struct p2p_bridge_context {
-	acpi_walk_callback	user_function;
-	struct pci_bus		*pci_bus;
-};
-
 /*
  * walk_p2p_bridge - discover and walk p2p bridges
  * @handle: points to an acpi_pci_root
@@ -192,8 +188,8 @@ walk_p2p_bridge(acpi_handle handle, u32 
 
 	struct pci_dev *dev;
 	struct pci_bus *pci_bus;
-	struct p2p_bridge_context child_context;
-	struct p2p_bridge_context *parent_context = context;
+	struct callback_args child_context;
+	struct callback_args *parent_context = context;
 
 	pci_bus = parent_context->pci_bus;
 	user_function = parent_context->user_function;
@@ -213,14 +209,16 @@ walk_p2p_bridge(acpi_handle handle, u32 
 	if (!dev || !dev->subordinate)
 		goto out;
 
+	child_context.pci_bus = dev->subordinate;
+	child_context.user_function = user_function;
+	child_context.root_handle = parent_context->root_handle;
+
 	dbg("p2p bridge walk, pci_bus = %x\n", dev->subordinate->number);
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
-				     user_function, dev->subordinate, NULL);
+				     user_function, &child_context, NULL);
 	if (ACPI_FAILURE(status))
 		goto out;
 
-	child_context.pci_bus = dev->subordinate;
-	child_context.user_function = user_function;
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
 				     walk_p2p_bridge, &child_context, NULL);
 out:
@@ -246,7 +244,7 @@ walk_root_bridge(acpi_handle handle, acp
 	acpi_status status;
 	acpi_handle dummy_handle;
 	struct pci_bus *pci_bus;
-	struct p2p_bridge_context context;
+	struct callback_args context;
 
 	/* If the bridge doesn't have _STA, we assume it is always there */
 	status = acpi_get_handle(handle, "_STA", &dummy_handle);
@@ -271,14 +269,16 @@ walk_root_bridge(acpi_handle handle, acp
 	if (!pci_bus)
 		return 0;
 
+	context.pci_bus = pci_bus;
+	context.user_function = user_function;
+	context.root_handle = handle;
+
 	dbg("root bridge walk, pci_bus = %x\n", pci_bus->number);
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
-				     user_function, pci_bus, NULL);
+				     user_function, &context, NULL);
 	if (ACPI_FAILURE(status))
 		return status;
 
-	context.pci_bus = pci_bus;
-	context.user_function = user_function;
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
 				     walk_p2p_bridge, &context, NULL);
 	if (ACPI_FAILURE(status))
@@ -310,11 +310,17 @@ acpi_pci_slot_add(acpi_handle handle)
 static void
 acpi_pci_slot_remove(acpi_handle handle)
 {
-	acpi_status status;
+	struct acpi_pci_slot *slot, *tmp;
 
-	status = walk_root_bridge(handle, unregister_slot);
-	if (ACPI_FAILURE(status))
-		err("%s: unregister_slot failure - %d\n", __func__, status);
+	mutex_lock(&slot_list_lock);
+	list_for_each_entry_safe(slot, tmp, &slot_list, list) {
+		if (slot->root_handle == handle) {
+			list_del(&slot->list);
+			pci_destroy_slot(slot->pci_slot);
+			kfree(slot);
+		}
+	}
+	mutex_unlock(&slot_list_lock);
 }
 
 #ifdef CONFIG_DMI

Re: [PATCH 6/16][BUG] ACPI pci_slot: Fix slot removal path (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Current ACPI pci slot driver scans ACPI namespace and slot list on
> pci_bus structure to find the pci_slot pointer. It is definitely
> redundant. In addition, it scans slot list on pci_bus without holding
> pci_bus_sem. This patch fixes those problems.

Tested and merged, thanks.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/acpi/pci_slot.c |  104 +++++++++++++++++++++++++-----------------------
>  1 file changed, 55 insertions(+), 49 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/acpi/pci_slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/acpi/pci_slot.c
> +++ linux-2.6.25-rc6/drivers/acpi/pci_slot.c
> @@ -55,9 +55,17 @@ ACPI_MODULE_NAME("pci_slot");
>  				MY_NAME , ## arg);		\
>  	} while (0)
>  
> +struct acpi_pci_slot {
> +	acpi_handle root_handle;	/* handle of the root bridge */
> +	struct pci_slot *pci_slot;	/* corresponding pci_slot */
> +	struct list_head list;		/* node in the list of slots */
> +};
> +
>  static int acpi_pci_slot_add(acpi_handle handle);
>  static void acpi_pci_slot_remove(acpi_handle handle);
>  
> +static LIST_HEAD(slot_list);
> +static DEFINE_MUTEX(slot_list_lock);
>  static struct acpi_pci_driver acpi_pci_slot_driver = {
>  	.add = acpi_pci_slot_add,
>  	.remove = acpi_pci_slot_remove,
> @@ -105,34 +113,11 @@ out:
>  	return retval;
>  }
>  
> -/*
> - * unregister_slot
> - *
> - * Called once for each SxFy object in the namespace. Each call to
> - * pci_destroy_slot decrements the refcount on the pci_slot, and
> - * eventually calls kobject_unregister at the appropriate time.
> - */
> -static acpi_status
> -unregister_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
> -{
> -	int device;
> -	unsigned long sun;
> -	struct pci_slot *slot, *tmp;
> -	struct pci_bus *pci_bus = context;
> -
> -	if (check_slot(handle, &device, &sun))
> -		return AE_OK;
> -
> -	/* FIXME - Need pci_bus_sem to be held */
> -	list_for_each_entry_safe(slot, tmp, &pci_bus->slots, list) {
> -		if (slot->number == device) {
> -			pci_destroy_slot(slot);
> -			break;
> -		}
> -	}
> -
> -	return AE_OK;
> -}
> +struct callback_args {
> +	acpi_walk_callback	user_function;	/* only for walk_p2p_bridge */
> +	struct pci_bus		*pci_bus;
> +	acpi_handle		root_handle;
> +};
>  
>  /*
>   * register_slot
> @@ -150,17 +135,33 @@ register_slot(acpi_handle handle, u32 lv
>  	int device;
>  	unsigned long sun;
>  	char name[KOBJ_NAME_LEN];
> -
> +	struct acpi_pci_slot *slot;
>  	struct pci_slot *pci_slot;
> -	struct pci_bus *pci_bus = context;
> +	struct callback_args *parent_context = context;
> +	struct pci_bus *pci_bus = parent_context->pci_bus;
>  
>  	if (check_slot(handle, &device, &sun))
>  		return AE_OK;
>  
> +	slot = kmalloc(sizeof(*slot), GFP_KERNEL);
> +	if (!slot) {
> +		err("%s: cannot allocate memory\n", __FUNCTION__);
> +		return AE_OK;
> +	}
> +
>  	snprintf(name, sizeof(name), "%u", (u32)sun);
>  	pci_slot = pci_create_slot(pci_bus, device, name);
> -	if (IS_ERR(pci_slot))
> +	if (IS_ERR(pci_slot)) {
>  		err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
> +		kfree(slot);
> +	}
> +
> +	slot->root_handle = parent_context->root_handle;
> +	slot->pci_slot = pci_slot;
> +	INIT_LIST_HEAD(&slot->list);
> +	mutex_lock(&slot_list_lock);
> +	list_add(&slot->list, &slot_list);
> +	mutex_unlock(&slot_list_lock);
>  
>  	dbg("pci_slot: %#lx, pci_bus: %x, device: %d, name: %s\n",
>  		(uint64_t)pci_slot, pci_bus->number, device, name);
> @@ -168,11 +169,6 @@ register_slot(acpi_handle handle, u32 lv
>  	return AE_OK;
>  }
>  
> -struct p2p_bridge_context {
> -	acpi_walk_callback	user_function;
> -	struct pci_bus		*pci_bus;
> -};
> -
>  /*
>   * walk_p2p_bridge - discover and walk p2p bridges
>   * @handle: points to an acpi_pci_root
> @@ -192,8 +188,8 @@ walk_p2p_bridge(acpi_handle handle, u32 
>  
>  	struct pci_dev *dev;
>  	struct pci_bus *pci_bus;
> -	struct p2p_bridge_context child_context;
> -	struct p2p_bridge_context *parent_context = context;
> +	struct callback_args child_context;
> +	struct callback_args *parent_context = context;
>  
>  	pci_bus = parent_context->pci_bus;
>  	user_function = parent_context->user_function;
> @@ -213,14 +209,16 @@ walk_p2p_bridge(acpi_handle handle, u32 
>  	if (!dev || !dev->subordinate)
>  		goto out;
>  
> +	child_context.pci_bus = dev->subordinate;
> +	child_context.user_function = user_function;
> +	child_context.root_handle = parent_context->root_handle;
> +
>  	dbg("p2p bridge walk, pci_bus = %x\n", dev->subordinate->number);
>  	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
> -				     user_function, dev->subordinate, NULL);
> +				     user_function, &child_context, NULL);
>  	if (ACPI_FAILURE(status))
>  		goto out;
>  
> -	child_context.pci_bus = dev->subordinate;
> -	child_context.user_function = user_function;
>  	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
>  				     walk_p2p_bridge, &child_context, NULL);
>  out:
> @@ -246,7 +244,7 @@ walk_root_bridge(acpi_handle handle, acp
>  	acpi_status status;
>  	acpi_handle dummy_handle;
>  	struct pci_bus *pci_bus;
> -	struct p2p_bridge_context context;
> +	struct callback_args context;
>  
>  	/* If the bridge doesn't have _STA, we assume it is always there */
>  	status = acpi_get_handle(handle, "_STA", &dummy_handle);
> @@ -271,14 +269,16 @@ walk_root_bridge(acpi_handle handle, acp
>  	if (!pci_bus)
>  		return 0;
>  
> +	context.pci_bus = pci_bus;
> +	context.user_function = user_function;
> +	context.root_handle = handle;
> +
>  	dbg("root bridge walk, pci_bus = %x\n", pci_bus->number);
>  	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
> -				     user_function, pci_bus, NULL);
> +				     user_function, &context, NULL);
>  	if (ACPI_FAILURE(status))
>  		return status;
>  
> -	context.pci_bus = pci_bus;
> -	context.user_function = user_function;
>  	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
>  				     walk_p2p_bridge, &context, NULL);
>  	if (ACPI_FAILURE(status))
> @@ -310,11 +310,17 @@ acpi_pci_slot_add(acpi_handle handle)
>  static void
>  acpi_pci_slot_remove(acpi_handle handle)
>  {
> -	acpi_status status;
> +	struct acpi_pci_slot *slot, *tmp;
>  
> -	status = walk_root_bridge(handle, unregister_slot);
> -	if (ACPI_FAILURE(status))
> -		err("%s: unregister_slot failure - %d\n", __func__, status);
> +	mutex_lock(&slot_list_lock);
> +	list_for_each_entry_safe(slot, tmp, &slot_list, list) {
> +		if (slot->root_handle == handle) {
> +			list_del(&slot->list);
> +			pci_destroy_slot(slot->pci_slot);
> +			kfree(slot);
> +		}
> +	}
> +	mutex_unlock(&slot_list_lock);
>  }
>  
>  #ifdef CONFIG_DMI
> 
> 

[PATCH 7/16][BUG] PCI slot: Remove compiler warnings (Not for mainline!)

[Kenji Kaneshige]

Remove following compiler warnings.

drivers/pci/slot.c: In function 'pci_create_slot':
drivers/pci/slot.c:146: warning: format '%d' expects type 'int', but argument 4 has type 'atomic_t'
drivers/pci/slot.c: In function 'pci_destroy_slot':
drivers/pci/slot.c:198: warning: format '%d' expects type 'int', but argument 4 has type 'atomic_t'

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/slot.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -142,8 +142,8 @@ struct pci_slot *pci_create_slot(struct 
 		if (slot->number == slot_nr) {
 			kobject_get(&slot->kobj);
 			dbg("%s: bumped refcount to %d on %x:%d\n",
-				__func__, slot->kobj.kref.refcount,
-				parent->number, slot_nr);
+			    __func__, atomic_read(&slot->kobj.kref.refcount),
+			    parent->number, slot_nr);
 			goto out;
 		}
 	}
@@ -196,7 +196,8 @@ int pci_destroy_slot(struct pci_slot *sl
 	up_write(&pci_bus_sem);
 
 	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
-		slot->kobj.kref.refcount, slot->bus->number, slot->number);
+	    atomic_read(&slot->kobj.kref.refcount), slot->bus->number,
+	    slot->number);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pci_destroy_slot);

Re: [PATCH 7/16][BUG] PCI slot: Remove compiler warnings (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Remove following compiler warnings.

Merged, thanks.

/ac

> 
> drivers/pci/slot.c: In function 'pci_create_slot':
> drivers/pci/slot.c:146: warning: format '%d' expects type 'int', but argument 4 has type 'atomic_t'
> drivers/pci/slot.c: In function 'pci_destroy_slot':
> drivers/pci/slot.c:198: warning: format '%d' expects type 'int', but argument 4 has type 'atomic_t'
> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/slot.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -142,8 +142,8 @@ struct pci_slot *pci_create_slot(struct 
>  		if (slot->number == slot_nr) {
>  			kobject_get(&slot->kobj);
>  			dbg("%s: bumped refcount to %d on %x:%d\n",
> -				__func__, slot->kobj.kref.refcount,
> -				parent->number, slot_nr);
> +			    __func__, atomic_read(&slot->kobj.kref.refcount),
> +			    parent->number, slot_nr);
>  			goto out;
>  		}
>  	}
> @@ -196,7 +196,8 @@ int pci_destroy_slot(struct pci_slot *sl
>  	up_write(&pci_bus_sem);
>  
>  	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
> -		slot->kobj.kref.refcount, slot->bus->number, slot->number);
> +	    atomic_read(&slot->kobj.kref.refcount), slot->bus->number,
> +	    slot->number);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(pci_destroy_slot);
> 
> 

[PATCH 8/16][BUG] PCI slot: Fix invalid memory access (Not for mainline!)

[Kenji Kaneshige]

Fix kernel oops in pci_release_slot() when dbg() is enabled.

The dbg() in pci_release_slot() cause a kernel oops (invalid memory
access) because it accesses slot after it is released. We need to
access slot before calling kobject_put().

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/slot.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -191,13 +191,14 @@ EXPORT_SYMBOL_GPL(pci_create_slot);
 
 int pci_destroy_slot(struct pci_slot *slot)
 {
+	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
+	    atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,
+	    slot->number);
+
 	down_write(&pci_bus_sem);
 	kobject_put(&slot->kobj);
 	up_write(&pci_bus_sem);
 
-	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
-	    atomic_read(&slot->kobj.kref.refcount), slot->bus->number,
-	    slot->number);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pci_destroy_slot);

Re: [PATCH 8/16][BUG] PCI slot: Fix invalid memory access (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Fix kernel oops in pci_release_slot() when dbg() is enabled.

Merged, thanks.

/ac

> 
> The dbg() in pci_release_slot() cause a kernel oops (invalid memory
> access) because it accesses slot after it is released. We need to
> access slot before calling kobject_put().
> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/slot.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -191,13 +191,14 @@ EXPORT_SYMBOL_GPL(pci_create_slot);
>  
>  int pci_destroy_slot(struct pci_slot *slot)
>  {
> +	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
> +	    atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,
> +	    slot->number);
> +
>  	down_write(&pci_bus_sem);
>  	kobject_put(&slot->kobj);
>  	up_write(&pci_bus_sem);
>  
> -	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
> -	    atomic_read(&slot->kobj.kref.refcount), slot->bus->number,
> -	    slot->number);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(pci_destroy_slot);
> 
> 

[PATCH 9/16] PCI slot: Remove unused slot member from pci_dev (Not for mainline!)

[Kenji Kaneshige]

The 'slot' member in struct pci_dev is not used. Remove this unused
member.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 include/linux/pci.h |    1 -
 1 file changed, 1 deletion(-)

Index: linux-2.6.25-rc6/include/linux/pci.h
===================================================================
--- linux-2.6.25-rc6.orig/include/linux/pci.h
+++ linux-2.6.25-rc6/include/linux/pci.h
@@ -149,7 +149,6 @@ struct pci_dev {
 
 	void		*sysdata;	/* hook for sys-specific extension */
 	struct proc_dir_entry *procent;	/* device entry in /proc/bus/pci */
-	struct pci_slot	*slot;		/* Physical slot this device is in */
 
 	unsigned int	devfn;		/* encoded device & function index */
 	unsigned short	vendor;

Re: [PATCH 9/16] PCI slot: Remove unused slot member from pci_dev (Not for mainline!)

[Matthew Wilcox]

On Fri, Mar 21, 2008 at 01:16:19PM +0900, Kenji Kaneshige wrote:
> The 'slot' member in struct pci_dev is not used. Remove this unused
> member.

It *ought* to be used.  If it isn't yet, I guess that part of my patch
has been dropped, and it should be re-added.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 9/16] PCI slot: Remove unused slot member from pci_dev (Not for mainline!)

[Alex Chiang]

Hi willy,

* Matthew Wilcox <matthew@wil.cx>:
> On Fri, Mar 21, 2008 at 01:16:19PM +0900, Kenji Kaneshige wrote:
> > The 'slot' member in struct pci_dev is not used. Remove this unused
> > member.
> 
> It *ought* to be used.  If it isn't yet, I guess that part of my patch
> has been dropped, and it should be re-added.
 
I checked through your original patch set:

	http://www.parisc-linux.org/~willy/pci-slot-2b/

And although I see a bunch of uses of pci_bus->slot, I don't see
any uses of pci_dev->slot.

Wanna enlighten me? :)

/ac

[PATCH 10/16] PCI slot: Replace dbg with pr_debug (Not for mainline!)

[Kenji Kaneshige]

The dbg() macro doesn't work because the 'pci_slot_debug' flag never
be set unless we manually modify drivers/pci/slots.c. So we should use
pr_debug() instead. This patch replaces dbg() with pr_debug().

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/slot.c |   38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -9,15 +9,6 @@
 #include <linux/pci.h>
 #include "pci.h"
 
-static int pci_slot_debug;
-#define MY_NAME "slot"
-#define dbg(format, arg...)					\
-	do {							\
-		if (pci_slot_debug)				\
-			printk(KERN_DEBUG "%s: " format,	\
-				MY_NAME , ## arg);		\
-	} while (0)
-
 struct kset *pci_slots_kset;
 EXPORT_SYMBOL_GPL(pci_slots_kset);
 
@@ -71,8 +62,8 @@ static void pci_slot_release(struct kobj
 {
 	struct pci_slot *slot = to_pci_slot(kobj);
 
-	dbg("%s: releasing pci_slot on %x:%d\n", __func__,
-		slot->bus->number, slot->number);
+	pr_debug("%s: releasing pci_slot on %x:%d\n", __func__,
+		 slot->bus->number, slot->number);
 
 	list_del(&slot->list);
 
@@ -109,19 +100,19 @@ int pci_slot_add_hotplug(struct pci_bus 
 	}
 
 	if (!found) {
-		dbg("%s: slot not found\n", __func__);
+		pr_debug("%s: slot not found\n", __func__);
 		retval = -EEXIST;
 		goto out;
 	}
 
 	if (slot->release) {
-		dbg("%s: already claimed\n", __func__);
+		pr_debug("%s: already claimed\n", __func__);
 		retval = -EBUSY;
 		goto out;
 	}
 
-	dbg("%s: adding release function to %x:%d\n",
-		__func__, parent->number, slot_nr);
+	pr_debug("%s: adding release function to %x:%d\n",
+		 __func__, parent->number, slot_nr);
 	slot->release = release;
  out:
 	up_write(&pci_bus_sem);
@@ -141,9 +132,10 @@ struct pci_slot *pci_create_slot(struct 
 	list_for_each_entry(slot, &parent->slots, list) {
 		if (slot->number == slot_nr) {
 			kobject_get(&slot->kobj);
-			dbg("%s: bumped refcount to %d on %x:%d\n",
-			    __func__, atomic_read(&slot->kobj.kref.refcount),
-			    parent->number, slot_nr);
+			pr_debug("%s: bumped refcount to %d on %x:%d\n",
+				 __func__,
+				 atomic_read(&slot->kobj.kref.refcount),
+				 parent->number, slot_nr);
 			goto out;
 		}
 	}
@@ -173,8 +165,8 @@ struct pci_slot *pci_create_slot(struct 
 	INIT_LIST_HEAD(&slot->list);
 	list_add(&slot->list, &parent->slots);
 
-	dbg("%s: created pci_slot on %x:%d\n",
-		__func__, parent->number, slot_nr);
+	pr_debug("%s: created pci_slot on %x:%d\n",
+		 __func__, parent->number, slot_nr);
 
  out:
 	up_write(&pci_bus_sem);
@@ -191,9 +183,9 @@ EXPORT_SYMBOL_GPL(pci_create_slot);
 
 int pci_destroy_slot(struct pci_slot *slot)
 {
-	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
-	    atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,
-	    slot->number);
+	pr_debug("%s: decreased refcount to %d on %x:%d\n", __func__,
+		 atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,
+		 slot->number);
 
 	down_write(&pci_bus_sem);
 	kobject_put(&slot->kobj);

Re: [PATCH 10/16] PCI slot: Replace dbg with pr_debug (Not for mainline!)

[Matthew Wilcox]

On Fri, Mar 21, 2008 at 01:17:03PM +0900, Kenji Kaneshige wrote:
> The dbg() macro doesn't work because the 'pci_slot_debug' flag never
> be set unless we manually modify drivers/pci/slots.c. So we should use
> pr_debug() instead. This patch replaces dbg() with pr_debug().

ACK.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 10/16] PCI slot: Replace dbg with pr_debug (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> The dbg() macro doesn't work because the 'pci_slot_debug' flag never
> be set unless we manually modify drivers/pci/slots.c. So we should use
> pr_debug() instead. This patch replaces dbg() with pr_debug().

Merged, thanks.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/slot.c |   38 +++++++++++++++-----------------------
>  1 file changed, 15 insertions(+), 23 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -9,15 +9,6 @@
>  #include <linux/pci.h>
>  #include "pci.h"
>  
> -static int pci_slot_debug;
> -#define MY_NAME "slot"
> -#define dbg(format, arg...)					\
> -	do {							\
> -		if (pci_slot_debug)				\
> -			printk(KERN_DEBUG "%s: " format,	\
> -				MY_NAME , ## arg);		\
> -	} while (0)
> -
>  struct kset *pci_slots_kset;
>  EXPORT_SYMBOL_GPL(pci_slots_kset);
>  
> @@ -71,8 +62,8 @@ static void pci_slot_release(struct kobj
>  {
>  	struct pci_slot *slot = to_pci_slot(kobj);
>  
> -	dbg("%s: releasing pci_slot on %x:%d\n", __func__,
> -		slot->bus->number, slot->number);
> +	pr_debug("%s: releasing pci_slot on %x:%d\n", __func__,
> +		 slot->bus->number, slot->number);
>  
>  	list_del(&slot->list);
>  
> @@ -109,19 +100,19 @@ int pci_slot_add_hotplug(struct pci_bus 
>  	}
>  
>  	if (!found) {
> -		dbg("%s: slot not found\n", __func__);
> +		pr_debug("%s: slot not found\n", __func__);
>  		retval = -EEXIST;
>  		goto out;
>  	}
>  
>  	if (slot->release) {
> -		dbg("%s: already claimed\n", __func__);
> +		pr_debug("%s: already claimed\n", __func__);
>  		retval = -EBUSY;
>  		goto out;
>  	}
>  
> -	dbg("%s: adding release function to %x:%d\n",
> -		__func__, parent->number, slot_nr);
> +	pr_debug("%s: adding release function to %x:%d\n",
> +		 __func__, parent->number, slot_nr);
>  	slot->release = release;
>   out:
>  	up_write(&pci_bus_sem);
> @@ -141,9 +132,10 @@ struct pci_slot *pci_create_slot(struct 
>  	list_for_each_entry(slot, &parent->slots, list) {
>  		if (slot->number == slot_nr) {
>  			kobject_get(&slot->kobj);
> -			dbg("%s: bumped refcount to %d on %x:%d\n",
> -			    __func__, atomic_read(&slot->kobj.kref.refcount),
> -			    parent->number, slot_nr);
> +			pr_debug("%s: bumped refcount to %d on %x:%d\n",
> +				 __func__,
> +				 atomic_read(&slot->kobj.kref.refcount),
> +				 parent->number, slot_nr);
>  			goto out;
>  		}
>  	}
> @@ -173,8 +165,8 @@ struct pci_slot *pci_create_slot(struct 
>  	INIT_LIST_HEAD(&slot->list);
>  	list_add(&slot->list, &parent->slots);
>  
> -	dbg("%s: created pci_slot on %x:%d\n",
> -		__func__, parent->number, slot_nr);
> +	pr_debug("%s: created pci_slot on %x:%d\n",
> +		 __func__, parent->number, slot_nr);
>  
>   out:
>  	up_write(&pci_bus_sem);
> @@ -191,9 +183,9 @@ EXPORT_SYMBOL_GPL(pci_create_slot);
>  
>  int pci_destroy_slot(struct pci_slot *slot)
>  {
> -	dbg("%s: decreased refcount to %d on %x:%d\n", __func__,
> -	    atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,
> -	    slot->number);
> +	pr_debug("%s: decreased refcount to %d on %x:%d\n", __func__,
> +		 atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,
> +		 slot->number);
>  
>  	down_write(&pci_bus_sem);
>  	kobject_put(&slot->kobj);
> 
> 

[PATCH 11/16] PCI slot: Remove useless release handler (Not for mainline!)

[Kenji Kaneshige]

The release() handler of struct pci_slot never be called because
pci_hp_deregister() calls pci_slot_destroy() after setting
slot->release with NULL. So we don't need release() handler of struct
pci_slot. In addition, we don't need pci_slot_add_hotplug().

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/hotplug/pci_hotplug_core.c |   15 -----------
 drivers/pci/slot.c                     |   44 ---------------------------------
 include/linux/pci.h                    |    3 --
 3 files changed, 1 insertion(+), 61 deletions(-)

Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -67,9 +67,6 @@ static void pci_slot_release(struct kobj
 
 	list_del(&slot->list);
 
-	if (slot->release)
-		slot->release(slot);
-
 	remove_sysfs_files(slot);
 	kfree(slot);
 }
@@ -79,47 +76,6 @@ static struct kobj_type pci_slot_ktype =
 	.release = &pci_slot_release,
 };
 
-int pci_slot_add_hotplug(struct pci_bus *parent, int slot_nr,
-			 void (*release)(struct pci_slot *))
-{
-	struct pci_slot *slot;
-	int retval, found;
-
-	retval = found = 0;
-
-	down_write(&pci_bus_sem);
-
-	/* This slot should have already been created, so look for it. If
-	 * we can't find it, return -EEXIST.
-	 */
-	list_for_each_entry(slot, &parent->slots, list) {
-		if (slot->number == slot_nr) {
-			found = 1;
-			break;
-		}
-	}
-
-	if (!found) {
-		pr_debug("%s: slot not found\n", __func__);
-		retval = -EEXIST;
-		goto out;
-	}
-
-	if (slot->release) {
-		pr_debug("%s: already claimed\n", __func__);
-		retval = -EBUSY;
-		goto out;
-	}
-
-	pr_debug("%s: adding release function to %x:%d\n",
-		 __func__, parent->number, slot_nr);
-	slot->release = release;
- out:
-	up_write(&pci_bus_sem);
-	return retval;
-}
-EXPORT_SYMBOL_GPL(pci_slot_add_hotplug);
-
 struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
 				 const char *name)
 {
Index: linux-2.6.25-rc6/include/linux/pci.h
===================================================================
--- linux-2.6.25-rc6.orig/include/linux/pci.h
+++ linux-2.6.25-rc6/include/linux/pci.h
@@ -135,7 +135,6 @@ struct pci_slot {
 	struct hotplug_slot *hotplug;	/* Hotplug info (migrate over time) */
 	unsigned char number;		/* PCI_SLOT(pci_dev->devfn) */
 	struct kobject kobj;
-	void (*release)(struct pci_slot *);
 };
 
 /*
@@ -494,8 +493,6 @@ struct pci_bus *pci_add_new_bus(struct p
 				int busnr);
 struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
 				 const char *name);
-int pci_slot_add_hotplug(struct pci_bus *parent, int slot_nr,
-			 void (*release)(struct pci_slot *));
 int pci_destroy_slot(struct pci_slot *slot);
 int pci_scan_slot(struct pci_bus *bus, int devfn);
 struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn);
Index: linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/hotplug/pci_hotplug_core.c
+++ linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
@@ -537,12 +537,6 @@ static struct hotplug_slot *get_slot_fro
 	return NULL;
 }
 
-static void hotplug_release(struct pci_slot *slot)
-{
-	struct hotplug_slot *hotplug = slot->hotplug;
-	hotplug->release(hotplug);
-}
-
 /**
  * pci_hp_register - register a hotplug_slot with the PCI hotplug subsystem
  * @slot: pointer to the &struct hotplug_slot to register
@@ -576,12 +570,6 @@ int pci_hp_register(struct hotplug_slot 
 	if (IS_ERR(pci_slot))
 		return PTR_ERR(pci_slot);
 
-	result = pci_slot_add_hotplug(bus, slot_nr, hotplug_release);
-	if (result) {
-		pci_destroy_slot(pci_slot);
-		return result;
-	}
-
 	slot->pci_slot = pci_slot;
 	pci_slot->hotplug = slot;
 
@@ -631,8 +619,7 @@ int pci_hp_deregister(struct hotplug_slo
 	fs_remove_slot(slot);
 	dbg("Removed slot %s from the list\n", hotplug->name);
 
-	hotplug_release(slot);
-	slot->release = NULL;
+	hotplug->release(hotplug);
 	pci_destroy_slot(slot);
 
 	return 0;

Re: [PATCH 11/16] PCI slot: Remove useless release handler (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> The release() handler of struct pci_slot never be called because
> pci_hp_deregister() calls pci_slot_destroy() after setting
> slot->release with NULL. So we don't need release() handler of struct
> pci_slot. In addition, we don't need pci_slot_add_hotplug().

Thanks, this was a good change. I've tested and merged it.

I did make two small changes though:

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/hotplug/pci_hotplug_core.c |   15 -----------
>  drivers/pci/slot.c                     |   44 ---------------------------------
>  include/linux/pci.h                    |    3 --
>  3 files changed, 1 insertion(+), 61 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -67,9 +67,6 @@ static void pci_slot_release(struct kobj
>  
>  	list_del(&slot->list);
>  
> -	if (slot->release)
> -		slot->release(slot);
> -
>  	remove_sysfs_files(slot);
>  	kfree(slot);
>  }
> @@ -79,47 +76,6 @@ static struct kobj_type pci_slot_ktype =
>  	.release = &pci_slot_release,
>  };
>  
> -int pci_slot_add_hotplug(struct pci_bus *parent, int slot_nr,
> -			 void (*release)(struct pci_slot *))
> -{
> -	struct pci_slot *slot;
> -	int retval, found;
> -
> -	retval = found = 0;
> -
> -	down_write(&pci_bus_sem);
> -
> -	/* This slot should have already been created, so look for it. If
> -	 * we can't find it, return -EEXIST.
> -	 */
> -	list_for_each_entry(slot, &parent->slots, list) {
> -		if (slot->number == slot_nr) {
> -			found = 1;
> -			break;
> -		}
> -	}
> -
> -	if (!found) {
> -		pr_debug("%s: slot not found\n", __func__);
> -		retval = -EEXIST;
> -		goto out;
> -	}
> -
> -	if (slot->release) {
> -		pr_debug("%s: already claimed\n", __func__);
> -		retval = -EBUSY;
> -		goto out;
> -	}
> -
> -	pr_debug("%s: adding release function to %x:%d\n",
> -		 __func__, parent->number, slot_nr);
> -	slot->release = release;
> - out:
> -	up_write(&pci_bus_sem);
> -	return retval;
> -}
> -EXPORT_SYMBOL_GPL(pci_slot_add_hotplug);
> -
>  struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
>  				 const char *name)
>  {
> Index: linux-2.6.25-rc6/include/linux/pci.h
> ===================================================================
> --- linux-2.6.25-rc6.orig/include/linux/pci.h
> +++ linux-2.6.25-rc6/include/linux/pci.h
> @@ -135,7 +135,6 @@ struct pci_slot {
>  	struct hotplug_slot *hotplug;	/* Hotplug info (migrate over time) */
>  	unsigned char number;		/* PCI_SLOT(pci_dev->devfn) */
>  	struct kobject kobj;
> -	void (*release)(struct pci_slot *);
>  };
>  
>  /*
> @@ -494,8 +493,6 @@ struct pci_bus *pci_add_new_bus(struct p
>  				int busnr);
>  struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
>  				 const char *name);
> -int pci_slot_add_hotplug(struct pci_bus *parent, int slot_nr,
> -			 void (*release)(struct pci_slot *));
>  int pci_destroy_slot(struct pci_slot *slot);
>  int pci_scan_slot(struct pci_bus *bus, int devfn);
>  struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn);
> Index: linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/hotplug/pci_hotplug_core.c
> +++ linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
> @@ -537,12 +537,6 @@ static struct hotplug_slot *get_slot_fro
>  	return NULL;
>  }
>  
> -static void hotplug_release(struct pci_slot *slot)
> -{
> -	struct hotplug_slot *hotplug = slot->hotplug;
> -	hotplug->release(hotplug);
> -}
> -
>  /**
>   * pci_hp_register - register a hotplug_slot with the PCI hotplug subsystem
>   * @slot: pointer to the &struct hotplug_slot to register
> @@ -576,12 +570,6 @@ int pci_hp_register(struct hotplug_slot 
>  	if (IS_ERR(pci_slot))
>  		return PTR_ERR(pci_slot);

This prevents multiple hotplug drivers from claiming the same
slot.

+       if (pci_slot->hotplug) {
+               dbg("%s: already claimed\n", __func__);
+               pci_destroy_slot(pci_slot);
+               return -EBUSY;

>  
> -	result = pci_slot_add_hotplug(bus, slot_nr, hotplug_release);
> -	if (result) {
> -		pci_destroy_slot(pci_slot);
> -		return result;
> -	}
> -
>  	slot->pci_slot = pci_slot;
>  	pci_slot->hotplug = slot;
>  
> @@ -631,8 +619,7 @@ int pci_hp_deregister(struct hotplug_slo
>  	fs_remove_slot(slot);
>  	dbg("Removed slot %s from the list\n", hotplug->name);
>  
> -	hotplug_release(slot);
> -	slot->release = NULL;
> +	hotplug->release(hotplug);

+       slot->hotplug = NULL;

The above is needed if pci_slot is loaded, and you wish to
repeatedly load/unload acpiphp/pciehp/other hp drivers.

Thanks,

/ac

>  	pci_destroy_slot(slot);
>  
>  	return 0;
> 
> 

[PATCH 12/16] PCI slot: Use .default_attrs for address file (Not for mainline!)

[Kenji Kaneshige]

Use .default_attrs for 'address' file. This can simplify the code.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/slot.c |   36 +++++++++---------------------------
 1 file changed, 9 insertions(+), 27 deletions(-)

Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -39,25 +39,6 @@ static ssize_t address_read_file(struct 
 					slot->bus->number, slot->number);
 }
 
-static struct pci_slot_attribute pci_slot_attr_address = {
-	.attr = { .name = "address", .mode = S_IFREG | S_IRUGO },
-	.show = address_read_file,
-};
-
-static void remove_sysfs_files(struct pci_slot *slot)
-{
-	sysfs_remove_file(&slot->kobj, &pci_slot_attr_address.attr);
-}
-
-static int create_sysfs_files(struct pci_slot *slot)
-{
-	int result;
-
-	result = sysfs_create_file(&slot->kobj, &pci_slot_attr_address.attr);
-
-	return result;
-}
-
 static void pci_slot_release(struct kobject *kobj)
 {
 	struct pci_slot *slot = to_pci_slot(kobj);
@@ -67,13 +48,21 @@ static void pci_slot_release(struct kobj
 
 	list_del(&slot->list);
 
-	remove_sysfs_files(slot);
 	kfree(slot);
 }
 
+static struct pci_slot_attribute pci_slot_attr_address =
+	__ATTR(address, (S_IFREG | S_IRUGO), address_read_file, NULL);
+
+static struct attribute *pci_slot_default_attrs[] = {
+	&pci_slot_attr_address.attr,
+	NULL,
+};
+
 static struct kobj_type pci_slot_ktype = {
 	.sysfs_ops = &pci_slot_sysfs_ops,
 	.release = &pci_slot_release,
+	.default_attrs = pci_slot_default_attrs,
 };
 
 struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
@@ -114,10 +103,6 @@ struct pci_slot *pci_create_slot(struct 
 		goto err;
 	}
 
-	err = create_sysfs_files(slot);
-	if (err)
-		goto unregister;
-
 	INIT_LIST_HEAD(&slot->list);
 	list_add(&slot->list, &parent->slots);
 
@@ -127,9 +112,6 @@ struct pci_slot *pci_create_slot(struct 
  out:
 	up_write(&pci_bus_sem);
 	return slot;
-
- unregister:
-	kobject_put(&slot->kobj);
  err:
 	kfree(slot);
 	slot = ERR_PTR(err);

Re: [PATCH 12/16] PCI slot: Use .default_attrs for address file (Not for mainline!)

[Matthew Wilcox]

On Fri, Mar 21, 2008 at 01:19:39PM +0900, Kenji Kaneshige wrote:
> Use .default_attrs for 'address' file. This can simplify the code.

Seems like a good idea to me.  I don't think default_attrs was available
when I initially did this work.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 12/16] PCI slot: Use .default_attrs for address file (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Use .default_attrs for 'address' file. This can simplify the code.

Thanks, merged.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/slot.c |   36 +++++++++---------------------------
>  1 file changed, 9 insertions(+), 27 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -39,25 +39,6 @@ static ssize_t address_read_file(struct 
>  					slot->bus->number, slot->number);
>  }
>  
> -static struct pci_slot_attribute pci_slot_attr_address = {
> -	.attr = { .name = "address", .mode = S_IFREG | S_IRUGO },
> -	.show = address_read_file,
> -};
> -
> -static void remove_sysfs_files(struct pci_slot *slot)
> -{
> -	sysfs_remove_file(&slot->kobj, &pci_slot_attr_address.attr);
> -}
> -
> -static int create_sysfs_files(struct pci_slot *slot)
> -{
> -	int result;
> -
> -	result = sysfs_create_file(&slot->kobj, &pci_slot_attr_address.attr);
> -
> -	return result;
> -}
> -
>  static void pci_slot_release(struct kobject *kobj)
>  {
>  	struct pci_slot *slot = to_pci_slot(kobj);
> @@ -67,13 +48,21 @@ static void pci_slot_release(struct kobj
>  
>  	list_del(&slot->list);
>  
> -	remove_sysfs_files(slot);
>  	kfree(slot);
>  }
>  
> +static struct pci_slot_attribute pci_slot_attr_address =
> +	__ATTR(address, (S_IFREG | S_IRUGO), address_read_file, NULL);
> +
> +static struct attribute *pci_slot_default_attrs[] = {
> +	&pci_slot_attr_address.attr,
> +	NULL,
> +};
> +
>  static struct kobj_type pci_slot_ktype = {
>  	.sysfs_ops = &pci_slot_sysfs_ops,
>  	.release = &pci_slot_release,
> +	.default_attrs = pci_slot_default_attrs,
>  };
>  
>  struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
> @@ -114,10 +103,6 @@ struct pci_slot *pci_create_slot(struct 
>  		goto err;
>  	}
>  
> -	err = create_sysfs_files(slot);
> -	if (err)
> -		goto unregister;
> -
>  	INIT_LIST_HEAD(&slot->list);
>  	list_add(&slot->list, &parent->slots);
>  
> @@ -127,9 +112,6 @@ struct pci_slot *pci_create_slot(struct 
>   out:
>  	up_write(&pci_bus_sem);
>  	return slot;
> -
> - unregister:
> -	kobject_put(&slot->kobj);
>   err:
>  	kfree(slot);
>  	slot = ERR_PTR(err);
> 
> 

[PATCH 13/16] PCI slot: Fix return value of pci_create_slot() (Not for mainline!)

[Kenji Kaneshige]

We can use the error code from kobject_init_and_add() as it is.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/slot.c |    1 -
 1 file changed, 1 deletion(-)

Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -99,7 +99,6 @@ struct pci_slot *pci_create_slot(struct 
 				   "%s", name);
 	if (err) {
 		printk(KERN_ERR "Unable to register kobject %s\n", name);
-		err = -EINVAL;
 		goto err;
 	}
 

Re: [PATCH 13/16] PCI slot: Fix return value of pci_create_slot() (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> We can use the error code from kobject_init_and_add() as it is.

Thanks, merged.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/slot.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -99,7 +99,6 @@ struct pci_slot *pci_create_slot(struct 
>  				   "%s", name);
>  	if (err) {
>  		printk(KERN_ERR "Unable to register kobject %s\n", name);
> -		err = -EINVAL;
>  		goto err;
>  	}
>  
> 
> 

[PATCH 14/16] PCI slot: Change return value of pci_destroy_slot() (Not for mainline!)

[Kenji Kaneshige]

Since nobody checks the return value from pci_destroy_slot(), it
should be void.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/slot.c  |    4 +---
 include/linux/pci.h |    2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -118,7 +118,7 @@ struct pci_slot *pci_create_slot(struct 
 }
 EXPORT_SYMBOL_GPL(pci_create_slot);
 
-int pci_destroy_slot(struct pci_slot *slot)
+void pci_destroy_slot(struct pci_slot *slot)
 {
 	pr_debug("%s: decreased refcount to %d on %x:%d\n", __func__,
 		 atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,
@@ -127,8 +127,6 @@ int pci_destroy_slot(struct pci_slot *sl
 	down_write(&pci_bus_sem);
 	kobject_put(&slot->kobj);
 	up_write(&pci_bus_sem);
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(pci_destroy_slot);
 
Index: linux-2.6.25-rc6/include/linux/pci.h
===================================================================
--- linux-2.6.25-rc6.orig/include/linux/pci.h
+++ linux-2.6.25-rc6/include/linux/pci.h
@@ -493,7 +493,7 @@ struct pci_bus *pci_add_new_bus(struct p
 				int busnr);
 struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
 				 const char *name);
-int pci_destroy_slot(struct pci_slot *slot);
+void pci_destroy_slot(struct pci_slot *slot);
 int pci_scan_slot(struct pci_bus *bus, int devfn);
 struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn);
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus);

Re: [PATCH 14/16] PCI slot: Change return value of pci_destroy_slot() (Not for mainline!)

[Matthew Wilcox]

On Fri, Mar 21, 2008 at 01:26:13PM +0900, Kenji Kaneshige wrote:
> Since nobody checks the return value from pci_destroy_slot(), it
> should be void.

ACK

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 14/16] PCI slot: Change return value of pci_destroy_slot() (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Since nobody checks the return value from pci_destroy_slot(), it
> should be void.

Thanks, merged.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/slot.c  |    4 +---
>  include/linux/pci.h |    2 +-
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -118,7 +118,7 @@ struct pci_slot *pci_create_slot(struct 
>  }
>  EXPORT_SYMBOL_GPL(pci_create_slot);
>  
> -int pci_destroy_slot(struct pci_slot *slot)
> +void pci_destroy_slot(struct pci_slot *slot)
>  {
>  	pr_debug("%s: decreased refcount to %d on %x:%d\n", __func__,
>  		 atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,
> @@ -127,8 +127,6 @@ int pci_destroy_slot(struct pci_slot *sl
>  	down_write(&pci_bus_sem);
>  	kobject_put(&slot->kobj);
>  	up_write(&pci_bus_sem);
> -
> -	return 0;
>  }
>  EXPORT_SYMBOL_GPL(pci_destroy_slot);
>  
> Index: linux-2.6.25-rc6/include/linux/pci.h
> ===================================================================
> --- linux-2.6.25-rc6.orig/include/linux/pci.h
> +++ linux-2.6.25-rc6/include/linux/pci.h
> @@ -493,7 +493,7 @@ struct pci_bus *pci_add_new_bus(struct p
>  				int busnr);
>  struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
>  				 const char *name);
> -int pci_destroy_slot(struct pci_slot *slot);
> +void pci_destroy_slot(struct pci_slot *slot);
>  int pci_scan_slot(struct pci_bus *bus, int devfn);
>  struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn);
>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus);
> 
> 

[PATCH 15/16] PCI slot: Trivial cleanups for slot.c (Not for mainline!)

[Kenji Kaneshige]

Some trivial cleanups for drivers/pci/slot.c.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/slot.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Index: linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/slot.c
+++ linux-2.6.25-rc6/drivers/pci/slot.c
@@ -132,19 +132,16 @@ EXPORT_SYMBOL_GPL(pci_destroy_slot);
 
 static int pci_slot_init(void)
 {
-	int result = 0;
 	struct kset *pci_bus_kset;
 
 	pci_bus_kset = bus_get_kset(&pci_bus_type);
-
 	pci_slots_kset = kset_create_and_add("slots", NULL,
 						&pci_bus_kset->kobj);
 	if (!pci_slots_kset) {
-		result = -ENOMEM;
-		printk(KERN_ERR "PCI: Slot initialization failure (%d)\n",
-			result);
+		printk(KERN_ERR "PCI: Slot initialization failure\n");
+		return -ENOMEM;
 	}
-	return result;
+	return 0;
 }
 
 subsys_initcall(pci_slot_init);

Re: [PATCH 15/16] PCI slot: Trivial cleanups for slot.c (Not for mainline!)

[Matthew Wilcox]

On Fri, Mar 21, 2008 at 01:26:50PM +0900, Kenji Kaneshige wrote:
> Some trivial cleanups for drivers/pci/slot.c.

ACK

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 15/16] PCI slot: Trivial cleanups for slot.c (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Some trivial cleanups for drivers/pci/slot.c.

Thanks, merged.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/slot.c |    9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/slot.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/slot.c
> +++ linux-2.6.25-rc6/drivers/pci/slot.c
> @@ -132,19 +132,16 @@ EXPORT_SYMBOL_GPL(pci_destroy_slot);
>  
>  static int pci_slot_init(void)
>  {
> -	int result = 0;
>  	struct kset *pci_bus_kset;
>  
>  	pci_bus_kset = bus_get_kset(&pci_bus_type);
> -
>  	pci_slots_kset = kset_create_and_add("slots", NULL,
>  						&pci_bus_kset->kobj);
>  	if (!pci_slots_kset) {
> -		result = -ENOMEM;
> -		printk(KERN_ERR "PCI: Slot initialization failure (%d)\n",
> -			result);
> +		printk(KERN_ERR "PCI: Slot initialization failure\n");
> +		return -ENOMEM;
>  	}
> -	return result;
> +	return 0;
>  }
>  
>  subsys_initcall(pci_slot_init);
> 
> 

[PATCH 16/16][BUG] PCI hotplug core: add missing lock for hotplug slot list (Not for mainline!)

[Kenji Kaneshige]

Add missing lock for manipulating pci_hotplug_slot_list.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/hotplug/pci_hotplug_core.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Index: linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/pci/hotplug/pci_hotplug_core.c
+++ linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
@@ -61,6 +61,7 @@ static int debug;
 //////////////////////////////////////////////////////////////////
 
 static LIST_HEAD(pci_hotplug_slot_list);
+static DEFINE_SPINLOCK(pci_hotplug_slot_list_lock);
 
 /* these strings match up with the values in pci_bus_speed */
 static char *pci_bus_speed_strings[] = {
@@ -529,12 +530,16 @@ static struct hotplug_slot *get_slot_fro
 	struct hotplug_slot *slot;
 	struct list_head *tmp;
 
+	spin_lock(&pci_hotplug_slot_list_lock);
 	list_for_each (tmp, &pci_hotplug_slot_list) {
 		slot = list_entry (tmp, struct hotplug_slot, slot_list);
 		if (strcmp(slot->name, name) == 0)
-			return slot;
+			goto out;
 	}
-	return NULL;
+	slot = NULL;
+out:
+	spin_unlock(&pci_hotplug_slot_list_lock);
+	return slot;
 }
 
 /**
@@ -584,7 +589,9 @@ int pci_hp_register(struct hotplug_slot 
 		}
 	}
 
+	spin_lock(&pci_hotplug_slot_list_lock);
 	list_add(&slot->slot_list, &pci_hotplug_slot_list);
+	spin_unlock(&pci_hotplug_slot_list_lock);
 
 	result = fs_add_slot(pci_slot);
 	kobject_uevent(&pci_slot->kobj, KOBJ_ADD);
@@ -613,7 +620,9 @@ int pci_hp_deregister(struct hotplug_slo
 	if (temp != hotplug)
 		return -ENODEV;
 
+	spin_lock(&pci_hotplug_slot_list_lock);
 	list_del(&hotplug->slot_list);
+	spin_unlock(&pci_hotplug_slot_list_lock);
 
 	slot = hotplug->pci_slot;
 	fs_remove_slot(slot);

Re: [PATCH 16/16][BUG] PCI hotplug core: add missing lock for hotplug slot list (Not for mainline!)

[Alex Chiang]

* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Add missing lock for manipulating pci_hotplug_slot_list.

Nice work, thanks. I've merged it.

/ac

> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> ---
>  drivers/pci/hotplug/pci_hotplug_core.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/drivers/pci/hotplug/pci_hotplug_core.c
> +++ linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
> @@ -61,6 +61,7 @@ static int debug;
>  //////////////////////////////////////////////////////////////////
>  
>  static LIST_HEAD(pci_hotplug_slot_list);
> +static DEFINE_SPINLOCK(pci_hotplug_slot_list_lock);
>  
>  /* these strings match up with the values in pci_bus_speed */
>  static char *pci_bus_speed_strings[] = {
> @@ -529,12 +530,16 @@ static struct hotplug_slot *get_slot_fro
>  	struct hotplug_slot *slot;
>  	struct list_head *tmp;
>  
> +	spin_lock(&pci_hotplug_slot_list_lock);
>  	list_for_each (tmp, &pci_hotplug_slot_list) {
>  		slot = list_entry (tmp, struct hotplug_slot, slot_list);
>  		if (strcmp(slot->name, name) == 0)
> -			return slot;
> +			goto out;
>  	}
> -	return NULL;
> +	slot = NULL;
> +out:
> +	spin_unlock(&pci_hotplug_slot_list_lock);
> +	return slot;
>  }
>  
>  /**
> @@ -584,7 +589,9 @@ int pci_hp_register(struct hotplug_slot 
>  		}
>  	}
>  
> +	spin_lock(&pci_hotplug_slot_list_lock);
>  	list_add(&slot->slot_list, &pci_hotplug_slot_list);
> +	spin_unlock(&pci_hotplug_slot_list_lock);
>  
>  	result = fs_add_slot(pci_slot);
>  	kobject_uevent(&pci_slot->kobj, KOBJ_ADD);
> @@ -613,7 +620,9 @@ int pci_hp_deregister(struct hotplug_slo
>  	if (temp != hotplug)
>  		return -ENODEV;
>  
> +	spin_lock(&pci_hotplug_slot_list_lock);
>  	list_del(&hotplug->slot_list);
> +	spin_unlock(&pci_hotplug_slot_list_lock);
>  
>  	slot = hotplug->pci_slot;
>  	fs_remove_slot(slot);
> 
> 

Re: [PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects

[Alex Chiang]

Hi Kenji-san,

> I've reviewed and tested the latest version of Alex's "introduce
> pci_slot" series 
>
> But on the other hand, I found some bugs and some codes need to
> be changed. 

Thank you very much for the review and patches!

I will get started on reviewing / merging / testing these as soon
as I can, and will respond to each patch individually.

Thanks!

/ac