From: Sunil V L <sunilvl@ventanamicro.com>
Currently acpi_fwnode_get_reference_args() calls the public function
__acpi_node_get_property_reference() which ignores the nargs_prop
parameter. To fix this, make __acpi_node_get_property_reference() to
call the static acpi_fwnode_get_reference() so that callers of
fwnode_get_reference_args() can still pass a valid property name to
fetch the number of arguments.
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
drivers/acpi/property.c | 101 ++++++++++++++++++++--------------------
1 file changed, 50 insertions(+), 51 deletions(-)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 436019d96027..d4863746fb11 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -882,45 +882,10 @@ static struct fwnode_handle *acpi_parse_string_ref(const struct fwnode_handle *f
return &dn->fwnode;
}
-/**
- * __acpi_node_get_property_reference - returns handle to the referenced object
- * @fwnode: Firmware node to get the property from
- * @propname: Name of the property
- * @index: Index of the reference to return
- * @num_args: Maximum number of arguments after each reference
- * @args: Location to store the returned reference with optional arguments
- * (may be NULL)
- *
- * Find property with @name, verifify that it is a package containing at least
- * one object reference and if so, store the ACPI device object pointer to the
- * target object in @args->adev. If the reference includes arguments, store
- * them in the @args->args[] array.
- *
- * If there's more than one reference in the property value package, @index is
- * used to select the one to return.
- *
- * It is possible to leave holes in the property value set like in the
- * example below:
- *
- * Package () {
- * "cs-gpios",
- * Package () {
- * ^GPIO, 19, 0, 0,
- * ^GPIO, 20, 0, 0,
- * 0,
- * ^GPIO, 21, 0, 0,
- * }
- * }
- *
- * Calling this function with index %2 or index %3 return %-ENOENT. If the
- * property does not contain any more values %-ENOENT is returned. The NULL
- * entry must be single integer and preferably contain value %0.
- *
- * Return: %0 on success, negative error code on failure.
- */
-int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
- const char *propname, size_t index, size_t num_args,
- struct fwnode_reference_args *args)
+static int acpi_fwnode_get_reference_args(const struct fwnode_handle *fwnode,
+ const char *propname, const char *nargs_prop,
+ unsigned int args_count, unsigned int index,
+ struct fwnode_reference_args *args)
{
const union acpi_object *element, *end;
const union acpi_object *obj;
@@ -999,7 +964,7 @@ 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, args_count);
if (ret < 0)
return ret;
@@ -1017,7 +982,7 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
ret = acpi_get_ref_args(idx == index ? args : NULL,
ref_fwnode, &element, end,
- num_args);
+ args_count);
if (ret < 0)
return ret;
@@ -1039,6 +1004,50 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
return -ENOENT;
}
+
+/**
+ * __acpi_node_get_property_reference - returns handle to the referenced object
+ * @fwnode: Firmware node to get the property from
+ * @propname: Name of the property
+ * @index: Index of the reference to return
+ * @num_args: Maximum number of arguments after each reference
+ * @args: Location to store the returned reference with optional arguments
+ * (may be NULL)
+ *
+ * Find property with @name, verifify that it is a package containing at least
+ * one object reference and if so, store the ACPI device object pointer to the
+ * target object in @args->adev. If the reference includes arguments, store
+ * them in the @args->args[] array.
+ *
+ * If there's more than one reference in the property value package, @index is
+ * used to select the one to return.
+ *
+ * It is possible to leave holes in the property value set like in the
+ * example below:
+ *
+ * Package () {
+ * "cs-gpios",
+ * Package () {
+ * ^GPIO, 19, 0, 0,
+ * ^GPIO, 20, 0, 0,
+ * 0,
+ * ^GPIO, 21, 0, 0,
+ * }
+ * }
+ *
+ * Calling this function with index %2 or index %3 return %-ENOENT. If the
+ * property does not contain any more values %-ENOENT is returned. The NULL
+ * entry must be single integer and preferably contain value %0.
+ *
+ * Return: %0 on success, negative error code on failure.
+ */
+int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
+ const char *propname, size_t index,
+ size_t num_args,
+ struct fwnode_reference_args *args)
+{
+ return acpi_fwnode_get_reference_args(fwnode, propname, NULL, index, num_args, args);
+}
EXPORT_SYMBOL_GPL(__acpi_node_get_property_reference);
static int acpi_data_prop_read_single(const struct acpi_device_data *data,
@@ -1558,16 +1567,6 @@ acpi_fwnode_property_read_string_array(const struct fwnode_handle *fwnode,
val, nval);
}
-static int
-acpi_fwnode_get_reference_args(const struct fwnode_handle *fwnode,
- const char *prop, const char *nargs_prop,
- unsigned int args_count, unsigned int index,
- struct fwnode_reference_args *args)
-{
- return __acpi_node_get_property_reference(fwnode, prop, index,
- args_count, args);
-}
-
static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
{
const struct acpi_device *adev;
--
2.43.0On Wed, Jul 2, 2025 at 7:15 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> From: Sunil V L <sunilvl@ventanamicro.com>
>
> Currently acpi_fwnode_get_reference_args() calls the public function
> __acpi_node_get_property_reference() which ignores the nargs_prop
> parameter.
Which I suppose is a problem. Why is it so?
> To fix this, make __acpi_node_get_property_reference() to
> call the static acpi_fwnode_get_reference() so that callers of
> fwnode_get_reference_args() can still pass a valid property name to
> fetch the number of arguments.
Are the current callers of acpi_fwnode_get_reference_args() going to
be affected by this change and if so, then how?
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> drivers/acpi/property.c | 101 ++++++++++++++++++++--------------------
> 1 file changed, 50 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 436019d96027..d4863746fb11 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -882,45 +882,10 @@ static struct fwnode_handle *acpi_parse_string_ref(const struct fwnode_handle *f
> return &dn->fwnode;
> }
>
> -/**
> - * __acpi_node_get_property_reference - returns handle to the referenced object
> - * @fwnode: Firmware node to get the property from
> - * @propname: Name of the property
> - * @index: Index of the reference to return
> - * @num_args: Maximum number of arguments after each reference
> - * @args: Location to store the returned reference with optional arguments
> - * (may be NULL)
> - *
> - * Find property with @name, verifify that it is a package containing at least
> - * one object reference and if so, store the ACPI device object pointer to the
> - * target object in @args->adev. If the reference includes arguments, store
> - * them in the @args->args[] array.
> - *
> - * If there's more than one reference in the property value package, @index is
> - * used to select the one to return.
> - *
> - * It is possible to leave holes in the property value set like in the
> - * example below:
> - *
> - * Package () {
> - * "cs-gpios",
> - * Package () {
> - * ^GPIO, 19, 0, 0,
> - * ^GPIO, 20, 0, 0,
> - * 0,
> - * ^GPIO, 21, 0, 0,
> - * }
> - * }
> - *
> - * Calling this function with index %2 or index %3 return %-ENOENT. If the
> - * property does not contain any more values %-ENOENT is returned. The NULL
> - * entry must be single integer and preferably contain value %0.
> - *
> - * Return: %0 on success, negative error code on failure.
> - */
> -int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
> - const char *propname, size_t index, size_t num_args,
> - struct fwnode_reference_args *args)
> +static int acpi_fwnode_get_reference_args(const struct fwnode_handle *fwnode,
> + const char *propname, const char *nargs_prop,
> + unsigned int args_count, unsigned int index,
> + struct fwnode_reference_args *args)
> {
> const union acpi_object *element, *end;
> const union acpi_object *obj;
> @@ -999,7 +964,7 @@ 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, args_count);
> if (ret < 0)
> return ret;
>
> @@ -1017,7 +982,7 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
>
> ret = acpi_get_ref_args(idx == index ? args : NULL,
> ref_fwnode, &element, end,
> - num_args);
> + args_count);
> if (ret < 0)
> return ret;
>
> @@ -1039,6 +1004,50 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
>
> return -ENOENT;
> }
> +
> +/**
> + * __acpi_node_get_property_reference - returns handle to the referenced object
> + * @fwnode: Firmware node to get the property from
> + * @propname: Name of the property
> + * @index: Index of the reference to return
> + * @num_args: Maximum number of arguments after each reference
> + * @args: Location to store the returned reference with optional arguments
> + * (may be NULL)
> + *
> + * Find property with @name, verifify that it is a package containing at least
> + * one object reference and if so, store the ACPI device object pointer to the
> + * target object in @args->adev. If the reference includes arguments, store
> + * them in the @args->args[] array.
> + *
> + * If there's more than one reference in the property value package, @index is
> + * used to select the one to return.
> + *
> + * It is possible to leave holes in the property value set like in the
> + * example below:
> + *
> + * Package () {
> + * "cs-gpios",
> + * Package () {
> + * ^GPIO, 19, 0, 0,
> + * ^GPIO, 20, 0, 0,
> + * 0,
> + * ^GPIO, 21, 0, 0,
> + * }
> + * }
> + *
> + * Calling this function with index %2 or index %3 return %-ENOENT. If the
> + * property does not contain any more values %-ENOENT is returned. The NULL
> + * entry must be single integer and preferably contain value %0.
> + *
> + * Return: %0 on success, negative error code on failure.
> + */
> +int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
> + const char *propname, size_t index,
> + size_t num_args,
> + struct fwnode_reference_args *args)
> +{
> + return acpi_fwnode_get_reference_args(fwnode, propname, NULL, index, num_args, args);
> +}
> EXPORT_SYMBOL_GPL(__acpi_node_get_property_reference);
>
> static int acpi_data_prop_read_single(const struct acpi_device_data *data,
> @@ -1558,16 +1567,6 @@ acpi_fwnode_property_read_string_array(const struct fwnode_handle *fwnode,
> val, nval);
> }
>
> -static int
> -acpi_fwnode_get_reference_args(const struct fwnode_handle *fwnode,
> - const char *prop, const char *nargs_prop,
> - unsigned int args_count, unsigned int index,
> - struct fwnode_reference_args *args)
> -{
> - return __acpi_node_get_property_reference(fwnode, prop, index,
> - args_count, args);
> -}
> -
> static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
> {
> const struct acpi_device *adev;
> --
> 2.43.0
>
>
On Wed, Jul 02, 2025 at 12:07:36PM +0200, Rafael J. Wysocki wrote: > On Wed, Jul 2, 2025 at 7:15 AM Anup Patel <apatel@ventanamicro.com> wrote: > > > > From: Sunil V L <sunilvl@ventanamicro.com> > > > > Currently acpi_fwnode_get_reference_args() calls the public function > > __acpi_node_get_property_reference() which ignores the nargs_prop > > parameter. > > Which I suppose is a problem. Why is it so? > fwnode_property_get_reference_args() documents as below. * @nargs_prop: The name of the property telling the number of * arguments in the referred node. NULL if @nargs is known, * otherwise @nargs is ignored. Only relevant on OF. * @nargs: Number of arguments. Ignored if @nargs_prop is non-NULL. You can see that nargs_prop is not supported with ACPI currently. Since fwnode_property_get_reference_args() calls __acpi_node_get_property_reference(), there is no way to determine the nargs from the nargs_prop currently with ACPI. Since fwnode_property_get_reference_args() is a common API across DT and ACPI, it is a problem for users. > > To fix this, make __acpi_node_get_property_reference() to > > call the static acpi_fwnode_get_reference() so that callers of > > fwnode_get_reference_args() can still pass a valid property name to > > fetch the number of arguments. > > Are the current callers of acpi_fwnode_get_reference_args() going to > be affected by this change and if so, then how? > Good question!. If some one is currently passing both valid nargs_prop and nargs with ACPI, now with this change it will start getting the value from nargs_prop which was simply ignored earlier. However, I see only 2 combinations how fwnode_property_get_reference_args() is being used. (nargs_prop = NULL) && (args_count !=0) or (nargs_prop != NULL) && (args_count = 0) So, IMO it should be safe to make this change. But let me know if I am missing something. Thanks! Sunil
On Wed, Jul 2, 2025 at 4:45 PM Sunil V L <sunilvl@ventanamicro.com> wrote: > > On Wed, Jul 02, 2025 at 12:07:36PM +0200, Rafael J. Wysocki wrote: > > On Wed, Jul 2, 2025 at 7:15 AM Anup Patel <apatel@ventanamicro.com> wrote: > > > > > > From: Sunil V L <sunilvl@ventanamicro.com> > > > > > > Currently acpi_fwnode_get_reference_args() calls the public function > > > __acpi_node_get_property_reference() which ignores the nargs_prop > > > parameter. > > > > Which I suppose is a problem. Why is it so? > > > fwnode_property_get_reference_args() documents as below. > > * @nargs_prop: The name of the property telling the number of > * arguments in the referred node. NULL if @nargs is known, > * otherwise @nargs is ignored. Only relevant on OF. > * @nargs: Number of arguments. Ignored if @nargs_prop is non-NULL. > > You can see that nargs_prop is not supported with ACPI currently. Since > fwnode_property_get_reference_args() calls > __acpi_node_get_property_reference(), there is no way to determine the > nargs from the nargs_prop currently with ACPI. Since > fwnode_property_get_reference_args() is a common API across DT and ACPI, > it is a problem for users. So the problem is that if nargs_prop is passed to fwnode_property_get_reference_args() and it is a valid "cells" property, OF will use it to obtain the number of reference arguments and ACPI will ignore it. To make it work the same way in both cases, acpi_fwnode_get_reference_args() needs to be modified to take nargs_prop into account properly. This is the key information that needs to go into the patch changelog. > > > To fix this, make __acpi_node_get_property_reference() to > > > call the static acpi_fwnode_get_reference() so that callers of > > > fwnode_get_reference_args() can still pass a valid property name to > > > fetch the number of arguments. > > > > Are the current callers of acpi_fwnode_get_reference_args() going to > > be affected by this change and if so, then how? > > > Good question!. If some one is currently passing both valid nargs_prop and > nargs with ACPI, now with this change it will start getting the value > from nargs_prop which was simply ignored earlier. However, I see only 2 > combinations how fwnode_property_get_reference_args() is being used. > > (nargs_prop = NULL) && (args_count !=0) > or > (nargs_prop != NULL) && (args_count = 0) OK
© 2016 - 2025 Red Hat, Inc.