add more/better documentation to pico/multicore.h #620
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!620
Loading…
Reference in a new issue
No description provided.
Delete branch "multicore_doc"
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?
@ -44,3 +47,3 @@** Reset core1 and enter the given function on core 1 using the default core 1 stack (below core 0 stack)* Wake up (a previously reset) core 1 and enter the given function on core 1 using the default core 1 stack (below core 0 stack).*Presumably this always needs to run on core 0? What would happen if you accidentally ran it on core 1?
@ -33,95 +33,155 @@ extern "C" {#endifvector_Table->vector_table@ -33,95 +33,155 @@ extern "C" {#endiffunctions -> function
@ -33,95 +33,155 @@ extern "C" {#endifProbably don't need to explicitly mention
sphere, now that you've added\param sp?@ -75,0 +102,4 @@* core 1 launch or by the \ref multicore_lockout functions). Additionally they are often required for the exclusive use* of an RTOS (e.g. FreeRTOS SMP). For these reasons it is suggested that you do not use the FIFO for your own purposes* unless none of the above concerns apply; the majority of cases for transferring data between cores can be eqaully* well handled by using a \ref queueThis seems to be the only "launch" function that allows a vector table to be passed? I know nothing about low-level multicore programming, but would e.g. a
multicore_launch_core1_with_stack_and_vector_tablebe useful? 🤷♂️@ -33,95 +33,155 @@ extern "C" {#endif"exclusive used" -> "exclusive use"
@ -33,95 +33,155 @@ extern "C" {#endifThe "waiting sent" doesn't sound quite right IMHO. What about:
"Check the read FIFO to see if it contains data sent by the other core"
?
@ -33,95 +33,155 @@ extern "C" {#endifwash -> was 😉 🧼
@ -33,95 +33,155 @@ extern "C" {#endifNot sure if the "unsigned" is needed here, as
multicore_fifo_push_blockingjust says "A 32 bit value" ?@ -33,95 +33,155 @@ extern "C" {#endif"data to" -> "data from" ?
@ -33,95 +33,155 @@ extern "C" {#endif"there timeout" -> "the timeout"
@ -33,95 +33,155 @@ extern "C" {#endifwash -> was
pushed -> popped
@ -33,95 +33,155 @@ extern "C" {#endifPerhaps change "incoming FIFO" to "read FIFO" for consistency with the other function-descriptions?
Should this mention the ROE and WOF labels, to match up with the doxygen for the
multicore_fifo_clear_irqfunction?This "Sometimes it is useful to enter a critical section on both cores at once. On a single core system, this can easily be done..." seems like a bit of a logical tautology? 😉 I think I get the intention though.
cna -> can
Should the
/refbe\ref? (or does Doxygen allow both?)paused -> pause?
paused -> pause?
timout -> timeout
these -> this?
I'm not sure how the "the victim core is executing code from RAM" stuff here meshes with the other parts which talk about the victim core being paused? 😕
After reading this a couple of times, perhaps "This makes the lockout functions suitable for code that wants to write to flash..." makes more sense than "This allows the lockout functions to be used by code that wants to write to flash..." ? 🤷♂️ (I was confused by the "allows")
"force to" -> "forced to" ?
Given that you can only call
multicore_lockout_end_aftermulticore_lockout_start_, what's the use-case for ever needing/wanting to usemulticore_lockout_end_timeout_us? "if you have already waited for the victim core to enter the lockout state" - there doesn't seem to be any way to not wait for the victim core to enter the lockout state? 🤔EDIT: Oh, or will the victim core "eventually" enter the lockout state, if you called
multicore_lockout_start_timeout_uswith a small timeout value and it returned false? (If so, perhaps that's worth clarifying in themulticore_lockout_start_timeout_usdoxygen comments?)@ -44,3 +47,3 @@** Reset core1 and enter the given function on core 1 using the default core 1 stack (below core 0 stack)* Wake up (a previously reset) core 1 and enter the given function on core 1 using the default core 1 stack (below core 0 stack).*good point, it doesn't work right on core 1 added a note
Having read through the doxygen again, perhaps it makes sense to replace references to "pause" with "lockout state", with a brief description of what "lockout state" means? (I'm going to guess that it's just sitting in a busy loop, waiting to be told to exit the lockout state?)
@ -75,0 +102,4 @@* core 1 launch or by the \ref multicore_lockout functions). Additionally they are often required for the exclusive use* of an RTOS (e.g. FreeRTOS SMP). For these reasons it is suggested that you do not use the FIFO for your own purposes* unless none of the above concerns apply; the majority of cases for transferring data between cores can be eqaully* well handled by using a \ref queuei think it is fine, i doubt anyone has ever called it
Does this need to describe what happens if you try to "lockout" a core that's already in the lockout state? Is it a NOP?
@ -33,95 +33,155 @@ extern "C" {#endifnot sure how the emojis are better than 'h" :-)
added ROE/WOF
made it slightly - but not much - better, perhaps
the
improved
ok
yeah
multicore_lockout_end_timeout_usis largely pointless, but since i can't remove it i thought id improve the docs.most of the complication arises from the possibility of both cores trying to lock each other out the same time or so (which probably people are not actually doing)
@ -33,95 +33,155 @@ extern "C" {#endifMissed this one? To clarify:
"(data to the other core)" -> "(data from the other core)" ?
@ -112,4 +158,4 @@* This function will block until there is data ready to be read* Use multicore_fifo_rvalid() to check if data is ready to be read if you don't* want to block.*"the timeout occurred before data could be pushed" -> "the timeout occurred before data could be popped"
Just out of curiosity, would this only return false if
multicore_lockout_victim_inithadn't been called on the other core?@ -133,38 +193,117 @@ static inline void multicore_fifo_drain(void) {*nitpick: "inter process FIFOs" -> "inter-core FIFOs" ?
nitnitpick: This PR uses "inter-core FIFO", "intercore FIFO" and "multicore FIFO"
it will hang or timeout depending on which you call
I guess I didn't ask the question very clearly: I was trying to ask "In which conditions would this function return false?"
@ -97,9 +97,11 @@ bool watchdog_caused_reboot(void);* This would not be present if a watchdog reset is initiated by \ref watchdog_reboot or by the RP2040 bootromwatchdog_enabled -> watchdog_enable ?
@ -97,9 +97,11 @@ bool watchdog_caused_reboot(void);* This would not be present if a watchdog reset is initiated by \ref watchdog_reboot or by the RP2040 bootromThis does indeed look like a typo
approved + tiny typo => force merge