public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Linux ACPI <linux-acpi@vger.kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>
Subject: [PATCH v1 2/4] ACPI: property: Add code comments explaining what is going on
Date: Fri, 12 Sep 2025 21:40:58 +0200	[thread overview]
Message-ID: <2243680.irdbgypaU6@rafael.j.wysocki> (raw)
In-Reply-To: <5046661.31r3eYUQgx@rafael.j.wysocki>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In some places in the ACPI device properties handling code, it is
unclear why the code is what it is.  Some assumptions are not documented
and some pieces of code are based on experience that is not mentioned
anywhere.

Add code comments explaining these things.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/property.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -108,7 +108,18 @@ static bool acpi_nondev_subnode_extract(
 	if (handle)
 		acpi_get_parent(handle, &scope);
 
+	/*
+	 * Extract properties from the _DSD-equivalent package pointed to by
+	 * desc and use scope (if not NULL) for the completion of relative
+	 * pathname segments.
+	 *
+	 * The extracted properties will be held in the new data node dn.
+	 */
 	result = acpi_extract_properties(scope, desc, &dn->data);
+	/*
+	 * Look for subnodes in the _DSD-equivalent package pointed to by desc
+	 * and create child nodes of dn if there are any.
+	 */
 	if (acpi_enumerate_nondev_subnodes(scope, desc, &dn->data, &dn->fwnode))
 		result = true;
 
@@ -153,6 +164,12 @@ static bool acpi_nondev_subnode_ok(acpi_
 	acpi_handle handle;
 	acpi_status status;
 
+	/*
+	 * If the scope is unknown, the _DSD-equivalent package being parsed
+	 * was embedded in an outer _DSD-equivalent package as a result of
+	 * direct evaluation of an object pointed to by a reference.  In that
+	 * case, using a pathname as the target object pointer is invalid.
+	 */
 	if (!scope)
 		return false;
 
@@ -172,6 +189,10 @@ static bool acpi_add_nondev_subnodes(acp
 	bool ret = false;
 	int i;
 
+	/*
+	 * Every element in the links package is expected to represent a link
+	 * to a non-device node in a tree containing device-specific data.
+	 */
 	for (i = 0; i < links->package.count; i++) {
 		union acpi_object *link, *desc;
 		acpi_handle handle;
@@ -182,22 +203,48 @@ static bool acpi_add_nondev_subnodes(acp
 		if (link->package.count != 2)
 			continue;
 
-		/* The first one must be a string. */
+		/* The first one (the key) must be a string. */
 		if (link->package.elements[0].type != ACPI_TYPE_STRING)
 			continue;
 
-		/* The second one may be a string, a reference or a package. */
+		/*
+		 * The second one (the target) may be a string, a reference or
+		 * a package.
+		 */
 		switch (link->package.elements[1].type) {
 		case ACPI_TYPE_STRING:
+			/*
+			 * The string is expected to be a full pathname or a
+			 * pathname segment relative to the given scope.  That
+			 * pathname is expected to point to an object returning
+			 * a package that contains _DSD-equivalent information.
+			 */
 			result = acpi_nondev_subnode_ok(scope, link, list,
 							 parent);
 			break;
 		case ACPI_TYPE_LOCAL_REFERENCE:
+			/*
+			 * The reference is expected to point to an object
+			 * returning a package that contains _DSD-equivalent
+			 * information.
+			 */
 			handle = link->package.elements[1].reference.handle;
 			result = acpi_nondev_subnode_data_ok(handle, link, list,
 							     parent);
 			break;
 		case ACPI_TYPE_PACKAGE:
+			/*
+			 * This happens when the target package is embedded
+			 * within the links package as a result of direct
+			 * evaluation of an object pointed to by a reference.
+			 *
+			 * The target package is expected to contain _DSD-
+			 * equivalent information, but the scope in which it
+			 * is located in the original AML is unknown.  Thus
+			 * it cannot contain pathname segments represented as
+			 * strings because there is no way to build full
+			 * pathnames out of them.
+			 */
 			desc = &link->package.elements[1];
 			result = acpi_nondev_subnode_extract(desc, NULL, link,
 							     list, parent);




  parent reply	other threads:[~2025-09-12 19:43 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-12 19:37 [PATCH v1 0/4] ACPI: property: Two fixes, more documentation and a cleanup Rafael J. Wysocki
2025-09-12 19:39 ` [PATCH v1 1/4] ACPI: property: Fix buffer properties extraction for subnodes Rafael J. Wysocki
2025-09-15 11:04   ` Sakari Ailus
2025-09-15 12:09     ` Rafael J. Wysocki
2025-09-12 19:40 ` Rafael J. Wysocki [this message]
2025-09-15 11:32   ` [PATCH v1 2/4] ACPI: property: Add code comments explaining what is going on Sakari Ailus
2025-09-15 12:27     ` Rafael J. Wysocki
2025-09-15 17:12       ` Rafael J. Wysocki
2025-09-15 17:51         ` Sakari Ailus
2025-09-15 18:05           ` Rafael J. Wysocki
2025-09-15 20:23             ` Sakari Ailus
2025-09-15 20:18       ` Sakari Ailus
2025-09-16 11:10         ` Rafael J. Wysocki
2025-09-12 19:42 ` [PATCH v1 3/4] ACPI: property: Do not pass NULL handles to acpi_attach_data() Rafael J. Wysocki
2025-09-15 11:49   ` Sakari Ailus
2025-09-15 12:57     ` Rafael J. Wysocki
2025-09-15 20:43       ` Sakari Ailus
2025-09-12 19:43 ` [PATCH v1 4/4] ACPI: property: Adjust failure handling in acpi_nondev_subnode_extract() Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2243680.irdbgypaU6@rafael.j.wysocki \
    --to=rafael@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox