Hi Jesse, Ben, Kenji-san,
This is v14 of the physical PCI slot series.
This patchset has been kicking around -mm for the past few
months, and they were getting clobbered on a continual basis, so
let's say I'm quite motivated to get them into mainline. ;)
This mail describes two things:
- an update for handling pSeries
- explanation of how I did not regress Kenji-san's latest
changes
Review comments are much appreciated.
-----------------------
pSeries-related changes
-----------------------
The most recent outstanding issue with this series was breakage
with pSeries. In a nutshell, the problem was:
- pci_hp_register() interface changed to require a devfn
when registering a slot
- pSeries doesn't necessarily know the devfn of an
unpopulated slot
There are more details, of course, and they are in the archives.
I can dig them up if people want more context.
After working offline with BenH and Willy, we thought that the
best way forward was for the new infrastructure to provide a new
API, pci_update_slot_number(), which can be used by callers to
modify the slot number after slot creation.
This change goes hand in hand with modifying the semantics of
pci_hp_register() where callers are now allowed to pass -1 for
slot_nr to create a 'placeholder' slot.
The third related change is that the infrastructure will only
display an 'address' value of 'dddd:bb' (domain:bus) when the
device is -1. In the normal case, the full address of dddd:bb:dd
is displayed.
I did fold an earlier -mm fixup patch into these changes to
improve future bisectability.
I added kerneldoc to explain the APIs.
-----------------------------
maintaining Kenji-san's fixes
-----------------------------
Finally, this patch series slightly changes the logic introduced
by Kenji-san's patches:
9e4f2e8d4ddb04ad16a3828cd9a369a5a5287009
a86161b3134465f072d965ca7508ec9c1e2e52c7
For a86161b31344, the check against registering a slot with the
same name multiple times is removed. That check prevents a
scenario where multiple pcihp drivers try to claim the same slot.
The check is removed because the new code allows multiple callers
of pci_create_slot(). One callsite is pci_hp_register(), the
other is in the ACPI slot detection driver provided in patch 4/4.
In the case of multiple legitimate callers, the correct thing to
do is refcount the struct pci_slot's kobj.
In the case of two pcihp drivers attempting to claim the same
slot, pci_hp_register() returns -EBUSY to indicate it has already
been claimed. This logic has been part of the patch series from
the beginning.
Thus, the end behavior introduced by a86161b31344 is preserved,
although in a slightly different implementation.
The firmware defect that Kenji-san is trying to fix with
9e4f2e8d4d is the case where broken firmware will present the
kernel with slots like bb1:dd1, name "A" and bb2:dd2, name "A".
In other words, two different busses or two different devices on
the same bus, but both have the same name.
In this case, pci_create_slot() thinks they are two different
physical slots (which is true), and tries to register them with
the kobject core, which will then complain about registering two
objects with the same name. -EEXIST will be returned back up
through the pcihp core and back to pciehp, which will then printk
the message added by 9e4f2e8d4d.
Thus, the condition Kenji-san is trying to warn about with
9e4f2e8d4d is preserved.
Thanks,
/ac
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 <[email protected]>
Signed-off-by: Matthew Wilcox <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Kristen Carlson Accardi <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Kenji Kaneshige <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
drivers/pci/hotplug/fakephp.c | 84 ++++++++++++++---------------------------
1 files changed, 29 insertions(+), 55 deletions(-)
diff --git a/drivers/pci/hotplug/fakephp.c b/drivers/pci/hotplug/fakephp.c
index 7e9a827..b57223f 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,13 @@ 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 +150,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 +298,9 @@ static int enable_slot(struct hotplug_slot *hotplug_slot)
return 0;
}
-/* 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 +310,27 @@ static int disable_slot(struct hotplug_slot *slot)
dbg("%s - physical_slot = %s\n", __func__, 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.1.g1e61
From: Kenji Kaneshige <[email protected]>
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 <[email protected]>
Acked-by: Alex Chiang <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Kristen Carlson Accardi <[email protected]>
Cc: Len Brown <[email protected]>
Signed-off-by: Alex Chiang <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
lib/kobject.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/lib/kobject.c b/lib/kobject.c
index 718e510..dcade05 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -439,6 +439,7 @@ out:
return error;
}
+EXPORT_SYMBOL_GPL(kobject_rename);
/**
* kobject_move - move object to another parent
--
1.5.3.1.1.g1e61
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.
[folded in rpaphp-correctly-pci_hp_register-for-empty-pci-slots]
[[email protected]: build fix]
[[email protected]: make headers_check happy]
[[email protected]: nuther build fix]
[[email protected]: fix typo in #include]
Signed-off-by: Alex Chiang <[email protected]>
Signed-off-by: Matthew Wilcox <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Kristen Carlson Accardi <[email protected]>
Cc: Len Brown <[email protected]>
Acked-by: Kenji Kaneshige <[email protected]>
Tested-by: Badari Pulavarty <[email protected]>
Cc: Jesse Barnes <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
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 | 262 ++++++++++++-------------------
drivers/pci/hotplug/pciehp_core.c | 32 ++---
drivers/pci/hotplug/rpadlpar_sysfs.c | 5 +-
drivers/pci/hotplug/rpaphp_slot.c | 44 +-----
drivers/pci/hotplug/sgi_hotplug.c | 10 +-
drivers/pci/hotplug/shpchp_core.c | 20 +--
drivers/pci/pci.h | 13 ++
drivers/pci/probe.c | 1 +
drivers/pci/slot.c | 224 ++++++++++++++++++++++++++
include/linux/pci.h | 19 ++-
include/linux/pci_hotplug.h | 12 +-
20 files changed, 412 insertions(+), 298 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 7af68ba..0e496e8 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", __func__, 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 648596d..9342c84 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 ede9051..2b7c45e 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 <[email protected]>, Vernon Mauery <[email protected]>"
@@ -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", __func__);
@@ -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", __func__);
diff --git a/drivers/pci/hotplug/cpci_hotplug_core.c b/drivers/pci/hotplug/cpci_hotplug_core.c
index d8a6b80..9359479 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 36b115b..54defec 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 b57223f..40337a0 100644
--- a/drivers/pci/hotplug/fakephp.c
+++ b/drivers/pci/hotplug/fakephp.c
@@ -126,7 +126,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 dca7efc..8467d02 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 a11021e..47327d7 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"
@@ -60,41 +61,7 @@ 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,
-};
+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[] = {
@@ -149,16 +116,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 +132,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 +171,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 +203,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 +217,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 +237,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 +256,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 +284,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 +310,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 +341,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 +358,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 +369,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 +379,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 +389,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 +399,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 +409,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 +419,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 +450,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 +482,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 +501,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 +515,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);
@@ -599,12 +530,16 @@ static struct hotplug_slot *get_slot_from_name (const char *name)
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;
}
/**
@@ -616,10 +551,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 hotplug_slot *tmp;
+ struct pci_slot *pci_slot;
if (slot == NULL)
return -ENODEV;
@@ -631,24 +566,28 @@ int pci_hp_register (struct hotplug_slot *slot)
return -EINVAL;
}
- /* Check if we have already registered a slot with the same name. */
- tmp = get_slot_from_name(slot->name);
- if (tmp)
- return -EEXIST;
+ pci_slot = pci_create_slot(bus, slot_nr, slot->name);
+ if (IS_ERR(pci_slot))
+ return PTR_ERR(pci_slot);
- slot->kobj.kset = pci_hotplug_slots_kset;
- result = kobject_init_and_add(&slot->kobj, &hotplug_slot_ktype, NULL,
- "%s", slot->name);
- if (result) {
- err("Unable to register kobject '%s'", slot->name);
- return -EINVAL;
+ if (pci_slot->hotplug) {
+ dbg("%s: already claimed\n", __func__);
+ pci_destroy_slot(pci_slot);
+ return -EBUSY;
}
- list_add (&slot->slot_list, &pci_hotplug_slot_list);
+ slot->pci_slot = pci_slot;
+ pci_slot->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);
+ dbg("Added slot %s to the list\n", slot->name);
+
- result = fs_add_slot (slot);
- kobject_uevent(&slot->kobj, KOBJ_ADD);
- dbg ("Added slot %s to the list\n", slot->name);
return result;
}
@@ -661,22 +600,30 @@ 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);
+ 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);
+ dbg("Removed slot %s from the list\n", hotplug->name);
+
+ hotplug->release(hotplug);
+ slot->hotplug = NULL;
+ pci_destroy_slot(slot);
+
return 0;
}
@@ -690,13 +637,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;
}
@@ -704,36 +653,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);
@@ -745,7 +680,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 48a2ed3..38b8f37 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -72,7 +72,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);
@@ -85,7 +84,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,
};
@@ -252,7 +250,9 @@ 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) {
err("pci_hp_register failed with error %d\n", retval);
if (retval == -EEXIST)
@@ -263,7 +263,7 @@ static int init_slots(struct controller *ctrl)
}
/* create additional sysfs entries */
if (EMI(ctrl)) {
- 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);
@@ -296,7 +296,7 @@ static void cleanup_slots(struct controller *ctrl)
slot = list_entry(tmp, struct slot, slot_list);
list_del(&slot->slot_list);
if (EMI(ctrl))
- 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();
@@ -398,19 +398,8 @@ 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", __func__, 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)
+static int get_max_bus_speed(struct hotplug_slot *hotplug_slot,
+ enum pci_bus_speed *value)
{
struct slot *slot = hotplug_slot->private;
int retval;
@@ -471,7 +460,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 779c5db..a796301 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 "../pci.h"
#define DLPAR_KOBJ_NAME "control"
@@ -27,7 +29,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)
{
@@ -112,7 +113,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 56197b6..9b714ea 100644
--- a/drivers/pci/hotplug/rpaphp_slot.c
+++ b/drivers/pci/hotplug/rpaphp_slot.c
@@ -33,33 +33,6 @@
#include <asm/rtas.h>
#include "rpaphp.h"
-static ssize_t address_read_file (struct hotplug_slot *php_slot, char *buf)
-{
- int retval;
- struct slot *slot = (struct slot *)php_slot->private;
- struct pci_bus *bus;
-
- if (!slot)
- return -ENOENT;
-
- bus = slot->bus;
- if (!bus)
- return -ENOENT;
-
- if (bus->self)
- retval = sprintf(buf, pci_name(bus->self));
- else
- retval = sprintf(buf, "%04x:%02x:00.0",
- pci_domain_nr(bus), bus->number);
-
- return retval;
-}
-
-static struct hotplug_slot_attribute php_attr_address = {
- .attr = {.name = "address", .mode = S_IFREG | S_IRUGO},
- .show = address_read_file,
-};
-
/* free up the memory used by a slot */
static void rpaphp_release_slot(struct hotplug_slot *hotplug_slot)
{
@@ -135,9 +108,6 @@ int rpaphp_deregister_slot(struct slot *slot)
list_del(&slot->rpaphp_slot_list);
- /* remove "address" file */
- sysfs_remove_file(&php_slot->kobj, &php_attr_address.attr);
-
retval = pci_hp_deregister(php_slot);
if (retval)
err("Problem unregistering a slot %s\n", slot->name);
@@ -151,6 +121,7 @@ int rpaphp_register_slot(struct slot *slot)
{
struct hotplug_slot *php_slot = slot->hotplug_slot;
int retval;
+ int slotno;
dbg("%s registering slot:path[%s] index[%x], name[%s] pdomain[%x] type[%d]\n",
__func__, slot->dn->full_name, slot->index, slot->name,
@@ -162,19 +133,16 @@ int rpaphp_register_slot(struct slot *slot)
return -EAGAIN;
}
- retval = pci_hp_register(php_slot);
+ if (slot->dn->child)
+ slotno = PCI_SLOT(PCI_DN(slot->dn->child)->devfn);
+ else
+ slotno = -1;
+ retval = pci_hp_register(php_slot, slot->bus, slotno);
if (retval) {
err("pci_hp_register failed with error %d\n", retval);
return retval;
}
- /* create "address" file */
- retval = sysfs_create_file(&php_slot->kobj, &php_attr_address.attr);
- if (retval) {
- err("sysfs_create_file failed with error %d\n", retval);
- goto sysfs_fail;
- }
-
/* add slot to our internal list */
list_add(&slot->rpaphp_slot_list, &rpaphp_slot_head);
info("Slot [%s] registered\n", slot->name);
diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index 2fe37cd..2036a43 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 9784865..64afd70 100644
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -68,7 +68,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);
@@ -81,7 +80,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,
};
@@ -159,7 +157,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);
if (retval == -EEXIST)
@@ -288,19 +287,8 @@ 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", __func__, 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)
+static int get_max_bus_speed(struct hotplug_slot *hotplug_slot,
+ enum pci_bus_speed *value)
{
struct slot *slot = get_slot(hotplug_slot);
int retval;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 0a497c1..e1d7bbf 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -106,3 +106,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/probe.c b/drivers/pci/probe.c
index 3706ce7..b660d4a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -414,6 +414,7 @@ static struct pci_bus * pci_alloc_bus(void)
INIT_LIST_HEAD(&b->node);
INIT_LIST_HEAD(&b->children);
INIT_LIST_HEAD(&b->devices);
+ INIT_LIST_HEAD(&b->slots);
}
return b;
}
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
new file mode 100644
index 0000000..55aee0f
--- /dev/null
+++ b/drivers/pci/slot.c
@@ -0,0 +1,224 @@
+/*
+ * drivers/pci/slot.c
+ * Copyright (C) 2006 Matthew Wilcox <[email protected]>
+ * Copyright (C) 2006-2008 Hewlett-Packard Development Company, L.P.
+ * Alex Chiang <[email protected]>
+ */
+
+#include <linux/kobject.h>
+#include <linux/pci.h>
+#include <linux/err.h>
+#include "pci.h"
+
+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)
+{
+ if (slot->number == 0xff)
+ return sprintf(buf, "%04x:%02x\n",
+ pci_domain_nr(slot->bus),
+ slot->bus->number);
+ else
+ return sprintf(buf, "%04x:%02x:%02x\n",
+ pci_domain_nr(slot->bus),
+ slot->bus->number,
+ slot->number);
+}
+
+static void pci_slot_release(struct kobject *kobj)
+{
+ struct pci_slot *slot = to_pci_slot(kobj);
+
+ pr_debug("%s: releasing pci_slot on %x:%d\n", __func__,
+ slot->bus->number, slot->number);
+
+ list_del(&slot->list);
+
+ 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,
+};
+
+/**
+ * pci_create_slot - create or increment refcount for physical PCI slot
+ * @parent: struct pci_bus of parent bridge
+ * @slot_nr: PCI_SLOT(pci_dev->devfn) or -1 for placeholder
+ * @name: user visible string presented in /sys/bus/pci/slots/<name>
+ *
+ * PCI slots have first class attributes such as address, speed, width,
+ * and a &struct pci_slot is used to manage them. This interface will
+ * either return a new &struct pci_slot to the caller, or if the pci_slot
+ * already exists, its refcount will be incremented.
+ *
+ * Slots are uniquely identified by a @pci_bus, @slot_nr, @name tuple.
+ *
+ * Placeholder slots:
+ * In most cases, @pci_bus, @slot_nr will be sufficient to uniquely identify
+ * a slot. There is one notable exception - pSeries (rpaphp), where the
+ * @slot_nr cannot be determined until a device is actually inserted into
+ * the slot. In this scenario, the caller may pass -1 for @slot_nr.
+ *
+ * The following semantics are imposed when the caller passes @slot_nr ==
+ * -1. First, the check for existing %struct pci_slot is skipped, as the
+ * caller may know about several unpopulated slots on a given %struct
+ * pci_bus, and each slot would have a @slot_nr of -1. Uniqueness for
+ * these slots is then determined by the @name parameter. We expect
+ * kobject_init_and_add() to warn us if the caller attempts to create
+ * multiple slots with the same name. The other change in semantics is
+ * user-visible, which is the 'address' parameter presented in sysfs will
+ * consist solely of a dddd:bb tuple, where dddd is the PCI domain of the
+ * %struct pci_bus and bb is the bus number. In other words, the devfn of
+ * the 'placeholder' slot will not be displayed.
+ */
+
+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 (slot_nr == -1)
+ goto placeholder;
+
+ /* If we've already created this slot, bump refcount and return. */
+ list_for_each_entry(slot, &parent->slots, list) {
+ if (slot->number == slot_nr) {
+ kobject_get(&slot->kobj);
+ pr_debug("%s: inc refcount to %d on %04x:%02x:%02x\n",
+ __func__,
+ atomic_read(&slot->kobj.kref.refcount),
+ pci_domain_nr(parent), parent->number,
+ slot_nr);
+ goto out;
+ }
+ }
+
+placeholder:
+ 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);
+ goto err;
+ }
+
+ INIT_LIST_HEAD(&slot->list);
+ list_add(&slot->list, &parent->slots);
+
+ /* Don't care if debug printk has a -1 for slot_nr */
+ pr_debug("%s: created pci_slot on %04x:%02x:%02x\n",
+ __func__, pci_domain_nr(parent), parent->number, slot_nr);
+
+ out:
+ up_write(&pci_bus_sem);
+ return slot;
+ err:
+ kfree(slot);
+ slot = ERR_PTR(err);
+ goto out;
+}
+EXPORT_SYMBOL_GPL(pci_create_slot);
+
+/**
+ * pci_update_slot_number - update %struct pci_slot -> number
+ * @slot - %struct pci_slot to update
+ * @slot_nr - new number for slot
+ *
+ * The primary purpose of this interface is to allow callers who earlier
+ * created a placeholder slot in pci_create_slot() by passing a -1 as
+ * slot_nr, to update their %struct pci_slot with the correct @slot_nr.
+ */
+
+void pci_update_slot_number(struct pci_slot *slot, int slot_nr)
+{
+ down_write(&pci_bus_sem);
+
+ list_for_each_entry(slot, &slot->bus->slots, list)
+ WARN_ON(slot->number == slot_nr);
+
+ slot->number = slot_nr;
+ up_write(&pci_bus_sem);
+}
+EXPORT_SYMBOL_GPL(pci_update_slot_number);
+
+/**
+ * pci_destroy_slot - decrement refcount for physical PCI slot
+ * @slot: struct pci_slot to decrement
+ *
+ * %struct pci_slot is refcounted, so destroying them is really easy; we
+ * just call kobject_put on its kobj and let our release methods do the
+ * rest.
+ */
+
+void pci_destroy_slot(struct pci_slot *slot)
+{
+ pr_debug("%s: dec refcount to %d on %04x:%02x:%02x\n", __func__,
+ atomic_read(&slot->kobj.kref.refcount) - 1,
+ pci_domain_nr(slot->bus), slot->bus->number, slot->number);
+
+ down_write(&pci_bus_sem);
+ kobject_put(&slot->kobj);
+ up_write(&pci_bus_sem);
+}
+EXPORT_SYMBOL_GPL(pci_destroy_slot);
+
+static int pci_slot_init(void)
+{
+ 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) {
+ printk(KERN_ERR "PCI: Slot initialization failure\n");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+subsys_initcall(pci_slot_init);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 509159b..9a4f926 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -17,8 +17,7 @@
#ifndef LINUX_PCI_H
#define LINUX_PCI_H
-/* Include the pci register defines */
-#include <linux/pci_regs.h>
+#include <linux/pci_regs.h> /* The pci register defines */
/*
* The PCI interface treats multi-function devices as independent
@@ -49,12 +48,22 @@
#include <linux/list.h>
#include <linux/compiler.h>
#include <linux/errno.h>
+#include <linux/kobject.h>
#include <asm/atomic.h>
#include <linux/device.h>
/* Include the ID list */
#include <linux/pci_ids.h>
+/* pci_slot represents a physical slot */
+struct pci_slot {
+ struct pci_bus *bus; /* The bus this slot is on */
+ 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;
+};
+
/* File state for mmap()s on /proc/bus/pci/X/Y */
enum pci_mmap_state {
pci_mmap_io,
@@ -142,6 +151,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;
@@ -266,6 +276,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 list_head slots; /* list of slots on this bus */
struct resource *resource[PCI_BUS_NUM_RESOURCES];
/* address space routed to this bus */
@@ -488,6 +499,10 @@ 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 pci_destroy_slot(struct pci_slot *slot);
+void pci_update_slot_number(struct pci_slot *slot, int slot_nr);
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.1.g1e61
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.
[[email protected]: build fix]
[[email protected]: fix build with CONFIG_DMI=n]
Signed-off-by: Alex Chiang <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Kristen Carlson Accardi <[email protected]>
Cc: Len Brown <[email protected]>
Acked-by: Kenji Kaneshige <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
drivers/acpi/Kconfig | 9 +
drivers/acpi/Makefile | 1 +
drivers/acpi/pci_slot.c | 370 ++++++++++++++++++++++++++++++++
drivers/pci/hotplug/pci_hotplug_core.c | 16 ++
drivers/pci/hotplug/sgi_hotplug.c | 2 +-
5 files changed, 397 insertions(+), 1 deletions(-)
create mode 100644 drivers/acpi/pci_slot.c
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index c52fca8..250d41a 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -338,6 +338,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..4f9d26a
--- /dev/null
+++ b/drivers/acpi/pci_slot.c
@@ -0,0 +1,370 @@
+/*
+ * pci_slot.c - ACPI PCI Slot Driver
+ *
+ * The code here is heavily leveraged from the acpiphp module.
+ * Thanks to Matthew Wilcox <[email protected]> for much guidance.
+ * Thanks to Kenji Kaneshige <[email protected]> for code
+ * review and fixes.
+ *
+ * Copyright (C) 2007,2008 Hewlett-Packard Development Company, L.P.
+ * Alex Chiang <[email protected]>
+ *
+ * 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 <[email protected]>"
+#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)
+
+#define SLOT_NAME_SIZE 20 /* Inspired by #define in acpiphp.h */
+
+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,
+};
+
+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 */
+ status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
+ if (ACPI_SUCCESS(status) && !(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;
+}
+
+struct callback_args {
+ acpi_walk_callback user_function; /* only for walk_p2p_bridge */
+ struct pci_bus *pci_bus;
+ acpi_handle root_handle;
+};
+
+/*
+ * 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[SLOT_NAME_SIZE];
+ struct acpi_pci_slot *slot;
+ struct pci_slot *pci_slot;
+ 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", __func__);
+ 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));
+ 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: %p, pci_bus: %x, device: %d, name: %s\n",
+ pci_slot, pci_bus->number, device, name);
+
+ return AE_OK;
+}
+
+/*
+ * 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 callback_args child_context;
+ struct callback_args *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;
+
+ 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, &child_context, NULL);
+ if (ACPI_FAILURE(status))
+ goto out;
+
+ 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 callback_args 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;
+
+ 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, &context, NULL);
+ if (ACPI_FAILURE(status))
+ return status;
+
+ 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)
+{
+ struct acpi_pci_slot *slot, *tmp;
+
+ 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);
+}
+
+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 PRIMEQUEST",
+ .matches = {
+ DMI_MATCH(DMI_BIOS_VENDOR, "FUJITSU LIMITED"),
+ DMI_MATCH(DMI_BIOS_VERSION, "PRIMEQUEST"),
+ },
+ },
+ {}
+};
+
+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 47327d7..aea03ff 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -566,6 +566,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);
@@ -579,6 +584,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;
+ }
+ }
+
spin_lock(&pci_hotplug_slot_list_lock);
list_add(&slot->slot_list, &pci_hotplug_slot_list);
spin_unlock(&pci_hotplug_slot_list_lock);
diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index 2036a43..410fe03 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.1.g1e61
On Wed, Jun 04, 2008 at 02:16:06PM -0600, Alex Chiang wrote:
> From: Kenji Kaneshige <[email protected]>
>
> 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 <[email protected]>
> Acked-by: Alex Chiang <[email protected]>
> Cc: Greg KH <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Hi Alex-san,
I tried to test your patch using latest linux-next, but I could
not apply your patch with the following messages:
$ quilt push
Applying patch patches/fakephp-construct-one-fakephp-slot-per-pci-slot.patch
(Stripping trailing CRs from patch.)
patching file drivers/pci/hotplug/fakephp.c
Hunk #2 succeeded at 102 (offset 1 line).
Hunk #3 FAILED at 116.
Hunk #4 succeeded at 154 (offset 4 lines).
Hunk #5 succeeded at 299 (offset 1 line).
Hunk #6 succeeded at 314 (offset 4 lines).
patch unexpectedly ends in middle of line
1 out of 6 hunks FAILED -- rejects in file drivers/pci/hotplug/fakephp.c
patch unexpectedly ends in middle of line
Patch patches/fakephp-construct-one-fakephp-slot-per-pci-slot.patch does not apply (enforce with -f)
I think maybe your patch is conflicting with the following patch.
commit 372fa8fa0a77c47c7bcfae0ccd14e0c5f97d17fe
Author: Kay Sievers <[email protected]>
Date: Fri May 2 06:02:41 2008 +0200
driver-core: prepare for removal of 20 char limit from struct device
The access of struct device->bus_id is replaced by dev_name(), to
be able to use the non-limited kobject name after the conversion
of the device register callers.
Thanks,
Kenji Kaneshige
Alex Chiang wrote:
> Hi Jesse, Ben, Kenji-san,
>
> This is v14 of the physical PCI slot series.
>
> This patchset has been kicking around -mm for the past few
> months, and they were getting clobbered on a continual basis, so
> let's say I'm quite motivated to get them into mainline. ;)
>
> This mail describes two things:
>
> - an update for handling pSeries
> - explanation of how I did not regress Kenji-san's latest
> changes
>
> Review comments are much appreciated.
>
> -----------------------
> pSeries-related changes
> -----------------------
> The most recent outstanding issue with this series was breakage
> with pSeries. In a nutshell, the problem was:
>
> - pci_hp_register() interface changed to require a devfn
> when registering a slot
>
> - pSeries doesn't necessarily know the devfn of an
> unpopulated slot
>
> There are more details, of course, and they are in the archives.
> I can dig them up if people want more context.
>
> After working offline with BenH and Willy, we thought that the
> best way forward was for the new infrastructure to provide a new
> API, pci_update_slot_number(), which can be used by callers to
> modify the slot number after slot creation.
>
> This change goes hand in hand with modifying the semantics of
> pci_hp_register() where callers are now allowed to pass -1 for
> slot_nr to create a 'placeholder' slot.
>
> The third related change is that the infrastructure will only
> display an 'address' value of 'dddd:bb' (domain:bus) when the
> device is -1. In the normal case, the full address of dddd:bb:dd
> is displayed.
>
> I did fold an earlier -mm fixup patch into these changes to
> improve future bisectability.
>
> I added kerneldoc to explain the APIs.
>
> -----------------------------
> maintaining Kenji-san's fixes
> -----------------------------
> Finally, this patch series slightly changes the logic introduced
> by Kenji-san's patches:
>
> 9e4f2e8d4ddb04ad16a3828cd9a369a5a5287009
> a86161b3134465f072d965ca7508ec9c1e2e52c7
>
> For a86161b31344, the check against registering a slot with the
> same name multiple times is removed. That check prevents a
> scenario where multiple pcihp drivers try to claim the same slot.
>
> The check is removed because the new code allows multiple callers
> of pci_create_slot(). One callsite is pci_hp_register(), the
> other is in the ACPI slot detection driver provided in patch 4/4.
> In the case of multiple legitimate callers, the correct thing to
> do is refcount the struct pci_slot's kobj.
>
> In the case of two pcihp drivers attempting to claim the same
> slot, pci_hp_register() returns -EBUSY to indicate it has already
> been claimed. This logic has been part of the patch series from
> the beginning.
>
> Thus, the end behavior introduced by a86161b31344 is preserved,
> although in a slightly different implementation.
>
> The firmware defect that Kenji-san is trying to fix with
> 9e4f2e8d4d is the case where broken firmware will present the
> kernel with slots like bb1:dd1, name "A" and bb2:dd2, name "A".
>
> In other words, two different busses or two different devices on
> the same bus, but both have the same name.
>
> In this case, pci_create_slot() thinks they are two different
> physical slots (which is true), and tries to register them with
> the kobject core, which will then complain about registering two
> objects with the same name. -EEXIST will be returned back up
> through the pcihp core and back to pciehp, which will then printk
> the message added by 9e4f2e8d4d.
>
> Thus, the condition Kenji-san is trying to warn about with
> 9e4f2e8d4d is preserved.
>
> Thanks,
>
> /ac
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
Hi Kenji-san,
* Kenji Kaneshige <[email protected]>:
>
> I tried to test your patch using latest linux-next, but I could
> not apply your patch with the following messages:
Sorry, that latest patch series was against Linus' tree, not
linux-next.
> I think maybe your patch is conflicting with the following patch.
>
> commit 372fa8fa0a77c47c7bcfae0ccd14e0c5f97d17fe
> Author: Kay Sievers <[email protected]>
> Date: Fri May 2 06:02:41 2008 +0200
>
> driver-core: prepare for removal of 20 char limit from struct device
>
> The access of struct device->bus_id is replaced by dev_name(), to
> be able to use the non-limited kobject name after the conversion
> of the device register callers.
Yes, thanks for pointing this out. I've fixed patch 1/4.
I verified that patches 2, 3 and 4 apply cleanly against
linux-next, so this should be the only fix you'll need.
Sorry for the confusion.
/ac
commit c0175afea14cdaff3c33d894eb474eed1dcfa65d
Author: Alex Chiang <[email protected]>
Date: Wed Jun 4 21:04:46 2008 -0600
Construct one fakephp slot per PCI slot
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 <[email protected]>
Signed-off-by: Matthew Wilcox <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Kristen Carlson Accardi <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Kenji Kaneshige <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
diff --git a/drivers/pci/hotplug/fakephp.c b/drivers/pci/hotplug/fakephp.c
index d3393f4..e066959 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;
@@ -101,6 +102,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)
@@ -114,15 +116,13 @@ 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 = kstrdup(dev_name(&dev->dev), GFP_KERNEL);
- if (!slot->name)
- goto error_info;
- dbg("slot->name = %s\n", slot->name);
-
dslot = kzalloc(sizeof(struct dummy_slot), GFP_KERNEL);
if (!dslot)
- goto error_name;
+ 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;
@@ -140,8 +140,6 @@ static int add_slot(struct pci_dev *dev)
error_dslot:
kfree(dslot);
-error_name:
- kfree(slot->name);
error_info:
kfree(slot->info);
error_slot:
@@ -153,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)
@@ -301,23 +299,9 @@ static int enable_slot(struct hotplug_slot *hotplug_slot)
return 0;
}
-/* 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;
@@ -327,41 +311,27 @@ static int disable_slot(struct hotplug_slot *slot)
dbg("%s - physical_slot = %s\n", __func__, 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;
}
Alex-san,
Thank you for updated patch. I'll try it.
Unfortunately, I'm a little busy on the other jobs, so the test
report might be delayed until the beginning of next week.
Thanks,
Kenji Kaneshige
Alex Chiang wrote:
> Hi Kenji-san,
>
> * Kenji Kaneshige <[email protected]>:
>> I tried to test your patch using latest linux-next, but I could
>> not apply your patch with the following messages:
>
> Sorry, that latest patch series was against Linus' tree, not
> linux-next.
>
>> I think maybe your patch is conflicting with the following patch.
>>
>> commit 372fa8fa0a77c47c7bcfae0ccd14e0c5f97d17fe
>> Author: Kay Sievers <[email protected]>
>> Date: Fri May 2 06:02:41 2008 +0200
>>
>> driver-core: prepare for removal of 20 char limit from struct device
>>
>> The access of struct device->bus_id is replaced by dev_name(), to
>> be able to use the non-limited kobject name after the conversion
>> of the device register callers.
>
> Yes, thanks for pointing this out. I've fixed patch 1/4.
>
> I verified that patches 2, 3 and 4 apply cleanly against
> linux-next, so this should be the only fix you'll need.
>
> Sorry for the confusion.
>
> /ac
>
> commit c0175afea14cdaff3c33d894eb474eed1dcfa65d
> Author: Alex Chiang <[email protected]>
> Date: Wed Jun 4 21:04:46 2008 -0600
>
> Construct one fakephp slot per PCI slot
>
> 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 <[email protected]>
> Signed-off-by: Matthew Wilcox <[email protected]>
> Cc: Greg KH <[email protected]>
> Cc: Kristen Carlson Accardi <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: Kenji Kaneshige <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
>
> diff --git a/drivers/pci/hotplug/fakephp.c b/drivers/pci/hotplug/fakephp.c
> index d3393f4..e066959 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;
> @@ -101,6 +102,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)
> @@ -114,15 +116,13 @@ 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 = kstrdup(dev_name(&dev->dev), GFP_KERNEL);
> - if (!slot->name)
> - goto error_info;
> - dbg("slot->name = %s\n", slot->name);
> -
> dslot = kzalloc(sizeof(struct dummy_slot), GFP_KERNEL);
> if (!dslot)
> - goto error_name;
> + 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;
> @@ -140,8 +140,6 @@ static int add_slot(struct pci_dev *dev)
>
> error_dslot:
> kfree(dslot);
> -error_name:
> - kfree(slot->name);
> error_info:
> kfree(slot->info);
> error_slot:
> @@ -153,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)
> @@ -301,23 +299,9 @@ static int enable_slot(struct hotplug_slot *hotplug_slot)
> return 0;
> }
>
> -/* 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;
>
> @@ -327,41 +311,27 @@ static int disable_slot(struct hotplug_slot *slot)
>
> dbg("%s - physical_slot = %s\n", __func__, 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;
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
On Wed, 4 Jun 2008 14:16:06 -0600 Alex Chiang <[email protected]> wrote:
> From: Kenji Kaneshige <[email protected]>
>
> 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 <[email protected]>
> Acked-by: Alex Chiang <[email protected]>
> Cc: Greg KH <[email protected]>
> Cc: Kristen Carlson Accardi <[email protected]>
> Cc: Len Brown <[email protected]>
> Signed-off-by: Alex Chiang <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
> lib/kobject.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/lib/kobject.c b/lib/kobject.c
> index 718e510..dcade05 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -439,6 +439,7 @@ out:
>
> return error;
> }
> +EXPORT_SYMBOL_GPL(kobject_rename);
Shouldn't this go straight into 2.6.26?
Andrew Morton wrote:
> On Wed, 4 Jun 2008 14:16:06 -0600 Alex Chiang <[email protected]> wrote:
>
>> From: Kenji Kaneshige <[email protected]>
>>
>> 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 <[email protected]>
>> Acked-by: Alex Chiang <[email protected]>
>> Cc: Greg KH <[email protected]>
>> Cc: Kristen Carlson Accardi <[email protected]>
>> Cc: Len Brown <[email protected]>
>> Signed-off-by: Alex Chiang <[email protected]>
>> Signed-off-by: Andrew Morton <[email protected]>
>> ---
>> lib/kobject.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/lib/kobject.c b/lib/kobject.c
>> index 718e510..dcade05 100644
>> --- a/lib/kobject.c
>> +++ b/lib/kobject.c
>> @@ -439,6 +439,7 @@ out:
>>
>> return error;
>> }
>> +EXPORT_SYMBOL_GPL(kobject_rename);
>
> Shouldn't this go straight into 2.6.26?
>
>
I think "No", because currently its only required by Alex's "pci slot"
patches.
Thanks,
Kenji Kaneshige
On Thu, 05 Jun 2008 15:05:59 +0900 Kenji Kaneshige <[email protected]> wrote:
> Andrew Morton wrote:
> > On Wed, 4 Jun 2008 14:16:06 -0600 Alex Chiang <[email protected]> wrote:
> >
> >> From: Kenji Kaneshige <[email protected]>
> >>
> >> 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 <[email protected]>
> >> Acked-by: Alex Chiang <[email protected]>
> >> Cc: Greg KH <[email protected]>
> >> Cc: Kristen Carlson Accardi <[email protected]>
> >> Cc: Len Brown <[email protected]>
> >> Signed-off-by: Alex Chiang <[email protected]>
> >> Signed-off-by: Andrew Morton <[email protected]>
> >> ---
> >> lib/kobject.c | 1 +
> >> 1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/lib/kobject.c b/lib/kobject.c
> >> index 718e510..dcade05 100644
> >> --- a/lib/kobject.c
> >> +++ b/lib/kobject.c
> >> @@ -439,6 +439,7 @@ out:
> >>
> >> return error;
> >> }
> >> +EXPORT_SYMBOL_GPL(kobject_rename);
> >
> > Shouldn't this go straight into 2.6.26?
> >
> >
>
> I think "No", because currently its only required by Alex's "pci slot"
> patches.
In that case the fix should be integrated into the patch which it is
fixing.
Well, it doesn't _have_ to be integrated because in this case it can
precede acpi-pci-slot-detection-driver.patch. But we might as well
fold it, to keep the commit noise level down.
* Andrew Morton <[email protected]>:
> On Thu, 05 Jun 2008 15:05:59 +0900 Kenji Kaneshige <[email protected]> wrote:
>
> > Andrew Morton wrote:
> > > On Wed, 4 Jun 2008 14:16:06 -0600 Alex Chiang <[email protected]> wrote:
> > >
> > >> From: Kenji Kaneshige <[email protected]>
> > >>
> > >> 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 <[email protected]>
> > >> Acked-by: Alex Chiang <[email protected]>
> > >> Cc: Greg KH <[email protected]>
> > >> Cc: Kristen Carlson Accardi <[email protected]>
> > >> Cc: Len Brown <[email protected]>
> > >> Signed-off-by: Alex Chiang <[email protected]>
> > >> Signed-off-by: Andrew Morton <[email protected]>
> > >> ---
> > >> lib/kobject.c | 1 +
> > >> 1 files changed, 1 insertions(+), 0 deletions(-)
> > >>
> > >> diff --git a/lib/kobject.c b/lib/kobject.c
> > >> index 718e510..dcade05 100644
> > >> --- a/lib/kobject.c
> > >> +++ b/lib/kobject.c
> > >> @@ -439,6 +439,7 @@ out:
> > >>
> > >> return error;
> > >> }
> > >> +EXPORT_SYMBOL_GPL(kobject_rename);
> > >
> > > Shouldn't this go straight into 2.6.26?
Originally, gregkh told me "no" because then he would have to
deal with a bunch of patching removing an unused interface, which
is why it showed up as a separate patch in my series.
> > I think "No", because currently its only required by Alex's "pci slot"
> > patches.
>
> In that case the fix should be integrated into the patch which it is
> fixing.
>
> Well, it doesn't _have_ to be integrated because in this case it can
> precede acpi-pci-slot-detection-driver.patch. But we might as well
> fold it, to keep the commit noise level down.
Ok, that makes sense to me.
/ac
BTW. In the meantime, your current patch set seems fine to go.
I'll add code to actually do the renumbering of the slot separately,
along with some other cleanup & rework of the pseries PCI hotplug
I started doing.
Ben.
* Benjamin Herrenschmidt <[email protected]>:
> BTW. In the meantime, your current patch set seems fine to go.
That sounds suspiciously like an Acked-by: to me... ;)
> I'll add code to actually do the renumbering of the slot
> separately, along with some other cleanup & rework of the
> pseries PCI hotplug I started doing.
Great, thanks!
/ac
On Thu, 2008-06-05 at 22:51 -0600, Alex Chiang wrote:
> * Benjamin Herrenschmidt <[email protected]>:
> > BTW. In the meantime, your current patch set seems fine to go.
>
> That sounds suspiciously like an Acked-by: to me... ;)
Yup, the only thing is .. I would like to do a few more tests to make
sure we didn't break an obscure setup but I suppose we can always
do fixup patches if necessary. So yes,
Acked-by: Benjamin Herrenschmidt <[email protected]>
> > I'll add code to actually do the renumbering of the slot
> > separately, along with some other cleanup & rework of the
> > pseries PCI hotplug I started doing.
>
> Great, thanks!
>
Alex Chiang wrote:
> Hi Jesse, Ben, Kenji-san,
>
> This is v14 of the physical PCI slot series.
>
> This patchset has been kicking around -mm for the past few
> months, and they were getting clobbered on a continual basis, so
> let's say I'm quite motivated to get them into mainline. ;)
>
> This mail describes two things:
>
> - an update for handling pSeries
> - explanation of how I did not regress Kenji-san's latest
> changes
>
> Review comments are much appreciated.
>
> -----------------------
> pSeries-related changes
> -----------------------
> The most recent outstanding issue with this series was breakage
> with pSeries. In a nutshell, the problem was:
>
> - pci_hp_register() interface changed to require a devfn
> when registering a slot
>
> - pSeries doesn't necessarily know the devfn of an
> unpopulated slot
>
> There are more details, of course, and they are in the archives.
> I can dig them up if people want more context.
>
> After working offline with BenH and Willy, we thought that the
> best way forward was for the new infrastructure to provide a new
> API, pci_update_slot_number(), which can be used by callers to
> modify the slot number after slot creation.
>
> This change goes hand in hand with modifying the semantics of
> pci_hp_register() where callers are now allowed to pass -1 for
> slot_nr to create a 'placeholder' slot.
>
> The third related change is that the infrastructure will only
> display an 'address' value of 'dddd:bb' (domain:bus) when the
> device is -1. In the normal case, the full address of dddd:bb:dd
> is displayed.
>
> I did fold an earlier -mm fixup patch into these changes to
> improve future bisectability.
>
> I added kerneldoc to explain the APIs.
>
> -----------------------------
> maintaining Kenji-san's fixes
> -----------------------------
> Finally, this patch series slightly changes the logic introduced
> by Kenji-san's patches:
>
> 9e4f2e8d4ddb04ad16a3828cd9a369a5a5287009
> a86161b3134465f072d965ca7508ec9c1e2e52c7
>
> For a86161b31344, the check against registering a slot with the
> same name multiple times is removed. That check prevents a
> scenario where multiple pcihp drivers try to claim the same slot.
>
> The check is removed because the new code allows multiple callers
> of pci_create_slot(). One callsite is pci_hp_register(), the
> other is in the ACPI slot detection driver provided in patch 4/4.
> In the case of multiple legitimate callers, the correct thing to
> do is refcount the struct pci_slot's kobj.
>
> In the case of two pcihp drivers attempting to claim the same
> slot, pci_hp_register() returns -EBUSY to indicate it has already
> been claimed. This logic has been part of the patch series from
> the beginning.
>
> Thus, the end behavior introduced by a86161b31344 is preserved,
> although in a slightly different implementation.
>
> The firmware defect that Kenji-san is trying to fix with
> 9e4f2e8d4d is the case where broken firmware will present the
> kernel with slots like bb1:dd1, name "A" and bb2:dd2, name "A".
>
> In other words, two different busses or two different devices on
> the same bus, but both have the same name.
>
> In this case, pci_create_slot() thinks they are two different
> physical slots (which is true), and tries to register them with
> the kobject core, which will then complain about registering two
> objects with the same name. -EEXIST will be returned back up
> through the pcihp core and back to pciehp, which will then printk
> the message added by 9e4f2e8d4d.
>
> Thus, the condition Kenji-san is trying to warn about with
> 9e4f2e8d4d is preserved.
>
I tried your patches and I have a comment (question) about the behavior.
To emulate the (broken?) platform that assigns the same name to multiple
slots, I made a debug patch for pciehp driver to assign same name ('1000')
to all slots (my environment has two PCIe slots). With this patch, I
noticed that the behavior or pci_hp_register() (or pci_create_slot())
varies depending on whether pci_slot driver is loaded or not. See below.
(a) With pci_slot driver loaded
I got the following error message when I loaded pciehp driver.
pciehp: pci_hp_register failed with error -17
pciehp: Failed to register slot because of name collision. Try
'pciehp_slot_with_bus' module option.
pciehp: pciehp: slot initialization failed
(b) Without pci_slot driver loaded
I got the kernel stack dump and error messages when I loaded pciehp
driver.
kobject_add_internal failed for 1000 with -EEXIST, don't try to
register things with the same name in the same directory.
Call Trace:
[<a000000100015180>] show_stack+0x40/0xa0
sp=e0000040a086fb80 bsp=e0000040a0861158
[<a000000100015210>] dump_stack+0x30/0x60
sp=e0000040a086fd50 bsp=e0000040a0861140
[<a0000001003b3910>] kobject_add_internal+0x330/0x400
sp=e0000040a086fd50 bsp=e0000040a0861100
[<a0000001003b3bd0>] kobject_add_varg+0x90/0xc0
sp=e0000040a086fd50 bsp=e0000040a08610c8
[<a0000001003b3c90>] kobject_init_and_add+0x90/0xc0
sp=e0000040a086fd50 bsp=e0000040a0861068
[<a0000001003d69b0>] pci_create_slot+0x150/0x260
sp=e0000040a086fd80 bsp=e0000040a0861030
[<a000000200b71870>] pci_hp_register+0x130/0x880 [pci_hotplug]
sp=e0000040a086fd80 bsp=e0000040a0860ff0
[<a000000200ec1a60>] pciehp_probe+0x720/0xca0 [pciehp]
(snip...)
Unable to register kobject 1000
pciehp: pci_hp_register failed with error -17
pciehp: Failed to register slot because of name collision. Try
'pciehp_slot_with_bus' module option.
pciehp: pciehp: slot initialization failed
Could you tell me why that difference happen? And my expectation is
the result should be (a) above regardless of whether pci_slot driver
is loaded or not.
Thanks,
Kenji Kaneshige
* Kenji Kaneshige <[email protected]>:
>
> I tried your patches and I have a comment (question) about the behavior.
>
> To emulate the (broken?) platform that assigns the same name to multiple
> slots, I made a debug patch for pciehp driver to assign same name ('1000')
> to all slots (my environment has two PCIe slots). With this patch, I
> noticed that the behavior or pci_hp_register() (or pci_create_slot())
> varies depending on whether pci_slot driver is loaded or not. See below.
>
> (a) With pci_slot driver loaded
> I got the following error message when I loaded pciehp driver.
>
> pciehp: pci_hp_register failed with error -17
> pciehp: Failed to register slot because of name collision. Try
> 'pciehp_slot_with_bus' module option.
> pciehp: pciehp: slot initialization failed
> (b) Without pci_slot driver loaded
> I got the kernel stack dump and error messages when I loaded pciehp
> driver.
>
> kobject_add_internal failed for 1000 with -EEXIST, don't try to
> register things with the same name in the same directory.
>
> Call Trace:
> [<a000000100015180>] show_stack+0x40/0xa0
> sp=e0000040a086fb80 bsp=e0000040a0861158
> [<a000000100015210>] dump_stack+0x30/0x60
> sp=e0000040a086fd50 bsp=e0000040a0861140
> [<a0000001003b3910>] kobject_add_internal+0x330/0x400
> sp=e0000040a086fd50 bsp=e0000040a0861100
> [<a0000001003b3bd0>] kobject_add_varg+0x90/0xc0
> sp=e0000040a086fd50 bsp=e0000040a08610c8
> [<a0000001003b3c90>] kobject_init_and_add+0x90/0xc0
> sp=e0000040a086fd50 bsp=e0000040a0861068
> [<a0000001003d69b0>] pci_create_slot+0x150/0x260
> sp=e0000040a086fd80 bsp=e0000040a0861030
> [<a000000200b71870>] pci_hp_register+0x130/0x880 [pci_hotplug]
> sp=e0000040a086fd80 bsp=e0000040a0860ff0
> [<a000000200ec1a60>] pciehp_probe+0x720/0xca0 [pciehp]
>
> (snip...)
>
> Unable to register kobject 1000
> pciehp: pci_hp_register failed with error -17
> pciehp: Failed to register slot because of name collision. Try
> 'pciehp_slot_with_bus' module option.
> pciehp: pciehp: slot initialization failed
>
> Could you tell me why that difference happen? And my expectation is
> the result should be (a) above regardless of whether pci_slot driver
> is loaded or not.
The difference was because in (a), pci_slot created the slots and
when pciehp tried to register them, the pci slot infrastructure
simply refcounted them and returned, but did not try to
re-register the kobjects with the kobj core.
In (b), the pci hotplug core allowed you to create multiple slots
with the same name, and called pci_create_slot() multiple times.
This time, since you were trying to create new slot objects, we
did not refcount them; we actually did a kzalloc *and* tried to
register them with the kobject core, which is why we got that
stack trace.
I read your patch (a86161b3134465f) in closer detail and decided
that it can work without problems with my patch series.
- Your patch will keep track of hotplug slots and
disallow multiple hotplug slots with the same name
- the PCI slot infrastructure will still allow multiple
callers of pci_create_slot() to succeed by refcounting
identical slots
This is the correct behavior to allow pciehp and pci_slot to
coexist because pci_slot is not trying to register a hotplug
handler or do any other hotplug operations.
I will update the patch series and resend it.
Thanks for testing, sorry about the inconvenience.
/ac
Alex Chiang wrote:
> * Kenji Kaneshige <[email protected]>:
>> I tried your patches and I have a comment (question) about the behavior.
>>
>> To emulate the (broken?) platform that assigns the same name to multiple
>> slots, I made a debug patch for pciehp driver to assign same name ('1000')
>> to all slots (my environment has two PCIe slots). With this patch, I
>> noticed that the behavior or pci_hp_register() (or pci_create_slot())
>> varies depending on whether pci_slot driver is loaded or not. See below.
>>
>> (a) With pci_slot driver loaded
>> I got the following error message when I loaded pciehp driver.
>>
>> pciehp: pci_hp_register failed with error -17
>> pciehp: Failed to register slot because of name collision. Try
>> 'pciehp_slot_with_bus' module option.
>> pciehp: pciehp: slot initialization failed
>> (b) Without pci_slot driver loaded
>> I got the kernel stack dump and error messages when I loaded pciehp
>> driver.
>>
>> kobject_add_internal failed for 1000 with -EEXIST, don't try to
>> register things with the same name in the same directory.
>>
>> Call Trace:
>> [<a000000100015180>] show_stack+0x40/0xa0
>> sp=e0000040a086fb80 bsp=e0000040a0861158
>> [<a000000100015210>] dump_stack+0x30/0x60
>> sp=e0000040a086fd50 bsp=e0000040a0861140
>> [<a0000001003b3910>] kobject_add_internal+0x330/0x400
>> sp=e0000040a086fd50 bsp=e0000040a0861100
>> [<a0000001003b3bd0>] kobject_add_varg+0x90/0xc0
>> sp=e0000040a086fd50 bsp=e0000040a08610c8
>> [<a0000001003b3c90>] kobject_init_and_add+0x90/0xc0
>> sp=e0000040a086fd50 bsp=e0000040a0861068
>> [<a0000001003d69b0>] pci_create_slot+0x150/0x260
>> sp=e0000040a086fd80 bsp=e0000040a0861030
>> [<a000000200b71870>] pci_hp_register+0x130/0x880 [pci_hotplug]
>> sp=e0000040a086fd80 bsp=e0000040a0860ff0
>> [<a000000200ec1a60>] pciehp_probe+0x720/0xca0 [pciehp]
>>
>> (snip...)
>>
>> Unable to register kobject 1000
>> pciehp: pci_hp_register failed with error -17
>> pciehp: Failed to register slot because of name collision. Try
>> 'pciehp_slot_with_bus' module option.
>> pciehp: pciehp: slot initialization failed
>>
>> Could you tell me why that difference happen? And my expectation is
>> the result should be (a) above regardless of whether pci_slot driver
>> is loaded or not.
>
> The difference was because in (a), pci_slot created the slots and
> when pciehp tried to register them, the pci slot infrastructure
> simply refcounted them and returned, but did not try to
> re-register the kobjects with the kobj core.
>
> In (b), the pci hotplug core allowed you to create multiple slots
> with the same name, and called pci_create_slot() multiple times.
> This time, since you were trying to create new slot objects, we
> did not refcount them; we actually did a kzalloc *and* tried to
> register them with the kobject core, which is why we got that
> stack trace.
>
> I read your patch (a86161b3134465f) in closer detail and decided
> that it can work without problems with my patch series.
>
> - Your patch will keep track of hotplug slots and
> disallow multiple hotplug slots with the same name
>
> - the PCI slot infrastructure will still allow multiple
> callers of pci_create_slot() to succeed by refcounting
> identical slots
>
> This is the correct behavior to allow pciehp and pci_slot to
> coexist because pci_slot is not trying to register a hotplug
> handler or do any other hotplug operations.
>
Thank you for explanation. I understood.
But I have one concern about the behavior when pci slot driver is not
loaded. My patch (a86161b3134465f) prevents the kernel trace dump
because of duplicate kobject add that would happen in the following two
cases, though (b) is not described in the header of the patch (sorry).
(a) multiple driver attempt to handle the same slot.
(b) one or more driver attempt to register multiple slots with the
same name (This can happen if broken platform assigns the same
slot number to multiple hotplug slots, for example).
With your patch, duplicate kobject add in case (b) is not prevented.
That is my concern.
I made a below patch to prevent (b), please take a look. And could you
please consider merging it to "[PATCH 2/3] Introduce pci_slot" in your
latest series.
Thanks,
Kenji Kaneshige
---
drivers/pci/hotplug/pci_hotplug_core.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
Index: 20080610/drivers/pci/hotplug/pci_hotplug_core.c
===================================================================
--- 20080610.orig/drivers/pci/hotplug/pci_hotplug_core.c
+++ 20080610/drivers/pci/hotplug/pci_hotplug_core.c
@@ -555,6 +555,7 @@ int pci_hp_register(struct hotplug_slot
{
int result;
struct pci_slot *pci_slot;
+ struct hotplug_slot *tmp;
if (slot == NULL)
return -ENODEV;
@@ -567,6 +568,21 @@ int pci_hp_register(struct hotplug_slot
}
/*
+ * Prevent registering multiple hotplug slots with the same name.
+ */
+ spin_lock(&pci_hotplug_slot_list_lock);
+ list_for_each_entry(tmp, &pci_hotplug_slot_list, slot_list) {
+ pci_slot = tmp->pci_slot;
+ if (pci_slot->bus == bus && pci_slot->number == slot_nr)
+ continue;
+ if (!strcmp(tmp->name, slot->name)) {
+ spin_unlock(&pci_hotplug_slot_list_lock);
+ return -EEXIST;
+ }
+ }
+ spin_unlock(&pci_hotplug_slot_list_lock);
+
+ /*
* 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.
Kenji Kaneshige wrote:
> Alex Chiang wrote:
>> * Kenji Kaneshige <[email protected]>:
(snip.)
> Thank you for explanation. I understood.
>
> But I have one concern about the behavior when pci slot driver is not
> loaded. My patch (a86161b3134465f) prevents the kernel trace dump
> because of duplicate kobject add that would happen in the following two
> cases, though (b) is not described in the header of the patch (sorry).
>
> (a) multiple driver attempt to handle the same slot.
>
> (b) one or more driver attempt to register multiple slots with the
> same name (This can happen if broken platform assigns the same
> slot number to multiple hotplug slots, for example).
>
> With your patch, duplicate kobject add in case (b) is not prevented.
> That is my concern.
>
> I made a below patch to prevent (b), please take a look. And could you
> please consider merging it to "[PATCH 2/3] Introduce pci_slot" in your
> latest series.
>
> Thanks,
> Kenji Kaneshige
>
>
> ---
> drivers/pci/hotplug/pci_hotplug_core.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> Index: 20080610/drivers/pci/hotplug/pci_hotplug_core.c
> ===================================================================
> --- 20080610.orig/drivers/pci/hotplug/pci_hotplug_core.c
> +++ 20080610/drivers/pci/hotplug/pci_hotplug_core.c
> @@ -555,6 +555,7 @@ int pci_hp_register(struct hotplug_slot
> {
> int result;
> struct pci_slot *pci_slot;
> + struct hotplug_slot *tmp;
>
> if (slot == NULL)
> return -ENODEV;
> @@ -567,6 +568,21 @@ int pci_hp_register(struct hotplug_slot
> }
>
> /*
> + * Prevent registering multiple hotplug slots with the same name.
> + */
> + spin_lock(&pci_hotplug_slot_list_lock);
> + list_for_each_entry(tmp, &pci_hotplug_slot_list, slot_list) {
> + pci_slot = tmp->pci_slot;
> + if (pci_slot->bus == bus && pci_slot->number == slot_nr)
> + continue;
> + if (!strcmp(tmp->name, slot->name)) {
> + spin_unlock(&pci_hotplug_slot_list_lock);
> + return -EEXIST;
> + }
> + }
> + spin_unlock(&pci_hotplug_slot_list_lock);
> +
> + /*
> * 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.
>
>
Please note that I made this patch against linux-next with your
latest three patches applied.
Thanks,
Kenji Kaneshige
Hi Kenji-san,
* Kenji Kaneshige <[email protected]>:
> Alex Chiang wrote:
> > * Kenji Kaneshige <[email protected]>:
> >> I tried your patches and I have a comment (question) about the behavior.
> >>
> >> To emulate the (broken?) platform that assigns the same name to multiple
> >> slots, I made a debug patch for pciehp driver to assign same name ('1000')
> >> to all slots (my environment has two PCIe slots). With this patch, I
> >> noticed that the behavior or pci_hp_register() (or pci_create_slot())
> >> varies depending on whether pci_slot driver is loaded or not. See below.
> >>
> >> (a) With pci_slot driver loaded
> >> I got the following error message when I loaded pciehp driver.
> >>
> >> pciehp: pci_hp_register failed with error -17
> >> pciehp: Failed to register slot because of name collision. Try
> >> 'pciehp_slot_with_bus' module option.
> >> pciehp: pciehp: slot initialization failed
> >> (b) Without pci_slot driver loaded
> >> I got the kernel stack dump and error messages when I loaded pciehp
> >> driver.
> >>
> >> kobject_add_internal failed for 1000 with -EEXIST, don't try to
> >> register things with the same name in the same directory.
> >>
> >> Call Trace:
> >> [<a000000100015180>] show_stack+0x40/0xa0
> >> sp=e0000040a086fb80 bsp=e0000040a0861158
> >> [<a000000100015210>] dump_stack+0x30/0x60
> >> sp=e0000040a086fd50 bsp=e0000040a0861140
> >> [<a0000001003b3910>] kobject_add_internal+0x330/0x400
> >> sp=e0000040a086fd50 bsp=e0000040a0861100
> >> [<a0000001003b3bd0>] kobject_add_varg+0x90/0xc0
> >> sp=e0000040a086fd50 bsp=e0000040a08610c8
> >> [<a0000001003b3c90>] kobject_init_and_add+0x90/0xc0
> >> sp=e0000040a086fd50 bsp=e0000040a0861068
> >> [<a0000001003d69b0>] pci_create_slot+0x150/0x260
> >> sp=e0000040a086fd80 bsp=e0000040a0861030
> >> [<a000000200b71870>] pci_hp_register+0x130/0x880 [pci_hotplug]
> >> sp=e0000040a086fd80 bsp=e0000040a0860ff0
> >> [<a000000200ec1a60>] pciehp_probe+0x720/0xca0 [pciehp]
> >>
> >> (snip...)
> >>
> >> Unable to register kobject 1000
> >> pciehp: pci_hp_register failed with error -17
> >> pciehp: Failed to register slot because of name collision. Try
> >> 'pciehp_slot_with_bus' module option.
> >> pciehp: pciehp: slot initialization failed
> >>
> >> Could you tell me why that difference happen? And my expectation is
> >> the result should be (a) above regardless of whether pci_slot driver
> >> is loaded or not.
> >
> > The difference was because in (a), pci_slot created the slots and
> > when pciehp tried to register them, the pci slot infrastructure
> > simply refcounted them and returned, but did not try to
> > re-register the kobjects with the kobj core.
> >
> > In (b), the pci hotplug core allowed you to create multiple slots
> > with the same name, and called pci_create_slot() multiple times.
> > This time, since you were trying to create new slot objects, we
> > did not refcount them; we actually did a kzalloc *and* tried to
> > register them with the kobject core, which is why we got that
> > stack trace.
> >
> > I read your patch (a86161b3134465f) in closer detail and decided
> > that it can work without problems with my patch series.
> >
> > - Your patch will keep track of hotplug slots and
> > disallow multiple hotplug slots with the same name
> >
> > - the PCI slot infrastructure will still allow multiple
> > callers of pci_create_slot() to succeed by refcounting
> > identical slots
> >
> > This is the correct behavior to allow pciehp and pci_slot to
> > coexist because pci_slot is not trying to register a hotplug
> > handler or do any other hotplug operations.
> >
>
> Thank you for explanation. I understood.
>
> But I have one concern about the behavior when pci slot driver is not
> loaded. My patch (a86161b3134465f) prevents the kernel trace dump
> because of duplicate kobject add that would happen in the following two
> cases, though (b) is not described in the header of the patch (sorry).
>
> (a) multiple driver attempt to handle the same slot.
>
> (b) one or more driver attempt to register multiple slots with the
> same name (This can happen if broken platform assigns the same
> slot number to multiple hotplug slots, for example).
>
> With your patch, duplicate kobject add in case (b) is not prevented.
> That is my concern.
I apologize, I did not explain well enough. Let me try again.
After reading through your patch a86161b3134465f, I agree that we
should keep the functionality and the code. The refreshed patch
series I sent out yesterday (v15) includes your commit.
Here is the fully patched version of pci_hp_register, after
applying all 3 of my patches:
int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
{
int result;
struct pci_slot *pci_slot;
struct hotplug_slot *tmp;
if (slot == NULL)
return -ENODEV;
if ((slot->info == NULL) || (slot->ops == NULL))
return -EINVAL;
if (slot->release == NULL) {
dbg("Why are you trying to register a hotplug slot "
"without a proper release function?\n");
return -EINVAL;
}
/* Check if we have already registered a slot with the same name. */
tmp = get_slot_from_name(slot->name);
if (tmp)
return -EEXIST;
/*
* 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);
[...]
Note how we're checking get_slot_from_name. That should prevent
your scenario (b) that you describe above.
Maybe the diff was confusing, but I am definitely not removing
your code. I'm simply adding on top of a86161b3134465f, and not
removing it.
> I made a below patch to prevent (b), please take a look. And could you
> please consider merging it to "[PATCH 2/3] Introduce pci_slot" in your
> latest series.
Ok, now this is very confusing to me. Why is this patch so
different from a86161b3134465f?
Are you saying the call to get_slot_from_name() is no longer
sufficient?
Thanks,
/ac
>
> Thanks,
> Kenji Kaneshige
>
>
> ---
> drivers/pci/hotplug/pci_hotplug_core.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> Index: 20080610/drivers/pci/hotplug/pci_hotplug_core.c
> ===================================================================
> --- 20080610.orig/drivers/pci/hotplug/pci_hotplug_core.c
> +++ 20080610/drivers/pci/hotplug/pci_hotplug_core.c
> @@ -555,6 +555,7 @@ int pci_hp_register(struct hotplug_slot
> {
> int result;
> struct pci_slot *pci_slot;
> + struct hotplug_slot *tmp;
>
> if (slot == NULL)
> return -ENODEV;
> @@ -567,6 +568,21 @@ int pci_hp_register(struct hotplug_slot
> }
>
> /*
> + * Prevent registering multiple hotplug slots with the same name.
> + */
> + spin_lock(&pci_hotplug_slot_list_lock);
> + list_for_each_entry(tmp, &pci_hotplug_slot_list, slot_list) {
> + pci_slot = tmp->pci_slot;
> + if (pci_slot->bus == bus && pci_slot->number == slot_nr)
> + continue;
> + if (!strcmp(tmp->name, slot->name)) {
> + spin_unlock(&pci_hotplug_slot_list_lock);
> + return -EEXIST;
> + }
> + }
> + spin_unlock(&pci_hotplug_slot_list_lock);
> +
> + /*
> * 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.
>
>
> +#define ACPI_STA_FUNCTIONING (0x00000008)
please use the existing define in acpi/actypes.h:
#define ACPI_STA_DEVICE_FUNCTIONING 0x08
I don't notice any breakage WRT ACPI usage here,
I defer to Jesse on the PCI-ness parts.
Acked-by: Len Brown <[email protected]>
thanks,
-len
On Monday, June 09, 2008 8:12 pm Kenji Kaneshige wrote:
> > drivers/pci/hotplug/pci_hotplug_core.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > Index: 20080610/drivers/pci/hotplug/pci_hotplug_core.c
> > ===================================================================
> > --- 20080610.orig/drivers/pci/hotplug/pci_hotplug_core.c
> > +++ 20080610/drivers/pci/hotplug/pci_hotplug_core.c
> > @@ -555,6 +555,7 @@ int pci_hp_register(struct hotplug_slot
> > {
> > int result;
> > struct pci_slot *pci_slot;
> > + struct hotplug_slot *tmp;
> >
> > if (slot == NULL)
> > return -ENODEV;
> > @@ -567,6 +568,21 @@ int pci_hp_register(struct hotplug_slot
> > }
> >
> > /*
> > + * Prevent registering multiple hotplug slots with the same name.
> > + */
> > + spin_lock(&pci_hotplug_slot_list_lock);
> > + list_for_each_entry(tmp, &pci_hotplug_slot_list, slot_list) {
> > + pci_slot = tmp->pci_slot;
> > + if (pci_slot->bus == bus && pci_slot->number == slot_nr)
> > + continue;
> > + if (!strcmp(tmp->name, slot->name)) {
> > + spin_unlock(&pci_hotplug_slot_list_lock);
> > + return -EEXIST;
> > + }
> > + }
> > + spin_unlock(&pci_hotplug_slot_list_lock);
> > +
> > + /*
> > * 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.
>
> Please note that I made this patch against linux-next with your
> latest three patches applied.
I looked this over and it seems like it's already handled in Alex's latest
patchset (v15); can you take a look and make sure, Kenji-san (see Alex's
latest mail to you too).
I just need acks from you and Kristen before pushing this...
Thanks,
Jesse
* Len Brown <[email protected]>:
>
>
> > +#define ACPI_STA_FUNCTIONING (0x00000008)
>
> please use the existing define in acpi/actypes.h:
> #define ACPI_STA_DEVICE_FUNCTIONING 0x08
Ok, updated.
> I don't notice any breakage WRT ACPI usage here,
> I defer to Jesse on the PCI-ness parts.
>
> Acked-by: Len Brown <[email protected]>
Thanks!
/ac
* Jesse Barnes <[email protected]>:
> On Monday, June 09, 2008 8:12 pm Kenji Kaneshige wrote:
> > > drivers/pci/hotplug/pci_hotplug_core.c | 16 ++++++++++++++++
> > > 1 file changed, 16 insertions(+)
> > >
> > > Index: 20080610/drivers/pci/hotplug/pci_hotplug_core.c
> > > ===================================================================
> > > --- 20080610.orig/drivers/pci/hotplug/pci_hotplug_core.c
> > > +++ 20080610/drivers/pci/hotplug/pci_hotplug_core.c
> > > @@ -555,6 +555,7 @@ int pci_hp_register(struct hotplug_slot
> > > {
> > > int result;
> > > struct pci_slot *pci_slot;
> > > + struct hotplug_slot *tmp;
> > >
> > > if (slot == NULL)
> > > return -ENODEV;
> > > @@ -567,6 +568,21 @@ int pci_hp_register(struct hotplug_slot
> > > }
> > >
> > > /*
> > > + * Prevent registering multiple hotplug slots with the same name.
> > > + */
> > > + spin_lock(&pci_hotplug_slot_list_lock);
> > > + list_for_each_entry(tmp, &pci_hotplug_slot_list, slot_list) {
> > > + pci_slot = tmp->pci_slot;
> > > + if (pci_slot->bus == bus && pci_slot->number == slot_nr)
> > > + continue;
> > > + if (!strcmp(tmp->name, slot->name)) {
> > > + spin_unlock(&pci_hotplug_slot_list_lock);
> > > + return -EEXIST;
> > > + }
> > > + }
> > > + spin_unlock(&pci_hotplug_slot_list_lock);
> > > +
> > > + /*
> > > * 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.
> >
> > Please note that I made this patch against linux-next with your
> > latest three patches applied.
>
> I looked this over and it seems like it's already handled in Alex's latest
> patchset (v15); can you take a look and make sure, Kenji-san (see Alex's
> latest mail to you too).
I am about to send out a v16, which is based off of Jesse's
pci-2.6 tree, linux-next branch.
It would be good to verify that latest patchset.
Thanks,
/ac
> I just need acks from you and Kristen before pushing this...
>
> Thanks,
> Jesse
>
Alex-san, Jesse-san,
> Note how we're checking get_slot_from_name. That should prevent
> your scenario (b) that you describe above.
>
> Maybe the diff was confusing, but I am definitely not removing
> your code. I'm simply adding on top of a86161b3134465f, and not
> removing it.
>
I have to apologize. I was using v14 unintentionally on my test
environment yesterday, while I thought I was using v15.
I think v15 will prevent senario (b), though I have not tried it
yet. I'll check it again.
And I agree that Alex-san's patch go to Jesse-san's linux-next.
If I found something after that, I'll report it or send a
incremental patch. To tell the truth, I have several patches
that are waiting for Alex-san's patch to be merged to linux-next:)
>> I made a below patch to prevent (b), please take a look. And could you
>> please consider merging it to "[PATCH 2/3] Introduce pci_slot" in your
>> latest series.
>
> Ok, now this is very confusing to me. Why is this patch so
> different from a86161b3134465f?
>
> Are you saying the call to get_slot_from_name() is no longer
> sufficient?
>
Though I might misunderstand something about your patch, I thought
get_slot_from_name() approach would break what your patch is trying
to do.
My understanding about your patch is as follows:
(x) If multiple hotplug drivers try to register the same slot (try
to handle the same slot, in other words), pci_hp_register()
returns -EBUSY.
(y) If one or more drivers try to assign the same name to multiple
slots, pci_hp_register() returns -EEXIST.
I thought senario (x) will return -EEXIST instead of -EBUSY if we
use get_slot_from_name() approach. So I made a different patch.
In addition, regardless of whether my understanding is correct or not,
I noticed my patch I sent yesterday might be not good, because I made
it under the misunderstanding that I thought pci_hp_register() is called
even by ACPI pci slot driver...
Anyway, I'll look at your patch again after having several cups of coffee.
Thanks,
Kenji Kaneshige
Hi Kenji-san,
* Kenji Kaneshige <[email protected]>:
> Alex-san, Jesse-san,
>
>> Note how we're checking get_slot_from_name. That should prevent
>> your scenario (b) that you describe above.
>>
>> Maybe the diff was confusing, but I am definitely not removing
>> your code. I'm simply adding on top of a86161b3134465f, and not
>> removing it.
>>
>
> I have to apologize. I was using v14 unintentionally on my test
> environment yesterday, while I thought I was using v15.
>
> I think v15 will prevent senario (b), though I have not tried it
> yet. I'll check it again.
You can check either v15, which applies to Stephen Rothwell's
linux-next, or you can check v16, which applies to Jesse's
linux-next.
The only difference is in patch 1/3, where we are touching
fakephp, which is not the patch that is confusing us here. :)
> And I agree that Alex-san's patch go to Jesse-san's linux-next.
> If I found something after that, I'll report it or send a
> incremental patch. To tell the truth, I have several patches
> that are waiting for Alex-san's patch to be merged to linux-next:)
Yeah, I think incremental patches from here out are good.
>>> I made a below patch to prevent (b), please take a look. And could you
>>> please consider merging it to "[PATCH 2/3] Introduce pci_slot" in your
>>> latest series.
>>
>> Ok, now this is very confusing to me. Why is this patch so
>> different from a86161b3134465f?
>>
>> Are you saying the call to get_slot_from_name() is no longer
>> sufficient?
>>
>
> Though I might misunderstand something about your patch, I thought
> get_slot_from_name() approach would break what your patch is trying
> to do.
>
> My understanding about your patch is as follows:
>
> (x) If multiple hotplug drivers try to register the same slot (try
> to handle the same slot, in other words), pci_hp_register()
> returns -EBUSY.
>
> (y) If one or more drivers try to assign the same name to multiple
> slots, pci_hp_register() returns -EEXIST.
That was the original intent, but I think that returning -EEXIST
for (x) should be sufficient. If it turns out we really do want
-EBUSY for (x), we can add your latest fixup patch later.
> I thought senario (x) will return -EEXIST instead of -EBUSY if we
> use get_slot_from_name() approach. So I made a different patch.
>
> In addition, regardless of whether my understanding is correct or not,
> I noticed my patch I sent yesterday might be not good, because I made
> it under the misunderstanding that I thought pci_hp_register() is called
> even by ACPI pci slot driver...
Yes, it is confusing.
- PCI hotplug drivers call pci_hp_register()
- pci_hp_register() calls pci_slot_create()
- ACPI pci slot driver calls pci_slot_create()
So the get_slot_from_name() approach will not break the ACPI pci
slot driver, and it will continue to fix the broken platforms you
are dealing with.
> Anyway, I'll look at your patch again after having several cups of coffee.
:)
Thanks,
/ac
Alex-san,
Alex Chiang wrote:
> Hi Kenji-san,
>
> * Kenji Kaneshige <[email protected]>:
>> Alex-san, Jesse-san,
>>
>>> Note how we're checking get_slot_from_name. That should prevent
>>> your scenario (b) that you describe above.
>>>
>>> Maybe the diff was confusing, but I am definitely not removing
>>> your code. I'm simply adding on top of a86161b3134465f, and not
>>> removing it.
>>>
>> I have to apologize. I was using v14 unintentionally on my test
>> environment yesterday, while I thought I was using v15.
>>
>> I think v15 will prevent senario (b), though I have not tried it
>> yet. I'll check it again.
>
> You can check either v15, which applies to Stephen Rothwell's
> linux-next, or you can check v16, which applies to Jesse's
> linux-next.
>
> The only difference is in patch 1/3, where we are touching
> fakephp, which is not the patch that is confusing us here. :)
>
>> And I agree that Alex-san's patch go to Jesse-san's linux-next.
>> If I found something after that, I'll report it or send a
>> incremental patch. To tell the truth, I have several patches
>> that are waiting for Alex-san's patch to be merged to linux-next:)
>
> Yeah, I think incremental patches from here out are good.
>
>>>> I made a below patch to prevent (b), please take a look. And could you
>>>> please consider merging it to "[PATCH 2/3] Introduce pci_slot" in your
>>>> latest series.
>>> Ok, now this is very confusing to me. Why is this patch so
>>> different from a86161b3134465f?
>>>
>>> Are you saying the call to get_slot_from_name() is no longer
>>> sufficient?
>>>
>> Though I might misunderstand something about your patch, I thought
>> get_slot_from_name() approach would break what your patch is trying
>> to do.
>>
>> My understanding about your patch is as follows:
>>
>> (x) If multiple hotplug drivers try to register the same slot (try
>> to handle the same slot, in other words), pci_hp_register()
>> returns -EBUSY.
>>
>> (y) If one or more drivers try to assign the same name to multiple
>> slots, pci_hp_register() returns -EEXIST.
>
> That was the original intent, but I think that returning -EEXIST
> for (x) should be sufficient. If it turns out we really do want
> -EBUSY for (x), we can add your latest fixup patch later.
>
Ok, I understood. Thank you for clarification. I also think your
latest patch is sufficient.
Just in case, I would like to tell you the fact that (x) has following
two cases, though I think you already recognized about it.
(x-1) If multiple hotplug drivers try to handle the same slot with
different names, pci_hp_register() returns -EBUSY.
(x-2) If multiple hotplug drivers try to handle the same slot with
the same name, pci_hp_register() returns -EEXIST.
Thanks,
Kenji Kaneshige