Patchwork [v9,07/11] PCI, acpiphp: Move and enhance hotplug support of pci host bridge

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

Comments

Yinghai Lu - Jan. 18, 2013, 7:53 a.m.
We have partial hot-add support in acpiphp driver, and it is confusing.

Move host bridge hot-add support to pci_root.c, and keep acpiphp simple,
also add hot-remove support in pci_root.c.

How to test it: if sci_emu patch is applied,

Find out root bus number to acpi root name mapping from dmesg or /sys

  echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify
to remove root bus

  echo "\_SB.PCIB 1" > /sys/kernel/debug/acpi/sci_notify
to add back root bus

-v2: put back pci_root_hp change in one patch
-v3: add pcibios_resource_survey_bus() calling
-v4: remove not needed code with remove_bridge
-v5: put back support for acpiphp support for slots just on root bus.
-v6: change some functions to *_p2p_* to make it more clean.
-v7: split hot_added change out.
-v8: Move to pci_root.c instead of adding another file requested by Bjorn.
-v9: Fold three following patches into this one for easy review:
	a: Add missing hot_remove support for root device.
	b: Tang Chen noticed that hotplug through container will not update
	   acpi_root_bridge list. After closely checking, we don't need
	   that for struct for tracking and could use acpi_pci_root directly.
	c: Tang Chen found handle_root_bridge_removal is very similiar to
	   acpi_bus_hot_remove_device().  Change to handle_root_bridge_removal
	   to use acpi_bus_hot_remove_device.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/acpi/pci_root.c            |  139 ++++++++++++++++++++++++++++++++++++
 drivers/pci/hotplug/acpiphp_glue.c |   59 ++++-----------
 2 files changed, 154 insertions(+), 44 deletions(-)
Rafael J. Wysocki - Jan. 20, 2013, 10:55 p.m.
On Thursday, January 17, 2013 11:53:18 PM Yinghai Lu wrote:
> We have partial hot-add support in acpiphp driver, and it is confusing.
> 
> Move host bridge hot-add support to pci_root.c, and keep acpiphp simple,
> also add hot-remove support in pci_root.c.
> 
> How to test it: if sci_emu patch is applied,
> 
> Find out root bus number to acpi root name mapping from dmesg or /sys
> 
>   echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify
> to remove root bus
> 
>   echo "\_SB.PCIB 1" > /sys/kernel/debug/acpi/sci_notify
> to add back root bus
> 
> -v2: put back pci_root_hp change in one patch
> -v3: add pcibios_resource_survey_bus() calling
> -v4: remove not needed code with remove_bridge
> -v5: put back support for acpiphp support for slots just on root bus.
> -v6: change some functions to *_p2p_* to make it more clean.
> -v7: split hot_added change out.
> -v8: Move to pci_root.c instead of adding another file requested by Bjorn.
> -v9: Fold three following patches into this one for easy review:
> 	a: Add missing hot_remove support for root device.
> 	b: Tang Chen noticed that hotplug through container will not update
> 	   acpi_root_bridge list. After closely checking, we don't need
> 	   that for struct for tracking and could use acpi_pci_root directly.
> 	c: Tang Chen found handle_root_bridge_removal is very similiar to
> 	   acpi_bus_hot_remove_device().  Change to handle_root_bridge_removal
> 	   to use acpi_bus_hot_remove_device.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/acpi/pci_root.c            |  139 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/hotplug/acpiphp_glue.c |   59 ++++-----------
>  2 files changed, 154 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index bf5108a..3ce5d80 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -655,3 +655,142 @@ int __init acpi_pci_root_init(void)
>  
>  	return 0;
>  }
> +
> +/* Support root bridge hotplug */
> +
> +static void handle_root_bridge_insertion(acpi_handle handle)
> +{
> +	struct acpi_device *device, *pdevice;
> +	acpi_handle phandle;
> +	int ret_val;
> +
> +	acpi_get_parent(handle, &phandle);
> +	if (acpi_bus_get_device(phandle, &pdevice)) {
> +		printk(KERN_DEBUG "no parent device, assuming NULL\n");
> +		pdevice = NULL;
> +	}
> +	if (!acpi_bus_get_device(handle, &device)) {
> +		/* check if  pci root_bus is removed */
> +		struct acpi_pci_root *root = acpi_driver_data(device);
> +		if (pci_find_bus(root->segment, root->secondary.start))
> +			return;
> +
> +		printk(KERN_DEBUG "bus exists... trim\n");
> +		/* this shouldn't be in here, so remove
> +		 * the bus then re-add it...
> +		 */
> +		ret_val = acpi_bus_trim(device);

You said that this followed acpiphp, but the purpose of the trimming in there
seems to be to handle surprise removal and re-insertion, which I'm not sure is
OK with something like a host bridge.

The drawback is that if we have a spurious ACPI_NOTIFY_BUS_CHECK or
ACPI_NOTIFY_DEVICE_CHECK, we'll be trying to remove the whole bus here in
response.  That doesn't sound quite right.

> +		printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
> +	}
> +	if (acpi_bus_add(handle))
> +		printk(KERN_ERR "cannot add bridge to acpi list\n");
> +}
> +
> +static void handle_root_bridge_removal(struct acpi_device *device)
> +{
> +	struct acpi_eject_event *ej_event;
> +
> +	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
> +	if (!ej_event)

Shouldn't we do acpi_evaluate_hotplug_ost() here?

> +		return;
> +
> +	ej_event->device = device;
> +	ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
> +
> +	acpi_bus_hot_remove_device(ej_event);
> +}
> +
> +static void _handle_hotplug_event_root(struct work_struct *work)
> +{
> +	struct acpi_pci_root *root;
> +	char objname[64];
> +	struct acpi_buffer buffer = { .length = sizeof(objname),
> +				      .pointer = objname };
> +	struct acpi_hp_work *hp_work;
> +	acpi_handle handle;
> +	u32 type;
> +
> +	hp_work = container_of(work, struct acpi_hp_work, work);
> +	handle = hp_work->handle;
> +	type = hp_work->type;
> +
> +	root = acpi_pci_find_root(handle);
> +
> +	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);

