[U-Boot] [PATCH 4/4] MIPS: call debug_uart_init right before board_init_f

[Marek Vasut]

On 04/23/2017 09:04 PM, Daniel Schwierzeck wrote:
> 
> 
> Am 23.04.2017 um 19:08 schrieb Marek Vasut:
>> On 04/23/2017 02:04 PM, Álvaro Fernández Rojas wrote:
>>> Hi Marek,
>>>
>>> El 23/04/2017 a las 13:44, Marek Vasut escribió:
>>>> On 04/23/2017 01:31 PM, Álvaro Fernández Rojas wrote:
>>>>> Hi Marek,
>>>>>
>>>>> El 23/04/2017 a las 13:09, Marek Vasut escribió:
>>>>>> On 04/23/2017 12:50 PM, Álvaro Fernández Rojas wrote:
>>>>>>> From: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>>>>>>>
>>>>>>> All MIPS boards that support debug uart are calling debug_uart_init right at
>>>>>>> the beginning of board_early_init_f.
>>>>>>> Instead of doing that, let's provide a generic call to debug_uart_init right
>>>>>>> before the call to board_init_f if debug uart is enabled.
>>>>>>>
>>>>>>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>>>>>>> Signed-off-by: Álvaro Fernández Rojas <noltari at gmail.com>
>>>>>>> ---
>>>>>>>  arch/mips/cpu/start.S | 6 ++++++
>>>>>>>  1 file changed, 6 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/mips/cpu/start.S b/arch/mips/cpu/start.S
>>>>>>> index 6740fdf..f7dee81 100644
>>>>>>> --- a/arch/mips/cpu/start.S
>>>>>>> +++ b/arch/mips/cpu/start.S
>>>>>>> @@ -265,6 +265,12 @@ wr_done:
>>>>>>>  	setup_stack_gd
>>>>>>>  #endif
>>>>>>>  
>>>>>>> +#ifdef CONFIG_DEBUG_UART
>>>>>>> +	PTR_LA	t9, debug_uart_init
>>>>>>
>>>>>> This should be called from C code somewhere, in fact, doesn't SPL's
>>>>>> common code call debug_uart_init already ?
>>>>>
>>>>> Why should this be called from C code?
>>>>
>>>> Because the less stuff we have in assembler the better.
>>> ... or not.
>>>
>>>>
>>>>> BTW, I don't think that SPL common code calls debug_uart_init.
>>>>
>>>> That's where it would make sense, grepping through the source indicates
>>>> a lot of boards call this from their spl code. Also, you enter
>>>> board_init_f right below, so you can start there ...
>>> Actually I was going to do that and Daniel suggested otherwise :)
>>> https://lists.denx.de/pipermail/u-boot/2017-April/287388.html
>>> ---
>>> I think we should move this to MIPS start.S to support this for all MIPS
>>> boards in a generic way. Puttings following code between
>>> 'setup_stack_gd' and the jump to 'board_init_f' should work:
>>>
>>> #ifdef CONFIG_DEBUG_UART
>>> 	PTR_LA	t9, debug_uart_init
>>> 	jalr	t9
>>> 	 nop
>>> #endif
>>> ---
>>> BTW, like Daniel I also think this is the proper way to go.
>>
>> Can you elaborate why is it better to put more stuff into the assembler
>> rather than C? I'd love to hear the rationale for that.
>>
>> I agree this should be called from common code , I disagree this should
>> be called from assembler.
>>
> 
> The purpose of debug_uart_init is to run as early as possible to provide
> debug output in the init code until serial_init/console_init_f have been
> executed. The earliest point for debug_uart_init depends on arch and/or
> SoC. On MIPS this point is directly after the setup_stack_gd macro.
> Actually there are two points on MIPS. One is before calling
> lowlevel_init when the stack can be used from some SRAM (option
> CONFIG_MIPS_INIT_STACK_IN_SRAM). This allows on some MIPS SoC's (not on
> all!) to implement lowlevel_init in C and to use debug_uart. This can't
> be realized with common code called from board_init_f. And no MIPS board
> currently boots with SPL, so calling debug_uart_init from spl.c is also
> not an option.
> 
> Also the earliest board-specific callback from board_init_f is
> board_early_init_f from where some MIPS boards already call
> debug_uart_init. Before each new MIPS board copies this style, it's
> better to solve this in a generic way at least for MIPS. Solving this
> for all archs would mean to call debug_uart_init as the first function
> in board_init_f. But this would conflict with some archs (x86, xtensa,
> nios) which already calling debug_uart_init from start.S.

Then it might be time to drain the swamp ? Although that'd likely be way
out of scope of this patchset.

Isn't arch_cpu_init() called way before board_early_init_f() and
possibly the right place for you to put it ? If not, adding another
entry into init_sequence_f might be an option (note that this patch adds
the call to debug_uart_init() right before jumping into board_init_f ,
not right after setup_stack_gd ).

I cannot say I'm a big fan of growing something comparable to
init_sequence_f in mips assembler ... the assembler parts should only be
used to bring up the most basic environment to run C code, the rest
should be implemented in C code.

-- 
Best regards,
Marek Vasut