| Message ID | 1358495602-22867-10-git-send-email-yinghai@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thursday, January 17, 2013 11:53:20 PM Yinghai Lu wrote: > Could have root bus hot added later and there may be slots that need acpiphp. The patch implies that acpiphp_get_num_slots() is not necessary any more, but the changelog above doesn't really explain why that is so. Do I guess correctly that the bridge the slots are under may not be in bridge_list yet at the moment and so we don't really know whether or not we will need acpiphp_glue going forward? If that's the case: Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> but please say something like this in the changelog: "The result returned by acpiphp_get_num_slots() is meaningless, because the bridge the slots are under may be added after this function has been called, so drop acpiphp_get_num_slots() and the code using it." > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > --- > drivers/pci/hotplug/acpiphp.h | 1 - > drivers/pci/hotplug/acpiphp_core.c | 23 ++--------------------- > drivers/pci/hotplug/acpiphp_glue.c | 22 ---------------------- > 3 files changed, 2 insertions(+), 44 deletions(-) > > diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h > index a1afb5b..b3ead7a 100644 > --- a/drivers/pci/hotplug/acpiphp.h > +++ b/drivers/pci/hotplug/acpiphp.h > @@ -193,7 +193,6 @@ extern void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *slot); > /* acpiphp_glue.c */ > extern int acpiphp_glue_init (void); > extern void acpiphp_glue_exit (void); > -extern int acpiphp_get_num_slots (void); > typedef int (*acpiphp_callback)(struct acpiphp_slot *slot, void *data); > > extern int acpiphp_enable_slot (struct acpiphp_slot *slot); > diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c > index 96316b7..c2fd309 100644 > --- a/drivers/pci/hotplug/acpiphp_core.c > +++ b/drivers/pci/hotplug/acpiphp_core.c > @@ -50,7 +50,6 @@ > bool acpiphp_debug; > > /* local variables */ > -static int num_slots; > static struct acpiphp_attention_info *attention_info; > > #define DRIVER_VERSION "0.5" > @@ -272,25 +271,6 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value) > return 0; > } > > -static int __init init_acpi(void) > -{ > - int retval; > - > - /* initialize internal data structure etc. */ > - retval = acpiphp_glue_init(); > - > - /* read initial number of slots */ > - if (!retval) { > - num_slots = acpiphp_get_num_slots(); > - if (num_slots == 0) { > - acpiphp_glue_exit(); > - retval = -ENODEV; > - } > - } > - > - return retval; > -} > - > /** > * release_slot - free up the memory used by a slot > * @hotplug_slot: slot to free > @@ -379,7 +359,8 @@ static int __init acpiphp_init(void) > return 0; > > /* read all the ACPI info from the system */ > - return init_acpi(); > + /* initialize internal data structure etc. */ > + return acpiphp_glue_init(); > } > > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c > index 45917a5..25d6918 100644 > --- a/drivers/pci/hotplug/acpiphp_glue.c > +++ b/drivers/pci/hotplug/acpiphp_glue.c > @@ -1416,28 +1416,6 @@ void acpiphp_glue_exit(void) > acpi_pci_unregister_driver(&acpi_pci_hp_driver); > } > > - > -/** > - * acpiphp_get_num_slots - count number of slots in a system > - */ > -int __init acpiphp_get_num_slots(void) > -{ > - struct acpiphp_bridge *bridge; > - int num_slots = 0; > - > - list_for_each_entry(bridge, &bridge_list, list) { > - dbg("Bus %04x:%02x has %d slot%s\n", > - pci_domain_nr(bridge->pci_bus), > - bridge->pci_bus->number, bridge->nr_slots, > - bridge->nr_slots == 1 ? "" : "s"); > - num_slots += bridge->nr_slots; > - } > - > - dbg("Total %d slots\n", num_slots); > - return num_slots; > -} > - > - > /** > * acpiphp_enable_slot - power on slot > * @slot: ACPI PHP slot >
> > If that's the case: > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > but please say something like this in the changelog: > > "The result returned by acpiphp_get_num_slots() is meaningless, because > the bridge the slots are under may be added after this function has been > called, so drop acpiphp_get_num_slots() and the code using it." yes, I add you inputs into change log. Thanks a lot -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h index a1afb5b..b3ead7a 100644 --- a/drivers/pci/hotplug/acpiphp.h +++ b/drivers/pci/hotplug/acpiphp.h @@ -193,7 +193,6 @@ extern void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *slot); /* acpiphp_glue.c */ extern int acpiphp_glue_init (void); extern void acpiphp_glue_exit (void); -extern int acpiphp_get_num_slots (void); typedef int (*acpiphp_callback)(struct acpiphp_slot *slot, void *data); extern int acpiphp_enable_slot (struct acpiphp_slot *slot); diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c index 96316b7..c2fd309 100644 --- a/drivers/pci/hotplug/acpiphp_core.c +++ b/drivers/pci/hotplug/acpiphp_core.c @@ -50,7 +50,6 @@ bool acpiphp_debug; /* local variables */ -static int num_slots; static struct acpiphp_attention_info *attention_info; #define DRIVER_VERSION "0.5" @@ -272,25 +271,6 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value) return 0; } -static int __init init_acpi(void) -{ - int retval; - - /* initialize internal data structure etc. */ - retval = acpiphp_glue_init(); - - /* read initial number of slots */ - if (!retval) { - num_slots = acpiphp_get_num_slots(); - if (num_slots == 0) { - acpiphp_glue_exit(); - retval = -ENODEV; - } - } - - return retval; -} - /** * release_slot - free up the memory used by a slot * @hotplug_slot: slot to free @@ -379,7 +359,8 @@ static int __init acpiphp_init(void) return 0; /* read all the ACPI info from the system */ - return init_acpi(); + /* initialize internal data structure etc. */ + return acpiphp_glue_init(); } diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 45917a5..25d6918 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -1416,28 +1416,6 @@ void acpiphp_glue_exit(void) acpi_pci_unregister_driver(&acpi_pci_hp_driver); } - -/** - * acpiphp_get_num_slots - count number of slots in a system - */ -int __init acpiphp_get_num_slots(void) -{ - struct acpiphp_bridge *bridge; - int num_slots = 0; - - list_for_each_entry(bridge, &bridge_list, list) { - dbg("Bus %04x:%02x has %d slot%s\n", - pci_domain_nr(bridge->pci_bus), - bridge->pci_bus->number, bridge->nr_slots, - bridge->nr_slots == 1 ? "" : "s"); - num_slots += bridge->nr_slots; - } - - dbg("Total %d slots\n", num_slots); - return num_slots; -} - - /** * acpiphp_enable_slot - power on slot * @slot: ACPI PHP slot
Could have root bus hot added later and there may be slots that need acpiphp. Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- drivers/pci/hotplug/acpiphp.h | 1 - drivers/pci/hotplug/acpiphp_core.c | 23 ++--------------------- drivers/pci/hotplug/acpiphp_glue.c | 22 ---------------------- 3 files changed, 2 insertions(+), 44 deletions(-)