Is the path guaranteed not to be longer than 64 characters?

> +
> +	switch (type) {
> +	case ACPI_NOTIFY_BUS_CHECK:
> +		/* bus enumerate */
> +		printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
> +				 objname);

pr_debug() would be nicer (and analogously below).

> +		if (!root)
> +			handle_root_bridge_insertion(handle);
> +
> +		break;
> +
> +	case ACPI_NOTIFY_DEVICE_CHECK:
> +		/* device check */
> +		printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
> +				 objname);
> +		if (!root)
> +			handle_root_bridge_insertion(handle);
> +		break;
> +
> +	case ACPI_NOTIFY_EJECT_REQUEST:
> +		/* request device eject */
> +		printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__,
> +				 objname);
> +		if (root)
> +			handle_root_bridge_removal(root->device);
> +		break;
> +	default:
> +		printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
> +				 type, objname);
> +		break;
> +	}
> +
> +	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
> +}
> +
> +static void handle_hotplug_event_root(acpi_handle handle, u32 type,
> +					void *context)
> +{
> +	alloc_acpi_hp_work(handle, type, context,
> +				_handle_hotplug_event_root);
> +}
> +
> +static acpi_status __init
> +find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
> +{
> +	char objname[64];
> +	struct acpi_buffer buffer = { .length = sizeof(objname),
> +				      .pointer = objname };
> +	int *count = (int *)context;
> +
> +	if (!acpi_is_root_bridge(handle))
> +		return AE_OK;
> +
> +	(*count)++;
> +
> +	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> +
> +	acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> +				handle_hotplug_event_root, NULL);
> +	printk(KERN_DEBUG "acpi root: %s notify handler installed\n", objname);
> +
> +	return AE_OK;
> +}
> +
> +static int __init acpi_pci_root_hp_init(void)
> +{
> +	int num = 0;
> +
> +	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> +		ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
> +
> +	printk(KERN_DEBUG "Found %d acpi root devices\n", num);
> +
> +	return 0;
> +}
> +
> +subsys_initcall(acpi_pci_root_hp_init);

Why do we need a separate initcall here?  Is it not enough to call
acpi_pci_root_hp_init() from acpi_init() or acpi_scan_init(), or
acpi_pci_root_init() even?

> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 55e03b6..45917a5 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -543,10 +543,13 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
>  	acpi_status status;
>  	acpi_handle handle = bridge->handle;
>  
> -	status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> +	if (bridge->type != BRIDGE_TYPE_HOST) {
> +		status = acpi_remove_notify_handler(handle,
> +					    ACPI_SYSTEM_NOTIFY,
>  					    handle_hotplug_event_bridge);
> -	if (ACPI_FAILURE(status))
> -		err("failed to remove notify handler\n");
> +		if (ACPI_FAILURE(status))
> +			err("failed to remove notify handler\n");
> +	}
>  
>  	if ((bridge->type != BRIDGE_TYPE_HOST) &&
>  	    ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
> @@ -630,9 +633,6 @@ static void remove_bridge(struct acpi_pci_root *root)
>  	bridge = acpiphp_handle_to_bridge(handle);
>  	if (bridge)
>  		cleanup_bridge(bridge);
> -	else
> -		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> -					   handle_hotplug_event_bridge);
>  }
>  
>  static int power_on_slot(struct acpiphp_slot *slot)
> @@ -1123,18 +1123,12 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
>  }
>  
>  /* Program resources in newly inserted bridge */
> -static int acpiphp_configure_bridge (acpi_handle handle)
> +static int acpiphp_configure_p2p_bridge(acpi_handle handle)
>  {
> -	struct pci_bus *bus;
> +	struct pci_dev *pdev = acpi_get_pci_dev(handle);
> +	struct pci_bus *bus = pdev->subordinate;
>  
> -	if (acpi_is_root_bridge(handle)) {
> -		struct acpi_pci_root *root = acpi_pci_find_root(handle);
> -		bus = root->bus;
> -	} else {
> -		struct pci_dev *pdev = acpi_get_pci_dev(handle);
> -		bus = pdev->subordinate;
> -		pci_dev_put(pdev);
> -	}
> +	pci_dev_put(pdev);
>  
>  	pci_bus_size_bridges(bus);
>  	pci_bus_assign_resources(bus);
> @@ -1144,7 +1138,7 @@ static int acpiphp_configure_bridge (acpi_handle handle)
>  	return 0;
>  }
>  
> -static void handle_bridge_insertion(acpi_handle handle, u32 type)
> +static void handle_p2p_bridge_insertion(acpi_handle handle, u32 type)
>  {
>  	struct acpi_device *device;
>  
> @@ -1162,8 +1156,8 @@ static void handle_bridge_insertion(acpi_handle handle, u32 type)
>  		err("ACPI device object missing\n");
>  		return;
>  	}
> -	if (!acpiphp_configure_bridge(handle))
> -		add_bridge(handle);
> +	if (!acpiphp_configure_p2p_bridge(handle))
> +		add_p2p_bridge(handle);
>  	else
>  		err("cannot configure and start bridge\n");
>  
> @@ -1221,7 +1215,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
>  
>  	if (acpi_bus_get_device(handle, &device)) {
>  		/* This bridge must have just been physically inserted */
> -		handle_bridge_insertion(handle, type);
> +		handle_p2p_bridge_insertion(handle, type);
>  		goto out;
>  	}
>  
> @@ -1396,21 +1390,6 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type,
>  	alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_func);
>  }
>  
> -static acpi_status
> -find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
> -{
> -	int *count = (int *)context;
> -
> -	if (!acpi_is_root_bridge(handle))
> -		return AE_OK;
> -
> -	(*count)++;
> -	acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> -				    handle_hotplug_event_bridge, NULL);
> -
> -	return AE_OK ;
> -}
> -
>  static struct acpi_pci_driver acpi_pci_hp_driver = {
>  	.add =		add_bridge,
>  	.remove =	remove_bridge,
> @@ -1421,15 +1400,7 @@ static struct acpi_pci_driver acpi_pci_hp_driver = {
>   */
>  int __init acpiphp_glue_init(void)
>  {
> -	int num = 0;
> -
> -	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> -			ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
> -
> -	if (num <= 0)
> -		return -1;
> -	else
> -		acpi_pci_register_driver(&acpi_pci_hp_driver);
> +	acpi_pci_register_driver(&acpi_pci_hp_driver);
>  
>  	return 0;
>  }

Thanks,
Rafael
Yinghai Lu - Jan. 21, 2013, 2:32 a.m.
On Sun, Jan 20, 2013 at 2:55 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Thursday, January 17, 2013 11:53:18 PM Yinghai Lu wrote:
>> We have partial hot-add support in acpiphp driver, and it is confusing.
>>
>> Move host bridge hot-add support to pci_root.c, and keep acpiphp simple,
>> also add hot-remove support in pci_root.c.
>>
>> How to test it: if sci_emu patch is applied,
>>
>> Find out root bus number to acpi root name mapping from dmesg or /sys
>>
>>   echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify
>> to remove root bus
>>
>>   echo "\_SB.PCIB 1" > /sys/kernel/debug/acpi/sci_notify
>> to add back root bus
>>
>> -v2: put back pci_root_hp change in one patch
>> -v3: add pcibios_resource_survey_bus() calling
>> -v4: remove not needed code with remove_bridge
>> -v5: put back support for acpiphp support for slots just on root bus.
>> -v6: change some functions to *_p2p_* to make it more clean.
>> -v7: split hot_added change out.
>> -v8: Move to pci_root.c instead of adding another file requested by Bjorn.
>> -v9: Fold three following patches into this one for easy review:
>>       a: Add missing hot_remove support for root device.
>>       b: Tang Chen noticed that hotplug through container will not update
>>          acpi_root_bridge list. After closely checking, we don't need
>>          that for struct for tracking and could use acpi_pci_root directly.
>>       c: Tang Chen found handle_root_bridge_removal is very similiar to
>>          acpi_bus_hot_remove_device().  Change to handle_root_bridge_removal
>>          to use acpi_bus_hot_remove_device.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/acpi/pci_root.c            |  139 ++++++++++++++++++++++++++++++++++++
>>  drivers/pci/hotplug/acpiphp_glue.c |   59 ++++-----------
>>  2 files changed, 154 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index bf5108a..3ce5d80 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -655,3 +655,142 @@ int __init acpi_pci_root_init(void)
>>
>>       return 0;
>>  }
>> +
>> +/* Support root bridge hotplug */
>> +
>> +static void handle_root_bridge_insertion(acpi_handle handle)
>> +{
>> +     struct acpi_device *device, *pdevice;
>> +     acpi_handle phandle;
>> +     int ret_val;
>> +
>> +     acpi_get_parent(handle, &phandle);
>> +     if (acpi_bus_get_device(phandle, &pdevice)) {
>> +             printk(KERN_DEBUG "no parent device, assuming NULL\n");
>> +             pdevice = NULL;
>> +     }
>> +     if (!acpi_bus_get_device(handle, &device)) {
>> +             /* check if  pci root_bus is removed */
>> +             struct acpi_pci_root *root = acpi_driver_data(device);
>> +             if (pci_find_bus(root->segment, root->secondary.start))
>> +                     return;
>> +
>> +             printk(KERN_DEBUG "bus exists... trim\n");
>> +             /* this shouldn't be in here, so remove
>> +              * the bus then re-add it...
>> +              */
>> +             ret_val = acpi_bus_trim(device);
>
> You said that this followed acpiphp, but the purpose of the trimming in there
> seems to be to handle surprise removal and re-insertion, which I'm not sure is
> OK with something like a host bridge.

ok, will just bail out if it is there.

>
> The drawback is that if we have a spurious ACPI_NOTIFY_BUS_CHECK or
> ACPI_NOTIFY_DEVICE_CHECK, we'll be trying to remove the whole bus here in
> response.  That doesn't sound quite right.
>
>> +             printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
>> +     }
>> +     if (acpi_bus_add(handle))
>> +             printk(KERN_ERR "cannot add bridge to acpi list\n");
>> +}
>> +
>> +static void handle_root_bridge_removal(struct acpi_device *device)
>> +{
>> +     struct acpi_eject_event *ej_event;
>> +
>> +     ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
>> +     if (!ej_event)
>
> Shouldn't we do acpi_evaluate_hotplug_ost() here?

ok. will add

                /* Inform firmware the hot-remove operation has error */
                (void) acpi_evaluate_hotplug_ost(device->handle,
                                        ACPI_NOTIFY_EJECT_REQUEST,
                                        ACPI_OST_SC_NON_SPECIFIC_FAILURE,
                                        NULL);

before return.

>
>> +             return;
>> +
>> +     ej_event->device = device;
>> +     ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
>> +
>> +     acpi_bus_hot_remove_device(ej_event);
>> +}
>> +
>> +static void _handle_hotplug_event_root(struct work_struct *work)
>> +{
>> +     struct acpi_pci_root *root;
>> +     char objname[64];
>> +     struct acpi_buffer buffer = { .length = sizeof(objname),
>> +                                   .pointer = objname };
>> +     struct acpi_hp_work *hp_work;
>> +     acpi_handle handle;
>> +     u32 type;
>> +
>> +     hp_work = container_of(work, struct acpi_hp_work, work);
>> +     handle = hp_work->handle;
>> +     type = hp_work->type;
>> +
>> +     root = acpi_pci_find_root(handle);
>> +
>> +     acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>
> Is the path guaranteed not to be longer than 64 characters?

good. will use

struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER };

instead.

>
>> +
>> +     switch (type) {
>> +     case ACPI_NOTIFY_BUS_CHECK:
>> +             /* bus enumerate */
>> +             printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
>> +                              objname);
>
> pr_debug() would be nicer (and analogously below).

maybe we can change that later after it gets stable a bit.

>
>> +             if (!root)
>> +                     handle_root_bridge_insertion(handle);
>> +
>> +             break;
>> +
>> +     case ACPI_NOTIFY_DEVICE_CHECK:
>> +             /* device check */
>> +             printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
>> +                              objname);
>> +             if (!root)
>> +                     handle_root_bridge_insertion(handle);
>> +             break;
>> +
>> +     case ACPI_NOTIFY_EJECT_REQUEST:
>> +             /* request device eject */
>> +             printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__,
>> +                              objname);
>> +             if (root)
>> +                     handle_root_bridge_removal(root->device);
>> +             break;
>> +     default:
>> +             printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
>> +                              type, objname);
>> +             break;
>> +     }
>> +
>> +     kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
>> +}
>> +
>> +static void handle_hotplug_event_root(acpi_handle handle, u32 type,
>> +                                     void *context)
>> +{
>> +     alloc_acpi_hp_work(handle, type, context,
>> +                             _handle_hotplug_event_root);
>> +}
>> +
>> +static acpi_status __init
>> +find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
>> +{
>> +     char objname[64];
>> +     struct acpi_buffer buffer = { .length = sizeof(objname),
>> +                                   .pointer = objname };
>> +     int *count = (int *)context;
>> +
>> +     if (!acpi_is_root_bridge(handle))
>> +             return AE_OK;
>> +
>> +     (*count)++;
>> +
>> +     acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>> +
>> +     acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
>> +                             handle_hotplug_event_root, NULL);
>> +     printk(KERN_DEBUG "acpi root: %s notify handler installed\n", objname);
>> +
>> +     return AE_OK;
>> +}
>> +
>> +static int __init acpi_pci_root_hp_init(void)
>> +{
>> +     int num = 0;
>> +
>> +     acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
>> +             ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
>> +
>> +     printk(KERN_DEBUG "Found %d acpi root devices\n", num);
>> +
>> +     return 0;
>> +}
>> +
>> +subsys_initcall(acpi_pci_root_hp_init);
>
> Why do we need a separate initcall here?  Is it not enough to call
> acpi_pci_root_hp_init() from acpi_init() or acpi_scan_init(), or
> acpi_pci_root_init() even?

ok, will give it a try.

Thanks

Yinghai
--
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/acpi/pci_root.c b/drivers/acpi/pci_root.c
index bf5108a..3ce5d80 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -655,3 +655,142 @@  int __init acpi_pci_root_init(void)
 
 	return 0;
 }
+
+/* Support root bridge hotplug */
+
+static void handle_root_bridge_insertion(acpi_handle handle)
+{
+	struct acpi_device *device, *pdevice;
+	acpi_handle phandle;
+	int ret_val;
+
+	acpi_get_parent(handle, &phandle);
+	if (acpi_bus_get_device(phandle, &pdevice)) {
+		printk(KERN_DEBUG "no parent device, assuming NULL\n");
+		pdevice = NULL;
+	}
+	if (!acpi_bus_get_device(handle, &device)) {
+		/* check if  pci root_bus is removed */
+		struct acpi_pci_root *root = acpi_driver_data(device);
+		if (pci_find_bus(root->segment, root->secondary.start))
+			return;
+
+		printk(KERN_DEBUG "bus exists... trim\n");
+		/* this shouldn't be in here, so remove
+		 * the bus then re-add it...
+		 */
+		ret_val = acpi_bus_trim(device);
+		printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
+	}
+	if (acpi_bus_add(handle))
+		printk(KERN_ERR "cannot add bridge to acpi list\n");
+}
+
+static void handle_root_bridge_removal(struct acpi_device *device)
+{
+	struct acpi_eject_event *ej_event;
+
+	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
+	if (!ej_event)
+		return;
+
+	ej_event->device = device;
+	ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
+
+	acpi_bus_hot_remove_device(ej_event);
+}
+
+static void _handle_hotplug_event_root(struct work_struct *work)
+{
+	struct acpi_pci_root *root;
+	char objname[64];
+	struct acpi_buffer buffer = { .length = sizeof(objname),
+				      .pointer = objname };
+	struct acpi_hp_work *hp_work;
+	acpi_handle handle;
+	u32 type;
+
+	hp_work = container_of(work, struct acpi_hp_work, work);
+	handle = hp_work->handle;
+	type = hp_work->type;
+
+	root = acpi_pci_find_root(handle);
+
+	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+
+	switch (type) {
+	case ACPI_NOTIFY_BUS_CHECK:
+		/* bus enumerate */
+		printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
+				 objname);
+		if (!root)
+			handle_root_bridge_insertion(handle);
+
+		break;
+
+	case ACPI_NOTIFY_DEVICE_CHECK:
+		/* device check */
+		printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
+				 objname);
+		if (!root)
+			handle_root_bridge_insertion(handle);
+		break;
+
+	case ACPI_NOTIFY_EJECT_REQUEST:
+		/* request device eject */
+		printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__,
+				 objname);
+		if (root)
+			handle_root_bridge_removal(root->device);
+		break;
+	default:
+		printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
+				 type, objname);
+		break;
+	}
+
+	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
+}
+
+static void handle_hotplug_event_root(acpi_handle handle, u32 type,
+					void *context)
+{
+	alloc_acpi_hp_work(handle, type, context,
+				_handle_hotplug_event_root);
+}
+
+static acpi_status __init
+find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	char objname[64];
+	struct acpi_buffer buffer = { .length = sizeof(objname),
+				      .pointer = objname };
+	int *count = (int *)context;
+
+	if (!acpi_is_root_bridge(handle))
+		return AE_OK;
+
+	(*count)++;
+
+	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+
+	acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+				handle_hotplug_event_root, NULL);
+	printk(KERN_DEBUG "acpi root: %s notify handler installed\n", objname);
+
+	return AE_OK;
+}
+
+static int __init acpi_pci_root_hp_init(void)
+{
+	int num = 0;
+
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+		ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
+
+	printk(KERN_DEBUG "Found %d acpi root devices\n", num);
+
+	return 0;
+}
+
+subsys_initcall(acpi_pci_root_hp_init);
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 55e03b6..45917a5 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -543,10 +543,13 @@  static void cleanup_bridge(struct acpiphp_bridge *bridge)
 	acpi_status status;
 	acpi_handle handle = bridge->handle;
 
