Merge pull request #36 from riscv-rust/clippy

Apply clippy changes

.github/workflows/clippy.yaml

@@ -0,0 +1,43 @@
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+  merge_group:
+
+name: Lints compliance check
+
+env:
+  # Bypass clippy warnings produced by Svd2Rust
+  CLIPPY_PARAMS: -W clippy::all -D warnings -A clippy::module_inception -A clippy::needless_lifetimes
+
+jobs:
+  clippy:
+    strategy:
+      matrix:
+        board: [hifive1, hifive1-revb, redv, lofive, lofive-r1]
+        toolchain: [ stable, nightly ]
+        include:
+          # Nightly is only for reference and allowed to fail
+          - toolchain: nightly
+            experimental: true
+    runs-on: ubuntu-latest
+    continue-on-error: ${{ matrix.experimental || false }}
+    steps:
+    - uses: actions/checkout@v4
+    - name: Update Rust toolchain and install Clippy
+      run: rustup update ${{ matrix.toolchain }} && rustup default ${{ matrix.toolchain }} && rustup component add clippy
+    - name: Install Rust target
+      run: rustup target install riscv32imc-unknown-none-elf
+    - name: Run clippy (direct mode)
+      run: cargo clippy --features board-${{ matrix.board }} -- $CLIPPY_PARAMS
+    - name: Run clippy (vectored mode)
+      run: cargo clippy --features virq,board-${{ matrix.board }} -- $CLIPPY_PARAMS
+
+   # Job to check that all the lint checks succeeded
+  clippy-check:
+    needs:
+    - clippy
+    runs-on: ubuntu-latest
+    if: always()
+    steps:
+      - run: jq --exit-status 'all(.result == "success")' <<< '${{ toJson(needs) }}'

e310x-hal/CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ## [Unreleased]
 
 ### Changed
+- Apply clippy changes
 - Use `portable-atomic` with `zaamo` feature to use native `amo*` operations.
 - Official target is now `riscv32imc-unknown-none-elf`, as it does not fully support the A extension.
 - Update `e310x` dependency and adapt code

e310x-hal/src/clock.rs

