From 9f052bc500c8e075d02c22e1d38f0b326d12e9e8 Mon Sep 17 00:00:00 2001 From: Anna Bridge Date: Wed, 8 Jun 2016 18:41:19 +0100 Subject: [PATCH 1/3] Update function get_interrupts_disabled() to return a bool value rather than an integer. This removes the need for all users to mask the returned value with 0x1 to determine interrupt status. Expose this function externally to allow other users to check interrupt status in a manner which will work for both cortex-A and cortex-M . Usage: bool disabled = get_interrupts_disabled(); --- hal/api/critical.h | 12 ++++++++++++ hal/common/critical.c | 18 +++++++++--------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/hal/api/critical.h b/hal/api/critical.h index cca96fa9746..89e0cf20d80 100644 --- a/hal/api/critical.h +++ b/hal/api/critical.h @@ -24,6 +24,18 @@ extern "C" { #endif + +/** Determine the current interrupts disabled state + * + * This function can be called to determine whether or not interrupts are currently disabled. + * \note + * NOTE: + * This function works for both cortex-A and cortex-M, although the underlyng implementation + * differs. + * @return true if interrupts are disabled, false otherwise + */ +bool get_interrupts_disabled(void); + /** Mark the start of a critical section * * This function should be called to mark the start of a critical section of code. diff --git a/hal/common/critical.c b/hal/common/critical.c index ec5e30e77e6..6dafe8f6388 100644 --- a/hal/common/critical.c +++ b/hal/common/critical.c @@ -27,26 +27,26 @@ #define EXCLUSIVE_ACCESS (!defined (__CORTEX_M0) && !defined (__CORTEX_M0PLUS)) static volatile uint32_t interrupt_enable_counter = 0; -static volatile uint32_t critical_interrupts_disabled = 0; +static volatile bool critical_interrupts_disabled = false; -static inline uint32_t get_interrupts_disabled(void) +bool get_interrupts_disabled(void) { #if defined(__CORTEX_A9) - uint32_t interrupts_disabled = (__get_CPSR() & 0x80) >> 7; + bool interrupts_disabled = (bool)(((__get_CPSR() & 0x80) >> 7) & 0x1); #else - uint32_t interrupts_disabled = __get_PRIMASK(); + bool interrupts_disabled = (bool)(__get_PRIMASK() & 0x1); #endif return interrupts_disabled; } void core_util_critical_section_enter() { - uint32_t interrupts_disabled = get_interrupts_disabled(); + bool interrupts_disabled = get_interrupts_disabled(); __disable_irq(); /* Save the interrupt disabled state as it was prior to any nested critical section lock use */ if (!interrupt_enable_counter) { - critical_interrupts_disabled = interrupts_disabled & 0x1; + critical_interrupts_disabled = interrupts_disabled; } /* If the interrupt_enable_counter overflows or we are in a nested critical section and interrupts @@ -56,7 +56,7 @@ void core_util_critical_section_enter() // FIXME #ifndef FEATURE_UVISOR if (interrupt_enable_counter > 0) { - MBED_ASSERT(interrupts_disabled & 0x1); + MBED_ASSERT(interrupts_disabled); } #else #warning "core_util_critical_section_enter needs fixing to work from unprivileged code" @@ -71,9 +71,9 @@ void core_util_critical_section_exit() // FIXME #ifndef FEATURE_UVISOR - uint32_t interrupts_disabled = get_interrupts_disabled(); /* get the current interrupt disabled state */ + bool interrupts_disabled = get_interrupts_disabled(); /* get the current interrupt disabled state */ - MBED_ASSERT(interrupts_disabled & 0x1); /* Interrupts must be disabled on invoking an exit from a critical section */ + MBED_ASSERT(interrupts_disabled); /* Interrupts must be disabled on invoking an exit from a critical section */ #else #warning "core_util_critical_section_exit needs fixing to work from unprivileged code" #endif /* FEATURE_UVISOR */ From 2292794385640b17e4c7bbdd09c4b8975286d638 Mon Sep 17 00:00:00 2001 From: Anna Bridge Date: Thu, 9 Jun 2016 16:41:58 +0100 Subject: [PATCH 2/3] Reverse the logic to get_interrupts_disabled() to are_interrupts_enabled() and update the using functions accordingly. Usage: bool interrupts_enabled = are_interrupts_enabled() Remove superfluos shift in are_interrupts_enabled(). --- hal/api/critical.h | 8 ++++---- hal/common/critical.c | 11 +++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/hal/api/critical.h b/hal/api/critical.h index 89e0cf20d80..464ce10e976 100644 --- a/hal/api/critical.h +++ b/hal/api/critical.h @@ -25,16 +25,16 @@ extern "C" { #endif -/** Determine the current interrupts disabled state +/** Determine the current interrupts enabled state * - * This function can be called to determine whether or not interrupts are currently disabled. + * This function can be called to determine whether or not interrupts are currently enabled. * \note * NOTE: * This function works for both cortex-A and cortex-M, although the underlyng implementation * differs. - * @return true if interrupts are disabled, false otherwise + * @return true if interrupts are enabled, false otherwise */ -bool get_interrupts_disabled(void); +bool are_interrupts_enabled(void); /** Mark the start of a critical section * diff --git a/hal/common/critical.c b/hal/common/critical.c index 6dafe8f6388..6426416f441 100644 --- a/hal/common/critical.c +++ b/hal/common/critical.c @@ -29,19 +29,18 @@ static volatile uint32_t interrupt_enable_counter = 0; static volatile bool critical_interrupts_disabled = false; -bool get_interrupts_disabled(void) +bool are_interrupts_enabled(void) { #if defined(__CORTEX_A9) - bool interrupts_disabled = (bool)(((__get_CPSR() & 0x80) >> 7) & 0x1); + return ((__get_CPSR() & 0x80) == 0); #else - bool interrupts_disabled = (bool)(__get_PRIMASK() & 0x1); + return ((__get_PRIMASK() & 0x1) == 0); #endif - return interrupts_disabled; } void core_util_critical_section_enter() { - bool interrupts_disabled = get_interrupts_disabled(); + bool interrupts_disabled = !are_interrupts_enabled(); __disable_irq(); /* Save the interrupt disabled state as it was prior to any nested critical section lock use */ @@ -71,7 +70,7 @@ void core_util_critical_section_exit() // FIXME #ifndef FEATURE_UVISOR - bool interrupts_disabled = get_interrupts_disabled(); /* get the current interrupt disabled state */ + bool interrupts_disabled = !are_interrupts_enabled(); /* get the current interrupt disabled state */ MBED_ASSERT(interrupts_disabled); /* Interrupts must be disabled on invoking an exit from a critical section */ #else From 329f8a10dc72eb2361e8487e47fe73189769e4b2 Mon Sep 17 00:00:00 2001 From: Anna Bridge Date: Fri, 1 Jul 2016 15:08:31 +0100 Subject: [PATCH 3/3] Add core_util_ prefix to are_interrupts_enabled() function. For consistency with other exposed functions from this file, core_util_ prefix should be added. --- hal/api/critical.h | 2 +- hal/common/critical.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hal/api/critical.h b/hal/api/critical.h index 464ce10e976..2c288a54301 100644 --- a/hal/api/critical.h +++ b/hal/api/critical.h @@ -34,7 +34,7 @@ extern "C" { * differs. * @return true if interrupts are enabled, false otherwise */ -bool are_interrupts_enabled(void); +bool core_util_are_interrupts_enabled(void); /** Mark the start of a critical section * diff --git a/hal/common/critical.c b/hal/common/critical.c index 6426416f441..47bfd23b128 100644 --- a/hal/common/critical.c +++ b/hal/common/critical.c @@ -29,7 +29,7 @@ static volatile uint32_t interrupt_enable_counter = 0; static volatile bool critical_interrupts_disabled = false; -bool are_interrupts_enabled(void) +bool core_util_are_interrupts_enabled(void) { #if defined(__CORTEX_A9) return ((__get_CPSR() & 0x80) == 0); @@ -40,7 +40,7 @@ bool are_interrupts_enabled(void) void core_util_critical_section_enter() { - bool interrupts_disabled = !are_interrupts_enabled(); + bool interrupts_disabled = !core_util_are_interrupts_enabled(); __disable_irq(); /* Save the interrupt disabled state as it was prior to any nested critical section lock use */ @@ -70,7 +70,7 @@ void core_util_critical_section_exit() // FIXME #ifndef FEATURE_UVISOR - bool interrupts_disabled = !are_interrupts_enabled(); /* get the current interrupt disabled state */ + bool interrupts_disabled = !core_util_are_interrupts_enabled(); /* get the current interrupt disabled state */ MBED_ASSERT(interrupts_disabled); /* Interrupts must be disabled on invoking an exit from a critical section */ #else