-	status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+	if (bridge->type != BRIDGE_TYPE_HOST) {
+		status = acpi_remove_notify_handler(handle,
+					    ACPI_SYSTEM_NOTIFY,
 					    handle_hotplug_event_bridge);
-	if (ACPI_FAILURE(status))
-		err("failed to remove notify handler\n");
+		if (ACPI_FAILURE(status))
+			err("failed to remove notify handler\n");
+	}
 
 	if ((bridge->type != BRIDGE_TYPE_HOST) &&
 	    ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
@@ -630,9 +633,6 @@  static void remove_bridge(struct acpi_pci_root *root)
 	bridge = acpiphp_handle_to_bridge(handle);
 	if (bridge)
 		cleanup_bridge(bridge);
-	else
-		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-					   handle_hotplug_event_bridge);
 }
 
 static int power_on_slot(struct acpiphp_slot *slot)
@@ -1123,18 +1123,12 @@  static void acpiphp_sanitize_bus(struct pci_bus *bus)
 }
 
 /* Program resources in newly inserted bridge */
-static int acpiphp_configure_bridge (acpi_handle handle)
+static int acpiphp_configure_p2p_bridge(acpi_handle handle)
 {
-	struct pci_bus *bus;
+	struct pci_dev *pdev = acpi_get_pci_dev(handle);
+	struct pci_bus *bus = pdev->subordinate;
 
-	if (acpi_is_root_bridge(handle)) {
-		struct acpi_pci_root *root = acpi_pci_find_root(handle);
-		bus = root->bus;
-	} else {
-		struct pci_dev *pdev = acpi_get_pci_dev(handle);
-		bus = pdev->subordinate;
-		pci_dev_put(pdev);
-	}
+	pci_dev_put(pdev);
 
 	pci_bus_size_bridges(bus);
 	pci_bus_assign_resources(bus);
@@ -1144,7 +1138,7 @@  static int acpiphp_configure_bridge (acpi_handle handle)
 	return 0;
 }
 
