Re: [PATCH 1/1] ACPI: property: Fix REF STR... reference parsing

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Rafael,

On Mon, Mar 31, 2025 at 03:19:07PM +0200, Rafael J. Wysocki wrote:
> On Mon, Mar 31, 2025 at 2:18 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Restore parsing of ACPI data node references consisting of a device node
> > reference followed by one or more child data node names.
> >
> > Fixes: 9880702d123f ("ACPI: property: Support using strings in reference properties")
> > Cc: stable@vger.kernel.org # for 6.8 and later
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > Hi Rafael,
> >
> > It seems that support for REF STR... references got accidentally removed
> > when pure STR reference were added. The former are documented in
> > Documentation/firmware-guide/acpi/dsd/graph.rst .
> 
> It would be good to provide an ASL example that is not parsed as
> expected before the change and will be parsed correctly after it.
> 
> Admittedly, I can't quite recall the history leading to the above
> commit, but this paragraphs is present in its changelog:
> 
> "Also remove the mechanism allowing data nodes to be referenced
>  indirectly, with the help of an object reference pointing to the
>  "ancestor" device and a path relative to it (this mechanism is not
>  expected to be in use in any production platform firmware in the field)."
> 
> so the change in question appears to be intentional rather than accidental.

Right. This still continues to be documented in
Documentation/firmware-guide/acpi/dsd/graph.rst as noted in my previous
e-mail. A sample ASL snippet is can also be found in the same file. I only
noticed this when I tried to use such references.

The other option indeed is to change the documentation and hope no-one
depends on it.

> 
> >  drivers/acpi/property.c | 23 ++++++++++++++++++++---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> > index 436019d96027..4e3202a0b305 100644
> > --- a/drivers/acpi/property.c
> > +++ b/drivers/acpi/property.c
> > @@ -807,10 +807,27 @@ acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
> >  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 follow_strings)
> >  {
> >         u32 nargs = 0, i;
> >
> > +       /*
> > +        * Parse REF STR... references by following named child nodes below the
> > +        * device node pointed by REF.
> > +        */
> > +       if (follow_strings) {
> > +               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 (possibly represented as a string) or end of the
> > @@ -999,7 +1016,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, num_args, true);
> >                         if (ret < 0)
> >                                 return ret;
> >
> > @@ -1017,7 +1034,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);
> > +                                               num_args, false);
> >                         if (ret < 0)
> >                                 return ret;
> >

-- 
Regards,

Sakari Ailus