Fix warnings & clean up (#2961)

* Fix precedence warnings * Clean up * Try to catch documentation warnings * Deny rustdoc lints, fix issues * Check documentation for wifi and embassy * Fix h2
esp-rs · Jan 15, 2025 · 7ea1296 · 7ea1296
1 parent 04f62ec
commit 7ea1296
Show file tree

Hide file tree

Showing 28 changed files with 189 additions and 232 deletions.
diff --git a/.github/actions/check-esp-hal/action.yml b/.github/actions/check-esp-hal/action.yml
@@ -35,7 +35,7 @@ runs:
       run: cargo $LINTING_TOOLCHAIN xtask run-doc-test esp-hal ${{ inputs.device }}
     - name: Check documentation
       shell: bash
-      run: cargo $LINTING_TOOLCHAIN xtask build-documentation --packages esp-hal --chips ${{ inputs.device }}
+      run: cargo $LINTING_TOOLCHAIN xtask build-documentation --packages esp-hal --packages esp-wifi --packages esp-hal-embassy --chips ${{ inputs.device }}
     # Build all supported examples for the low-power core first (if present):
     - name: Build prerequisite examples (esp-lp-hal)
       shell: bash

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -24,7 +24,6 @@ env:
   CARGO_TERM_COLOR: always
   GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
   MSRV: "1.84.0"
-  RUSTDOCFLAGS: -Dwarnings
   DEFMT_LOG: trace
 
 # Cancel any currently running workflows from the same PR, branch, or

diff --git a/esp-alloc/src/lib.rs b/esp-alloc/src/lib.rs
@@ -166,7 +166,7 @@ impl Display for RegionStats {
 
 #[cfg(feature = "defmt")]
 impl defmt::Format for RegionStats {
-    fn format(&self, fmt: defmt::Formatter) {
+    fn format(&self, fmt: defmt::Formatter<'_>) {
         let usage_percent = self.used * 100 / self.size;
 
         if self.capabilities.contains(MemoryCapability::Internal) {
@@ -287,7 +287,7 @@ impl Display for HeapStats {
 
 #[cfg(feature = "defmt")]
 impl defmt::Format for HeapStats {
-    fn format(&self, fmt: defmt::Formatter) {
+    fn format(&self, fmt: defmt::Formatter<'_>) {
         defmt::write!(fmt, "HEAP INFO\n");
         defmt::write!(fmt, "Size: {}\n", self.size);
         defmt::write!(fmt, "Current usage: {}\n", self.current_usage);

diff --git a/esp-config/src/lib.rs b/esp-config/src/lib.rs
@@ -3,7 +3,7 @@
 #![doc = document_features::document_features!(feature_label = r#"<span class="stab portability"><code>{feature}</code></span>"#)]
 #![doc(html_logo_url = "https://avatars.githubusercontent.com/u/46717278")]
 #![cfg_attr(not(feature = "build"), no_std)]
-#![deny(missing_docs, rust_2018_idioms)]
+#![deny(missing_docs, rust_2018_idioms, rustdoc::all)]
 
 #[cfg(feature = "build")]
 mod generate;

diff --git a/esp-hal-embassy/src/lib.rs b/esp-hal-embassy/src/lib.rs
@@ -45,7 +45,7 @@
 //! ## Feature Flags
 #![doc = document_features::document_features!()]
 #![doc(html_logo_url = "https://avatars.githubusercontent.com/u/46717278")]
-#![deny(missing_docs)]
+#![deny(missing_docs, rust_2018_idioms, rustdoc::all)]
 #![cfg_attr(xtensa, feature(asm_experimental_arch))]
 #![no_std]
 

diff --git a/esp-hal/src/clock/clocks_ll/esp32c6.rs b/esp-hal/src/clock/clocks_ll/esp32c6.rs
@@ -257,7 +257,6 @@ const REGI2C_RTC_SLAVE_ID_V: u8 = 0xFF;
 const REGI2C_RTC_SLAVE_ID_S: u8 = 0;
 const REGI2C_RTC_ADDR_V: u8 = 0xFF;
 const REGI2C_RTC_ADDR_S: u8 = 8;
-const REGI2C_RTC_WR_CNTL_V: u8 = 0x1;
 const REGI2C_RTC_WR_CNTL_S: u8 = 24;
 const REGI2C_RTC_DATA_V: u8 = 0xFF;
 const REGI2C_RTC_DATA_S: u8 = 16;
@@ -287,94 +286,55 @@ fn regi2c_enable_block(block: u8) {
         .modify(|_, w| w.lp_i2c_ana_mast_i2c_mat_clk_en().set_bit());
 
     // Before config I2C register, enable corresponding slave.
+    let en_bit = match block {
+        v if v == REGI2C_BBPLL => REGI2C_BBPLL_DEVICE_EN,
+        v if v == REGI2C_BIAS => REGI2C_BIAS_DEVICE_EN,
+        v if v == REGI2C_DIG_REG => REGI2C_DIG_REG_DEVICE_EN,
+        v if v == REGI2C_ULP_CAL => REGI2C_ULP_CAL_DEVICE_EN,
+        v if v == REGI2C_SAR_I2C => REGI2C_SAR_I2C_DEVICE_EN,
+        _ => return,
+    };
+
     unsafe {
-        match block {
-            REGI2C_BBPLL => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() | REGI2C_BBPLL_DEVICE_EN)
-                });
-            }
-            REGI2C_BIAS => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() | REGI2C_BIAS_DEVICE_EN)
-                });
-            }
-            REGI2C_DIG_REG => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() | REGI2C_DIG_REG_DEVICE_EN)
-                });
-            }
-            REGI2C_ULP_CAL => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() | REGI2C_ULP_CAL_DEVICE_EN)
-                });
-            }
-            REGI2C_SAR_I2C => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() | REGI2C_SAR_I2C_DEVICE_EN)
-                });
-            }
-            _ => (),
-        }
+        lp_i2c_ana.device_en().modify(|r, w| {
+            w.lp_i2c_ana_mast_i2c_device_en()
+                .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() | en_bit)
+        });
     }
 }
 
 fn regi2c_disable_block(block: u8) {
+    let en_bit = match block {
+        v if v == REGI2C_BBPLL => REGI2C_BBPLL_DEVICE_EN,
+        v if v == REGI2C_BIAS => REGI2C_BIAS_DEVICE_EN,
+        v if v == REGI2C_DIG_REG => REGI2C_DIG_REG_DEVICE_EN,
+        v if v == REGI2C_ULP_CAL => REGI2C_ULP_CAL_DEVICE_EN,
+        v if v == REGI2C_SAR_I2C => REGI2C_SAR_I2C_DEVICE_EN,
+        _ => return,
+    };
+
     unsafe {
         let lp_i2c_ana = &*crate::peripherals::LP_I2C_ANA_MST::ptr();
-        match block {
-            REGI2C_BBPLL => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() & !REGI2C_BBPLL_DEVICE_EN)
-                });
-            }
-            REGI2C_BIAS => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() & !REGI2C_BIAS_DEVICE_EN)
-                });
-            }
-            REGI2C_DIG_REG => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() & !REGI2C_DIG_REG_DEVICE_EN)
-                });
-            }
-            REGI2C_ULP_CAL => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() & !REGI2C_ULP_CAL_DEVICE_EN)
-                });
-            }
-            REGI2C_SAR_I2C => {
-                lp_i2c_ana.device_en().modify(|r, w| {
-                    w.lp_i2c_ana_mast_i2c_device_en()
-                        .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() & !REGI2C_SAR_I2C_DEVICE_EN)
-                });
-            }
-            _ => (),
-        }
+        lp_i2c_ana.device_en().modify(|r, w| {
+            w.lp_i2c_ana_mast_i2c_device_en()
+                .bits(r.lp_i2c_ana_mast_i2c_device_en().bits() & !en_bit)
+        });
     }
 }
 
 pub(crate) fn regi2c_write(block: u8, _host_id: u8, reg_add: u8, data: u8) {
     regi2c_enable_block(block);
     let lp_i2c_ana = unsafe { &*crate::peripherals::LP_I2C_ANA_MST::ptr() };
 
-    let temp: u32 = ((block as u32 & REGI2C_RTC_SLAVE_ID_V as u32) << REGI2C_RTC_SLAVE_ID_S as u32)
-                    | ((reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S as u32)
-                    | ((0x1 & REGI2C_RTC_WR_CNTL_V as u32) << REGI2C_RTC_WR_CNTL_S as u32) // 0: READ I2C register; 1: Write I2C register;
-                    | (((data as u32) & REGI2C_RTC_DATA_V as u32) << REGI2C_RTC_DATA_S as u32);
+    let block_shifted = (block as u32 & REGI2C_RTC_SLAVE_ID_V as u32) << REGI2C_RTC_SLAVE_ID_S;
+    let reg_add_shifted = (reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S;
+    let write_bit = 1u32 << REGI2C_RTC_WR_CNTL_S;
 
-    unsafe {
-        lp_i2c_ana.i2c0_ctrl().modify(|_, w| w.bits(temp));
-    }
+    let new_value = (data as u32) << REGI2C_RTC_DATA_S;
+
+    lp_i2c_ana
+        .i2c0_ctrl()
+        .write(|w| unsafe { w.bits(block_shifted | reg_add_shifted | write_bit | new_value) });
 
     while lp_i2c_ana
         .i2c0_ctrl()
@@ -387,20 +347,20 @@ pub(crate) fn regi2c_write(block: u8, _host_id: u8, reg_add: u8, data: u8) {
 }
 
 pub(crate) fn regi2c_write_mask(block: u8, _host_id: u8, reg_add: u8, msb: u8, lsb: u8, data: u8) {
-    assert!(msb - lsb < 8);
+    assert!(msb < 8 + lsb);
     let lp_i2c_ana = unsafe { &*crate::peripherals::LP_I2C_ANA_MST::ptr() };
+    regi2c_enable_block(block);
 
-    unsafe {
-        regi2c_enable_block(block);
+    let block_shifted = (block as u32 & REGI2C_RTC_SLAVE_ID_V as u32) << REGI2C_RTC_SLAVE_ID_S;
+    let reg_add_shifted = (reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S;
+    let write_bit = 1u32 << REGI2C_RTC_WR_CNTL_S;
 
+    unsafe {
         // Read the i2c bus register
-        let mut temp: u32 = ((block as u32 & REGI2C_RTC_SLAVE_ID_V as u32)
-            << REGI2C_RTC_SLAVE_ID_S as u32)
-            | (reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S as u32;
-
-        lp_i2c_ana
-            .i2c0_ctrl()
-            .modify(|_, w| w.lp_i2c_ana_mast_i2c0_ctrl().bits(temp));
+        lp_i2c_ana.i2c0_ctrl().write(|w| {
+            w.lp_i2c_ana_mast_i2c0_ctrl()
+                .bits(block_shifted | reg_add_shifted)
+        });
 
         while lp_i2c_ana
             .i2c0_ctrl()
@@ -409,23 +369,25 @@ pub(crate) fn regi2c_write_mask(block: u8, _host_id: u8, reg_add: u8, msb: u8, l
             .bit()
         {}
 
-        temp = lp_i2c_ana
+        let mut temp = lp_i2c_ana
             .i2c0_data()
             .read()
             .lp_i2c_ana_mast_i2c0_rdata()
             .bits() as u32;
 
-        // Write the i2c bus register
+        // Mask the value field
         temp &= (!(0xFFFFFFFF << lsb)) | (0xFFFFFFFF << (msb + 1));
-        temp |= (data as u32 & (!(0xFFFFFFFF << (msb as u32 - lsb as u32 + 1)))) << (lsb as u32);
-        temp = ((block as u32 & REGI2C_RTC_SLAVE_ID_V as u32) << REGI2C_RTC_SLAVE_ID_S as u32)
-            | ((reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S as u32)
-            | ((0x1 & REGI2C_RTC_WR_CNTL_V as u32) << REGI2C_RTC_WR_CNTL_S as u32)
-            | ((temp & REGI2C_RTC_DATA_V as u32) << REGI2C_RTC_DATA_S as u32);
 
-        lp_i2c_ana
-            .i2c0_ctrl()
-            .modify(|_, w| w.lp_i2c_ana_mast_i2c0_ctrl().bits(temp));
+        // Write the value into the temporary
+        temp |= (data as u32 & (!(0xFFFFFFFF << (msb - lsb + 1)))) << lsb;
+
+        // Write the i2c bus register
+        let new_value = (temp & REGI2C_RTC_DATA_V as u32) << REGI2C_RTC_DATA_S;
+
+        lp_i2c_ana.i2c0_ctrl().write(|w| {
+            w.lp_i2c_ana_mast_i2c0_ctrl()
+                .bits(block_shifted | reg_add_shifted | write_bit | new_value)
+        });
 
         while lp_i2c_ana
             .i2c0_ctrl()

diff --git a/esp-hal/src/clock/clocks_ll/esp32h2.rs b/esp-hal/src/clock/clocks_ll/esp32h2.rs
@@ -51,7 +51,6 @@ const REGI2C_RTC_SLAVE_ID_V: u8 = 0xFF;
 const REGI2C_RTC_SLAVE_ID_S: u8 = 0;
 const REGI2C_RTC_ADDR_V: u8 = 0xFF;
 const REGI2C_RTC_ADDR_S: u8 = 8;
-const REGI2C_RTC_WR_CNTL_V: u8 = 0x1;
 const REGI2C_RTC_WR_CNTL_S: u8 = 24;
 const REGI2C_RTC_DATA_V: u8 = 0xFF;
 const REGI2C_RTC_DATA_S: u8 = 16;
@@ -241,45 +240,28 @@ fn regi2c_enable_block(block: u8) {
         .modify(|_, w| w.clk_i2c_mst_en().set_bit());
 
     // Before config I2C register, enable corresponding slave.
-    match block {
-        v if v == REGI2C_BBPLL => {
-            reg_set_bit(I2C_MST_ANA_CONF1_REG, REGI2C_BBPLL_RD_MASK);
-        }
-        v if v == REGI2C_BIAS => {
-            reg_set_bit(I2C_MST_ANA_CONF1_REG, REGI2C_BIAS_RD_MASK);
-        }
-        v if v == REGI2C_PMU => {
-            reg_set_bit(I2C_MST_ANA_CONF1_REG, REGI2C_DIG_REG_RD_MASK);
-        }
-        v if v == REGI2C_ULP_CAL => {
-            reg_set_bit(I2C_MST_ANA_CONF1_REG, REGI2C_ULP_CAL_RD_MASK);
-        }
-        v if v == REGI2C_SAR_I2C => {
-            reg_set_bit(I2C_MST_ANA_CONF1_REG, REGI2C_SAR_I2C_RD_MASK);
-        }
-        _ => (),
-    }
+
+    let en_mask = match block {
+        v if v == REGI2C_BBPLL => REGI2C_BBPLL_RD_MASK,
+        v if v == REGI2C_BIAS => REGI2C_BIAS_RD_MASK,
+        v if v == REGI2C_PMU => REGI2C_DIG_REG_RD_MASK,
+        v if v == REGI2C_ULP_CAL => REGI2C_ULP_CAL_RD_MASK,
+        v if v == REGI2C_SAR_I2C => REGI2C_SAR_I2C_RD_MASK,
+        _ => return,
+    };
+    reg_set_bit(I2C_MST_ANA_CONF1_REG, en_mask)
 }
 
 fn regi2c_disable_block(block: u8) {
-    match block {
-        v if v == REGI2C_BBPLL => {
-            reg_clr_bit(I2C_MST_ANA_CONF1_REG, REGI2C_BBPLL_RD_MASK);
-        }
-        v if v == REGI2C_BIAS => {
-            reg_clr_bit(I2C_MST_ANA_CONF1_REG, REGI2C_BIAS_RD_MASK);
-        }
-        v if v == REGI2C_PMU => {
-            reg_clr_bit(I2C_MST_ANA_CONF1_REG, REGI2C_DIG_REG_RD_MASK);
-        }
-        v if v == REGI2C_ULP_CAL => {
-            reg_clr_bit(I2C_MST_ANA_CONF1_REG, REGI2C_ULP_CAL_RD_MASK);
-        }
-        v if v == REGI2C_SAR_I2C => {
-            reg_clr_bit(I2C_MST_ANA_CONF1_REG, REGI2C_SAR_I2C_RD_MASK);
-        }
-        _ => (),
-    }
+    let en_mask = match block {
+        v if v == REGI2C_BBPLL => REGI2C_BBPLL_RD_MASK,
+        v if v == REGI2C_BIAS => REGI2C_BIAS_RD_MASK,
+        v if v == REGI2C_PMU => REGI2C_DIG_REG_RD_MASK,
+        v if v == REGI2C_ULP_CAL => REGI2C_ULP_CAL_RD_MASK,
+        v if v == REGI2C_SAR_I2C => REGI2C_SAR_I2C_RD_MASK,
+        _ => return,
+    };
+    reg_clr_bit(I2C_MST_ANA_CONF1_REG, en_mask)
 }
 
 fn reg_set_bit(reg: u32, bit: u32) {
@@ -309,30 +291,35 @@ fn reg_get_field(reg: u32, s: u32, v: u32) -> u32 {
 }
 
 pub(crate) fn regi2c_write_mask(block: u8, _host_id: u8, reg_add: u8, msb: u8, lsb: u8, data: u8) {
-    assert!(msb - lsb < 8);
+    assert!(msb < 8 + lsb);
     regi2c_enable_block(block);
 
+    let block_shifted = (block as u32 & REGI2C_RTC_SLAVE_ID_V as u32) << REGI2C_RTC_SLAVE_ID_S;
+    let reg_add_shifted = (reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S;
+    let write_bit = 1u32 << REGI2C_RTC_WR_CNTL_S;
+
     // Read the i2c bus register
     while reg_get_bit(I2C_MST_I2C0_CTRL_REG, REGI2C_RTC_BUSY) != 0 {}
 
-    let mut temp: u32 = ((block as u32 & REGI2C_RTC_SLAVE_ID_V as u32)
-        << REGI2C_RTC_SLAVE_ID_S as u32)
-        | (reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S as u32;
-    reg_write(I2C_MST_I2C0_CTRL_REG, temp);
+    reg_write(I2C_MST_I2C0_CTRL_REG, block_shifted | reg_add_shifted);
     while reg_get_bit(I2C_MST_I2C0_CTRL_REG, REGI2C_RTC_BUSY) != 0 {}
-    temp = reg_get_field(
+    let mut temp = reg_get_field(
         I2C_MST_I2C0_CTRL_REG,
         REGI2C_RTC_DATA_S as u32,
         REGI2C_RTC_DATA_V as u32,
     );
-    // Write the i2c bus register
+
+    // Mask the value field
     temp &= (!(0xFFFFFFFF << lsb)) | (0xFFFFFFFF << (msb + 1));
-    temp |= (data as u32 & (!(0xFFFFFFFF << (msb as u32 - lsb as u32 + 1)))) << (lsb as u32);
-    temp = ((block as u32 & REGI2C_RTC_SLAVE_ID_V as u32) << REGI2C_RTC_SLAVE_ID_S as u32)
-        | ((reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S as u32)
-        | ((0x1 & REGI2C_RTC_WR_CNTL_V as u32) << REGI2C_RTC_WR_CNTL_S as u32)
-        | ((temp & REGI2C_RTC_DATA_V as u32) << REGI2C_RTC_DATA_S as u32);
-    reg_write(I2C_MST_I2C0_CTRL_REG, temp);
+
+    // Write the value into the temporary
+    temp |= (data as u32 & (!(0xFFFFFFFF << (msb - lsb + 1)))) << lsb;
+
+    let new_value = (temp & REGI2C_RTC_DATA_V as u32) << REGI2C_RTC_DATA_S;
+    reg_write(
+        I2C_MST_I2C0_CTRL_REG,
+        block_shifted | reg_add_shifted | write_bit | new_value,
+    );
     while reg_get_bit(I2C_MST_I2C0_CTRL_REG, REGI2C_RTC_BUSY) != 0 {}
 
     regi2c_disable_block(block);

diff --git a/esp-hal/src/gpio/etm.rs b/esp-hal/src/gpio/etm.rs
@@ -344,7 +344,9 @@ fn enable_task_channel(channel: u8, pin: u8) {
     // bit 0 = en, bit 1-3 = channel
     unsafe {
         ptr.write_volatile(
-            ptr.read_volatile() & !(0xf << shift) | 1 << shift | (channel as u32) << (shift + 1),
+            ptr.read_volatile() & !(0xf << shift)
+                | (1 << shift)
+                | ((channel as u32) << (shift + 1)),
         );
     }
 }