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@xxxxxxxxxxxxxxx>
(Please, use my @linux.intel.com address for LKML and related)
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
> ---
> include/acpi/nhlt.h | 68 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/include/acpi/nhlt.h b/include/acpi/nhlt.h
> index a2b93b08218f..076aac41a74e 100644
> --- a/include/acpi/nhlt.h
> +++ b/include/acpi/nhlt.h
> @@ -81,4 +81,72 @@ static inline void acpi_nhlt_put_gbl_table(void)
> __cfg->capabilities_size == struct_size(__cfg, mics, __cfg->num_mics) ? \
> __cfg : NULL; })
>
> +/**
> + * acpi_nhlt_endpoint_fmtscfg - Get the formats configuration space.
> + * @ep: the endpoint to retrieve the space for.
> + *
> + * Return: A pointer to the formats configuration space.
> + */
> +static inline struct acpi_nhlt_formats_config *
> +acpi_nhlt_endpoint_fmtscfg(const struct acpi_nhlt_endpoint *ep)
> +{
> + struct acpi_nhlt_cfg *cfg = __acpi_nhlt_endpoint_cfg(ep);
> +
> + return (struct acpi_nhlt_formats_config *)((u8 *)(cfg + 1) + cfg->capabilities_size);
> +}
> +
> +#define __acpi_nhlt_first_endpoint(tb) \
> + ((void *)(tb + 1))
> +
> +#define __acpi_nhlt_next_endpoint(ep) \
> + ((void *)((u8 *)(ep) + (ep)->descriptor_length))
> +
> +#define __acpi_nhlt_get_endpoint(tb, ep, i) \
> + ((i) ? __acpi_nhlt_next_endpoint(ep) : __acpi_nhlt_first_endpoint(tb))
> +
> +#define __acpi_nhlt_first_fmtcfg(fmts) \
> + ((void *)(fmts + 1))
> +
> +#define __acpi_nhlt_next_fmtcfg(fmt) \
> + ((void *)((u8 *)((fmt) + 1) + (fmt)->capability_size))
> +
> +#define __acpi_nhlt_get_fmtcfg(fmts, fmt, i) \
> + ((i) ? __acpi_nhlt_next_fmtcfg(fmt) : __acpi_nhlt_first_fmtcfg(fmts))
> +
> +/*
> + * The for_each_nhlt_xxx() macros rely on an iterator to deal with the
I would do s/xxx/*/
> + * 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?
> + __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.
> + __i++)
> +
> +/**
> + * for_each_nhlt_endpoint_fmtcfg - Iterate over format configurations in an endpoint.
> + * @ep: the pointer to an endpoint.
> + * @fmt: the pointer to format to use as loop cursor.
> + */
> +#define for_each_nhlt_endpoint_fmtcfg(ep, fmt) \
> + for_each_nhlt_fmtcfg(acpi_nhlt_endpoint_fmtscfg(ep), fmt)
> +
> #endif /* __ACPI_NHLT_H__ */
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
