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

Core(M): Correct Cortex-M55/M85 TPIU type and align core header #48

Merged
merged 2 commits into from
Oct 10, 2023

Conversation

ghost
Copy link

@ghost ghost commented Sep 26, 2023

No description provided.

@ghost ghost requested a review from JonatanAntoni September 26, 2023 06:48
@github-actions
Copy link

Test Results

   152 files     152 suites   0s ⏱️
     56 tests      54 ✔️        2 💤 0
8 116 runs  6 260 ✔️ 1 856 💤 0

Results for commit f4e234b.

@JonatanAntoni
Copy link
Member

@EdmundPlayer, can you comment on the identification registers?

@ghost
Copy link
Author

ghost commented Sep 26, 2023

For consistency reasons we we should either add 'Peripheral Identification Register' and 'Component Identification Register' to all Peripheral Units or remove them. It does not make sense to have them for just some peripheral units.
I decided to remove them.

@EdmundPlayer
Copy link

In the Armv7-M Architecture Reference Manual, these fall under "Optional CoreSight management and ID registers." However, in the Armv8-M Architecture Reference Manual, it looks like these registers have dedicated sections in the manual.

Although these registers could in theory be accessed by privileged software, I believe they're really intended to be accessed by a debug tool/driver via the CoreSight protocol.

I don't see any harm in adding these registers, as it could potentially be useful for people writing debug tools in C/C++. See PMU_Type in core_cm55.h, for example.

When there's already an architectural header like pmu_v8.h, it might make more sense to move these data structures into the separate header because then we won't have to keep duplicating the information across different core headers. If there is anything specific to a particular core, e.g., M85-specific PMU events, then we can still use the core header for that information.

For the TPIU, one minor change I'd make it to rename TPI to TPIU for the comments (to make TPIU a more searchable term), e.g.,

  \defgroup CMSIS_TPI     Trace Port Interface (TPI)
  \brief    Type definitions for the Trace Port Interface (TPI)

should become

  \defgroup CMSIS_TPIU     Trace Port Interface (TPIU)
  \brief    Type definitions for the Trace Port Interface (TPIU)

assuming this doesn't break anything.

@JonatanAntoni JonatanAntoni merged commit 915eb56 into main Oct 10, 2023
@JonatanAntoni JonatanAntoni deleted the CM55_85_TPIU branch October 10, 2023 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants