-
Notifications
You must be signed in to change notification settings - Fork 3k
Let libraries, targets configure bootloader #5909
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
Conversation
56e6d39
to
6327d24
Compare
@sarahmarshy @cmonr @0xc0170 Could you review? |
90398ba
to
db3e87a
Compare
Would it still work at the application level? |
Yes. |
tools/toolchains/__init__.py
Outdated
self.cc.append("-D%s=0x%x" % define) | ||
self.cppc.append("-D%s=0x%x" % define) | ||
if region.active: | ||
self.ld.append(self.make_ld_define(*define)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this still work for MBED_APP_START
in the linker scripts? It seems like MBED_APP_START_ADDR
would be the new define?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried these changes with bootloader blinky for K64F and it did not work. I could see that it only -DAPPLICATION_ADDR
and -DAPPLICATION_SIZE
were passed to the linker. The k64f linker script expects MBED_APP_START
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. Let me fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linker script defines need to be added to the profile like on master - https://github.com/ARMmbed/mbed-os/blob/master/tools/build_api.py#L301-L304. The current changes do not pass the correct defines to the linker.
@sarahmarshy The linker defines are passed to the linker, just named incorrectly. The do not need to be part of the profile, just part of the command line. |
741248d
to
ca56a49
Compare
@sarahmarshy That should do it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried with bootloader blinky, and it works now! Good work. LGTM.
now to rebase #5950 |
/morph build |
7433ca8
to
6eebc71
Compare
Rebased |
Build : FAILUREBuild number : 1027 |
@studavekar Looks like the build experienced a license timeout with the ARM compiler. |
May be issue with license server/connectivity, re-triggering it /morph build |
Build : SUCCESSBuild number : 1028 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 710 |
/morph uvisor-test |
Test : SUCCESSBuild number : 837 |
/morph uvisor-test |
1 similar comment
/morph uvisor-test |
@cmonr Poke. Looks ready to merge |
@cmonr double poke. 😄 |
@0xc0170 fine, we can poke you too! 😆 |
Jeez, can't a guy enjoy a morning taco anymore? 😆 |
Abstract
This PR lets libraries and targets configure boot loader parameters like any other parameter. This allows a release of a boot loader as an mbed library, with an
mbed_lib.json
that contains links to the boot loader binaries.Usage
The following patch, to
mbed-os-example-bootloader-blinky
, shows how this allows a standalonebootloader
directory to contain all information required to merge a boot loader.TODO