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

[Simon Glass]

Hi Albert,

On Mon, Oct 24, 2011 at 11:41 PM, Albert ARIBAUD
<albert.u.boot at aribaud.net> wrote:
> 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.

Certainly if we can manage it so much the better.

The memory is set up by the boot ROM and the A9 has just come out of reset.

>
> 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.

But it does involve some extremely early soc-specific code. With my
current patch set there is none of this in start.S (the #ifdef TEGRA2
currently there is removed).

>
>>>> 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?

Yes of course - this is the C function ap20_cpu_is_cortexa9(),
currently used from C code.

But please think about what you actually want in start.S. Do you
really want an #ifdef TEGRA2 with a check of a SOC register, followed
by a branch around the cp15_init code?

>
>>>> 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,

Yes I'm tempted to go with this even if the ARM ARM is silent on the
matter. I will try it out today.

Regards,
Simon

> Amicalement,
> --
> Albert.
>