[PATCH 00/11] ACPI: Buffer property and reference as string support

[PATCH 00/11] ACPI: Buffer property and reference as string support

[Sakari Ailus]

Hello everyone,

This set adds support for _DSD buffer properties (specified by DSD Guide
<URL:https://github.com/UEFI/DSD-Guide/blob/main/dsd-guide.md>) as well as
support for references as strings. Reference property type was previously
supported for device objects only, whereas string references enable
referencing also _DSD sub-node objects --- also included in the set.

The ACPICA patch has been submitted to upstream but not merged yet.


Sakari Ailus (11):
  ACPI: property: Return type of acpi_add_nondev_subnodes() should be
    bool
  ACPI: acpica: Constify pathname argument for acpi_get_handle()
  ACPI: property: Tie data nodes to acpi handles
  ACPI: property: Use acpi_object_type consistently in property ref
    parsing
  ACPI: property: Move property ref argument parsing into a new function
  ACPI: property: Switch node property referencing from ifs to a switch
  ACPI: Initialise device child list early to access data nodes early
  ACPI: property: Parse data node string references in properties
  ACPI: property: Unify integer value reading functions
  ACPI: property: Add support for parsing buffer property UUID
  ACPI: property: Read buffer properties as integers

 drivers/acpi/acpica/nsxfname.c |   2 +-
 drivers/acpi/property.c        | 517 ++++++++++++++++++++++++---------
 drivers/acpi/scan.c            |   2 +-
 include/acpi/acpi_bus.h        |   3 +-
 include/acpi/acpixf.h          |   2 +-
 include/linux/acpi.h           |   2 +-
 6 files changed, 382 insertions(+), 146 deletions(-)

-- 
2.30.2

[PATCH 01/11] ACPI: property: Return type of acpi_add_nondev_subnodes() should be bool

[Sakari Ailus]

The value acpi_add_nondev_subnodes() returns is bool so change the return
type of the function to match that.

Fixes: 445b0eb058f5 ("ACPI / property: Add support for data-only subnodes")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index d3173811614ec..bc9a645f8bb77 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -155,10 +155,10 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
 	return acpi_nondev_subnode_data_ok(handle, link, list, parent);
 }
 
