Re: [PATCH v7 14/24] ACPI: property: Refactor acpi_fwnode_get_reference_args()

["Rafael J. Wysocki" <rafael@kernel.org>]

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?

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