-static void handle_bridge_insertion(acpi_handle handle, u32 type)
+static void handle_p2p_bridge_insertion(acpi_handle handle, u32 type)
 {
 	struct acpi_device *device;
 
@@ -1162,8 +1156,8 @@  static void handle_bridge_insertion(acpi_handle handle, u32 type)
 		err("ACPI device object missing\n");
 		return;
 	}
-	if (!acpiphp_configure_bridge(handle))
-		add_bridge(handle);
+	if (!acpiphp_configure_p2p_bridge(handle))
+		add_p2p_bridge(handle);
 	else
 		err("cannot configure and start bridge\n");
 
@@ -1221,7 +1215,7 @@  static void _handle_hotplug_event_bridge(struct work_struct *work)
 
 	if (acpi_bus_get_device(handle, &device)) {
 		/* This bridge must have just been physically inserted */
-		handle_bridge_insertion(handle, type);
+		handle_p2p_bridge_insertion(handle, type);
 		goto out;
 	}
 
@@ -1396,21 +1390,6 @@  static void handle_hotplug_event_func(acpi_handle handle, u32 type,
 	alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_func);
 }
 
-static acpi_status
-find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-	int *count = (int *)context;
-
-	if (!acpi_is_root_bridge(handle))
-		return AE_OK;
-
-	(*count)++;
-	acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-				    handle_hotplug_event_bridge, NULL);
-
-	return AE_OK ;
-}
-
 static struct acpi_pci_driver acpi_pci_hp_driver = {
 	.add =		add_bridge,
 	.remove =	remove_bridge,
@@ -1421,15 +1400,7 @@  static struct acpi_pci_driver acpi_pci_hp_driver = {
  */
 int __init acpiphp_glue_init(void)
 {
-	int num = 0;
-
-	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
-			ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
-
-	if (num <= 0)
-		return -1;
-	else
-		acpi_pci_register_driver(&acpi_pci_hp_driver);
+	acpi_pci_register_driver(&acpi_pci_hp_driver);
 
 	return 0;
 }