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

Hi Felix,

On Mon, 14 Feb 2022 at 09:57, Felix Brack fb@ltec.ch 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

SoC-specific

You need the hyphen for this to make sense, since you are creating an adjective.

patch provides a generic location to add the required code. This code has to be SoC and not board specific.

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(+)

More nits below.

Reviewed-by: Simon Glass sjg@chromium.org

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.
  

Please mention the order in which the board and SoC functions are called.

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.

Again please fix your specifics.

• • 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 \
  

-- 2.25.1