@@ -175,10 +175,10 @@ impl CoreClk {
     /// The resulting frequency may differ by 0-2% from the requested
     fn configure_pll(&self, pllref_freq: Hertz, divout_freq: Hertz) -> Hertz {
         let pllref_freq = pllref_freq.0;
-        assert!(PLLREF_MIN <= pllref_freq && pllref_freq <= PLLREF_MAX);
+        assert!((PLLREF_MIN..=PLLREF_MAX).contains(&pllref_freq));
 
         let divout_freq = divout_freq.0;
-        assert!(DIVOUT_MIN <= divout_freq && divout_freq <= DIVOUT_MAX);
+        assert!((DIVOUT_MIN..=DIVOUT_MAX).contains(&divout_freq));
 
         // Calculate PLL Output Divider settings
         let divider_div;
@@ -205,7 +205,7 @@ impl CoreClk {
             2 * (divider_div + 1)
         };
         let pllout_freq = divout_freq * d;
-        assert!(PLLOUT_MIN <= pllout_freq && pllout_freq <= PLLOUT_MAX);
+        assert!((PLLOUT_MIN..=PLLOUT_MAX).contains(&pllout_freq));
 
         // Calculate PLL R ratio
         let r = match pllref_freq {
@@ -218,7 +218,7 @@ impl CoreClk {
 
         // Calculate refr frequency
         let refr_freq = pllref_freq / r;
-        assert!(REFR_MIN <= refr_freq && refr_freq <= REFR_MAX);
+        assert!((REFR_MIN..=REFR_MAX).contains(&refr_freq));
 
         // Calculate PLL Q ratio
         let q = match pllout_freq {
@@ -230,7 +230,7 @@ impl CoreClk {
 
         // Calculate the desired vco frequency
         let target_vco_freq = pllout_freq * q;
-        assert!(VCO_MIN <= target_vco_freq && target_vco_freq <= VCO_MAX);
+        assert!((VCO_MIN..=VCO_MAX).contains(&target_vco_freq));
 
         // Calculate PLL F ratio
         let f = target_vco_freq / refr_freq;
@@ -249,15 +249,15 @@ impl CoreClk {
         } else {
             (f_lo, vco_lo)
         };
-        assert!(VCO_MIN <= vco_freq && vco_freq <= VCO_MAX);
+        assert!((VCO_MIN..=VCO_MAX).contains(&vco_freq));
 
         // Calculate actual pllout frequency
         let pllout_freq = vco_freq / q;
-        assert!(PLLOUT_MIN <= pllout_freq && pllout_freq <= PLLOUT_MAX);
+        assert!((PLLOUT_MIN..=PLLOUT_MAX).contains(&pllout_freq));
 
         // Calculate actual divout frequency
         let divout_freq = pllout_freq / d;
-        assert!(DIVOUT_MIN <= divout_freq && divout_freq <= DIVOUT_MAX);
+        assert!((DIVOUT_MIN..=DIVOUT_MAX).contains(&divout_freq));
 
         // Calculate bit-values
         let r: u8 = (r - 1) as u8;

e310x-hal/src/core/mod.rs

@@ -26,6 +26,10 @@ impl CorePeripherals {
     }
 
     /// Steal the peripherals
+    ///
+    /// # Safety
+    ///
+    /// Using this function may break the guarantees of the singleton pattern.
     pub unsafe fn steal() -> Self {
         let p = e310x::Peripherals::steal();
         Self::new(p.clint, p.plic)

e310x-hal/src/core/plic.rs

@@ -43,11 +43,11 @@ impl Priority {
     }
 }
 
-impl Into<u32> for Priority {
+impl From<Priority> for u32 {
     /// Returns the numeric priority for writing to a
     /// interrupt priority or the plic threshold register.
-    fn into(self) -> u32 {
-        match self {
+    fn from(val: Priority) -> Self {
+        match val {
             Priority::P0 => 0,
             Priority::P1 => 1,
             Priority::P2 => 2,

e310x-hal/src/delay.rs

@@ -6,6 +6,7 @@ use embedded_hal::blocking::delay::{DelayMs, DelayUs};
 use riscv::register::{mie, mip};
 
 /// Machine timer (mtime) as a busyloop delay provider
+#[derive(Default)]
 pub struct Delay;
 
 const TICKS_PER_SECOND: u64 = 32768;

e310x-hal/src/device.rs

@@ -173,7 +173,11 @@ impl DeviceResources {
         e310x::Peripherals::take().map(DeviceResources::from)
     }
 
-    /// Unchecked version of `DeviceResources::take`
+    /// Unchecked version of [`DeviceResources::take`].
+    ///
+    /// # Safety
+    ///
+    /// Using this function may break the guarantees of the singleton pattern.
     pub unsafe fn steal() -> Self {
         e310x::Peripherals::steal().into()
     }

e310x-hal/src/i2c.rs

@@ -19,12 +19,16 @@ use e310x::{i2c0, I2c0};
 use embedded_hal::blocking::i2c::{Read, Write, WriteRead};
 
 /// SDA pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait SdaPin<I2C> {}
-/// SCL pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait SclPin<I2C> {}
+mod sealed {
+    /// SDA pin
+    pub trait SdaPin<I2C> {}
 
-unsafe impl<T> SdaPin<I2c0> for gpio0::Pin12<IOF0<T>> {}
-unsafe impl<T> SclPin<I2c0> for gpio0::Pin13<IOF0<T>> {}
+    /// SCL pin
+    pub trait SclPin<I2C> {}
+}
+
+impl<T> sealed::SdaPin<I2c0> for gpio0::Pin12<IOF0<T>> {}
+impl<T> sealed::SclPin<I2c0> for gpio0::Pin13<IOF0<T>> {}
 
 /// I2C error
 #[derive(Debug, Eq, PartialEq)]
@@ -61,8 +65,8 @@ impl<SDA, SCL> I2c<I2c0, (SDA, SCL)> {
     /// Configures an I2C peripheral
     pub fn new(i2c: I2c0, sda: SDA, scl: SCL, speed: Speed, clocks: Clocks) -> Self
     where
-        SDA: SdaPin<I2c0>,
-        SCL: SclPin<I2c0>,
+        SDA: sealed::SdaPin<I2c0>,
+        SCL: sealed::SclPin<I2c0>,
     {
         // Calculate prescaler value
         let desired_speed = match speed {
@@ -115,13 +119,13 @@ impl<I2C: Deref<Target = i2c0::RegisterBlock>, PINS> I2c<I2C, PINS> {
     {
         self.i2c.cr().write(|w| unsafe {
             let mut value: u32 = 0;
-            f(mem::transmute(&mut value));
+            f(mem::transmute::<&mut u32, &mut i2c0::cr::W>(&mut value));
             w.bits(value)
         });
     }
 
     fn read_sr(&self) -> i2c0::sr::R {
-        unsafe { mem::transmute(self.i2c.sr().read()) }
+        self.i2c.sr().read()
     }
 
     fn write_byte(&self, byte: u8) {

e310x-hal/src/pmu.rs

@@ -132,7 +132,7 @@ pub trait PMUExt {
     ///
     /// Stores user data `UD` to backup registers.
     ///
-    /// # *WARNING*
+    /// # Safety
     ///
     /// `user_data` value must not contain un-serializable types such as pointers or references.
     ///
@@ -160,7 +160,7 @@ pub trait PMUExt {
     ///
     /// Restores user data `UD` from backup registers.
     ///
-    /// # *WARNING*
+    /// # Safety
     ///
     /// `user_data` value must not contain un-serializable types such as pointers or references.
     ///
@@ -269,9 +269,9 @@ impl PMUExt for Pmu {
         let ptr_u32 = ptr as *const u32;
         let sliced = core::slice::from_raw_parts(ptr_u32, reg_count);
 
-        for i in 0..sliced.len() {
-            backup.backup(i).write(|w| w.bits(sliced[i]));
-        }
+        backup.backup_iter().enumerate().for_each(|(i, backup_r)| {
+            backup_r.write(|w| w.bits(sliced[i]));
+        });
 
         Ok(())
     }
@@ -297,9 +297,9 @@ impl PMUExt for Pmu {
         let ptr_u32 = ptr as *mut u32;
         let sliced = core::slice::from_raw_parts_mut(ptr_u32, reg_count);
 
-        for i in 0..sliced.len() {
-            sliced[i] = backup.backup(i).read().bits();
-        }
+        backup.backup_iter().enumerate().for_each(|(i, backup_r)| {
+            sliced[i] = backup_r.read().bits();
+        });
 
         Ok(())
     }

e310x-hal/src/pwm.rs

@@ -91,6 +91,7 @@ mod pwm2_impl {
 }
 
 /// PWM channel
+#[derive(Copy, Clone)]
 pub struct Channel<PWM> {
     _pwm: PhantomData<PWM>,
     cmp_index: CmpIndex,
@@ -109,17 +110,6 @@ impl<PWM> Channel<PWM> {
     }
 }
 
-impl<PWM> Clone for Channel<PWM> {
-    fn clone(&self) -> Self {
-        Self {
-            _pwm: self._pwm.clone(),
-            cmp_index: self.cmp_index.clone(),
-        }
-    }
-}
-
-impl<PWM> Copy for Channel<PWM> {}
-
 #[doc(hidden)]
 pub trait PwmX: Deref<Target = pwm0::RegisterBlock> {
     type CmpWidth: Ord;

e310x-hal/src/serial.rs

@@ -26,22 +26,21 @@ use core::mem;
 #[allow(unused_imports)]
 use e310x::{uart0, Uart0, Uart1};
 
-// FIXME these should be "closed" traits
-/// TX pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait TxPin<UART> {}
+mod sealed {
+    /// TX pin
+    pub trait TxPin<UART> {}
 
-/// RX pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait RxPin<UART> {}
+    /// RX pin
+    pub trait RxPin<UART> {}
+}
 
-unsafe impl<T> TxPin<Uart0> for gpio0::Pin17<IOF0<T>> {}
-unsafe impl<T> RxPin<Uart0> for gpio0::Pin16<IOF0<T>> {}
+impl<T> sealed::TxPin<Uart0> for gpio0::Pin17<IOF0<T>> {}
+impl<T> sealed::RxPin<Uart0> for gpio0::Pin16<IOF0<T>> {}
 
 #[cfg(feature = "g002")]
-mod g002_ims {
-    use super::{gpio0, RxPin, TxPin, Uart1, IOF0};
-    unsafe impl<T> TxPin<Uart1> for gpio0::Pin18<IOF0<T>> {}
-    unsafe impl<T> RxPin<Uart1> for gpio0::Pin23<IOF0<T>> {}
-}
+impl<T> sealed::TxPin<Uart1> for gpio0::Pin18<IOF0<T>> {}
+#[cfg(feature = "g002")]
+impl<T> sealed::RxPin<Uart1> for gpio0::Pin23<IOF0<T>> {}
 
 #[doc(hidden)]
 pub trait UartX: Deref<Target = uart0::RegisterBlock> {}
@@ -68,8 +67,8 @@ impl<UART: UartX, TX, RX> Serial<UART, (TX, RX)> {
     /// Configures a UART peripheral to provide serial communication
     pub fn new(uart: UART, pins: (TX, RX), baud_rate: Bps, clocks: Clocks) -> Self
     where
-        TX: TxPin<UART>,
-        RX: RxPin<UART>,
+        TX: sealed::TxPin<UART>,
+        RX: sealed::RxPin<UART>,
     {
         let div = clocks.tlclk().0 / baud_rate.0 - 1;
         unsafe {
@@ -125,7 +124,7 @@ impl<UART: UartX> serial::Read<u8> for Rx<UART> {
         if rxdata.empty().bit_is_set() {
             Err(::nb::Error::WouldBlock)
         } else {
-            Ok(rxdata.data().bits() as u8)
+            Ok(rxdata.data().bits())
         }
     }
 }
@@ -162,8 +161,8 @@ impl<TX, RX> Serial<Uart0, (TX, RX)> {
     #[deprecated(note = "Please use Serial::new function instead")]
     pub fn uart0(uart: Uart0, pins: (TX, RX), baud_rate: Bps, clocks: Clocks) -> Self
     where
-        TX: TxPin<Uart0>,
-        RX: RxPin<Uart0>,
+        TX: sealed::TxPin<Uart0>,
+        RX: sealed::RxPin<Uart0>,
     {
         Self::new(uart, pins, baud_rate, clocks)
     }
@@ -175,8 +174,8 @@ impl<TX, RX> Serial<Uart1, (TX, RX)> {
     #[deprecated(note = "Please use Serial::new function instead")]
     pub fn uart1(uart: Uart1, pins: (TX, RX), baud_rate: Bps, clocks: Clocks) -> Self
     where
-        TX: TxPin<Uart1>,
-        RX: RxPin<Uart1>,
+        TX: sealed::TxPin<Uart1>,
+        RX: sealed::RxPin<Uart1>,
     {
         Self::new(uart, pins, baud_rate, clocks)
     }

e310x-hal/src/spi/bus.rs

@@ -210,10 +210,7 @@ where
         Ok(())
     }
 
-    pub(crate) fn exec<'op>(
-        &mut self,
-        operations: &mut [Operation<'op, u8>],
-    ) -> Result<(), Infallible> {
+    pub(crate) fn exec(&mut self, operations: &mut [Operation<'_, u8>]) -> Result<(), Infallible> {
         for op in operations {
             match op {
                 Operation::Transfer(words) => {

e310x-hal/src/spi/exclusive_device.rs

@@ -110,7 +110,7 @@ where
 {
     type Error = Infallible;
 
-    fn exec<'op>(&mut self, operations: &mut [Operation<'op, u8>]) -> Result<(), Infallible> {
+    fn exec(&mut self, operations: &mut [Operation<'_, u8>]) -> Result<(), Infallible> {
         self.bus.start_frame();
         let result = self.bus.exec(operations);
         self.bus.end_frame();

e310x-hal/src/spi/shared_device.rs

@@ -149,7 +149,7 @@ where
 {
     type Error = Infallible;
 
-    fn exec<'op>(&mut self, operations: &mut [Operation<'op, u8>]) -> Result<(), Infallible> {
+    fn exec(&mut self, operations: &mut [Operation<'_, u8>]) -> Result<(), Infallible> {
         interrupt::free(|| {
             let mut bus = self.bus.borrow_mut();