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

Troy Kisky
2023-02-22T19:39:41Z

On Wed, Feb 22, 2023 at 10:42 AM Troy Kisky <troykiskyboundary@gmail.com>
wrote:

> On Wed, Feb 22, 2023 at 10:18 AM Tom Rini <trini@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@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) && IS_ENABLED_NOCHECK(CONFIG_PCI)
ns.bdf = pci_ofplat_get_devfn(dtplat->reg[0]);
+#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)

/*
* 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
};