* [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M...
2008-08-21 21:11 [PATCH 0/2] pciehp/shpchp prevent duplicate slot names Alex Chiang
@ 2008-08-21 21:13 ` Alex Chiang
2008-08-21 21:13 ` [PATCH 2/2] shpchp: " Alex Chiang
2008-08-21 22:16 ` [PATCH 0/2] pciehp/shpchp prevent duplicate slot names Jesse Barnes
2 siblings, 0 replies; 4+ messages in thread
From: Alex Chiang @ 2008-08-21 21:13 UTC (permalink / raw)
To: jbarnes, kaneshige.kenji, linux-pci, linux-kernel
Commit 3800345f723fd130d50434d4717b99d4a9f383c8 (pciehp: fix slot name)
introduces the pciehp_slot_with_bus module parameter, which was intended
to help work around broken firmware that assigns the same name to multiple
slots.
Commit 9e4f2e8d4ddb04ad16a3828cd9a369a5a5287009 (pciehp: add message about
pciehp_slot_with_bus option) tells the user to use the above parameter
in the event of a name collision.
This approach is sub-optimal because it requires too much work from
the user.
Instead, let's rename the slot on behalf of the user. If firmware
assigns the name N to multiple slots, then:
The first registered slot is assigned N
The second registered slot is assigned N-1
The third registered slot is assigned N-2
The Mth registered slot becomes N-M
In the event we overflow the slot->name parameter, we report an
error to the user.
This is a temporary fix until the entire PCI core can be reworked
such that individual drivers no longer have to manage their own
slot names.
Signed-off-by: Alex Chiang <achiang@hp.com>
---
drivers/pci/hotplug/pciehp.h | 1 -
drivers/pci/hotplug/pciehp_core.c | 21 ++++++++++++++-------
drivers/pci/hotplug/pciehp_hpc.c | 11 +----------
3 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index e3a1e7e..9e6cec6 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -43,7 +43,6 @@ extern int pciehp_poll_mode;
extern int pciehp_poll_time;
extern int pciehp_debug;
extern int pciehp_force;
-extern int pciehp_slot_with_bus;
extern struct workqueue_struct *pciehp_wq;
#define dbg(format, arg...) \
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 3677495..4fd5355 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -41,7 +41,6 @@ int pciehp_debug;
int pciehp_poll_mode;
int pciehp_poll_time;
int pciehp_force;
-int pciehp_slot_with_bus;
struct workqueue_struct *pciehp_wq;
#define DRIVER_VERSION "0.4"
@@ -56,12 +55,10 @@ module_param(pciehp_debug, bool, 0644);
module_param(pciehp_poll_mode, bool, 0644);
module_param(pciehp_poll_time, int, 0644);
module_param(pciehp_force, bool, 0644);
-module_param(pciehp_slot_with_bus, bool, 0644);
MODULE_PARM_DESC(pciehp_debug, "Debugging mode enabled or not");
MODULE_PARM_DESC(pciehp_poll_mode, "Using polling mechanism for hot-plug events or not");
MODULE_PARM_DESC(pciehp_poll_time, "Polling mechanism frequency, in seconds");
MODULE_PARM_DESC(pciehp_force, "Force pciehp, even if _OSC and OSHP are missing");
-MODULE_PARM_DESC(pciehp_slot_with_bus, "Use bus number in the slot name");
#define PCIE_MODULE_NAME "pciehp"
@@ -194,6 +191,7 @@ static int init_slots(struct controller *ctrl)
struct slot *slot;
struct hotplug_slot *hotplug_slot;
struct hotplug_slot_info *info;
+ int len, dup = 1;
int retval = -ENOMEM;
list_for_each_entry(slot, &ctrl->slot_list, slot_list) {
@@ -220,15 +218,24 @@ 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);
+duplicate_name:
retval = pci_hp_register(hotplug_slot,
ctrl->pci_dev->subordinate,
slot->device);
if (retval) {
+ /*
+ * If slot N already exists, we'll try to create
+ * slot N-1, N-2 ... N-M, until we overflow.
+ */
+ if (retval == -EEXIST) {
+ len = snprintf(slot->name, SLOT_NAME_SIZE,
+ "%d-%d", slot->number, dup++);
+ if (len < SLOT_NAME_SIZE)
+ goto duplicate_name;
+ else
+ err("duplicate slot name overflow\n");
+ }
err("pci_hp_register failed with error %d\n", retval);
- if (retval == -EEXIST)
- err("Failed to register slot because of name "
- "collision. Try \'pciehp_slot_with_bus\' "
- "module option.\n");
goto error_info;
}
/* create additional sysfs entries */
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index ad27e9e..ab31f5b 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -1030,15 +1030,6 @@ static void pcie_shutdown_notification(struct controller *ctrl)
pciehp_free_irq(ctrl);
}
-static void make_slot_name(struct slot *slot)
-{
- if (pciehp_slot_with_bus)
- snprintf(slot->name, SLOT_NAME_SIZE, "%04d_%04d",
- slot->bus, slot->number);
- else
- snprintf(slot->name, SLOT_NAME_SIZE, "%d", slot->number);
-}
-
static int pcie_init_slot(struct controller *ctrl)
{
struct slot *slot;
@@ -1053,7 +1044,7 @@ static int pcie_init_slot(struct controller *ctrl)
slot->device = ctrl->slot_device_offset + slot->hp_slot;
slot->hpc_ops = ctrl->hpc_ops;
slot->number = ctrl->first_slot;
- make_slot_name(slot);
+ snprintf(slot->name, SLOT_NAME_SIZE, "%d", slot->number);
mutex_init(&slot->lock);
INIT_DELAYED_WORK(&slot->work, pciehp_queue_pushbutton_work);
list_add(&slot->slot_list, &ctrl->slot_list);
--
1.6.0.rc0.g95f8
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] shpchp: Rename duplicate slot name N as N-1, N-2, N-M...
2008-08-21 21:11 [PATCH 0/2] pciehp/shpchp prevent duplicate slot names Alex Chiang
2008-08-21 21:13 ` [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M Alex Chiang
@ 2008-08-21 21:13 ` Alex Chiang
2008-08-21 22:16 ` [PATCH 0/2] pciehp/shpchp prevent duplicate slot names Jesse Barnes
2 siblings, 0 replies; 4+ messages in thread
From: Alex Chiang @ 2008-08-21 21:13 UTC (permalink / raw)
To: jbarnes, kaneshige.kenji, linux-pci, linux-kernel
Commit ef0ff95f136f0f2d035667af5d18b824609de320 (shpchp: fix slot name)
introduces the shpchp_slot_with_bus module parameter, which was intended
to help work around broken firmware that assigns the same name to multiple
slots.
Commit b3bd307c628af2f0a581c42d5d7e4bcdbbf64b6a (shpchp: add message about
shpchp_slot_with_bus option) tells the user to use the above parameter
in the event of a name collision.
This approach is sub-optimal because it requires too much work from
the user.
Instead, let's rename the slot on behalf of the user. If firmware
assigns the name N to multiple slots, then:
The first registered slot is assigned N
The second registered slot is assigned N-1
The third registered slot is assigned N-2
The Mth registered slot becomes N-M
In the event we overflow the slot->name parameter, we report an
error to the user.
This is a temporary fix until the entire PCI core can be reworked
such that individual drivers no longer have to manage their own
slot names.
Signed-off-by: Alex Chiang <achiang@hp.com>
---
drivers/pci/hotplug/shpchp_core.c | 34 +++++++++++++++-------------------
1 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
index a8cbd03..cc38615 100644
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -39,7 +39,6 @@
int shpchp_debug;
int shpchp_poll_mode;
int shpchp_poll_time;
-static int shpchp_slot_with_bus;
struct workqueue_struct *shpchp_wq;
#define DRIVER_VERSION "0.4"
@@ -53,11 +52,9 @@ MODULE_LICENSE("GPL");
module_param(shpchp_debug, bool, 0644);
module_param(shpchp_poll_mode, bool, 0644);
module_param(shpchp_poll_time, int, 0644);
-module_param(shpchp_slot_with_bus, bool, 0644);
MODULE_PARM_DESC(shpchp_debug, "Debugging mode enabled or not");
MODULE_PARM_DESC(shpchp_poll_mode, "Using polling mechanism for hot-plug events or not");
MODULE_PARM_DESC(shpchp_poll_time, "Polling mechanism frequency, in seconds");
-MODULE_PARM_DESC(shpchp_slot_with_bus, "Use bus number in the slot name");
#define SHPC_MODULE_NAME "shpchp"
@@ -99,23 +96,13 @@ static void release_slot(struct hotplug_slot *hotplug_slot)
kfree(slot);
}
-static void make_slot_name(struct slot *slot)
-{
- if (shpchp_slot_with_bus)
- snprintf(slot->hotplug_slot->name, SLOT_NAME_SIZE, "%04d_%04d",
- slot->bus, slot->number);
- else
- snprintf(slot->hotplug_slot->name, SLOT_NAME_SIZE, "%d",
- slot->number);
-}
-
static int init_slots(struct controller *ctrl)
{
struct slot *slot;
struct hotplug_slot *hotplug_slot;
struct hotplug_slot_info *info;
int retval = -ENOMEM;
- int i;
+ int i, len, dup = 1;
for (i = 0; i < ctrl->num_slots; i++) {
slot = kzalloc(sizeof(*slot), GFP_KERNEL);
@@ -146,7 +133,7 @@ static int init_slots(struct controller *ctrl)
/* register this slot with the hotplug pci core */
hotplug_slot->private = slot;
hotplug_slot->release = &release_slot;
- make_slot_name(slot);
+ snprintf(slot->name, SLOT_NAME_SIZE, "%d", slot->number);
hotplug_slot->ops = &shpchp_hotplug_slot_ops;
get_power_status(hotplug_slot, &info->power_status);
@@ -157,14 +144,23 @@ 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);
+duplicate_name:
retval = pci_hp_register(slot->hotplug_slot,
ctrl->pci_dev->subordinate, slot->device);
if (retval) {
+ /*
+ * If slot N already exists, we'll try to create
+ * slot N-1, N-2 ... N-M, until we overflow.
+ */
+ if (retval == -EEXIST) {
+ len = snprintf(slot->name, SLOT_NAME_SIZE,
+ "%d-%d", slot->number, dup++);
+ if (len < SLOT_NAME_SIZE)
+ goto duplicate_name;
+ else
+ err("duplicate slot name overflow\n");
+ }
err("pci_hp_register failed with error %d\n", retval);
- if (retval == -EEXIST)
- err("Failed to register slot because of name "
- "collision. Try \'shpchp_slot_with_bus\' "
- "module option.\n");
goto error_info;
}
--
1.6.0.rc0.g95f8
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 0/2] pciehp/shpchp prevent duplicate slot names
2008-08-21 21:11 [PATCH 0/2] pciehp/shpchp prevent duplicate slot names Alex Chiang
2008-08-21 21:13 ` [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M Alex Chiang
2008-08-21 21:13 ` [PATCH 2/2] shpchp: " Alex Chiang
@ 2008-08-21 22:16 ` Jesse Barnes
2 siblings, 0 replies; 4+ messages in thread
From: Jesse Barnes @ 2008-08-21 22:16 UTC (permalink / raw)
To: Alex Chiang; +Cc: kaneshige.kenji, linux-pci, linux-kernel
On Thursday, August 21, 2008 2:11 pm Alex Chiang wrote:
> Hi Jesse,
>
> Here are patches:
>
> [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M...
> [PATCH 2/2] shpchp: Rename duplicate slot name N as N-1, N-2, N-M...
>
> Against Linus's 2.6.27-rc4.
>
> As noted in the changelogs, these are temporary patches, meant as
> placeholders until I complete the longer patch series that allows
> the PCI core to manage the slot names on behalf of the drivers.
>
> The longer patch series will keep the same behavior that these
> temp patches implement, so that's goodness.
>
> These patches have already been tested and acked by Kenji-san.
>
> http://article.gmane.org/gmane.linux.kernel/714583
> http://article.gmane.org/gmane.linux.kernel/714582
>
> So I think you can add his sign-offs too.
Excellent, thanks Alex & Kenji-san. I applied this to my for-linus branch.
I'll have Linus pull it soon.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 4+ messages in thread