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

arm/qemu: add cortex-r5 support #15593

Merged
merged 3 commits into from
Jan 17, 2025
Merged

arm/qemu: add cortex-r5 support #15593

merged 3 commits into from
Jan 17, 2025

Conversation

yf13
Copy link
Contributor

@yf13 yf13 commented Jan 17, 2025

Summary

This adds QEMU emulated Cortex-R5 chip with virt sample board config.
Thanks @hujun260 for the discussions.

Impacts

QEMU emulated armv7 boards

Testing

  • local checks with qemu-armv7a:nsh, qemu-armv7r:nsh
  • CI checks.

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Board: arm Size: L The size of the change in this PR is large labels Jan 17, 2025
yf13 added 2 commits January 17, 2025 19:48
This revise armv7-r/ header files needed to support QEMU cortex-r5
virtual process for armv7-r family.

Signed-off-by: Yanfeng Liu <[email protected]>
This adds support for QEMU Cortex-R5 virtual processor on existing QEMU
Cortex-A7 code base with profile support in `armv7-r/` and `armv7-a/`.

Signed-off-by: Yanfeng Liu <[email protected]>
@nuttxpr
Copy link

nuttxpr commented Jan 17, 2025

[Experimental Bot, please feedback here]

No, this PR description does not fully meet the NuttX requirements. While it provides some information, it lacks crucial details and depth required for a proper review. Here's a breakdown of the missing information:

Summary:

  • Why is this change necessary? The PR states what it does, but not why. Is this for testing, supporting a new platform, or something else? What problem does it solve or functionality does it enable?
  • What functional part of the code is being changed? This is too vague. Which files/directories were modified? Mentioning specific subsystems (e.g., board support, architecture-specific code, QEMU emulation layer) would be helpful.
  • How does the change exactly work? More detail is needed. Does it introduce new drivers? Modify existing ones? What configurations are needed?

Impact:

While the "Impacts" section mentions QEMU emulated ARMv7 boards, it needs to address all the specific points required:

  • New feature? Existing feature changed? Specify explicitly. This is likely a new feature.
  • Impact on user: Does this change how users interact with NuttX? Probably not, but explicitly state "NO" if that's the case.
  • Impact on build: Will users need to do anything different to build for this new target? If so, explain. If not, explicitly state "NO".
  • Impact on hardware: This is addressed partially, but could be more precise. Does it affect any existing hardware? Likely "NO". Be clear about this.
  • Impact on documentation: Does this require updates to the NuttX documentation? If so, were those updates included in the PR? If not needed, state "NO".
  • Impact on security: Explicitly state "NO" if there are no security implications.
  • Impact on compatibility: Explicitly address backward, forward, and interoperability compatibility. Likely "NO" for backward compatibility if it's a new feature, but specify.
  • Anything else to consider: None mentioned. Even if there's nothing, explicitly state "None".

Testing:

  • Build Host(s): Insufficient detail. Provide OS, CPU architecture, compiler version (e.g., "Linux x86_64, GCC 12.2.0").
  • Target(s): More specific information is needed. "qemu-armv7a:nsh" is the configuration, not the architecture. State something like "ARMv7-A (QEMU), nsh configuration" and similarly for "qemu-armv7r:nsh" (e.g., "ARMv7-R (QEMU), nsh configuration").
  • Testing logs: The placeholder text needs to be replaced with actual logs demonstrating the functionality before and after the change. Show that the new board configuration works as expected. Just showing that the existing configurations still work isn't sufficient.

In short, the PR description needs significant expansion to meet the requirements. Provide more context, details, and specific answers to all the questions posed in the template. This will make it easier for reviewers to understand and evaluate the changes.

This adds support for qemu-armv7r board with flat mode sample
config for using with QEMU emulated Cortex-R5 processor.

Signed-off-by: Yanfeng Liu <[email protected]>
@acassis acassis merged commit e0ed715 into apache:master Jan 17, 2025
25 checks passed
#ifdef CONFIG_ARCH_ARMV7R
/* dont return per armv7-r/arm_head.S design */

nx_start();
Copy link
Contributor

Choose a reason for hiding this comment

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

can we unify armv7-a and armv7-r start code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is per this commet. If it is still needed, maybe in the future we can check if armv7-a side can take the same approach for unifying?

Copy link
Contributor

Choose a reason for hiding this comment

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

My suggestion is: either let armv7-r call nx_start in assembly code, or remove nx_start in assemly code in armv7-a arm_head.S.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see #15624

****************************************************************************/

/****************************************************************************
* Name: imx_bringup
Copy link
Contributor

Choose a reason for hiding this comment

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

qemu_bringup

Copy link
Contributor Author

@yf13 yf13 Jan 21, 2025

Choose a reason for hiding this comment

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

thanks! see #15620

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm Issues related to ARM (32-bit) architecture Board: arm Size: L The size of the change in this PR is large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants