* [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices @ 2013-01-30 16:10 Jiang Liu 2013-01-30 16:10 ` [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug Jiang Liu 2013-02-01 23:06 ` [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices Bjorn Helgaas 0 siblings, 2 replies; 11+ messages in thread From: Jiang Liu @ 2013-01-30 16:10 UTC (permalink / raw) To: Bjorn Helgaas, Yinghai Lu, Rafael J . Wysocki Cc: Jiang Liu, Yijing Wang, linux-kernel, linux-pci From: Jiang Liu <jiang.liu@huawei.com> With commit 4f535093cf8f6da8c "PCI: Put pci_dev in device tree as early as possible", companion ACPI devices should be created before creating correspoding PCI devices, otherwise it will break the ACPI PCI binding logic. Signed-off-by: Jiang Liu <jiang.liu@huawei.com> --- Hi Bjorn, This patch set applies to your pci/yinghai-root-bus branch. There are still other potential bugs in the acpiphp driver under investigation. So I will send out these two first to catch up with the 3.9 merging window. Thanks! Gerry --- drivers/pci/hotplug/acpiphp_glue.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index bd784ff..acb7af2 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -839,6 +839,9 @@ static int __ref enable_device(struct acpiphp_slot *slot) if (slot->flags & SLOT_ENABLED) goto err_exit; + list_for_each_entry(func, &slot->funcs, sibling) + acpiphp_bus_add(func); + num = pci_scan_slot(bus, PCI_DEVFN(slot->device, 0)); if (num == 0) { /* Maybe only part of funcs are added. */ @@ -862,9 +865,6 @@ static int __ref enable_device(struct acpiphp_slot *slot) } } - list_for_each_entry(func, &slot->funcs, sibling) - acpiphp_bus_add(func); - pci_bus_assign_resources(bus); acpiphp_sanitize_bus(bus); acpiphp_set_hpp_values(bus); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug 2013-01-30 16:10 [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices Jiang Liu @ 2013-01-30 16:10 ` Jiang Liu 2013-01-30 17:27 ` Yinghai Lu 2013-02-01 23:18 ` Bjorn Helgaas 2013-02-01 23:06 ` [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices Bjorn Helgaas 1 sibling, 2 replies; 11+ messages in thread From: Jiang Liu @ 2013-01-30 16:10 UTC (permalink / raw) To: Bjorn Helgaas, Yinghai Lu, Rafael J . Wysocki Cc: Jiang Liu, Yijing Wang, linux-kernel, linux-pci From: Jiang Liu <jiang.liu@huawei.com> Commit 668192b678201d2fff27c "PCI: acpiphp: Move host bridge hotplug to pci_root.c" has moved PCI host bridge hotplug logic from acpiphp to pci_root, but there is still PCI host bridge hotplug related dead code left in acpiphp. So remove those dead code. Now companion ACPI devices are always created before corresponding PCI devices. And the ACPI event handle_hotplug_event_bridge() will be installed only if it has associated PCI device. So remove dead code to handle bridge hot-adding in function handle_hotplug_event_bridge(). Signed-off-by: Jiang Liu <jiang.liu@huawei.com> --- drivers/pci/hotplug/acpiphp.h | 13 +--- drivers/pci/hotplug/acpiphp_glue.c | 124 ++++-------------------------------- 2 files changed, 14 insertions(+), 123 deletions(-) diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h index b3ead7a..b70ac00 100644 --- a/drivers/pci/hotplug/acpiphp.h +++ b/drivers/pci/hotplug/acpiphp.h @@ -79,7 +79,6 @@ struct acpiphp_bridge { /* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */ struct acpiphp_func *func; - int type; int nr_slots; u32 flags; @@ -146,10 +145,6 @@ struct acpiphp_attention_info /* PCI bus bridge HID */ #define ACPI_PCI_HOST_HID "PNP0A03" -/* PCI BRIDGE type */ -#define BRIDGE_TYPE_HOST 0 -#define BRIDGE_TYPE_P2P 1 - /* ACPI _STA method value (ignore bit 4; battery present) */ #define ACPI_STA_PRESENT (0x00000001) #define ACPI_STA_ENABLED (0x00000002) @@ -158,13 +153,7 @@ struct acpiphp_attention_info #define ACPI_STA_ALL (0x0000000f) /* bridge flags */ -#define BRIDGE_HAS_STA (0x00000001) -#define BRIDGE_HAS_EJ0 (0x00000002) -#define BRIDGE_HAS_HPP (0x00000004) -#define BRIDGE_HAS_PS0 (0x00000010) -#define BRIDGE_HAS_PS1 (0x00000020) -#define BRIDGE_HAS_PS2 (0x00000040) -#define BRIDGE_HAS_PS3 (0x00000080) +#define BRIDGE_HAS_EJ0 (0x00000001) /* slot flags */ diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index acb7af2..4681d2c 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -325,8 +325,8 @@ static void init_bridge_misc(struct acpiphp_bridge *bridge) return; } - /* install notify handler */ - if (bridge->type != BRIDGE_TYPE_HOST) { + /* install notify handler for P2P bridges */ + if (!pci_is_root_bus(bridge->pci_bus)) { if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { status = acpi_remove_notify_handler(bridge->func->handle, ACPI_SYSTEM_NOTIFY, @@ -369,27 +369,12 @@ static struct acpiphp_func *acpiphp_bridge_handle_to_function(acpi_handle handle static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge) { acpi_handle dummy_handle; + struct acpiphp_func *func; if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, - "_STA", &dummy_handle))) - bridge->flags |= BRIDGE_HAS_STA; - - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, - "_EJ0", &dummy_handle))) + "_EJ0", &dummy_handle))) { bridge->flags |= BRIDGE_HAS_EJ0; - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, - "_PS0", &dummy_handle))) - bridge->flags |= BRIDGE_HAS_PS0; - - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, - "_PS3", &dummy_handle))) - bridge->flags |= BRIDGE_HAS_PS3; - - /* is this ejectable p2p bridge? */ - if (bridge->flags & BRIDGE_HAS_EJ0) { - struct acpiphp_func *func; - dbg("found ejectable p2p bridge\n"); /* make link between PCI bridge and PCI function */ @@ -412,7 +397,6 @@ static void add_host_bridge(struct acpi_pci_root *root) if (bridge == NULL) return; - bridge->type = BRIDGE_TYPE_HOST; bridge->handle = handle; bridge->pci_bus = root->bus; @@ -432,7 +416,6 @@ static void add_p2p_bridge(acpi_handle *handle) return; } - bridge->type = BRIDGE_TYPE_P2P; bridge->handle = handle; config_p2p_bridge_flags(bridge); @@ -543,7 +526,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) acpi_status status; acpi_handle handle = bridge->handle; - if (bridge->type != BRIDGE_TYPE_HOST) { + if (!pci_is_root_bus(bridge->pci_bus)) { status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY, handle_hotplug_event_bridge); @@ -551,8 +534,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) err("failed to remove notify handler\n"); } - if ((bridge->type != BRIDGE_TYPE_HOST) && - ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) { + if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { status = acpi_install_notify_handler(bridge->func->handle, ACPI_SYSTEM_NOTIFY, handle_hotplug_event_func, @@ -1122,64 +1104,11 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus) } } -/* Program resources in newly inserted bridge */ -static int acpiphp_configure_p2p_bridge(acpi_handle handle) -{ - struct pci_dev *pdev = acpi_get_pci_dev(handle); - struct pci_bus *bus = pdev->subordinate; - - pci_dev_put(pdev); - - pci_bus_size_bridges(bus); - pci_bus_assign_resources(bus); - acpiphp_sanitize_bus(bus); - acpiphp_set_hpp_values(bus); - pci_enable_bridges(bus); - return 0; -} - -static void handle_p2p_bridge_insertion(acpi_handle handle, u32 type) -{ - struct acpi_device *device; - - if ((type != ACPI_NOTIFY_BUS_CHECK) && - (type != ACPI_NOTIFY_DEVICE_CHECK)) { - err("unexpected notification type %d\n", type); - return; - } - - if (acpi_bus_scan(handle)) { - err("cannot add bridge to acpi list\n"); - return; - } - if (acpi_bus_get_device(handle, &device)) { - err("ACPI device object missing\n"); - return; - } - if (!acpiphp_configure_p2p_bridge(handle)) - add_p2p_bridge(handle); - else - err("cannot configure and start bridge\n"); - -} - /* * ACPI event handlers */ static acpi_status -count_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) -{ - int *count = (int *)context; - struct acpiphp_bridge *bridge; - - bridge = acpiphp_handle_to_bridge(handle); - if (bridge) - (*count)++; - return AE_OK ; -} - -static acpi_status check_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) { struct acpiphp_bridge *bridge; @@ -1203,8 +1132,6 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) char objname[64]; struct acpi_buffer buffer = { .length = sizeof(objname), .pointer = objname }; - struct acpi_device *device; - int num_sub_bridges = 0; struct acpi_hp_work *hp_work; acpi_handle handle; u32 type; @@ -1212,23 +1139,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) hp_work = container_of(work, struct acpi_hp_work, work); handle = hp_work->handle; type = hp_work->type; - - if (acpi_bus_get_device(handle, &device)) { - /* This bridge must have just been physically inserted */ - handle_p2p_bridge_insertion(handle, type); - goto out; - } - - bridge = acpiphp_handle_to_bridge(handle); - if (type == ACPI_NOTIFY_BUS_CHECK) { - acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, ACPI_UINT32_MAX, - count_sub_bridges, NULL, &num_sub_bridges, NULL); - } - - if (!bridge && !num_sub_bridges) { - err("cannot get bridge info\n"); - goto out; - } + bridge = (struct acpiphp_bridge *)hp_work->context; acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); @@ -1236,14 +1147,10 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) case ACPI_NOTIFY_BUS_CHECK: /* bus re-enumerate */ dbg("%s: Bus check notify on %s\n", __func__, objname); - if (bridge) { - dbg("%s: re-enumerating slots under %s\n", - __func__, objname); - acpiphp_check_bridge(bridge); - } - if (num_sub_bridges) - acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, - ACPI_UINT32_MAX, check_sub_bridges, NULL, NULL, NULL); + dbg("%s: re-enumerating slots under %s\n", __func__, objname); + acpiphp_check_bridge(bridge); + acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, + ACPI_UINT32_MAX, check_sub_bridges, NULL, NULL, NULL); break; case ACPI_NOTIFY_DEVICE_CHECK: @@ -1260,8 +1167,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) case ACPI_NOTIFY_EJECT_REQUEST: /* request device eject */ dbg("%s: Device eject notify on %s\n", __func__, objname); - if ((bridge->type != BRIDGE_TYPE_HOST) && - (bridge->flags & BRIDGE_HAS_EJ0)) { + if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { struct acpiphp_slot *slot; slot = bridge->func->slot; if (!acpiphp_disable_slot(slot)) @@ -1289,7 +1195,6 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) break; } -out: kfree(hp_work); /* allocated in handle_hotplug_event_bridge */ } @@ -1324,17 +1229,14 @@ static void _handle_hotplug_event_func(struct work_struct *work) struct acpi_hp_work *hp_work; acpi_handle handle; u32 type; - void *context; hp_work = container_of(work, struct acpi_hp_work, work); handle = hp_work->handle; type = hp_work->type; - context = hp_work->context; + func = (struct acpiphp_func *)hp_work->context; acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); - func = (struct acpiphp_func *)context; - switch (type) { case ACPI_NOTIFY_BUS_CHECK: /* bus re-enumerate */ -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug 2013-01-30 16:10 ` [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug Jiang Liu @ 2013-01-30 17:27 ` Yinghai Lu 2013-01-31 15:37 ` Jiang Liu 2013-02-01 23:18 ` Bjorn Helgaas 1 sibling, 1 reply; 11+ messages in thread From: Yinghai Lu @ 2013-01-30 17:27 UTC (permalink / raw) To: Jiang Liu Cc: Bjorn Helgaas, Rafael J . Wysocki, Jiang Liu, Yijing Wang, linux-kernel, linux-pci On Wed, Jan 30, 2013 at 8:10 AM, Jiang Liu <liuj97@gmail.com> wrote: > From: Jiang Liu <jiang.liu@huawei.com> > > Commit 668192b678201d2fff27c "PCI: acpiphp: Move host bridge hotplug > to pci_root.c" has moved PCI host bridge hotplug logic from acpiphp > to pci_root, but there is still PCI host bridge hotplug related > dead code left in acpiphp. So remove those dead code. > > Now companion ACPI devices are always created before corresponding > PCI devices. And the ACPI event handle_hotplug_event_bridge() will be > installed only if it has associated PCI device. So remove dead code to > handle bridge hot-adding in function handle_hotplug_event_bridge(). > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > --- > drivers/pci/hotplug/acpiphp.h | 13 +--- > drivers/pci/hotplug/acpiphp_glue.c | 124 ++++-------------------------------- > 2 files changed, 14 insertions(+), 123 deletions(-) > > diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h > index b3ead7a..b70ac00 100644 > --- a/drivers/pci/hotplug/acpiphp.h > +++ b/drivers/pci/hotplug/acpiphp.h > @@ -79,7 +79,6 @@ struct acpiphp_bridge { > /* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */ > struct acpiphp_func *func; > > - int type; > int nr_slots; > > u32 flags; > @@ -146,10 +145,6 @@ struct acpiphp_attention_info > /* PCI bus bridge HID */ > #define ACPI_PCI_HOST_HID "PNP0A03" > > -/* PCI BRIDGE type */ > -#define BRIDGE_TYPE_HOST 0 > -#define BRIDGE_TYPE_P2P 1 > - > /* ACPI _STA method value (ignore bit 4; battery present) */ > #define ACPI_STA_PRESENT (0x00000001) > #define ACPI_STA_ENABLED (0x00000002) > @@ -158,13 +153,7 @@ struct acpiphp_attention_info > #define ACPI_STA_ALL (0x0000000f) > > /* bridge flags */ > -#define BRIDGE_HAS_STA (0x00000001) > -#define BRIDGE_HAS_EJ0 (0x00000002) > -#define BRIDGE_HAS_HPP (0x00000004) > -#define BRIDGE_HAS_PS0 (0x00000010) > -#define BRIDGE_HAS_PS1 (0x00000020) > -#define BRIDGE_HAS_PS2 (0x00000040) > -#define BRIDGE_HAS_PS3 (0x00000080) > +#define BRIDGE_HAS_EJ0 (0x00000001) > > /* slot flags */ > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c > index acb7af2..4681d2c 100644 > --- a/drivers/pci/hotplug/acpiphp_glue.c > +++ b/drivers/pci/hotplug/acpiphp_glue.c > @@ -325,8 +325,8 @@ static void init_bridge_misc(struct acpiphp_bridge *bridge) > return; > } > > - /* install notify handler */ > - if (bridge->type != BRIDGE_TYPE_HOST) { > + /* install notify handler for P2P bridges */ > + if (!pci_is_root_bus(bridge->pci_bus)) { > if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { > status = acpi_remove_notify_handler(bridge->func->handle, > ACPI_SYSTEM_NOTIFY, > @@ -369,27 +369,12 @@ static struct acpiphp_func *acpiphp_bridge_handle_to_function(acpi_handle handle > static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge) > { > acpi_handle dummy_handle; > + struct acpiphp_func *func; > > if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, > - "_STA", &dummy_handle))) > - bridge->flags |= BRIDGE_HAS_STA; > - > - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, > - "_EJ0", &dummy_handle))) > + "_EJ0", &dummy_handle))) { > bridge->flags |= BRIDGE_HAS_EJ0; > > - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, > - "_PS0", &dummy_handle))) > - bridge->flags |= BRIDGE_HAS_PS0; > - > - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, > - "_PS3", &dummy_handle))) > - bridge->flags |= BRIDGE_HAS_PS3; > - > - /* is this ejectable p2p bridge? */ > - if (bridge->flags & BRIDGE_HAS_EJ0) { > - struct acpiphp_func *func; > - > dbg("found ejectable p2p bridge\n"); > > /* make link between PCI bridge and PCI function */ > @@ -412,7 +397,6 @@ static void add_host_bridge(struct acpi_pci_root *root) > if (bridge == NULL) > return; > > - bridge->type = BRIDGE_TYPE_HOST; > bridge->handle = handle; > > bridge->pci_bus = root->bus; > @@ -432,7 +416,6 @@ static void add_p2p_bridge(acpi_handle *handle) > return; > } > > - bridge->type = BRIDGE_TYPE_P2P; > bridge->handle = handle; > config_p2p_bridge_flags(bridge); > > @@ -543,7 +526,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) > acpi_status status; > acpi_handle handle = bridge->handle; > > - if (bridge->type != BRIDGE_TYPE_HOST) { > + if (!pci_is_root_bus(bridge->pci_bus)) { > status = acpi_remove_notify_handler(handle, > ACPI_SYSTEM_NOTIFY, > handle_hotplug_event_bridge); > @@ -551,8 +534,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) > err("failed to remove notify handler\n"); > } > > - if ((bridge->type != BRIDGE_TYPE_HOST) && > - ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) { > + if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { > status = acpi_install_notify_handler(bridge->func->handle, > ACPI_SYSTEM_NOTIFY, > handle_hotplug_event_func, > @@ -1122,64 +1104,11 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus) > } > } > > -/* Program resources in newly inserted bridge */ > -static int acpiphp_configure_p2p_bridge(acpi_handle handle) > -{ > - struct pci_dev *pdev = acpi_get_pci_dev(handle); > - struct pci_bus *bus = pdev->subordinate; > - > - pci_dev_put(pdev); > - > - pci_bus_size_bridges(bus); > - pci_bus_assign_resources(bus); > - acpiphp_sanitize_bus(bus); > - acpiphp_set_hpp_values(bus); > - pci_enable_bridges(bus); > - return 0; > -} > - > -static void handle_p2p_bridge_insertion(acpi_handle handle, u32 type) > -{ > - struct acpi_device *device; > - > - if ((type != ACPI_NOTIFY_BUS_CHECK) && > - (type != ACPI_NOTIFY_DEVICE_CHECK)) { > - err("unexpected notification type %d\n", type); > - return; > - } > - > - if (acpi_bus_scan(handle)) { > - err("cannot add bridge to acpi list\n"); > - return; > - } > - if (acpi_bus_get_device(handle, &device)) { > - err("ACPI device object missing\n"); > - return; > - } > - if (!acpiphp_configure_p2p_bridge(handle)) > - add_p2p_bridge(handle); > - else > - err("cannot configure and start bridge\n"); > - > -} > - > /* > * ACPI event handlers > */ > > static acpi_status > -count_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) > -{ > - int *count = (int *)context; > - struct acpiphp_bridge *bridge; > - > - bridge = acpiphp_handle_to_bridge(handle); > - if (bridge) > - (*count)++; > - return AE_OK ; > -} > - > -static acpi_status > check_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) > { > struct acpiphp_bridge *bridge; > @@ -1203,8 +1132,6 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) > char objname[64]; > struct acpi_buffer buffer = { .length = sizeof(objname), > .pointer = objname }; > - struct acpi_device *device; > - int num_sub_bridges = 0; > struct acpi_hp_work *hp_work; > acpi_handle handle; > u32 type; > @@ -1212,23 +1139,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) > hp_work = container_of(work, struct acpi_hp_work, work); > handle = hp_work->handle; > type = hp_work->type; > - > - if (acpi_bus_get_device(handle, &device)) { > - /* This bridge must have just been physically inserted */ > - handle_p2p_bridge_insertion(handle, type); > - goto out; > - } According to my understanding, for acpiphp we still need to support: one slot is on root bus 0, and will take p2p bridge directly. I thought that I pointed this out when Yijing Wang or someone else pose similar patch. Thanks Yinghai ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug 2013-01-30 17:27 ` Yinghai Lu @ 2013-01-31 15:37 ` Jiang Liu 2013-01-31 15:59 ` Yinghai Lu 0 siblings, 1 reply; 11+ messages in thread From: Jiang Liu @ 2013-01-31 15:37 UTC (permalink / raw) To: Yinghai Lu Cc: Bjorn Helgaas, Rafael J . Wysocki, Jiang Liu, Yijing Wang, linux-kernel, linux-pci On 01/31/2013 01:27 AM, Yinghai Lu wrote: > On Wed, Jan 30, 2013 at 8:10 AM, Jiang Liu <liuj97@gmail.com> wrote: >> From: Jiang Liu <jiang.liu@huawei.com> >> >> Commit 668192b678201d2fff27c "PCI: acpiphp: Move host bridge hotplug >> to pci_root.c" has moved PCI host bridge hotplug logic from acpiphp >> to pci_root, but there is still PCI host bridge hotplug related >> dead code left in acpiphp. So remove those dead code. >> >> Now companion ACPI devices are always created before corresponding >> PCI devices. And the ACPI event handle_hotplug_event_bridge() will be >> installed only if it has associated PCI device. So remove dead code to >> handle bridge hot-adding in function handle_hotplug_event_bridge(). >> >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> >> --- >> drivers/pci/hotplug/acpiphp.h | 13 +--- >> drivers/pci/hotplug/acpiphp_glue.c | 124 ++++-------------------------------- >> 2 files changed, 14 insertions(+), 123 deletions(-) >> >> diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h >> index b3ead7a..b70ac00 100644 >> --- a/drivers/pci/hotplug/acpiphp.h >> +++ b/drivers/pci/hotplug/acpiphp.h >> @@ -79,7 +79,6 @@ struct acpiphp_bridge { >> /* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */ >> struct acpiphp_func *func; >> >> - int type; >> int nr_slots; >> >> u32 flags; >> @@ -146,10 +145,6 @@ struct acpiphp_attention_info >> /* PCI bus bridge HID */ >> #define ACPI_PCI_HOST_HID "PNP0A03" >> >> -/* PCI BRIDGE type */ >> -#define BRIDGE_TYPE_HOST 0 >> -#define BRIDGE_TYPE_P2P 1 >> - >> /* ACPI _STA method value (ignore bit 4; battery present) */ >> #define ACPI_STA_PRESENT (0x00000001) >> #define ACPI_STA_ENABLED (0x00000002) >> @@ -158,13 +153,7 @@ struct acpiphp_attention_info >> #define ACPI_STA_ALL (0x0000000f) >> >> /* bridge flags */ >> -#define BRIDGE_HAS_STA (0x00000001) >> -#define BRIDGE_HAS_EJ0 (0x00000002) >> -#define BRIDGE_HAS_HPP (0x00000004) >> -#define BRIDGE_HAS_PS0 (0x00000010) >> -#define BRIDGE_HAS_PS1 (0x00000020) >> -#define BRIDGE_HAS_PS2 (0x00000040) >> -#define BRIDGE_HAS_PS3 (0x00000080) >> +#define BRIDGE_HAS_EJ0 (0x00000001) >> >> /* slot flags */ >> >> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c >> index acb7af2..4681d2c 100644 >> --- a/drivers/pci/hotplug/acpiphp_glue.c >> +++ b/drivers/pci/hotplug/acpiphp_glue.c >> @@ -325,8 +325,8 @@ static void init_bridge_misc(struct acpiphp_bridge *bridge) >> return; >> } >> >> - /* install notify handler */ >> - if (bridge->type != BRIDGE_TYPE_HOST) { >> + /* install notify handler for P2P bridges */ >> + if (!pci_is_root_bus(bridge->pci_bus)) { >> if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { >> status = acpi_remove_notify_handler(bridge->func->handle, >> ACPI_SYSTEM_NOTIFY, >> @@ -369,27 +369,12 @@ static struct acpiphp_func *acpiphp_bridge_handle_to_function(acpi_handle handle >> static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge) >> { >> acpi_handle dummy_handle; >> + struct acpiphp_func *func; >> >> if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, >> - "_STA", &dummy_handle))) >> - bridge->flags |= BRIDGE_HAS_STA; >> - >> - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, >> - "_EJ0", &dummy_handle))) >> + "_EJ0", &dummy_handle))) { >> bridge->flags |= BRIDGE_HAS_EJ0; >> >> - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, >> - "_PS0", &dummy_handle))) >> - bridge->flags |= BRIDGE_HAS_PS0; >> - >> - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, >> - "_PS3", &dummy_handle))) >> - bridge->flags |= BRIDGE_HAS_PS3; >> - >> - /* is this ejectable p2p bridge? */ >> - if (bridge->flags & BRIDGE_HAS_EJ0) { >> - struct acpiphp_func *func; >> - >> dbg("found ejectable p2p bridge\n"); >> >> /* make link between PCI bridge and PCI function */ >> @@ -412,7 +397,6 @@ static void add_host_bridge(struct acpi_pci_root *root) >> if (bridge == NULL) >> return; >> >> - bridge->type = BRIDGE_TYPE_HOST; >> bridge->handle = handle; >> >> bridge->pci_bus = root->bus; >> @@ -432,7 +416,6 @@ static void add_p2p_bridge(acpi_handle *handle) >> return; >> } >> >> - bridge->type = BRIDGE_TYPE_P2P; >> bridge->handle = handle; >> config_p2p_bridge_flags(bridge); >> >> @@ -543,7 +526,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) >> acpi_status status; >> acpi_handle handle = bridge->handle; >> >> - if (bridge->type != BRIDGE_TYPE_HOST) { >> + if (!pci_is_root_bus(bridge->pci_bus)) { >> status = acpi_remove_notify_handler(handle, >> ACPI_SYSTEM_NOTIFY, >> handle_hotplug_event_bridge); >> @@ -551,8 +534,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) >> err("failed to remove notify handler\n"); >> } >> >> - if ((bridge->type != BRIDGE_TYPE_HOST) && >> - ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) { >> + if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { >> status = acpi_install_notify_handler(bridge->func->handle, >> ACPI_SYSTEM_NOTIFY, >> handle_hotplug_event_func, >> @@ -1122,64 +1104,11 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus) >> } >> } >> >> -/* Program resources in newly inserted bridge */ >> -static int acpiphp_configure_p2p_bridge(acpi_handle handle) >> -{ >> - struct pci_dev *pdev = acpi_get_pci_dev(handle); >> - struct pci_bus *bus = pdev->subordinate; >> - >> - pci_dev_put(pdev); >> - >> - pci_bus_size_bridges(bus); >> - pci_bus_assign_resources(bus); >> - acpiphp_sanitize_bus(bus); >> - acpiphp_set_hpp_values(bus); >> - pci_enable_bridges(bus); >> - return 0; >> -} >> - >> -static void handle_p2p_bridge_insertion(acpi_handle handle, u32 type) >> -{ >> - struct acpi_device *device; >> - >> - if ((type != ACPI_NOTIFY_BUS_CHECK) && >> - (type != ACPI_NOTIFY_DEVICE_CHECK)) { >> - err("unexpected notification type %d\n", type); >> - return; >> - } >> - >> - if (acpi_bus_scan(handle)) { >> - err("cannot add bridge to acpi list\n"); >> - return; >> - } >> - if (acpi_bus_get_device(handle, &device)) { >> - err("ACPI device object missing\n"); >> - return; >> - } >> - if (!acpiphp_configure_p2p_bridge(handle)) >> - add_p2p_bridge(handle); >> - else >> - err("cannot configure and start bridge\n"); >> - >> -} >> - >> /* >> * ACPI event handlers >> */ >> >> static acpi_status >> -count_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) >> -{ >> - int *count = (int *)context; >> - struct acpiphp_bridge *bridge; >> - >> - bridge = acpiphp_handle_to_bridge(handle); >> - if (bridge) >> - (*count)++; >> - return AE_OK ; >> -} >> - >> -static acpi_status >> check_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) >> { >> struct acpiphp_bridge *bridge; >> @@ -1203,8 +1132,6 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) >> char objname[64]; >> struct acpi_buffer buffer = { .length = sizeof(objname), >> .pointer = objname }; >> - struct acpi_device *device; >> - int num_sub_bridges = 0; >> struct acpi_hp_work *hp_work; >> acpi_handle handle; >> u32 type; >> @@ -1212,23 +1139,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) >> hp_work = container_of(work, struct acpi_hp_work, work); >> handle = hp_work->handle; >> type = hp_work->type; >> - >> - if (acpi_bus_get_device(handle, &device)) { >> - /* This bridge must have just been physically inserted */ >> - handle_p2p_bridge_insertion(handle, type); >> - goto out; >> - } > > According to my understanding, for acpiphp we still need to support: > > one slot is on root bus 0, and will take p2p bridge directly. Hi Yinghai, I think the acpiphp driver still supports PCI hotplug slots directly attached to PCI host bridges with the patch applied. The situations are: 1) The acpiphp driver won't install ACPI system event handler onto ACPI handles for PCI/PCIe host bridges, otherwise it will conflict with the PCI host bridge hotplug driver. 2) If there's no PCI device attached to a PCI hotplug slot or the attached PCI device is not a P2P bridge, acpiphp installs handle_hotplug_event_func() to handle ACPI system events. Otherwise handle_hotplug_event_bridge() will be installed. 3) When hot-adding a P2P bridge, acpiphp replaces handle_hotplug_event_func() with handle_hotplug_event_bridge(), so hot-removal of P2P bridge will be handled by handle_hotplug_event_bridge(). 4) When hot-removing a P2P bridge, acpi replaces handle_hotplug_event_bridge() with handle_hotplug_event_func(). So P2P bridge hot-add will always be handled by handle_hotplug_event_func() instead of handle_hotplug_event_bridge() 5) The acpiphp driver still scans for hotplug slots directly attached to PCI host bridges. So I think the code in handle_hotplug_event_bridge() to handle P2P hot-addition is dead. Yijing has helped to test the code by faking ACPI CUSTOM_METHOD, and it does work as expected. Regards! Gerry > > I thought that I pointed this out when Yijing Wang or someone else > pose similar patch. > > Thanks > > Yinghai > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug 2013-01-31 15:37 ` Jiang Liu @ 2013-01-31 15:59 ` Yinghai Lu 2013-02-01 8:50 ` Yijing Wang 0 siblings, 1 reply; 11+ messages in thread From: Yinghai Lu @ 2013-01-31 15:59 UTC (permalink / raw) To: Jiang Liu Cc: Bjorn Helgaas, Rafael J . Wysocki, Jiang Liu, Yijing Wang, linux-kernel, linux-pci On Thu, Jan 31, 2013 at 7:37 AM, Jiang Liu <liuj97@gmail.com> wrote: > > So I think the code in handle_hotplug_event_bridge() to handle P2P hot-addition > is dead. Yijing has helped to test the code by faking ACPI CUSTOM_METHOD, and it > does work as expected. ok, then. It would be better if you can append some print out from Yijing's test in the changelog. Thanks Yinghai ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug 2013-01-31 15:59 ` Yinghai Lu @ 2013-02-01 8:50 ` Yijing Wang 2013-02-02 0:01 ` Yinghai Lu 0 siblings, 1 reply; 11+ messages in thread From: Yijing Wang @ 2013-02-01 8:50 UTC (permalink / raw) To: Yinghai Lu Cc: Jiang Liu, Bjorn Helgaas, Rafael J . Wysocki, Jiang Liu, linux-kernel, linux-pci On 2013/1/31 23:59, Yinghai Lu wrote: > On Thu, Jan 31, 2013 at 7:37 AM, Jiang Liu <liuj97@gmail.com> wrote: >> >> So I think the code in handle_hotplug_event_bridge() to handle P2P hot-addition >> is dead. Yijing has helped to test the code by faking ACPI CUSTOM_METHOD, and it >> does work as expected. > > ok, then. > > It would be better if you can append some print out from Yijing's test > in the changelog. > Hi Yinghai, I used custom_method module to make root port device 0000:00:07.0 support acpi hotplug (by adding _EJ0 method), This root port device also connected to a slot which support hotplug. I tested to hot plug root port device 0000:00:07.0, and test steps and info as bellow, if there are somethings missing during test, please let me know. Thanks very much! linux-ha2:/sys/bus/pci/slots # lspci -tv -+-[0000:40]-+-00.0-[0000:41]-- | +-01.0-[0000:42]--+-00.0 Intel Corporation 82576 Gigabit Network Connection | | \-00.1 Intel Corporation 82576 Gigabit Network Connection | +-03.0-[0000:43]----00.0 LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS | +-04.0-[0000:44]-- | +-05.0-[0000:45]-- | +-07.0-[0000:46]-- | +-0d.0 Intel Corporation Device 343a | +-0d.1 Intel Corporation Device 343b | +-0d.2 Intel Corporation Device 343c | +-0d.3 Intel Corporation Device 343d | +-0d.4 Intel Corporation 5520/5500/X58 Physical Layer Port 0 | +-0d.5 Intel Corporation 5520/5500 Physical Layer Port 1 | +-0d.6 Intel Corporation Device 341a | +-0d.7 Intel Corporation Device 341b | +-0e.0 Intel Corporation Device 341c | +-0e.1 Intel Corporation Device 341d | +-0e.2 Intel Corporation Device 341e | +-0e.3 Intel Corporation Device 341f | +-0e.4 Intel Corporation Device 3439 | +-0f.0 Intel Corporation Device 3424 | +-10.0 Intel Corporation 5520/5500/X58 Physical and Link Layer Registers Port 0 | +-10.1 Intel Corporation 5520/5500/X58 Routing and Protocol Layer Registers Port 0 | +-11.0 Intel Corporation 5520/5500 Physical and Link Layer Registers Port 1 | +-11.1 Intel Corporation 5520/5500 Routing & Protocol Layer Register Port 1 | +-13.0 Intel Corporation 5520/5500/X58 I/O Hub I/OxAPIC Interrupt Controller | +-14.0 Intel Corporation 5520/5500/X58 I/O Hub System Management Registers | +-14.1 Intel Corporation 5520/5500/X58 I/O Hub GPIO and Scratch Pad Registers | +-14.2 Intel Corporation 5520/5500/X58 I/O Hub Control Status and RAS Registers | +-14.3 Intel Corporation 5520/5500/X58 I/O Hub Throttle Registers | +-15.0 Intel Corporation 5520/5500/X58 Trusted Execution Technology Registers | +-16.0 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.1 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.2 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.3 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.4 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.5 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.6 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | \-16.7 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device \-[0000:00]-+-00.0 Intel Corporation 5520/5500/X58 I/O Hub to ESI Port +-01.0-[0000:01]--+-00.0 Intel Corporation 82576 Gigabit Network Connection | \-00.1 Intel Corporation 82576 Gigabit Network Connection +-03.0-[0000:02]----00.0 LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS +-04.0-[0000:03]-- +-05.0-[0000:04]-- +-07.0-[0000:05]--+-00.0 nVidia Corporation GT218 [GeForce G210] | \-00.1 nVidia Corporation High Definition Audio Controller +-0d.0 Intel Corporation Device 343a +-0d.1 Intel Corporation Device 343b +-0d.2 Intel Corporation Device 343c +-0d.3 Intel Corporation Device 343d +-0d.4 Intel Corporation 5520/5500/X58 Physical Layer Port 0 +-0d.5 Intel Corporation 5520/5500 Physical Layer Port 1 +-0d.6 Intel Corporation Device 341a +-0d.7 Intel Corporation Device 341b +-0e.0 Intel Corporation Device 341c +-0e.1 Intel Corporation Device 341d +-0e.2 Intel Corporation Device 341e +-0e.3 Intel Corporation Device 341f +-0e.4 Intel Corporation Device 3439 +-0f.0 Intel Corporation Device 3424 +-10.0 Intel Corporation 5520/5500/X58 Physical and Link Layer Registers Port 0 +-10.1 Intel Corporation 5520/5500/X58 Routing and Protocol Layer Registers Port 0 +-11.0 Intel Corporation 5520/5500 Physical and Link Layer Registers Port 1 +-11.1 Intel Corporation 5520/5500 Routing & Protocol Layer Register Port 1 +-13.0 Intel Corporation 5520/5500/X58 I/O Hub I/OxAPIC Interrupt Controller +-14.0 Intel Corporation 5520/5500/X58 I/O Hub System Management Registers +-14.1 Intel Corporation 5520/5500/X58 I/O Hub GPIO and Scratch Pad Registers +-14.2 Intel Corporation 5520/5500/X58 I/O Hub Control Status and RAS Registers +-14.3 Intel Corporation 5520/5500/X58 I/O Hub Throttle Registers +-15.0 Intel Corporation 5520/5500/X58 Trusted Execution Technology Registers +-16.0 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.1 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.2 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.3 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.4 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.5 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.6 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.7 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-1a.0 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #4 +-1a.1 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #5 +-1a.2 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #6 +-1a.7 Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #2 +-1c.0-[0000:06]-- +-1c.1-[0000:07]-- +-1c.2-[0000:08]-- +-1c.3-[0000:09]-- +-1c.4-[0000:0a]----00.0 XGI Technology Inc. (eXtreme Graphics Innovation) Z11/Z11M +-1c.5-[0000:0b]-- +-1d.0 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #1 +-1d.1 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #2 +-1d.2 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #3 +-1d.7 Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #1 +-1e.0-[0000:0c]-- +-1f.0 Intel Corporation 82801JIB (ICH10) LPC Interface Controller +-1f.2 Intel Corporation 82801JI (ICH10 Family) 4 port SATA IDE Controller #1 +-1f.3 Intel Corporation 82801JI (ICH10 Family) SMBus Controller \-1f.5 Intel Corporation 82801JI (ICH10 Family) 2 port SATA IDE Controller #2 linux-ha2:/sys/bus/pci/slots # ls 0 0-1 1 3 5 7 linux-ha2:/sys/bus/pci/slots # cat 1/address 0000:00:07 linux-ha2:/sys/bus/pci/slots # cd 1 linux-ha2:/sys/bus/pci/slots/1 # ls adapter address attention cur_bus_speed latch max_bus_speed module power linux-ha2:/sys/bus/pci/slots/1 # cat power 1 linux-ha2:/sys/bus/pci/slots/1 # echo "\_SB_.NOD0.PCI0.MRP7 3" > /sys/kernel/debug/acpi/sci_notify linux-ha2:/sys/bus/pci/slots/1 # cat power 0 linux-ha2:/sys/bus/pci/slots/1 # lspci -tv -+-[0000:40]-+-00.0-[0000:41]-- | +-01.0-[0000:42]--+-00.0 Intel Corporation 82576 Gigabit Network Connection | | \-00.1 Intel Corporation 82576 Gigabit Network Connection | +-03.0-[0000:43]----00.0 LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS | +-04.0-[0000:44]-- | +-05.0-[0000:45]-- | +-07.0-[0000:46]-- | +-0d.0 Intel Corporation Device 343a | +-0d.1 Intel Corporation Device 343b | +-0d.2 Intel Corporation Device 343c | +-0d.3 Intel Corporation Device 343d | +-0d.4 Intel Corporation 5520/5500/X58 Physical Layer Port 0 | +-0d.5 Intel Corporation 5520/5500 Physical Layer Port 1 | +-0d.6 Intel Corporation Device 341a | +-0d.7 Intel Corporation Device 341b | +-0e.0 Intel Corporation Device 341c | +-0e.1 Intel Corporation Device 341d | +-0e.2 Intel Corporation Device 341e | +-0e.3 Intel Corporation Device 341f | +-0e.4 Intel Corporation Device 3439 | +-0f.0 Intel Corporation Device 3424 | +-10.0 Intel Corporation 5520/5500/X58 Physical and Link Layer Registers Port 0 | +-10.1 Intel Corporation 5520/5500/X58 Routing and Protocol Layer Registers Port 0 | +-11.0 Intel Corporation 5520/5500 Physical and Link Layer Registers Port 1 | +-11.1 Intel Corporation 5520/5500 Routing & Protocol Layer Register Port 1 | +-13.0 Intel Corporation 5520/5500/X58 I/O Hub I/OxAPIC Interrupt Controller | +-14.0 Intel Corporation 5520/5500/X58 I/O Hub System Management Registers | +-14.1 Intel Corporation 5520/5500/X58 I/O Hub GPIO and Scratch Pad Registers | +-14.2 Intel Corporation 5520/5500/X58 I/O Hub Control Status and RAS Registers | +-14.3 Intel Corporation 5520/5500/X58 I/O Hub Throttle Registers | +-15.0 Intel Corporation 5520/5500/X58 Trusted Execution Technology Registers | +-16.0 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.1 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.2 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.3 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.4 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.5 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.6 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | \-16.7 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device \-[0000:00]-+-00.0 Intel Corporation 5520/5500/X58 I/O Hub to ESI Port +-01.0-[0000:01]--+-00.0 Intel Corporation 82576 Gigabit Network Connection | \-00.1 Intel Corporation 82576 Gigabit Network Connection +-03.0-[0000:02]----00.0 LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS +-04.0-[0000:03]-- +-05.0-[0000:04]-- +-0d.0 Intel Corporation Device 343a +-0d.1 Intel Corporation Device 343b +-0d.2 Intel Corporation Device 343c +-0d.3 Intel Corporation Device 343d +-0d.4 Intel Corporation 5520/5500/X58 Physical Layer Port 0 +-0d.5 Intel Corporation 5520/5500 Physical Layer Port 1 +-0d.6 Intel Corporation Device 341a +-0d.7 Intel Corporation Device 341b +-0e.0 Intel Corporation Device 341c +-0e.1 Intel Corporation Device 341d +-0e.2 Intel Corporation Device 341e +-0e.3 Intel Corporation Device 341f +-0e.4 Intel Corporation Device 3439 +-0f.0 Intel Corporation Device 3424 +-10.0 Intel Corporation 5520/5500/X58 Physical and Link Layer Registers Port 0 +-10.1 Intel Corporation 5520/5500/X58 Routing and Protocol Layer Registers Port 0 +-11.0 Intel Corporation 5520/5500 Physical and Link Layer Registers Port 1 +-11.1 Intel Corporation 5520/5500 Routing & Protocol Layer Register Port 1 +-13.0 Intel Corporation 5520/5500/X58 I/O Hub I/OxAPIC Interrupt Controller +-14.0 Intel Corporation 5520/5500/X58 I/O Hub System Management Registers +-14.1 Intel Corporation 5520/5500/X58 I/O Hub GPIO and Scratch Pad Registers +-14.2 Intel Corporation 5520/5500/X58 I/O Hub Control Status and RAS Registers +-14.3 Intel Corporation 5520/5500/X58 I/O Hub Throttle Registers +-15.0 Intel Corporation 5520/5500/X58 Trusted Execution Technology Registers +-16.0 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.1 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.2 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.3 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.4 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.5 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.6 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.7 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-1a.0 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #4 +-1a.1 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #5 +-1a.2 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #6 +-1a.7 Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #2 +-1c.0-[0000:06]-- +-1c.1-[0000:07]-- +-1c.2-[0000:08]-- +-1c.3-[0000:09]-- +-1c.4-[0000:0a]----00.0 XGI Technology Inc. (eXtreme Graphics Innovation) Z11/Z11M +-1c.5-[0000:0b]-- +-1d.0 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #1 +-1d.1 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #2 +-1d.2 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #3 +-1d.7 Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #1 +-1e.0-[0000:0c]-- +-1f.0 Intel Corporation 82801JIB (ICH10) LPC Interface Controller +-1f.2 Intel Corporation 82801JI (ICH10 Family) 4 port SATA IDE Controller #1 +-1f.3 Intel Corporation 82801JI (ICH10 Family) SMBus Controller \-1f.5 Intel Corporation 82801JI (ICH10 Family) 2 port SATA IDE Controller #2 linux-ha2:/sys/bus/pci/slots/1 # echo "\_SB_.NOD0.PCI0.MRP7 1" > /sys/kernel/debug/acpi/sci_notify linux-ha2:/sys/bus/pci/slots/1 # cat power 1 linux-ha2:/sys/bus/pci/slots/1 # lspci -tv -+-[0000:40]-+-00.0-[0000:41]-- | +-01.0-[0000:42]--+-00.0 Intel Corporation 82576 Gigabit Network Connection | | \-00.1 Intel Corporation 82576 Gigabit Network Connection | +-03.0-[0000:43]----00.0 LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS | +-04.0-[0000:44]-- | +-05.0-[0000:45]-- | +-07.0-[0000:46]-- | +-0d.0 Intel Corporation Device 343a | +-0d.1 Intel Corporation Device 343b | +-0d.2 Intel Corporation Device 343c | +-0d.3 Intel Corporation Device 343d | +-0d.4 Intel Corporation 5520/5500/X58 Physical Layer Port 0 | +-0d.5 Intel Corporation 5520/5500 Physical Layer Port 1 | +-0d.6 Intel Corporation Device 341a | +-0d.7 Intel Corporation Device 341b | +-0e.0 Intel Corporation Device 341c | +-0e.1 Intel Corporation Device 341d | +-0e.2 Intel Corporation Device 341e | +-0e.3 Intel Corporation Device 341f | +-0e.4 Intel Corporation Device 3439 | +-0f.0 Intel Corporation Device 3424 | +-10.0 Intel Corporation 5520/5500/X58 Physical and Link Layer Registers Port 0 | +-10.1 Intel Corporation 5520/5500/X58 Routing and Protocol Layer Registers Port 0 | +-11.0 Intel Corporation 5520/5500 Physical and Link Layer Registers Port 1 | +-11.1 Intel Corporation 5520/5500 Routing & Protocol Layer Register Port 1 | +-13.0 Intel Corporation 5520/5500/X58 I/O Hub I/OxAPIC Interrupt Controller | +-14.0 Intel Corporation 5520/5500/X58 I/O Hub System Management Registers | +-14.1 Intel Corporation 5520/5500/X58 I/O Hub GPIO and Scratch Pad Registers | +-14.2 Intel Corporation 5520/5500/X58 I/O Hub Control Status and RAS Registers | +-14.3 Intel Corporation 5520/5500/X58 I/O Hub Throttle Registers | +-15.0 Intel Corporation 5520/5500/X58 Trusted Execution Technology Registers | +-16.0 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.1 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.2 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.3 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.4 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.5 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | +-16.6 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device | \-16.7 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device \-[0000:00]-+-00.0 Intel Corporation 5520/5500/X58 I/O Hub to ESI Port +-01.0-[0000:01]--+-00.0 Intel Corporation 82576 Gigabit Network Connection | \-00.1 Intel Corporation 82576 Gigabit Network Connection +-03.0-[0000:02]----00.0 LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS +-04.0-[0000:03]-- +-05.0-[0000:04]-- +-07.0-[0000:05]--+-00.0 nVidia Corporation GT218 [GeForce G210] | \-00.1 nVidia Corporation High Definition Audio Controller +-0d.0 Intel Corporation Device 343a +-0d.1 Intel Corporation Device 343b +-0d.2 Intel Corporation Device 343c +-0d.3 Intel Corporation Device 343d +-0d.4 Intel Corporation 5520/5500/X58 Physical Layer Port 0 +-0d.5 Intel Corporation 5520/5500 Physical Layer Port 1 +-0d.6 Intel Corporation Device 341a +-0d.7 Intel Corporation Device 341b +-0e.0 Intel Corporation Device 341c +-0e.1 Intel Corporation Device 341d +-0e.2 Intel Corporation Device 341e +-0e.3 Intel Corporation Device 341f +-0e.4 Intel Corporation Device 3439 +-0f.0 Intel Corporation Device 3424 +-10.0 Intel Corporation 5520/5500/X58 Physical and Link Layer Registers Port 0 +-10.1 Intel Corporation 5520/5500/X58 Routing and Protocol Layer Registers Port 0 +-11.0 Intel Corporation 5520/5500 Physical and Link Layer Registers Port 1 +-11.1 Intel Corporation 5520/5500 Routing & Protocol Layer Register Port 1 +-13.0 Intel Corporation 5520/5500/X58 I/O Hub I/OxAPIC Interrupt Controller +-14.0 Intel Corporation 5520/5500/X58 I/O Hub System Management Registers +-14.1 Intel Corporation 5520/5500/X58 I/O Hub GPIO and Scratch Pad Registers +-14.2 Intel Corporation 5520/5500/X58 I/O Hub Control Status and RAS Registers +-14.3 Intel Corporation 5520/5500/X58 I/O Hub Throttle Registers +-15.0 Intel Corporation 5520/5500/X58 Trusted Execution Technology Registers +-16.0 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.1 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.2 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.3 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.4 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.5 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.6 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-16.7 Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device +-1a.0 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #4 +-1a.1 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #5 +-1a.2 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #6 +-1a.7 Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #2 +-1c.0-[0000:06]-- +-1c.1-[0000:07]-- +-1c.2-[0000:08]-- +-1c.3-[0000:09]-- +-1c.4-[0000:0a]----00.0 XGI Technology Inc. (eXtreme Graphics Innovation) Z11/Z11M +-1c.5-[0000:0b]-- +-1d.0 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #1 +-1d.1 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #2 +-1d.2 Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #3 +-1d.7 Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #1 +-1e.0-[0000:0c]-- +-1f.0 Intel Corporation 82801JIB (ICH10) LPC Interface Controller +-1f.2 Intel Corporation 82801JI (ICH10 Family) 4 port SATA IDE Controller #1 +-1f.3 Intel Corporation 82801JI (ICH10 Family) SMBus Controller \-1f.5 Intel Corporation 82801JI (ICH10 Family) 2 port SATA IDE Controller #2 > Thanks > > Yinghai > > -- Thanks! Yijing ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug 2013-02-01 8:50 ` Yijing Wang @ 2013-02-02 0:01 ` Yinghai Lu 0 siblings, 0 replies; 11+ messages in thread From: Yinghai Lu @ 2013-02-02 0:01 UTC (permalink / raw) To: Yijing Wang Cc: Jiang Liu, Bjorn Helgaas, Rafael J . Wysocki, Jiang Liu, linux-kernel, linux-pci On Fri, Feb 1, 2013 at 12:50 AM, Yijing Wang <wangyijing@huawei.com> wrote: > On 2013/1/31 23:59, Yinghai Lu wrote: >> On Thu, Jan 31, 2013 at 7:37 AM, Jiang Liu <liuj97@gmail.com> wrote: >>> >>> So I think the code in handle_hotplug_event_bridge() to handle P2P hot-addition >>> is dead. Yijing has helped to test the code by faking ACPI CUSTOM_METHOD, and it >>> does work as expected. >> >> ok, then. >> >> It would be better if you can append some print out from Yijing's test >> in the changelog. >> > > Hi Yinghai, > I used custom_method module to make root port device 0000:00:07.0 support acpi hotplug (by adding _EJ0 method), > This root port device also connected to a slot which support hotplug. I tested to hot plug root port device 0000:00:07.0, > and test steps and info as bellow, if there are somethings missing during test, please let me know. > Thanks very much! > > linux-ha2:/sys/bus/pci/slots # lspci -tv > \-[0000:00]-+-00.0 Intel Corporation 5520/5500/X58 I/O Hub to ESI Port > +-01.0-[0000:01]--+-00.0 Intel Corporation 82576 Gigabit Network Connection > | \-00.1 Intel Corporation 82576 Gigabit Network Connection > +-03.0-[0000:02]----00.0 LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS > +-04.0-[0000:03]-- > +-05.0-[0000:04]-- > +-07.0-[0000:05]--+-00.0 nVidia Corporation GT218 [GeForce G210] > | \-00.1 nVidia Corporation High Definition Audio Controller > +-0d.0 Intel Corporation Device 343a > linux-ha2:/sys/bus/pci/slots # ls > 0 0-1 1 3 5 7 > linux-ha2:/sys/bus/pci/slots # cat 1/address > 0000:00:07 > linux-ha2:/sys/bus/pci/slots # cd 1 > linux-ha2:/sys/bus/pci/slots/1 # ls > adapter address attention cur_bus_speed latch max_bus_speed module power > linux-ha2:/sys/bus/pci/slots/1 # cat power > 1 > linux-ha2:/sys/bus/pci/slots/1 # echo "\_SB_.NOD0.PCI0.MRP7 3" > /sys/kernel/debug/acpi/sci_notify > linux-ha2:/sys/bus/pci/slots/1 # cat power > 0 > linux-ha2:/sys/bus/pci/slots/1 # lspci -tv > \-[0000:00]-+-00.0 Intel Corporation 5520/5500/X58 I/O Hub to ESI Port > +-01.0-[0000:01]--+-00.0 Intel Corporation 82576 Gigabit Network Connection > | \-00.1 Intel Corporation 82576 Gigabit Network Connection > +-03.0-[0000:02]----00.0 LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS > +-04.0-[0000:03]-- > +-05.0-[0000:04]-- > +-0d.0 Intel Corporation Device 343a > linux-ha2:/sys/bus/pci/slots/1 # echo "\_SB_.NOD0.PCI0.MRP7 1" > /sys/kernel/debug/acpi/sci_notify > linux-ha2:/sys/bus/pci/slots/1 # cat power > 1 > linux-ha2:/sys/bus/pci/slots/1 # lspci -tv > \-[0000:00]-+-00.0 Intel Corporation 5520/5500/X58 I/O Hub to ESI Port > +-01.0-[0000:01]--+-00.0 Intel Corporation 82576 Gigabit Network Connection > | \-00.1 Intel Corporation 82576 Gigabit Network Connection > +-03.0-[0000:02]----00.0 LSI Logic / Symbios Logic SAS1064ET PCI-Express Fusion-MPT SAS > +-04.0-[0000:03]-- > +-05.0-[0000:04]-- > +-07.0-[0000:05]--+-00.0 nVidia Corporation GT218 [GeForce G210] > | \-00.1 nVidia Corporation High Definition Audio Controller > +-0d.0 Intel Corporation Device 343a looks good. Thanks ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug 2013-01-30 16:10 ` [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug Jiang Liu 2013-01-30 17:27 ` Yinghai Lu @ 2013-02-01 23:18 ` Bjorn Helgaas 1 sibling, 0 replies; 11+ messages in thread From: Bjorn Helgaas @ 2013-02-01 23:18 UTC (permalink / raw) To: Jiang Liu Cc: Yinghai Lu, Rafael J . Wysocki, Jiang Liu, Yijing Wang, linux-kernel, linux-pci On Wed, Jan 30, 2013 at 9:10 AM, Jiang Liu <liuj97@gmail.com> wrote: > From: Jiang Liu <jiang.liu@huawei.com> > > Commit 668192b678201d2fff27c "PCI: acpiphp: Move host bridge hotplug > to pci_root.c" has moved PCI host bridge hotplug logic from acpiphp > to pci_root, but there is still PCI host bridge hotplug related > dead code left in acpiphp. So remove those dead code. > > Now companion ACPI devices are always created before corresponding > PCI devices. And the ACPI event handle_hotplug_event_bridge() will be > installed only if it has associated PCI device. So remove dead code to > handle bridge hot-adding in function handle_hotplug_event_bridge(). > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> Applied to pci/yinghai-root-bus-hotplug for v3.9. Thanks! > --- > drivers/pci/hotplug/acpiphp.h | 13 +--- > drivers/pci/hotplug/acpiphp_glue.c | 124 ++++-------------------------------- > 2 files changed, 14 insertions(+), 123 deletions(-) > > diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h > index b3ead7a..b70ac00 100644 > --- a/drivers/pci/hotplug/acpiphp.h > +++ b/drivers/pci/hotplug/acpiphp.h > @@ -79,7 +79,6 @@ struct acpiphp_bridge { > /* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */ > struct acpiphp_func *func; > > - int type; > int nr_slots; > > u32 flags; > @@ -146,10 +145,6 @@ struct acpiphp_attention_info > /* PCI bus bridge HID */ > #define ACPI_PCI_HOST_HID "PNP0A03" > > -/* PCI BRIDGE type */ > -#define BRIDGE_TYPE_HOST 0 > -#define BRIDGE_TYPE_P2P 1 > - > /* ACPI _STA method value (ignore bit 4; battery present) */ > #define ACPI_STA_PRESENT (0x00000001) > #define ACPI_STA_ENABLED (0x00000002) > @@ -158,13 +153,7 @@ struct acpiphp_attention_info > #define ACPI_STA_ALL (0x0000000f) > > /* bridge flags */ > -#define BRIDGE_HAS_STA (0x00000001) > -#define BRIDGE_HAS_EJ0 (0x00000002) > -#define BRIDGE_HAS_HPP (0x00000004) > -#define BRIDGE_HAS_PS0 (0x00000010) > -#define BRIDGE_HAS_PS1 (0x00000020) > -#define BRIDGE_HAS_PS2 (0x00000040) > -#define BRIDGE_HAS_PS3 (0x00000080) > +#define BRIDGE_HAS_EJ0 (0x00000001) > > /* slot flags */ > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c > index acb7af2..4681d2c 100644 > --- a/drivers/pci/hotplug/acpiphp_glue.c > +++ b/drivers/pci/hotplug/acpiphp_glue.c > @@ -325,8 +325,8 @@ static void init_bridge_misc(struct acpiphp_bridge *bridge) > return; > } > > - /* install notify handler */ > - if (bridge->type != BRIDGE_TYPE_HOST) { > + /* install notify handler for P2P bridges */ > + if (!pci_is_root_bus(bridge->pci_bus)) { > if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { > status = acpi_remove_notify_handler(bridge->func->handle, > ACPI_SYSTEM_NOTIFY, > @@ -369,27 +369,12 @@ static struct acpiphp_func *acpiphp_bridge_handle_to_function(acpi_handle handle > static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge) > { > acpi_handle dummy_handle; > + struct acpiphp_func *func; > > if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, > - "_STA", &dummy_handle))) > - bridge->flags |= BRIDGE_HAS_STA; > - > - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, > - "_EJ0", &dummy_handle))) > + "_EJ0", &dummy_handle))) { > bridge->flags |= BRIDGE_HAS_EJ0; > > - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, > - "_PS0", &dummy_handle))) > - bridge->flags |= BRIDGE_HAS_PS0; > - > - if (ACPI_SUCCESS(acpi_get_handle(bridge->handle, > - "_PS3", &dummy_handle))) > - bridge->flags |= BRIDGE_HAS_PS3; > - > - /* is this ejectable p2p bridge? */ > - if (bridge->flags & BRIDGE_HAS_EJ0) { > - struct acpiphp_func *func; > - > dbg("found ejectable p2p bridge\n"); > > /* make link between PCI bridge and PCI function */ > @@ -412,7 +397,6 @@ static void add_host_bridge(struct acpi_pci_root *root) > if (bridge == NULL) > return; > > - bridge->type = BRIDGE_TYPE_HOST; > bridge->handle = handle; > > bridge->pci_bus = root->bus; > @@ -432,7 +416,6 @@ static void add_p2p_bridge(acpi_handle *handle) > return; > } > > - bridge->type = BRIDGE_TYPE_P2P; > bridge->handle = handle; > config_p2p_bridge_flags(bridge); > > @@ -543,7 +526,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) > acpi_status status; > acpi_handle handle = bridge->handle; > > - if (bridge->type != BRIDGE_TYPE_HOST) { > + if (!pci_is_root_bus(bridge->pci_bus)) { > status = acpi_remove_notify_handler(handle, > ACPI_SYSTEM_NOTIFY, > handle_hotplug_event_bridge); > @@ -551,8 +534,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) > err("failed to remove notify handler\n"); > } > > - if ((bridge->type != BRIDGE_TYPE_HOST) && > - ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) { > + if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { > status = acpi_install_notify_handler(bridge->func->handle, > ACPI_SYSTEM_NOTIFY, > handle_hotplug_event_func, > @@ -1122,64 +1104,11 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus) > } > } > > -/* Program resources in newly inserted bridge */ > -static int acpiphp_configure_p2p_bridge(acpi_handle handle) > -{ > - struct pci_dev *pdev = acpi_get_pci_dev(handle); > - struct pci_bus *bus = pdev->subordinate; > - > - pci_dev_put(pdev); > - > - pci_bus_size_bridges(bus); > - pci_bus_assign_resources(bus); > - acpiphp_sanitize_bus(bus); > - acpiphp_set_hpp_values(bus); > - pci_enable_bridges(bus); > - return 0; > -} > - > -static void handle_p2p_bridge_insertion(acpi_handle handle, u32 type) > -{ > - struct acpi_device *device; > - > - if ((type != ACPI_NOTIFY_BUS_CHECK) && > - (type != ACPI_NOTIFY_DEVICE_CHECK)) { > - err("unexpected notification type %d\n", type); > - return; > - } > - > - if (acpi_bus_scan(handle)) { > - err("cannot add bridge to acpi list\n"); > - return; > - } > - if (acpi_bus_get_device(handle, &device)) { > - err("ACPI device object missing\n"); > - return; > - } > - if (!acpiphp_configure_p2p_bridge(handle)) > - add_p2p_bridge(handle); > - else > - err("cannot configure and start bridge\n"); > - > -} > - > /* > * ACPI event handlers > */ > > static acpi_status > -count_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) > -{ > - int *count = (int *)context; > - struct acpiphp_bridge *bridge; > - > - bridge = acpiphp_handle_to_bridge(handle); > - if (bridge) > - (*count)++; > - return AE_OK ; > -} > - > -static acpi_status > check_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) > { > struct acpiphp_bridge *bridge; > @@ -1203,8 +1132,6 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) > char objname[64]; > struct acpi_buffer buffer = { .length = sizeof(objname), > .pointer = objname }; > - struct acpi_device *device; > - int num_sub_bridges = 0; > struct acpi_hp_work *hp_work; > acpi_handle handle; > u32 type; > @@ -1212,23 +1139,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) > hp_work = container_of(work, struct acpi_hp_work, work); > handle = hp_work->handle; > type = hp_work->type; > - > - if (acpi_bus_get_device(handle, &device)) { > - /* This bridge must have just been physically inserted */ > - handle_p2p_bridge_insertion(handle, type); > - goto out; > - } > - > - bridge = acpiphp_handle_to_bridge(handle); > - if (type == ACPI_NOTIFY_BUS_CHECK) { > - acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, ACPI_UINT32_MAX, > - count_sub_bridges, NULL, &num_sub_bridges, NULL); > - } > - > - if (!bridge && !num_sub_bridges) { > - err("cannot get bridge info\n"); > - goto out; > - } > + bridge = (struct acpiphp_bridge *)hp_work->context; > > acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); > > @@ -1236,14 +1147,10 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) > case ACPI_NOTIFY_BUS_CHECK: > /* bus re-enumerate */ > dbg("%s: Bus check notify on %s\n", __func__, objname); > - if (bridge) { > - dbg("%s: re-enumerating slots under %s\n", > - __func__, objname); > - acpiphp_check_bridge(bridge); > - } > - if (num_sub_bridges) > - acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, > - ACPI_UINT32_MAX, check_sub_bridges, NULL, NULL, NULL); > + dbg("%s: re-enumerating slots under %s\n", __func__, objname); > + acpiphp_check_bridge(bridge); > + acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, > + ACPI_UINT32_MAX, check_sub_bridges, NULL, NULL, NULL); > break; > > case ACPI_NOTIFY_DEVICE_CHECK: > @@ -1260,8 +1167,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) > case ACPI_NOTIFY_EJECT_REQUEST: > /* request device eject */ > dbg("%s: Device eject notify on %s\n", __func__, objname); > - if ((bridge->type != BRIDGE_TYPE_HOST) && > - (bridge->flags & BRIDGE_HAS_EJ0)) { > + if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { > struct acpiphp_slot *slot; > slot = bridge->func->slot; > if (!acpiphp_disable_slot(slot)) > @@ -1289,7 +1195,6 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) > break; > } > > -out: > kfree(hp_work); /* allocated in handle_hotplug_event_bridge */ > } > > @@ -1324,17 +1229,14 @@ static void _handle_hotplug_event_func(struct work_struct *work) > struct acpi_hp_work *hp_work; > acpi_handle handle; > u32 type; > - void *context; > > hp_work = container_of(work, struct acpi_hp_work, work); > handle = hp_work->handle; > type = hp_work->type; > - context = hp_work->context; > + func = (struct acpiphp_func *)hp_work->context; > > acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); > > - func = (struct acpiphp_func *)context; > - > switch (type) { > case ACPI_NOTIFY_BUS_CHECK: > /* bus re-enumerate */ > -- > 1.7.9.5 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices 2013-01-30 16:10 [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices Jiang Liu 2013-01-30 16:10 ` [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug Jiang Liu @ 2013-02-01 23:06 ` Bjorn Helgaas 2013-02-01 23:17 ` Bjorn Helgaas 2013-02-02 2:24 ` Yijing Wang 1 sibling, 2 replies; 11+ messages in thread From: Bjorn Helgaas @ 2013-02-01 23:06 UTC (permalink / raw) To: Jiang Liu Cc: Yinghai Lu, Rafael J . Wysocki, Jiang Liu, Yijing Wang, linux-kernel, linux-pci On Wed, Jan 30, 2013 at 9:10 AM, Jiang Liu <liuj97@gmail.com> wrote: > From: Jiang Liu <jiang.liu@huawei.com> > > With commit 4f535093cf8f6da8c "PCI: Put pci_dev in device tree as > early as possible", companion ACPI devices should be created before > creating correspoding PCI devices, otherwise it will break the ACPI > PCI binding logic. > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > --- > Hi Bjorn, > This patch set applies to your pci/yinghai-root-bus branch. > There are still other potential bugs in the acpiphp driver under > investigation. So I will send out these two first to catch up with > the 3.9 merging window. > Thanks! > Gerry I assume this fixes some sort of user-visible issue with acpiphp. What failure does the user see? > --- > drivers/pci/hotplug/acpiphp_glue.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c > index bd784ff..acb7af2 100644 > --- a/drivers/pci/hotplug/acpiphp_glue.c > +++ b/drivers/pci/hotplug/acpiphp_glue.c > @@ -839,6 +839,9 @@ static int __ref enable_device(struct acpiphp_slot *slot) > if (slot->flags & SLOT_ENABLED) > goto err_exit; > > + list_for_each_entry(func, &slot->funcs, sibling) > + acpiphp_bus_add(func); > + > num = pci_scan_slot(bus, PCI_DEVFN(slot->device, 0)); > if (num == 0) { > /* Maybe only part of funcs are added. */ > @@ -862,9 +865,6 @@ static int __ref enable_device(struct acpiphp_slot *slot) > } > } > > - list_for_each_entry(func, &slot->funcs, sibling) > - acpiphp_bus_add(func); > - > pci_bus_assign_resources(bus); > acpiphp_sanitize_bus(bus); > acpiphp_set_hpp_values(bus); > -- > 1.7.9.5 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices 2013-02-01 23:06 ` [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices Bjorn Helgaas @ 2013-02-01 23:17 ` Bjorn Helgaas 2013-02-02 2:24 ` Yijing Wang 1 sibling, 0 replies; 11+ messages in thread From: Bjorn Helgaas @ 2013-02-01 23:17 UTC (permalink / raw) To: Jiang Liu Cc: Yinghai Lu, Rafael J . Wysocki, Jiang Liu, Yijing Wang, linux-kernel, linux-pci On Fri, Feb 1, 2013 at 4:06 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Wed, Jan 30, 2013 at 9:10 AM, Jiang Liu <liuj97@gmail.com> wrote: >> From: Jiang Liu <jiang.liu@huawei.com> >> >> With commit 4f535093cf8f6da8c "PCI: Put pci_dev in device tree as >> early as possible", companion ACPI devices should be created before >> creating correspoding PCI devices, otherwise it will break the ACPI >> PCI binding logic. >> >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> Applied to pci/yinghai-root-bus for v3.9. Thanks! >> --- >> Hi Bjorn, >> This patch set applies to your pci/yinghai-root-bus branch. >> There are still other potential bugs in the acpiphp driver under >> investigation. So I will send out these two first to catch up with >> the 3.9 merging window. >> Thanks! >> Gerry > > I assume this fixes some sort of user-visible issue with acpiphp. > What failure does the user see? > >> --- >> drivers/pci/hotplug/acpiphp_glue.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c >> index bd784ff..acb7af2 100644 >> --- a/drivers/pci/hotplug/acpiphp_glue.c >> +++ b/drivers/pci/hotplug/acpiphp_glue.c >> @@ -839,6 +839,9 @@ static int __ref enable_device(struct acpiphp_slot *slot) >> if (slot->flags & SLOT_ENABLED) >> goto err_exit; >> >> + list_for_each_entry(func, &slot->funcs, sibling) >> + acpiphp_bus_add(func); >> + >> num = pci_scan_slot(bus, PCI_DEVFN(slot->device, 0)); >> if (num == 0) { >> /* Maybe only part of funcs are added. */ >> @@ -862,9 +865,6 @@ static int __ref enable_device(struct acpiphp_slot *slot) >> } >> } >> >> - list_for_each_entry(func, &slot->funcs, sibling) >> - acpiphp_bus_add(func); >> - >> pci_bus_assign_resources(bus); >> acpiphp_sanitize_bus(bus); >> acpiphp_set_hpp_values(bus); >> -- >> 1.7.9.5 >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices 2013-02-01 23:06 ` [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices Bjorn Helgaas 2013-02-01 23:17 ` Bjorn Helgaas @ 2013-02-02 2:24 ` Yijing Wang 1 sibling, 0 replies; 11+ messages in thread From: Yijing Wang @ 2013-02-02 2:24 UTC (permalink / raw) To: Bjorn Helgaas Cc: Jiang Liu, Yinghai Lu, Rafael J . Wysocki, Jiang Liu, linux-kernel, linux-pci On 2013/2/2 7:06, Bjorn Helgaas wrote: > On Wed, Jan 30, 2013 at 9:10 AM, Jiang Liu <liuj97@gmail.com> wrote: >> From: Jiang Liu <jiang.liu@huawei.com> >> >> With commit 4f535093cf8f6da8c "PCI: Put pci_dev in device tree as >> early as possible", companion ACPI devices should be created before >> creating correspoding PCI devices, otherwise it will break the ACPI >> PCI binding logic. >> >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> >> --- >> Hi Bjorn, >> This patch set applies to your pci/yinghai-root-bus branch. >> There are still other potential bugs in the acpiphp driver under >> investigation. So I will send out these two first to catch up with >> the 3.9 merging window. >> Thanks! >> Gerry > > I assume this fixes some sort of user-visible issue with acpiphp. > What failure does the user see? Hi Bjorn, Before applied this patch, after hot remove and hot add pci device by acpiphp, we can find firmware_node under the pci device was missed. Detailed info as bellow: linux-ha2:/sys/bus/pci/slots # ls /sys/bus/pci/devices/0000\:05\:00.0/ -al total 0 drwxr-xr-x 3 root root 0 Jan 31 10:19 . drwxr-xr-x 9 root root 0 Jan 31 10:19 .. -r--r--r-- 1 root root 65536 Jan 31 10:31 boot_vga -rw-r--r-- 1 root root 65536 Jan 31 10:31 broken_parity_status -r--r--r-- 1 root root 65536 Jan 31 10:19 class -rw-r--r-- 1 root root 4096 Jan 31 10:31 config -r--r--r-- 1 root root 65536 Jan 31 10:31 consistent_dma_mask_bits -rw-r--r-- 1 root root 65536 Jan 31 10:31 d3cold_allowed -r--r--r-- 1 root root 65536 Jan 31 10:19 device -r--r--r-- 1 root root 65536 Jan 31 10:31 dma_mask_bits -rw------- 1 root root 65536 Jan 31 10:31 enable lrwxrwxrwx 1 root root 0 Jan 31 10:31 firmware_node -> ../../../LNXSYSTM:00/device:00/ACPI0004:00/PNP0A08:00/device:07/device:08 -r--r--r-- 1 root root 65536 Jan 31 10:19 irq -r--r--r-- 1 root root 65536 Jan 31 10:31 local_cpulist -r--r--r-- 1 root root 65536 Jan 31 10:20 local_cpus -r--r--r-- 1 root root 65536 Jan 31 10:31 modalias -rw-r--r-- 1 root root 65536 Jan 31 10:31 msi_bus -r--r--r-- 1 root root 65536 Jan 31 10:31 numa_node drwxr-xr-x 2 root root 0 Jan 31 10:31 power --w--w---- 1 root root 65536 Jan 31 10:31 remove --w--w---- 1 root root 65536 Jan 31 10:31 rescan -r--r--r-- 1 root root 65536 Jan 31 10:19 resource -rw------- 1 root root 16777216 Jan 31 10:31 resource0 -rw------- 1 root root 268435456 Jan 31 10:31 resource1 -rw------- 1 root root 268435456 Jan 31 10:31 resource1_wc -rw------- 1 root root 33554432 Jan 31 10:31 resource3 -rw------- 1 root root 33554432 Jan 31 10:31 resource3_wc -rw------- 1 root root 128 Jan 31 10:31 resource5 -rw------- 1 root root 524288 Jan 31 10:31 rom lrwxrwxrwx 1 root root 0 Jan 31 10:19 subsystem -> ../../../../bus/pci -r--r--r-- 1 root root 65536 Jan 31 10:19 subsystem_device -r--r--r-- 1 root root 65536 Jan 31 10:19 subsystem_vendor -rw-r--r-- 1 root root 65536 Jan 31 10:19 uevent -r--r--r-- 1 root root 65536 Jan 31 10:19 vendor linux-ha2:/sys/bus/pci/slots # cd 0 linux-ha2:/sys/bus/pci/slots/0 # cat address 0000:05:00 linux-ha2:/sys/bus/pci/slots/0 # cat power 1 linux-ha2:/sys/bus/pci/slots/0 # echo 0 > power linux-ha2:/sys/bus/pci/slots/0 # echo 1 > power linux-ha2:/sys/bus/pci/slots/0 # ls /sys/bus/pci/devices/0000\:05\:00.0/ -al total 0 drwxr-xr-x 3 root root 0 Jan 31 10:31 . drwxr-xr-x 9 root root 0 Jan 31 10:19 .. -r--r--r-- 1 root root 65536 Jan 31 10:31 boot_vga -rw-r--r-- 1 root root 65536 Jan 31 10:31 broken_parity_status -r--r--r-- 1 root root 65536 Jan 31 10:31 class -rw-r--r-- 1 root root 4096 Jan 31 10:31 config -r--r--r-- 1 root root 65536 Jan 31 10:31 consistent_dma_mask_bits -rw-r--r-- 1 root root 65536 Jan 31 10:31 d3cold_allowed -r--r--r-- 1 root root 65536 Jan 31 10:31 device -r--r--r-- 1 root root 65536 Jan 31 10:31 dma_mask_bits -rw------- 1 root root 65536 Jan 31 10:31 enable -r--r--r-- 1 root root 65536 Jan 31 10:31 irq -r--r--r-- 1 root root 65536 Jan 31 10:31 local_cpulist -r--r--r-- 1 root root 65536 Jan 31 10:31 local_cpus -r--r--r-- 1 root root 65536 Jan 31 10:31 modalias -rw-r--r-- 1 root root 65536 Jan 31 10:31 msi_bus -r--r--r-- 1 root root 65536 Jan 31 10:31 numa_node drwxr-xr-x 2 root root 0 Jan 31 10:31 power --w--w---- 1 root root 65536 Jan 31 10:31 remove --w--w---- 1 root root 65536 Jan 31 10:31 rescan -r--r--r-- 1 root root 65536 Jan 31 10:31 resource -rw------- 1 root root 16777216 Jan 31 10:31 resource0 -rw------- 1 root root 268435456 Jan 31 10:31 resource1 -rw------- 1 root root 268435456 Jan 31 10:31 resource1_wc -rw------- 1 root root 33554432 Jan 31 10:31 resource3 -rw------- 1 root root 33554432 Jan 31 10:31 resource3_wc -rw------- 1 root root 128 Jan 31 10:31 resource5 -rw------- 1 root root 524288 Jan 31 10:31 rom lrwxrwxrwx 1 root root 0 Jan 31 10:31 subsystem -> ../../../../bus/pci -r--r--r-- 1 root root 65536 Jan 31 10:31 subsystem_device -r--r--r-- 1 root root 65536 Jan 31 10:31 subsystem_vendor -rw-r--r-- 1 root root 65536 Jan 31 10:31 uevent -r--r--r-- 1 root root 65536 Jan 31 10:31 vendor linux-ha2:/sys/bus/pci/slots/0 # So after hot add pci device again, we lost the firmware_node link under device. firmware_node -> ../../../LNXSYSTM:00/device:00/ACPI0004:00/PNP0A08:00/device:07/device:08 > >> --- >> drivers/pci/hotplug/acpiphp_glue.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c >> index bd784ff..acb7af2 100644 >> --- a/drivers/pci/hotplug/acpiphp_glue.c >> +++ b/drivers/pci/hotplug/acpiphp_glue.c >> @@ -839,6 +839,9 @@ static int __ref enable_device(struct acpiphp_slot *slot) >> if (slot->flags & SLOT_ENABLED) >> goto err_exit; >> >> + list_for_each_entry(func, &slot->funcs, sibling) >> + acpiphp_bus_add(func); >> + >> num = pci_scan_slot(bus, PCI_DEVFN(slot->device, 0)); >> if (num == 0) { >> /* Maybe only part of funcs are added. */ >> @@ -862,9 +865,6 @@ static int __ref enable_device(struct acpiphp_slot *slot) >> } >> } >> >> - list_for_each_entry(func, &slot->funcs, sibling) >> - acpiphp_bus_add(func); >> - >> pci_bus_assign_resources(bus); >> acpiphp_sanitize_bus(bus); >> acpiphp_set_hpp_values(bus); >> -- >> 1.7.9.5 >> > > -- Thanks! Yijing ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-02-02 2:25 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-01-30 16:10 [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices Jiang Liu 2013-01-30 16:10 ` [PATCH 2/2] acpiphp: remove dead code for PCI host bridge hotplug Jiang Liu 2013-01-30 17:27 ` Yinghai Lu 2013-01-31 15:37 ` Jiang Liu 2013-01-31 15:59 ` Yinghai Lu 2013-02-01 8:50 ` Yijing Wang 2013-02-02 0:01 ` Yinghai Lu 2013-02-01 23:18 ` Bjorn Helgaas 2013-02-01 23:06 ` [PATCH 1/2] acpiphp: create companion ACPI devices before creating PCI devices Bjorn Helgaas 2013-02-01 23:17 ` Bjorn Helgaas 2013-02-02 2:24 ` Yijing Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).