On Wed, May 10, 2023 at 03:05:38PM -0700, Troy Kisky wrote:
Hi Tom,
You are looking at an old patch, here's the new.
It was the new one, sorry, I just edited out NOCHECK at least for now.
commit c969bedb9cb6029360e6fe7e25a331680fabe3ee Author: Troy Kisky troykiskyboundary@gmail.com Date: Thu Feb 23 08:01:46 2023 -0800
ns16550: match when to define bdf with uart code When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI) bdf is no longer accessible. So add preprocessor protection to avoid access. Series-changes: 2 - changed condition of when to include field bdf - added protection to another instance of bdf in uart.c - Thanks to Simon for getting this corrected Signed-off-by: Troy Kisky <troykiskyboundary@gmail.com> Reviewed-by: Simon Glass <sjg@chromium.org>diff --git a/arch/x86/cpu/apollolake/uart.c b/arch/x86/cpu/apollolake/uart.c index a9362436000..878aa48ed76 100644 --- a/arch/x86/cpu/apollolake/uart.c +++ b/arch/x86/cpu/apollolake/uart.c @@ -79,10 +79,12 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
static int apl_ns16550_probe(struct udevice *dev) { +#if IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) struct apl_ns16550_plat *plat = dev_get_plat(dev);
if (!CONFIG_IS_ENABLED(PCI)) apl_uart_init(plat->ns16550.bdf, plat->ns16550.base);+#endif
return ns16550_serial_probe(dev);} @@ -110,7 +112,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 IS_ENABLED_NOCHECK(CONFIG_PCI) && defined(CONFIG_SPL_BUILD) ns.bdf = pci_ofplat_get_devfn(dtplat->reg[0]); +#endif memcpy(plat, &ns, sizeof(ns)); #else int ret; diff --git a/include/ns16550.h b/include/ns16550.h index e7e68663d03..41b977b5b26 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_BUILD) int bdf; #endif }; __________________________
It reads as, If the bdf exists, then if spl var PCI is not enabled, then init uart.
The PCI code will handle it if PCI is enabled.
So, PCI needs to be enabled, and SPL_PCI needs to not be enabled, and currently building for SPL
So the case is CONFIG_PCI=y, CONFIG_SPL_PCI=n and CONFIG_SPL_BUILD=y? That's what the code needs to test and express clearly and likely with a comment.