fixup: apply review suggestions

Author: 0xllx0

riscv/src/register/mvien.rs

@@ -1,6 +1,7 @@
 //! mvien register
 
 use crate::bits::{bf_extract, bf_insert};
+use riscv_pac::result::{Error, Result};
 use riscv_pac::InterruptNumber;
 
 #[cfg(target_arch = "riscv32")]
@@ -49,29 +50,45 @@ impl Mvien {
     }
 
     /// Check if a specific core interrupt source is enabled.
+    ///
+    /// Returns `Error` if the interrupt number is invalid.
     #[inline]
-    pub fn is_enabled<I: InterruptNumber>(&self, interrupt: I) -> bool {
+    pub fn is_enabled<I: InterruptNumber>(&self, interrupt: I) -> Result<bool> {
         let n = interrupt.number();
-        Self::is_valid_interrupt(n) && bf_extract(self.bits, n, 1) != 0
+        if Self::is_valid_interrupt(n) {
+            Ok(bf_extract(self.bits, n, 1) != 0)
+        } else {
+            Err(Error::InvalidVariant(n))
+        }
     }
 
     /// Enable a specific core interrupt source.
+    ///
+    /// Returns `Error` if the interrupt number is invalid.
     #[inline]
-    pub fn enable<I: InterruptNumber>(&mut self, interrupt: I) {
+    pub fn enable<I: InterruptNumber>(&mut self, interrupt: I) -> Result<()> {
         let n = interrupt.number();
 
         if Self::is_valid_interrupt(n) {
             self.bits = bf_insert(self.bits, n, 1, 1);
+            Ok(())
+        } else {
+            Err(Error::InvalidVariant(n))
         }
     }
 
     /// Disable a specific core interrupt source.
+    ///
+    /// Returns `Error` if the interrupt number is invalid.
     #[inline]
-    pub fn disable<I: InterruptNumber>(&mut self, interrupt: I) {
+    pub fn disable<I: InterruptNumber>(&mut self, interrupt: I) -> Result<()> {
         let n = interrupt.number();
 
         if Self::is_valid_interrupt(n) {
             self.bits = bf_insert(self.bits, n, 1, 0);
+            Ok(())
+        } else {
+            Err(Error::InvalidVariant(n))
         }
     }
 }
@@ -94,15 +111,14 @@ read_composite_csr!(super::mvienh::read().bits(), read().bits());
 #[cfg(test)]
 mod tests {
     use super::*;
-    use riscv_pac::result::{Error, Result};
 
     /// Represents a custom set of virtual interrupts.
     ///
     /// NOTE: a real implementation may want to enumerate the valid virtual interrupt variants.
     #[derive(Clone, Copy, Debug, Eq, PartialEq)]
     pub struct VirtualInterrupt(usize);
 
-    /// SAFETY: `Interrupt` represents the standard RISC-V interrupts
+    /// SAFETY: `VirtualInterrupt` represents the virtual RISC-V interrupts
     unsafe impl InterruptNumber for VirtualInterrupt {
         const MAX_INTERRUPT_NUMBER: usize = Mvien::MAX_INTERRUPT;
 
@@ -132,13 +148,13 @@ mod tests {
         (0..=VirtualInterrupt::MAX_INTERRUPT_NUMBER)
             .filter_map(|n| VirtualInterrupt::from_number(n).ok())
             .for_each(|int| {
-                assert!(!m.is_enabled(int));
+                assert_eq!(m.is_enabled(int), Ok(false));
 
-                m.enable(int);
-                assert!(m.is_enabled(int));
+                assert!(m.enable(int).is_ok());
+                assert_eq!(m.is_enabled(int), Ok(true));
 
-                m.disable(int);
-                assert!(!m.is_enabled(int));
+                assert!(m.disable(int).is_ok());
+                assert_eq!(m.is_enabled(int), Ok(false));
             });
     }
 }

riscv/src/register/mvienh.rs

@@ -1,6 +1,7 @@
 //! mvienh register
 
 use crate::bits::{bf_extract, bf_insert};
+use riscv_pac::result::{Error, Result};
 use riscv_pac::InterruptNumber;
 
 read_write_csr! {
@@ -33,45 +34,57 @@ impl Mvienh {
     }
 
     /// Check if a specific core interrupt source is enabled.
+    ///
+    /// Returns `Error` if the interrupt number is invalid.
     #[inline]
-    pub fn is_enabled<I: InterruptNumber>(&self, interrupt: I) -> bool {
+    pub fn is_enabled<I: InterruptNumber>(&self, interrupt: I) -> Result<bool> {
         let n = interrupt.number();
-        Self::is_valid_interrupt(n) && bf_extract(self.bits, Self::shift_interrupt(n), 1) != 0
+
+        if Self::is_valid_interrupt(n) {
+            Ok(bf_extract(self.bits, Self::shift_interrupt(n), 1) != 0)
+        } else {
+            Err(Error::InvalidVariant(n))
+        }
     }
 
     /// Enable a specific core interrupt source.
     #[inline]
-    pub fn enable<I: InterruptNumber>(&mut self, interrupt: I) {
+    pub fn enable<I: InterruptNumber>(&mut self, interrupt: I) -> Result<()> {
         let n = interrupt.number();
 
         if Self::is_valid_interrupt(n) {
             self.bits = bf_insert(self.bits, Self::shift_interrupt(n), 1, 1);
+            Ok(())
+        } else {
+            Err(Error::InvalidVariant(n))
         }
     }
 
     /// Disable a specific core interrupt source.
     #[inline]
-    pub fn disable<I: InterruptNumber>(&mut self, interrupt: I) {
+    pub fn disable<I: InterruptNumber>(&mut self, interrupt: I) -> Result<()> {
         let n = interrupt.number();
 
         if Self::is_valid_interrupt(n) {
             self.bits = bf_insert(self.bits, Self::shift_interrupt(n), 1, 0);
+            Ok(())
+        } else {
+            Err(Error::InvalidVariant(n))
         }
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use riscv_pac::result::{Error, Result};
 
     /// Represents a custom set of virtual interrupts.
     ///
     /// NOTE: a real implementation may want to enumerate the valid virtual interrupt variants.
     #[derive(Clone, Copy, Debug, Eq, PartialEq)]
     pub struct VirtualInterrupt(usize);
 
-    /// SAFETY: `Interrupt` represents the standard RISC-V interrupts
+    /// SAFETY: `VirtualInterrupt` represents the virtual RISC-V interrupts
     unsafe impl InterruptNumber for VirtualInterrupt {
         const MAX_INTERRUPT_NUMBER: usize = Mvienh::MAX_INTERRUPT;
 
@@ -97,13 +110,13 @@ mod tests {
         (Mvienh::MIN_INTERRUPT..=Mvienh::MAX_INTERRUPT)
             .filter_map(|n| VirtualInterrupt::from_number(n).ok())
             .for_each(|int| {
-                assert!(!m.is_enabled(int));
+                assert_eq!(m.is_enabled(int), Ok(false));
 
-                m.enable(int);
-                assert!(m.is_enabled(int));
+                assert!(m.enable(int).is_ok());
+                assert_eq!(m.is_enabled(int), Ok(true));
 
-                m.disable(int);
-                assert!(!m.is_enabled(int));
+                assert!(m.disable(int).is_ok());
+                assert_eq!(m.is_enabled(int), Ok(false));
             });
     }
 }