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

[Patrick Rudolph <patrick.rudolph@9elements.com>]

[-- Attachment #1: Type: text/plain, Size: 3348 bytes --]

On Tue, May 24, 2022 at 11:35 AM Zev Weiss <zev@bewilderbeest.net> wrote:

> On Tue, May 24, 2022 at 02:30:02AM PDT, Patrick Rudolph wrote:
> > On Tue, May 24, 2022 at 1:07 AM Zev Weiss <zweiss@equinix.com> wrote:
> >
> > > 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?
> > >
> > > As I understand it only re-routes the UART5 to UART1 I/Os. However it
> > doesn't make any
> > sense to re-route the UART5 if it's disabled.
> > I'll push a new patch.
> >
>
> Bear in mind that there's a difference between UART5 and the debug-UART
> feature that can be enabled -- UART5 is a UART pretty much like all the
> others, but it happens to be the one on which the debug feature operates
> by default (listening for a secret password and providing access to
> various special operations via a simple command-line interface).  Even
> with CONFIG_ASPEED_ENABLE_DEBUG_UART=n, UART5 is still available for use
> as a normal UART, it just doesn't have that magic debug capability
> turned on.
>
> Oh, I didn't know that. With that knowledge there should be no need to
depend on the
CONFIG_ASPEED_ENABLE_DEBUG_UART when re-routing uart5.
I'll try another aproach.

>
> Zev
>
>

[-- Attachment #2: Type: text/html, Size: 4682 bytes --]