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

Add standard upload method variables #369

Merged
merged 6 commits into from
Oct 8, 2024

Conversation

multiplemonomials
Copy link
Collaborator

@multiplemonomials multiplemonomials commented Oct 4, 2024

Summary of changes

This PR fixes some deficiencies in the upload method system which are starting to become more of a problem in the newest version of Mbed.

One issue has been around for a while: if you want to set the serial number of the device to program, it requires a different way to do that for each upload method: PYOCD_PROBE_UID, STLINK_PROBE_SN, etc. All of these are set to the same "type" of thing (the USB serial number of the probe to use), and yet the name is different for each upload method you use. This is silly, and these variables should be unified into one common parameter to set. This change does that.

The other issue is more recent. @ccli8 's recent PRs have switched many tools from using the elf file to the bin/hex file to upload, and this is good because it's required for TFM targets and targets with post build hooks that modify the image to work. However, when uploading a bin file, tools need to be told what address to load it at. This information hasn't existed in Mbed until recently, and it was not being provided to the upload methods at all.

This actually caused a bug I noticed in testing, where flashing code using OpenOCD does not work (just silently left the old version of the code on there) because it defaults to loading to address 0x0, and that is not where some targets put their flash image.

To fix these issue, I've added two new "common" upload method options: MBED_UPLOAD_SERIAL_NUMBER and MBED_UPLOAD_BASE_ADDR. The former is intended to be set by the user, and defaults to empty though it will pick up the value from the legacy options it replaces if any of those are set. The latter is initialized based on the configured first flash bank start address (MBED_CONFIGURED_ROM_START) but can also be set to a custom address in the user's upload method configuration. I also made MBED_GDB_PORT into a proper option if you want to change the port that the GDB server runs on.

Impact of changes

  • Upload method UID/serial number arguments condensed into one common one, MBED_UPLOAD_SERIAL_NUMBER. However, legacy variables are still supported and will print a warning if used.
  • New option to specify where a bin file is loaded to in the target's memory, MBED_UPLOAD_BASE_ADDR
  • Fixed bug where some upload methods would always flash the bin/hex file to address 0, even for devices (e.g. STM32 micros) where that is not a valid flash location
  • Improved error message if you have an old build directory from before the memory bank information change landed that needs to be reconfigured

Migration actions required

Documentation

Will update the upload method documentation once this is ready to be merged


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR

So far I have verified this with st-flash, OpenOCD, pyocd, and LinkServer, and code can be flashed and runs OK.

@multiplemonomials
Copy link
Collaborator Author

@JohnK1987 @ccli8 could one of you review and approve? Thx!

Copy link

@ccli8 ccli8 left a comment

Choose a reason for hiding this comment

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

Except confused on MBED_UPLOAD_BASE_ADDR used with hex file, no other concern.

@multiplemonomials multiplemonomials merged commit 11369b4 into master Oct 8, 2024
52 checks passed
@multiplemonomials multiplemonomials deleted the dev/standard-upload-method-variables branch October 8, 2024 16:14
@multiplemonomials
Copy link
Collaborator Author

Updated the wiki page here with docs: https://github.com/mbed-ce/mbed-os/wiki/Upload-Methods. Made sure to add a warning about the base address option when hex files are used.

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

Successfully merging this pull request may close these issues.

3 participants