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

[Simon Glass]

Hi Albert,

On Tue, Oct 25, 2011 at 6:44 AM, Simon Glass <sjg at chromium.org> wrote:
> 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?

^^^^
This is the bit we need to sort out.

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

Silly me of course it doesn't work - since if the CP15 doesn't have
those features then you get undef instruction exception. Please see
^^^ above!

Regards,
Simon

>
> Regards,
> Simon
>
>> Amicalement,
>> --
>> Albert.
>>
>