diff mbox

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

Message ID 53C4F50B.2070502@hale.at
State Not Applicable
Delegated to: Stefano Babic
Headers show

Commit Message

Helmut Raiger July 15, 2014, 9:31 a.m. UTC
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:

         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.

Comments

Magnus Lilja Aug. 4, 2014, 8:23 p.m. UTC | #1
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
Jeroen Hofstee Aug. 4, 2014, 8:40 p.m. UTC | #2
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
Fabio Estevam Aug. 5, 2014, 12:28 p.m. UTC | #3
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
Magnus Lilja Aug. 5, 2014, 12:32 p.m. UTC | #4
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
Helmut Raiger Aug. 13, 2014, 12:01 p.m. UTC | #5
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.
Magnus Lilja Aug. 15, 2014, 5:45 p.m. UTC | #6
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
Benoît Thébaudeau Aug. 19, 2014, 8:55 p.m. UTC | #7
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
Fabio Estevam Aug. 19, 2014, 10:01 p.m. UTC | #8
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
Benoît Thébaudeau Sept. 3, 2014, 12:53 a.m. UTC | #9
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
Tom Rini Sept. 3, 2014, 1:13 a.m. UTC | #10
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.
Magnus Lilja Sept. 4, 2014, 7:12 p.m. UTC | #11
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
Benoît Thébaudeau Sept. 4, 2014, 7:50 p.m. UTC | #12
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
Magnus Lilja Sept. 4, 2014, 7:59 p.m. UTC | #13
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
diff mbox

Patch

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