Early debug UART not working on AM33XX SoC

Felix Brack fb at ltec.ch
Tue Feb 1 18:13:00 CET 2022

Hello Simon

On 01.02.22 15:05, Simon Glass wrote:
> Hi Felix,
> On Tue, 1 Feb 2022 at 03:48, Felix Brack <fb at ltec.ch> wrote:
>> Hello Simon,
>> On 31.01.22 17:12, Simon Glass wrote:
>>> Hi Felix,
>>> On Mon, 31 Jan 2022 at 02:43, Felix Brack <fb at ltec.ch> wrote:
>>>> Hello Simon
>>>> On 27.01.22 18:33, Simon Glass wrote:
>>>>> Hi Felix,
>>>>> On Thu, 27 Jan 2022 at 09:27, Felix Brack <fb at ltec.ch> wrote:
>>>>>> Hello Simon,
>>>>>> On 27.01.22 16:05, Simon Glass wrote:
>>>>>>> Hi Felix,
>>>>>>> On Wed, 26 Jan 2022 at 07:02, Felix Brack <fb at ltec.ch> wrote:
>>>>>>>> Hello Simon,
>>>>>>>> I am trying to get the current U-Boot master working on the PDU001
>>>>>>>> board. This involves the use of an early debug UART.
>>>>>>>> With commit 0dba4586 (arm: Init the debug UART) the early debug UART on
>>>>>>>> the AM33XX SoC doesn't work anymore.
>>>>>>>> By looking at the code involved I believe a call to
>>>>>>>> setup_clocks_for_console() implemented in clock_am33xx.c before the call
>>>>>>>> to debug_uart_init() is missing. This is also what happens prior to
>>>>>>>> commit 0dba4586 by a call to early_system_init() which in turn calls
>>>>>>>> setup_early_clocks() which then calls setup_clocks_for_console().
>>>>>>>> My quick and dirty fix consist of inserting a call in crt0.S to
>>>>>>>> setup_clocks_for_console() just before the call to debug_uart_init()
>>>>>>>> which was added in commit 0dba4586. I have guarded this call with
>>>>>>>> #if/#endif testing for CONFIG_AM33XX. The code sequence in crt0.S now
>>>>>>>> looks like this:
>>>>>>>>   #if defined(CONFIG_AM33XX)
>>>>>>>>     bl setup_clocks_for_console
>>>>>>>>   #endif
>>>>>>>>     bl debug_uart_init
>>>>>>>> #endif
>>>>>>>> However, from what I understand the crt0.S is for _all_ ARM boards hence
>>>>>>>> it's probably a bad idea polluting it with such #if/#endif tests for
>>>>>>>> different SoCs. What do you think?
>>>>>>>> If my quick and dirty fix is not that dirty I would be happy to send a
>>>>>>>> patch which would also include the removal of the currently remaining
>>>>>>>> call to debug_uart_init() in am33xx/board.c
>>>>>>>> Please apologize my narrow view of the matter dealing only with AM33XX SoCs.
>>>>>>> Are you able to put that call into board_debug_uart_init() and enable
>>>>>> Sure I can but there two drawbacks:
>>>>>> 1. this fixes the problem only for my board, others might remain broken
>>>>> I suggest you make the function common to all boards that need it.
>>>> I will check that. Maybe the board_debug_uart_init() is not the right
>>>> place then as it is board specific. Probably better to have a AM33XX
>>>> platform specific function.
>>> Despite the board_ prefix you can define it in an SoC file, for
>>> example. It may not always be board-specific.
>> Thanks for the hint! In fact I was not aware of that. However for the
>> PDU001 board board_debug_uart_init() must remain with the board as it
>> (also) configures the pin multiplexer which is specific to the PDU001 board.
>> For the patch to be useful for other boards too, I think a platform
>> specific function is required. A weak function named something like
>> platform_debug_uart_init(). This function could then be implemented in
>> the platform specific board.c file, in my case the one for AM33XX.
> Would it be OK to create a non-weak function that is called from
> board_debug_uart_init()?
> But yes we could create another function. How about
> soc_debug_uart_init() as a name, though?
>> But where to place the default, empty implementation? In the common file
>> board_init.c probably?
> At present we have a CONFIG option to enable board_debug_uart_init()
> so we could do the same for soc. But should it run first or second?
> That is why I feel that doing everything from board_debug_uart_init()
> might be better.
>> Frankly I don't feel really comfortable with my own proposal above. It
>> sounds a little bit like patchwork to me. On the other hand I don't see
>> a better solution given the fact that it should solve not just my
>> problem. Should I use it as foundation for sending a patch anyway?
>>>> Having said that there is still something I don't understand: commit
>>>> 0dba4586 has added a call to debug_uart_init() in crt0.S but not removed
>>>> any existing call to debug_uart_init(). Hence this function is called twice.
>>>> Is this intentional (and if so, why ?) or are the remaining calls some
>>>> sort of leftovers?
>>> Just that I was not confident removing it from the various affected
>>> boards since some have the same problem as you found with yours.
>>> Calling it twice is generally fine.
>> Agreed. It is just a little bit confusing especially with
> I will do a patch to remove them and see what happens.
Great! I can't imagine that this should provoke any additional problems.

> Regards,
> Simon

Regards, Felix

More information about the U-Boot mailing list