serial: ns16550: da8xx (freon/primus) is not omap-like

These are SoCs in the lineage of TI C6x DSPs, and as such have the same uart as TI Keystone SoCs.

Signed-off-by Matthijs van Duin <[email protected]>

---
 drivers/serial/ns16550.c | 8 ++++----
 include/ns16550.h        | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

drivers/serial/ns16550.c +1 -2

index c702304e79bd..9cec58c887c8 100644
@@ -36,7 +36,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #endif
 #endif /* !CONFIG_DM_SERIAL */
 
-#if defined(CONFIG_SOC_KEYSTONE)
+#if defined(CONFIG_SOC_KEYSTONE) || defined(CONFIG_SOC_DA8XX)
 #define UART_REG_VAL_PWREMU_MGMT_UART_DISABLE   0
 #define UART_REG_VAL_PWREMU_MGMT_UART_ENABLE ((1 << 14) | (1 << 13) | (1 << 0))
 #undef UART_MCRVAL

@@ -181,12 +181,12 @@ void NS16550_init(NS16550_t com_port, int baud_divisor)
 	serial_out(ns16550_getfcr(com_port), &com_port->fcr);
 	if (baud_divisor != -1)
 		NS16550_setbrg(com_port, baud_divisor);
-#if defined(CONFIG_ARCH_OMAP2PLUS) || defined(CONFIG_SOC_DA8XX)

This introduces a warning on all SOC_DA8XX systems because they do not set CONFIG_SYS_NS16550_MEM32 so serial_out is writeb

Are you able to test SOC_DA8XX platforms?

Quoted 2 lines · Tom Rini

This introduces a warning on all SOC_DA8XX systems because they do not set CONFIG_SYS_NS16550_MEM32 so serial_out is writeb

Well that's a mistake. Its uart uses 32-bit registers, and 8-bit access is not documented to be acceptable.

Apologies for forgetting to at least compile-test this for all affected targets, I really should have done that.

Quoted 1 line

Are you able to test SOC_DA8XX platforms?

No, this was based on my knowledge that freon/primus SoCs (OMAP-L1xx/AM1xxx/TMS320C674x/DA8xx) are architecturally members of the TI C6x family and not remotely related to the OMAP (despite the part code "OMAP-L1xx"), and I double-checked the technical reference
manual: their UART is identical to that on Keystone.

OK. For v2, if you can convert CONFIG_SYS_NS16550_MEM32 to Kconfig as well I'd appreciate it.

I'll see about digging out and setting up one of my da8xx platforms for a quick runtime test too.

Quoted 2 lines · Tom Rini

OK. For v2, if you can convert CONFIG_SYS_NS16550_MEM32 to Kconfig as well I'd appreciate it.

I'm not hugely comfortable doing that, since that would affect even more targets. To be honest, I don't understand why it even exists as a separate var instead of just testing whether CONFIG_SYS_NS16550_REG_SIZE is 4 or -4. Are there actually some crazy targets that allocate 4 bytes per register yet fail if you use 32-bit access?

Would you object hugely to me just implicitly setting CONFIG_SYS_NS16550_MEM32 in ns16550.h for the c6x uart variant? I think it makes a lot of sense to do so, since in that case the driver itself explicitly depends on being able to write a value that doesn't fit in a byte.

Quoted 10 lines · Matthijs van Duin

Quoted 2 lines · Tom Rini

OK. For v2, if you can convert CONFIG_SYS_NS16550_MEM32 to Kconfig as well I'd appreciate it.

I'm not hugely comfortable doing that, since that would affect even more targets. To be honest, I don't understand why it even exists as a separate var instead of just testing whether CONFIG_SYS_NS16550_REG_SIZE is 4 or -4. Are there actually some crazy targets that allocate 4 bytes per register yet fail if you use 32-bit access?

It's most likely a case that just wasn't cleaned up. If you want to correct things by dropping CONFIG_SYS_NS16550_MEM32 and checking CONFIG_SYS_NS16550_REG_SIZE instead, that's fine.

Quoted 5 lines

Would you object hugely to me just implicitly setting CONFIG_SYS_NS16550_MEM32 in ns16550.h for the c6x uart variant? I think it makes a lot of sense to do so, since in that case the driver itself explicitly depends on being able to write a value that doesn't fit in a byte.

Well, it's just SOC_DA8XX that isn't doing it today, and we need to migrate that CONFIG symbol or do away with it.

--
Tom

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot