[U-Boot] SPL broken on i.mx31 platforms

[Benoît Thébaudeau]

Hi,

On Thu, Jul 3, 2014 at 10:19 AM, Helmut Raiger <helmut.raiger at hale.at> wrote:
> On 07/03/2014 01:20 AM, Benoît Thébaudeau wrote:
>>
>> )Dear Helmut Raiger,
>>
>> On Wed, Jul 2, 2014 at 9:04 AM, Helmut Raiger <helmut.raiger at hale.at>
>> wrote:
>>>>
>>>>       the commit 41623c91 breaks the SPL on i.mx31 platforms.
>>
>> Here, you are talking about mx31pdk, right?
>
> Actually im talking TT-01, but it has no contributed NAND boot code (which I
> was working on), but it should hit mx31pdk in the same way.
> This should answer Albert's question about the board.

Then, since you are out of tree, can you test with the HEAD vs.
41623c91 vs. 41623c91^ mx31pdk codes, replacing the _reset lines with
"b reset" or "bl reset" after "_start:" for 41623c91 and HEAD? It
would probably run U-Boot on TT-01 too. You can remove board-specific
initializations like GPIO outputs from mx31pdk.c in order not to risk
damaging the board (just keep a valid UART to see the boot). This is
just to make sure that there is nothing wrong in your out-of-tree code
that could interfere with the mainline changes, like a custom SPL
linker script that would miss the *(.vectors) section.

>>>> The original startup code (start.S) was position independent to
>>>> allow relocation in board_init_f. This is necessary as the internal
>>>> RAM used by the IPL to load the first 2kB from NAND is also
>>>> used by the NAND controller to buffer pages.
>>>> As far as the issue goes: where and how exactly is the code not
>>>> position independent now?
>>>
>>> The first statement is now:
>>>
>>>      ldr pc, _reset
>>> _reset: .word reset
>>>
>>> reset is an address generated by the linker (here 0x87dc0060).
>>> Originally this was:
>>>
>>>      b reset
>>>
>>> which is a relative branch (here 0x60 bytes forward).
>>> I have to add, that changing this back does not fix the issue, something
>>> else is in disorder.
>>> I'm still working on it.
>>
>> Do you mean reverting this commit, or only these two lines? In the
>> former case, where does bisect lead to, and in the latter case, have
>> you checked the assembler code in the linked SPL for this branch?
>
> I was bisecting the issue by hand :-! , always rebasing my developement
> branch to somewhere in the master tree and found commit 41623c91 that way,
> as stated in the first message of the thread.
> Reverting the commit fixes it, but it should only be a few offending lines.

You are talking about rebasing, reverting, and testing with modified
mainline. Just to make things clear, do you confirm that reverting
commit 41623c91 on top of mainline works (not just rebasing before
this commit)? You mentioned failing tests with a modified mainline, so
I want to make sure that there is no other offending commit after
41623c91 that would interfere with these tests.

>>>>>> Does changing the startup code back to PIC generate any
>>>>>> drawbacks on other ARM platforms?
>>>>
>>>> How would this changing back to PIC be implemented? Not by reverting
>>>> the patch, I hope.
>>>
>>> No, it will only be a minor change, I think, but I thought there might
>>> have been an additional intention behind the change to position
>>> dependent code. One could link the first part to 0xB8000000
>>> (the original position of the SPL when loaded by the IPL) and
>>> the part after the relocation to CONFIG_SPL_TEXT_BASE.
>>
>> Actually, the ROM bootloader first copies the first NAND page to
>> 0xB8000000. Then, the SPL placed here but linked at
>> CONFIG_SPL_TEXT_BASE copies itself to CONFIG_SPL_TEXT_BASE in order to
>> free the NFC buffer so that it can be used by the SPL. There is no
>> relocation going on at this stage, but only a copy, and the SPL code
>> size is limited to 2 kiB. Then, the SPL does its NAND load job towards
>> CONFIG_SYS_TEXT_BASE and starts executing the non-SPL binary, which
>> then relocates itself according to the heap size, etc.
>>
>> Hence, as you said the code launching board_init_f() must be either
>> PIC or linked at 0xB8000000, and the rest of the SPL code must be
>> either PIC or linked at CONFIG_SPL_TEXT_BASE. So the fact that this
>> commit breaks the "PICness" of the initial branch is indeed an issue.
>>
>> As a test, can you try to replace the 2 _reset lines in vectors.S with
>> "b _reset" after "_start:", and a "_reset:" label at the end of the
>> file? This assumes that start.S starts with the "reset:" label, and
>> that the start.o .text sections are placed right after the .vectors
>> section in the SPL linker script, both being true for mx31pdk.
>
>
> I was using the word 'relocation' instead of copying. I did
> what you suggest, but this does not completely fix the issue.

If the only wrong commit is 41623c91, I do not see what else could be
wrong, hence my questions above.

What do you mean by "not completely"? Is there any progress?

> At a quick glance at the assembler code of board_init_f it looked
> PIC to me, but I'll have to check.

If you duplicated mx31pdk's, it should be PIC.

> I'm currently torn between projects, which is why I have no
> further input right now.

No problem.

Benoît