xiaoxiang781216 commented on code in PR #13529:
URL: https://github.com/apache/nuttx/pull/13529#discussion_r1766247275
##########
arch/arm/include/armv7-a/irq.h:
##########
@@ -457,11 +460,8 @@ static inline_function int up_cpu_index(void)
/* Read the Multiprocessor Affinity Register (MPIDR) */
- __asm__ __volatile__
- (
- "mrc " "p15, " "0" ", %0, " "c0" ", " "c0" ", " "5" "\n"
- : "=r"(mpidr)
- );
+ ARM_ISB();
Review Comment:
why add barrier for readonly register
##########
arch/arm/include/armv7-a/irq.h:
##########
@@ -36,6 +36,9 @@
# include <stdint.h>
#endif
+#include "cp15.h"
Review Comment:
```suggestion
#include <arch/cp15.h>
```
##########
arch/arm/src/Makefile:
##########
@@ -37,11 +37,12 @@ else # ARM9, ARM7TDMI,
etc.
ARCH_SUBDIR = arm
endif
-ARCH_SRCDIR = $(TOPDIR)$(DELIM)arch$(DELIM)$(CONFIG_ARCH)$(DELIM)src
+ARCH_SRCDIR = $(TOPDIR)$(DELIM)arch$(DELIM)$(CONFIG_ARCH)
Review Comment:
how about cmake
##########
arch/arm/include/armv7-a/irq.h:
##########
@@ -36,6 +36,9 @@
# include <stdint.h>
#endif
+#include "cp15.h"
+#include "barriers.h"
Review Comment:
ditto
##########
arch/arm/src/Makefile:
##########
@@ -37,11 +37,12 @@ else # ARM9, ARM7TDMI,
etc.
ARCH_SUBDIR = arm
endif
-ARCH_SRCDIR = $(TOPDIR)$(DELIM)arch$(DELIM)$(CONFIG_ARCH)$(DELIM)src
+ARCH_SRCDIR = $(TOPDIR)$(DELIM)arch$(DELIM)$(CONFIG_ARCH)
-INCLUDES += ${INCDIR_PREFIX}$(ARCH_SRCDIR)$(DELIM)chip
-INCLUDES += ${INCDIR_PREFIX}$(ARCH_SRCDIR)$(DELIM)common
-INCLUDES += ${INCDIR_PREFIX}$(ARCH_SRCDIR)$(DELIM)$(ARCH_SUBDIR)
+INCLUDES += ${INCDIR_PREFIX}$(ARCH_SRCDIR)$(DELIM)src$(DELIM)chip
+INCLUDES += ${INCDIR_PREFIX}$(ARCH_SRCDIR)$(DELIM)src$(DELIM)common
+INCLUDES += ${INCDIR_PREFIX}$(ARCH_SRCDIR)$(DELIM)src$(DELIM)$(ARCH_SUBDIR)
+INCLUDES += ${INCDIR_PREFIX}$(ARCH_SRCDIR)$(DELIM)include$(DELIM)$(ARCH_SUBDIR)
Review Comment:
why need change
##########
arch/arm/include/armv7-a/irq.h:
##########
@@ -500,23 +500,15 @@ static inline_function uint32_t up_getsp(void)
noinstrument_function
static inline_function uint32_t *up_current_regs(void)
{
- uint32_t *regs;
- __asm__ __volatile__
- (
- "mrc " "p15, " "0" ", %0, " "c13" ", " "c0" ", " "4" "\n"
- : "=r"(regs)
- );
- return regs;
+ ARM_ISB();
+ return (uint32_t *)CP15_GET(TPIDRPRW);
}
noinstrument_function
static inline_function void up_set_current_regs(uint32_t *regs)
{
- __asm__ __volatile__
- (
- "mcr " "p15, " "0" ", %0, " "c13" ", " "c0" ", " "4" "\n"
- :: "r"(regs)
- );
+ CP15_SET(TPIDRPRW, regs);
+ ARM_ISB();
Review Comment:
ditto
##########
arch/arm/include/armv7-a/irq.h:
##########
@@ -500,23 +500,15 @@ static inline_function uint32_t up_getsp(void)
noinstrument_function
static inline_function uint32_t *up_current_regs(void)
{
- uint32_t *regs;
- __asm__ __volatile__
- (
- "mrc " "p15, " "0" ", %0, " "c13" ", " "c0" ", " "4" "\n"
- : "=r"(regs)
- );
- return regs;
+ ARM_ISB();
Review Comment:
could you point out which document require to add barrier for TPIDRPRW read?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