-static int acpi_add_nondev_subnodes(acpi_handle scope,
-				    const union acpi_object *links,
-				    struct list_head *list,
-				    struct fwnode_handle *parent)
+static bool acpi_add_nondev_subnodes(acpi_handle scope,
+				     const union acpi_object *links,
+				     struct list_head *list,
+				     struct fwnode_handle *parent)
 {
 	bool ret = false;
 	int i;
-- 
2.30.2

[PATCH 02/11] ACPI: acpica: Constify pathname argument for acpi_get_handle()

[Sakari Ailus]

acpi_get_handle() uses the pathname argument to find a handle related to
that pathname but it does not need to modify it. Make it const, in order
to be able to pass const pathname to it.

Cc: "Moore, Robert" <robert.moore@intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/acpica/nsxfname.c | 2 +-
 include/acpi/acpixf.h          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpica/nsxfname.c b/drivers/acpi/acpica/nsxfname.c
index b2cfdfef31947..a0592d15dd37c 100644
--- a/drivers/acpi/acpica/nsxfname.c
+++ b/drivers/acpi/acpica/nsxfname.c
@@ -44,7 +44,7 @@ static char *acpi_ns_copy_device_id(struct acpi_pnp_device_id *dest,
 
 acpi_status
 acpi_get_handle(acpi_handle parent,
-		acpi_string pathname, acpi_handle *ret_handle)
+		const char *pathname, acpi_handle *ret_handle)
 {
 	acpi_status status;
 	struct acpi_namespace_node *node = NULL;
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 67c0b9e734b64..085f23d833349 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -526,7 +526,7 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status
 					   struct acpi_buffer *ret_path_ptr))
 ACPI_EXTERNAL_RETURN_STATUS(acpi_status
 			     acpi_get_handle(acpi_handle parent,
-					     acpi_string pathname,
+					     const char *pathname,
 					     acpi_handle *ret_handle))
 ACPI_EXTERNAL_RETURN_STATUS(acpi_status
 			     acpi_attach_data(acpi_handle object,
-- 
2.30.2

Re: [PATCH 02/11] ACPI: acpica: Constify pathname argument for acpi_get_handle()

[Rafael J. Wysocki]

On Fri, May 6, 2022 at 2:58 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> acpi_get_handle() uses the pathname argument to find a handle related to
> that pathname but it does not need to modify it. Make it const, in order
> to be able to pass const pathname to it.
>
> Cc: "Moore, Robert" <robert.moore@intel.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Which patches in the rest of the series depend on this one?

> ---
>  drivers/acpi/acpica/nsxfname.c | 2 +-
>  include/acpi/acpixf.h          | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/acpica/nsxfname.c b/drivers/acpi/acpica/nsxfname.c
> index b2cfdfef31947..a0592d15dd37c 100644
> --- a/drivers/acpi/acpica/nsxfname.c
> +++ b/drivers/acpi/acpica/nsxfname.c
> @@ -44,7 +44,7 @@ static char *acpi_ns_copy_device_id(struct acpi_pnp_device_id *dest,
>
>  acpi_status
>  acpi_get_handle(acpi_handle parent,
> -               acpi_string pathname, acpi_handle *ret_handle)
> +               const char *pathname, acpi_handle *ret_handle)
>  {
>         acpi_status status;
>         struct acpi_namespace_node *node = NULL;
> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> index 67c0b9e734b64..085f23d833349 100644
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -526,7 +526,7 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>                                            struct acpi_buffer *ret_path_ptr))
>  ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>                              acpi_get_handle(acpi_handle parent,
> -                                            acpi_string pathname,
> +                                            const char *pathname,
>                                              acpi_handle *ret_handle))
>  ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>                              acpi_attach_data(acpi_handle object,
> --
> 2.30.2
>

Re: [PATCH 02/11] ACPI: acpica: Constify pathname argument for acpi_get_handle()

[Sakari Ailus]

Hi Rafael,

On Tue, May 17, 2022 at 06:21:44PM +0200, Rafael J. Wysocki wrote:
> On Fri, May 6, 2022 at 2:58 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > acpi_get_handle() uses the pathname argument to find a handle related to
> > that pathname but it does not need to modify it. Make it const, in order
> > to be able to pass const pathname to it.
> >
> > Cc: "Moore, Robert" <robert.moore@intel.com>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Which patches in the rest of the series depend on this one?

"ACPI: property: Parse data node string references in properties", i.e.
patch 8 in this set.

-- 
Sakari Ailus

Re: [PATCH 02/11] ACPI: acpica: Constify pathname argument for acpi_get_handle()

[Rafael J. Wysocki]

On Wed, May 18, 2022 at 6:14 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Rafael,
>
> On Tue, May 17, 2022 at 06:21:44PM +0200, Rafael J. Wysocki wrote:
> > On Fri, May 6, 2022 at 2:58 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > acpi_get_handle() uses the pathname argument to find a handle related to
> > > that pathname but it does not need to modify it. Make it const, in order
> > > to be able to pass const pathname to it.
> > >
> > > Cc: "Moore, Robert" <robert.moore@intel.com>
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >
> > Which patches in the rest of the series depend on this one?
>
> "ACPI: property: Parse data node string references in properties", i.e.
> patch 8 in this set.

So I think I can apply the rest of the series, except for patch 8 and
patch 7 (as per the previous discussion) for the upcoming merge
window.

Would that work?

Re: [PATCH 02/11] ACPI: acpica: Constify pathname argument for acpi_get_handle()

[Sakari Ailus]

Hi Rafael,

On Wed, May 18, 2022 at 09:07:31PM +0200, Rafael J. Wysocki wrote:
> On Wed, May 18, 2022 at 6:14 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Rafael,
> >
> > On Tue, May 17, 2022 at 06:21:44PM +0200, Rafael J. Wysocki wrote:
> > > On Fri, May 6, 2022 at 2:58 PM Sakari Ailus
> > > <sakari.ailus@linux.intel.com> wrote:
> > > >
> > > > acpi_get_handle() uses the pathname argument to find a handle related to
> > > > that pathname but it does not need to modify it. Make it const, in order
> > > > to be able to pass const pathname to it.
> > > >
> > > > Cc: "Moore, Robert" <robert.moore@intel.com>
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > >
> > > Which patches in the rest of the series depend on this one?
> >
> > "ACPI: property: Parse data node string references in properties", i.e.
> > patch 8 in this set.
> 
> So I think I can apply the rest of the series, except for patch 8 and
> patch 7 (as per the previous discussion) for the upcoming merge
> window.
> 
> Would that work?

I suppose it would apply to this one, too?

Postponing these works for me.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 02/11] ACPI: acpica: Constify pathname argument for acpi_get_handle()

[Rafael J. Wysocki]

On Wed, May 18, 2022 at 9:41 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Rafael,
>
> On Wed, May 18, 2022 at 09:07:31PM +0200, Rafael J. Wysocki wrote:
> > On Wed, May 18, 2022 at 6:14 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > Hi Rafael,
> > >
> > > On Tue, May 17, 2022 at 06:21:44PM +0200, Rafael J. Wysocki wrote:
> > > > On Fri, May 6, 2022 at 2:58 PM Sakari Ailus
> > > > <sakari.ailus@linux.intel.com> wrote:
> > > > >
> > > > > acpi_get_handle() uses the pathname argument to find a handle related to
> > > > > that pathname but it does not need to modify it. Make it const, in order
> > > > > to be able to pass const pathname to it.
> > > > >
> > > > > Cc: "Moore, Robert" <robert.moore@intel.com>
> > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > >
> > > > Which patches in the rest of the series depend on this one?
> > >
> > > "ACPI: property: Parse data node string references in properties", i.e.
> > > patch 8 in this set.
> >
> > So I think I can apply the rest of the series, except for patch 8 and
> > patch 7 (as per the previous discussion) for the upcoming merge
> > window.
> >
> > Would that work?
>
> I suppose it would apply to this one, too?

Patch [2/11] will come to the kernel from upstream ACPICA at one point.

> Postponing these works for me.

So I tried to apply the series without patches [2,7-8/11], but that
didn't work (patch [9/11] did not apply).

Please resubmit without them or let's defer the entire series until we
get patch [2/11] from ACPICA.

Re: [PATCH 02/11] ACPI: acpica: Constify pathname argument for acpi_get_handle()

[Sakari Ailus]

Rafael,

On Thu, May 19, 2022 at 08:05:00PM +0200, Rafael J. Wysocki wrote:
> On Wed, May 18, 2022 at 9:41 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Rafael,
> >
> > On Wed, May 18, 2022 at 09:07:31PM +0200, Rafael J. Wysocki wrote:
> > > On Wed, May 18, 2022 at 6:14 PM Sakari Ailus
> > > <sakari.ailus@linux.intel.com> wrote:
> > > >
> > > > Hi Rafael,
> > > >
> > > > On Tue, May 17, 2022 at 06:21:44PM +0200, Rafael J. Wysocki wrote:
> > > > > On Fri, May 6, 2022 at 2:58 PM Sakari Ailus
> > > > > <sakari.ailus@linux.intel.com> wrote:
> > > > > >
> > > > > > acpi_get_handle() uses the pathname argument to find a handle related to
> > > > > > that pathname but it does not need to modify it. Make it const, in order
> > > > > > to be able to pass const pathname to it.
> > > > > >
> > > > > > Cc: "Moore, Robert" <robert.moore@intel.com>
> > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > >
> > > > > Which patches in the rest of the series depend on this one?
> > > >
> > > > "ACPI: property: Parse data node string references in properties", i.e.
> > > > patch 8 in this set.
> > >
> > > So I think I can apply the rest of the series, except for patch 8 and
> > > patch 7 (as per the previous discussion) for the upcoming merge
> > > window.
> > >
> > > Would that work?
> >
> > I suppose it would apply to this one, too?
> 
> Patch [2/11] will come to the kernel from upstream ACPICA at one point.
> 
> > Postponing these works for me.
> 
> So I tried to apply the series without patches [2,7-8/11], but that
> didn't work (patch [9/11] did not apply).
> 
> Please resubmit without them or let's defer the entire series until we
> get patch [2/11] from ACPICA.

v2 sent, with these three patches dropped. Funnily enough, I saw no
conflicts with your linux-next branch. git am is more picky than git rebase
though.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 02/11] ACPI: acpica: Constify pathname argument for acpi_get_handle()

[Shevchenko, Andriy]

On Fri, May 20, 2022 at 09:13:06AM +0300, Sakari Ailus wrote:
> On Thu, May 19, 2022 at 08:05:00PM +0200, Rafael J. Wysocki wrote:

...

> Funnily enough, I saw no
> conflicts with your linux-next branch. git am is more picky than git rebase
> though.

My experience with both is telling me that it might be related to the context,
rebase is really trying hard to resolve possible conflicts / context issues
automatically. That said, I believe `git am -C1 ...` should solve that.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 03/11] ACPI: property: Tie data nodes to acpi handles

[Sakari Ailus]

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.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 42 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index bc9a645f8bb77..f8c83ee6c8d59 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -340,6 +340,43 @@ acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
 	return props;
 }
 
+static void acpi_nondev_subnode_tag(acpi_handle handle, void *context)
+{
+}
+
+static void acpi_untie_nondev_subnodes(struct acpi_device_data *data)
+{
+	struct acpi_data_node *dn;
+
+	list_for_each_entry(dn, &data->subnodes, sibling) {
+		acpi_detach_data(dn->handle, acpi_nondev_subnode_tag);
+
+		acpi_untie_nondev_subnodes(&dn->data);
+	}
+}
+
+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;
+}
+
 static bool acpi_extract_properties(const union acpi_object *desc,
 				    struct acpi_device_data *data)
 {
@@ -419,7 +456,9 @@ void acpi_init_properties(struct acpi_device *adev)
 					&adev->data, acpi_fwnode_handle(adev)))
 		adev->data.pointer = buf.pointer;
 
