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

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

[Helmut Raiger]

Hi,

     the commit 41623c91 breaks the SPL on i.mx31 platforms.
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.

Does changing the startup code back to PIC generate any
drawbacks on other ARM platforms?
What was the intention of the change besides unifying?

Could someone verify the issue on a i.mx31 platform.

Helmut


--
Scanned by MailScanner.

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

[Fabio Estevam]

Hi Helmut,

On Tue, Jul 1, 2014 at 10:33 AM, Helmut Raiger <helmut.raiger@hale.at> wrote:
> Hi,
>
>     the commit 41623c91 breaks the SPL on i.mx31 platforms.
> 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.
>
> Does changing the startup code back to PIC generate any
> drawbacks on other ARM platforms?
> What was the intention of the change besides unifying?
>
> Could someone verify the issue on a i.mx31 platform.

Magnus also reported the same issue to me recently.

I don't have access to a mx31 board at the moment to investigate this bug.

Maybe Albert, Stefano or Magnus could help?

Thanks,

Fabio Estevam

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

[Albert ARIBAUD]

Hi Fabio,

On Tue, 1 Jul 2014 10:53:57 -0300, Fabio Estevam <festevam@gmail.com>
wrote:

> Hi Helmut,
> 
> On Tue, Jul 1, 2014 at 10:33 AM, Helmut Raiger <helmut.raiger@hale.at> wrote:
> > Hi,
> >
> >     the commit 41623c91 breaks the SPL on i.mx31 platforms.
> > 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?

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

> > What was the intention of the change besides unifying?

Any other intent would be stated in the commit message.

> > Could someone verify the issue on a i.mx31 platform.
> 
> Magnus also reported the same issue to me recently.
> 
> I don't have access to a mx31 board at the moment to investigate this bug.
> 
> Maybe Albert, Stefano or Magnus could help?

I don't have mx31 hardware either.

> Thanks,
> 
> Fabio Estevam


Amicalement,
-- 
Albert.

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

[Helmut Raiger]

Hi,
> Hi,
>
>      the commit 41623c91 breaks the SPL on i.mx31 platforms.
> 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.

>>> 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.
>>> What was the intention of the change besides unifying?
> Any other intent would be stated in the commit message.
Ok, so the PIC issue was just an oversight.

> Maybe Albert, Stefano or Magnus could help?
> I don't have mx31 hardware either.
>
>
It seems we are the only ones still dealing with this old lad ;-)

Helmut


--
Scanned by MailScanner.

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

[Stefano Babic]

Hi Helmut,

On 01/07/2014 15:53, Fabio Estevam wrote:
> 
> I don't have access to a mx31 board at the moment to investigate this bug.
> 
> Maybe Albert, Stefano or Magnus could help?

I have not at the moment access to a mx31 board, sorry.

Regards,
Stefano



-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

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

[Albert ARIBAUD]

Hi Helmut,

On Wed, 02 Jul 2014 09:04:46 +0200, Helmut Raiger
<helmut.raiger@hale.at> wrote:

> Hi,
> > Hi,
> >
> >      the commit 41623c91 breaks the SPL on i.mx31 platforms.
> > 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.
> 
> >>> 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.
> >>> What was the intention of the change besides unifying?
> > Any other intent would be stated in the commit message.
> Ok, so the PIC issue was just an oversight.
> 
> > Maybe Albert, Stefano or Magnus could help?
> > I don't have mx31 hardware either.
> >
> >
> It seems we are the only ones still dealing with this old lad ;-)

Full position independence was a lucky side effect of the code, not an
intended feature; relocation is precisely here because moving code
around usually won't work.

However, no relocation record (relative or other) are generated for the
indirect vectors, so that'll need a fix as well. It basically works in
U-Boot because most of the targets get loaded at their link-time base
address, so the indirect vectors are correct.

I suspect in your case, SPL was linked for one address and gets jumped
into at another address.

Can you tell me which board exactly you are experiencing the issue on?

> Helmut

Amicalement,
-- 
Albert.

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

[Benoît Thébaudeau]

)Dear Helmut Raiger,

On Wed, Jul 2, 2014 at 9:04 AM, Helmut Raiger <helmut.raiger@hale.at> wrote:
>>      the commit 41623c91 breaks the SPL on i.mx31 platforms.

Here, you are talking about mx31pdk, right?

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

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

Best regards,
Beno?t

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

[Helmut Raiger]

Hi,

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

>>>>> 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.
At a quick glance at the assembler code of board_init_f it looked
PIC to me, but I'll have to check.
I'm currently torn between projects, which is why I have no
further input right now.

Helmut


--
Scanned by MailScanner.

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

[Albert ARIBAUD]

Hi Helmut,

On Thu, 03 Jul 2014 10:19:39 +0200, Helmut Raiger
<helmut.raiger@hale.at> wrote:

> Hi,
> 
> 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@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.

It does, thanks -- but I fail to see any SPL code built for TT-01.
You're getting SPL issues with another target, right?

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

Ok, I think I'm getting it, but actually you don't need PIC (your code
won't run at arbitrary locations), you need VMAs vs LMAs (your code
will run partly at one location, partly at another, but will be loaded
at only one of these locations).

Therefore, we should be able to manage this in the linker script, by
basically defining two output sections: the first one with a VMA and
LMA equal to 0xB8000000 both, and which would contain the 'copier' code;
and the second one with a VMA equal to CONFIG_SPL_TEXT_BASE (so that it
links properly for running at that address) and a LMA equal to 0xB800000
(so that it gets lumped with the first section in the less-than-2K ELF
file produced by the linker.

(actually we'd have several output sections with VMA==LMA, but it
does not affect the core of the idea.)

 Does it make sense to you?

> Helmut

Amicalement,
-- 
Albert.

[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@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@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

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

[Benoît Thébaudeau]

Hi Albert,

On Thu, Jul 3, 2014 at 3:35 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Helmut,
>
> On Thu, 03 Jul 2014 10:19:39 +0200, Helmut Raiger
> <helmut.raiger@hale.at> wrote:
>
>> Hi,
>>
>> 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@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.
>
> It does, thanks -- but I fail to see any SPL code built for TT-01.
> You're getting SPL issues with another target, right?

Helmut seems to be working on a custom TT-01 variant (or just with a
specific hardware configuration using dip switches, or jumpers, etc.)
using the i.MX31 NAND internal boot rather than the mainline boot
source.

>> >> 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.
>
> Ok, I think I'm getting it, but actually you don't need PIC (your code
> won't run at arbitrary locations), you need VMAs vs LMAs (your code
> will run partly at one location, partly at another, but will be loaded
> at only one of these locations).
>
> Therefore, we should be able to manage this in the linker script, by
> basically defining two output sections: the first one with a VMA and
> LMA equal to 0xB8000000 both, and which would contain the 'copier' code;
> and the second one with a VMA equal to CONFIG_SPL_TEXT_BASE (so that it
> links properly for running at that address) and a LMA equal to 0xB800000
> (so that it gets lumped with the first section in the less-than-2K ELF
> file produced by the linker.
>
> (actually we'd have several output sections with VMA==LMA, but it
> does not affect the core of the idea.)
>
>  Does it make sense to you?

It makes sense to me. That should work, but it'd be better to avoid
adding a custom linker script. A simple fix in vectors.S would be
preferable if possible. Also, the __image_copy_start stuff should be
taken care of with such a change.

BTW, I see that you skipped arch/arm/cpu/arm1136/u-boot-spl.lds in
commit 41623c91 (addition of "*(.vectors)"). Was it intentional? It
silently changes woodburn_sd because the fallback exception vectors no
longer exist. This should not cause a build error because the _start
symbol is duplicated in this linker script. The board may also boot
correctly with this, but the default vectors can be useful in some
cases, especially for debugging exceptions.

Cordialement,
Beno?t

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

[Albert ARIBAUD]

Hi Beno?t,

On Thu, 3 Jul 2014 22:58:56 +0200, Beno?t Th?baudeau
<benoit.thebaudeau.dev@gmail.com> wrote:

> Hi Albert,
> 
> On Thu, Jul 3, 2014 at 3:35 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
> > Hi Helmut,
> >
> > On Thu, 03 Jul 2014 10:19:39 +0200, Helmut Raiger
> > <helmut.raiger@hale.at> wrote:
> >
> >> Hi,
> >>
> >> 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@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.
> >
> > It does, thanks -- but I fail to see any SPL code built for TT-01.
> > You're getting SPL issues with another target, right?
> 
> Helmut seems to be working on a custom TT-01 variant (or just with a
> specific hardware configuration using dip switches, or jumpers, etc.)
> using the i.MX31 NAND internal boot rather than the mainline boot
> source.
> 
> >> >> 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.
> >
> > Ok, I think I'm getting it, but actually you don't need PIC (your code
> > won't run at arbitrary locations), you need VMAs vs LMAs (your code
> > will run partly at one location, partly at another, but will be loaded
> > at only one of these locations).
> >
> > Therefore, we should be able to manage this in the linker script, by
> > basically defining two output sections: the first one with a VMA and
> > LMA equal to 0xB8000000 both, and which would contain the 'copier' code;
> > and the second one with a VMA equal to CONFIG_SPL_TEXT_BASE (so that it
> > links properly for running at that address) and a LMA equal to 0xB800000
> > (so that it gets lumped with the first section in the less-than-2K ELF
> > file produced by the linker.
> >
> > (actually we'd have several output sections with VMA==LMA, but it
> > does not affect the core of the idea.)
> >
> >  Does it make sense to you?
> 
> It makes sense to me. That should work, but it'd be better to avoid
> adding a custom linker script. A simple fix in vectors.S would be
> preferable if possible. Also, the __image_copy_start stuff should be
> taken care of with such a change.

I do not intend to have this in a custom linker script; I want this to
be in the common ARM SPL linker script (I hope it is what Helmut's
TT-01 changes use) -- I also want to get rid of all custom linker
scripts in the ARM par of U-Boot, but I could not get this done for
2014.07.

> BTW, I see that you skipped arch/arm/cpu/arm1136/u-boot-spl.lds in
> commit 41623c91 (addition of "*(.vectors)"). Was it intentional?

Not that I can remember: I did modify the arm1136 start.S, so te linker
scripts should have followed. 

> It silently changes woodburn_sd because the fallback exception vectors no
> longer exist. This should not cause a build error because the _start
> symbol is duplicated in this linker script. The board may also boot
> correctly with this, but the default vectors can be useful in some
> cases, especially for debugging exceptions.

Can you post a patch today? I'll pull it in a PR I'll do today before
COB.

> Cordialement,
> Beno?t

Amicalement,
-- 
Albert.

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

[Magnus Lilja]

Hi all,

(I hope gmail formats this mail according to the mailing list requirements,
my apologies if it doesn't)

On 4 July 2014 10:50, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:

> Hi Beno?t,
>
> On Thu, 3 Jul 2014 22:58:56 +0200, Beno?t Th?baudeau
> <benoit.thebaudeau.dev@gmail.com> wrote:
>
> > Hi Albert,
> > It makes sense to me. That should work, but it'd be better to avoid
> > adding a custom linker script. A simple fix in vectors.S would be
> > preferable if possible. Also, the __image_copy_start stuff should be
> > taken care of with such a change.
>
> I do not intend to have this in a custom linker script; I want this to
> be in the common ARM SPL linker script (I hope it is what Helmut's
> TT-01 changes use) -- I also want to get rid of all custom linker
> scripts in the ARM par of U-Boot, but I could not get this done for
> 2014.07.
>
>
I have access to i.MX31 board (PDK) and can definitely help testing any
patch to this problem. However, I won't be able to do any verification
until beginning of August.

Regards, Magnus Lilja

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

[Benoît Thébaudeau]

Hi Albert,

On Fri, Jul 4, 2014 at 10:50 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> On Thu, 3 Jul 2014 22:58:56 +0200, Beno?t Th?baudeau
> <benoit.thebaudeau.dev@gmail.com> wrote:
>> On Thu, Jul 3, 2014 at 3:35 PM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> wrote:
>> > On Thu, 03 Jul 2014 10:19:39 +0200, Helmut Raiger
>> > <helmut.raiger@hale.at> wrote:
>> >> On 07/03/2014 01:20 AM, Beno?t Th?baudeau wrote:
>> >> > On Wed, Jul 2, 2014 at 9:04 AM, Helmut Raiger <helmut.raiger@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.
>> >
>> > It does, thanks -- but I fail to see any SPL code built for TT-01.
>> > You're getting SPL issues with another target, right?
>>
>> Helmut seems to be working on a custom TT-01 variant (or just with a
>> specific hardware configuration using dip switches, or jumpers, etc.)
>> using the i.MX31 NAND internal boot rather than the mainline boot
>> source.
>>
>> >> >> 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.
>> >
>> > Ok, I think I'm getting it, but actually you don't need PIC (your code
>> > won't run at arbitrary locations), you need VMAs vs LMAs (your code
>> > will run partly at one location, partly at another, but will be loaded
>> > at only one of these locations).
>> >
>> > Therefore, we should be able to manage this in the linker script, by
>> > basically defining two output sections: the first one with a VMA and
>> > LMA equal to 0xB8000000 both, and which would contain the 'copier' code;
>> > and the second one with a VMA equal to CONFIG_SPL_TEXT_BASE (so that it
>> > links properly for running at that address) and a LMA equal to 0xB800000
>> > (so that it gets lumped with the first section in the less-than-2K ELF
>> > file produced by the linker.
>> >
>> > (actually we'd have several output sections with VMA==LMA, but it
>> > does not affect the core of the idea.)
>> >
>> >  Does it make sense to you?
>>
>> It makes sense to me. That should work, but it'd be better to avoid
>> adding a custom linker script. A simple fix in vectors.S would be
>> preferable if possible. Also, the __image_copy_start stuff should be
>> taken care of with such a change.
>
> I do not intend to have this in a custom linker script; I want this to
> be in the common ARM SPL linker script (I hope it is what Helmut's
> TT-01 changes use) -- I also want to get rid of all custom linker
> scripts in the ARM par of U-Boot, but I could not get this done for
> 2014.07.

Looks good.

>> BTW, I see that you skipped arch/arm/cpu/arm1136/u-boot-spl.lds in
>> commit 41623c91 (addition of "*(.vectors)"). Was it intentional?
>
> Not that I can remember: I did modify the arm1136 start.S, so te linker
> scripts should have followed.

OK. There seems to be the same issue for
board/ait/cam_enc_4xx/u-boot-spl.lds . All the ARM arch and ARM boards
linker scripts should probably be rechecked.

>> It silently changes woodburn_sd because the fallback exception vectors no
>> longer exist. This should not cause a build error because the _start
>> symbol is duplicated in this linker script. The board may also boot
>> correctly with this, but the default vectors can be useful in some
>> cases, especially for debugging exceptions.
>
> Can you post a patch today? I'll pull it in a PR I'll do today before
> COB.

Sorry, I can't before next week at best.

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

[Benoît Thébaudeau]

Hi Magnus,

On Fri, Jul 4, 2014 at 12:42 PM, Magnus Lilja <lilja.magnus@gmail.com> wrote:
> Hi all,
>
> (I hope gmail formats this mail according to the mailing list requirements,
> my apologies if it doesn't)

I think that HTML is forbidden. You can switch Gmail to plain text
mode with the "More options" down arrow in the lower right corner of
the composing frame.

> On 4 July 2014 10:50, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
>>
>> Hi Beno?t,
>>
>> On Thu, 3 Jul 2014 22:58:56 +0200, Beno?t Th?baudeau
>> <benoit.thebaudeau.dev@gmail.com> wrote:
>>
>> > Hi Albert,
>> > It makes sense to me. That should work, but it'd be better to avoid
>> > adding a custom linker script. A simple fix in vectors.S would be
>> > preferable if possible. Also, the __image_copy_start stuff should be
>> > taken care of with such a change.
>>
>> I do not intend to have this in a custom linker script; I want this to
>> be in the common ARM SPL linker script (I hope it is what Helmut's
>> TT-01 changes use) -- I also want to get rid of all custom linker
>> scripts in the ARM par of U-Boot, but I could not get this done for
>> 2014.07.
>>
>
> I have access to i.MX31 board (PDK) and can definitely help testing any
> patch to this problem.

Thanks, that will help. The first things to test will be:
 - confirm the issue on mx31pdk with mainline,
 - confirm that reverting commit 41623c91 on top of mainline is
sufficient to fix the issue,
 - test from mainline whether removing the two "_reset" lines in
vectors.S and adding "b reset" or "bl reset" right after "_start:"
(still in vectors.S) fixes the issue or improves things in any way.

> However, I won't be able to do any verification until
> beginning of August.

No problem. Maybe Helmut will make some progress in the meantime.

Regards,
Beno?t

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

[Helmut Raiger]

On 07/03/2014 10:58 PM, Beno?t Th?baudeau wrote:
> Hi,
>
> On Thu, Jul 3, 2014 at 10:19 AM, Helmut Raiger <helmut.raiger@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@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.

1) Simply reverting the 41623c91 on HEAD makes it work again.
2) Replace ldr pc, _reset with b reset, still hangs
3) I'm using no special linker scripts, board_init_f() is pretty much 
the same as in mx31pdk.


> 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.
Yes reverting 41623c91 on HEAD works, there is no other offending commit.

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

I meant, that the SPL is now doing the RAM init and copying of the SPL code
correctly. RAM is working, the SPL code is at 0x87dc0000 after that 
(CRCed it via JTAG).
I could not track it further (I have very limited development time right 
now ... repeating myself).

After all I need to debug further. If someone could test the current 
state on the
mx31pdk, this still would be great. Just to rule out any other board 
specific issues.

Helmut


--
Scanned by MailScanner.

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

[Helmut Raiger]

On 07/08/2014 10:05 AM, Helmut Raiger wrote:
>
> I meant, that the SPL is now doing the RAM init and copying of the SPL 
> code
> correctly. RAM is working, the SPL code is at 0x87dc0000 after that 
> (CRCed it via JTAG).
> I could not track it further (I have very limited development time 
> right now ... repeating myself).
>
> After all I need to debug further. If someone could test the current 
> state on the
> mx31pdk, this still would be great. Just to rule out any other board 
> specific issues.
>
> Helmut

Ok, I've got some new input and I have no clue what's going on.

1) I fixed the first branch to reset (uses b reset instead of ldr pc, 
_reset)
2) The startup does:

   b reset
...

reset:
     - set the cpu to SVC32 mode
     - bl  cpu_init_crit (cache and MMU stuff, calls low_level_init)
     - bl _main

low_level_init:
    unlike in mx31pdk this only does:

     ldr    r0, =ARM_PPMRR      /* start from AIPS 2GB region */
     mcr    p15, 0, r0, c15, c2, 4
     mov    pc, lr

_main:
     - set up stack pointer to internal SRAM
     - bl board_init_f

As I can't break right after reset with peedi, I do:

   - reset stop
   - set pc 0xb8000000
   - go -> system never returns

next:
    reset stop
    - set pc 0xb8000000
    - break add hard 0xb80000c8        // set hardware breakpoint to the 
'bl board_init_f' instruction
    - go
     ... it stops at the breakpoint
    - go
     ... I'm at the u-boot command prompt

If I set the breakpoint somewhere after the bl board_init_f it fails, if 
I break a few instructions
early and go from there it also fails. Tracking down the location 
in-between I end up at:

at the end of cpu_init_crit():

     mov    ip, lr        /* persevere link reg across call */
     bl    lowlevel_init    /* go setup pll,mux,memory */
->    mov    lr, ip        /* restore link */
     mov    pc, lr        /* back to my caller */

That is, if I set the breakpoint to mov lr, ip it works, if I set it one 
instruction later it ends working.

Could be struggling with the debugger here, as the SPL probably ran a 
few instruction before it
was stopped. Might this be a cache issue? But it is invalidated in 
cpu_init_crit() anyway ...

Feeling a little dumbstruck right now ...

Sorry for the mess, but it's hard to describe.

Any ideas?
Helmut


--
Scanned by MailScanner.

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

[Helmut Raiger]

On 07/08/2014 02:32 PM, Helmut Raiger wrote:
> On 07/08/2014 10:05 AM, Helmut Raiger wrote:
>>
>> I meant, that the SPL is now doing the RAM init and copying of the 
>> SPL code
>> correctly. RAM is working, the SPL code is at 0x87dc0000 after that 
>> (CRCed it via JTAG).
>> I could not track it further (I have very limited development time 
>> right now ... repeating myself).
>>
>> After all I need to debug further. If someone could test the current 
>> state on the
>> mx31pdk, this still would be great. Just to rule out any other board 
>> specific issues.
>>
>> Helmut
>
> Ok, I've got some new input and I have no clue what's going on.
>
> 1) I fixed the first branch to reset (uses b reset instead of ldr pc, 
> _reset)
> 2) The startup does:
>
>   b reset
> ...
>
> reset:
>     - set the cpu to SVC32 mode
>     - bl  cpu_init_crit (cache and MMU stuff, calls low_level_init)
>     - bl _main
>
> low_level_init:
>    unlike in mx31pdk this only does:
>
>     ldr    r0, =ARM_PPMRR      /* start from AIPS 2GB region */
>     mcr    p15, 0, r0, c15, c2, 4
>     mov    pc, lr
>
> _main:
>     - set up stack pointer to internal SRAM
>     - bl board_init_f
>
> As I can't break right after reset with peedi, I do:
>
>   - reset stop
>   - set pc 0xb8000000
>   - go -> system never returns
>
> next:
>    reset stop
>    - set pc 0xb8000000
>    - break add hard 0xb80000c8        // set hardware breakpoint to 
> the 'bl board_init_f' instruction
>    - go
>     ... it stops at the breakpoint
>    - go
>     ... I'm at the u-boot command prompt
>
> If I set the breakpoint somewhere after the bl board_init_f it fails, 
> if I break a few instructions
> early and go from there it also fails. Tracking down the location 
> in-between I end up at:
>
> at the end of cpu_init_crit():
>
>     mov    ip, lr        /* persevere link reg across call */
>     bl    lowlevel_init    /* go setup pll,mux,memory */
> ->    mov    lr, ip        /* restore link */
>     mov    pc, lr        /* back to my caller */
>
> That is, if I set the breakpoint to mov lr, ip it works, if I set it 
> one instruction later it ends working.
>
> Could be struggling with the debugger here, as the SPL probably ran a 
> few instruction before it
> was stopped. Might this be a cache issue? But it is invalidated in 
> cpu_init_crit() anyway ...
>
> Feeling a little dumbstruck right now ...
>
> Sorry for the mess, but it's hard to describe.
>
> Any ideas?
> Helmut

Just putting 'to whom it may concern' in CC, don't know the rule here.
Helmut


--
Scanned by MailScanner.

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

[Helmut Raiger]

Hi,

     finally I have the SPL running again, with this fix:

diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S
index 1cfcca9..53bde12 100644
--- a/arch/arm/cpu/arm1136/start.S
+++ b/arch/arm/cpu/arm1136/start.S
@@ -91,4 +91,9 @@ cpu_init_crit:
         bl      lowlevel_init   /* go setup pll,mux,memory */
         mov     lr, ip          /* restore link */
         mov     pc, lr          /* back to my caller */
+
+       nop
+       nop
+       nop
+
  #endif /* CONFIG_SKIP_LOWLEVEL_INIT */
diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
index d68cc47..8c0e3c1 100644
--- a/arch/arm/lib/vectors.S
+++ b/arch/arm/lib/vectors.S
@@ -50,7 +50,8 @@ _start:
  #endif

  _start:
-       ldr     pc, _reset
+       /* be position independent if SPL is linked at different location */
+       b       reset
         ldr     pc, _undefined_instruction
         ldr     pc, _software_interrupt
         ldr     pc, _prefetch_abort


I have no clue why the nops are necessary after cpu_init_crit(), but it 
won't work
unless there are at least 3 nops there (pipeline?).
It would be nice to have _some_ explanation for this before posting a patch.

