On Wed, May 10, 2023 at 02:46:14PM -0600, Simon Glass wrote:
Hi Tom,
On Wed, 10 May 2023 at 14:41, Tom Rini trini@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 troykiskyboundary@gmail.com Reviewed-by: Simon Glass sjg@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.
Digging at this when you get back is fine since it still makes no sense to say "If PCI is enabled, when PCI is not enabled actually do something". There's no side-effects on "dev" from the call to dev_get_plat which is the only thing that happens, in this case.