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.

Sign up for GitHub

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

Update TPI_FFCR definition #406

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Update TPI_FFCR definition #406

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08bbc85
Update TPI_FFCR definition
Kochise Aug 20, 2018
7f76d3d
Restored legacy TPI_FFCR_TrigIn macros
Kochise Sep 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions CMSIS/Core/Include/core_cm4.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1108,12 +1108,39 @@ typedef struct
#define TPI_FFSR_FlInProg_Msk (0x1UL /*<< TPI_FFSR_FlInProg_Pos*/) /*!< TPI FFSR: FlInProg Mask */

/* TPI Formatter and Flush Control Register Definitions */
#define TPI_FFCR_StopTrig_Pos 13U /*!< TPI FFCR: StopTrig Position */
#define TPI_FFCR_StopTrig_Msk (0x1UL << TPI_FFCR_StopTrig_Pos) /*!< TPI FFCR: StopTrig Mask */

#define TPI_FFCR_StopFI_Pos 12U /*!< TPI FFCR: StopFI Position */
#define TPI_FFCR_StopFI_Msk (0x1UL << TPI_FFCR_StopFI_Pos) /*!< TPI FFCR: StopFI Mask */

#define TPI_FFCR_TrigFI_Pos 10U /*!< TPI FFCR: TrigFI Position */
#define TPI_FFCR_TrigFI_Msk (0x1UL << TPI_FFCR_TrigFI_Pos) /*!< TPI FFCR: TrigFI Mask */

#define TPI_FFCR_TrigEVT_Pos 9U /*!< TPI FFCR: TrigEVT Position */
#define TPI_FFCR_TrigEVT_Msk (0x1UL << TPI_FFCR_TrigEVT_Pos) /*!< TPI FFCR: TrigEVT Mask */

#define TPI_FFCR_TrigIN_Pos 8U /*!< TPI FFCR: TrigIN Position */
#define TPI_FFCR_TrigIN_Msk (0x1UL << TPI_FFCR_TrigIN_Pos) /*!< TPI FFCR: TrigIN Mask */

#define TPI_FFCR_TrigIn_Pos 8U /*!< TPI FFCR: TrigIn Position */
Copy link
Member

Choose a reason for hiding this comment

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

We need to be careful with removing these defines. We must not break backward compatibility.
What about keeping them for a while and just mark them deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, but then why not documenting them properly to promote them to "official" status ? Are they really "deprecated" ?

Copy link
Member

Choose a reason for hiding this comment

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

Its a matter of how to keep a common style. You added #define TPI_FFCR_TrigIN_Pos 8U instead, i.e. upper case N instead of lower case one. Do you prefer to keep both? In this case we should agree on one official version. Or should we simply stay with the lower case version and drop your new one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put those in CAPITAL because it is the way they are declared in http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0337e/BABIFBCI.html table 13.8 (direct link http://infocenter.arm.com/help/topic/com.arm.doc.ddi0337e/BABIFBCI.html#BIIJEJFB doesn't work) hence I wanted to stay as close as possible to the official documentation (for what it worth, considering the numerous divergences)

Copy link
Member

Choose a reason for hiding this comment

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

That makes totally sense to me. But removing the non-capital form might break existing implementations. Thats why I think we might simply keep both for a while.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable enough to me.

Copy link
Member

Choose a reason for hiding this comment

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

May I ask you to update the PR accordingly? I'll merge it asap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done...

#define TPI_FFCR_TrigIn_Msk (0x1UL << TPI_FFCR_TrigIn_Pos) /*!< TPI FFCR: TrigIn Mask */

#define TPI_FFCR_FOnMan_Pos 6U /*!< TPI FFCR: FOnMan Position */
#define TPI_FFCR_FOnMan_Msk (0x1UL << TPI_FFCR_FOnMan_Pos) /*!< TPI FFCR: FOnMan Mask */

#define TPI_FFCR_FOnTrig_Pos 5U /*!< TPI FFCR: FOnTrig Position */
#define TPI_FFCR_FOnTrig_Msk (0x1UL << TPI_FFCR_FOnTrig_Pos) /*!< TPI FFCR: FOnTrig Mask */

#define TPI_FFCR_FOnFlln_Pos 4U /*!< TPI FFCR: FOnFlln Position */
#define TPI_FFCR_FOnFlln_Msk (0x1UL << TPI_FFCR_FOnFlln_Pos) /*!< TPI FFCR: FOnFlln Mask */

#define TPI_FFCR_EnFCont_Pos 1U /*!< TPI FFCR: EnFCont Position */
#define TPI_FFCR_EnFCont_Msk (0x1UL << TPI_FFCR_EnFCont_Pos) /*!< TPI FFCR: EnFCont Mask */

#define TPI_FFCR_EnFTC_Pos 0U /*!< TPI FFCR: EnFTC Position */
#define TPI_FFCR_EnFTC_Msk (0x1UL << TPI_FFCR_EnFTC_Pos) /*!< TPI FFCR: EnFTC Mask */

/* TPI TRIGGER Register Definitions */
#define TPI_TRIGGER_TRIGGER_Pos 0U /*!< TPI TRIGGER: TRIGGER Position */
#define TPI_TRIGGER_TRIGGER_Msk (0x1UL /*<< TPI_TRIGGER_TRIGGER_Pos*/) /*!< TPI TRIGGER: TRIGGER Mask */
Expand Down