On Thursday, December 13, 2012 09:05:35 PM Jiang Liu wrote:
> On 12/13/2012 06:32 AM, Rafael J. Wysocki wrote:
> > On Thursday, December 13, 2012 12:38:01 AM Jiang Liu wrote:
> >> On 12/10/2012 07:00 AM, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >>>
> >>> Currently, as soon as an ACPI device node object (struct acpi_device)
> >> snip
> >>
> >>> @@ -1600,48 +1608,77 @@ static acpi_status acpi_bus_check_add(ac
> >>> * We may already have an acpi_device from a previous enumeration. If
> >>> * so, we needn't add it again, but we may still have to start it.
> >>> */
> >>> - device = NULL;
> >>> acpi_bus_get_device(handle, &device);
> >>> if (ops->acpi_op_add && !device) {
> >>> - acpi_add_single_object(&device, handle, type, sta, ops);
> >>> - /* Is the device a known good platform device? */
> >>> - if (device
> >>> - && !acpi_match_device_ids(device, acpi_platform_device_ids))
> >>> - acpi_create_platform_device(device);
> >>> - }
> >>> + struct acpi_bus_ops add_ops = *ops;
> >>>
> >>> - if (!device)
> >>> - return AE_CTRL_DEPTH;
> >>> -
> >>> - if (ops->acpi_op_start && !(ops->acpi_op_add)) {
> >>> - status = acpi_start_single_object(device);
> >>> - if (ACPI_FAILURE(status))
> >>> + add_ops.acpi_op_match = 0;
> >>> + acpi_add_single_object(&device, handle, type, sta, &add_ops);
> >>> + if (!device)
> >>> return AE_CTRL_DEPTH;
> >>> +
> >>> + device->bus_ops.acpi_op_match = 1;
> >>> }
> >>>
> >>> if (!*return_value)
> >>> *return_value = device;
> >>> +
> >>> return AE_OK;
> >>> }
> >>>
> >>> +static acpi_status acpi_bus_probe_start(acpi_handle handle, u32 lvl,
> >>> + void *context, void **not_used)
> >>> +{
> >>> + struct acpi_bus_ops *ops = context;
> >>> + struct acpi_device *device;
> >>> + acpi_status status = AE_OK;
> >>> +
> >>> + if (acpi_bus_get_device(handle, &device))
> >>> + return AE_CTRL_DEPTH;
> >>> +
> >>> + if (ops->acpi_op_add) {
> >>> + if (!acpi_match_device_ids(device, acpi_platform_device_ids)) {
> >>> + /* This is a known good platform device. */
> >>> + acpi_create_platform_device(device);
> >>> + } else {
> >>> + int ret = device_attach(&device->dev);
> >>> + acpi_hot_add_bind(device);
> >>> + if (ret)
> >>> + status = AE_CTRL_DEPTH;
> >>> + }
> >>> + } else if (ops->acpi_op_start) {
> >>> + if (ACPI_FAILURE(acpi_start_single_object(device)))
> >>> + status = AE_CTRL_DEPTH;
> >>> + }
> >>> + return status;
> >>> +}
> >>> +
> >>> static int acpi_bus_scan(acpi_handle handle, struct acpi_bus_ops *ops,
> >>> struct acpi_device **child)
> >>> {
> >>> - acpi_status status;
> >>> void *device = NULL;
> >>> + acpi_status status;
> >>> + int ret = 0;
> >>>
> >>> status = acpi_bus_check_add(handle, 0, ops, &device);
> >>> - if (ACPI_SUCCESS(status))
> >>> + if (ACPI_FAILURE(status)) {
> >>> + ret = -ENODEV;
> >>> + goto out;
> >>> + }
> >>> +
> >>> + acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> >>> + acpi_bus_check_add, NULL, ops, &device);
> >>> + if (device)
> >>> acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> >>> - acpi_bus_check_add, NULL, ops, &device);
> >>> + acpi_bus_probe_start, NULL, ops, NULL);
> >> Hi Rafael,
> >> Should we call acpi_bus_probe_start for the top device corresponding to
> >> "handle" too here?
> >
> > Do you mean separately? I don't think so. It will be covered by the namespace
> > walking, won't it?
> Hi Rafael,
> According to test results from Yijing, we do need to call acpi_bus_probe_start
> for the top device corresponding to "handle".
> Comments for acpi_walk_namespace says:
> /*******************************************************************************
> *
> * FUNCTION: acpi_walk_namespace
> *
> * DESCRIPTION: Performs a modified depth-first walk of the namespace tree,
> * starting (and ending) at the object specified by start_handle.
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> * The callback function is called whenever an object that matches
> * the type parameter is found. If the callback function returns
> * a non-zero value, the search is terminated immediately and this
> * value is returned to the caller.
> *
> * The point of this procedure is to provide a generic namespace
> * a non-zero value, the search is terminated immediately and this
> * value is returned to the caller.
> *
> * The point of this procedure is to provide a generic namespace
> * walk routine that can be called from multiple places to
> * provide multiple services; the callback function(s) can be
> * tailored to each task, whether it is a print function,
> * a compare function, etc.
> *
> ******************************************************************************/
>
> But acpi_ns_walk_namespace() doesn't really call the pre_order_visit and post_order_visit
> for the start_handle. That means acpi_walk_namespace won't call the callback for the top
> handle.
You are right.
I'll fix that and send updated series. It looks like Bjorn wants me to rework
the changelogs anyway. :-)
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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
