Re: [PATCH] arm: am33xx: Fix early debug UART initialization

On Mon, Feb 14, 2022 at 05:56:52PM +0100, Felix Brack wrote:

The changes from commit 0dba45864b2a ("arm: Init the debug UART") prevent the early debug UART from being initialized correctly. By adding a new SoC specific early UART initialization function this patch provides a generic location to add the required code. This code has to be SoC and not board specific. For the am33xx SoCs the fix consist of configuring early clocks.

Signed-off-by: Felix Brack fb@ltec.ch

arch/arm/mach-omap2/am33xx/board.c | 7 +++++++ drivers/serial/Kconfig | 13 +++++++++++++ include/debug_uart.h | 15 +++++++++++++++ 3 files changed, 35 insertions(+)

diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c index c44667668e..a7f0445b93 100644 --- a/arch/arm/mach-omap2/am33xx/board.c +++ b/arch/arm/mach-omap2/am33xx/board.c @@ -604,3 +604,10 @@ int arch_cpu_init_dm(void) #endif return 0; }

+#if IS_ENABLED(CONFIG_DEBUG_UART_SOC_INIT) +void soc_debug_uart_init(void) +{

• setup_early_clocks();

+} +#endif diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 345d1881f5..3da4064d35 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -490,6 +490,19 @@ config DEBUG_UART_SHIFT value. Use this value to specify the shift to use, where 0=byte registers, 2=32-bit word registers, etc.

+config DEBUG_UART_SOC_INIT

• bool "Enable SoC-specific debug UART init"
• depends on DEBUG_UART
• help

•  Boards using the same SoC might require some common settings before
  

•  the debug UART can be used. The board specific board_debug_uart_init()
  

•  is not the right place for such common settings as they would apply
  

•  to a specific board only instead of all boards using the same SoC.
  

•  When this option is enabled, the SoC specific function
  

•  soc_debug_uart_init() will be called when debug_uart_init() is called.
  

•  You can put any code here that is needed to set up the UART ready for
  

•  use.
  

config DEBUG_UART_BOARD_INIT bool "Enable board-specific debug UART init" depends on DEBUG_UART diff --git a/include/debug_uart.h b/include/debug_uart.h index 714b369e6f..ebc84c44cd 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -42,6 +42,12 @@

• • Immediately afterwards, add DEBUG_UART_FUNCS to define the rest of the

• functionality (printch(), etc.)
  

• • If your SoC needs additional init for the UART to work, enable
• • CONFIG_DEBUG_UART_SOC_INIT and write a function called
• • soc_debug_uart_init() to perform that init. When debug_uart_init() is
• • called, the init will happen automatically. Board specific code does not
• • go here, see board_debug_uart_init() below.
• • If your board needs additional init for the UART to work, enable
  • CONFIG_DEBUG_UART_BOARD_INIT and write a function called
  • board_debug_uart_init() to perform that init. When debug_uart_init() is

@@ -61,6 +67,14 @@ */ void debug_uart_init(void);

+#ifdef CONFIG_DEBUG_UART_SOC_INIT +void soc_debug_uart_init(void); +#else +static inline void soc_debug_uart_init(void) +{ +} +#endif

#ifdef CONFIG_DEBUG_UART_BOARD_INIT void board_debug_uart_init(void); #else @@ -192,6 +206,7 @@ void printdec(unsigned int value); \ void debug_uart_init(void) \ { \

• soc_debug_uart_init(); \
  

  board_debug_uart_init(); \ _debug_uart_init(); \ _DEBUG_UART_ANNOUNCE \

I'd be inclined to just re-use board_debug_uart_init on am33xx like other "board" function we have in the SoC level board.c file. That's likely what other platforms are doing as well where this is more SoC than board dependent? Thanks for digging in here BTW, I think I never saw this myself when I needed the UART because I have mine set for UART boot first (and then it does SD boot).