Re: [PATCH 3/4] ACPI: NHLT: Table manipulation helpers

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2023-07-12 5:36 PM, Andy Shevchenko wrote:
> On Wed, Jul 12, 2023 at 11:10:47AM +0200, Cezary Rojewski wrote:
>> The table is composed of a range of endpoints with each describing
>> audio formats they support. Thus most of the operations involve
>> iterating over elements of the table. Simplify the process by
>> implementing range of getters.
> 
> A few nit-picks below.
> In general, LGTM,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> (Please, use my @linux.intel.com address for LKML and related)

...

>> +/*
>> + * The for_each_nhlt_xxx() macros rely on an iterator to deal with the
> 
> I would do s/xxx/*/

Ack.

>> + * variable length of each endpoint structure and the possible presence
>> + * of an OED-Config used by Windows only.
>> + */
>> +
>> +/**
>> + * for_each_nhlt_endpoint - Iterate over endpoints in a NHLT table.
>> + * @tb:		the pointer to a NHLT table.
>> + * @ep:		the pointer to endpoint to use as loop cursor.
>> + */
>> +#define for_each_nhlt_endpoint(tb, ep)					\
>> +	for (unsigned int __i = 0;					\
>> +	     __i < (tb)->endpoint_count &&				\
>> +		((ep) = __acpi_nhlt_get_endpoint(tb, ep, __i));		\
> 
> Do you really need ep to be in parentheses?

Agree. Also, checked how include/linux/list.h looks like and it aligns 
with your proposal.

>> +	     __i++)
>> +
>> +/**
>> + * for_each_nhlt_fmtcfg - Iterate over format configurations.
>> + * @fmts:	the pointer to formats configuration space.
>> + * @fmt:	the pointer to format to use as loop cursor.
>> + */
>> +#define for_each_nhlt_fmtcfg(fmts, fmt)					\
>> +	for (unsigned int __i = 0;					\
>> +	     __i < (fmts)->formats_count &&				\
>> +		((fmt) = __acpi_nhlt_get_fmtcfg(fmts, fmt, __i));	\
> 
> Similar for fmt.

Ditto.

>> +	     __i++)