On 01/31/2013 01:27 AM, Yinghai Lu wrote: > On Wed, Jan 30, 2013 at 8:10 AM, Jiang Liu <liuj97@xxxxxxxxx> wrote: >> From: Jiang Liu <jiang.liu@xxxxxxxxxx> >> >> 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@xxxxxxxxxx> >> --- >> 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html