Helmut

--
Scanned by MailScanner.

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

[Magnus Lilja]

Hi

On 8 July 2014 10:05, Helmut Raiger <helmut.raiger@hale.at> wrote:
> I meant, that the SPL is now doing the RAM init and copying of the SPL code
> correctly. RAM is working, the SPL code is at 0x87dc0000 after that (CRCed
> it via JTAG).
> I could not track it further (I have very limited development time right now
> ... repeating myself).
>
> After all I need to debug further. If someone could test the current state
> on the
> mx31pdk, this still would be great. Just to rule out any other board
> specific issues.

I can confirm that a recent U-boot (I think it was 2014.04) did not
work for me on mx31pdk, I think I had to go back to 2013.04 to get a
working U-boot.

So I don't think it's a board specific issue.

Regards, Magnus

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

[Helmut Raiger]

On 07/11/2014 09:56 AM, Magnus Lilja wrote:
> Hi
>
> On 8 July 2014 10:05, Helmut Raiger <helmut.raiger@hale.at> wrote:
>> I meant, that the SPL is now doing the RAM init and copying of the SPL code
>> correctly. RAM is working, the SPL code is at 0x87dc0000 after that (CRCed
>> it via JTAG).
>> I could not track it further (I have very limited development time right now
>> ... repeating myself).
>>
>> After all I need to debug further. If someone could test the current state
>> on the
>> mx31pdk, this still would be great. Just to rule out any other board
>> specific issues.
> I can confirm that a recent U-boot (I think it was 2014.04) did not
> work for me on mx31pdk, I think I had to go back to 2013.04 to get a
> working U-boot.
>
> So I don't think it's a board specific issue.
>
> Regards, Magnus
>
> --
> Scanned by MailScanner.
>
Hi,

    thx Magnus for the test, could you possibly change the few lines of 
code and test again:

diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S
index 1cfcca9..53bde12 100644
--- a/arch/arm/cpu/arm1136/start.S
+++ b/arch/arm/cpu/arm1136/start.S
@@ -91,4 +91,9 @@ cpu_init_crit:
         bl      lowlevel_init   /* go setup pll,mux,memory */
         mov     lr, ip          /* restore link */
         mov     pc, lr          /* back to my caller */
+
+       nop
+       nop
+       nop
+
  #endif /* CONFIG_SKIP_LOWLEVEL_INIT */
diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
index d68cc47..8c0e3c1 100644
--- a/arch/arm/lib/vectors.S
+++ b/arch/arm/lib/vectors.S
@@ -50,7 +50,8 @@ _start:
  #endif

  _start:
-       ldr     pc, _reset
+       /* be position independent if SPL is linked at different 
location */
+       b       reset
         ldr     pc, _undefined_instruction
         ldr     pc, _software_interrupt
         ldr     pc, _prefetch_abort


This fixes the SPL, in a later test I had to another nop and I still have
no explanation why.

Again thanks for the support.
Helmut


--
Scanned by MailScanner.

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

[Magnus Lilja]

Hi

On 15 July 2014 11:31, Helmut Raiger <helmut.raiger@hale.at> wrote:
> Hi,
>
>    thx Magnus for the test, could you possibly change the few lines of code
> and test again:
>
> diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S
> index 1cfcca9..53bde12 100644
> --- a/arch/arm/cpu/arm1136/start.S
> +++ b/arch/arm/cpu/arm1136/start.S
> @@ -91,4 +91,9 @@ cpu_init_crit:
>
>         bl      lowlevel_init   /* go setup pll,mux,memory */
>         mov     lr, ip          /* restore link */
>         mov     pc, lr          /* back to my caller */
> +
> +       nop
> +       nop
> +       nop
> +
>  #endif /* CONFIG_SKIP_LOWLEVEL_INIT */
> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
> index d68cc47..8c0e3c1 100644
> --- a/arch/arm/lib/vectors.S
> +++ b/arch/arm/lib/vectors.S
> @@ -50,7 +50,8 @@ _start:
>  #endif
>
>  _start:
> -       ldr     pc, _reset
> +       /* be position independent if SPL is linked at different location */
> +       b       reset
>         ldr     pc, _undefined_instruction
>         ldr     pc, _software_interrupt
>         ldr     pc, _prefetch_abort
>
>
> This fixes the SPL, in a later test I had to another nop and I still have
> no explanation why.
>
> Again thanks for the support.

