On Wed, Feb 22, 2023 at 10:18 AM Tom Rini <tr...@konsulko.com> wrote:
> On Tue, Feb 21, 2023 at 05:38:14PM -0800, Troy Kisky wrote:
>
> > When switching defined(CONFIG_PCI) to CONFIG_IS_ENABLED(PCI)
> > bdf is no longer accessible. So change to preprocessor to avoid access.
> >
> > Signed-off-by: Troy Kisky <troykiskybound...@gmail.com>
> > ---
> >
> > arch/x86/cpu/apollolake/uart.c | 6 +++---
> > include/ns16550.h | 2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/cpu/apollolake/uart.c
> b/arch/x86/cpu/apollolake/uart.c
> > index a9362436000..143217755ff 100644
> > --- a/arch/x86/cpu/apollolake/uart.c
> > +++ b/arch/x86/cpu/apollolake/uart.c
> > @@ -79,11 +79,11 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
> >
> > static int apl_ns16550_probe(struct udevice *dev)
> > {
> > +#if !CONFIG_IS_ENABLED(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);
> > }
> >
> > diff --git a/include/ns16550.h b/include/ns16550.h
> > index e7e68663d03..8d7eb7d8f9c 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 !CONFIG_IS_ENABLED(PCI) || CONFIG_IS_ENABLED(OF_PLATDATA)
> > int bdf;
> > #endif
> > };
>
> This isn't equivalent. This means platforms such as am335x_evm which do
> not enable PCI nor SPL_PCI now get this field and grow their rodata.
>
> --
> Tom
>
tkisky@OS2:~/u-boot-tkisky$ make am335x_evm_defconfig
#
# configuration written to .config
#
tkisky@OS2:~/u-boot-tkisky$ grep -w CONFIG_INTEL_APOLLOLAKE .config
tkisky@OS2:~/u-boot-tkisky$
Would this be better ?
#if defined(CONFIG_INTEL_APOLLOLAKE) && ( !CONFIG_IS_ENABLED(PCI) ||
CONFIG_IS_ENABLED(OF_PLATDATA))
I don't understand what bdf is, but
#if defined(CONFIG_PCI) && defined(CONFIG_SPL)
int bdf;
#endif
in the include/ns16550.h, doesn't make sense with the usage in uart.c
if (!CONFIG_IS_ENABLED(PCI))
apl_uart_init(plat->ns16550.bdf, plat->ns16550.base);
_________
I think I might be trying to paper over a bug.
Thanks
Troy
