<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, May 24, 2022 at 1:07 AM Zev Weiss <<a href="mailto:zweiss@equinix.com">zweiss@equinix.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, May 23, 2022 at 06:40:04AM PDT, Patrick Rudolph wrote:<br>
>Introduce CONFIG_ASPEED_DEBUG_UART_TO_UART1 and reuse existing<br>
>platform code to route the debug uart to RDX1/TDX1.<br>
>This is required on IBM/Genesis3 as it uses RDX1/TDX1 as debug uart.<br>
><br>
>Signed-off-by: Patrick Rudolph <<a href="mailto:patrick.rudolph@9elements.com" target="_blank">patrick.rudolph@9elements.com</a>><br>
>Reviewed-by: Joel Stanley <<a href="mailto:joel@jms.id.au" target="_blank">joel@jms.id.au</a>><br>
>---<br>
> arch/arm/mach-aspeed/Kconfig            | 5 +++++<br>
> arch/arm/mach-aspeed/ast2500/platform.S | 2 +-<br>
> 2 files changed, 6 insertions(+), 1 deletion(-)<br>
><br>
>diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig<br>
>index edb5520aec..a38f070381 100644<br>
>--- a/arch/arm/mach-aspeed/Kconfig<br>
>+++ b/arch/arm/mach-aspeed/Kconfig<br>
>@@ -82,6 +82,11 @@ config ASPEED_ENABLE_DEBUG_UART<br>
>         systems, but may be useful to enable for debugging during<br>
>         development.<br>
><br>
>+config ASPEED_DEBUG_UART_TO_UART1<br>
>+      bool "Route debug UART to UART1"<br>
>+      depends on ASPEED_AST2500<br>
>+      help<br>
>+        Route debug UART to TXD1/RXD1 pins.<br>
<br>
Any reason not to put this in 'if ASPEED_ENABLE_DEBUG_UART' as suggested<br>
in the previous review?  And since that one already has the<br>
ASPEED_AST2500 dependency, I think it'd obviate the need to have that<br>
specified on ASPEED_DEBUG_UART_TO_UART1.<br>
<br>
While we're at it, slightly more detail in the help text would good I<br>
think, perhaps just "... instead of the default TXD5/RXD5."<br>
<br>
Though actually, looking at the datasheet I'm now not certain if this<br>
does exactly what I had been thinking previously -- if I'm understanding<br>
it right, it's not so much switching the debug-UART functionality from<br>
UART5 to UART1, it's re-routing UART5 itself to the I/Os typically used<br>
for UART1?  Which seems somewhat different, and I guess would ultimately<br>
be independent of the debug-UART itself being enabled or disabled, in<br>
which case maybe what I said earlier wasn't entirely appropriate...maybe<br>
someone with more expertise on the ast2500 UARTs (e.g. Andrew?) can<br>
weigh in?<br>
<br></blockquote><div>As I understand it only re-routes the UART5 to UART1 I/Os. However it doesn't make any</div><div>sense to re-route the UART5 if it's disabled.</div><div>I'll push a new patch.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> endif<br>
><br>
> config ASPEED_PALLADIUM<br>
>diff --git a/arch/arm/mach-aspeed/ast2500/platform.S b/arch/arm/mach-aspeed/ast2500/platform.S<br>
>index aef55c4a0a..a97ebebcca 100644<br>
>--- a/arch/arm/mach-aspeed/ast2500/platform.S<br>
>+++ b/arch/arm/mach-aspeed/ast2500/platform.S<br>
>@@ -795,7 +795,7 @@ wait_ddr_reset:<br>
>     /* end delay 10ms */<br>
><br>
> /* Debug - UART console message */<br>
>-#ifdef CONFIG_DRAM_UART_TO_UART1<br>
>+#ifdef CONFIG_ASPEED_DEBUG_UART_TO_UART1<br>
>     ldr   r0, =0x1e78909c                        @ route UART5 to UART Port1, 2016.08.29<br>
>     ldr   r1, =0x10000004<br>
>     str   r1, [r0]<br>
>-- <br>
>2.35.3<br>
></blockquote></div></div>