I have now done some tests on i.MX31 PDK:
* v2013.04 and v2014.04 works
* v2014.07 and "tip of tree as of this mail" does not work

To make it work (both v2014.07 and tip of tree) I only have to change
the "ldr pc, _reset" to "b reset". I do not have to apply the nop-part
of the patch.

On a different note I noticed that I received a warning that the PDK
board does not use "generic board", I will try to fix this but I don't
know how much work that is and when I can submit a patch.

Regards, Magnus

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

[Jeroen Hofstee]

Hello Magnus,

On 04-08-14 22:23, Magnus Lilja wrote:

<snip>
>
> On a different note I noticed that I received a warning that the PDK
> board does not use "generic board", I will try to fix this but I don't
> know how much work that is and when I can submit a patch.
typically it is trivial, just add:

#define CONFIG_SYS_GENERIC_BOARD

Once tested Albert typically picks this up also after the
merge window as a trivial fix. As a bonus the board might
be compiled with clang ;)

Regards,
Jeroen

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

[Fabio Estevam]

Hi Magnus,

On Mon, Aug 4, 2014 at 5:23 PM, Magnus Lilja <lilja.magnus@gmail.com> wrote:

> I have now done some tests on i.MX31 PDK:
> * v2013.04 and v2014.04 works
> * v2014.07 and "tip of tree as of this mail" does not work
>
> To make it work (both v2014.07 and tip of tree) I only have to change
> the "ldr pc, _reset" to "b reset". I do not have to apply the nop-part
> of the patch.

Could you please submit this change as a formal patch to the list?
Please Cc Albert Aribaud on it.

> On a different note I noticed that I received a warning that the PDK
> board does not use "generic board", I will try to fix this but I don't
> know how much work that is and when I can submit a patch.

Just add "#define CONFIG_SYS_GENERIC_BOARD" into the board config file
and give it a try.

If it works, please send this patch as well (with Stefano Babic on Cc).

Thanks

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

[Magnus Lilja]

Hi Fabio,

On 5 August 2014 14:28, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Magnus,
>
> On Mon, Aug 4, 2014 at 5:23 PM, Magnus Lilja <lilja.magnus@gmail.com> wrote:
>
>> I have now done some tests on i.MX31 PDK:
>> * v2013.04 and v2014.04 works
>> * v2014.07 and "tip of tree as of this mail" does not work
>>
>> To make it work (both v2014.07 and tip of tree) I only have to change
>> the "ldr pc, _reset" to "b reset". I do not have to apply the nop-part
>> of the patch.
>
> Could you please submit this change as a formal patch to the list?
> Please Cc Albert Aribaud on it.

I would expect Helmut to create a formal patch then I can test that
and add a Tested-by.

>> On a different note I noticed that I received a warning that the PDK
>> board does not use "generic board", I will try to fix this but I don't
>> know how much work that is and when I can submit a patch.
>
> Just add "#define CONFIG_SYS_GENERIC_BOARD" into the board config file
> and give it a try.
>
> If it works, please send this patch as well (with Stefano Babic on Cc).

That I can do.

Regards, Magnus

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

[Helmut Raiger]

On 08/05/2014 02:32 PM, Magnus Lilja wrote:
> Hi Fabio,
>
> On 5 August 2014 14:28, Fabio Estevam <festevam@gmail.com> wrote:
>> Hi Magnus,
>>
>> On Mon, Aug 4, 2014 at 5:23 PM, Magnus Lilja <lilja.magnus@gmail.com> wrote:
>>
>>> I have now done some tests on i.MX31 PDK:
>>> * v2013.04 and v2014.04 works
>>> * v2014.07 and "tip of tree as of this mail" does not work
>>>
>>> To make it work (both v2014.07 and tip of tree) I only have to change
>>> the "ldr pc, _reset" to "b reset". I do not have to apply the nop-part
>>> of the patch.
>> Could you please submit this change as a formal patch to the list?
>> Please Cc Albert Aribaud on it.
> I would expect Helmut to create a formal patch then I can test that
> and add a Tested-by.
>
The problem is it does not work with only the 'b reset' change on my 
platform.
Should I provide a patch with the nops and the question marks around them?
It still could be a toolchain difference, mine is pretty old:

$ arm-angstrom-linux-gnueabi-gcc --version
arm-angstrom-linux-gnueabi-gcc (GCC) 4.7.2
Copyright (C) 2012 Free Software Foundation, Inc.

When I objdump the elf file I can see the very same code in 
cpu_init_crit() as in start.S,
whatever that might mean (objdump is from the same toolchain).

>>> On a different note I noticed that I received a warning that the PDK
>>> board does not use "generic board", I will try to fix this but I don't
>>> know how much work that is and when I can submit a patch.
>> Just add "#define CONFIG_SYS_GENERIC_BOARD" into the board config file
>> and give it a try.
>>
>> If it works, please send this patch as well (with Stefano Babic on Cc).
> That I can do.
>
> Regards, Magnus

I tested the CONFIG_SYS_GENERIC_BOARD on TT-01 and it works fine.

Helmut

--
Scanned by MailScanner.

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

[Magnus Lilja]

Hi

On 13 August 2014 14:01, Helmut Raiger <helmut.raiger@hale.at> wrote:
> On 08/05/2014 02:32 PM, Magnus Lilja wrote:
>>
>> Hi Fabio,
>>
>> On 5 August 2014 14:28, Fabio Estevam <festevam@gmail.com> wrote:
>>>
>>> Hi Magnus,
>>>
>> I would expect Helmut to create a formal patch then I can test that
>> and add a Tested-by.
>>
> The problem is it does not work with only the 'b reset' change on my
> platform.
> Should I provide a patch with the nops and the question marks around them?
> It still could be a toolchain difference, mine is pretty old:
>
> $ arm-angstrom-linux-gnueabi-gcc --version
> arm-angstrom-linux-gnueabi-gcc (GCC) 4.7.2
> Copyright (C) 2012 Free Software Foundation, Inc.
>
> When I objdump the elf file I can see the very same code in cpu_init_crit()
> as in start.S,
> whatever that might mean (objdump is from the same toolchain).

I use an even older gcc so I don't think that's the problem. I use:
arm-none-linux-gnueabi-gcc (Sourcery CodeBench Lite 2011.09-70) 4.6.1

Not sure how you should proceed with the path.

Regards,

Magnus

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

[Benoît Thébaudeau]

Hi Magnus, all,

On Fri, Aug 15, 2014 at 7:45 PM, Magnus Lilja <lilja.magnus@gmail.com> wrote:
> Hi
>
> On 13 August 2014 14:01, Helmut Raiger <helmut.raiger@hale.at> wrote:
>> On 08/05/2014 02:32 PM, Magnus Lilja wrote:
>>>
>>> Hi Fabio,
>>>
>>> On 5 August 2014 14:28, Fabio Estevam <festevam@gmail.com> wrote:
>>>>
>>>> Hi Magnus,
>>>>
>>> I would expect Helmut to create a formal patch then I can test that
>>> and add a Tested-by.
>>>
>> The problem is it does not work with only the 'b reset' change on my
>> platform.
>> Should I provide a patch with the nops and the question marks around them?
>> It still could be a toolchain difference, mine is pretty old:
>>
>> $ arm-angstrom-linux-gnueabi-gcc --version
>> arm-angstrom-linux-gnueabi-gcc (GCC) 4.7.2
>> Copyright (C) 2012 Free Software Foundation, Inc.
>>
>> When I objdump the elf file I can see the very same code in cpu_init_crit()
>> as in start.S,
>> whatever that might mean (objdump is from the same toolchain).
>
> I use an even older gcc so I don't think that's the problem. I use:
> arm-none-linux-gnueabi-gcc (Sourcery CodeBench Lite 2011.09-70) 4.6.1
>
> Not sure how you should proceed with the path.

IMHO, the 'b reset' and the 'nop nop nop' are two different issues, so
Helmut should create a formal patch for the 'b reset' issue right now,
which will fix mx31pdk (and maybe other boards) for the release. Then,
once the 'nop nop nop' issue has been resolved for TT-01 (cache issue
or something else), another formal patch should be created for this
issue, unless it is purely out of tree.

Albert, Fabio, what do you think?

Best regards,
Beno?t

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

[Fabio Estevam]

On Tue, Aug 19, 2014 at 5:55 PM, Beno?t Th?baudeau
<benoit.thebaudeau.dev@gmail.com> wrote:

>> Not sure how you should proceed with the path.
>
> IMHO, the 'b reset' and the 'nop nop nop' are two different issues, so
> Helmut should create a formal patch for the 'b reset' issue right now,
> which will fix mx31pdk (and maybe other boards) for the release. Then,
> once the 'nop nop nop' issue has been resolved for TT-01 (cache issue
> or something else), another formal patch should be created for this
> issue, unless it is purely out of tree.
>
> Albert, Fabio, what do you think?

Sounds like a good approach.

Thanks

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

[Benoît Thébaudeau]

Hi Helmut, all,

On Tue, Aug 19, 2014 at 10:55 PM, Beno?t Th?baudeau
<benoit.thebaudeau.dev@gmail.com> wrote:
> On Fri, Aug 15, 2014 at 7:45 PM, Magnus Lilja <lilja.magnus@gmail.com> wrote:
>> On 13 August 2014 14:01, Helmut Raiger <helmut.raiger@hale.at> wrote:
>>> On 08/05/2014 02:32 PM, Magnus Lilja wrote:
>>>> I would expect Helmut to create a formal patch then I can test that
>>>> and add a Tested-by.
>>>>
>>> The problem is it does not work with only the 'b reset' change on my
>>> platform.
>>> Should I provide a patch with the nops and the question marks around them?
>>> It still could be a toolchain difference, mine is pretty old:
>>>
>>> $ arm-angstrom-linux-gnueabi-gcc --version
>>> arm-angstrom-linux-gnueabi-gcc (GCC) 4.7.2
>>> Copyright (C) 2012 Free Software Foundation, Inc.
>>>
>>> When I objdump the elf file I can see the very same code in cpu_init_crit()
>>> as in start.S,
>>> whatever that might mean (objdump is from the same toolchain).
>>
>> I use an even older gcc so I don't think that's the problem. I use:
>> arm-none-linux-gnueabi-gcc (Sourcery CodeBench Lite 2011.09-70) 4.6.1
>>
>> Not sure how you should proceed with the path.
>
> IMHO, the 'b reset' and the 'nop nop nop' are two different issues, so
> Helmut should create a formal patch for the 'b reset' issue right now,
> which will fix mx31pdk (and maybe other boards) for the release. Then,
> once the 'nop nop nop' issue has been resolved for TT-01 (cache issue
> or something else), another formal patch should be created for this
> issue, unless it is purely out of tree.

v2014.10 is getting closer with the release of -rc2. It would be much
better to get mx31pdk fixed for this release. Helmut, can you send a
patch for the 'b reset' issue? If not, do you agree that someone else
(maybe the board maintainer: Magnus?) sends it with a 'Reported-by:
Helmut Raiger <helmut.raiger@hale.at>'?

Best regards,
Beno?t

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

[Tom Rini]

