diff mbox series

[u-boot,v2019.04-aspeed-openbmc,v3,2/3] arm/mach-aspeed: Add support for CONFIG_DRAM_UART_TO_UART1

Message ID 20220519150719.22338-3-patrick.rudolph@9elements.com
State New
Headers show
Series Add support for IBM Genesis3 | expand

Commit Message

Patrick Rudolph May 19, 2022, 3:07 p.m. UTC 
Update the Kconfig and allow a board to use CONFIG_DRAM_UART_TO_UART1.
The platform code already uses this Kconfig symbol, but it always
evaluated to false.
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/ast2500/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Zev Weiss May 20, 2022, 7:36 a.m. UTC | #1 
On Thu, May 19, 2022 at 08:07:18AM PDT, Patrick Rudolph wrote:
> Update the Kconfig and allow a board to use CONFIG_DRAM_UART_TO_UART1.
> The platform code already uses this Kconfig symbol, but it always
> evaluated to false.
> 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/ast2500/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/mach-aspeed/ast2500/Kconfig b/arch/arm/mach-aspeed/ast2500/Kconfig
> index 4f992f442d..e7ff00cdba 100644
> --- a/arch/arm/mach-aspeed/ast2500/Kconfig
> +++ b/arch/arm/mach-aspeed/ast2500/Kconfig
> @@ -17,6 +17,12 @@ config TARGET_EVB_AST2500
>  	  20 pin JTAG, pinouts for 14 I2Cs, 3 SPIs and eSPI, 8 PWMs.
>  endchoice
>  
> +config DRAM_UART_TO_UART1
> +	bool
> +	prompt "Route debug UART to UART1"
> +	help
> +	  Route debug UART to TXD1/RXD1 pins.
> +

Given that the debug UART is now disabled by default and only available 
via a combination of CONFIG_ASPEED_ALLOW_DANGEROUS_BACKDOORS=y and 
CONFIG_ASPEED_ENABLE_DEBUG_UART=y, I'd suggest moving this to 
arch/arm/mach-aspeed/Kconfig in the ASPEED_ALLOW_DANGEROUS_BACKDOORS 
'if' block, perhaps in a nested 'if' block conditional on 
ASPEED_ENABLE_DEBUG_UART.

Also, I realize that name comes from previously existing code in 
platform.S, but it's not exactly the clearest, most descriptive name in 
the world (it's the debug UART, what's it go to do with DRAM?).  If 
we're going to promote it to a Kconfig option I think it'd be nice to 
improve the naming somewhat, perhaps just s/DRAM/DEBUG/.

>  source "board/aspeed/evb_ast2500/Kconfig"
>  
>  endif
> -- 
> 2.35.3
>
Zev Weiss May 20, 2022, 8:04 a.m. UTC | #2 
On Fri, May 20, 2022 at 12:36:49AM PDT, Zev Weiss wrote:
> On Thu, May 19, 2022 at 08:07:18AM PDT, Patrick Rudolph wrote:
> > Update the Kconfig and allow a board to use CONFIG_DRAM_UART_TO_UART1.
> > The platform code already uses this Kconfig symbol, but it always
> > evaluated to false.
> > 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/ast2500/Kconfig | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/mach-aspeed/ast2500/Kconfig b/arch/arm/mach-aspeed/ast2500/Kconfig
> > index 4f992f442d..e7ff00cdba 100644
> > --- a/arch/arm/mach-aspeed/ast2500/Kconfig
> > +++ b/arch/arm/mach-aspeed/ast2500/Kconfig
> > @@ -17,6 +17,12 @@ config TARGET_EVB_AST2500
> >  	  20 pin JTAG, pinouts for 14 I2Cs, 3 SPIs and eSPI, 8 PWMs.
> >  endchoice
> >  
> > +config DRAM_UART_TO_UART1
> > +	bool
> > +	prompt "Route debug UART to UART1"
> > +	help
> > +	  Route debug UART to TXD1/RXD1 pins.
> > +
> 
> Given that the debug UART is now disabled by default and only available 
> via a combination of CONFIG_ASPEED_ALLOW_DANGEROUS_BACKDOORS=y and 
> CONFIG_ASPEED_ENABLE_DEBUG_UART=y, I'd suggest moving this to 
> arch/arm/mach-aspeed/Kconfig in the ASPEED_ALLOW_DANGEROUS_BACKDOORS 
> 'if' block, perhaps in a nested 'if' block conditional on 
> ASPEED_ENABLE_DEBUG_UART.
> 
> Also, I realize that name comes from previously existing code in 
> platform.S, but it's not exactly the clearest, most descriptive name in 
> the world (it's the debug UART, what's it go to do with DRAM?).  If 
> we're going to promote it to a Kconfig option I think it'd be nice to 
> improve the naming somewhat, perhaps just s/DRAM/DEBUG/.

Or actually s/DRAM/ASPEED_DEBUG/, since Kconfig symbols are a global 
namespace and DEBUG_UART_TO_UART1 would be a bit overly generic, IMO.

> 
> >  source "board/aspeed/evb_ast2500/Kconfig"
> >  
> >  endif
> > -- 
> > 2.35.3
> >
diff mbox series

Patch

diff --git a/arch/arm/mach-aspeed/ast2500/Kconfig b/arch/arm/mach-aspeed/ast2500/Kconfig
index 4f992f442d..e7ff00cdba 100644
--- a/arch/arm/mach-aspeed/ast2500/Kconfig
+++ b/arch/arm/mach-aspeed/ast2500/Kconfig
@@ -17,6 +17,12 @@  config TARGET_EVB_AST2500
 	  20 pin JTAG, pinouts for 14 I2Cs, 3 SPIs and eSPI, 8 PWMs.
 endchoice
 
+config DRAM_UART_TO_UART1
+	bool
+	prompt "Route debug UART to UART1"
+	help
+	  Route debug UART to TXD1/RXD1 pins.
+
 source "board/aspeed/evb_ast2500/Kconfig"
 
 endif