[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Sat, Oct 22, 2011 at 9:13 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Albert,
>
> On Sat, Oct 22, 2011 at 12:56 AM, Albert ARIBAUD
> <albert.u.boot at aribaud.net> wrote:
>> Le 22/10/2011 07:05, Simon Glass a écrit :
>>
>>>>> Well I actually haven't moved it! It is just that previously it was
>>>>> impossible to call cp15_init from anywhere later.
>>>>
>>>> It is moved, in that it belongs to low level init... of A9.
>>>
>>> OK, I see - you mean moved in order if not in source code file.
>>
>> Yes. In the code, though, it belongs to low-level init.
>
> Arguably this could go in a library like arch/arm/cpu/armv7/lib/cache.S
>
>>
>>>>> What you say can be done, it would involve some assembler though and
>>>>> would need to be another CONFIG option. Was trying to avoid adding new
>>>>> assembler.
>>>>
>>>> Low level init is about assembler and, well, low level. :)
>>>
>>> Yes but it's yuck. Part of the clean-up is to remove most of the
>>> assembler - really very little is needed.
>>
>> I don't think "yuck" is a valid reasoned argument against assembly language
>> :) but I assume what you mainly mean is 'hard to understand and replaceable
>> by easy to understand C code'. I agree to the first part, and to the second
>> one for the general case, but not for the startup path where, precisely, we
>> don't have a working C run-time and most of the understanding requires
>> knowledge of hardware, not only general programming knowledge. Past that, C
>
> I'm not arguing for the cache init stuff to move to C, just trying to
> avoid any creeping growth of assembler when C can do it.
>
>> is ok -- but then beware: some C++ guy could chime in and contend that C is
>> "yuck" by your own standards. :)
>
> The C++ guy who finds himself in U-Boot is probably lost :-)
>
>>
>>>> But I don't see why there should be another CONFIG option. IIUC, you
>>>> should
>>>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level
>>>> init
>>>> code under it, you would do the equivalent of a switch, with one branch
>>>> for
>>>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc).
>>>
>>> Yes I can, but I need to be able to call cp15_init. And I can't do
>>> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is
>>> compiled out! So I need to move that cp15_init code outside the
>>> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do,
>>> honest!
>>
>> I understand, and I think the approach here is wrong, because you *do* want
>> low-level init in the A9 case, and thus you should not have
>> CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init to
>> be empty, and the AVP and A9 must follow the same startup path. And all this
>> it not necessarily required for all armv7 platforms, but some of them will
>> want lowlevel init, and some not. Considering all this...
>>
>> For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which
>> start.S would use to compile out the low level init code definition (like
>> CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline
>> CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific
>> version of lowlevel_init.
>
> If I understand you correctly, this is the opposite of what I want. It
> is basically what we have now.
>
> All ARMv7 chips are going to turn off MMU, caches, flush TLBs, and the
> like - this is an architecture feature, not an SOC one. We have
> conflated ARM arch init with SOC init as you say, but the solution is
> not to force the SOC to copy code from start.S just so that it can do
> ARM arch init later.
>
> And I'd argue for LOWLEVEL_INIT being about setting up the memory so
> we can run code, and in particular relocate it later. The cp15_init
> stuff should arguably be in arch_cpu_init() (or perhaps a new
> arch_init()) as the first thing called from board_init_f() in my view.
>
> So how about I create a patch to move the cp15_init() code into
> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?

I had a look at this, and it is a bit of a pain since the SPL builds
all bring in files by name from random directories around U-Boot,
including arch/arm/cpu/armv7/start.S. If I move code out of that file
into lib.S or cp15.S then I need to fix up various SPL builds. But it
seems to me that it would be better for arch/arm/cpu/armv7/Makefile
(another other Makefiles) to specify what object files are needed for
an SPL build, rather than putting it in *_spl/Makefile.

Therefore I have left this along, and would like proceed as previously!

I will send an updated patch series when MAKEALL finishes.

Regards,
Simon

>
>>
>> In the longer term, I would be more in favor of a weak definition system
>> with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of
>> fond of it) for lowlevel init -- but that will require looking into asm weak
>> symbol handling and may require splitting of the assembly code function by
>> function.
>
> I think you are trying to avoid the
>
> #ifdef CONFIG_ARCH_CPU_INIT
>   arch_cpu_init,
> #endif
> #ifdef ...
>   some_other_thing
> #endif
>
> in the board_init_f() function table. Yes I agree it would tidy things
> up, although I wonder if Graeme's initcall thing isn't better again.
> It has the same confusion level coefficient (only the linker decides
> what code is included) with a bit more flexibility. Then each arch /
> board / cpu / soc / squirrel can decide on its own init and put it in
> its own file. C only please :-)
>
> Regards,
> Simon
>
>>
>>> Regards,
>>> Simon
>>
>> Amicalement,
>> --
>> Albert.
>>
>