[PATCH v1 17/24] ns16550: match when to define bdf with uart code

Simon Glass sjg at chromium.org
Wed Feb 22 21:56:54 CET 2023


Hi Troy,

On Wed, 22 Feb 2023 at 12:39, Troy Kisky <troykiskyboundary at gmail.com> wrote:
>
>
> On Wed, Feb 22, 2023 at 10:42 AM Troy Kisky <troykiskyboundary at gmail.com> wrote:
>>
>> On Wed, Feb 22, 2023 at 10:18 AM Tom Rini <trini at 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 <troykiskyboundary at 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
>
>
> How does this look ?
>
> diff --git a/arch/x86/cpu/apollolake/uart.c b/arch/x86/cpu/apollolake/uart.c
> index a9362436000..da184638cb9 100644
> --- a/arch/x86/cpu/apollolake/uart.c
> +++ b/arch/x86/cpu/apollolake/uart.c
> @@ -79,10 +79,11 @@ void apl_uart_init(pci_dev_t bdf, ulong base)
>
>  static int apl_ns16550_probe(struct udevice *dev)
>  {
> +#if defined(CONFIG_SPL) && IS_ENABLED_NOCHECK(CONFIG_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);
>  }
> @@ -110,7 +111,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 defined(CONFIG_SPL) && (CONFIG_PCI)
>         ns.bdf = pci_ofplat_get_devfn(dtplat->reg[0]);

This should only be called when PCI is disabled in the particular SPL phase.

> +#endif
>         memcpy(plat, &ns, sizeof(ns));
>  #else
>         int ret;
> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> index 2bc704e1104..d18c49686dd 100644
> --- a/include/linux/kconfig.h
> +++ b/include/linux/kconfig.h
> @@ -27,6 +27,7 @@
>   * 0 otherwise.
>   */
>  #define IS_ENABLED(option)     config_enabled(option, 0)
> +#define IS_ENABLED_NOCHECK(option) config_enabled(option, 0)

What is this for? It looks the same as the one above.

>
>  /*
>   * U-Boot add-on: Helper macros to reference to different macros (prefixed by
> diff --git a/include/ns16550.h b/include/ns16550.h
> index e7e68663d03..f416e67e68f 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)
>         int bdf;
>  #endif
>  };
>

Regards,
Simon


More information about the U-Boot mailing list