[U-Boot] [PATCH 1/4] 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 matthijsvanduin@gmail.com --- drivers/serial/ns16550.c | 8 ++++---- include/ns16550.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index c702304e79bd..9cec58c887c8 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -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) +#if defined(CONFIG_ARCH_OMAP2PLUS) /* /16 is proper to hit 115200 with 48MHz */ serial_out(0, &com_port->mdr1); #endif -#if defined(CONFIG_SOC_KEYSTONE) - serial_out(UART_REG_VAL_PWREMU_MGMT_UART_ENABLE, &com_port->regC); +#if defined(CONFIG_SOC_KEYSTONE) || defined(CONFIG_SOC_DA8XX) + serial_out(UART_REG_VAL_PWREMU_MGMT_UART_ENABLE, &com_port->pwr_mgmt); #endif }
diff --git a/include/ns16550.h b/include/ns16550.h index 5fcbcd2e74e3..a988d297e144 100644 --- a/include/ns16550.h +++ b/include/ns16550.h @@ -71,7 +71,7 @@ struct NS16550 { UART_REG(lsr); /* 5 */ UART_REG(msr); /* 6 */ UART_REG(spr); /* 7 */ -#ifdef CONFIG_SOC_DA8XX +#if defined(CONFIG_SOC_KEYSTONE) || defined(CONFIG_SOC_DA8XX) UART_REG(reg8); /* 8 */ UART_REG(reg9); /* 9 */ UART_REG(revid1); /* A */
On Mon, Jan 08, 2018 at 01:50:59PM +0100, Matthijs van Duin wrote:
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 matthijsvanduin@gmail.com
drivers/serial/ns16550.c | 8 ++++---- include/ns16550.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index c702304e79bd..9cec58c887c8 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -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) +#if defined(CONFIG_ARCH_OMAP2PLUS) /* /16 is proper to hit 115200 with 48MHz */ serial_out(0, &com_port->mdr1); #endif -#if defined(CONFIG_SOC_KEYSTONE)
- serial_out(UART_REG_VAL_PWREMU_MGMT_UART_ENABLE, &com_port->regC);
+#if defined(CONFIG_SOC_KEYSTONE) || defined(CONFIG_SOC_DA8XX)
- serial_out(UART_REG_VAL_PWREMU_MGMT_UART_ENABLE, &com_port->pwr_mgmt);
#endif }
This introduces a warning on all SOC_DA8XX systems because they do not set CONFIG_SYS_NS16550_MEM32 so serial_out is writeb and UART_REG_VAL_PWREMU_MGMT_UART_ENABLE is larger than a u8. Are you able to test SOC_DA8XX platforms? Thanks!
On 19 January 2018 at 16:26, Tom Rini trini@konsulko.com wrote:
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.
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.
Matthijs
On Fri, Jan 19, 2018 at 04:39:15PM +0100, Matthijs van Duin wrote:
On 19 January 2018 at 16:26, Tom Rini trini@konsulko.com wrote:
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.
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. Thanks!
On 19 January 2018 at 16:41, Tom Rini trini@konsulko.com wrote:
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.
I'll see about digging out and setting up one of my da8xx platforms for a quick runtime test too.
Thanks!
Matthijs
On Fri, Jan 19, 2018 at 05:39:32PM +0100, Matthijs van Duin wrote:
On 19 January 2018 at 16:41, Tom Rini trini@konsulko.com wrote:
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.
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.
On Fri, Jan 19, 2018 at 2:22 PM, Tom Rini trini@konsulko.com wrote:
On Fri, Jan 19, 2018 at 05:39:32PM +0100, Matthijs van Duin wrote:
On 19 January 2018 at 16:41, Tom Rini trini@konsulko.com wrote:
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.
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.
If you want to CC me on the next round of the patch, I can test it on a DA850-EVM.
adam
-- Tom
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
participants (3)
-
Adam Ford -
Matthijs van Duin -
Tom Rini