[U-Boot] [PATCH v1 6/8] avr32: add generic board support

[Simon Glass]

Hi Andreas,

On 28 January 2015 at 03:04, Andreas Bießmann
<andreas.devel at googlemail.com> wrote:
> Hi Simon,
>
> On 01/28/2015 04:09 AM, Simon Glass wrote:
>> Hi,
>>
>> On 27 January 2015 at 17:16, Andreas Bießmann
>> <andreas.devel at googlemail.com> wrote:
>>> Signed-off-by: Andreas Bießmann <andreas.devel at googlemail.com>
>>> ---
>>>
>>> Changes in v1:
>>> - add timer_init in board_r
>>> - remove extern declaration of mmu_init_r()
>>>
>>>  arch/avr32/config.mk            |    3 +++
>>>  arch/avr32/cpu/u-boot.lds       |    2 ++
>>>  arch/avr32/include/asm/config.h |    1 +
>>>  arch/avr32/include/asm/u-boot.h |    7 +++++++
>>>  arch/avr32/lib/Makefile         |    2 ++
>>>  arch/avr32/lib/interrupts.c     |    5 +++++
>>>  common/board_f.c                |   13 ++++++++++---
>>>  common/board_r.c                |   26 +++++++++++++++++++++++---
>>>  include/asm-generic/u-boot.h    |    4 ++++
>>>  9 files changed, 57 insertions(+), 6 deletions(-)
>
>
>>> diff --git a/common/board_f.c b/common/board_f.c
>>> index 215108b..1a735ae 100644
>>> --- a/common/board_f.c
>>> +++ b/common/board_f.c
>>> @@ -174,7 +174,7 @@ static int announce_dram_init(void)
>>>         return 0;
>>>  }
>>>
>>> -#if defined(CONFIG_MIPS) || defined(CONFIG_PPC)
>>> +#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || defined(CONFIG_AVR32)
>>>  static int init_func_ram(void)
>>>  {
>>>  #ifdef CONFIG_BOARD_TYPES
>>> @@ -268,6 +268,8 @@ static int setup_mon_len(void)
>>>         gd->mon_len = (ulong)&_end - (ulong)_init;
>>>  #elif defined(CONFIG_BLACKFIN) || defined(CONFIG_NIOS2)
>>>         gd->mon_len = CONFIG_SYS_MONITOR_LEN;
>>> +#elif defined(CONFIG_AVR32)
>>> +       gd->mon_len = (ulong)&__bss_end - (ulong)&_text;
>>
>> Would we be able to unify the symbols here rather than having a
>> special case for AVR32?
>
> well, I'm not __ARM__ nor CONFIG_SANDBOX, CONFIG_BLACKFIN or
> CONFIG_NIOS2. The else path may fit, I'll check that.
>
>>>  #else
>>>         /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */
>>>         gd->mon_len = (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE;
>>> @@ -581,7 +583,7 @@ static int reserve_stacks(void)
>>>         gd->irq_sp = gd->start_addr_sp;
>>>  # endif
>>>  #else
>>> -# ifdef CONFIG_PPC
>>> +# if defined(CONFIG_PPC) || defined(CONFIG_AVR32)
>>>         ulong *s;
>>>  # endif
>>>
>>> @@ -611,6 +613,11 @@ static int reserve_stacks(void)
>>>         s = (ulong *) gd->start_addr_sp;
>>>         *s = 0; /* Terminate back chain */
>>>         *++s = 0; /* NULL return address */
>>> +# elif defined(CONFIG_AVR32)
>>> +       gd->arch.stack_end = gd->start_addr_sp;
>>> +       s = (ulong *)gd->start_addr_sp;
>>> +       *s = 0;
>>> +       *--s = 0;
>>
>> Can we create an arch_reserve_stacks() function to handle this, and
>> put it in the AVR32 code?
>
> I'll do so.
>
>>
>>>  # endif /* Architecture specific code */
>>>
>>>         return 0;
>>> @@ -912,7 +919,7 @@ static init_fnc_t init_sequence_f[] = {
>>>  #if defined(CONFIG_ARM) || defined(CONFIG_X86)
>>>         dram_init,              /* configure available RAM banks */
>>>  #endif
>>> -#if defined(CONFIG_MIPS) || defined(CONFIG_PPC)
>>> +#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || defined(CONFIG_AVR32)
>>>         init_func_ram,
>>
>> Can you use dram_init() instead?
>
> I have to check that. Can we do that later when we convert mips/ppc? I
> fear it is a bit complex.

Later is fine if this doesn't work. But if every arch uses another
arch as an excuse not to clean up no one will get there :-)

>
>>>  #endif
>>>  #ifdef CONFIG_POST
>>> diff --git a/common/board_r.c b/common/board_r.c
>>> index e712902..3620438 100644
>>> --- a/common/board_r.c
>>> +++ b/common/board_r.c
>>> @@ -55,6 +55,9 @@
>>>  #include <dm/root.h>
>>>  #include <linux/compiler.h>
>>>  #include <linux/err.h>
>>> +#ifdef CONFIG_AVR32
>>> +#include <asm/arch/mmu.h>
>>> +#endif
>>
>> What needs this? Let's move it into AVR32 code.
>
> The mmu_init_r needs this. I dunno where to place it. ARM has the TLB
> setup before activating d-caches (in initr_caches). This is not required
> for avr32 and therefore not the right place.

OK, then maybe we need to add this as a new method that any arch can use?

I'm wondering whether we should (later) have the concept of an arch
'driver', with methods for all of these things.

>
>>> @@ -894,6 +910,10 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
>>>         int i;
>>>  #endif
>>>
>>> +#ifdef CONFIG_AVR32
>>> +       mmu_init_r(dest_addr);
>>> +#endif
>>
>> What does this do? It feels like it shold perhaps happen in the init
>> sequence. See x86 which has a similar requirement, or at least I think
>> it is similar.
>
> can you point me to the x86 code?

I was thinking of init_cache_f_r(). Is that similar to what you want?

>
>> The objective ultimately is to remove all #ifdefs from board_f/r.c.
>
> That's clear to me. I thought we can first gather all the stuff in
> common/board_* and in a second turn we clean it up. Obvious things
> cleaned up before, but questionnaire things are cleaned up in a second
> turn. At least this was my objective ;)

Sounds good. I suppose the immediate objective is to get everything in
generic board, but we should clean it up soon.

Regards,
Simon