Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lpc1768 bootloader support #5038

Merged
merged 6 commits into from
Oct 5, 2017

Conversation

chrissnow
Copy link
Contributor

Description

Enable bootloaders for LPC1768 targets.

CRP word was being added to application builds which would overlap the bootloader and cause an sector erase inside the bootloader region.

Linker scripts have been updated.

Status

READY

Migrations

NO

Related PRs

Todos

Deploy notes

Steps to test or reproduce

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 6, 2017

CRP word was being added to application builds which would overlap the bootloader and cause an sector erase inside the bootloader region.

How is this to be done? Is it via targets/TARGET_NXP/TARGET_LPC176X/device/CRP.c the file that places the number directly at defined memory?

@chrissnow
Copy link
Contributor Author

It was previously inside the startup file but since the assembly flags didn't provide anything to distinguish from a bootloaded application to a normal one I moved it to CRP.c


I now realise there is some more work to do as I forgot there is a startup file per toolchain.

@chrissnow
Copy link
Contributor Author

@0xc0170 do you think this is the right approach before I sort the others?

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 7, 2017

Jenkins CI fails:

21:59:11 [ARCH_PRO_arm-none-eabi-gcc_minimal] /usr/local/gcc-arm-none-eabi-4_9-2015q3/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld:.build/ARCH_PRO_arm-none-eabi-gcc_minimal/.link_script.ld:15: warning: memory region `m_data' not declared
21:59:11 [ARCH_PRO_arm-none-eabi-gcc_minimal] /usr/local/gcc-arm-none-eabi-4_9-2015q3/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld: section .CRPSection loaded at [000002fc,000002ff] overlaps section .text loaded at [00000000,00013263]
21:59:11 [ARCH_PRO_arm-none-eabi-gcc_minimal] collect2: error: ld returned 1 exit status

@0xc0170 do you think this is the right approach before I sort the others?

I do not fully understand the since the assembly flags didn't provide anything to distinguish from a bootloaded application to a normal one

@chrissnow
Copy link
Contributor Author

I have messed up the GCC linker script, odd how it built for me but I know why it's failed.

The problem is I need to know whether to include the CRP word based on a compiler flag such as BOOTLOADER_ADDR, it must not be included in the build if the app is bootloaded (the bootloader already set's CRP)

However that inst available in assembly compilation so I moved it into a C file.
this means the CRP word must be located correctly but GCC doesn't seem to allow static positioning like this

__attribute__((section(".ARM.__at_0x000002FC")))

so I need to place it with the linker.

@chrissnow
Copy link
Contributor Author

CRP is now in the correct place for GCC,IAR and ARM, this will be included in the bootloader and the application.

This is done so if the application bin was flashed directly it would still have a valid CRP.

CRP verified by checking .bin files manually.

@b-kelder
Copy link

b-kelder commented Sep 12, 2017

Thanks for getting this to work, I had some trouble figuring it out myself. I tested the implementation and everything is working on my end.

@chrissnow
Copy link
Contributor Author

@bramkelder Glad it works for you too, IAP is fixed in anther PR if you need that too.
you can also use this branch which has it all merged for testing
https://github.com/chrissnow/mbed-os/tree/CSDEV

It's been a bit of trouble to get it all to work!

I think this is now complete.

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 19, 2017

cc @c1728p9

#elif defined (__ARMCC_VERSION)
const long CRP_Key __attribute__((used)) __attribute__((at(APPLICATION_ADDR+0x000002FC))) = CRP;
#elif defined (__GNUC__)
const long CRP_Key __attribute__((used)) __attribute__((section(".CRPSection"))) = CRP;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could simplify this by handling placement in the linker scripts. This could then be simplified to something like this for all the toolchains:

MBED_SECTION(".CRPSection") const long CRP_Key = CRP;

The scatter file changes would look something like this:

ER_IROM0 MBED_APP_START 0x2FC  {  ; load address = execution address
    *.o (RESET, +First)
    .ANY (+RO)
}
ER_CRP (MBED_APP_START + 0x2FC) 4)  {
    *.o (.CRPSection)
}
ER_IROM1 (MBED_APP_START + (0x2FC + 4)) (MBED_APP_SIZE - (0x2FC + 4))  {
    *(InRoot$$Sections)
    .ANY (+RO)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@c1728p9 I tried your suggestion and get the following

[ERROR] Error: L6788E: Scatter-loading of execution region ER_IROM1 to execution address [0x00010300,0x0001737c) will cause the contents of execution region ER_IROM1 at load-address [0x000100c4,0x00017140) to be corrupted at run-time.
Error: L6202E: __main.o(!!!main) cannot be assigned to non-root region 'ER_IROM1'
Error: L6202E: __scatter.o(!!!scatter) cannot be assigned to non-root region 'ER_IROM1'
Error: L6202E: __dczerorl.o(!!dczerorl) cannot be assigned to non-root region 'ER_IROM1'
Error: L6202E: __scatter_copy.o(!!handler_copy) cannot be assigned to non-root region 'ER_IROM1'
Error: L6202E: __scatter_zi.o(!!handler_zi) cannot be assigned to non-root region 'ER_IROM1'
Error: L6202E: anon$$obj.o(Region$$Table) cannot be assigned to non-root region 'ER_IROM1'
Error: L6203E: Entry point (0x00010301) lies within non-root region ER_IROM1.

Any assistance appreciated, I'm no expert on linker files!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell 'InRoot$$Sections' can only be placed in a root section, as it needs to place some ARM library stuff there. The root section here would be ER_IROM0, instead of ER_IROM1.
Moving it over might solve the issue?

Then again, I have no expert knowledge on this either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That gets rid of most errors but leaves me with

ER_IROM0 MBED_APP_START 0x2FC  {  ; load address = execution address
    *.o (RESET, +First)
    *(InRoot$$Sections)
    .ANY (+RO)
}
ER_CRP (MBED_APP_START + 0x2FC) 4  {
    *.o (.CRPSection)
}
ER_IROM1 (MBED_APP_START + (0x2FC + 4)) (MBED_APP_SIZE - (0x2FC + 4))  {
    .ANY (+RO)
}

Error: L6788E: Scatter-loading of execution region ER_IROM1 to execution address
0x00010300,0x00017d80) will cause the contents of execution region ER_IROM1 at load-address [0x000101a4,0x00017c24) to be corrupted at run-time.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears to be overwriting itself, though I can't really see how...

This occurs when scatter-loading takes place and an execution region is put in a position where it partially or wholly overwrites another execution region (which can be itself or another region).
For example, the following code generates this error:

LOAD_ROM 0x0000 0x4000 {
    EXEC1 0x4000 0x4000 {
        *(+RW,+ZI)
    }
    EXEC2 0x0000 0x4000 {
        *(+RO)
    }
}

This code does not generate the error:

LOAD_ROM 0x0000 0x4000
{
 EXEC1 0x0000 0x4000
 {
 * (+RO)
 }
 EXEC2 0x4000 0x4000
 {
 * (+RW,+ZI)
 }
}
Source: https://developer.arm.com/docs/dui0807/f/linker-errors-and-warnings/list-of-the-armlink-error-and-warning-messages

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try adding the FIXED keyword to ER_CRP. This will force it to be at the right address

ER_CRP (MBED_APP_START + 0x2FC) FIXED 4  {
    *.o (.CRPSection)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem to be related to the CRP section as removing it still causes

Error: L6788E: Scatter-loading of execution region ER_IROM1 to execution address [0x00010300,0x00017d80) will cause the contents of execution region ER_IROM1 at load-address [0x000101a4,0x00017c24) to be corrupted at run-time.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can try and use AssignExpr() to make sure ER_IROM 1's execution address cannot overlap with its load address, though I don't know if it will make a difference.

ER_IROM0 MBED_APP_START 0x2FC  {  ; load address = execution address
    *.o (RESET, +First)
    *(InRoot$$Sections)
    .ANY (+RO)
}
ER_CRP (MBED_APP_START + 0x2FC) 4  {
    *.o (.CRPSection)
}
ER_IROM1 AlignExpr(+0, 0x2000)( (MBED_APP_SIZE - (0x2FC + 4))  { ; Align it at the end of the previous section but set its execution address to 0x20000
    .ANY (+RO)
}

The code above obviously needs a better address then the hardcoded 0x20000 but it might work in your situation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try this:

ER_IROM0 MBED_APP_START 0x2FC  {  ; load address = execution address
    *.o (RESET, +First)
    .ANY (+RO)
}
ER_CRP (MBED_APP_START + 0x2FC) FIXED 4  {
    *.o (.CRPSection)
}
ER_IROM1 (MBED_APP_START + (0x2FC + 4)) FIXED (MBED_APP_SIZE - (0x2FC + 4))  {
    *(InRoot$$Sections)
    .ANY (+RO)
}

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 29, 2017

@chrissnow Have you find a way to simplify the crc placement, or staying as it is now here?

@chrissnow
Copy link
Contributor Author

@0xc0170 I'd like to simplify it but still need to sort out the scatter file issue as above.
Any suggestions welcome :-)

@chrissnow
Copy link
Contributor Author

@0xc0170 I think it's going to have to go as is unless @c1728p9 or @bramkelder can get the arm linker to work.

I appreciate it's not as elegant as it could be but I seem to be fighting a losing battle here!

@chrissnow
Copy link
Contributor Author

@0xc0170 I'm pretty sure this is now finished, Thanks @c1728p9 for figuring the linker issues out :-)

I have no idea what CircleCI is but it's error seems unrelated and common to other recent PR CI run's

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 2, 2017

/morph test

@mbed-bot
Copy link

mbed-bot commented Oct 2, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1490

All builds and test passed!

@theotherjimmy
Copy link
Contributor

@chrissnow Could you change the sha of the last commit so that it kicks out circle CI? We'll rerun testing after that.

@chrissnow
Copy link
Contributor Author

@theotherjimmy That should have done it.

@theotherjimmy
Copy link
Contributor

/morph test

@theotherjimmy
Copy link
Contributor

Thanks!

@mbed-bot
Copy link

mbed-bot commented Oct 3, 2017

Result: ABORTED

Your command has finished executing! Here's what you wrote!

/morph test

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 4, 2017

/morph test

@mbed-bot
Copy link

mbed-bot commented Oct 4, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1521

All builds and test passed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants