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

riscv: add fallible functions #222

Merged
merged 10 commits into from
Jul 17, 2024
4 changes: 4 additions & 0 deletions riscv-pac/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Added

- Add `result` module for `Error` and `Result` types

## [v0.1.1] - 2024-02-15

- Fix crates.io badge links
Expand Down
10 changes: 7 additions & 3 deletions riscv-pac/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#![no_std]

pub mod result;

use result::Result;

/// Trait for enums of target-specific external interrupt numbers.
///
/// This trait should be implemented by a peripheral access crate (PAC)
Expand All @@ -23,7 +27,7 @@ pub unsafe trait InterruptNumber: Copy {

/// Tries to convert a number to a valid interrupt source.
/// If the conversion fails, it returns an error with the number back.
fn from_number(value: u16) -> Result<Self, u16>;
fn from_number(value: u16) -> Result<Self>;
}

/// Trait for enums of priority levels.
Expand All @@ -49,7 +53,7 @@ pub unsafe trait PriorityNumber: Copy {

/// Tries to convert a number to a valid priority level.
/// If the conversion fails, it returns an error with the number back.
fn from_number(value: u8) -> Result<Self, u8>;
fn from_number(value: u8) -> Result<Self>;
}

/// Trait for enums of HART identifiers.
Expand All @@ -75,5 +79,5 @@ pub unsafe trait HartIdNumber: Copy {

/// Tries to convert a number to a valid HART ID.
/// If the conversion fails, it returns an error with the number back.
fn from_number(value: u16) -> Result<Self, u16>;
fn from_number(value: u16) -> Result<Self>;
}
48 changes: 48 additions & 0 deletions riscv-pac/src/result.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
use core::fmt;

/// Convenience alias for the [Result](core::result::Result) type for the library.
pub type Result<T> = core::result::Result<T, Error>;

/// Represents error variants for the library.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum Error {
/// Attempted out-of-bounds access.
IndexOutOfBounds {
index: usize,
min: usize,
max: usize,
},
/// Invalid field value.
InvalidValue {
field: &'static str,
value: usize,
bitmask: usize,
},
/// Invalid value of a register field that does not match any known variants.
InvalidVariant { field: &'static str, value: usize },
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could differentiate errors regarding fields of a register and errors regarding the whole register/value. The idea is avoiding defining the field in the riscv-peripheral examples, as I think it is not necessary.

Suggested change
/// Invalid field value.
InvalidValue {
field: &'static str,
value: usize,
bitmask: usize,
},
/// Invalid value of a register field that does not match any known variants.
InvalidVariant { field: &'static str, value: usize },
/// Invalid value of a register field.
InvalidFieldValue {
field: &'static str,
value: usize,
bitmask: usize,
},
/// Invalid value of a register field that does not match any known variants.
InvalidFieldVariant { field: &'static str, value: usize },
/// Invalid value.
InvalidValue {
value: usize,
bitmask: usize,
},
/// Invalid value that does not match any known variants.
InvalidVariant(usize),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we could differentiate errors regarding fields of a register and errors regarding the whole register/value.

This makes sense to me. The value covers the entire structure, so this separation could be useful in other cases without subfields.

/// Unimplemented function or type.
Unimplemented,
}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::IndexOutOfBounds { index, min, max } => write!(
f,
"out-of-bounds access, index: {index}, min: {min}, max: {max}"
),
Self::InvalidValue {
field,
value,
bitmask,
} => write!(
f,
"invalid {field} field value: {value:#x}, valid bitmask: {bitmask:#x}",
),
Self::InvalidVariant { field, value } => {
write!(f, "invalid {field} field variant: {value:#x}")
}
Self::Unimplemented => write!(f, "unimplemented"),
}
}
}
4 changes: 4 additions & 0 deletions riscv-peripheral/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Added

- use `riscv-pac` result types for trait implementations

### Fixed

- `clippy` fixes
Expand Down
22 changes: 16 additions & 6 deletions riscv-peripheral/examples/e310x.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//! This is a simple example of how to use the `riscv-peripheral` crate to generate
//! peripheral definitions for a target.

use riscv_pac::result::{Error, Result};
use riscv_pac::{HartIdNumber, InterruptNumber, PriorityNumber};

#[repr(u16)]
Expand All @@ -19,9 +20,12 @@ unsafe impl HartIdNumber for HartId {
}

#[inline]
fn from_number(number: u16) -> Result<Self, u16> {
fn from_number(number: u16) -> Result<Self> {
if number > Self::MAX_HART_ID_NUMBER {
Err(number)
Err(Error::InvalidVariant {
field: "hart_id",
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding field to these errors feels unnecessary to me.

value: number as usize,
})
} else {
// SAFETY: valid context number
Ok(unsafe { core::mem::transmute(number) })
Expand Down Expand Up @@ -95,9 +99,12 @@ unsafe impl InterruptNumber for Interrupt {
}

#[inline]
fn from_number(number: u16) -> Result<Self, u16> {
fn from_number(number: u16) -> Result<Self> {
if number == 0 || number > Self::MAX_INTERRUPT_NUMBER {
Err(number)
Err(Error::InvalidVariant {
field: "interrupt",
value: number as usize,
})
} else {
// SAFETY: valid interrupt number
Ok(unsafe { core::mem::transmute(number) })
Expand Down Expand Up @@ -127,9 +134,12 @@ unsafe impl PriorityNumber for Priority {
}

#[inline]
fn from_number(number: u8) -> Result<Self, u8> {
fn from_number(number: u8) -> Result<Self> {
if number > Self::MAX_PRIORITY_NUMBER {
Err(number)
Err(Error::InvalidVariant {
field: "priority",
value: number as usize,
})
} else {
// SAFETY: valid priority number
Ok(unsafe { core::mem::transmute(number) })
Expand Down
16 changes: 13 additions & 3 deletions riscv-peripheral/src/aclint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ impl<C: Clint> CLINT<C> {
#[cfg(test)]
pub(crate) mod test {
use super::HartIdNumber;
use riscv_pac::result::{Error, Result};

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
#[repr(u16)]
Expand All @@ -81,9 +82,12 @@ pub(crate) mod test {
}

#[inline]
fn from_number(number: u16) -> Result<Self, u16> {
fn from_number(number: u16) -> Result<Self> {
if number > Self::MAX_HART_ID_NUMBER {
Err(number)
Err(Error::InvalidVariant {
field: "hart_id",
value: number as usize,
})
} else {
// SAFETY: valid context number
Ok(unsafe { core::mem::transmute(number) })
Expand All @@ -101,7 +105,13 @@ pub(crate) mod test {
assert_eq!(HartId::from_number(1), Ok(HartId::H1));
assert_eq!(HartId::from_number(2), Ok(HartId::H2));

assert_eq!(HartId::from_number(3), Err(3));
assert_eq!(
HartId::from_number(3),
Err(Error::InvalidVariant {
field: "hart_id",
value: 3
})
);
}

#[allow(dead_code)]
Expand Down
5 changes: 3 additions & 2 deletions riscv-peripheral/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
///
/// ```
/// use riscv_peripheral::clint_codegen;
/// use riscv_pac::result::{Error, Result};
///
/// /// HART IDs for the target CLINT peripheral
/// #[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand All @@ -40,9 +41,9 @@
/// unsafe impl riscv_peripheral::aclint::HartIdNumber for HartId {
/// const MAX_HART_ID_NUMBER: u16 = 2;
/// fn number(self) -> u16 { self as _ }
/// fn from_number(number: u16) -> Result<Self, u16> {
/// fn from_number(number: u16) -> Result<Self> {
/// if number > Self::MAX_HART_ID_NUMBER {
/// Err(number)
/// Err(Error::InvalidVariant { field: "hart_id", value: number as usize })
/// } else {
/// // SAFETY: valid context number
/// Ok(unsafe { core::mem::transmute(number) })
Expand Down
57 changes: 46 additions & 11 deletions riscv-peripheral/src/plic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ pub mod pendings;
pub mod priorities;
pub mod threshold;

pub use riscv_pac::{HartIdNumber, InterruptNumber, PriorityNumber}; // re-export useful riscv-pac traits
// re-export useful riscv-pac traits
pub use riscv_pac::{HartIdNumber, InterruptNumber, PriorityNumber};

/// Trait for a PLIC peripheral.
///
Expand Down Expand Up @@ -145,6 +146,7 @@ impl<P: Plic> CTX<P> {
#[cfg(test)]
pub(crate) mod test {
use super::{HartIdNumber, InterruptNumber, PriorityNumber};
use riscv_pac::result::{Error, Result};

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
#[repr(u16)]
Expand Down Expand Up @@ -181,9 +183,12 @@ pub(crate) mod test {
}

#[inline]
fn from_number(number: u16) -> Result<Self, u16> {
fn from_number(number: u16) -> Result<Self> {
if number > Self::MAX_INTERRUPT_NUMBER || number == 0 {
Err(number)
Err(Error::InvalidVariant {
field: "interrupt",
value: number as usize,
})
} else {
// SAFETY: valid interrupt number
Ok(unsafe { core::mem::transmute(number) })
Expand All @@ -200,9 +205,12 @@ pub(crate) mod test {
}

#[inline]
fn from_number(number: u8) -> Result<Self, u8> {
fn from_number(number: u8) -> Result<Self> {
if number > Self::MAX_PRIORITY_NUMBER {
Err(number)
Err(Error::InvalidVariant {
field: "priority",
value: number as usize,
})
} else {
// SAFETY: valid priority number
Ok(unsafe { core::mem::transmute(number) })
Expand All @@ -219,9 +227,12 @@ pub(crate) mod test {
}

#[inline]
fn from_number(number: u16) -> Result<Self, u16> {
fn from_number(number: u16) -> Result<Self> {
if number > Self::MAX_HART_ID_NUMBER {
Err(number)
Err(Error::InvalidVariant {
field: "context",
value: number as usize,
})
} else {
// SAFETY: valid context number
Ok(unsafe { core::mem::transmute(number) })
Expand All @@ -241,8 +252,20 @@ pub(crate) mod test {
assert_eq!(Interrupt::from_number(3), Ok(Interrupt::I3));
assert_eq!(Interrupt::from_number(4), Ok(Interrupt::I4));

assert_eq!(Interrupt::from_number(0), Err(0));
assert_eq!(Interrupt::from_number(5), Err(5));
assert_eq!(
Interrupt::from_number(0),
Err(Error::InvalidVariant {
field: "interrupt",
value: 0
})
);
assert_eq!(
Interrupt::from_number(5),
Err(Error::InvalidVariant {
field: "interrupt",
value: 5
})
);
}

#[test]
Expand All @@ -257,7 +280,13 @@ pub(crate) mod test {
assert_eq!(Priority::from_number(2), Ok(Priority::P2));
assert_eq!(Priority::from_number(3), Ok(Priority::P3));

assert_eq!(Priority::from_number(4), Err(4));
assert_eq!(
Priority::from_number(4),
Err(Error::InvalidVariant {
field: "priority",
value: 4
})
);
}

#[test]
Expand All @@ -270,7 +299,13 @@ pub(crate) mod test {
assert_eq!(Context::from_number(1), Ok(Context::C1));
assert_eq!(Context::from_number(2), Ok(Context::C2));

assert_eq!(Context::from_number(3), Err(3));
assert_eq!(
Context::from_number(3),
Err(Error::InvalidVariant {
field: "context",
value: 3
})
);
}

#[allow(dead_code)]
Expand Down
1 change: 1 addition & 0 deletions riscv/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Add `riscv::register::mcountinhibit` module for `mcountinhibit` CSR
- Add `Mcounteren` in-memory update functions
- Add `Mstatus` vector extension support
- Add fallible counterparts to all functions that `panic`

### Fixed

Expand Down
1 change: 1 addition & 0 deletions riscv/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ critical-section-single-hart = ["critical-section/restore-state-bool"]
[dependencies]
critical-section = "1.1.2"
embedded-hal = "1.0.0"
riscv-pac = { path = "../riscv-pac", version = "0.1.1", default-features = false }
1 change: 1 addition & 0 deletions riscv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub(crate) mod bits;
pub mod delay;
pub mod interrupt;
pub mod register;
pub use riscv_pac::*;

#[macro_use]
mod macros;
Expand Down
Loading
Loading