Skip to content
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.

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

Add a shim header to support locking without ldrex/strex #1517

Merged
merged 3 commits into from
Dec 17, 2024

Conversation

martinling
Copy link
Member

Required to allow the USB queue code to be built for the M0.

Required to allow the USB queue code to be built for the M0.
@martinling martinling marked this pull request as ready for review December 16, 2024 19:48
@martinling martinling requested a review from miek December 16, 2024 19:48
@u-foka
Copy link

u-foka commented Dec 16, 2024

Required to allow the USB queue code to be built for the M0.

Hello again :) Sorry to bug you with this, but to achieve this goal, it would be great if you could also change the nvic.h include to dispatch ;) Thanks for all the help!

#1516 (comment)

Copy link
Member

@miek miek left a comment

Choose a reason for hiding this comment

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

One minor concern I have is about the side-effects of using one of the functions on their own (and leaving interrupts disabled for example) but, given that ldrex/strex must be used in a pair, I'm ok with assuming that for these functions too. I think it'd be worth adding a comment to that effect though.

@martinling
Copy link
Member Author

Both changes made.

@mossmann mossmann merged commit cfdfe8a into greatscottgadgets:master Dec 17, 2024
17 checks passed
@gullradriel
Copy link
Contributor

Thanks a lot :-)

@gullradriel
Copy link
Contributor

gullradriel commented Dec 17, 2024

Back from testing.
Sorry for pinging you anew, @martinling .
Compiling on M0 is giving errors for undefined references to the intrinsic __disable_irq and __enable_irq.
The only two solutions for us were to either #include "hal.h" from chibios, or use the asm alternatives to those functions.
Since I don't think hackrf code is using chibios, would it be possible to use the following code instead:

static inline uint32_t load_exclusive(volatile uint32_t* addr)
{
    __asm volatile ("cpsid i");  // __disable_irq();
    return *addr;
}

static inline uint32_t store_exclusive(uint32_t val, volatile uint32_t* addr)
{
    *addr = val;
    __asm volatile ("cpsie i");  // __enable_irq();
    return 0;
}

Or maybe you prefer the version with the "memory" clobber ?

static inline uint32_t load_exclusive(volatile uint32_t* addr)
{
    __asm volatile ("cpsid i" ::: "memory");  // __disable_irq();
    return *addr;
}

static inline uint32_t store_exclusive(uint32_t val, volatile uint32_t* addr)
{
    *addr = val;
    __asm volatile ("cpsie i" ::: "memory");  // __enable_irq();
    return 0;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants