Skip to content

kinetis/ldscript: handle _rom_offset#11630

Merged
cladmi merged 5 commits intoRIOT-OS:masterfrom
fjmolinas:pr_kinetis_ld
Jun 5, 2019
Merged

kinetis/ldscript: handle _rom_offset#11630
cladmi merged 5 commits intoRIOT-OS:masterfrom
fjmolinas:pr_kinetis_ld

Conversation

@fjmolinas
Copy link
Copy Markdown
Contributor

@fjmolinas fjmolinas commented Jun 4, 2019 •
edited by cladmi
Loading

Contribution description

This PR includes ROM_OFFSET handling for kinetis boards. This is a required step to be able to flash and link at an offset so riotboot can be used (among others). With this PR tests/cortexm_common_ldscript pases on kinetis platforms and is therefor whitelisted.

Testing procedure

Run tests/cortexm_common_ldscript on kinetis based boards:

BOARD=frdm-kw41z make -C tests/cortexm_common_ldscript/

Run tests/cortexm_common_ldscript on boards that where already whtelisted (CI should check this anyway):

BOARD=samr21-xpro make -C tests/cortexm_common_ldscript/

Issues/PRs references

#11562 depends on it.

@fjmolinas fjmolinas added Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Area: OTA Area: Over-the-air updates labels Jun 4, 2019
@fjmolinas fjmolinas requested a review from cladmi June 4, 2019 16:01
@fjmolinas fjmolinas mentioned this pull request Jun 4, 2019
Merged
3 tasks
@fjmolinas fjmolinas removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Jun 4, 2019
fjmolinas added 3 commits June 4, 2019 18:05
@fjmolinas
cortexm_common/ldscript: add common script for rom_offset calculation
1e5a485
@fjmolinas
cortexm_common/ldscript: use cortexm_rom_offset.ld
43182bd
@fjmolinas
kinetis/ldscript: include _rom_offset
f0311ce 
- To be able to flash at an offset the vector table must be
  relocated accordingly to the IMAGE_OFFSET, therefore linkage
  needs to take the offset into account.
@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Jun 4, 2019
fjmolinas added 2 commits June 4, 2019 18:11
@fjmolinas
tests/cortexm_common_ld_script: add kinetis to whitelist
985ee95
@fjmolinas
kinetis/Makefile.include: cleanup unused LINKFLAGS
1e0f44c 
- _rom_start_addr, _ram_start_addr, _rom_length and _ran_length are
  already defined in cortexm_common/Makefile.included and can therefore
  be removed from kinetis/Makefile.include
- _ram_base_addr is never used and was not in commit history so
  is also removed
@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Jun 4, 2019
@cladmi
Copy link
Copy Markdown
Contributor

cladmi commented Jun 5, 2019

Edited the description, APP_VER is not required for testing.

@cladmi
Copy link
Copy Markdown
Contributor

cladmi commented Jun 5, 2019

Note also, the commit order on github is not the real one. It is needed to check the local one for reviewing.

cladmi
cladmi approved these changes Jun 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@cladmi cladmi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ACK ran the described tests (also ran by CI as they are compile time tests).
I agree with the changes and justification.

I could still use tests/riotboot on iotlab-m3 and samr21-xpro.
And tests/riotboot indeed work with #11562 on frdm-kw41z and frdm-k64f.

Note on the kinetis.ld with an offset the .fcfield will not be at 0x400 so generating it and the linker scripts checks are useless. However I agree with the current handling to not use a different linker script when using an offset even if the check and .fcfield is not needed there.

@cladmi cladmi merged commit bbb6dec into RIOT-OS:master Jun 5, 2019
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Jun 5, 2019

@cladmi Thanks for the Review!

@fjmolinas fjmolinas deleted the pr_kinetis_ld branch August 7, 2019 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@cladmi cladmi cladmi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area: build system Area: Build system Area: OTA Area: Over-the-air updates CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fjmolinas @cladmi