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

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 \

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) \ { \
board_debug_uart_init(); \ _debug_uart_init(); \ _DEBUG_UART_ANNOUNCE \soc_debug_uart_init(); \
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).

Hello Tom,
On 14.02.22 23:45, Tom Rini wrote:
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) \ { \
board_debug_uart_init(); \ _debug_uart_init(); \ _DEBUG_UART_ANNOUNCE \soc_debug_uart_init(); \
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).
On the PDU001 board the board specific board_debug_uart_init() is (for now) responsible for the pin configuration of UART3 (the debug UART).
From a discussion with Simon I concluded that other boards based on the
AM33XX SoCs are also affected from the problem introduced with commit 0dba45864b2a. The function board_debug_uart_init() can not be moved to the SoC level since some boards (like the PDU001) require it. Hence the idea of a SoC specific soc_debug_uart_init() function. However I now realize that such a "one fits it all" solution is probably not the right way to fix things. It might be better to fix this on board level especially since there are some boards that are not affected at all. Hence I think it is best to drop my patch. I will send a new patch fixing the problem for the PDU001 board specifically.

On Tue, Feb 15, 2022 at 11:53:41AM +0100, Felix Brack wrote:
Hello Tom,
On 14.02.22 23:45, Tom Rini wrote:
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) \ { \
board_debug_uart_init(); \ _debug_uart_init(); \ _DEBUG_UART_ANNOUNCE \soc_debug_uart_init(); \
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).
On the PDU001 board the board specific board_debug_uart_init() is (for now) responsible for the pin configuration of UART3 (the debug UART). From a discussion with Simon I concluded that other boards based on the AM33XX SoCs are also affected from the problem introduced with commit 0dba45864b2a. The function board_debug_uart_init() can not be moved to the SoC level since some boards (like the PDU001) require it. Hence the idea of a SoC specific soc_debug_uart_init() function. However I now realize that such a "one fits it all" solution is probably not the right way to fix things. It might be better to fix this on board level especially since there are some boards that are not affected at all. Hence I think it is best to drop my patch. I will send a new patch fixing the problem for the PDU001 board specifically.
Ah, OK, thanks.

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

Hello Simon,
On 23.02.22 23:58, Simon Glass wrote:
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
Thanks for the review! However it looks like this patch will never get applied. If I'm wrong please let me know and I'll be happy to send a fixed version.

Hi Felix,
On Thu, 24 Feb 2022 at 03:01, Felix Brack fb@ltec.ch wrote:
Hello Simon,
On 23.02.22 23:58, Simon Glass wrote:
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
Thanks for the review! However it looks like this patch will never get applied. If I'm wrong please let me know and I'll be happy to send a fixed version.
OK, I suppose it can wait until another board comes along that needs it.
Regards, Simon
participants (3)
-
Felix Brack
-
Simon Glass
-
Tom Rini