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

[Albert ARIBAUD]

Hi Simon,

Le 25/10/2011 07:02, Simon Glass a écrit :
> Hi Albert,
>
> On Mon, Oct 24, 2011 at 2:21 PM, Albert ARIBAUD
> <albert.u.boot at aribaud.net>  wrote:
>> Le 24/10/2011 22:14, Simon Glass a écrit :
>>
>>>>> 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 don't mind moving it to armv7/lib. I do mind the "later".
>>>
>>> OK, but the AVP cannot execute this code, whereas the A9 needs to
>>> execute it on start-up.
>>
>> Yes, and that's the real problem: if you did not have that AVP running the
>> same code path as the A9, you wouldn't even have though of moving cp15 init
>> elsewhere.
>
> What is your point here? I am trying to get to a simple
> easy-to-understand execution path in a fairly complex system. So far
> the model is that the AVP starts up U-Boot until arch_cpu_init(), then
> halts and the A9 takes over and runs the same code again. It is very
> easy to understand and debug. It is easy to set breakpoints in startup
> code on either core. It is possible to even put printf() debugging on
> either core with the pre-console support. It has a lot of advantages
> over more complex schemes. It removes a CONFIG_TEGRA2 from start.S
> too!

My point is precisely to keep the execution path simple and reliable for 
as many ARM architectures as possible, and I consider that placing parts 
of the low level initialization at different times for different cores 
is not simple and possibly not reliable, as it makes memory state 
uncertain during part of the start.S path for the A9.

I do understand that to you, this platform you're working on is easy to 
understand and debug, but then, you've got some experience with it, and 
it all makes sens to you, but it might not to other ARM developser who 
don't have access to it.

Besides, what I suggest doing (keeping cp15 access where it is, but 
subject it to the condition that it's not the AVP trying to execute it) 
does not preclude doing early printf() etc.

>>> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that
>>> this option exists suggests that platforms are permitted to do it. We
>>> don't need to do any memory init, and the CP15 init can be called in
>>> arch_cpu_init() once we establish that we are an A9 and not an AVP.
>>> Does that sound ok?
>>
>> Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is not
>> "I'm running this code on two CPUs", it is "those inits have already been
>> done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by SPL,
>> so SPL has done the inits already.
>
> I thought it pre-dated SPL, but ok.

It may have, and before or after SPL, it was/is immensely useful as a 
debugging aid. As such, it nearly disappeared at one point, but with 
SPL, it has received a non-debugging, functional, utility.

>> And I also agree that we should do the cp15 init as soon as we have checked
>> that we're the A9, not the AVP.
>>
>> But this check has to be done during low level init, based on some immediate
>> identification, not based on where in the code we are.
>
> There is no facility in there for this at present. Are you asking for
> a new call to find out at run-time whether the CP15 init should be
> called? If so, then I can fairly easily do that.

Is there really no register that the core can access which would tell it 
which core it actually is? No version/revision register either?

>>> The only problem I can see with this is if the data cache is on. This
>>> could happen if you load U-Boot with (say) tftp and jump to it. Well
>>> 'don't do that!'. We also hope that the D-cache has at least been
>>> flushed or there is nothing we can do. One fix for this (prior to the
>>> refactor you are planning and mentioned in your earlier email) is
>>> perhaps to implement a run-time check for the existence of CP15. Is
>>> that better or worse?
>>
>> According to ARM, coprocessors 14 and 15 should always be supported from
>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
>> specification available, I'd like to have a look at it.
>
> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
> init on the AVP and see what happens.

Please do check this. It may be that you can actually write to it from 
AVP even though it has no or not all expected effect; and if that is 
confirmed, then you don't have to separate the boot paths any more,

> Regards,
> Simon

Amicalement,
-- 
Albert.