-
Notifications
You must be signed in to change notification settings - Fork 17
Add a C re-entrant critical section API #18
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* PackageLicenseDeclared: Apache-2.0 | ||
* Copyright (c) 2015 ARM Limited | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#ifndef __MBED_UTIL_CRITICAL_H__ | ||
#define __MBED_UTIL_CRITICAL_H__ | ||
|
||
#ifdef __cplusplus | ||
namespace mbed { | ||
namespace util { | ||
extern "C" { | ||
#endif | ||
|
||
void critical_section_enter(); | ||
void critical_section_exit(); | ||
|
||
#ifdef __cplusplus | ||
} // extern "C" | ||
} // namspace util | ||
} // namespace mbed | ||
#endif | ||
|
||
|
||
#endif // __MBED_UTIL_CRITICAL_H__ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/* | ||
* PackageLicenseDeclared: Apache-2.0 | ||
* Copyright (c) 2015 ARM Limited | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
#include <stdint.h> // uint32_t, UINT32_MAX | ||
#include <stddef.h> // NULL | ||
#include "cmsis-core/core_generic.h" //__disable_irq, __enable_irq | ||
|
||
// Module include | ||
#include "mbed-util/critical.h" | ||
|
||
static volatile uint32_t interruptEnableCounter = 0; | ||
static volatile uint32_t critical_primask = 0; | ||
const uint32_t invalid = 0xFFFFFFFEUL; | ||
void critical_section_enter() | ||
{ | ||
/* sample the primask */ | ||
if (!interruptEnableCounter) { | ||
critical_primask = __get_PRIMASK(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably use atomic_cas There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the primask like this introduces a race condition. Instead, we should check to see that interrupts are currently enabled, and assert if they are not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't atomic_cas c++ and thus not usable in a c based critical section lock function? |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If interruptEnableCounter is non-zero, we should check the primask. If interrupts are enabled, we should assert that something has enabled them. |
||
__disable_irq(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the interrupts are disabled and you call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't disable interrupts, call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I won't disable interrupts, but library code (for example) might not care if it runs with interrupts enabled or not, but still re-enable interrupts by mistake if it uses these functions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That said, I could preserve the primask if |
||
/* not allowed to overflow the interruptEnableCounter. */ | ||
if (interruptEnableCounter == UINT32_MAX) { | ||
/* Generate a fault */ | ||
((void(*)(void))&invalid)(); | ||
} | ||
interruptEnableCounter++; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably use atomic_incr |
||
} | ||
|
||
void critical_section_exit() | ||
{ | ||
if (interruptEnableCounter) interruptEnableCounter--; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably use atomic_decr There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should check the primask here and see if someone has enabled interrupts when they shouldn't be. |
||
if (!interruptEnableCounter && !critical_primask) __enable_irq(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, let's not. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
/* | ||
* PackageLicenseDeclared: Apache-2.0 | ||
* Copyright (c) 2015 ARM Limited | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#include "cmsis-core/core_generic.h" // __get_PRIMASK | ||
#include "mbed/test_env.h" // MBED_HOSTTEST_* definitions | ||
// Module include | ||
#include "mbed-util/critical.h" | ||
|
||
void app_start(int argc, char *argv[]) | ||
{ | ||
MBED_HOSTTEST_TIMEOUT(5); | ||
MBED_HOSTTEST_SELECT(default); | ||
MBED_HOSTTEST_DESCRIPTION(mbed-util critical section test); | ||
MBED_HOSTTEST_START("MBED_UTIL_CRITICAL"); | ||
bool pass = false; | ||
do { | ||
// Check the current primask | ||
uint32_t primask; | ||
if ((primask = __get_PRIMASK()) != 0) break; | ||
|
||
// Test a single critical section | ||
mbed::util::critical_section_enter(); | ||
if ((primask = __get_PRIMASK()) == 0) break; | ||
mbed::util::critical_section_exit(); | ||
if ((primask = __get_PRIMASK()) != 0) break; | ||
|
||
// Test multiple nested critical sections | ||
for (int i = 0; i < 16; i++) { | ||
mbed::util::critical_section_enter(); | ||
} | ||
if ((primask = __get_PRIMASK()) == 0) break; | ||
for (int i = 0; i < 16; i++) { | ||
mbed::util::critical_section_exit(); | ||
} | ||
if ((primask = __get_PRIMASK()) != 0) break; | ||
|
||
pass = true; | ||
} while (0); | ||
|
||
MBED_HOSTTEST_RESULT(pass); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable is not required.