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.


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.


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

cheers,

/ac

--

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 oth...
[ message continues ]

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

--

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);


--

Nice work, thanks. I've merged it.

--

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);


--

Thanks, merged.

--

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."
--

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);


--

Thanks, merged.

--

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."
--

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_f...
[ message continues ]

Merged, thanks.

--

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."
--

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;
 	}
 


--

Thanks, merged.

--

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, &pa...
[ message continues ]

Thanks, merged.

--

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."
--

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...
[ message continues ]

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


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);

+       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,

--

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);


--

Merged, thanks.

--

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;


--

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."
--

Hi willy,

 
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

--

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 @@ struc...
[ message continues ]

Merged, thanks.

--

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);
-		...
[ message continues ]

Tested and merged, thanks.

--

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);


--

Merged, thanks.

--

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


--

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


--

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
--

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

/ac

--

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;
 		}


--

Merged, thanks.

--

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);


--

Merged, thanks.

--

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"),
 		},
 	},
 	{}


--

Merged, thanks.

--

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;
+		last...
[ message continues ]

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_d...
[ message continues ]

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."
--

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

--

Hi Willy,


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


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

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

--

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




--

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_sl...
[ message continues ]