On 2012-10-25 9:31, Tang Chen wrote:
> Hi Toshi,
>
> On 10/25/2012 01:14 AM, Toshi Kani wrote:
>> On Wed, 2012-10-24 at 14:05 +0800, Tang Chen wrote:
>>> +static int container_device_remove(struct acpi_device *device)
>>> +{
>>> + int ret;
>>> + struct acpi_eject_event *ej_event;
>>> +
>>> + /* stop container device at first */
>>> + ret = acpi_bus_trim(device, 0);
>>
>> Hi Tang,
>>
>> Why do you need to call acpi_bus_trim(device,0) to stop the container
>> device first?
>
> This issue was introduced by Lu Yinghai, I think he could give a better
> answer than me. :)
> Please refer to the following url:
>
> http://www.spinics.net/lists/linux-pci/msg17667.html
>
> However, this is not applied into the pci tree yet.
We have worked out a patch set to clean up the logic for PCI/ACPI binding
relationship. It updates PCI/ACPI binding relationship by registering bus
notification onto pci_bus_type instead of hooking into the ACPI/glue.c.
To accommodate that patch set, the ACPI device destroy process has been
split into two steps:
1) acpi_bus_trim(device,0) to unbind ACPI drivers
2) acpi_bus_trim(device,1) to destroy ACPI devices
>
>>
>>> + printk(KERN_WARNING "acpi_bus_trim stop return %x\n", ret);
>>
>> Do you need this message in the normal case? If so, I'd suggest to use
>> pr_debug().
>>
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* event originated from ACPI eject notification */
>>> + device->flags.eject_pending = 1;
>>
>> You do not need to set the eject_pending flag when the handler calls
>> acpi_bus_hot_remove_device(). It was set before because the handler did
>> not initiate the hot-remove operation.
>
> I just set it to keep the logic the same as before.
> And thanks for telling me this. :)
>
>>
> ...
>>> + printk(KERN_WARNING "Container driver received %s event\n",
>>> + "ACPI_NOTIFY_EJECT_REQUEST");
>>
>> Same as other comment. Suggest to use pr_debug().
>
> OK. :)
>
>>
>>> +
>>> + if (!present || ACPI_FAILURE(status) || !device)
>>> + break;
>>> +
>>> + result = container_device_remove(device);
>>> + if (result) {
>>> + printk(KERN_WARNING "Failed to remove container\n");
>>
>> Please use pr_warn().
>>
>> Thanks,
>> -Toshi
>>
>>> + break;
>>> }
>>> - break;
>>> +
>>> + return;
>>>
>>> default:
>>> /* non-hotplug event; possibly handled by other handler */
>>
>>
>> --
>> 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
>>
>
>
> .
>
--
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