-	if (!adev->data.pointer) {
+	if (!adev->data.pointer ||
+	    acpi_tie_nondev_subnodes(&adev->data) < 0) {
+		acpi_untie_nondev_subnodes(&adev->data);
 		acpi_handle_debug(adev->handle, "Invalid _DSD data, skipping\n");
 		ACPI_FREE(buf.pointer);
 	}
@@ -462,6 +501,7 @@ static void acpi_destroy_nondev_subnodes(struct list_head *list)
 
 void acpi_free_properties(struct acpi_device *adev)
 {
+	acpi_untie_nondev_subnodes(&adev->data);
 	acpi_destroy_nondev_subnodes(&adev->data.subnodes);
 	ACPI_FREE((void *)adev->data.pointer);
 	adev->data.of_compatible = NULL;
-- 
2.30.2

[PATCH 04/11] ACPI: property: Use acpi_object_type consistently in property ref parsing

[Sakari Ailus]

The type of union acpi_object field type is acpi_object_type. Use that
instead of int.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index f8c83ee6c8d59..b36cb7e36e420 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -793,7 +793,7 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 			 * nor integer, return an error, we can't parse it.
 			 */
 			for (i = 0; element + i < end && i < num_args; i++) {
-				int type = element[i].type;
+				acpi_object_type type = element[i].type;
 
 				if (type == ACPI_TYPE_LOCAL_REFERENCE)
 					break;
-- 
2.30.2

[PATCH 05/11] ACPI: property: Move property ref argument parsing into a new function

[Sakari Ailus]

Split out property reference argument parsing out of the
__acpi_node_get_property_reference() function into a new one,
acpi_get_ref_args(). The new function will be needed also for parsing
string references soon.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 105 +++++++++++++++++++++++-----------------
 1 file changed, 60 insertions(+), 45 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index b36cb7e36e420..dd6cce955ee28 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -673,6 +673,60 @@ acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
 	return NULL;
 }
 
+static int
+acpi_get_ref_args(struct fwnode_reference_args *args,
+		  struct fwnode_handle *ref_fwnode,
+		  const union acpi_object **element,
+		  const union acpi_object *end, size_t num_args)
+{
+	u32 nargs = 0, i;
+
+	/*
+	 * Find the referred data extension node under the
+	 * referred device node.
+	 */
+	for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
+	     (*element)++) {
+		const char *child_name = (*element)->string.pointer;
+
+		ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode,
+							      child_name);
+		if (!ref_fwnode)
+			return -EINVAL;
+	}
+
+	/*
+	 * Assume the following integer elements are all args. Stop counting on
+	 * the first reference or end of the package arguments. In case of
+	 * neither reference, nor integer, return an error, we can't parse it.
+	 */
+	for (i = 0; (*element) + i < end && i < num_args; i++) {
+		acpi_object_type type = (*element)[i].type;
+
+		if (type == ACPI_TYPE_LOCAL_REFERENCE)
+			break;
+
+		if (type == ACPI_TYPE_INTEGER)
+			nargs++;
+		else
+			return -EINVAL;
+	}
+
+	if (nargs > NR_FWNODE_REFERENCE_ARGS)
+		return -EINVAL;
+
+	if (args) {
+		args->fwnode = ref_fwnode;
+		args->nargs = nargs;
+		for (i = 0; i < nargs; i++)
+			args->args[i] = (*element)[i].integer.value;
+	}
+
+	(*element) += nargs;
+
+	return 0;
+}
+
 /**
  * __acpi_node_get_property_reference - returns handle to the referenced object
  * @fwnode: Firmware node to get the property from
@@ -761,61 +815,22 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 	end = element + obj->package.count;
 
 	while (element < end) {
-		u32 nargs, i;
-
 		if (element->type == ACPI_TYPE_LOCAL_REFERENCE) {
-			struct fwnode_handle *ref_fwnode;
-
 			device = acpi_fetch_acpi_dev(element->reference.handle);
 			if (!device)
 				return -EINVAL;
 
-			nargs = 0;
 			element++;
 
-			/*
-			 * Find the referred data extension node under the
-			 * referred device node.
-			 */
-			for (ref_fwnode = acpi_fwnode_handle(device);
-			     element < end && element->type == ACPI_TYPE_STRING;
-			     element++) {
-				ref_fwnode = acpi_fwnode_get_named_child_node(
-					ref_fwnode, element->string.pointer);
-				if (!ref_fwnode)
-					return -EINVAL;
-			}
-
-			/*
-			 * Assume the following integer elements are all args.
-			 * Stop counting on the first reference or end of the
-			 * package arguments. In case of neither reference,
-			 * nor integer, return an error, we can't parse it.
-			 */
-			for (i = 0; element + i < end && i < num_args; i++) {
-				acpi_object_type type = element[i].type;
-
-				if (type == ACPI_TYPE_LOCAL_REFERENCE)
-					break;
-				if (type == ACPI_TYPE_INTEGER)
-					nargs++;
-				else
-					return -EINVAL;
-			}
-
-			if (nargs > NR_FWNODE_REFERENCE_ARGS)
-				return -EINVAL;
-
-			if (idx == index) {
-				args->fwnode = ref_fwnode;
-				args->nargs = nargs;
-				for (i = 0; i < nargs; i++)
-					args->args[i] = element[i].integer.value;
+			ret = acpi_get_ref_args(idx == index ? args : NULL,
+						acpi_fwnode_handle(device),
+						&element, end, num_args);
+			if (ret < 0)
+				return ret;
 
+			if (idx == index)
 				return 0;
-			}
 
-			element += nargs;
 		} else if (element->type == ACPI_TYPE_INTEGER) {
 			if (idx == index)
 				return -ENOENT;
-- 
2.30.2

[PATCH 06/11] ACPI: property: Switch node property referencing from ifs to a switch

[Sakari Ailus]

__acpi_node_get_property_reference() uses a series of if () statements for
testing the same variable. There's soon going to be one more value to be
tested.

Switch to use switch() instead.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index dd6cce955ee28..a8e8a214a524f 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -780,11 +780,9 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 	if (ret)
 		return ret == -EINVAL ? -ENOENT : -EINVAL;
 
-	/*
-	 * The simplest case is when the value is a single reference.  Just
-	 * return that reference then.
-	 */
-	if (obj->type == ACPI_TYPE_LOCAL_REFERENCE) {
+	switch (obj->type) {
+	case ACPI_TYPE_LOCAL_REFERENCE:
+		/* Plain single reference without arguments. */
 		if (index)
 			return -ENOENT;
 
@@ -795,19 +793,21 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 		args->fwnode = acpi_fwnode_handle(device);
 		args->nargs = 0;
 		return 0;
+	case ACPI_TYPE_PACKAGE:
+		/*
+		 * If it is not a single reference, then it is a package of
+		 * references followed by number of ints as follows:
+		 *
+		 *  Package () { REF, INT, REF, INT, INT }
+		 *
+		 * The index argument is then used to determine which reference
+		 * the caller wants (along with the arguments).
+		 */
+		break;
+	default:
+		return -EINVAL;
 	}
 
-	/*
-	 * If it is not a single reference, then it is a package of
-	 * references followed by number of ints as follows:
-	 *
-	 *  Package () { REF, INT, REF, INT, INT }
-	 *
-	 * The index argument is then used to determine which reference
-	 * the caller wants (along with the arguments).
-	 */
-	if (obj->type != ACPI_TYPE_PACKAGE)
-		return -EINVAL;
 	if (index >= obj->package.count)
 		return -ENOENT;
 
@@ -815,7 +815,8 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 	end = element + obj->package.count;
 
 	while (element < end) {
-		if (element->type == ACPI_TYPE_LOCAL_REFERENCE) {
+		switch (element->type) {
+		case ACPI_TYPE_LOCAL_REFERENCE:
 			device = acpi_fetch_acpi_dev(element->reference.handle);
 			if (!device)
 				return -EINVAL;
@@ -831,11 +832,13 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 			if (idx == index)
 				return 0;
 
-		} else if (element->type == ACPI_TYPE_INTEGER) {
+			break;
+		case ACPI_TYPE_INTEGER:
 			if (idx == index)
 				return -ENOENT;
 			element++;
-		} else {
+			break;
+		default:
 			return -EINVAL;
 		}
 
-- 
2.30.2

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

[Sakari Ailus]

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>
---
 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

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

[Rafael J. Wysocki]

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
>

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

[Sakari Ailus]

Hi Rafael,

On Fri, May 06, 2022 at 03:28:10PM +0200, Rafael J. Wysocki wrote:
> 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.

I actually intended to send this one later if it were to be still relevant.
Please ignore it for now, I'll drop it from v2.

It is currently needed needed for accessing device's data nodes before
registering the device.

-- 
Sakari Ailus

[PATCH 08/11] ACPI: property: Parse data node string references in properties

[Sakari Ailus]

Add support for parsing property references using strings, besides
reference objects that were previously supported. This allows also
referencing data nodes which was not possible with reference objects.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 103 ++++++++++++++++++++++++++++++++++------
 1 file changed, 89 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index a8e8a214a524f..9325a9b2c0543 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -677,7 +677,8 @@ static int
 acpi_get_ref_args(struct fwnode_reference_args *args,
 		  struct fwnode_handle *ref_fwnode,
 		  const union acpi_object **element,
-		  const union acpi_object *end, size_t num_args)
+		  const union acpi_object *end, size_t num_args,
+		  bool subnode_string)
 {
 	u32 nargs = 0, i;
 
@@ -685,14 +686,17 @@ acpi_get_ref_args(struct fwnode_reference_args *args,
 	 * Find the referred data extension node under the
 	 * referred device node.
 	 */
-	for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
-	     (*element)++) {
-		const char *child_name = (*element)->string.pointer;
-
-		ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode,
-							      child_name);
-		if (!ref_fwnode)
-			return -EINVAL;
+	if (subnode_string) {
+		for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
+		     (*element)++) {
+			const char *child_name = (*element)->string.pointer;
+
+			ref_fwnode =
+				acpi_fwnode_get_named_child_node(ref_fwnode,
+								 child_name);
+			if (!ref_fwnode)
+				return -EINVAL;
+		}
 	}
 
 	/*
@@ -703,7 +707,8 @@ acpi_get_ref_args(struct fwnode_reference_args *args,
 	for (i = 0; (*element) + i < end && i < num_args; i++) {
 		acpi_object_type type = (*element)[i].type;
 
-		if (type == ACPI_TYPE_LOCAL_REFERENCE)
+		if (type == ACPI_TYPE_LOCAL_REFERENCE ||
+		    (!subnode_string && type == ACPI_TYPE_STRING))
 			break;
 
 		if (type == ACPI_TYPE_INTEGER)
@@ -727,6 +732,43 @@ acpi_get_ref_args(struct fwnode_reference_args *args,
 	return 0;
 }
 
+static struct fwnode_handle *
+acpi_parse_string_ref(const struct fwnode_handle *fwnode, const char *refstring)
+{
+	acpi_handle scope, handle;
+	struct acpi_data_node *dn;
+	struct acpi_device *device;
+	acpi_status status;
+
+	if (is_acpi_device_node(fwnode)) {
+		scope = to_acpi_device_node(fwnode)->handle;
+	} else if (is_acpi_data_node(fwnode)) {
+		scope = to_acpi_data_node(fwnode)->handle;
+	} else {
+		pr_err("ACPI: bad node type\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	status = acpi_get_handle(scope, refstring, &handle);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_debug(scope, "can't get handle for %s", refstring);
+		return ERR_PTR(-EINVAL);
+	}
+
+	device = acpi_fetch_acpi_dev(handle);
+	if (device)
+		return acpi_fwnode_handle(device);
+
+	status = acpi_get_data_full(handle, acpi_nondev_subnode_tag,
+				    (void **)&dn, NULL);
+	if (ACPI_FAILURE(status) || !dn) {
+		acpi_handle_debug(handle, "can't find subnode");
+		return ERR_PTR(-EINVAL);
+	}
+
+	return &dn->fwnode;
+}
+
 /**
  * __acpi_node_get_property_reference - returns handle to the referenced object
  * @fwnode: Firmware node to get the property from
@@ -769,6 +811,7 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 	const union acpi_object *element, *end;
 	const union acpi_object *obj;
 	const struct acpi_device_data *data;
+	struct fwnode_handle *ref_fwnode;
 	struct acpi_device *device;
 	int ret, idx = 0;
 
@@ -792,16 +835,29 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 
 		args->fwnode = acpi_fwnode_handle(device);
 		args->nargs = 0;
+		return 0;
+	case ACPI_TYPE_STRING:
+		if (index)
+			return -ENOENT;
+
+		ref_fwnode = acpi_parse_string_ref(fwnode, obj->string.pointer);
+		if (IS_ERR(ref_fwnode))
+			return PTR_ERR(ref_fwnode);
+
+		args->fwnode = ref_fwnode;
+		args->nargs = 0;
+
 		return 0;
 	case ACPI_TYPE_PACKAGE:
 		/*
 		 * If it is not a single reference, then it is a package of
-		 * references followed by number of ints as follows:
+		 * references, followed by number of ints as follows:
 		 *
 		 *  Package () { REF, INT, REF, INT, INT }
 		 *
-		 * The index argument is then used to determine which reference
-		 * the caller wants (along with the arguments).
+		 * Here, REF may be either a local reference or a string. The
+		 * index argument is then used to determine which reference the
+		 * caller wants (along with the arguments).
 		 */
 		break;
 	default:
@@ -825,7 +881,26 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 
 			ret = acpi_get_ref_args(idx == index ? args : NULL,
 						acpi_fwnode_handle(device),
-						&element, end, num_args);
+						&element, end, num_args, true);
+			if (ret < 0)
+				return ret;
+
+			if (idx == index)
+				return 0;
+
+			break;
+		case ACPI_TYPE_STRING:
+			ref_fwnode =
+				acpi_parse_string_ref(fwnode,
+						      element->string.pointer);
+			if (IS_ERR(ref_fwnode))
+				return PTR_ERR(ref_fwnode);
+
+			element++;
+
+			ret = acpi_get_ref_args(idx == index ? args : NULL,
+						ref_fwnode, &element, end,
+						num_args, false);
 			if (ret < 0)
 				return ret;
 
-- 
2.30.2

[PATCH 09/11] ACPI: property: Unify integer value reading functions

[Sakari Ailus]

Unify functions reading ACPI property integer values into a single macro,
and call that macro to generate the functions for each bit depth.

Also use size_t for the counter instead of int.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 79 +++++++++++------------------------------
 1 file changed, 20 insertions(+), 59 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 9325a9b2c0543..2205612fd6e98 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -985,67 +985,28 @@ static int acpi_data_prop_read_single(const struct acpi_device_data *data,
 	return ret;
 }
 
-static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
-				       size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-		if (items[i].integer.value > U8_MAX)
-			return -EOVERFLOW;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
-
-static int acpi_copy_property_array_u16(const union acpi_object *items,
-					u16 *val, size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-		if (items[i].integer.value > U16_MAX)
-			return -EOVERFLOW;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
-
-static int acpi_copy_property_array_u32(const union acpi_object *items,
-					u32 *val, size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-		if (items[i].integer.value > U32_MAX)
-			return -EOVERFLOW;
-
-		val[i] = items[i].integer.value;
+#define DECLARE_ACPI_PROPERTY_COPY(bits)				\
+	static int							\
+	acpi_copy_property_array_u##bits(const union acpi_object *items, \
+					 u##bits *val, size_t nval)	\
+	{								\
+		size_t i;						\
+									\
+		for (i = 0; i < nval; i++) {				\
+			if (items[i].type != ACPI_TYPE_INTEGER)		\
+				return -EPROTO;				\
+			if (items[i].integer.value > U##bits##_MAX)	\
+				return -EOVERFLOW;			\
+									\
+			val[i] = items[i].integer.value;		\
+		}							\
+		return 0;						\
 	}
-	return 0;
-}
 
-static int acpi_copy_property_array_u64(const union acpi_object *items,
-					u64 *val, size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
+DECLARE_ACPI_PROPERTY_COPY(8)
+DECLARE_ACPI_PROPERTY_COPY(16)
+DECLARE_ACPI_PROPERTY_COPY(32)
+DECLARE_ACPI_PROPERTY_COPY(64)
 
 static int acpi_copy_property_array_string(const union acpi_object *items,
 					   char **val, size_t nval)
-- 
2.30.2

[PATCH 10/11] ACPI: property: Add support for parsing buffer property UUID

[Sakari Ailus]

Add support for newly added buffer property UUID, as defined in the DSD
guide.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 141 ++++++++++++++++++++++++++++++++++++----
 include/acpi/acpi_bus.h |   3 +-
 include/linux/acpi.h    |   2 +-
 3 files changed, 131 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 2205612fd6e98..d3f6d95e0fb19 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -55,14 +55,19 @@ static const guid_t ads_guid =
 	GUID_INIT(0xdbb8e3e6, 0x5886, 0x4ba6,
 		  0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
 
+static const guid_t buffer_prop_guid =
+	GUID_INIT(0xedb12dd0, 0x363d, 0x4085,
+		  0xa3, 0xd2, 0x49, 0x52, 0x2c, 0xa1, 0x60, 0xc4);
+
 static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
-					   const union acpi_object *desc,
+					   union acpi_object *desc,
 					   struct acpi_device_data *data,
 					   struct fwnode_handle *parent);
-static bool acpi_extract_properties(const union acpi_object *desc,
+static bool acpi_extract_properties(acpi_handle handle,
+				    union acpi_object *desc,
 				    struct acpi_device_data *data);
 
-static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
+static bool acpi_nondev_subnode_extract(union acpi_object *desc,
 					acpi_handle handle,
 					const union acpi_object *link,
 					struct list_head *list,
@@ -81,7 +86,7 @@ static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
 	INIT_LIST_HEAD(&dn->data.properties);
 	INIT_LIST_HEAD(&dn->data.subnodes);
 
-	result = acpi_extract_properties(desc, &dn->data);
+	result = acpi_extract_properties(handle, desc, &dn->data);
 
 	if (handle) {
 		acpi_handle scope;
@@ -156,7 +161,7 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
 }
 
 static bool acpi_add_nondev_subnodes(acpi_handle scope,
-				     const union acpi_object *links,
+				     union acpi_object *links,
 				     struct list_head *list,
 				     struct fwnode_handle *parent)
 {
@@ -164,7 +169,7 @@ static bool acpi_add_nondev_subnodes(acpi_handle scope,
 	int i;
 
 	for (i = 0; i < links->package.count; i++) {
-		const union acpi_object *link, *desc;
+		union acpi_object *link, *desc;
 		acpi_handle handle;
 		bool result;
 
@@ -204,7 +209,7 @@ static bool acpi_add_nondev_subnodes(acpi_handle scope,
 }
 
 static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
-					   const union acpi_object *desc,
+					   union acpi_object *desc,
 					   struct acpi_device_data *data,
 					   struct fwnode_handle *parent)
 {
@@ -212,7 +217,8 @@ static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
 
 	/* Look for the ACPI data subnodes GUID. */
 	for (i = 0; i < desc->package.count; i += 2) {
-		const union acpi_object *guid, *links;
+		const union acpi_object *guid;
+		union acpi_object *links;
 
 		guid = &desc->package.elements[i];
 		links = &desc->package.elements[i + 1];
@@ -325,7 +331,7 @@ static bool acpi_is_property_guid(const guid_t *guid)
 
 struct acpi_device_properties *
 acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
-		    const union acpi_object *properties)
+		    union acpi_object *properties)
 {
 	struct acpi_device_properties *props;
 
@@ -377,7 +383,103 @@ static int acpi_tie_nondev_subnodes(struct acpi_device_data *data)
 	return 0;
 }
 
-static bool acpi_extract_properties(const union acpi_object *desc,
+static void acpi_data_add_buffer_props(acpi_handle handle,
+				       struct acpi_device_data *data,
+				       union acpi_object *properties)
+{
+	struct acpi_device_properties *props;
+	union acpi_object *package;
+	size_t alloc_size;
+	unsigned int i;
+	u32 *count;
+
+	if (check_mul_overflow((size_t)properties->package.count,
+			       sizeof(*package) + sizeof(void *),
+			       &alloc_size) ||
+	    check_add_overflow(sizeof(*props) + sizeof(*package), alloc_size,
+			       &alloc_size)) {
+		acpi_handle_warn(handle,
+				 "can't allocate memory for %u buffer props",
+				 properties->package.count);
+		return;
+	}
+
+	props = kvzalloc(alloc_size, GFP_KERNEL);
+	if (!props)
+		return;
+
+	props->guid = &buffer_prop_guid;
+	props->bufs = (void *)(props + 1);
+	props->properties = (void *)(props->bufs + properties->package.count);
+
+	/* Outer package */
+	package = props->properties;
+	package->type = ACPI_TYPE_PACKAGE;
+	package->package.elements = package + 1;
+	count = &package->package.count;
+	*count = 0;
+
+	/* Inner packages */
+	package++;
+
+	for (i = 0; i < properties->package.count; i++) {
+		struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+		union acpi_object *property = &properties->package.elements[i];
+		union acpi_object *prop, *obj, *buf_obj;
+		acpi_status status;
+
+		if (property->type != ACPI_TYPE_PACKAGE ||
+		    property->package.count != 2) {
+			acpi_handle_warn(handle,
+					 "buffer property %u has %u entries\n",
+					 i, property->package.count);
+			continue;
+		}
+
+		prop = &property->package.elements[0];
+		obj = &property->package.elements[1];
+
+		if (prop->type != ACPI_TYPE_STRING ||
+		    obj->type != ACPI_TYPE_STRING) {
+			acpi_handle_warn(handle,
+					 "wrong object types %u and %u\n",
+					 prop->type, obj->type);
+			continue;
+		}
+
+		status = acpi_evaluate_object_typed(handle, obj->string.pointer,
+						    NULL, &buf,
+						    ACPI_TYPE_BUFFER);
+		if (ACPI_FAILURE(status)) {
+			acpi_handle_warn(handle,
+					 "can't evaluate \"%s\" as buffer\n",
+					 obj->string.pointer);
+			continue;
+		}
+
+		package->type = ACPI_TYPE_PACKAGE;
+		package->package.elements = prop;
+		package->package.count = 2;
+
+		buf_obj = buf.pointer;
+
+		/* Replace the string object with a buffer object */
+		obj->type = ACPI_TYPE_BUFFER;
+		obj->buffer.length = buf_obj->buffer.length;
+		obj->buffer.pointer = buf_obj->buffer.pointer;
+
+		props->bufs[i] = buf.pointer;
+		package++;
+		(*count)++;
+	}
+
+	if (*count)
+		list_add(&props->list, &data->properties);
+	else
+		kvfree(props);
+}
+
+static bool acpi_extract_properties(acpi_handle scope, union acpi_object *desc,
 				    struct acpi_device_data *data)
 {
 	int i;
@@ -387,7 +489,8 @@ static bool acpi_extract_properties(const union acpi_object *desc,
 
 	/* Look for the device properties GUID. */
 	for (i = 0; i < desc->package.count; i += 2) {
-		const union acpi_object *guid, *properties;
+		const union acpi_object *guid;
+		union acpi_object *properties;
 
 		guid = &desc->package.elements[i];
 		properties = &desc->package.elements[i + 1];
@@ -401,6 +504,12 @@ static bool acpi_extract_properties(const union acpi_object *desc,
 		    properties->type != ACPI_TYPE_PACKAGE)
 			break;
 
+		if (guid_equal((guid_t *)guid->buffer.pointer,
+			       &buffer_prop_guid)) {
+			acpi_data_add_buffer_props(scope, data, properties);
+			continue;
+		}
+
 		if (!acpi_is_property_guid((guid_t *)guid->buffer.pointer))
 			continue;
 
@@ -447,7 +556,7 @@ void acpi_init_properties(struct acpi_device *adev)
 	if (ACPI_FAILURE(status))
 		goto out;
 
-	if (acpi_extract_properties(buf.pointer, &adev->data)) {
+	if (acpi_extract_properties(adev->handle, buf.pointer, &adev->data)) {
 		adev->data.pointer = buf.pointer;
 		if (acpi_of)
 			acpi_init_of_compatible(adev);
@@ -477,8 +586,14 @@ static void acpi_free_device_properties(struct list_head *list)
 	struct acpi_device_properties *props, *tmp;
 
 	list_for_each_entry_safe(props, tmp, list, list) {
+		u32 i;
+
 		list_del(&props->list);
-		kfree(props);
+		/* Buffer data properties were separately allocated */
+		if (props->bufs)
+			for (i = 0; i < props->properties->package.count; i++)
+				ACPI_FREE(props->bufs[i]);
+		kvfree(props);
 	}
 }
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index b44aaffedb914..8df984013ee71 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -344,8 +344,9 @@ struct acpi_device_physical_node {
 
 struct acpi_device_properties {
 	const guid_t *guid;
-	const union acpi_object *properties;
+	union acpi_object *properties;
 	struct list_head list;
+	void **bufs;
 };
 
 /* ACPI Device Specific Data (_DSD) */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d7136d13aa442..a411bae44b7e2 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1204,7 +1204,7 @@ static inline bool acpi_dev_has_props(const struct acpi_device *adev)
 
 struct acpi_device_properties *
 acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
-		    const union acpi_object *properties);
+		    union acpi_object *properties);
 
 int acpi_node_prop_get(const struct fwnode_handle *fwnode, const char *propname,
 		       void **valptr);
-- 
2.30.2

[PATCH 11/11] ACPI: property: Read buffer properties as integers

[Sakari Ailus]

Instead of adding a new property type, read buffer properties as integers.
Even though the internal representation in ACPI is different, the data
type is the same (byte) than on 8-bit integers.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index d3f6d95e0fb19..f23d15abadd65 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1108,6 +1108,10 @@ static int acpi_data_prop_read_single(const struct acpi_device_data *data,
 		size_t i;						\
 									\
 		for (i = 0; i < nval; i++) {				\
+			if (items->type == ACPI_TYPE_BUFFER) {		\
+				val[i] = items->buffer.pointer[i];	\
+				continue;				\
+			}						\
 			if (items[i].type != ACPI_TYPE_INTEGER)		\
 				return -EPROTO;				\
 			if (items[i].integer.value > U##bits##_MAX)	\
@@ -1163,18 +1167,40 @@ static int acpi_data_prop_read(const struct acpi_device_data *data,
 	}
 
 	ret = acpi_data_get_property_array(data, propname, ACPI_TYPE_ANY, &obj);
+	if (ret && proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64)
+		ret = acpi_data_get_property(data, propname, ACPI_TYPE_BUFFER,
+					     &obj);
 	if (ret)
 		return ret;
 
-	if (!val)
+	if (!val) {
+		if (obj->type == ACPI_TYPE_BUFFER)
+			return obj->buffer.length;
+
 		return obj->package.count;
+	}
 
-	if (proptype != DEV_PROP_STRING && nval > obj->package.count)
-		return -EOVERFLOW;
+	switch (proptype) {
+	case DEV_PROP_STRING:
+		break;
+	case DEV_PROP_U8 ... DEV_PROP_U64:
+		if (obj->type == ACPI_TYPE_BUFFER) {
+			if (nval <= obj->buffer.length)
+				break;
+			return -EOVERFLOW;
+		}
+		fallthrough;
+	default:
+		if (nval > obj->package.count)
+			return -EOVERFLOW;
+	}
 	if (nval == 0)
 		return -EINVAL;
 
-	items = obj->package.elements;
+	if (obj->type != ACPI_TYPE_BUFFER)
+		items = obj->package.elements;
+	else
+		items = obj;
 
 	switch (proptype) {
 	case DEV_PROP_U8:
-- 
2.30.2