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: > > > > On 2023-07-19 5:36 PM, Andy Shevchenko wrote: > > > On Wed, Jul 19, 2023 at 04:47:31PM +0200, Cezary Rojewski wrote: > > >> On 2023-07-17 5:00 PM, Cezary Rojewski wrote: > > > > ... > > > > >>> +++ 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; No capitals in variable names, please. > > >> +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.
