Re: [PATCH v1 2/4] ACPI: property: Add code comments explaining what is going on

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

Hi Rafael,

On Mon, Sep 15, 2025 at 02:27:16PM +0200, Rafael J. Wysocki wrote:
> Hi Sakari,
> 
> On Mon, Sep 15, 2025 at 1:32 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Rafael,
> >
> > On Fri, Sep 12, 2025 at 09:40:58PM +0200, Rafael J. Wysocki wrote:
> > > +                     /*
> > > +                      * The reference is expected to point to an object
> > > +                      * returning a package that contains _DSD-equivalent
> > > +                      * information.
> > > +                      */
> > >                       handle = link->package.elements[1].reference.handle;
> > >                       result = acpi_nondev_subnode_data_ok(handle, link, list,
> > >                                                            parent);
> > >                       break;
> > >               case ACPI_TYPE_PACKAGE:
> >
> > And similarly, the result of an evaluation here is a package when a
> > reference points to a name object (i.e. a data node).
> 
> Well, I'm not sure how this remark is related to the new comment below.
> 
> Do you mean that this always happens when a reference is used in ASL
> to point to the target here?

As long as the target is a non-device object (or name or method object at
least), which is required by DSD-guide for (non-string-)referenced objects.

> 
> But the comment would still be valid in that case, wouldn't it?

After re-reading the first paragraph, I agree.

> 
> > > +                     /*
> > > +                      * This happens when the target package is embedded
> > > +                      * within the links package as a result of direct
> > > +                      * evaluation of an object pointed to by a reference.
> > > +                      *
> > > +                      * The target package is expected to contain _DSD-
> > > +                      * equivalent information, but the scope in which it
> > > +                      * is located in the original AML is unknown.  Thus
> > > +                      * it cannot contain pathname segments represented as
> > > +                      * strings because there is no way to build full
> > > +                      * pathnames out of them.

Is the "original AML" relevant? Aren't we just interested in how the
evaluation result was reached instead of what was its actual path? We won't
know the latter in any case. What would you think of:

			/*
			 * Evaluating a reference results in a package object
			 * (required by DSD guide) allocated on the fly. The
			 * actual target object of the reference isn't
			 * available.
			 */

I guess nothing prevents having further string references within the
object?

I think it'd be best to deprecate direct references in the DSD guide.

> > > +                      */
> > >                       desc = &link->package.elements[1];
> > >                       result = acpi_nondev_subnode_extract(desc, NULL, link,
> > >                                                            list, parent);
> > >
> >

-- 
Kind regards,

Sakari Ailus