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 <troykiskybound...@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 <troykiskybound...@gmail.com> > Reviewed-by: Simon Glass <s...@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. -- Tom
signature.asc
Description: PGP signature