Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756962AbYGaKee (ORCPT ); Thu, 31 Jul 2008 06:34:34 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755967AbYGaKeF (ORCPT ); Thu, 31 Jul 2008 06:34:05 -0400 Received: from fgwmail7.fujitsu.co.jp ([192.51.44.37]:34445 "EHLO fgwmail7.fujitsu.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755764AbYGaKeC (ORCPT ); Thu, 31 Jul 2008 06:34:02 -0400 Message-ID: <489194BE.8090505@jp.fujitsu.com> Date: Thu, 31 Jul 2008 19:32:30 +0900 From: Kenji Kaneshige User-Agent: Thunderbird 2.0.0.16 (Windows/20080708) MIME-Version: 1.0 To: Alex Chiang , Kenji Kaneshige , Matthew Wilcox , Pierre Ossman , Jesse Barnes , LKML , linux-pci@vger.kernel.org Subject: Re: [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M... References: <200807241407.18543.jbarnes@virtuousgeek.org> <20080724235127.40bd0ac9@mjolnir.drzeus.cx> <200807241506.58973.jbarnes@virtuousgeek.org> <20080724222914.GG5307@ldl.fc.hp.com> <20080725004926.5f201c70@mjolnir.drzeus.cx> <20080724230827.GA30302@ldl.fc.hp.com> <20080725012916.06679a6d@mjolnir.drzeus.cx> <20080725032909.GA6701@parisc-linux.org> <48895D3C.7040100@jp.fujitsu.com> <20080730023842.GA4269@ldl.fc.hp.com> <20080730024248.GB4269@ldl.fc.hp.com> In-Reply-To: <20080730024248.GB4269@ldl.fc.hp.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5470 Lines: 147 Tested-by & Acked-by: Kenji Kaneshige 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 > --- > 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); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/