Re: [PATCH 07/11] ACPI: Initialise device child list early to access data nodes early

Rafael J. Wysocki
2022-05-06T13:28:25Z

On Fri, May 6, 2022 at 2:58 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> The properties, including data nodes, are initialised in
> acpi_init_device_object(). Traversing the data nodes also requires the
> device's child list to be initialised which happens much later in
> __acpi_device_add(). The function also makes the device visible in the
> system, so setting up its properties and nodes is too late by then.
>
> To address this, move the child list initialisation before
> acpi_init_properties() in acpi_init_device_object().
>
> Note that this is currently not an issue as the properties will only be
> accessed by drivers. In the near future accessing the properties will be
> done in the ACPI framework itself, and doing so requires this change.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

There is a problem with this that the children list is redundant and
not really safe to use and so it will be dropped. I actually have a
series of patches to do that in the works.

I'm also unsure why it would be necessary to initialize the list of
the device's children earlier, because adding anything to that list
requires a child device object to be registered with cannot happen
before registering the parent itself.

> ---
> drivers/acpi/scan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 762b61f67e6c6..86c4e9a473edc 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -680,7 +680,6 @@ static int __acpi_device_add(struct acpi_device *device,
> * -------
> * Link this device to its parent and siblings.
> */
> - INIT_LIST_HEAD(&device->children);
> INIT_LIST_HEAD(&device->node);
> INIT_LIST_HEAD(&device->wakeup_list);
> INIT_LIST_HEAD(&device->physical_node_list);
> @@ -1786,6 +1785,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
> acpi_set_device_status(device, ACPI_STA_DEFAULT);
> acpi_device_get_busid(device);
> acpi_set_pnp_ids(handle, &device->pnp, type);
> + INIT_LIST_HEAD(&device->children);
> acpi_init_properties(device);
> acpi_bus_get_flags(device);
> device->flags.match_driver = false;
> --
> 2.30.2
>