Re: [PATCH v1 3/4] ACPI: property: Do not pass NULL handles to acpi_attach_data()

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Rafael,

On Fri, Sep 12, 2025 at 09:42:55PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> In certain circumstances, the ACPI handle of a data-only node may be
> NULL, in which case it does not make sense to attempt to attach that
> node to an ACPI namespace object, so update the code to avoid attempts
> to do so.
> 
> This prevents confusing and unuseful error messages from being printed.
> 
> Also document the fact that the ACPI handle of a data-only node may be
> NULL, and when that happens, in a code comment.
> 
> In addition, make acpi_add_nondev_subnodes() print a diagnostic message
> for each data-only node with an unknown ACPI namespace scope.
> 
> Fixes: 1d52f10917a7 ("ACPI: property: Tie data nodes to acpi handles")
> Cc: 6.0+ <stable@vger.kernel.org> # 6.0+
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/property.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -124,6 +124,10 @@ static bool acpi_nondev_subnode_extract(
>  		result = true;
>  
>  	if (result) {
> +		/*
> +		 * This will be NULL if the desc package is embedded in an outer
> +		 * _DSD-equivalent package and its scope cannot be determined.
> +		 */

I think indeed this happens in particular when when references to
non-device nodes; there's no handle because when you get is basically a
dynamically allocated copy of a node.

>  		dn->handle = handle;
>  		dn->data.pointer = desc;
>  		list_add_tail(&dn->sibling, list);
> @@ -245,6 +249,8 @@ static bool acpi_add_nondev_subnodes(acp
>  			 * strings because there is no way to build full
>  			 * pathnames out of them.
>  			 */
> +			acpi_handle_info(scope, "Unknown namespace scope of node %s\n",
> +					 link->package.elements[0].string.pointer);
>  			desc = &link->package.elements[1];
>  			result = acpi_nondev_subnode_extract(desc, NULL, link,
>  							     list, parent);
> @@ -408,6 +414,9 @@ static void acpi_untie_nondev_subnodes(s
>  	struct acpi_data_node *dn;
>  
>  	list_for_each_entry(dn, &data->subnodes, sibling) {
> +		if (!dn->handle)
> +			continue;
> +
>  		acpi_detach_data(dn->handle, acpi_nondev_subnode_tag);
>  
>  		acpi_untie_nondev_subnodes(&dn->data);
> @@ -422,6 +431,9 @@ static bool acpi_tie_nondev_subnodes(str
>  		acpi_status status;
>  		bool ret;
>  
> +		if (!dn->handle)
> +			continue;
> +
>  		status = acpi_attach_data(dn->handle, acpi_nondev_subnode_tag, dn);
>  		if (ACPI_FAILURE(status) && status != AE_ALREADY_EXISTS) {
>  			acpi_handle_err(dn->handle, "Can't tag data node\n");
> 
> 
> 

-- 
Kind regards,

Sakari Ailus