[U-Boot] [PATCH] zynq: Use arch_cpu_init() instead of lowlevel_init()

[Albert ARIBAUD]

Hi Michal,

On Tue, 24 Sep 2013 12:38:38 +0200, Michal Simek <monstr at monstr.eu>
wrote:

> Hi Albert,
> 
> On 09/23/2013 04:37 PM, Albert ARIBAUD wrote:
> > Hi Michal,
> > 
> > On Mon, 23 Sep 2013 16:19:52 +0200, Michal Simek <monstr at monstr.eu>
> > wrote:
> > 
> >> On 09/23/2013 02:31 PM, Albert ARIBAUD wrote:
> >>> Hi Michal,
> >>>
> >>> On Thu, 22 Aug 2013 14:52:02 +0200, Michal Simek
> >>> <michal.simek at xilinx.com> wrote:
> >>>
> >>>> Zynq lowlevel_init() was implemented in C but stack
> >>>> pointer is setup after function call in _main().
> >>>> Move architecture setup to arch_cpu_init() which is call
> >>>> as the first function in board_init_f() which
> >>>> already have correct stack pointer.
> >>>>
> >>>> Reported-by: Sven Schwermer <sven.schwermer at tuhh.de>
> >>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> >>>> ---
> >>>> I can't see any problem to call zynq setup a little
> >>>> bit later. There is already expectation that u-boot
> >>>> runs from DDR.
> >>>> Moving lowlevel_init from C to ASM is possible but
> >>>> I will have to introduce new macros with hardcoded
> >>>> values. Using structures is much nicer.
> >>>>
> >>>> ---
> >>>>  arch/arm/cpu/armv7/zynq/cpu.c | 6 ++++++
> >>>>  1 file changed, 6 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/cpu/armv7/zynq/cpu.c b/arch/arm/cpu/armv7/zynq/cpu.c
> >>>> index 4367d1a..8846f30 100644
> >>>> --- a/arch/arm/cpu/armv7/zynq/cpu.c
> >>>> +++ b/arch/arm/cpu/armv7/zynq/cpu.c
> >>>> @@ -11,6 +11,10 @@
> >>>>
> >>>>  void lowlevel_init(void)
> >>>>  {
> >>>> +}
> >>>
> >>> I'd rather you deleted lowlevel_init() as a C function with this
> >>> name should not exist.
> >>
> >> Ok. Do you want me to create almost empty low_level.S or just use
> >> arch/arm/cpu/arvm7/lowlevel_init.S and define empty s_init()?
> > 
> > Urgh. I realize removing the C function would give you more work than
> > simply keeping it empty until the whole s_init() mess is cleaned up. :(
> > 
> > I'll take your change as-is, sorry for the noise.
> 
> In connection to this topic we have recently found one issue
> regarding to neon instruction which u-boot uses.
> 
> We have this code to enable them in asm and adding this to lowlevel_init.S
> is straight way how to do so.
>         mov     r0, r0
>         mrc     p15, 0, r1, c1, c0, 2
>         orr     r1, r1, #(0xf << 20)
>         mcr     p15, 0, r1, c1, c0, 2
> 
>         fmrx    r1, FPEXC
>         orr     r1,r1, #(1<<30)
>         fmxr    FPEXC, r1
> 
> Is it ok to create zynq asm specific lowlevel function
> or doing this through s_init() or you have nice a clean way how
> this should be solved when you are saying that s_init() is mess.

Sorry for responding slowly.

I suspect when you say neon instruction that U-Boot uses, you mean neon
instructions that GCC is allowed to emit while building U-Boot, right?
So we're talking about neon insns in C code only, not asm, correct?

If this is correct, then does something prevent you from enabling
neon instructions as early as possible, in e.g. the lowlevel_init
routine?

> Thanks,
> Michal

Amicalement,
-- 
Albert.