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

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

On 2023-07-20 7:05 PM, Rafael J. Wysocki wrote:
> On Thu, Jul 20, 2023 at 7:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> On Thu, Jul 20, 2023 at 11:15 AM Cezary Rojewski
>> <cezary.rojewski@intel.com> 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!