On Fri, Sep 23, 2016 at 4:57 PM, Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> wrote: > DT allows holes or empty phandles for references. This is used for example > in SPI subsystem where some chip selects are native and others are regular > GPIOs. In ACPI _DSD we currently do not support this but instead the > preceding reference "consumes" all following integer arguments. > > For example we would like to support something like the below ASL fragment > for SPI: > > Package () { > "cs-gpios", > Package () { > ^GPIO, 19, 0, 0, // GPIO CS0 > 0, // Native CS > ^GPIO, 20, 0, 0, // GPIO CS1 > } > } > > The zero in the middle means "no entry" or NULL reference. To support this > we add a new function acpi_node_get_property_reference_fixed_args() that > takes number of expected arguments as parameter. Function returns -ENOENT > if the corresponding index resolves to the "no entry" reference. > > Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > --- > drivers/acpi/property.c | 116 +++++++++++++++++++++++++++++++++++------------- > include/linux/acpi.h | 3 ++ > 2 files changed, 89 insertions(+), 30 deletions(-) > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > index f2fd3fee588a..b255c4442a05 100644 > --- a/drivers/acpi/property.c > +++ b/drivers/acpi/property.c > @@ -472,6 +472,7 @@ static int acpi_data_get_property_array(struct acpi_device_data *data, > * @data: ACPI device data object containing the property > * @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 > * > * Find property with @name, verifify that it is a package containing at least > @@ -485,8 +486,8 @@ static int acpi_data_get_property_array(struct acpi_device_data *data, > * Return: %0 on success, negative error code on failure. > */ > static int acpi_data_get_property_reference(struct acpi_device_data *data, > - const char *propname, size_t index, > - struct acpi_reference_args *args) > + const char *propname, size_t index, size_t num_args, > + struct acpi_reference_args *args) > { > const union acpi_object *element, *end; > const union acpi_object *obj; > @@ -532,41 +533,52 @@ static int acpi_data_get_property_reference(struct acpi_device_data *data, > while (element < end) { > u32 nargs, i; > > - if (element->type != ACPI_TYPE_LOCAL_REFERENCE) > - return -EPROTO; > - > - ret = acpi_bus_get_device(element->reference.handle, &device); > - if (ret) > - return -ENODEV; > - > - element++; > - nargs = 0; > - > - /* assume following integer elements are all args */ > - for (i = 0; element + i < end; i++) { > - int type = element[i].type; > + if (element->type == ACPI_TYPE_LOCAL_REFERENCE) { > + ret = acpi_bus_get_device(element->reference.handle, > + &device); > + if (ret) > + return -ENODEV; > + > + nargs = 0; > + element++; > + > + /* assume following integer elements are all args */ > + for (i = 0; element + i < end && i < num_args; i++) { > + int type = element[i].type; > + > + if (type == ACPI_TYPE_INTEGER) > + nargs++; > + else if (type == ACPI_TYPE_LOCAL_REFERENCE) > + break; > + else > + return -EPROTO; > + } > > - if (type == ACPI_TYPE_INTEGER) > - nargs++; > - else if (type == ACPI_TYPE_LOCAL_REFERENCE) > - break; > - else > + if (nargs > MAX_ACPI_REFERENCE_ARGS) > return -EPROTO; > - } > > - if (idx++ == index) { > - args->adev = device; > - args->nargs = nargs; > - for (i = 0; i < nargs; i++) > - args->args[i] = element[i].integer.value; > + if (idx == index) { > + args->adev = device; > + args->nargs = nargs; > + for (i = 0; i < nargs; i++) > + args->args[i] = element[i].integer.value; > > - return 0; > + return 0; > + } > + > + element += nargs; > + } else if (element->type == ACPI_TYPE_INTEGER) { > + if (idx == index) > + return -ENOENT; > + element++; > + } else { > + return -EPROTO; > } > > - element += nargs; > + idx++; > } > > - return -EPROTO; > + return -ENODATA; > } > > /** > @@ -582,10 +594,54 @@ int acpi_node_get_property_reference(struct fwnode_handle *fwnode, > { > struct acpi_device_data *data = acpi_device_data_of_node(fwnode); > > - return data ? acpi_data_get_property_reference(data, name, index, args) : -EINVAL; > + if (!data) > + return -EINVAL; > + > + return acpi_data_get_property_reference(data, name, index, > + MAX_ACPI_REFERENCE_ARGS, args); > } > EXPORT_SYMBOL_GPL(acpi_node_get_property_reference); gpiolib-acpi is the only user of this AFAICS, so why don't we change the definition of acpi_node_get_property_reference() to take one extra argument and update the caller to pass MAX_ACPI_REFERENCE_ARGS? We'd avoid having to define the thing below then. > > +/** > + * acpi_node_get_property_reference_fixed_args - get a reference from property > + * @fwnode: Firmware node to get the property from. > + * @propname: Name of the property. > + * @index: Index of the reference to return. > + * @num_args: Expected number of arguments following each reference. > + * @args: Location to store the returned reference with optional arguments. > + * > + * Using this function it is possible to leave "holes" in the property > + * value set like in an 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 return %-ENOENT and with index %3 > + * returns the last entry. If the property does not contain any more values > + * %-ENODATA is returned. The NULL entry must be single integer and > + * preferably contain value %0. > + */ > +int acpi_node_get_property_reference_fixed_args(struct fwnode_handle *fwnode, > + const char *name, size_t index, size_t num_args, > + struct acpi_reference_args *args) > +{ > + struct acpi_device_data *data = acpi_device_data_of_node(fwnode); > + > + if (!data) > + return -EINVAL; > + > + return acpi_data_get_property_reference(data, name, index, num_args, > + args); > +} > +EXPORT_SYMBOL_GPL(acpi_node_get_property_reference_fixed_args); > + > static int acpi_data_prop_read_single(struct acpi_device_data *data, > const char *propname, > enum dev_prop_type proptype, void *val) > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index c5eaf2f80a4c..c5d7c503843f 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -930,6 +930,9 @@ int acpi_dev_get_property(struct acpi_device *adev, const char *name, > int acpi_node_get_property_reference(struct fwnode_handle *fwnode, > const char *name, size_t index, > struct acpi_reference_args *args); > +int acpi_node_get_property_reference_fixed_args(struct fwnode_handle *fwnode, > + const char *name, size_t index, size_t num_args, > + struct acpi_reference_args *args); > > int acpi_node_prop_get(struct fwnode_handle *fwnode, const char *propname, > void **valptr); > -- Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html