| Message ID | 1357944049-29620-10-git-send-email-yinghai@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Friday, January 11, 2013 02:40:36 PM Yinghai Lu wrote: > Add missing hot_remove support for root device. > > How to test it? > 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 > > -v2: separate stop and remove, so it will be safe for comingi > acpi_pci_bind_notify() changes. > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > Cc: Len Brown <lenb@kernel.org> > Cc: linux-acpi@vger.kernel.org > --- > drivers/acpi/pci_root.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 67 insertions(+) > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index 5c1f462c..5ae36d8 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -740,6 +740,12 @@ static void add_acpi_root_bridge(acpi_handle handle) > list_add(&bridge->list, &acpi_root_bridge_list); > } > > +static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge) > +{ > + list_del(&bridge->list); > + kfree(bridge); > +} > + > struct acpi_root_hp_work { > struct work_struct work; > acpi_handle handle; > @@ -800,6 +806,61 @@ static void handle_root_bridge_insertion(acpi_handle handle) > printk(KERN_ERR "cannot add bridge to acpi list\n"); > } > > +static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val) > +{ > + acpi_status status; > + struct acpi_object_list arg_list; > + union acpi_object arg; > + > + arg_list.count = 1; > + arg_list.pointer = &arg; > + arg.type = ACPI_TYPE_INTEGER; > + arg.integer.value = val; > + > + status = acpi_evaluate_object(handle, cmd, &arg_list, NULL); > + if (ACPI_FAILURE(status)) { > + pr_warn("%s: %s to %d failed\n", > + __func__, cmd, val); > + return -1; Please use a meaningful error code. > + } > + > + return 0; > +} > + > +static void handle_root_bridge_removal(acpi_handle handle, > + struct acpi_root_bridge *bridge) > +{ > + u32 flags = 0; > + struct acpi_device *device; > + > + if (bridge) { > + flags = bridge->flags; > + remove_acpi_root_bridge(bridge); > + } > + > + if (!acpi_bus_get_device(handle, &device)) { > + int ret_val; > + > + /* remove pci devices at first */ > + ret_val = acpi_bus_trim(device, 0); > + printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val); > + > + /* remove acpi devices */ > + ret_val = acpi_bus_trim(device, 1); > + printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", ret_val); First of all, I don't agree with the way acpi_bus_trim() is used here, as I said in the previous message. Second, this code duplicates the code you're adding on [08/22] almost exactly. Please put it into a one separate function instead of duplicating it like this. > + } > + > + if (flags & ROOT_BRIDGE_HAS_PS3) { > + acpi_status status; > + > + status = acpi_evaluate_object(handle, "_PS3", NULL, NULL); > + if (ACPI_FAILURE(status)) > + pr_warn("%s: _PS3 failed\n", __func__); No, please. acpi_device_set_power() is for that. > + } > + if (flags & ROOT_BRIDGE_HAS_EJ0) > + acpi_root_evaluate_object(handle, "_EJ0", 1); That seems to duplicate some code from scan.c. > +} > + > static void _handle_hotplug_event_root(struct work_struct *work) > { > struct acpi_root_bridge *bridge; > @@ -840,6 +901,12 @@ static void _handle_hotplug_event_root(struct work_struct *work) > } > break; > > + case ACPI_NOTIFY_EJECT_REQUEST: > + /* request device eject */ > + printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__, > + objname); > + handle_root_bridge_removal(handle, bridge); > + break; > default: > printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n", > type, objname); > Thanks, Rafael
On Sat, Jan 12, 2013 at 3:26 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Friday, January 11, 2013 02:40:36 PM Yinghai Lu wrote: >> Add missing hot_remove support for root device. >> >> How to test it? >> 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 >> >> -v2: separate stop and remove, so it will be safe for comingi >> acpi_pci_bind_notify() changes. >> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> Cc: Len Brown <lenb@kernel.org> >> Cc: linux-acpi@vger.kernel.org >> --- >> drivers/acpi/pci_root.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 67 insertions(+) >> >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >> index 5c1f462c..5ae36d8 100644 >> --- a/drivers/acpi/pci_root.c >> +++ b/drivers/acpi/pci_root.c >> @@ -740,6 +740,12 @@ static void add_acpi_root_bridge(acpi_handle handle) >> list_add(&bridge->list, &acpi_root_bridge_list); >> } >> >> +static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge) >> +{ >> + list_del(&bridge->list); >> + kfree(bridge); >> +} >> + >> struct acpi_root_hp_work { >> struct work_struct work; >> acpi_handle handle; >> @@ -800,6 +806,61 @@ static void handle_root_bridge_insertion(acpi_handle handle) >> printk(KERN_ERR "cannot add bridge to acpi list\n"); >> } >> >> +static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val) >> +{ >> + acpi_status status; >> + struct acpi_object_list arg_list; >> + union acpi_object arg; >> + >> + arg_list.count = 1; >> + arg_list.pointer = &arg; >> + arg.type = ACPI_TYPE_INTEGER; >> + arg.integer.value = val; >> + >> + status = acpi_evaluate_object(handle, cmd, &arg_list, NULL); >> + if (ACPI_FAILURE(status)) { >> + pr_warn("%s: %s to %d failed\n", >> + __func__, cmd, val); >> + return -1; > > Please use a meaningful error code. > >> + } >> + >> + return 0; >> +} >> + >> +static void handle_root_bridge_removal(acpi_handle handle, >> + struct acpi_root_bridge *bridge) >> +{ >> + u32 flags = 0; >> + struct acpi_device *device; >> + >> + if (bridge) { >> + flags = bridge->flags; >> + remove_acpi_root_bridge(bridge); >> + } >> + >> + if (!acpi_bus_get_device(handle, &device)) { >> + int ret_val; >> + >> + /* remove pci devices at first */ >> + ret_val = acpi_bus_trim(device, 0); >> + printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val); >> + >> + /* remove acpi devices */ >> + ret_val = acpi_bus_trim(device, 1); >> + printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", ret_val); > > First of all, I don't agree with the way acpi_bus_trim() is used here, as > I said in the previous message. > > Second, this code duplicates the code you're adding on [08/22] almost exactly. > Please put it into a one separate function instead of duplicating it like this. > >> + } >> + >> + if (flags & ROOT_BRIDGE_HAS_PS3) { >> + acpi_status status; >> + >> + status = acpi_evaluate_object(handle, "_PS3", NULL, NULL); >> + if (ACPI_FAILURE(status)) >> + pr_warn("%s: _PS3 failed\n", __func__); > > No, please. acpi_device_set_power() is for that. > >> + } >> + if (flags & ROOT_BRIDGE_HAS_EJ0) >> + acpi_root_evaluate_object(handle, "_EJ0", 1); > > That seems to duplicate some code from scan.c. yes, will address in the following patch. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 5c1f462c..5ae36d8 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -740,6 +740,12 @@ static void add_acpi_root_bridge(acpi_handle handle) list_add(&bridge->list, &acpi_root_bridge_list); } +static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge) +{ + list_del(&bridge->list); + kfree(bridge); +} + struct acpi_root_hp_work { struct work_struct work; acpi_handle handle; @@ -800,6 +806,61 @@ static void handle_root_bridge_insertion(acpi_handle handle) printk(KERN_ERR "cannot add bridge to acpi list\n"); } +static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val) +{ + acpi_status status; + struct acpi_object_list arg_list; + union acpi_object arg; + + arg_list.count = 1; + arg_list.pointer = &arg; + arg.type = ACPI_TYPE_INTEGER; + arg.integer.value = val; + + status = acpi_evaluate_object(handle, cmd, &arg_list, NULL); + if (ACPI_FAILURE(status)) { + pr_warn("%s: %s to %d failed\n", + __func__, cmd, val); + return -1; + } + + return 0; +} + +static void handle_root_bridge_removal(acpi_handle handle, + struct acpi_root_bridge *bridge) +{ + u32 flags = 0; + struct acpi_device *device; + + if (bridge) { + flags = bridge->flags; + remove_acpi_root_bridge(bridge); + } + + if (!acpi_bus_get_device(handle, &device)) { + int ret_val; + + /* remove pci devices at first */ + ret_val = acpi_bus_trim(device, 0); + printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val); + + /* remove acpi devices */ + ret_val = acpi_bus_trim(device, 1); + printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", ret_val); + } + + if (flags & ROOT_BRIDGE_HAS_PS3) { + acpi_status status; + + status = acpi_evaluate_object(handle, "_PS3", NULL, NULL); + if (ACPI_FAILURE(status)) + pr_warn("%s: _PS3 failed\n", __func__); + } + if (flags & ROOT_BRIDGE_HAS_EJ0) + acpi_root_evaluate_object(handle, "_EJ0", 1); +} + static void _handle_hotplug_event_root(struct work_struct *work) { struct acpi_root_bridge *bridge; @@ -840,6 +901,12 @@ static void _handle_hotplug_event_root(struct work_struct *work) } break; + case ACPI_NOTIFY_EJECT_REQUEST: + /* request device eject */ + printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__, + objname); + handle_root_bridge_removal(handle, bridge); + break; default: printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n", type, objname);
Add missing hot_remove support for root device. How to test it? 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 -v2: separate stop and remove, so it will be safe for comingi acpi_pci_bind_notify() changes. Signed-off-by: Yinghai Lu <yinghai@kernel.org> Cc: Len Brown <lenb@kernel.org> Cc: linux-acpi@vger.kernel.org --- drivers/acpi/pci_root.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+)