Hi Felix, On Mon, 31 Jan 2022 at 02:43, Felix Brack <f...@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 <f...@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 <f...@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_DEBUG_UART) && CONFIG_IS_ENABLED(SERIAL) > >>>> #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 > >>> CONFIG_DEBUG_UART_BOARD_INIT ? > >>> > >> 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. > > 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. > > >> 2. the now redundant call to setup_early_clocks() in am33xx/board.c > >> remains (at least it does not seem to hurt ;-)) > > > > Yes, it is possible to use flags to avoid that, but might not be worth it. > > > >> > >> I will prepare a patch with a modified board_debug_uart_init() for the > >> PDU001 board then. > >> > >> -- > >> Many thanks for your help and kind regards, Felix Regards, Simon
