-
-
Notifications
You must be signed in to change notification settings - Fork 781
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
Feature: Identify STM32MP15x_CM4 coprocessor #1546
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good. There are a few items of refinement we'd like to see and a couple of mistakes need fixing, but this is excellent work.
Additional review note: Per the contribution guidelines, the |
The first commit introduces a new target driver, so I thought to write the full prefix for context. Following commits omit the Applied the review changes trying to keep logic intact, as separate small commits specific to review items. Tested with a stm32f103cb bluepill running Please let me know whether I can squash these cosmetic commits into one, and when I can fast-forward the |
It seems like after addressing some of the review items above in your working copy, you never pushed, so we've added notes where that is obvious to us. We'd like to get this merged ASAP. Regarding the structuring of the fixup commits, we'd appreciate the fixes being applied as fixups to the base commits of the PR and we'll then rebase merge the resulting commit series. For example, an initial commit that provides the probe changes, the next adding memory maps, and another couple after that implementing Flash routines (erase, then write), is what we typically aim for in this kind of scenario. |
This should look better. If something else is wrong, please submit additional review on top of updated patchset contents, preferably with (updated) line numbers. Otherwise, since the feature/style pass is complete, I am aiming to commence the rebase pass this week to get into v1.10 release.
There is only one base commit I see, the other 9 are minor edits (fixups). There is no flash in MP1. To reduce the size of one giant
As of now this patchset takes up +432 bytes on |
Your proposed patch series structure seems fine - we'll let you get the rebase done and review the result as we think that'll probably then be accepting the change set for merge. |
* STM32MP157/3/1 has Cortex-A7 core(s) and Cortex-M4 coprocessor * Detect the CM4 (when booting SoC in Engineering mode, and when CA7 starts a CM4 firmware in Production mode) * Provide default SRAM map as visible to M4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only item preventing this PR from being ready for merge, is the one on the RAM base macro naming. Once that's solved this looks ready to go and we can approve for merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, we'll get this merged after we merge the Cortex-A fix PR.
Detailed description
STM32MP157, MP153, MP151 is a MPU, microprocessor SoC with dual Cortex-A7 cores and one Cortex-M4 microcontroller/coprocessor core all connected to a lot of STM32-standard peripherals over quite a few multilayer buses. It has a proper SWJ-DP exposing JTAG and SWD capable pins. These are used for custom platform bring-up (CA7 secure bootloader chain) and for runtime-debugging coprocessor firmware.
The
stm32mp15_probe()
I implement is sufficient to work with coprocessor firmware. I also pulled a couple functions from H7/H5 target driver code to read out silicon revision and electronic signature unique ID.MP15x has a complicated SWJ Access Port setup which interferes with debugging the CA7 cores by BMD. The recommended solution by ST is to use upstream OpenOCD or their fork, and that accesses a few registers over all three APs.
MP13x doesn't have a coprocessor and is outside this PR's scope.
MP2 is not released to General Availability yet.
Engineering Boot exposes the CM4 immediately (spinning in a loop in RETRAM), but Production Boot starts up the SoC with CM4 hidden. From my experience it appears later when OpenSTLinux boots, initializes remoteproc framework and manipulates DBGMCU_CR.
We did not significantly alter TrustZone-A settings in our software platform, but take note it can manifest in problems, too. Same on sleep states -- not using any, so no problems triggered by C-Sleep/C-Stop.
Your checklist for this pull request
make PROBE_HOST=native
)make PROBE_HOST=hosted
)Closing issues