[PATCH v3 17/23] ns16550: match when to define bdf with uart code

[Tom Rini]

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 at 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 at gmail.com>
>     Reviewed-by: Simon Glass <sjg at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230510/bea38230/attachment.sig>