diff --git a/src/rp2_common/pico_multicore/include/pico/multicore.h b/src/rp2_common/pico_multicore/include/pico/multicore.h index 9b4622a98..fe198aed6 100644 --- a/src/rp2_common/pico_multicore/include/pico/multicore.h +++ b/src/rp2_common/pico_multicore/include/pico/multicore.h @@ -437,7 +437,7 @@ static inline uint multicore_doorbell_irq_num(uint doorbell_num) { * The core which wishes to lockout the other core calls \ref multicore_lockout_start_blocking or * \ref multicore_lockout_start_timeout_us to interrupt the other "victim" core and wait for it to be in a * "locked out" state. Once the lockout is no longer needed it calls \ref multicore_lockout_end_blocking or - * \ref multicore_lockout_end_timeout_us to release the lockout and wait for confirmation. + * \ref multicore_lockout_end_timeout_us to release the lockout. * * \note Because multicore lockout uses the intercore FIFOs, the FIFOs cannot be used for any other purpose */ @@ -491,27 +491,31 @@ void multicore_lockout_start_blocking(void); */ bool multicore_lockout_start_timeout_us(uint64_t timeout_us); -/*! \brief Release the other core from a locked out state amd wait for it to acknowledge +/*! \brief Release the other core from a locked out state * \ingroup multicore_lockout * - * \note The other core must previously have been "locked out" by calling a `multicore_lockout_start_` function - * from this core + * The other core must previously have been "locked out" by calling a `multicore_lockout_start_` function + * from this core. + * + * \note The other core will leave the lockout state if this function is called. + * The function only blocks for access to a lockout mutex, it does not wait for the other core + * to leave the lockout state. */ void multicore_lockout_end_blocking(void); -/*! \brief Release the other core from a locked out state amd wait up to a time limit for it to acknowledge +/*! \brief Release the other core from a locked out state * \ingroup multicore_lockout * * The other core must previously have been "locked out" by calling a `multicore_lockout_start_` function * from this core * - * \note be very careful using small timeout values, as a timeout here will leave the "lockout" functionality - * in a bad state. It is probably preferable to use \ref multicore_lockout_end_blocking anyway as if you have - * already waited for the victim core to enter the lockout state, then the victim core will be ready to exit - * the lockout state very quickly. + * \note The other core will leave the lockout state if this function returns true. + * The function only blocks for access to a lockout mutex, it does not wait for the other core + * to leave the lockout state. If the lockout mutex could not be acquired, the function returns + * false and no action is taken. * * \param timeout_us the timeout in microseconds - * \return true if the other core successfully exited locked out state within the timeout, false otherwise + * \return true if the other core will leave the lockout state, false otherwise */ bool multicore_lockout_end_timeout_us(uint64_t timeout_us); diff --git a/src/rp2_common/pico_multicore/multicore.c b/src/rp2_common/pico_multicore/multicore.c index f8f5660db..3e397c74e 100644 --- a/src/rp2_common/pico_multicore/multicore.c +++ b/src/rp2_common/pico_multicore/multicore.c @@ -202,25 +202,27 @@ void multicore_launch_core1_raw(void (*entry)(void), uint32_t *sp, uint32_t vect irq_set_enabled(irq_num, enabled); } -#define LOCKOUT_MAGIC_START 0x73a8831eu -#define LOCKOUT_MAGIC_END (~LOCKOUT_MAGIC_START) - +// A non-zero initialisation value is used in order to reduce the chance of +// entering the lock handler on bootup due to a 0-word being present in the FIFO +static volatile uint32_t lockout_request_id = 0x73a8831eu; static mutex_t lockout_mutex; -static bool lockout_in_progress; // note this method is in RAM because lockout is used when writing to flash // it only makes inline calls static void __isr __not_in_flash_func(multicore_lockout_handler)(void) { multicore_fifo_clear_irq(); while (multicore_fifo_rvalid()) { - if (sio_hw->fifo_rd == LOCKOUT_MAGIC_START) { + uint32_t request_id = sio_hw->fifo_rd; + if (request_id == lockout_request_id) { + // valid lockout request received uint32_t save = save_and_disable_interrupts(); - multicore_fifo_push_blocking_inline(LOCKOUT_MAGIC_START); - while (multicore_fifo_pop_blocking_inline() != LOCKOUT_MAGIC_END) { - tight_loop_contents(); // not tight but endless potentially + multicore_fifo_push_blocking_inline(request_id); + // wait for the lockout to expire + while (request_id == lockout_request_id) { + // when lockout_request_id is updated, the other CPU core calls __sev + __wfe(); } restore_interrupts_from_disabled(save); - multicore_fifo_push_blocking_inline(LOCKOUT_MAGIC_END); } } } @@ -257,7 +259,7 @@ void multicore_lockout_victim_deinit(void) { } } -static bool multicore_lockout_handshake(uint32_t magic, absolute_time_t until) { +static bool multicore_lockout_handshake(uint32_t request_id, absolute_time_t until) { uint irq_num = SIO_FIFO_IRQ_NUM(get_core_num()); bool enabled = irq_is_enabled(irq_num); if (enabled) irq_set_enabled(irq_num, false); @@ -267,7 +269,7 @@ static bool multicore_lockout_handshake(uint32_t magic, absolute_time_t until) { if (next_timeout_us < 0) { break; } - multicore_fifo_push_timeout_us(magic, (uint64_t)next_timeout_us); + multicore_fifo_push_timeout_us(request_id, (uint64_t)next_timeout_us); next_timeout_us = absolute_time_diff_us(get_absolute_time(), until); if (next_timeout_us < 0) { break; @@ -276,7 +278,7 @@ static bool multicore_lockout_handshake(uint32_t magic, absolute_time_t until) { if (!multicore_fifo_pop_timeout_us((uint64_t)next_timeout_us, &word)) { break; } - if (word == magic) { + if (word == request_id) { rc = true; } } while (!rc); @@ -284,14 +286,28 @@ static bool multicore_lockout_handshake(uint32_t magic, absolute_time_t until) { return rc; } +static uint32_t update_lockout_request_id(void) { + // generate new number and then update shared variable + uint32_t new_request_id = lockout_request_id + 1; + lockout_request_id = new_request_id; + // notify other core + __sev(); + return new_request_id; +} + static bool multicore_lockout_start_block_until(absolute_time_t until) { check_lockout_mutex_init(); if (!mutex_enter_block_until(&lockout_mutex, until)) { return false; } - hard_assert(!lockout_in_progress); - bool rc = multicore_lockout_handshake(LOCKOUT_MAGIC_START, until); - lockout_in_progress = rc; + // generate a new request_id number + uint32_t request_id = update_lockout_request_id(); + // attempt to lock out + bool rc = multicore_lockout_handshake(request_id, until); + if (!rc) { + // lockout failed - cancel it + update_lockout_request_id(); + } mutex_exit(&lockout_mutex); return rc; } @@ -309,13 +325,10 @@ static bool multicore_lockout_end_block_until(absolute_time_t until) { if (!mutex_enter_block_until(&lockout_mutex, until)) { return false; } - assert(lockout_in_progress); - bool rc = multicore_lockout_handshake(LOCKOUT_MAGIC_END, until); - if (rc) { - lockout_in_progress = false; - } + // lockout finished - cancel it + update_lockout_request_id(); mutex_exit(&lockout_mutex); - return rc; + return true; } bool multicore_lockout_end_timeout_us(uint64_t timeout_us) { @@ -402,4 +415,4 @@ void multicore_doorbell_unclaim(uint doorbell_num, uint core_mask) { } -#endif \ No newline at end of file +#endif