Skip to content

Add 'volatile' to all cmsdk device registers #14

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

Merged
merged 1 commit into from
Oct 2, 2025
Merged

Add 'volatile' to all cmsdk device registers #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 7 additions & 6 deletions api-tests/platform/drivers/uart/cmsdk/pal_uart.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,18 @@
#define CMSDK_UART_INTSTATUS_TXIRQ_Pos 0 /* CMSDK_UART STATUS: TXIRQ Position */
#define CMSDK_UART_INTSTATUS_TXIRQ_Msk (0x01ul << CMSDK_UART_INTSTATUS_TXIRQ_Pos)
/* CMSDK_UART STATUS: TXIRQ Mask */
typedef volatile uint32_t vuint32_t;
Copy link

@tejlmand tejlmand Sep 30, 2025

Choose a reason for hiding this comment

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

why are we typedef'ing this ?

When I see code and read: volatile uint32_t then I immediately know what this means / does.

But reading vuint32_t I might need to make a guess / assumption or go find the origin.

I don't see a value in this particular case.

Copy link
Author

@keith-packard keith-packard Sep 30, 2025

Choose a reason for hiding this comment

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

sorry, it's a convention I brought from other embedded projects I've done where the typedef is global and applied uniformly throughout the codebase. I used it here to avoid having the indentation of these files get messed up which would have made the patch harder to review, but I agree it doesn't really improve the readability of there resulting code.


/* typedef's */
typedef struct {
uint32_t DATA; /* Offset: 0x000 (R/W) Data Register */
uint32_t STATE; /* Offset: 0x004 (R/W) Status state */
uint32_t CTRL; /* Offset: 0x008 (R/W) Control Register */
vuint32_t DATA; /* Offset: 0x000 (R/W) Data Register */
vuint32_t STATE; /* Offset: 0x004 (R/W) Status state */
vuint32_t CTRL; /* Offset: 0x008 (R/W) Control Register */
union {
uint32_t INTSTATUS; /* Offset: 0x00C (R/ ) Interrupt Status Register */
uint32_t INTCLEAR; /* Offset: 0x00C ( /W) Interrupt Clear Register */
vuint32_t INTSTATUS; /* Offset: 0x00C (R/ ) Interrupt Status Register */
vuint32_t INTCLEAR; /* Offset: 0x00C ( /W) Interrupt Clear Register */
};
uint32_t BAUDDIV; /* Offset: 0x010 (R/W) Baud rate divider */
vuint32_t BAUDDIV; /* Offset: 0x010 (R/W) Baud rate divider */
} uart_t;


Expand Down
24 changes: 13 additions & 11 deletions api-tests/platform/drivers/watchdog/cmsdk/pal_wd_cmsdk.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,20 @@

#define WDOG_TIMER_MAX_VALUE 0xFFFFFFFF

typedef volatile uint32_t vuint32_t;

typedef struct {
uint32_t LOAD; /* Offset: 0x000 (R/W) Watchdog Load Register */
uint32_t VALUE; /* Offset: 0x004 (R/ ) Watchdog Value Register */
uint32_t CTRL; /* Offset: 0x008 (R/W) Watchdog Control Register */
uint32_t INTCLR; /* Offset: 0x00C ( /W) Watchdog Clear Interrupt Register */
uint32_t RAWINTSTAT; /* Offset: 0x010 (R/ ) Watchdog Raw Interrupt Status Register */
uint32_t MASKINTSTAT; /* Offset: 0x014 (R/ ) Watchdog Interrupt Status Register */
uint32_t RESERVED0[762];
uint32_t LOCK; /* Offset: 0xC00 (R/W) Watchdog Lock Register */
uint32_t RESERVED1[191];
uint32_t ITCR; /* Offset: 0xF00 (R/W) Watchdog Integration Test Control Register */
uint32_t ITOP; /* Offset: 0xF04 ( /W) Watchdog Integration Test Output Set Register */
vuint32_t LOAD; /* Offset: 0x000 (R/W) Watchdog Load Register */
vuint32_t VALUE; /* Offset: 0x004 (R/ ) Watchdog Value Register */
vuint32_t CTRL; /* Offset: 0x008 (R/W) Watchdog Control Register */
vuint32_t INTCLR; /* Offset: 0x00C ( /W) Watchdog Clear Interrupt Register */
vuint32_t RAWINTSTAT; /* Offset: 0x010 (R/ ) Watchdog Raw Interrupt Status Register */
vuint32_t MASKINTSTAT; /* Offset: 0x014 (R/ ) Watchdog Interrupt Status Register */
vuint32_t RESERVED0[762];
vuint32_t LOCK; /* Offset: 0xC00 (R/W) Watchdog Lock Register */
vuint32_t RESERVED1[191];
vuint32_t ITCR; /* Offset: 0xF00 (R/W) Watchdog Integration Test Control Register */
vuint32_t ITOP; /* Offset: 0xF04 ( /W) Watchdog Integration Test Output Set Register */
} wd_timer_t;

/* WATCHDOG LOAD Register Definitions */
Expand Down