[v3,4/8] ACPI: property: Move property ref argument parsing into a new function

Split out property reference argument parsing out of the
__acpi_node_get_property_reference() function into a new one,
acpi_get_ref_args(). The new function will be needed also for parsing
string references soon.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 105 +++++++++++++++++++++++-----------------
 1 file changed, 60 insertions(+), 45 deletions(-)

On Wed, May 25, 2022 at 04:01:19PM +0300, Sakari Ailus wrote:
> Split out property reference argument parsing out of the
> __acpi_node_get_property_reference() function into a new one,
> acpi_get_ref_args(). The new function will be needed also for parsing
> string references soon.

...

> +static int
> +acpi_get_ref_args(struct fwnode_reference_args *args,

You can at least make these two on one line.

> +		  struct fwnode_handle *ref_fwnode,

Calling it fwnode would save a few lines of code even with your strictness
of 80.

> +		  const union acpi_object **element,
> +		  const union acpi_object *end, size_t num_args)
> +{
> +	u32 nargs = 0, i;
> +
> +	/*
> +	 * Find the referred data extension node under the
> +	 * referred device node.
> +	 */
> +	for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
> +	     (*element)++) {
> +		const char *child_name = (*element)->string.pointer;

I believe this on one line is better to read.

> +		ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode,
> +							      child_name);

This too.

> +		if (!ref_fwnode)
> +			return -EINVAL;
> +	}
> +
> +	/*
> +	 * Assume the following integer elements are all args. Stop counting on
> +	 * the first reference or end of the package arguments. In case of
> +	 * neither reference, nor integer, return an error, we can't parse it.
> +	 */
> +	for (i = 0; (*element) + i < end && i < num_args; i++) {
> +		acpi_object_type type = (*element)[i].type;
> +
> +		if (type == ACPI_TYPE_LOCAL_REFERENCE)
> +			break;
> +
> +		if (type == ACPI_TYPE_INTEGER)
> +			nargs++;
> +		else
> +			return -EINVAL;
> +	}
> +
> +	if (nargs > NR_FWNODE_REFERENCE_ARGS)
> +		return -EINVAL;
> +
> +	if (args) {
> +		args->fwnode = ref_fwnode;
> +		args->nargs = nargs;
> +		for (i = 0; i < nargs; i++)
> +			args->args[i] = (*element)[i].integer.value;
> +	}
> +
> +	(*element) += nargs;
> +
> +	return 0;
> +}

Hi Andy,

On Wed, May 25, 2022 at 08:28:57PM +0300, Andy Shevchenko wrote:
> On Wed, May 25, 2022 at 04:01:19PM +0300, Sakari Ailus wrote:
> > Split out property reference argument parsing out of the
> > __acpi_node_get_property_reference() function into a new one,
> > acpi_get_ref_args(). The new function will be needed also for parsing
> > string references soon.
> 
> ...
> 
> > +static int
> > +acpi_get_ref_args(struct fwnode_reference_args *args,
> 
> You can at least make these two on one line.

Agreed.

> 
> > +		  struct fwnode_handle *ref_fwnode,
> 
> Calling it fwnode would save a few lines of code even with your strictness
> of 80.

I'm just moving the code as much as possible as-is from elsewhere, thus
preferring to keep the naming.

> 
> > +		  const union acpi_object **element,
> > +		  const union acpi_object *end, size_t num_args)
> > +{
> > +	u32 nargs = 0, i;
> > +
> > +	/*
> > +	 * Find the referred data extension node under the
> > +	 * referred device node.
> > +	 */
> > +	for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
> > +	     (*element)++) {
> > +		const char *child_name = (*element)->string.pointer;
> 
> I believe this on one line is better to read.

I assume you mean the for loop.

I have to disargee. Everything doesn't automatically become better by
putting more stuff on the same line.

> 
> > +		ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode,
> > +							      child_name);
> 
> This too.

I think this would be affected less than the loop.