On Tue, May 16, 2023 at 1:24 PM Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > Hi Rafael, > > On Fri, May 12, 2023 at 06:04:26PM +0200, Rafael J. Wysocki wrote: > > On Wed, Mar 29, 2023 at 12:10 PM Sakari Ailus > > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > > > Add support for parsing property references using strings, besides > > > reference objects that were previously supported. This allows also > > > referencing data nodes which was not possible with reference objects. > > > > > > Also add pr_fmt() macro to prefix printouts. > > > > > > While at it, update copyright. > > > > Although I said that it looked good to me, some minor improvements can > > still be made. > > > > First off, the above changelog is a bit terse. > > > > I think that it would help to provide an example of device properties > > that would not be parsed properly before the change and can be parsed > > now. > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > > --- > > > drivers/acpi/property.c | 110 ++++++++++++++++++++++++++++++++++------ > > > 1 file changed, 94 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > > > index b8d9eb9a433e..08831ffba26c 100644 > > > --- a/drivers/acpi/property.c > > > +++ b/drivers/acpi/property.c > > > @@ -2,14 +2,17 @@ > > > /* > > > * ACPI device specific properties support. > > > * > > > - * Copyright (C) 2014, Intel Corporation > > > + * Copyright (C) 2014-2023, Intel Corporation > > > * All rights reserved. > > > * > > > * Authors: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > > > - * Darren Hart <dvhart@xxxxxxxxxxxxxxx> > > > - * Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > > > + * Darren Hart <dvhart@xxxxxxxxxxxxxxx> > > > + * Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > > > > I'm not sure if the whitespace change here is really useful. > > I did that to address a comment from Andy --- the earlier lines used spaces > for indentation. > > > > > > + * Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > > */ > > > > > > +#define pr_fmt(fmt) "ACPI: " fmt > > > + > > > #include <linux/acpi.h> > > > #include <linux/device.h> > > > #include <linux/export.h> > > > @@ -795,7 +798,8 @@ 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 subnode_string) > > > > The meaning of the new argument isn't really clear. it would be good > > to somehow help a casual reader of the code to find this out more > > easily. > > I can add comments to v9. If you can send me an example of ASL that will be parsed correctly after this change, but not before, it will help a bit.