Skip to content

Fix hardware spinlock IDs on RP2350 #2645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fix hardware spinlock IDs on RP2350 #2645

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion src/rp2_common/hardware_sync_spin_lock/include/hardware/sync/spin_lock.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "pico.h"
#include "hardware/sync.h"

// PICO_CONFIG: PICO_USE_SW_SPIN_LOCKS, Use software implementation for spin locks, type=bool, default=1 on RP2350 due to errata, group=hardware_sync
// PICO_CONFIG: PICO_USE_SW_SPIN_LOCKS, Use software implementation for spin locks, type=bool, default=1 on RP2350 due to errata E2, group=hardware_sync
#ifndef PICO_USE_SW_SPIN_LOCKS
#if PICO_RP2350
#define PICO_USE_SW_SPIN_LOCKS 1
Expand All @@ -19,53 +19,93 @@

// PICO_CONFIG: PICO_SPINLOCK_ID_IRQ, Spinlock ID for IRQ protection, min=0, max=31, default=9, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_IRQ
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_IRQ 5
#else
#define PICO_SPINLOCK_ID_IRQ 9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kilograham In the "original" version of this code, do you remember why the "defined" spinlocks start at 9? I.e. is it actually okay for the "modified" version of this code to use spinlocks 5, 6 and 7?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was getting to comment on this; the main reason is that they are used by pico-extras stuff, so yeah, we want to be careful about what we assign into that range (and pick something that isn't likely to be used at runtime) - i just havent spent any brain cycles picking which, but @will-v-pi you are free to ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In pico-extras, scan video uses 2-5, and audio uses 6&7 - so that would mean starting SDK ones from 10. Note that the pico-extras spin locks will be affected by E2 too, so if we wanted to assign all those safe ones too, the SDK ones would need to start from 19

Alternatively, I could put errors in the ifdefs in pico-extras which these clash with, to say that if on RP2350 and not using software spin locks, you need to define your own IDs for the clashes? I think I’d prefer this option, as it gives the SDK all the spin locks, and the people it affects can pick their own spin locks to use (either SDK bits they don’t use, the OS ones, or from the ranges at the end)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened raspberrypi/pico-extras#99 in pico-extras to throw errors when not using software spin locks, with the error message explaining that you need to define the IDs

#endif
#endif

// PICO_CONFIG: PICO_SPINLOCK_ID_TIMER, Spinlock ID for Timer protection, min=0, max=31, default=10, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_TIMER
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_TIMER 6
#else
#define PICO_SPINLOCK_ID_TIMER 10
#endif
#endif

// PICO_CONFIG: PICO_SPINLOCK_ID_HARDWARE_CLAIM, Spinlock ID for Hardware claim protection, min=0, max=31, default=11, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_HARDWARE_CLAIM
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_HARDWARE_CLAIM 7
#else
#define PICO_SPINLOCK_ID_HARDWARE_CLAIM 11
#endif
#endif

// PICO_CONFIG: PICO_SPINLOCK_ID_RAND, Spinlock ID for Random Number Generator, min=0, max=31, default=12, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_RAND
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_RAND 10
#else
#define PICO_SPINLOCK_ID_RAND 12
#endif
#endif

// PICO_CONFIG: PICO_SPINLOCK_ID_ATOMIC, Spinlock ID for atomics, min=0, max=31, default=13, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_ATOMIC
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_ATOMIC 11
#else
#define PICO_SPINLOCK_ID_ATOMIC 13
#endif
#endif

// PICO_CONFIG: PICO_SPINLOCK_ID_OS1, First Spinlock ID reserved for use by low level OS style software, min=0, max=31, default=14, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_OS1
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_OS1 18
#else
#define PICO_SPINLOCK_ID_OS1 14
#endif
#endif

// PICO_CONFIG: PICO_SPINLOCK_ID_OS2, Second Spinlock ID reserved for use by low level OS style software, min=0, max=31, default=15, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_OS2
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_OS2 19
#else
#define PICO_SPINLOCK_ID_OS2 15
#endif
#endif

// PICO_CONFIG: PICO_SPINLOCK_ID_STRIPED_FIRST, Lowest Spinlock ID in the 'striped' range, min=0, max=31, default=16, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_STRIPED_FIRST
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_STRIPED_FIRST 20
#else
#define PICO_SPINLOCK_ID_STRIPED_FIRST 16
#endif
#endif

// PICO_CONFIG: PICO_SPINLOCK_ID_STRIPED_LAST, Highest Spinlock ID in the 'striped' range, min=0, max=31, default=23, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_STRIPED_LAST
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_STRIPED_LAST 25
#else
#define PICO_SPINLOCK_ID_STRIPED_LAST 23
#endif
#endif

// PICO_CONFIG: PICO_SPINLOCK_ID_CLAIM_FREE_FIRST, Lowest Spinlock ID in the 'claim free' range, min=0, max=31, default=24, group=hardware_sync
#ifndef PICO_SPINLOCK_ID_CLAIM_FREE_FIRST
#if PICO_RP2350 && !PICO_USE_SW_SPIN_LOCKS
#define PICO_SPINLOCK_ID_CLAIM_FREE_FIRST 26
#else
#define PICO_SPINLOCK_ID_CLAIM_FREE_FIRST 24
#endif
#endif

#ifdef PICO_SPINLOCK_ID_CLAIM_FREE_END
#warning PICO_SPINLOCK_ID_CLAIM_FREE_END has been renamed to PICO_SPINLOCK_ID_CLAIM_FREE_LAST
Expand Down
Loading