Re: [PATCH u-boot v2019.04-aspeed-openbmc v4 2/2] arm/mach-aspeed: Add support for CONFIG_ASPEED_DEBUG_UART_TO_UART1

[Zev Weiss <zweiss@equinix.com>]

On Mon, May 23, 2022 at 06:40:04AM PDT, Patrick Rudolph wrote:
>Introduce CONFIG_ASPEED_DEBUG_UART_TO_UART1 and reuse existing
>platform code to route the debug uart to RDX1/TDX1.
>This is required on IBM/Genesis3 as it uses RDX1/TDX1 as debug uart.
>
>Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>Reviewed-by: Joel Stanley <joel@jms.id.au>
>---
> arch/arm/mach-aspeed/Kconfig            | 5 +++++
> arch/arm/mach-aspeed/ast2500/platform.S | 2 +-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig
>index edb5520aec..a38f070381 100644
>--- a/arch/arm/mach-aspeed/Kconfig
>+++ b/arch/arm/mach-aspeed/Kconfig
>@@ -82,6 +82,11 @@ config ASPEED_ENABLE_DEBUG_UART
> 	  systems, but may be useful to enable for debugging during
> 	  development.
>
>+config ASPEED_DEBUG_UART_TO_UART1
>+	bool "Route debug UART to UART1"
>+	depends on ASPEED_AST2500
>+	help
>+	  Route debug UART to TXD1/RXD1 pins.

Any reason not to put this in 'if ASPEED_ENABLE_DEBUG_UART' as suggested
in the previous review?  And since that one already has the
ASPEED_AST2500 dependency, I think it'd obviate the need to have that
specified on ASPEED_DEBUG_UART_TO_UART1.

While we're at it, slightly more detail in the help text would good I
think, perhaps just "... instead of the default TXD5/RXD5."

Though actually, looking at the datasheet I'm now not certain if this
does exactly what I had been thinking previously -- if I'm understanding
it right, it's not so much switching the debug-UART functionality from
UART5 to UART1, it's re-routing UART5 itself to the I/Os typically used
for UART1?  Which seems somewhat different, and I guess would ultimately
be independent of the debug-UART itself being enabled or disabled, in
which case maybe what I said earlier wasn't entirely appropriate...maybe
someone with more expertise on the ast2500 UARTs (e.g. Andrew?) can
weigh in?

> endif
>
> config ASPEED_PALLADIUM
>diff --git a/arch/arm/mach-aspeed/ast2500/platform.S b/arch/arm/mach-aspeed/ast2500/platform.S
>index aef55c4a0a..a97ebebcca 100644
>--- a/arch/arm/mach-aspeed/ast2500/platform.S
>+++ b/arch/arm/mach-aspeed/ast2500/platform.S
>@@ -795,7 +795,7 @@ wait_ddr_reset:
>     /* end delay 10ms */
>
> /* Debug - UART console message */
>-#ifdef CONFIG_DRAM_UART_TO_UART1
>+#ifdef CONFIG_ASPEED_DEBUG_UART_TO_UART1
>     ldr   r0, =0x1e78909c                        @ route UART5 to UART Port1, 2016.08.29
>     ldr   r1, =0x10000004
>     str   r1, [r0]
>-- 
>2.35.3
>