Skip to content

arch: arm: clean up inclusions in assembly files #19709

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

ioannisg
Copy link
Member

@ioannisg ioannisg commented Oct 9, 2019

A clean-up commit that removes unnecessary inclusions from
assembly files in arm/core and arm/core/cortex_m.

Signed-off-by: Ioannis Glaropoulos [email protected]

No functional changes.

A second commit enhances the file descriptions under /arm/core, to stress that these are common source for -M and -R CPUs

@ioannisg ioannisg added the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Oct 9, 2019
@ioannisg ioannisg added this to the v2.1.0 milestone Oct 9, 2019
@ioannisg ioannisg force-pushed the arch_arm_cortex_m_clean_up_inclusions branch from 3c7d59f to d3bed7c Compare October 9, 2019 08:19
@ioannisg ioannisg added area: ARM ARM (32-bit) Architecture Enhancement Changes/Updates/Additions to existing features labels Oct 9, 2019
@ioannisg ioannisg self-assigned this Oct 9, 2019
@ioannisg ioannisg requested review from wentongwu and agansari October 9, 2019 09:09
@ioannisg
Copy link
Member Author

ioannisg commented Oct 9, 2019

@stephanosio can you review this?

Copy link
Member Author

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

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

@stephanosio thanks again for the review.
So, I did another iteration and I am leaning towards the following order for inclusions in ASM:

  • never include kernel_structs.h
  • include always toolchain.h and linker/sections.h
  • include offsets-short.h, if ASM accesses "offset" constants
  • include arch/cpu.h, if ASM accesses CMSIS constants (defined locally in include/arch/arm)
  • include file-specific headers, if ASM needs them (e.g. vector-table.h)

@ioannisg ioannisg force-pushed the arch_arm_cortex_m_clean_up_inclusions branch 2 times, most recently from 1fda513 to 7d35fb2 Compare October 9, 2019 13:12
Copy link
Member Author

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

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

@stephanosio could you take a look once more?

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

Functionally, it looks good to me.

On a side note, not to be pedantic or anything but z_NanoFatalErrorHandler in the comment seems to have been phased out.

@ioannisg
Copy link
Member Author

ioannisg commented Oct 9, 2019

Functionally, it looks good to me.

On a side note, not to be pedantic or anything but z_NanoFatalErrorHandler in the comment seems to have been phased out.

This needs to be updated, good catch. I'll add it to the change.

A clean-up commit that removes unnecessary inclusions from
assembly files in arm/core and arm/core/cortex_m. It also
ogranizes the inclusions based on the following order and
set of rules:
- never include kernel_structs.h
- include toolchain.h and linker/sections.h in all ASM files
- include offsets-short.h, if ASM accesses offset constants
- include arch/cpu.h, if ASM accesses CMSIS constants
  (defined locally in include/arch/arm)
- include file-specific headers, if needed (e.g. vector-table.h)

Signed-off-by: Ioannis Glaropoulos <[email protected]>
arch/arm/core is shared between Cortex-M and Cortex-R, so
enhance the file description headers accordingly.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg ioannisg force-pushed the arch_arm_cortex_m_clean_up_inclusions branch from 7d35fb2 to ee1e268 Compare October 9, 2019 14:50
@MaureenHelm MaureenHelm merged commit 61c60be into zephyrproject-rtos:master Oct 9, 2019
@wentongwu
Copy link
Collaborator

@ioannisg is there any supporting Zephyr’s board based on Cortex-R? Thanks

@stephanosio
Copy link
Member

is there any supporting Zephyr’s board based on Cortex-R? Thanks

@wentongwu I'm currently working on TI Hercules support, which includes support for LAUNCHXL2-RM46 (see #19644 for more info). It only boots hello_world at this time, but it will soon have more supported features.

@wentongwu
Copy link
Collaborator

is there any supporting Zephyr’s board based on Cortex-R? Thanks

@wentongwu I'm currently working on TI Hercules support, which includes support for LAUNCHXL2-RM46 (see #19644 for more info). It only boots hello_world at this time, but it will soon have more supported features.

@stephanosio Thanks for the info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture Enhancement Changes/Updates/Additions to existing features Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants