Re: [PATCH v1 17/24] ns16550: match when to define bdf with uart code

Simon Glass
2023-02-22T21:17:05Z

Hi Troy,

On Wed, 22 Feb 2023 at 14:06, Troy Kisky <troykiskyboundary@gmail.com> wrote:
>
> On Wed, Feb 22, 2023 at 12:41 PM Troy Kisky <troykiskyboundary@gmail.com> wrote:
>>
>>
>>
>>>
>>> How does this look ?
>>>
>>> diff --git a/arch/x86/cpu/apollolake/uart.c b/arch/x86/cpu/apollolake/uart.c
>>> index a9362436000..da184638cb9 100644
>>> --- a/arch/x86/cpu/apollolake/uart.c
>>> +++ b/arch/x86/cpu/apollolake/uart.c
>>> @@ -79,10 +79,11 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
>>>
>>> static int apl_ns16550_probe(struct udevice *dev)
>>> {
>>> +#if defined(CONFIG_SPL) && IS_ENABLED_NOCHECK(CONFIG_PCI)
>>> struct apl_ns16550_plat *plat = dev_get_plat(dev);
>>>
>>> - if (!CONFIG_IS_ENABLED(PCI))
>>> - apl_uart_init(plat->ns16550.bdf, plat->ns16550.base);
>>> + apl_uart_init(plat->ns16550.bdf, plat->ns16550.base);
>>> +#endif
>>>
>>> return ns16550_serial_probe(dev);
>>> }
>>> @@ -110,7 +111,9 @@ static int apl_ns16550_of_to_plat(struct udevice *dev)
>>> ns.reg_offset = 0;
>>> ns.clock = dtplat->clock_frequency;
>>> ns.fcr = UART_FCR_DEFVAL;
>>> +#if defined(CONFIG_SPL) && IS_ENABLED_NOCHECK(CONFIG_PCI)
>>> ns.bdf = pci_ofplat_get_devfn(dtplat->reg[0]);
>>> +#endif
>>> memcpy(plat, &ns, sizeof(ns));
>>> #else
>>> int ret;
>>> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
>>> index 2bc704e1104..d18c49686dd 100644
>>> --- a/include/linux/kconfig.h
>>> +++ b/include/linux/kconfig.h
>>> @@ -27,6 +27,7 @@
>>> * 0 otherwise.
>>> */
>>> #define IS_ENABLED(option) config_enabled(option, 0)
>>> +#define IS_ENABLED_NOCHECK(option) config_enabled(option, 0)
>>>
>>> /*
>>> * U-Boot add-on: Helper macros to reference to different macros (prefixed by
>>> diff --git a/include/ns16550.h b/include/ns16550.h
>>> index e7e68663d03..f416e67e68f 100644
>>> --- a/include/ns16550.h
>>> +++ b/include/ns16550.h
>>> @@ -74,7 +74,7 @@ struct ns16550_plat {
>>> int clock;
>>> u32 fcr;
>>> int flags;
>>> -#if defined(CONFIG_PCI) && defined(CONFIG_SPL)
>>> +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL)
>>> int bdf;
>>> #endif
>>> };
>>>
>>
>>
>> Or maybe I should s/CONFIG_SPL/CONFIG_SPL_BUILD/ in these places ?
>
>
> Or better
> defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)
>
> since Simon specified TPL use.

Strangely, TPL_BUILD is a subset of SPL_BUILD. SPL_BUILD means that it
is not U-Boot proper, so could be SPL, TPL or VPL. We should probably
rename that meaning of SPL_BUILD to XPL_BUILD one day, so that
SPL_BUILD really just means SPL.

So no need for the || defined(CONFIG_TPL_BUILD)

Regards,
Simon