Skip to content

STM32 baremetal support step1 (F0/F1/F3/H7/L0) #12992

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 4, 2020
Merged

STM32 baremetal support step1 (F0/F1/F3/H7/L0) #12992

merged 5 commits into from
Jun 4, 2020

Conversation

jeromecoutant
Copy link
Collaborator

Summary of changes

Major patches are linker script updates to support baremetal.

@MarceloSalazar @evedon @0xc0170
@ARMmbed/team-st-mcd

Impact of changes

Baremetal tests are OK:
mbed test xxx --test-config tests/configs/baremetal.json

Migration actions required

Documentation

Pull request type 

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] 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)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@ciarmcom ciarmcom requested a review from a team May 18, 2020 17:00
@ciarmcom
Copy link
Member

@jeromecoutant, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@evedon evedon requested a review from hugueskamba May 19, 2020 08:26
hugueskamba
hugueskamba reviewed May 19, 2020
View reviewed changes
targets/targets.json
@@ -1358,6 +1358,9 @@
"macro_name": "CLOCK_SOURCE"
}
},
"overrides": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this necessary given this PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes as #12988 is not merged

Copy link
Contributor

Choose a reason for hiding this comment

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

should it be as dependency and this updated once it is merged?

targets/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/device/cmsis_nvic.h
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this header be included in the appropriate linker files?

targets/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/device/TOOLCHAIN_GCC_ARM/STM32F070XB.ld
#endif

StackSize = MBED_BOOT_STACK_SIZE;
/* Round up VECTORS_SIZE to 8 bytes */
#define NVIC_NUM_VECTORS 48
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is already defined in cmsis_nvic.h.

targets/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/device/TOOLCHAIN_ARM/stm32f070xb.sct
#endif

#define Stack_Size MBED_BOOT_STACK_SIZE
/* Round up VECTORS_SIZE to 8 bytes */
#define NVIC_NUM_VECTORS 48
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is already defined in cmsis_nvic.h.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but cmsis_nvic.h for M0 core is not so simple as other cores.
So redefining value here is a good workaround.
I ill make it better later

Copy link
Contributor

Choose a reason for hiding this comment

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

I dont understand, we can't get the value from the header file to be in here ? Or at least to check if its not already defined ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we can, but no with a "quick" patch.
So I used the method described in https://os.mbed.com/docs/mbed-os/v5.15/porting/porting-bootstrap.html

Copy link
Contributor

Choose a reason for hiding this comment

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

why is it also in cmsis_nvic.h ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because mbed needs it...

Copy link
Contributor

Choose a reason for hiding this comment

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

True. Linker script should do #include "../cmsis_nvic.h" and not define it again. This is a bug in our docs.

@hugueskamba can you create an issue if you agree? the docs should be updated. The default place in our targets is cmsis_nvic.h file, and linker script should include it (or we need to change this and refactor most of our targets).

This was referenced May 20, 2020
Merged
Merged
@jeromecoutant
Copy link
Collaborator Author

Hi @0xc0170 and @MarceloSalazar
I would like to add more update in each STM32 family.
Do you plan to merge this PR soon (then I am working on top of this one),
or can I update this one, or ...?
Thx

@0xc0170
Copy link
Contributor

0xc0170 commented May 25, 2020

@jeromecoutant I am reviewing now. Just add it here, we allow only fixes for rc2 until we release.

@jeromecoutant
Copy link
Collaborator Author

"baremetal support" is not considered as fix ?

@0xc0170
Copy link
Contributor

0xc0170 commented May 25, 2020

"baremetal support" is not considered as fix ?

To clarifyr, only fixes that are found during OOB we are currently doing, exceptions could be made.

0xc0170
0xc0170 approved these changes May 25, 2020
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

With a note about possible doc update

@mergify mergify bot added needs: CI and removed needs: review labels May 25, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented May 25, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented May 25, 2020

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 1
Build artifacts

@jeromecoutant
Copy link
Collaborator Author

mergify bot added ready for merge label

good :-)

@0xc0170 0xc0170 added the release-type: patch Indentifies a PR as containing just a patch label Jun 4, 2020
@0xc0170 0xc0170 merged commit ed0cadf into ARMmbed:master Jun 4, 2020
@mergify mergify bot removed the ready for merge label Jun 4, 2020
@mergify
Copy link

mergify bot commented Jun 4, 2020

This PR does not contain release version label after merging.

@mergify mergify bot added the release version missing When PR does not contain release version, bot should label it and we fix it afterwards label Jun 4, 2020
@jeromecoutant jeromecoutant deleted the PR_BAREMETAL_SUPPORT_STEP1 branch June 5, 2020 14:52
@jeromecoutant jeromecoutant mentioned this pull request Jun 9, 2020
Merged
@adbridge adbridge added release-version: 6.1.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Jun 24, 2020
@0xc0170 0xc0170 removed the release version missing When PR does not contain release version, bot should label it and we fix it afterwards label Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugueskamba hugueskamba hugueskamba left review comments

@0xc0170 0xc0170 0xc0170 approved these changes

Assignees
No one assigned
Labels
release-version: 6.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jeromecoutant @ciarmcom @0xc0170 @mbed-ci @hugueskamba @adbridge