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

[Daniel Schwierzeck]

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.

-- 
- Daniel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170423/6356693c/attachment.sig>