Patchwork [v9,09/11] PCI, acpiphp: Don't bailout even no slots found yet.

login
register
mail settings
Submitter Yinghai Lu
Date Jan. 18, 2013, 7:53 a.m.
Message ID <1358495602-22867-10-git-send-email-yinghai@kernel.org>
Download mbox | patch
Permalink /patch/213510/
State Superseded
Headers show

Comments

Yinghai Lu - Jan. 18, 2013, 7:53 a.m.
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(-)
Rafael J. Wysocki - Jan. 20, 2013, 11:08 p.m.
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
>
Yinghai Lu - Jan. 21, 2013, 2:42 a.m.
>
> 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

Patch

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