Re: [PATCH v3 2/8] ACPI: property: Tie data nodes to acpi handles

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, May 25, 2022 at 04:01:17PM +0300, Sakari Ailus wrote:
> ACPICA allows associating additional information (i.e. pointers with
> specific tag) to acpi_handles. The acpi_device's are associated to
> acpi_handle's in acpi_tie_acpi_dev() in scan.c, do the same here for the
> _DSD data nodes.
> 
> This allows direct data node references in properties, implemented later on
> in the series.

...

> +static int acpi_tie_nondev_subnodes(struct acpi_device_data *data)
> +{
> +	struct acpi_data_node *dn;
> +
> +	list_for_each_entry(dn, &data->subnodes, sibling) {
> +		acpi_status status;
> +		int ret;
> +
> +		status = acpi_attach_data(dn->handle, acpi_nondev_subnode_tag, dn);
> +		if (ACPI_FAILURE(status)) {
> +			acpi_handle_err(dn->handle, "Can't tag data node\n");
> +			return 0;
> +		}
> +
> +		ret = acpi_tie_nondev_subnodes(&dn->data);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

...

> -	if (!adev->data.pointer) {
> +	if (!adev->data.pointer ||

> +	    acpi_tie_nondev_subnodes(&adev->data) < 0) {
> +		acpi_untie_nondev_subnodes(&adev->data);

I don't know this part of the code, but this looks unusual. Shouldn't _tie()
take care of proper error path itself?

Also, it's a bit strange to call _untie() when _tie() wasn't called.

>  		acpi_handle_debug(adev->handle, "Invalid _DSD data, skipping\n");
>  		ACPI_FREE(buf.pointer);
>  	}

-- 
With Best Regards,
Andy Shevchenko