-
-
Notifications
You must be signed in to change notification settings - Fork 396
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: Use AP specific types & enumerate Addresses rather than APs #2683
Conversation
7961632
to
693b4b0
Compare
The following changelog fragments do not match the expected pattern:
Files should start with one of the categories followed by a dash, and end with '.md' Valid categories are:
|
4571ff7
to
e829bff
Compare
c7b6def
to
c33729a
Compare
@bugadani I’m sorry the last commit is bigger, I didn’t really now how to detangle the various parts. |
changelog/changed-factorize-MemoryInterface-slight-variations.md
Outdated
Show resolved
Hide resolved
c33729a
to
dcee752
Compare
There are a few things that I’m not fully satisfied in this PR but I had to stop somewhere :P
There are more refactor coming with the introduction of APv2 & ADIv6 (mostly moving some modules around to make place for an "apv2" module. |
dcee752
to
2a5d786
Compare
00b02b0
to
74233de
Compare
NGL this is very hard to review. I don't have experience with the code, and even though I assume most of this is just pushing stuff around, I can't track what goes where. The size of the whole diff makes my computer cry, and the online vscode is just not optimal to read diffs either :) Because we squash PRs into single commits, I would recommend to split out the independent parts into separate PRs. That would make them easier to review and approve, as well as change tracking/blaming/reverting independent parts becomes about an order of magnitude easier. I would recommend submitting the initial refactors in one go, the bugfix in a second phase, and the last commit's madness last. As you said the last commit is hard to split up, I guess I'll have to bite the bullet there, but it will take time and it would be unfortunate if the PR gathered conflicts in the meantime - merging the rest should somewhat reduce the diff surface to help with that. |
74233de
to
bac99d0
Compare
I openned the following PRs:
and converted this one back to draft, hopefully I can clarify this madness. |
94a6415
to
3685203
Compare
3685203
to
21ef060
Compare
1eea56d
to
a7b8185
Compare
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.
I'm not done reviewing yet, completing this will take time.
a7b8185
to
f9ea724
Compare
f9ea724
to
f57c24c
Compare
8b99a1e
to
083c6b1
Compare
- Enumerate Memory AP addresses instead of giving the number of valid (Generic and Memory) APs. This will enable future development where APv2’s live in disjoint memory spaces. - Use AP specific types This should allow future developments to enabled things like secure debug, memory tagging, etc - Drop `ApInformation` and `MemoryApInformation` The AP specific types carry these information internally so these types are no longer required.
083c6b1
to
93fe5ed
Compare
@Tiwalun should I leave this PR unmerged for a short while, or you'll only have comments after 5 minutes after it gets merged? 🫠 |
I think my interpretation of a short while is different than yours 😄 Feel free to merge this, I don't think I have time to look at this before the weekend. |
(Generic and Memory) APs.
This will enable future development where APv2’s live in disjoint memory
spaces.
This should allow future developments to enabled things like secure
debug, memory tagging, etc
ApInformation
andMemoryApInformation
The AP specific types carry these information internally so these types
are no longer required.
Follows: