Skip to contentSkip to conversation

Physical PCI slot objects

PATCH Superseded Alex Chiang 4 participants 13 messages Nov 14, 2007 – Nov 15, 2007

Subsystems (5)

v2 of 16 Superseded Latest is v16 →

lore
Share & tools
lore
#0 lore raw

v2 cover Physical PCI slot objects

* Alex Chiang <[email protected]>:

Quoted 4 lines

Actually, I just reworked my patch this morning, and believe that I have a much cleaner implementation now that should fix a lot of the errors you saw.

Patch 1/5 - reworked to fix stupid compile bug
Patch 2/5 - incorporate Eike, Linas, and Willy's comments Patch 3/5 - no change
Patch 4/5 - was acpi-pci-slot-driver patch, now modifies
pci_add_hotplug(). I changed the ordering on this so the tree doesn't break at this point in the series
Patch 5/5 - now is acpi-pci-slot-driver patch, cleaned up
implementation so our slot detection is a little better

Thanks.

/ac

#1 lore raw

PATCH v2 1/5 Remove path attribute from sgi_hotplug

Rename the slot to be the contents of the 'path' sysfs attribute, and delete the attribute. The mapping from pci address to slot name is supposed to be done through the 'address' file, which will be provided automatically later in this series of patches.

---
 drivers/pci/hotplug/sgi_hotplug.c |   32 +-------------------------------
 1 files changed, 1 insertions(+), 31 deletions(-)
drivers/pci/hotplug/sgi_hotplug.c +1 -31
index ef07c36..693519e 100644
@@ -91,21 +91,6 @@ static struct hotplug_slot_ops sn_hotplug_slot_ops = {
 
 static DEFINE_MUTEX(sn_hotplug_mutex);
 
-static ssize_t path_show (struct hotplug_slot *bss_hotplug_slot,
-	       		  char *buf)
-{
-	int retval = -ENOENT;
-	struct slot *slot = bss_hotplug_slot->private;
-
-	if (!slot)
-		return retval;
-
-	retval = sprintf (buf, "%s\n", slot->physical_path);
-	return retval;
-}
-
-static struct hotplug_slot_attribute sn_slot_path_attr = __ATTR_RO(path);
-
 static int sn_pci_slot_valid(struct pci_bus *pci_bus, int device)
 {
 	struct pcibus_info *pcibus_info;
@@ -173,18 +158,10 @@ static int sn_hp_slot_private_alloc(struct hotplug_slot *bss_hotplug_slot,
 		return -ENOMEM;
 	bss_hotplug_slot->private = slot;
 
-	bss_hotplug_slot->name = kmalloc(SN_SLOT_NAME_SIZE, GFP_KERNEL);
-	if (!bss_hotplug_slot->name) {
-		kfree(bss_hotplug_slot->private);
-		return -ENOMEM;
-	}
+	bss_hotplug_slot->name = slot->physical_path;
 
 	slot->device_num = device;
 	slot->pci_bus = pci_bus;
-	sprintf(bss_hotplug_slot->name, "%04x:%02x:%02x",
-		pci_domain_nr(pci_bus),
-		((u16)pcibus_info->pbi_buscommon.bs_persist_busnum),
-		device + 1);
 
 	sn_generate_path(pci_bus, slot->physical_path);
 
@@ -203,8 +180,6 @@ static struct hotplug_slot * sn_hp_destroy(void)
 		bss_hotplug_slot = slot->hotplug_slot;
 		list_del(&((struct slot *)bss_hotplug_slot->private)->
 			 hp_list);
-		sysfs_remove_file(&bss_hotplug_slot->kobj,
-				  &sn_slot_path_attr.attr);
 		break;
 	}
 	return bss_hotplug_slot;
@@ -653,11 +628,6 @@ static int sn_hotplug_slot_register(struct pci_bus *pci_bus)
 		rc = pci_hp_register(bss_hotplug_slot);
 		if (rc)
 			goto register_err;
-
-		rc = sysfs_create_file(&bss_hotplug_slot->kobj,
-				       &sn_slot_path_attr.attr);
-		if (rc)
-			goto register_err;
 	}
 	dev_dbg(&pci_bus->self->dev, "Registered bus with hotplug\n");
 	return rc;
Signature · 1 line
1.5.3.1.1.g1e61
#2 lore raw

PATCH v2 2/5 Construct one fakephp slot per pci slot

In reply to 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.

---
 drivers/pci/hotplug/fakephp.c |   82 ++++++++++++++++-------------------------
 1 files changed, 32 insertions(+), 50 deletions(-)
drivers/pci/hotplug/fakephp.c +32 -50
index 027f686..851796b 100644
@@ -59,10 +59,13 @@
 #define DRIVER_AUTHOR	"Greg Kroah-Hartman <[email protected]>"
 #define DRIVER_DESC	"Fake PCI Hot Plug Controller Driver"
 
+#define SLOT_NAME_SIZE KOBJ_NAME_LEN
+
 struct dummy_slot {
 	struct list_head node;
 	struct hotplug_slot *slot;
 	struct pci_dev *dev;
+	char name[8];
 };
 
 static int debug;
@@ -93,6 +96,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)
@@ -106,13 +110,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 = kmalloc(sizeof(struct dummy_slot), GFP_KERNEL);
 	if (!dslot)
 		goto error_info;
 
+	slot->name = dslot->name;
+	snprintf(slot->name, 8, "fake%d", count++);
+	dbg("slot->name = %s\n", slot->name);
+
 	slot->ops = &dummy_hotplug_slot_ops;
 	slot->release = &dummy_release;
 	slot->private = dslot;
@@ -141,17 +146,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)
@@ -275,23 +280,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;
 
@@ -301,36 +292,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;
-	}
-	/* 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;
+
+		/* don't disable bridged devices just yet, we can't handle
+		 * them easily... */
+		if (dev->subordinate) {
+			err("Can't remove PCI devices with other PCI devices behind it yet.\n");
+			return -ENODEV;
 		}
-	}
 
-	/* remove the device from the pci core */
-	pci_remove_bus_device(dslot->dev);
 
-	/* blow away this sysfs entry and other parts. */
-	remove_slot(dslot);
+		/* remove the device from the pci core */
+		pci_remove_bus_device(dslot->dev);
 
+		/* blow away this sysfs entry and other parts. */
+		remove_slot(dslot);
+
+		pci_dev_put(dev);
+	}
 	return 0;
 }
 
Signature · 1 line
1.5.3.1.1.g1e61
#3 lore raw

Re: [PATCH 2/5] Construct one fakephp slot per pci slot

Quoted 4 lines · 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.

+#define SLOT_NAME_SIZE KOBJ_NAME_LEN

Defined, then never used ... how about s/KOBJ_NAME_LEN/8/, then use SLOT_NAME_SIZE throughout?

Signature · 4 lines
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."
#4 lore raw

Re: [PATCH 2/5] Construct one fakephp slot per pci slot

* Matthew Wilcox <[email protected]>:

Quoted 8 lines · Alex Chiang
Quoted 4 lines

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.

+#define SLOT_NAME_SIZE KOBJ_NAME_LEN

Defined, then never used ... how about s/KOBJ_NAME_LEN/8/, then use SLOT_NAME_SIZE throughout?

Whoops, good catch. I had a braino there.

Thanks.

/ac

#5 lore raw

Re: [PATCH 2/5] Construct one fakephp slot per pci slot

Am Mittwoch, 14. November 2007 schrieb Alex Chiang:

Quoted 11 lines

* Matthew Wilcox <[email protected]>:

Quoted 8 lines · Alex Chiang
Quoted 4 lines

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.

+#define SLOT_NAME_SIZE KOBJ_NAME_LEN

Defined, then never used ... how about s/KOBJ_NAME_LEN/8/, then use SLOT_NAME_SIZE throughout?

Whoops, good catch. I had a braino there.

Just drop it and use sizeof(dslot->name) in snprintf(). Just be careful not to use slot->name there as it would always be sizeof(void*)!

Eike

#6 lore raw

Re: [PATCH 2/5] Construct one fakephp slot per pci slot

* Rolf Eike Beer <[email protected]>:

Quoted 9 lines

Am Mittwoch, 14. November 2007 schrieb Alex Chiang:

Quoted 8 lines

* Matthew Wilcox <[email protected]>:

Quoted 7 lines

On Wed, Nov 14, 2007 at 12:37:29PM -0700, Alex Chiang

Quoted 5 lines

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.

+#define SLOT_NAME_SIZE KOBJ_NAME_LEN

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

Defined, then never used ... how about s/KOBJ_NAME_LEN/8/, then use SLOT_NAME_SIZE throughout?

Whoops, good catch. I had a braino there.

Just drop it and use sizeof(dslot->name) in snprintf(). Just be careful not to use slot->name there as it would always be sizeof(void*)!

Thanks, even better.

/ac

#7 lore raw

Re: [PATCH 2/5] Construct one fakephp slot per pci slot

In reply to Matthew Wilcox

* Matthew Wilcox <[email protected]>:

Quoted 8 lines · Alex Chiang
Quoted 4 lines

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.

+#define SLOT_NAME_SIZE KOBJ_NAME_LEN

Defined, then never used ... how about s/KOBJ_NAME_LEN/8/, then use SLOT_NAME_SIZE throughout?

Here's another try with your suggestion.

From: Alex Chiang <[email protected]>

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.

---
 drivers/pci/hotplug/fakephp.c |   82 ++++++++++++++++-------------------------
 1 files changed, 32 insertions(+), 50 deletions(-)
drivers/pci/hotplug/fakephp.c +32 -50
index 027f686..1759ea1 100644
@@ -59,10 +59,13 @@
 #define DRIVER_AUTHOR	"Greg Kroah-Hartman <[email protected]>"
 #define DRIVER_DESC	"Fake PCI Hot Plug Controller Driver"
 
+#define SLOT_NAME_SIZE 8
+
 struct dummy_slot {
 	struct list_head node;
 	struct hotplug_slot *slot;
 	struct pci_dev *dev;
+	char name[SLOT_NAME_SIZE];
 };
 
 static int debug;
@@ -93,6 +96,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)
@@ -106,13 +110,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 = kmalloc(sizeof(struct dummy_slot), GFP_KERNEL);
 	if (!dslot)
 		goto error_info;
 
+	slot->name = dslot->name;
+	snprintf(slot->name, SLOT_NAME_SIZE, "fake%d", count++);
+	dbg("slot->name = %s\n", slot->name);
+
 	slot->ops = &dummy_hotplug_slot_ops;
 	slot->release = &dummy_release;
 	slot->private = dslot;
@@ -141,17 +146,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)
@@ -275,23 +280,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;
 
@@ -301,36 +292,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;
-	}
-	/* 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;
+
+		/* don't disable bridged devices just yet, we can't handle
+		 * them easily... */
+		if (dev->subordinate) {
+			err("Can't remove PCI devices with other PCI devices behind it yet.\n");
+			return -ENODEV;
 		}
-	}
 
-	/* remove the device from the pci core */
-	pci_remove_bus_device(dslot->dev);
 
-	/* blow away this sysfs entry and other parts. */
-	remove_slot(dslot);
+		/* remove the device from the pci core */
+		pci_remove_bus_device(dslot->dev);
 
+		/* blow away this sysfs entry and other parts. */
+		remove_slot(dslot);
+
+		pci_dev_put(dev);
+	}
 	return 0;
 }
 
Signature · 1 line
1.5.3.1.1.g1e61
#8 lore raw

PATCH v2 3/5 Introduce pci_slot

In reply to Alex Chiang

- Make pci_slot the primary sysfs entity. hotplug_slot becomes a subsidiary structure.
- 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.

---
 drivers/pci/Makefile                    |    2 +-
 drivers/pci/hotplug/acpiphp.h           |    1 -
 drivers/pci/hotplug/acpiphp_core.c      |   23 +---
 drivers/pci/hotplug/acpiphp_glue.c      |   16 --
 drivers/pci/hotplug/acpiphp_ibm.c       |    5 +-
 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  |  237 +++++++++++--------------------
 drivers/pci/hotplug/pciehp_core.c       |   22 +---
 drivers/pci/hotplug/rpadlpar_sysfs.c    |    4 +-
 drivers/pci/hotplug/sgi_hotplug.c       |    2 +-
 drivers/pci/hotplug/shpchp_core.c       |   17 +--
 drivers/pci/pci.h                       |   13 ++
 drivers/pci/slot.c                      |  157 ++++++++++++++++++++
 include/linux/pci.h                     |   14 ++
 include/linux/pci_hotplug.h             |   12 +--
 18 files changed, 293 insertions(+), 243 deletions(-)
 create mode 100644 drivers/pci/slot.c
drivers/pci/Makefile +1 -1
index 5550556..12f0b2d 100644
@@ -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
 
drivers/pci/hotplug/acpiphp.h +0 -1
index f6cc0c5..ab46189 100644
@@ -216,7 +216,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;
drivers/pci/hotplug/acpiphp_core.c +3 -20
index a0ca63a..34b8d0b 100644
@@ -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,
 };
 
 
@@ -279,23 +277,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;
@@ -362,7 +343,9 @@ 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) {
 		err("pci_hp_register failed with error %d\n", retval);
 		goto error_hpslot;
drivers/pci/hotplug/acpiphp_glue.c +0 -16
index 1e125b5..02a2fd7 100644
@@ -1874,19 +1874,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;
-}
drivers/pci/hotplug/acpiphp_ibm.c +3 -2
index 56829f8..927b1fb 100644
@@ -35,6 +35,7 @@
 #include <linux/moduleparam.h>
 
 #include "acpiphp.h"
+extern struct kset pci_slots_subsys;
 
 #define DRIVER_VERSION	"1.0.1"
 #define DRIVER_AUTHOR	"Irene Zubarev <[email protected]>, Vernon Mauery <[email protected]>"
@@ -428,7 +429,7 @@ static int __init ibm_acpiphp_init(void)
 	int retval = 0;
 	acpi_status status;
 	struct acpi_device *device;
-	struct kobject *sysdir = &pci_hotplug_slots_subsys.kobj;
+	struct kobject *sysdir = &pci_slots_subsys.kobj;
 
 	dbg("%s\n", __FUNCTION__);
 
@@ -475,7 +476,7 @@ init_return:
 static void __exit ibm_acpiphp_exit(void)
 {
 	acpi_status status;
-	struct kobject *sysdir = &pci_hotplug_slots_subsys.kobj;
+	struct kobject *sysdir = &pci_slots_subsys.kobj;
 
 	dbg("%s\n", __FUNCTION__);
 
drivers/pci/hotplug/cpci_hotplug_core.c +1 -1
index ed4d44e..aa47b80 100644
@@ -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;
drivers/pci/hotplug/cpqphp_core.c +3 -1
index a96b739..67f6a0c 100644
@@ -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;
drivers/pci/hotplug/fakephp.c +1 -1
index 851796b..2dd4100 100644
@@ -122,7 +122,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;
drivers/pci/hotplug/ibmphp_ebda.c +2 -1
index 600ed7b..eb7a1c0 100644
@@ -1000,7 +1000,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 ();
drivers/pci/hotplug/pci_hotplug_core.c +81 -156

527 lines not on this page · raw message

drivers/pci/hotplug/pciehp_core.c +5 -17
index 6462ac3..8fee402 100644
@@ -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,16 @@ 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->number);
 		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 +285,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 +387,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;
drivers/pci/hotplug/rpadlpar_sysfs.c +3 -1
index a080fed..3663507 100644
@@ -23,6 +23,8 @@
 
 #define MAX_DRC_NAME_LEN 64
 
+extern struct kset pci_slots_subsys;
+
 /* Store return code of dlpar operation in attribute struct */
 struct dlpar_io_attr {
 	int rc;
@@ -130,7 +132,7 @@ struct kobj_type ktype_dlpar_io = {
 
 struct kset dlpar_io_kset = {
 	.kobj = {.ktype = &ktype_dlpar_io,
-		 .parent = &pci_hotplug_slots_subsys.kobj},
+		 .parent = &pci_slots_subsys.kobj},
 	.ktype = &ktype_dlpar_io,
 };
 
drivers/pci/hotplug/sgi_hotplug.c +1 -1
index 693519e..cc74602 100644
@@ -625,7 +625,7 @@ 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;
 	}
drivers/pci/hotplug/shpchp_core.c +2 -15
index 80dec97..22c4d2e 100644
@@ -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);
drivers/pci/pci.h +13 -0
index fc87e14..4bcfabf 100644
@@ -91,3 +91,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_subsys;
+
+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)
+
drivers/pci/slot.c new file +157 -0

157 lines not on this page · raw message

include/linux/pci.h +14 -0
index 0dd93bb..a075667 100644
@@ -129,6 +129,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.
  */
@@ -140,6 +150,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;
@@ -261,6 +272,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 */
 
@@ -470,6 +482,8 @@ static inline struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *s
 }
 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, void (*release)(struct pci_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);
include/linux/pci_hotplug.h +3 -9
index ab4cb6e..bb36c59 100644
@@ -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_subsys;
 
 /* PCI Setting Record (Type 0) */
 struct hpp_type0 {
Signature · 1 line
1.5.3.1.1.g1e61
#9 lore raw

PATCH v2 4/5 Add pci_slot_add_hotplug()

In reply to Alex Chiang

Change the semantics of pci_create_slot() such that it does not require a hotplug release() method when creating a slot. Now, we can use this interface to create a pci_slot for any physical PCI slots, not just hotpluggable ones.

Add new pci_slot_add_hotplug() interface so that various PCI hotplug drivers can add hotplug release() methods to pre-existing physical slots.

---
 drivers/pci/hotplug/pci_hotplug_core.c |    2 +-
 drivers/pci/slot.c                     |   36 +++++++++++++++++++++++++++----
 include/linux/pci.h                    |    4 ++-
 3 files changed, 35 insertions(+), 7 deletions(-)
drivers/pci/hotplug/pci_hotplug_core.c +1 -1
index c667684..c4b3e61 100644
@@ -568,7 +568,7 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
 		return -EINVAL;
 	}
 
-	pci_slot = pci_create_slot(bus, slot_nr, slot->name, hotplug_release);
+	pci_slot = pci_slot_add_hotplug(bus, slot_nr, hotplug_release);
 	if (IS_ERR(pci_slot))
 		return PTR_ERR(pci_slot);
 	slot->pci_slot = pci_slot;
drivers/pci/slot.c +31 -5
index 5f0b69b..61e86b2 100644
@@ -71,20 +71,47 @@ static int create_sysfs_files(struct pci_slot *slot)
 	return result;
 }
 
+struct pci_slot *pci_slot_add_hotplug(struct pci_bus *parent, int slot_nr,
+				      void (*release)(struct pci_slot *))
+{
+	struct pci_slot *slot;
+	int 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) {
+		slot = ERR_PTR(-EEXIST);
+		goto out;
+	}
+
+	slot->release = release;
+ out:
+	up_write(&pci_bus_sem);
+	return slot;
+}
+EXPORT_SYMBOL_GPL(pci_slot_add_hotplug);
+
 struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
-			const char *name, void (*release)(struct pci_slot *))
+				 const char *name)
 {
 	struct pci_slot *slot;
 	int err;
 
 	down_write(&pci_bus_sem);
 
-	/* If we've already created this slot, return -EEXIST.  The same slot
-	 * may be described twice (eg, by ACPI and PCIe) */
+	/* If we've already created this slot, just return. */
 	for (slot = parent->slot; slot; slot = slot->next) {
 		if (slot->number != slot_nr)
 			continue;
-		slot = ERR_PTR(-EEXIST);
 		goto out;
 	}
 
@@ -96,7 +123,6 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
 
 	slot->bus = parent;
 	slot->number = slot_nr;
-	slot->release = release;
 
 	kobject_set_name(&slot->kobj, "%s", name);
 	kobj_set_kset_s(slot, pci_slots_subsys);
include/linux/pci.h +3 -1
index a075667..62116a5 100644
@@ -483,7 +483,9 @@ static inline struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *s
 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, void (*release)(struct pci_slot *));
+				 const char *name);
+struct pci_slot *pci_slot_add_hotplug(struct pci_bus *parent, int slot_nr,
+				    void (*release)(struct pci_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);
Signature · 1 line
1.5.3.1.1.g1e61
#10 lore raw

PATCH v2 5/5 ACPI PCI slot detection driver

In reply to 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.

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.

---
 drivers/acpi/Makefile   |    3 +-
 drivers/acpi/pci_slot.c |  203 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 205 insertions(+), 1 deletions(-)
 create mode 100644 drivers/acpi/pci_slot.c
drivers/acpi/Makefile +2 -1
index 54e3ab0..f6caec8 100644
@@ -47,7 +47,8 @@ obj-$(CONFIG_ACPI_FAN)		+= fan.o
 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-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o \
+				   pci_slot.o
 obj-$(CONFIG_ACPI_POWER)	+= power.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-$(CONFIG_ACPI_CONTAINER)	+= container.o
drivers/acpi/pci_slot.c new file +203 -0

203 lines not on this page · raw message

Signature · 1 line
1.5.3.1.1.g1e61
#11 lore raw

PATCH v2 5/5 ACPI PCI slot detection driver

In reply to 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.

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.

---
 drivers/acpi/Makefile   |    3 +-
 drivers/acpi/pci_slot.c |  203 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 205 insertions(+), 1 deletions(-)
 create mode 100644 drivers/acpi/pci_slot.c
drivers/acpi/Makefile +2 -1
index 54e3ab0..f6caec8 100644
@@ -47,7 +47,8 @@ obj-$(CONFIG_ACPI_FAN)		+= fan.o
 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-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o \
+				   pci_slot.o
 obj-$(CONFIG_ACPI_POWER)	+= power.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-$(CONFIG_ACPI_CONTAINER)	+= container.o
drivers/acpi/pci_slot.c new file +203 -0

203 lines not on this page · raw message

Signature · 1 line
1.5.3.1.1.g1e61
#12 lore raw
In reply to Alex Chiang
Quoted 5 lines · Alex Chiang

* Alex Chiang <[email protected]>:

Quoted 4 lines

Actually, I just reworked my patch this morning, and believe that I have a much cleaner implementation now that should fix a lot of the errors you saw.

Sorry, I lost access to the x3850 that I was using and may not be able to get back on it until early next week.

I think I will go ahead and send you the comments in response to your earlier message that I was going to hold off on until I tried your revised patches. It may contain something you haven't considered.

Gary

Signature · 6 lines
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
[email protected]
http://www.ibm.com/linux/ltc