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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-07-20 7:05 PM, Rafael J. Wysocki wrote:
On Thu, Jul 20, 2023 at 7:01 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:

On Thu, Jul 20, 2023 at 11:15 AM Cezary Rojewski
<cezary.rojewski@xxxxxxxxx> wrote:

...

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;

No capitals in variable names, please.

acpi_gbl_NHLT follows the path set by acpi_gbl_DSDT, _FADT and others. Why would NHLT be an exception? Is this because it's not defined under ACPICA?

Uncapitalizing nonetheless in v3.

+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?

I leave this to Rafael as it's his territory.

Rafael, which option do you prefer?

Regardless of IKP and my CI returning success on compilation tests,
clearly there is a problem when CONFIG_ACPI_NHLT.

Putting the definition of acpi_gbl_NHLT into tables.c would be fine
with me, but in any case, because it is an exported symbol, it needs a
description in a kerneldoc comment.

That said, you can also do something like this in a header file:

#ifdef CONFIG_ACPI_NHLT
extern struct acpi_table_nhlt *acpi_gbl_nhlt;
#else
#define acpi_gbl_nhlt    NULL
#endif

and require the acpi_gbl_nhlt users to include it.

Simplest solutions usually work the best. Surprised I haven't thought about it earlier!



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux