Rework lock_core / timers #378
No reviewers
Labels
No labels
bazel
bluetooth
board settings
boot_stage2
bug
build
c++
can't reproduce
clang
clib-interface
cmsis
code cleanliness
code size
compiler version issues
cyw43_driver
debug
discussion
documentation
duplicate
dynamic_clock
elf2uf2
enhancement
errata
filesystem
flash layout
floating point
FreeRTOS
hardware_adc
hardware_claim
hardware_clocks
hardware_dma
hardware_exception
hardware_flash
hardware_gpio
hardware_i2c
hardware_irq
hardware_pio
hardware_pwm
hardware_rcp
hardware_rtc
hardware_spi
hardware_sync
hardware_uart
hardware_vreg
hardware_watchdog
hardware_xosc
help wanted
host
IAR
installation
invalid
linker scripts
lto
lwip
mbedtls
missing API
needs reporter detail
networking
newlib
newlib-multicore
non-c
non-cmake
non-gcc
openocd
orphan PR
OS integration
pico_async_context
pico_base
pico_binary_info
pico_flash
picolibc
pico_lwip
pico_malloc
pico_multicore
pico_printf
pico_runtime
pico_stdio
pico_stdio_usb
pico_sync
pico_time
pico_util
pioasm
port
question
review
review2
rtos interop
runtime init
static library
svd
test
tinyusb
vscode
wifi integration
wontfix
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-raspberrypi/pico-sdk!378
Loading…
Reference in a new issue
No description provided.
Delete branch "lock_core_rework"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
implementations to replace WFE/SEV with something more appropriate
Looks okay to me. I spent a while reviewing as requested by Graham.
@ -13,0 +20,4 @@* lock which is used only to protect the contents of the rest of the structure as part of implementing the synchronization* primitive. As such, the spin_lock member of lock core is never still held on return from any function for the primitive.** \ref critical_section is an exceptional case in that it does not have a lock_core_t and simply wraps a pin lock, providing"wraps a pin lock" -> "wraps a spin lock" ?
@ -13,0 +28,4 @@** By default the SDK just uses the processors' events via SEV and WEV for notification and blocking as these are sufficient for* cross core, and notification from interrupt handlers. However macros are defined in this file that abstract the wait* and notify mechanisms to allow the SDK locking functions to effectively be used within an RTOS on other environment."RTOS on other environment" -> "RTOS or other environment" ?
@ -13,0 +32,4 @@** When implementing an RTOS, it is desirable for the SDK synchronization primitives that wait, to block the calling task (and immediately yield),* and those that notify, to wake a blocked task which isn't on processor. At least the wait macro implementation needs to be atomic with the protecting* spin_lock unlock from the callers point of view; i.e. the task should unlock the spin lock when as it starts its wait. Such implementation is"spin lock when as it starts" -> "spin lock when it starts" ?
@ -34,0 +114,4 @@** \param lock the lock_core for the primitive which needs to block* \param save the uint32_t value that should be passed to spin_unlock when the spin lock is unlocked. (i.e. the `PRIMASK`* state when the spin lock was acquireMixture of using "this method" and "this macro" (although I dunno if this difference is significant?)
@ -34,0 +182,4 @@* RTOS which is able to block the current task until the scheduler tick before* the given time)** \param until the \ref absolute_time_t value"if can" -> "it can"
@ -36,2 +39,4 @@#define MAX_RECURSION_STATE ((uint8_t)255)/*! \brief Initialise a mutex structureHopefully people won't accidentally assume that
MAX_RECURSION_STATEcorresponds to the "1 is a maxed out recursive lock" described above?@ -35,0 +33,4 @@lock_owner_id_t owner; //! owner id LOCK_INVALID_OWNER_ID for unowneduint8_t recursion_state; //! 0 means non recursive (owner or unowned)//! 1 is a maxed out recursive lock//! 2-254 is an owned lockI wonder if it'd have been worth making this
uint8_ta typedef (e.g.recursion_state_t) ? 🤷 Or perhaps we already have enough typedefs scattered throughout the place.