On Wed, Sep 03, 2014 at 02:53:17AM +0200, Beno?t Th?baudeau wrote:
> Hi Helmut, all,
> 
> On Tue, Aug 19, 2014 at 10:55 PM, Beno?t Th?baudeau
> <benoit.thebaudeau.dev@gmail.com> wrote:
> > On Fri, Aug 15, 2014 at 7:45 PM, Magnus Lilja <lilja.magnus@gmail.com> wrote:
> >> On 13 August 2014 14:01, Helmut Raiger <helmut.raiger@hale.at> wrote:
> >>> On 08/05/2014 02:32 PM, Magnus Lilja wrote:
> >>>> I would expect Helmut to create a formal patch then I can test that
> >>>> and add a Tested-by.
> >>>>
> >>> The problem is it does not work with only the 'b reset' change on my
> >>> platform.
> >>> Should I provide a patch with the nops and the question marks around them?
> >>> It still could be a toolchain difference, mine is pretty old:
> >>>
> >>> $ arm-angstrom-linux-gnueabi-gcc --version
> >>> arm-angstrom-linux-gnueabi-gcc (GCC) 4.7.2
> >>> Copyright (C) 2012 Free Software Foundation, Inc.
> >>>
> >>> When I objdump the elf file I can see the very same code in cpu_init_crit()
> >>> as in start.S,
> >>> whatever that might mean (objdump is from the same toolchain).
> >>
> >> I use an even older gcc so I don't think that's the problem. I use:
> >> arm-none-linux-gnueabi-gcc (Sourcery CodeBench Lite 2011.09-70) 4.6.1
> >>
> >> Not sure how you should proceed with the path.
> >
> > IMHO, the 'b reset' and the 'nop nop nop' are two different issues, so
> > Helmut should create a formal patch for the 'b reset' issue right now,
> > which will fix mx31pdk (and maybe other boards) for the release. Then,
> > once the 'nop nop nop' issue has been resolved for TT-01 (cache issue
> > or something else), another formal patch should be created for this
> > issue, unless it is purely out of tree.
> 
> v2014.10 is getting closer with the release of -rc2. It would be much
> better to get mx31pdk fixed for this release. Helmut, can you send a
> patch for the 'b reset' issue? If not, do you agree that someone else
> (maybe the board maintainer: Magnus?) sends it with a 'Reported-by:
> Helmut Raiger <helmut.raiger@hale.at>'?

Yes, please, I'd like to see this fixed for the release proper.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140902/eb98954b/attachment.pgp>

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

[Magnus Lilja]

Hi

On 3 September 2014 03:13, Tom Rini <trini@ti.com> wrote:
> On Wed, Sep 03, 2014 at 02:53:17AM +0200, Beno?t Th?baudeau wrote:
>> > IMHO, the 'b reset' and the 'nop nop nop' are two different issues, so
>> > Helmut should create a formal patch for the 'b reset' issue right now,
>> > which will fix mx31pdk (and maybe other boards) for the release. Then,
>> > once the 'nop nop nop' issue has been resolved for TT-01 (cache issue
>> > or something else), another formal patch should be created for this
>> > issue, unless it is purely out of tree.
>>
>> v2014.10 is getting closer with the release of -rc2. It would be much
>> better to get mx31pdk fixed for this release. Helmut, can you send a
>> patch for the 'b reset' issue? If not, do you agree that someone else
>> (maybe the board maintainer: Magnus?) sends it with a 'Reported-by:
>> Helmut Raiger <helmut.raiger@hale.at>'?
>
> Yes, please, I'd like to see this fixed for the release proper.

I'll try to test and submit a match in a couple of days.

Regards, Magnus

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

[Benoît Thébaudeau]

Hi Magnus,

On Thu, Sep 4, 2014 at 9:12 PM, Magnus Lilja <lilja.magnus@gmail.com> wrote:
> On 3 September 2014 03:13, Tom Rini <trini@ti.com> wrote:
>> On Wed, Sep 03, 2014 at 02:53:17AM +0200, Beno?t Th?baudeau wrote:
>>> > IMHO, the 'b reset' and the 'nop nop nop' are two different issues, so
>>> > Helmut should create a formal patch for the 'b reset' issue right now,
>>> > which will fix mx31pdk (and maybe other boards) for the release. Then,
>>> > once the 'nop nop nop' issue has been resolved for TT-01 (cache issue
>>> > or something else), another formal patch should be created for this
>>> > issue, unless it is purely out of tree.
>>>
>>> v2014.10 is getting closer with the release of -rc2. It would be much
>>> better to get mx31pdk fixed for this release. Helmut, can you send a
>>> patch for the 'b reset' issue? If not, do you agree that someone else
>>> (maybe the board maintainer: Magnus?) sends it with a 'Reported-by:
>>> Helmut Raiger <helmut.raiger@hale.at>'?
>>
>> Yes, please, I'd like to see this fixed for the release proper.
>
> I'll try to test and submit a match in a couple of days.

I have already submitted a patch for that yesterday. It's 'arm: Make
reset position-independent', and you were in Cc. So you just have to
test and to add your 'Tested-by'.

Thanks in advance.

Best regards,
Beno?t

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

[Magnus Lilja]

Hi


On 4 September 2014 21:50, Beno?t Th?baudeau
<benoit.thebaudeau.dev@gmail.com> wrote:
> Hi Magnus,
>
>>>> v2014.10 is getting closer with the release of -rc2. It would be much
>>>> better to get mx31pdk fixed for this release. Helmut, can you send a
>>>> patch for the 'b reset' issue? If not, do you agree that someone else
>>>> (maybe the board maintainer: Magnus?) sends it with a 'Reported-by:
>>>> Helmut Raiger <helmut.raiger@hale.at>'?
>>>
>>> Yes, please, I'd like to see this fixed for the release proper.
>>
>> I'll try to test and submit a match in a couple of days.
>
> I have already submitted a patch for that yesterday. It's 'arm: Make
> reset position-independent', and you were in Cc. So you just have to
> test and to add your 'Tested-by'.

Ah, missed that. Excellent, I'll test that within a day or two then!

Regards, Magnus