[U-Boot] SPL broken on i.mx31 platforms
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.
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
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,
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.
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 resetwhich 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,
)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 resetwhich 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
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 resetwhich 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.
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,
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
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,
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
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
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.
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 resetwhich 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
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.
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.
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.
- 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 promptIf 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.
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.
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
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.
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 */
nopnopnop#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_abortThis 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
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
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
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
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.
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
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
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
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
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.
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
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
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
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
participants (8)
-
Albert ARIBAUD -
Benoît Thébaudeau -
Fabio Estevam -
Helmut Raiger -
Jeroen Hofstee -
Magnus Lilja -
Stefano Babic -
Tom Rini