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

Support for atomic Peripherals::take() #613

Closed
VictorKoenders opened this issue May 25, 2022 · 3 comments
Closed

Support for atomic Peripherals::take() #613

VictorKoenders opened this issue May 25, 2022 · 3 comments

Comments

@VictorKoenders
Copy link

VictorKoenders commented May 25, 2022

Hello,

I'm currently working on a toy kernel on a raspberry pi, and I'm using svd2rust to generate the bindings to the aarch64 BCM2837 chip that the raspberry pi 3b is running on.

When generating the peripherals I cannot select a Target because the aarch64 is multi-core and critical sections do not work. Therefor the Peripherals::take() -> Option<Self> method won't be generated.

Currently I inject the following code into the generated output:

use core::sync::atomic::{AtomicBool, Ordering};

static LOCKED: AtomicBool = AtomicBool::new(false);

impl Peripherals {
    pub fn take() -> Option<Self> {
        let was_locked = LOCKED.compare_exchange(false, true, Ordering::Acquire, Ordering::Acquire).is_ok();
        if was_locked {
            None
        } else {
            Some(unsafe { Self::steal() })
        }
    }
}

impl Drop for Peripherals {
    fn drop(&mut self) {
        LOCKED.store(false, Ordering::Relaxed);
    }
}

I was considering porting this to svd2rust but I'm not sure how to approach this. I could:

  • add a Target::Atomic
  • Add a Target::Custom(TargetOptions) and
#[non_exhaustive]
#[derive(Default)]
pub struct TargetOptions {
    pub atomic: bool,
    pub interrupt_free: bool,
    // other options
}

(This would make all the other Target an alias for a hard-coded TargetOptions implementation)

  • Add a new boolean field to svd2rust::utils::Config to overwrite Peripherals::take() with an atomic implementation when that field is set

Or maybe another approach is better. What do you think?

@Emilgardis
Copy link
Member

I think it makes sense to do it the other way around, Target is the core thing, and TargetOptions is what is consumed, it has the target and its options

@reitermarkus
Copy link
Member

@VictorKoenders, does #651 fix this? You will need an implementation for critical_section that works correctly on multi-core of course.

@burrbull
Copy link
Member

burrbull commented Nov 7, 2022

Closing as solved

@burrbull burrbull closed this as completed Nov 7, 2022
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

No branches or pull requests

4 participants