[U-Boot] Why is ns16550 guarded by !OF_PLATDATA ?

I'm trying to use the ns16550 DM driver in a platform where havind a dtb in SPL is not plausible, so we're using platdata.
Now for ns16550 The U_BOOT_DRIVER is guarded by !OF_PLATDATA, so the driver is not compiled in SPL. This seems inconsistent with other U_BOOT_DRIVERs. This was introduced in the following commit [1]:
* b2927fb dm: serial: ns16550: Update to support of-platdata
The reasoning was that the platdata structure is unknown, but one would have to provide a 'struct ns16550_platdata' when using platdata, and that is the case with the boards that use this driver with platdata.
Is this a misguided change, or am I missing something deeper? I can prepare a patch to resolve this, if this is the consensus.
Alex
[1] https://lists.denx.de/pipermail/u-boot/2016-July/259744.html

Hi,
On 16 March 2017 at 15:00, Alexandru Gagniuc alex.g@adaptrum.com wrote:
I'm trying to use the ns16550 DM driver in a platform where havind a dtb in SPL is not plausible, so we're using platdata.
Now for ns16550 The U_BOOT_DRIVER is guarded by !OF_PLATDATA, so the driver is not compiled in SPL. This seems inconsistent with other U_BOOT_DRIVERs. This was introduced in the following commit [1]:
- b2927fb dm: serial: ns16550: Update to support of-platdata
The reasoning was that the platdata structure is unknown, but one would have to provide a 'struct ns16550_platdata' when using platdata, and that is the case with the boards that use this driver with platdata.
Is this a misguided change, or am I missing something deeper? I can prepare a patch to resolve this, if this is the consensus.
You could take it out of the #ifdef, but be careful not to call and fdt functions when OF_PLATDATA is defined. You'll have to drop the of_match and compatible strings too.
Alex
[1] https://lists.denx.de/pipermail/u-boot/2016-July/259744.html
Regards, Simon

Do not condition the compilation of the U_BOOT_DRIVER by !OF_PLATDATA. This is inconsistent with the majority of other drivers. This also blocks OF_PLATDATA boards with an 16550-compatible serial from using serial in SPL.
Signed-off-by: Alexandru Gagniuc alex.g@adaptrum.com --- drivers/serial/ns16550.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 1f819d4..9738a47 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -452,8 +452,7 @@ const struct dm_serial_ops ns16550_serial_ops = { .setbrg = ns16550_serial_setbrg, };
-#if !CONFIG_IS_ENABLED(OF_PLATDATA) -#if CONFIG_IS_ENABLED(OF_CONTROL) +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) /* * Please consider existing compatible strings before adding a new * one to keep this table compact. Or you may add a generic "ns16550" @@ -473,13 +472,13 @@ static const struct udevice_id ns16550_serial_ids[] = { { .compatible = "ti,dra742-uart", .data = PORT_NS16550 }, {} }; -#endif +#endif /* OF_CONTROL && !OF_PLATDATA */
#if CONFIG_IS_ENABLED(SERIAL_PRESENT) U_BOOT_DRIVER(ns16550_serial) = { .name = "ns16550_serial", .id = UCLASS_SERIAL, -#if CONFIG_IS_ENABLED(OF_CONTROL) +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) .of_match = ns16550_serial_ids, .ofdata_to_platdata = ns16550_serial_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct ns16550_platdata), @@ -489,6 +488,6 @@ U_BOOT_DRIVER(ns16550_serial) = { .ops = &ns16550_serial_ops, .flags = DM_FLAG_PRE_RELOC, }; -#endif -#endif /* !OF_PLATDATA */ +#endif /* SERIAL_PRESENT */ + #endif /* CONFIG_DM_SERIAL */

On 27 March 2017 at 13:54, Alexandru Gagniuc alex.g@adaptrum.com wrote:
Do not condition the compilation of the U_BOOT_DRIVER by !OF_PLATDATA. This is inconsistent with the majority of other drivers. This also blocks OF_PLATDATA boards with an 16550-compatible serial from using serial in SPL.
Signed-off-by: Alexandru Gagniuc alex.g@adaptrum.com
drivers/serial/ns16550.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On 31 March 2017 at 22:23, Simon Glass sjg@chromium.org wrote:
On 27 March 2017 at 13:54, Alexandru Gagniuc alex.g@adaptrum.com wrote:
Do not condition the compilation of the U_BOOT_DRIVER by !OF_PLATDATA. This is inconsistent with the majority of other drivers. This also blocks OF_PLATDATA boards with an 16550-compatible serial from using serial in SPL.
Signed-off-by: Alexandru Gagniuc alex.g@adaptrum.com
drivers/serial/ns16550.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!
participants (2)
-
Alexandru Gagniuc
-
Simon Glass