anchao commented on code in PR #14865: URL: https://github.com/apache/nuttx/pull/14865#discussion_r1855653271
########## arch/arm/src/dm320/dm320_decodeirq.c: ########## @@ -109,11 +102,9 @@ uint32_t *arm_decodeirq(uint32_t *regs) } #endif - /* Set current_regs to NULL to indicate that we are no longer in - * an interrupt handler. - */ + /* Set irq flag */ - up_set_current_regs(NULL); + arm_set_irq_flag(false); Review Comment: why not keep the orignal style? Will `bool` have better performance than `uint32_t`? ########## arch/arm/include/arm/irq.h: ########## @@ -239,40 +231,17 @@ static inline irqstate_t up_irq_enable(void) int up_cpu_index(void) noinstrument_function; #endif /* CONFIG_ARCH_HAVE_MULTICPU */ -noinstrument_function -static inline_function uint32_t *up_current_regs(void) -{ -#ifdef CONFIG_SMP - return (uint32_t *)g_current_regs[up_cpu_index()]; -#else - return (uint32_t *)g_current_regs[0]; -#endif -} - -noinstrument_function -static inline_function void up_set_current_regs(uint32_t *regs) -{ -#ifdef CONFIG_SMP - g_current_regs[up_cpu_index()] = regs; -#else - g_current_regs[0] = regs; -#endif -} - noinstrument_function static inline_function bool up_interrupt_context(void) { #ifdef CONFIG_SMP irqstate_t flags = up_irq_save(); -#endif - - bool ret = up_current_regs() != NULL; - -#ifdef CONFIG_SMP + bool ret = g_interrupt_context[up_cpu_index()]; Review Comment: ```suggestion bool ret = g_interrupt_context[this_cpu()]; ``` replace all up_cpu_index() to this_cpu() ########## arch/arm/src/dm320/dm320_decodeirq.c: ########## @@ -45,7 +45,8 @@ uint32_t *arm_decodeirq(uint32_t *regs) struct tcb_s *tcb = this_task(); #ifdef CONFIG_SUPPRESS_INTERRUPTS - up_set_current_regs(regs); + tcb->xcp.regs = regs; + arm_set_irq_flag(true); Review Comment: I prefer the previous naming or up_set_interrupt_context() to arm_set_irq_flag() ########## arch/arm/src/common/arm_internal.h: ########## @@ -427,6 +439,18 @@ void arm_prefetchabort(uint32_t *regs); uint32_t *arm_syscall(uint32_t *regs); void arm_undefinedinsn(uint32_t *regs); +/* IRQ Flag */ + +noinstrument_function +static inline_function void arm_set_irq_flag(bool flag) Review Comment: ```suggestion static inline_function void up_set_interrupt_context(bool flag) ``` get/set g_interrupt_context should be pair. -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org