* [PATCH v2 1/8] ACPI: property: Return type of acpi_add_nondev_subnodes() should be bool
2022-05-20 6:11 [PATCH v2 0/8] ACPI: Buffer property and reference as string support Sakari Ailus
@ 2022-05-20 6:11 ` Sakari Ailus
2022-05-20 6:11 ` [PATCH v2 2/8] ACPI: property: Tie data nodes to acpi handles Sakari Ailus
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2022-05-20 6:11 UTC (permalink / raw)
To: linux-acpi; +Cc: rafael, andriy.shevchenko
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/8] ACPI: property: Tie data nodes to acpi handles
2022-05-20 6:11 [PATCH v2 0/8] ACPI: Buffer property and reference as string support Sakari Ailus
2022-05-20 6:11 ` [PATCH v2 1/8] ACPI: property: Return type of acpi_add_nondev_subnodes() should be bool Sakari Ailus
@ 2022-05-20 6:11 ` Sakari Ailus
2022-05-20 6:11 ` [PATCH v2 3/8] ACPI: property: Use acpi_object_type consistently in property ref parsing Sakari Ailus
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2022-05-20 6:11 UTC (permalink / raw)
To: linux-acpi; +Cc: rafael, andriy.shevchenko
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/8] ACPI: property: Use acpi_object_type consistently in property ref parsing
2022-05-20 6:11 [PATCH v2 0/8] ACPI: Buffer property and reference as string support Sakari Ailus
2022-05-20 6:11 ` [PATCH v2 1/8] ACPI: property: Return type of acpi_add_nondev_subnodes() should be bool Sakari Ailus
2022-05-20 6:11 ` [PATCH v2 2/8] ACPI: property: Tie data nodes to acpi handles Sakari Ailus
@ 2022-05-20 6:11 ` Sakari Ailus
2022-05-20 6:11 ` [PATCH v2 4/8] ACPI: property: Move property ref argument parsing into a new function Sakari Ailus
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2022-05-20 6:11 UTC (permalink / raw)
To: linux-acpi; +Cc: rafael, andriy.shevchenko
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/8] ACPI: property: Move property ref argument parsing into a new function
2022-05-20 6:11 [PATCH v2 0/8] ACPI: Buffer property and reference as string support Sakari Ailus
` (2 preceding siblings ...)
2022-05-20 6:11 ` [PATCH v2 3/8] ACPI: property: Use acpi_object_type consistently in property ref parsing Sakari Ailus
@ 2022-05-20 6:11 ` Sakari Ailus
2022-05-20 6:11 ` [PATCH v2 5/8] ACPI: property: Switch node property referencing from ifs to a switch Sakari Ailus
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2022-05-20 6:11 UTC (permalink / raw)
To: linux-acpi; +Cc: rafael, andriy.shevchenko
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/8] ACPI: property: Switch node property referencing from ifs to a switch
2022-05-20 6:11 [PATCH v2 0/8] ACPI: Buffer property and reference as string support Sakari Ailus
` (3 preceding siblings ...)
2022-05-20 6:11 ` [PATCH v2 4/8] ACPI: property: Move property ref argument parsing into a new function Sakari Ailus
@ 2022-05-20 6:11 ` Sakari Ailus
2022-05-20 6:11 ` [PATCH v2 6/8] ACPI: property: Unify integer value reading functions Sakari Ailus
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2022-05-20 6:11 UTC (permalink / raw)
To: linux-acpi; +Cc: rafael, andriy.shevchenko
__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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 6/8] ACPI: property: Unify integer value reading functions
2022-05-20 6:11 [PATCH v2 0/8] ACPI: Buffer property and reference as string support Sakari Ailus
` (4 preceding siblings ...)
2022-05-20 6:11 ` [PATCH v2 5/8] ACPI: property: Switch node property referencing from ifs to a switch Sakari Ailus
@ 2022-05-20 6:11 ` Sakari Ailus
2022-05-20 14:25 ` Andy Shevchenko
2022-05-20 6:11 ` [PATCH v2 7/8] ACPI: property: Add support for parsing buffer property UUID Sakari Ailus
2022-05-20 6:11 ` [PATCH v2 8/8] ACPI: property: Read buffer properties as integers Sakari Ailus
7 siblings, 1 reply; 13+ messages in thread
From: Sakari Ailus @ 2022-05-20 6:11 UTC (permalink / raw)
To: linux-acpi; +Cc: rafael, andriy.shevchenko
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 a8e8a214a524f..8449479e18442 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -910,67 +910,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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/8] ACPI: property: Unify integer value reading functions
2022-05-20 6:11 ` [PATCH v2 6/8] ACPI: property: Unify integer value reading functions Sakari Ailus
@ 2022-05-20 14:25 ` Andy Shevchenko
2022-05-20 22:04 ` Sakari Ailus
0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2022-05-20 14:25 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-acpi, rafael
On Fri, May 20, 2022 at 09:11:46AM +0300, Sakari Ailus wrote:
> 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.
...
> +#define DECLARE_ACPI_PROPERTY_COPY(bits) \
> + static int \
> + acpi_copy_property_array_u##bits(const union acpi_object *items, \
Personally I find much better if function templates in macros are not indented
additionally.
> + 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; \
> }
On top of that, we use a minimum compiler that supports _Generic(). Why not to
use it?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/8] ACPI: property: Unify integer value reading functions
2022-05-20 14:25 ` Andy Shevchenko
@ 2022-05-20 22:04 ` Sakari Ailus
0 siblings, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2022-05-20 22:04 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-acpi, rafael
Hi Andy,
Thanks for the review.
On Fri, May 20, 2022 at 05:25:09PM +0300, Andy Shevchenko wrote:
> On Fri, May 20, 2022 at 09:11:46AM +0300, Sakari Ailus wrote:
> > 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.
>
> ...
>
> > +#define DECLARE_ACPI_PROPERTY_COPY(bits) \
> > + static int \
> > + acpi_copy_property_array_u##bits(const union acpi_object *items, \
>
> Personally I find much better if function templates in macros are not indented
> additionally.
I prefer it the way it is in this patch: indenting the macro has grounds as
it is part of the same pre-processor directive even if it's split on
multiple lines.
>
> > + 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; \
> > }
>
> On top of that, we use a minimum compiler that supports _Generic(). Why not to
> use it?
Good idea, I'll look into this. Perhaps we'll even get rid of having to
define multiple functions for this.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 7/8] ACPI: property: Add support for parsing buffer property UUID
2022-05-20 6:11 [PATCH v2 0/8] ACPI: Buffer property and reference as string support Sakari Ailus
` (5 preceding siblings ...)
2022-05-20 6:11 ` [PATCH v2 6/8] ACPI: property: Unify integer value reading functions Sakari Ailus
@ 2022-05-20 6:11 ` Sakari Ailus
2022-05-20 14:28 ` Andy Shevchenko
2022-05-20 6:11 ` [PATCH v2 8/8] ACPI: property: Read buffer properties as integers Sakari Ailus
7 siblings, 1 reply; 13+ messages in thread
From: Sakari Ailus @ 2022-05-20 6:11 UTC (permalink / raw)
To: linux-acpi; +Cc: rafael, andriy.shevchenko
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 8449479e18442..93157af151c9b 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 772590e2eddb8..d5c60537982d8 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 fadda404bcc9b..2213a8f046571 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1201,7 +1201,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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 7/8] ACPI: property: Add support for parsing buffer property UUID
2022-05-20 6:11 ` [PATCH v2 7/8] ACPI: property: Add support for parsing buffer property UUID Sakari Ailus
@ 2022-05-20 14:28 ` Andy Shevchenko
2022-05-25 8:13 ` Sakari Ailus
0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2022-05-20 14:28 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-acpi, rafael
On Fri, May 20, 2022 at 09:11:47AM +0300, Sakari Ailus wrote:
> Add support for newly added buffer property UUID, as defined in the DSD
> guide.
...
> 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);
Looking at the first one, can union member be const in the last one?
...
> struct acpi_device_properties {
> const guid_t *guid;
> - const union acpi_object *properties;
> + union acpi_object *properties;
Probably this change explains on the above question...
> struct list_head list;
> + void **bufs;
> };
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 7/8] ACPI: property: Add support for parsing buffer property UUID
2022-05-20 14:28 ` Andy Shevchenko
@ 2022-05-25 8:13 ` Sakari Ailus
0 siblings, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2022-05-25 8:13 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-acpi, rafael
Hi Andy,
On Fri, May 20, 2022 at 05:28:52PM +0300, Andy Shevchenko wrote:
> On Fri, May 20, 2022 at 09:11:47AM +0300, Sakari Ailus wrote:
> > Add support for newly added buffer property UUID, as defined in the DSD
> > guide.
>
> ...
>
> > 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);
>
> Looking at the first one, can union member be const in the last one?
The object contains _DSD properties that will be changed if the properties
are of buffer type --- see acpi_data_add_buffer_props() in the same patch.
>
> ...
>
> > struct acpi_device_properties {
> > const guid_t *guid;
> > - const union acpi_object *properties;
> > + union acpi_object *properties;
>
> Probably this change explains on the above question...
>
> > struct list_head list;
> > + void **bufs;
> > };
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 8/8] ACPI: property: Read buffer properties as integers
2022-05-20 6:11 [PATCH v2 0/8] ACPI: Buffer property and reference as string support Sakari Ailus
` (6 preceding siblings ...)
2022-05-20 6:11 ` [PATCH v2 7/8] ACPI: property: Add support for parsing buffer property UUID Sakari Ailus
@ 2022-05-20 6:11 ` Sakari Ailus
7 siblings, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2022-05-20 6:11 UTC (permalink / raw)
To: linux-acpi; +Cc: rafael, andriy.shevchenko
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 93157af151c9b..e94dc4dbbf627 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1033,6 +1033,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) \
@@ -1088,18 +1092,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
^ permalink raw reply related [flat|nested] 13+ messages in thread