Re: [PATCH v2 2/4] ACPI: NHLT: Introduce acpi_gbl_NHLT

[Cezary Rojewski <cezary.rojewski@xxxxxxxxx>]

On 2023-07-17 5:00 PM, Cezary Rojewski wrote:
> While there is no strict limit to amount of NHLT tables present, usually
> just the first one is utilized. To simplify implementation of sound
> drivers, provide publicly accessible pointer. Accessing it after calling
> acpi_nhlt_get_gbl_table() yields the first NHLT table met during the
> scan.

...

> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -594,6 +594,9 @@ config ACPI_PRMT
>   	  substantially increase computational overhead related to the
>   	  initialization of some server systems.
>   
> +config ACPI_NHLT
> +	bool
> +
>   endif	# ACPI
>   
>   config X86_PM_TIMER
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index feb36c0b9446..8de34970e7db 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -93,6 +93,7 @@ obj-$(CONFIG_ACPI)		+= container.o
>   obj-$(CONFIG_ACPI_THERMAL)	+= thermal.o
>   obj-$(CONFIG_ACPI_PLATFORM_PROFILE) 	+= platform_profile.o
>   obj-$(CONFIG_ACPI_NFIT)		+= nfit/
> +obj-$(CONFIG_ACPI_NHLT)		+= nhlt.o
>   obj-$(CONFIG_ACPI_NUMA)		+= numa/
>   obj-$(CONFIG_ACPI)		+= acpi_memhotplug.o
>   obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o
> diff --git a/drivers/acpi/nhlt.c b/drivers/acpi/nhlt.c
> new file mode 100644
> index 000000000000..90d74d0d803e
> --- /dev/null
> +++ b/drivers/acpi/nhlt.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright(c) 2023 Intel Corporation. All rights reserved.
> +//
> +// Authors: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
> +//          Amadeusz Slawinski <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
> +//
> +
> +#include <linux/export.h>
> +#include <acpi/nhlt.h>
> +
> +struct acpi_table_nhlt *acpi_gbl_NHLT;
> +EXPORT_SYMBOL_GPL(acpi_gbl_NHLT);

This approach generates a problem with undefined symbol "acpi_gbl_NHLT" 
when ACPI_NHLT is disabled. As nhlt.c is not built when said kconfig is 
disabled, symbol is never defined.

Proposed solution - modify drivers/acpi/tables.c with:

+#include <acpi/nhlt.h>
+
+struct acpi_table_nhlt *acpi_gbl_NHLT;
+EXPORT_SYMBOL_GPL(acpi_gbl_NHLT);

as tables.c is always built the symbol is always there.
The only other option I see is:

-obj-$(CONFIG_ACPI_NHLT)	+= nhlt.o
+obj-y				+= nhlt.o

and modifying nhlt.c so it's essentially split in half with:
#if IS_ENABLED(CONFIG_ACPI_NHLT)

but such solutions stinks. I prefer the first approach.
What to you find guys?

> diff --git a/include/acpi/nhlt.h b/include/acpi/nhlt.h
> index af3ec45ba4f9..a2b93b08218f 100644
> --- a/include/acpi/nhlt.h
> +++ b/include/acpi/nhlt.h
> @@ -13,6 +13,24 @@
>   #include <linux/overflow.h>
>   #include <linux/types.h>
>   
> +/* System-wide pointer to the first NHLT table. */
> +extern struct acpi_table_nhlt *acpi_gbl_NHLT;
> +
> +/*
> + * A sound driver may utilize the two below on its initialization and removal
> + * respectively to avoid excessive mapping and unmapping of the memory
> + * occupied by the table between streaming operations.
> + */
> +static inline acpi_status acpi_nhlt_get_gbl_table(void)
> +{
> +	return acpi_get_table(ACPI_SIG_NHLT, 0, (struct acpi_table_header **)(&acpi_gbl_NHLT));
> +}
> +
> +static inline void acpi_nhlt_put_gbl_table(void)
> +{
> +	acpi_put_table((struct acpi_table_header *)acpi_gbl_NHLT);
> +}
> +
>   #define __acpi_nhlt_endpoint_cfg(ep)	((void *)((ep) + 1))
>   
>   /*