[U-Boot] [PATCH 12/14] microblaze: Move architecture to use generic board init

[Simon Glass]

Hi Michal,

On 30 January 2015 at 03:13, Michal Simek <monstr at monstr.eu> wrote:
> Hi Simon,
>
> On 01/29/2015 04:45 PM, Simon Glass wrote:
>> Hi Michal,
>>
>>
>> On 29 January 2015 at 07:20, Michal Simek <michal.simek at xilinx.com> wrote:
>>> Hi Simon,
>>>
>>> On 01/29/2015 03:15 AM, Simon Glass wrote:
>>>> Hi Michal,
>>>>
>>>> On 27 January 2015 at 08:20, Michal Simek <michal.simek at xilinx.com> wrote:
>>>>> Compile code with -fPIC to get GOT. Do not build SPL
>>>>> with fPIC because it increasing SPL size for nothing.
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>>>> ---
>>>>>
>>>
>>> ...
>>>
>>>>> -#if defined(CONFIG_MIPS) || defined(CONFIG_PPC)
>>>>> +#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || defined(CONFIG_MICROBLAZE)
>>>>>  static int init_func_ram(void)
>>>>>  {
>>>>
>>>> Can you use dram_init() instead?
>>>
>>> No problem will do it in v2.
>>>
>>> Where dram_init(void) should be declared?
>>> I see that others have it in arch header. Maybe better to have
>>> it in any shared location.
>>
>> Yes a shared location makes sense.
>
> I have sent patch for it. Please review.
>
>>
>>>
>>>>> --- a/common/board_r.c
>>>>> +++ b/common/board_r.c
>>>>> @@ -829,7 +829,7 @@ init_fnc_t init_sequence_r[] = {
>>>>>  #if defined(CONFIG_ARM)
>>>>>         initr_enable_interrupts,
>>>>>  #endif
>>>>> -#ifdef CONFIG_X86
>>>>> +#if defined(CONFIG_X86) || defined(CONFIG_MICROBLAZE)
>>>>>         timer_init,             /* initialize timer */
>>>>>  #endif
>>>>
>>>> Could you use interrupt_init() immediately before?
>>>
>>> What do you mean by that?
>>> Do you want to do timer_init in interrupt_init()?
>>
>> Yes, then if we remove it from x86 also we can drop both #ifdefs. Do
>> you see any drawback with that?
>
> timer_init looks like standard function which is used by x86 (board_r), arm, mips, blackfin.
> (board_f)
>
> Based on board_r.c PPC is doing timer_init in cpu_init_r.
>
> For Microblaze my preference is to use this location because timer code is using
> interrupts which are initialized above.
>
> Adding timer_init to interrupt_init doesn't look right because then interrupt_init will hide
> one init step.

Fair enough, I agree they are different. One way or another we'll get
rid of the #ifdefs in those files one day...

Regards,
Simon