Hi Tom,
On Wed, 10 May 2023 at 14:41, Tom Rini <tr...@konsulko.com> wrote:
>
> On Mon, Mar 13, 2023 at 02:31:39PM -0700, Troy Kisky wrote:
>
> > When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI)
> > bdf is no longer accessible. So add preprocessor protection
> > to avoid access.
> >
> > Signed-off-by: Troy Kisky <troykiskybound...@gmail.com>
> > Reviewed-by: Simon Glass <s...@chromium.org>
> > ---
> >
> > (no changes since v2)
> >
> > Changes in v2:
> > - 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
> >
> > arch/x86/cpu/apollolake/uart.c | 4 ++++
> > include/ns16550.h | 2 +-
> > 2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > 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(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);
> > }
>
> Looking at this again, this hunk here doesn't make sense. Reading
> outloud, "if we have CONFIG_PCI enabled and CONFIG_SPL_BUILD, declare
> plat to be .. and then if we do not have PCI enabled ...". I really
> don't know what the code is supposed to be doing here.
This is because the bdf member is only present in SPL when PCI is enabled.
Perhaps it could be CONFIG_SPL instead of CONFIG_SPL_BUILD ?
You can apply whatever makes it build if you like. I can look at it
either tonight or when I get back.
Regards,
Simon
