Skip to content

Commit

Permalink
CI check for clippy now actually works (and passes!) for Xtensa
Browse files Browse the repository at this point in the history
  • Loading branch information
jessebraham committed Feb 1, 2024
1 parent 43eeb85 commit 473e2d7
Show file tree
Hide file tree
Showing 23 changed files with 135 additions and 103 deletions.
13 changes: 10 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -740,16 +740,23 @@ jobs:
- uses: actions/checkout@v4
- uses: esp-rs/[email protected]
with:
default: true
ldproxy: false
- uses: Swatinem/rust-cache@v2

# Run 'cargo clippy' on all packages targeting Xtensa:
- name: clippy (esp32-hal)
run: cd esp32-hal && cargo clippy -- -D warnings
run: |
cargo clippy -Zbuild-std=core --manifest-path=esp-hal/Cargo.toml --target=xtensa-esp32-none-elf --features=esp32,xtal-40mhz \
-- -D warnings -A clippy::too_many_arguments -A clippy::type_complexity
- name: clippy (esp32s2-hal)
run: cd esp32s2-hal && cargo clippy -- -D warnings
run: |
cargo clippy -Zbuild-std=core --manifest-path=esp-hal/Cargo.toml --target=xtensa-esp32s2-none-elf --features=esp32s2 \
-- -D warnings -A clippy::too_many_arguments -A clippy::type_complexity
- name: clippy (esp32s3-hal)
run: cd esp32s3-hal && cargo clippy -- -D warnings
run: |
cargo clippy -Zbuild-std=core --manifest-path=esp-hal/Cargo.toml --target=xtensa-esp32s3-none-elf --features=esp32s3 \
-- -D warnings -A clippy::too_many_arguments -A clippy::type_complexity
rustfmt:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/analog/adc/esp32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ macro_rules! impl_adc_interface {
}
}

mod implementation {
mod adc_implementation {
use crate::peripherals::{ADC1, ADC2};

impl_adc_interface! {
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/analog/adc/xtensa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ macro_rules! impl_adc_interface {
}
}

mod implementation {
mod adc_implementation {
use crate::peripherals::{ADC1, ADC2};

impl_adc_interface! {
Expand Down
7 changes: 3 additions & 4 deletions esp-hal/src/clock/clocks_ll/esp32c6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ pub(crate) fn esp32c6_rtc_freq_to_pll_mhz_raw(cpu_clock_speed_mhz: u32) {
pcr.cpu_freq_conf()
.modify(|_, w| w.cpu_hs_120m_force().clear_bit());

CpuClockSource::PLL.select();
CpuClockSource::Pll.select();

ets_update_cpu_frequency(cpu_clock_speed_mhz);
}
Expand Down Expand Up @@ -378,7 +378,7 @@ const REGI2C_RTC_WR_CNTL_S: u8 = 24;
const REGI2C_RTC_DATA_V: u8 = 0xFF;
const REGI2C_RTC_DATA_S: u8 = 16;

const LP_I2C_ANA_MST_I2C0_CTRL_REG: u32 = DR_REG_LP_I2C_ANA_MST_BASE + 0x0;
const LP_I2C_ANA_MST_I2C0_CTRL_REG: u32 = DR_REG_LP_I2C_ANA_MST_BASE;
const LP_I2C_ANA_MST_I2C0_BUSY: u32 = 1 << 25;

const LP_I2C_ANA_MST_I2C0_DATA_REG: u32 = DR_REG_LP_I2C_ANA_MST_BASE + 0x8;
Expand Down Expand Up @@ -463,8 +463,7 @@ pub(crate) fn regi2c_write_mask(block: u8, _host_id: u8, reg_add: u8, msb: u8, l
);
// Write the i2c bus register
temp &= (!(0xFFFFFFFF << lsb)) | (0xFFFFFFFF << (msb + 1));
temp =
((data as u32 & (!(0xFFFFFFFF << (msb as u32 - lsb as u32 + 1)))) << (lsb as u32)) | temp;
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)
Expand Down
4 changes: 2 additions & 2 deletions esp-hal/src/delay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl embedded_hal_1::delay::DelayNs for Delay {
}

#[cfg(riscv)]
mod delay {
mod implementation {
use super::*;
use crate::{clock::Clocks, systimer::SystemTimer};

Expand Down Expand Up @@ -95,7 +95,7 @@ mod delay {
}

#[cfg(xtensa)]
mod delay {
mod implementation {
use super::*;
use crate::clock::Clocks;

Expand Down
2 changes: 2 additions & 0 deletions esp-hal/src/dma/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,8 @@ where
R::set_in_priority(priority);
}

// TODO: Should this function be marked as `unsafe`, like `clippy` suggests?
#[allow(clippy::not_unsafe_ptr_arg_deref)]
fn prepare_transfer_without_start(
&mut self,
descriptors: &mut [DmaDescriptor],
Expand Down
35 changes: 19 additions & 16 deletions esp-hal/src/gpio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2527,11 +2527,14 @@ pub mod lp_gpio {

#[inline(always)]
fn get_pin_reg(pin: u8) -> &'static crate::peripherals::lp_io::GPIO0 {
let lp_io = unsafe { &*crate::peripherals::LP_IO::PTR };

// ideally we should change the SVD and make the GPIOx registers into an
// array
unsafe { core::mem::transmute((lp_io.gpio0().as_ptr()).add(pin as usize)) }
unsafe {
let lp_io = &*crate::peripherals::LP_IO::PTR;
let pin_ptr = (lp_io.gpio0().as_ptr()).add(pin as usize);

&*(pin_ptr as *const esp32c6::generic::Reg<esp32c6::lp_io::gpio0::GPIO0_SPEC>)
}
}

/// Configures a pin for use as a low power pin
Expand All @@ -2547,18 +2550,18 @@ pub mod lp_gpio {
) => {
paste::paste!{
$(
impl<MODE> crate::gpio::lp_gpio::IntoLowPowerPin<$gpionum> for GpioPin<MODE, $gpionum> {
fn into_low_power(self) -> crate::gpio::lp_gpio::LowPowerPin<Unknown, $gpionum> {
crate::gpio::lp_gpio::init_low_power_pin($gpionum);
crate::gpio::lp_gpio::LowPowerPin {
impl<MODE> $crate::gpio::lp_gpio::IntoLowPowerPin<$gpionum> for GpioPin<MODE, $gpionum> {
fn into_low_power(self) -> $crate::gpio::lp_gpio::LowPowerPin<Unknown, $gpionum> {
$crate::gpio::lp_gpio::init_low_power_pin($gpionum);
$crate::gpio::lp_gpio::LowPowerPin {
private: core::marker::PhantomData,
}
}
}

impl<MODE> crate::gpio::RTCPin for GpioPin<MODE, $gpionum> {
impl<MODE> $crate::gpio::RTCPin for GpioPin<MODE, $gpionum> {
unsafe fn apply_wakeup(&mut self, wakeup: bool, level: u8) {
let lp_io = &*crate::peripherals::LP_IO::ptr();
let lp_io = &*$crate::peripherals::LP_IO::ptr();
lp_io.[< pin $gpionum >]().modify(|_, w| {
w
.[< lp_gpio $gpionum _wakeup_enable >]().bit(wakeup)
Expand All @@ -2569,7 +2572,7 @@ pub mod lp_gpio {
fn rtcio_pad_hold(&mut self, enable: bool) {
let mask = 1 << $gpionum;
unsafe {
let lp_aon = &*crate::peripherals::LP_AON::ptr();
let lp_aon = &*$crate::peripherals::LP_AON::ptr();

lp_aon.gpio_hold0().modify(|r, w| {
if enable {
Expand All @@ -2583,11 +2586,11 @@ pub mod lp_gpio {

/// Set the LP properties of the pin. If `mux` is true then then pin is
/// routed to LP_IO, when false it is routed to IO_MUX.
fn rtc_set_config(&mut self, input_enable: bool, mux: bool, func: crate::gpio::RtcFunction) {
fn rtc_set_config(&mut self, input_enable: bool, mux: bool, func: $crate::gpio::RtcFunction) {
let mask = 1 << $gpionum;
unsafe {
// Select LP_IO
let lp_aon = &*crate::peripherals::LP_AON::ptr();
let lp_aon = &*$crate::peripherals::LP_AON::ptr();
lp_aon
.gpio_mux()
.modify(|r, w| {
Expand All @@ -2599,7 +2602,7 @@ pub mod lp_gpio {
});

// Configure input, function and select normal operation registers
let lp_io = &*crate::peripherals::LP_IO::ptr();
let lp_io = &*$crate::peripherals::LP_IO::ptr();
lp_io.[< gpio $gpionum >]().modify(|_, w| {
w
.[< lp_gpio $gpionum _slp_sel >]().bit(false)
Expand All @@ -2610,14 +2613,14 @@ pub mod lp_gpio {
}
}

impl<MODE> crate::gpio::RTCPinWithResistors for GpioPin<MODE, $gpionum> {
impl<MODE> $crate::gpio::RTCPinWithResistors for GpioPin<MODE, $gpionum> {
fn rtcio_pullup(&mut self, enable: bool) {
let lp_io = unsafe { &*crate::peripherals::LP_IO::ptr() };
let lp_io = unsafe { &*$crate::peripherals::LP_IO::ptr() };
lp_io.[< gpio $gpionum >]().modify(|_, w| w.[< lp_gpio $gpionum _fun_wpu >]().bit(enable));
}

fn rtcio_pulldown(&mut self, enable: bool) {
let lp_io = unsafe { &*crate::peripherals::LP_IO::ptr() };
let lp_io = unsafe { &*$crate::peripherals::LP_IO::ptr() };
lp_io.[< gpio $gpionum >]().modify(|_, w| w.[< lp_gpio $gpionum _fun_wpd >]().bit(enable));
}
}
Expand Down
52 changes: 25 additions & 27 deletions esp-hal/src/i2s.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1347,35 +1347,33 @@ mod private {
clkm_div_y = 0;
clkm_div_z = 0;
clkm_div_yn1 = 1;
} else {
if clock_settings.numerator > clock_settings.denominator / 2 {
clkm_div_x = clock_settings
.denominator
.overflowing_div(
clock_settings
.denominator
.overflowing_sub(clock_settings.numerator)
.0,
)
.0
.overflowing_sub(1)
.0;
clkm_div_y = clock_settings.denominator
% (clock_settings
} else if clock_settings.numerator > clock_settings.denominator / 2 {
clkm_div_x = clock_settings
.denominator
.overflowing_div(
clock_settings
.denominator
.overflowing_sub(clock_settings.numerator)
.0);
clkm_div_z = clock_settings
.0,
)
.0
.overflowing_sub(1)
.0;
clkm_div_y = clock_settings.denominator
% (clock_settings
.denominator
.overflowing_sub(clock_settings.numerator)
.0;
clkm_div_yn1 = 1;
} else {
clkm_div_x = clock_settings.denominator / clock_settings.numerator - 1;
clkm_div_y = clock_settings.denominator % clock_settings.numerator;
clkm_div_z = clock_settings.numerator;
clkm_div_yn1 = 0;
}
.0);
clkm_div_z = clock_settings
.denominator
.overflowing_sub(clock_settings.numerator)
.0;
clkm_div_yn1 = 1;
} else {
clkm_div_x = clock_settings.denominator / clock_settings.numerator - 1;
clkm_div_y = clock_settings.denominator % clock_settings.numerator;
clkm_div_z = clock_settings.numerator;
clkm_div_yn1 = 0;
}

pcr.i2s_tx_clkm_div_conf().modify(|_, w| {
Expand All @@ -1384,7 +1382,7 @@ mod private {
.i2s_tx_clkm_div_y()
.variant(clkm_div_y as u16)
.i2s_tx_clkm_div_yn1()
.variant(if clkm_div_yn1 != 0 { true } else { false })
.variant(clkm_div_yn1 != 0)
.i2s_tx_clkm_div_z()
.variant(clkm_div_z as u16)
});
Expand Down Expand Up @@ -1415,7 +1413,7 @@ mod private {
.i2s_rx_clkm_div_y()
.variant(clkm_div_y as u16)
.i2s_rx_clkm_div_yn1()
.variant(if clkm_div_yn1 != 0 { true } else { false })
.variant(clkm_div_yn1 != 0)
.i2s_rx_clkm_div_z()
.variant(clkm_div_z as u16)
});
Expand Down
4 changes: 4 additions & 0 deletions esp-hal/src/mcpwm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ impl<'a> PeripheralClockConfig<'a> {
pub struct FrequencyError;

/// A MCPWM peripheral
///
/// # Safety
///
/// ???
pub unsafe trait PwmPeripheral: Deref<Target = RegisterBlock> {
/// Enable peripheral
fn enable();
Expand Down
21 changes: 8 additions & 13 deletions esp-hal/src/parl_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1115,10 +1115,7 @@ where
}
}

fn internal_init<'d, CH>(
dma_channel: &mut Channel<'d, CH>,
frequency: HertzU32,
) -> Result<(), Error>
fn internal_init<CH>(dma_channel: &mut Channel<'_, CH>, frequency: HertzU32) -> Result<(), Error>
where
CH: ChannelTypes,
CH::P: ParlIoPeripheral,
Expand Down Expand Up @@ -1196,7 +1193,7 @@ where
})
}

fn start_write_bytes_dma<'w>(&mut self, ptr: *const u8, len: usize) -> Result<(), Error> {
fn start_write_bytes_dma(&mut self, ptr: *const u8, len: usize) -> Result<(), Error> {
let pcr = unsafe { &*crate::peripherals::PCR::PTR };
pcr.parl_clk_tx_conf()
.modify(|_, w| w.parl_tx_rst_en().set_bit());
Expand Down Expand Up @@ -1306,10 +1303,8 @@ where
{
let (ptr, len) = unsafe { words.write_buffer() };

if !Instance::is_suc_eof_generated_externally() {
if len > MAX_DMA_SIZE {
return Err(Error::MaxDmaTransferSizeExceeded);
}
if !Instance::is_suc_eof_generated_externally() && len > MAX_DMA_SIZE {
return Err(Error::MaxDmaTransferSizeExceeded);
}

self.start_receive_bytes_dma(ptr, len)?;
Expand All @@ -1320,7 +1315,7 @@ where
})
}

fn start_receive_bytes_dma<'w>(&mut self, ptr: *mut u8, len: usize) -> Result<(), Error> {
fn start_receive_bytes_dma(&mut self, ptr: *mut u8, len: usize) -> Result<(), Error> {
let pcr = unsafe { &*crate::peripherals::PCR::PTR };
pcr.parl_clk_rx_conf()
.modify(|_, w| w.parl_rx_rst_en().set_bit());
Expand Down Expand Up @@ -1383,7 +1378,7 @@ where
unsafe {
let buffer = core::ptr::read(&self.buffer);
let payload = core::ptr::read(&self.instance);
let err = (&self).instance.rx_channel.has_error();
let err = self.instance.rx_channel.has_error();
mem::forget(self);
if err {
Err((DmaError::DescriptorError, buffer, payload))
Expand Down Expand Up @@ -1659,7 +1654,7 @@ mod private {

reg_block
.tx_cfg0()
.modify(|_, w| w.tx_bytelen().variant(len as u16));
.modify(|_, w| w.tx_bytelen().variant(len));
}

pub fn is_tx_ready() -> bool {
Expand Down Expand Up @@ -1731,7 +1726,7 @@ mod private {

reg_block
.rx_cfg0()
.modify(|_, w| w.rx_data_bytelen().variant(len as u16));
.modify(|_, w| w.rx_data_bytelen().variant(len));
}

pub fn set_rx_sample_mode(sample_mode: SampleMode) {
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/rtc_cntl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ pub fn get_wakeup_cause() -> SleepSource {

#[cfg(any(esp32c6, esp32h2))]
let wakeup_cause = WakeupReason::from_bits_retain(unsafe {
(&*crate::peripherals::PMU::PTR)
(*crate::peripherals::PMU::PTR)
.slp_wakeup_status0()
.read()
.wakeup_cause()
Expand Down
Loading

0 comments on commit 473e2d7

Please sign in to comment.