Re: [PATCH v8 09/22] PCI, ACPI: Add pci_root_hp hot removal notification support.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jan 12, 2013 at 3:26 PM, Rafael J. Wysocki <rjw@xxxxxxx> 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@xxxxxxxxxx>
>> Cc: Len Brown <lenb@xxxxxxxxxx>
>> Cc: linux-acpi@xxxxxxxxxxxxxxx
>> ---
>>  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-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux