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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx # for 6.8 and later
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > 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
