mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] pciehp/shpchp prevent duplicate slot names
@ 2008-08-21 21:11 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
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Alex Chiang @ 2008-08-21 21:11 UTC (permalink / raw)
  To: jbarnes; +Cc: kaneshige.kenji, linux-pci, linux-kernel

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.

Thanks,

/ac


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread

* Re: [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M...
  2008-07-30  2:42                   ` [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M Alex Chiang
@ 2008-07-31 10:32                     ` Kenji Kaneshige
  0 siblings, 0 replies; 6+ messages in thread
From: Kenji Kaneshige @ 2008-07-31 10:32 UTC (permalink / raw)
  To: Alex Chiang, Kenji Kaneshige, Matthew Wilcox, Pierre Ossman,
	Jesse Barnes, LKML, linux-pci

Tested-by & Acked-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

Thnaks,
Kenji Kaneshige



Alex Chiang wrote:
> Commit 3800345f723fd130d50434d4717b99d4a9f383c8 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 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.
> 
> 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);



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M...
  2008-07-30  2:38                 ` Alex Chiang
@ 2008-07-30  2:42                   ` Alex Chiang
  2008-07-31 10:32                     ` Kenji Kaneshige
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Chiang @ 2008-07-30  2:42 UTC (permalink / raw)
  To: Kenji Kaneshige, Matthew Wilcox, Pierre Ossman, Jesse Barnes,
	LKML, linux-pci

Commit 3800345f723fd130d50434d4717b99d4a9f383c8 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 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.

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] 6+ messages in thread

end of thread, other threads:[~2008-08-21 22:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 0/2] pciehp/shpchp prevent duplicate slot names Jesse Barnes
  -- strict thread matches above, loose matches on Subject: below --
2008-07-24 21:07 post 2.6.26 requires pciehp_slot_with_bus Jesse Barnes
2008-07-24 21:51 ` Pierre Ossman
2008-07-24 22:06   ` Jesse Barnes
2008-07-24 22:29     ` Alex Chiang
2008-07-24 22:49       ` Pierre Ossman
2008-07-24 23:08         ` Alex Chiang
2008-07-24 23:29           ` Pierre Ossman
2008-07-25  3:29             ` Matthew Wilcox
2008-07-25  4:57               ` Kenji Kaneshige
2008-07-30  2:38                 ` Alex Chiang
2008-07-30  2:42                   ` [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M Alex Chiang
2008-07-31 10:32                     ` Kenji Kaneshige

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®