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

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

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. 

-- 
Kind regards,

Sakari Ailus