From 11ff06a3688811305974d44e0e17417db4ad9fce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Thu, 24 Apr 2025 13:39:07 +0200 Subject: [PATCH] Fix esp32 coex (#3403) * Make sure to enable relevant interrupts * Fix more * Make `current_runlevel` public * Check that interrupts are enabled when initializing * Try to be more honest in `is_in_isr` * Adjust log levels * Really fix ESP32 COEX * Improve COEX example * fmt * CHANGELOG * CHANGELOG * Clarify on `esp_bt_controller_config_t` * Take INTENABLE into account * Test esp-wifi init fails * Simplify test code * Clarify comment * fmt * Rename * Adapt the test * Revert unrelated changes (esp_bt_controller_config_t) * Align Xtensa (current_runlevel) * Additional test * Change test --- esp-hal/CHANGELOG.md | 1 + esp-hal/src/interrupt/riscv.rs | 4 +- esp-hal/src/interrupt/xtensa.rs | 2 +- esp-wifi/CHANGELOG.md | 1 + esp-wifi/src/ble/btdm.rs | 15 ++- esp-wifi/src/ble/os_adapter_esp32.rs | 125 +++++++++++++++-------- esp-wifi/src/ble/os_adapter_esp32c2.rs | 2 + esp-wifi/src/ble/os_adapter_esp32c3.rs | 2 + esp-wifi/src/ble/os_adapter_esp32c6.rs | 2 + esp-wifi/src/ble/os_adapter_esp32h2.rs | 2 + esp-wifi/src/ble/os_adapter_esp32s3.rs | 2 + esp-wifi/src/compat/common.rs | 10 ++ esp-wifi/src/lib.rs | 25 +++++ esp-wifi/src/wifi/internal.rs | 3 +- esp-wifi/src/wifi/mod.rs | 6 ++ esp-wifi/src/wifi/os_adapter.rs | 8 +- examples/src/bin/wifi_coex.rs | 58 ++++++----- hil-test/Cargo.toml | 5 + hil-test/tests/esp_wifi_floats.rs | 13 +++ hil-test/tests/esp_wifi_init.rs | 133 +++++++++++++++++++++++++ 20 files changed, 341 insertions(+), 78 deletions(-) create mode 100644 hil-test/tests/esp_wifi_init.rs diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index e263af59a..87ac578f7 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -47,6 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `ESP_HAL_CONFIG_PLACE_SPI_DRIVER_IN_RAM` configuration option has been renamed to `ESP_HAL_CONFIG_PLACE_SPI_MASTER_DRIVER_IN_RAM`. (#3402) - Made the `ParlIo` traits for `TxPins`, `RxPins`, `ConfigurePins` public (#3398) - Renamed `Flex::enable_input` to `set_input_enable` (#3387) +- Make `esp_hal::interrupt::current_runlevel` public under the unstable feature (#3403) ### Fixed diff --git a/esp-hal/src/interrupt/riscv.rs b/esp-hal/src/interrupt/riscv.rs index 3cc73e720..28d658846 100644 --- a/esp-hal/src/interrupt/riscv.rs +++ b/esp-hal/src/interrupt/riscv.rs @@ -704,7 +704,7 @@ mod classic { } /// Get the current run level (the level below which interrupts are masked). - pub(crate) fn current_runlevel() -> Priority { + pub fn current_runlevel() -> Priority { let intr = INTERRUPT_CORE0::regs(); let prev_interrupt_priority = intr.cpu_int_thresh().read().bits().saturating_sub(1) as u8; @@ -872,7 +872,7 @@ mod plic { } /// Get the current run level (the level below which interrupts are masked). - pub(crate) fn current_runlevel() -> Priority { + pub fn current_runlevel() -> Priority { let prev_interrupt_priority = PLIC_MX::regs() .mxint_thresh() .read() diff --git a/esp-hal/src/interrupt/xtensa.rs b/esp-hal/src/interrupt/xtensa.rs index 9bfb31144..2b60fe95a 100644 --- a/esp-hal/src/interrupt/xtensa.rs +++ b/esp-hal/src/interrupt/xtensa.rs @@ -333,7 +333,7 @@ unsafe fn core1_interrupt_peripheral() -> *const crate::pac::interrupt_core1::Re } /// Get the current run level (the level below which interrupts are masked). -pub(crate) fn current_runlevel() -> Priority { +pub fn current_runlevel() -> Priority { let ps: u32; unsafe { core::arch::asm!("rsr.ps {0}", out(reg) ps) }; diff --git a/esp-wifi/CHANGELOG.md b/esp-wifi/CHANGELOG.md index 3f4c839d3..3fdd44641 100644 --- a/esp-wifi/CHANGELOG.md +++ b/esp-wifi/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Allow `Configuration::None`, set country early, changed default power-save-mode to None (#3364) - Enterprise WPA fixed for ESP32-S2 (#3406) +- COEX on ESP32 is now working (#3403) ### Removed diff --git a/esp-wifi/src/ble/btdm.rs b/esp-wifi/src/ble/btdm.rs index ac347dddb..4936312e2 100644 --- a/esp-wifi/src/ble/btdm.rs +++ b/esp-wifi/src/ble/btdm.rs @@ -258,7 +258,7 @@ unsafe extern "C" fn task_delete(task: *const ()) { #[ram] unsafe extern "C" fn is_in_isr() -> i32 { - 0 + crate::is_interrupts_disabled() as i32 } #[ram] @@ -302,7 +302,6 @@ unsafe extern "C" fn btdm_hus_2_lpcycles(us: u32) -> u32 { // Converts a duration in half us into a number of low power clock cycles. let cycles: u64 = (us as u64) << (g_btdm_lpcycle_us_frac as u64 / g_btdm_lpcycle_us as u64); trace!("btdm_hus_2_lpcycles {} {}", us, cycles); - // probably not right ... NX returns half of the values we calculate here cycles as u32 } @@ -485,7 +484,19 @@ pub fn send_hci(data: &[u8]) { } PACKET_SENT.store(false, Ordering::Relaxed); + + #[cfg(all(esp32, coex))] + ble_os_adapter_chip_specific::async_wakeup_request( + ble_os_adapter_chip_specific::BTDM_ASYNC_WAKEUP_REQ_HCI, + ); + API_vhci_host_send_packet(packet.as_ptr(), packet.len() as u16); + + #[cfg(all(esp32, coex))] + ble_os_adapter_chip_specific::async_wakeup_request_end( + ble_os_adapter_chip_specific::BTDM_ASYNC_WAKEUP_REQ_HCI, + ); + trace!("sent vhci host packet"); super::dump_packet_info(packet); diff --git a/esp-wifi/src/ble/os_adapter_esp32.rs b/esp-wifi/src/ble/os_adapter_esp32.rs index cef02e4c4..1261c7c84 100644 --- a/esp-wifi/src/ble/os_adapter_esp32.rs +++ b/esp-wifi/src/ble/os_adapter_esp32.rs @@ -202,16 +202,23 @@ extern "C" fn patch_apply() { extern "C" fn coex_version_get_wrapper(major: *mut u32, minor: *mut u32, patch: *mut u32) -> i32 { unsafe { - let coex_version = unwrap!( - core::ffi::CStr::from_ptr(crate::binary::include::coex_version_get()) - .to_str() - .ok() - ); - info!("COEX Version {}", coex_version); - // we should parse it ... for now just hardcoded - major.write_volatile(1); - minor.write_volatile(2); - patch.write_volatile(0); + let mut version = crate::binary::include::coex_version_t { + major: 0, + minor: 0, + patch: 0, + }; + if coex_version_get_value(&mut version) == 0 { + info!( + "COEX Version {}.{}.{}", + version.major, version.minor, version.patch + ); + + major.write_volatile(version.major as u32); + minor.write_volatile(version.minor as u32); + patch.write_volatile(version.patch as u32); + } else { + error!("Unable to get COEX version"); + } } 0 @@ -283,6 +290,8 @@ static BTDM_DRAM_AVAILABLE_REGION: [btdm_dram_available_region_t; 7] = [ ]; pub(crate) fn create_ble_config() -> esp_bt_controller_config_t { + // keep them aligned with BT_CONTROLLER_INIT_CONFIG_DEFAULT in ESP-IDF + // ideally _some_ of these values should be configurable esp_bt_controller_config_t { controller_task_stack_size: 4096, controller_task_prio: 110, @@ -380,18 +389,31 @@ pub(crate) fn disable_sleep_mode() { pub(crate) unsafe extern "C" fn coex_bt_wakeup_request() -> bool { trace!("coex_bt_wakeup_request"); - #[cfg(coex)] - return async_wakeup_request(BTDM_ASYNC_WAKEUP_REQ_COEX); - #[cfg(not(coex))] + // This should really be + // ```rust,norun + // #[cfg(coex)] + // return async_wakeup_request(BTDM_ASYNC_WAKEUP_REQ_COEX); + // #[cfg(not(coex))] + // true + // ``` + // + // But doing the right thing here keeps BT from working. + // In a similar scenario this function isn't called in ESP-IDF. true } pub(crate) unsafe extern "C" fn coex_bt_wakeup_request_end() { - warn!("coex_bt_wakeup_request_end"); + trace!("coex_bt_wakeup_request_end"); - #[cfg(coex)] - async_wakeup_request_end(BTDM_ASYNC_WAKEUP_REQ_COEX); + // This should really be + // ```rust,norun + // #[cfg(coex)] + // async_wakeup_request_end(BTDM_ASYNC_WAKEUP_REQ_COEX); + // ``` + // + // But doing the right thing here keeps BT from working. + // In a similar scenario this function isn't called in ESP-IDF. } #[allow(unused_variables)] @@ -427,7 +449,7 @@ pub(crate) unsafe extern "C" fn coex_bt_release(event: u32) -> i32 { pub(crate) unsafe extern "C" fn coex_register_bt_cb_wrapper( callback: unsafe extern "C" fn(), ) -> i32 { - warn!("coex_register_bt_cb {:?}", callback); + trace!("coex_register_bt_cb {:?}", callback); unsafe extern "C" { #[cfg(coex)] fn coex_register_bt_cb(callback: unsafe extern "C" fn()) -> i32; @@ -471,7 +493,7 @@ pub(crate) unsafe extern "C" fn coex_bb_reset_unlock(event: u32) { pub(crate) unsafe extern "C" fn coex_schm_register_btdm_callback_wrapper( callback: unsafe extern "C" fn(), ) -> i32 { - warn!("coex_schm_register_btdm_callback {:?}", callback); + trace!("coex_schm_register_btdm_callback {:?}", callback); unsafe extern "C" { #[cfg(coex)] fn coex_schm_register_callback(kind: u32, callback: unsafe extern "C" fn()) -> i32; @@ -516,7 +538,7 @@ pub(crate) unsafe extern "C" fn coex_schm_curr_phase_get() -> *const () { #[allow(unused_variables)] pub(crate) unsafe extern "C" fn coex_wifi_channel_get(primary: *mut u8, secondary: *mut u8) -> i32 { - warn!("coex_wifi_channel_get"); + trace!("coex_wifi_channel_get"); unsafe extern "C" { #[cfg(coex)] fn coex_wifi_channel_get(primary: *mut u8, secondary: *mut u8) -> i32; @@ -533,7 +555,7 @@ pub(crate) unsafe extern "C" fn coex_wifi_channel_get(primary: *mut u8, secondar pub(crate) unsafe extern "C" fn coex_register_wifi_channel_change_callback( callback: unsafe extern "C" fn(), ) -> i32 { - warn!("coex_register_wifi_channel_change_callback"); + trace!("coex_register_wifi_channel_change_callback"); unsafe extern "C" { #[cfg(coex)] fn coex_register_wifi_channel_change_callback(callback: unsafe extern "C" fn()) -> i32; @@ -558,6 +580,10 @@ pub(crate) unsafe extern "C" fn set_isr(n: i32, f: unsafe extern "C" fn(), arg: Interrupt::RWBT, interrupt::Priority::Priority1 )); + unwrap!(interrupt::enable( + Interrupt::RWBLE, + interrupt::Priority::Priority1 + )); } 7 => unsafe { ISR_INTERRUPT_7 = (f as *mut c_types::c_void, arg as *mut c_types::c_void); @@ -585,10 +611,10 @@ pub(crate) unsafe extern "C" fn ints_on(mask: u32) { } #[cfg(coex)] -const BTDM_ASYNC_WAKEUP_REQ_HCI: i32 = 0; +pub(crate) const BTDM_ASYNC_WAKEUP_REQ_HCI: i32 = 0; #[cfg(coex)] -const BTDM_ASYNC_WAKEUP_REQ_COEX: i32 = 1; +pub(crate) const BTDM_ASYNC_WAKEUP_REQ_COEX: i32 = 1; // const BTDM_ASYNC_WAKEUP_REQMAX: i32 = 2; @@ -606,26 +632,41 @@ const BTDM_ASYNC_WAKEUP_REQ_COEX: i32 = 1; /// /// ************************************************************************* #[cfg(coex)] -fn async_wakeup_request(event: i32) -> bool { - let mut do_wakeup_request = false; - - let request_lock = match event { - e if e == BTDM_ASYNC_WAKEUP_REQ_HCI => true, - e if e == BTDM_ASYNC_WAKEUP_REQ_COEX => false, - _ => return false, - }; +pub(crate) fn async_wakeup_request(event: i32) -> bool { + trace!("async_wakeup_request {event}"); unsafe extern "C" { + fn btdm_in_wakeup_requesting_set(set: bool); + fn btdm_power_state_active() -> bool; - fn btdm_wakeup_request(request_lock: bool); + + fn btdm_wakeup_request(); } - if !unsafe { btdm_power_state_active() } { - do_wakeup_request = true; - unsafe { btdm_wakeup_request(request_lock) }; - } + let request_lock = match event { + e if e == BTDM_ASYNC_WAKEUP_REQ_HCI => { + unsafe { + btdm_in_wakeup_requesting_set(true); + } + false + } + e if e == BTDM_ASYNC_WAKEUP_REQ_COEX => { + unsafe { + btdm_in_wakeup_requesting_set(true); + } - do_wakeup_request + if !unsafe { btdm_power_state_active() } { + unsafe { + btdm_wakeup_request(); + } + true + } else { + false + } + } + _ => return false, + }; + request_lock } /// ************************************************************************** @@ -642,7 +683,9 @@ fn async_wakeup_request(event: i32) -> bool { /// /// ************************************************************************* #[cfg(coex)] -fn async_wakeup_request_end(event: i32) { +pub(crate) fn async_wakeup_request_end(event: i32) { + trace!("async_wakeup_request_end {event}"); + let request_lock = match event { e if e == BTDM_ASYNC_WAKEUP_REQ_HCI => true, e if e == BTDM_ASYNC_WAKEUP_REQ_COEX => false, @@ -650,12 +693,10 @@ fn async_wakeup_request_end(event: i32) { }; unsafe extern "C" { - // this isn't found anywhere ... not a ROM function - // not in any of the libs - but the code will never call this anyway - - // fn btdm_wakeup_request_end(); + fn btdm_in_wakeup_requesting_set(set: bool); } + if request_lock { - // unsafe { btdm_wakeup_request_end() }; + unsafe { btdm_in_wakeup_requesting_set(false) }; } } diff --git a/esp-wifi/src/ble/os_adapter_esp32c2.rs b/esp-wifi/src/ble/os_adapter_esp32c2.rs index de31718f0..4237e2fa0 100644 --- a/esp-wifi/src/ble/os_adapter_esp32c2.rs +++ b/esp-wifi/src/ble/os_adapter_esp32c2.rs @@ -17,6 +17,8 @@ pub(crate) static mut ISR_INTERRUPT_7: ( *mut crate::binary::c_types::c_void, ) = (core::ptr::null_mut(), core::ptr::null_mut()); +// keep them aligned with BT_CONTROLLER_INIT_CONFIG_DEFAULT in ESP-IDF +// ideally _some_ of these values should be configurable pub(crate) static BLE_CONFIG: esp_bt_controller_config_t = esp_bt_controller_config_t { config_version: 0x20231124, ble_ll_resolv_list_size: 4, diff --git a/esp-wifi/src/ble/os_adapter_esp32c3.rs b/esp-wifi/src/ble/os_adapter_esp32c3.rs index 42db50d91..2764b5d79 100644 --- a/esp-wifi/src/ble/os_adapter_esp32c3.rs +++ b/esp-wifi/src/ble/os_adapter_esp32c3.rs @@ -251,6 +251,8 @@ unsafe extern "C" { } pub(crate) fn create_ble_config() -> esp_bt_controller_config_t { + // keep them aligned with BT_CONTROLLER_INIT_CONFIG_DEFAULT in ESP-IDF + // ideally _some_ of these values should be configurable esp_bt_controller_config_t { magic: 0x5a5aa5a5, version: 0x02404010, diff --git a/esp-wifi/src/ble/os_adapter_esp32c6.rs b/esp-wifi/src/ble/os_adapter_esp32c6.rs index bf344d1a6..34f47fb0e 100644 --- a/esp-wifi/src/ble/os_adapter_esp32c6.rs +++ b/esp-wifi/src/ble/os_adapter_esp32c6.rs @@ -17,6 +17,8 @@ pub(crate) static mut ISR_INTERRUPT_7: ( *mut crate::binary::c_types::c_void, ) = (core::ptr::null_mut(), core::ptr::null_mut()); +// keep them aligned with BT_CONTROLLER_INIT_CONFIG_DEFAULT in ESP-IDF +// ideally _some_ of these values should be configurable pub(crate) static BLE_CONFIG: esp_bt_controller_config_t = esp_bt_controller_config_t { config_version: 0x20240422, ble_ll_resolv_list_size: 4, diff --git a/esp-wifi/src/ble/os_adapter_esp32h2.rs b/esp-wifi/src/ble/os_adapter_esp32h2.rs index 2059b2c1c..e6b0e77f9 100644 --- a/esp-wifi/src/ble/os_adapter_esp32h2.rs +++ b/esp-wifi/src/ble/os_adapter_esp32h2.rs @@ -17,6 +17,8 @@ pub(crate) static mut ISR_INTERRUPT_3: ( *mut crate::binary::c_types::c_void, ) = (core::ptr::null_mut(), core::ptr::null_mut()); +// keep them aligned with BT_CONTROLLER_INIT_CONFIG_DEFAULT in ESP-IDF +// ideally _some_ of these values should be configurable pub(crate) static BLE_CONFIG: esp_bt_controller_config_t = esp_bt_controller_config_t { config_version: 0x20240422, ble_ll_resolv_list_size: 4, diff --git a/esp-wifi/src/ble/os_adapter_esp32s3.rs b/esp-wifi/src/ble/os_adapter_esp32s3.rs index 1402ca4fc..7a84d7e30 100644 --- a/esp-wifi/src/ble/os_adapter_esp32s3.rs +++ b/esp-wifi/src/ble/os_adapter_esp32s3.rs @@ -251,6 +251,8 @@ unsafe extern "C" { } pub(crate) fn create_ble_config() -> esp_bt_controller_config_t { + // keep them aligned with BT_CONTROLLER_INIT_CONFIG_DEFAULT in ESP-IDF + // ideally _some_ of these values should be configurable esp_bt_controller_config_t { magic: 0x5a5aa5a5, version: 0x02404010, diff --git a/esp-wifi/src/compat/common.rs b/esp-wifi/src/compat/common.rs index c1e3bd024..5bcb5fe02 100644 --- a/esp-wifi/src/compat/common.rs +++ b/esp-wifi/src/compat/common.rs @@ -205,6 +205,16 @@ pub(crate) fn sem_delete(semphr: *mut c_void) { } pub(crate) fn sem_take(semphr: *mut c_void, tick: u32) -> i32 { + // This shouldn't normally happen if we always report the correct state from + // `is_in_isr`. This is a last resort if the driver calls this anyways. + // (I haven't observed this to happen) + let tick = if tick == OSI_FUNCS_TIME_BLOCKING && crate::is_interrupts_disabled() { + warn!("blocking sem_take probably called from an ISR - return early"); + 1 + } else { + tick + }; + trace!(">>>> semphr_take {:?} block_time_tick {}", semphr, tick); let forever = tick == OSI_FUNCS_TIME_BLOCKING; diff --git a/esp-wifi/src/lib.rs b/esp-wifi/src/lib.rs index 6b8dbedfa..428d1f4ba 100644 --- a/esp-wifi/src/lib.rs +++ b/esp-wifi/src/lib.rs @@ -349,6 +349,8 @@ impl private::Sealed for Trng<'_> {} /// Initialize for using WiFi and or BLE. /// +/// Make sure to **not** call this function while interrupts are disabled. +/// /// # The `timer` argument /// /// The `timer` argument is a timer source that is used by the WiFi driver to @@ -378,6 +380,10 @@ pub fn init<'d>( _rng: impl EspWifiRngSource + 'd, _radio_clocks: RADIO_CLK<'d>, ) -> Result, InitializationError> { + if crate::is_interrupts_disabled() { + return Err(InitializationError::InterruptsDisabled); + } + // A minimum clock of 80MHz is required to operate WiFi module. const MIN_CLOCK: Rate = Rate::from_mhz(80); let clocks = Clocks::get(); @@ -467,6 +473,17 @@ pub unsafe fn deinit_unchecked() -> Result<(), InitializationError> { Ok(()) } +/// Returns true if at least some interrupt levels are disabled. +fn is_interrupts_disabled() -> bool { + #[cfg(target_arch = "xtensa")] + return hal::xtensa_lx::interrupt::get_level() != 0 + || hal::xtensa_lx::interrupt::get_mask() == 0; + + #[cfg(target_arch = "riscv32")] + return !hal::riscv::register::mstatus::read().mie() + || hal::interrupt::current_runlevel() >= hal::interrupt::Priority::Priority1; +} + pub(crate) mod private { pub trait Sealed {} } @@ -474,11 +491,19 @@ pub(crate) mod private { #[derive(Debug, Clone, Copy)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] /// Error which can be returned during [`init`]. +#[non_exhaustive] pub enum InitializationError { + /// A general error occurred. + /// The internal error code is reported. General(i32), + /// An error from the WiFi driver. #[cfg(feature = "wifi")] WifiError(WifiError), + /// The current CPU clock frequency is too low. WrongClockConfig, + /// Tried to initialize while interrupts are disabled. + /// This is not supported. + InterruptsDisabled, } #[cfg(feature = "wifi")] diff --git a/esp-wifi/src/wifi/internal.rs b/esp-wifi/src/wifi/internal.rs index 639d81039..c7b055abd 100644 --- a/esp-wifi/src/wifi/internal.rs +++ b/esp-wifi/src/wifi/internal.rs @@ -63,8 +63,7 @@ unsafe extern "C" fn semphr_give_from_isr_wrapper( #[cfg(coex)] unsafe extern "C" fn is_in_isr_wrapper() -> i32 { - // like original implementation - 0 + crate::is_interrupts_disabled() as i32 } #[unsafe(no_mangle)] diff --git a/esp-wifi/src/wifi/mod.rs b/esp-wifi/src/wifi/mod.rs index f0a4f9ab0..e9595349f 100644 --- a/esp-wifi/src/wifi/mod.rs +++ b/esp-wifi/src/wifi/mod.rs @@ -2608,10 +2608,16 @@ pub struct Interfaces<'d> { /// Create a WiFi controller and it's associated interfaces. /// /// Dropping the controller will deinitialize / stop WiFi. +/// +/// Make sure to **not** call this function while interrupts are disabled. pub fn new<'d>( inited: &'d EspWifiController<'d>, _device: crate::hal::peripherals::WIFI<'d>, ) -> Result<(WifiController<'d>, Interfaces<'d>), WifiError> { + if crate::is_interrupts_disabled() { + return Err(WifiError::Unsupported); + } + let mut controller = WifiController { _phantom: Default::default(), }; diff --git a/esp-wifi/src/wifi/os_adapter.rs b/esp-wifi/src/wifi/os_adapter.rs index c7abda80e..f9774ea64 100644 --- a/esp-wifi/src/wifi/os_adapter.rs +++ b/esp-wifi/src/wifi/os_adapter.rs @@ -177,10 +177,9 @@ pub unsafe extern "C" fn is_from_isr() -> bool { /// Spin lock data pointer /// /// ************************************************************************* -static mut FAKE_SPIN_LOCK: u8 = 1; pub unsafe extern "C" fn spin_lock_create() -> *mut crate::binary::c_types::c_void { - // original: return (void *)1; - let ptr = addr_of_mut!(FAKE_SPIN_LOCK); + let ptr = crate::compat::common::sem_create(1, 1); + trace!("spin_lock_create {:?}", ptr); ptr as *mut crate::binary::c_types::c_void } @@ -199,8 +198,9 @@ pub unsafe extern "C" fn spin_lock_create() -> *mut crate::binary::c_types::c_vo /// /// ************************************************************************* pub unsafe extern "C" fn spin_lock_delete(lock: *mut crate::binary::c_types::c_void) { - // original: DEBUGASSERT((int)lock == 1); trace!("spin_lock_delete {:?}", lock); + + crate::compat::common::sem_delete(lock); } /// ************************************************************************** diff --git a/examples/src/bin/wifi_coex.rs b/examples/src/bin/wifi_coex.rs index 9f5566935..087d2bea5 100644 --- a/examples/src/bin/wifi_coex.rs +++ b/examples/src/bin/wifi_coex.rs @@ -58,9 +58,17 @@ fn main() -> ! { let config = esp_hal::Config::default().with_cpu_clock(CpuClock::max()); let peripherals = esp_hal::init(config); - esp_alloc::heap_allocator!(size: 72 * 1024); // COEX needs more RAM - add some more - esp_alloc::heap_allocator!(#[unsafe(link_section = ".dram2_uninit")] size: 64 * 1024); + #[cfg(feature = "esp32")] + { + esp_alloc::heap_allocator!(#[unsafe(link_section = ".dram2_uninit")] size: 96 * 1024); + esp_alloc::heap_allocator!(size: 24 * 1024); + } + #[cfg(not(feature = "esp32"))] + { + esp_alloc::heap_allocator!(#[unsafe(link_section = ".dram2_uninit")] size: 64 * 1024); + esp_alloc::heap_allocator!(size: 64 * 1024); + } let timg0 = TimerGroup::new(peripherals.TIMG0); @@ -68,7 +76,29 @@ fn main() -> ! { let esp_wifi_ctrl = init(timg0.timer0, rng.clone(), peripherals.RADIO_CLK).unwrap(); - let bluetooth = peripherals.BT; + let now = || time::Instant::now().duration_since_epoch().as_millis(); + + // initializing Bluetooth first results in a more stable WiFi connection on ESP32 + let connector = BleConnector::new(&esp_wifi_ctrl, peripherals.BT); + let hci = HciConnector::new(connector, now); + let mut ble = Ble::new(&hci); + + println!("{:?}", ble.init()); + println!("{:?}", ble.cmd_set_le_advertising_parameters()); + println!( + "{:?}", + ble.cmd_set_le_advertising_data( + create_advertising_data(&[ + AdStructure::Flags(LE_GENERAL_DISCOVERABLE | BR_EDR_NOT_SUPPORTED), + AdStructure::ServiceUuids16(&[Uuid::Uuid16(0x1809)]), + AdStructure::CompleteLocalName(esp_hal::chip!()), + ]) + .unwrap() + ) + ); + println!("{:?}", ble.cmd_set_le_advertise_enable(true)); + + println!("started advertising"); let (mut controller, interfaces) = esp_wifi::wifi::new(&esp_wifi_ctrl, peripherals.WIFI).unwrap(); @@ -90,7 +120,6 @@ fn main() -> ! { }]); socket_set.add(dhcp_socket); - let now = || time::Instant::now().duration_since_epoch().as_millis(); let stack = Stack::new(iface, device, socket_set, now, rng.random()); let client_config = Configuration::Client(ClientConfiguration { @@ -131,27 +160,6 @@ fn main() -> ! { } } - let connector = BleConnector::new(&esp_wifi_ctrl, bluetooth); - let hci = HciConnector::new(connector, now); - let mut ble = Ble::new(&hci); - - println!("{:?}", ble.init()); - println!("{:?}", ble.cmd_set_le_advertising_parameters()); - println!( - "{:?}", - ble.cmd_set_le_advertising_data( - create_advertising_data(&[ - AdStructure::Flags(LE_GENERAL_DISCOVERABLE | BR_EDR_NOT_SUPPORTED), - AdStructure::ServiceUuids16(&[Uuid::Uuid16(0x1809)]), - AdStructure::CompleteLocalName(esp_hal::chip!()), - ]) - .unwrap() - ) - ); - println!("{:?}", ble.cmd_set_le_advertise_enable(true)); - - println!("started advertising"); - println!("Start busy loop on main"); let mut rx_buffer = [0u8; 128]; diff --git a/hil-test/Cargo.toml b/hil-test/Cargo.toml index 78110df82..52a693384 100644 --- a/hil-test/Cargo.toml +++ b/hil-test/Cargo.toml @@ -214,6 +214,11 @@ name = "esp_wifi_ble_controller" harness = false required-features = ["esp-wifi", "esp-alloc"] +[[test]] +name = "esp_wifi_init" +harness = false +required-features = ["esp-wifi", "esp-alloc"] + [dependencies] allocator-api2 = { version = "0.2.0", default-features = false, features = ["alloc"] } cfg-if = "1.0.0" diff --git a/hil-test/tests/esp_wifi_floats.rs b/hil-test/tests/esp_wifi_floats.rs index 8e931ae0b..67a2af2b2 100644 --- a/hil-test/tests/esp_wifi_floats.rs +++ b/hil-test/tests/esp_wifi_floats.rs @@ -129,6 +129,19 @@ mod tests { .start_app_core( unsafe { &mut *core::ptr::addr_of_mut!(APP_CORE_STACK) }, move || { + // app core starts with interrupts disabled + unsafe { + esp_hal::xtensa_lx::interrupt::enable_mask( + esp_hal::xtensa_lx_rt::interrupt::CpuInterruptLevel::Level1.mask() + | esp_hal::xtensa_lx_rt::interrupt::CpuInterruptLevel::Level2 + .mask() + | esp_hal::xtensa_lx_rt::interrupt::CpuInterruptLevel::Level3 + .mask() + | esp_hal::xtensa_lx_rt::interrupt::CpuInterruptLevel::Level6 + .mask(), + ); + } + let timg0 = TimerGroup::new(peripherals.TIMG0); let _init = esp_wifi::init( timg0.timer1, diff --git a/hil-test/tests/esp_wifi_init.rs b/hil-test/tests/esp_wifi_init.rs new file mode 100644 index 000000000..ec256deb0 --- /dev/null +++ b/hil-test/tests/esp_wifi_init.rs @@ -0,0 +1,133 @@ +//! Test we get an error when attempting to initialize esp-wifi with interrupts +//! disabled in common ways + +//% CHIPS: esp32 esp32s2 esp32c2 esp32c3 esp32c6 esp32s3 +//% FEATURES: unstable esp-wifi esp-alloc esp-wifi/wifi embassy + +#![no_std] +#![no_main] + +use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, signal::Signal}; +#[cfg(target_arch = "riscv32")] +use esp_hal::riscv::interrupt::free as interrupt_free; +#[cfg(target_arch = "xtensa")] +use esp_hal::xtensa_lx::interrupt::free as interrupt_free; +use esp_hal::{ + clock::CpuClock, + interrupt::{Priority, software::SoftwareInterruptControl}, + peripherals::{Peripherals, RADIO_CLK, RNG, TIMG0}, + rng::Rng, + timer::timg::TimerGroup, +}; +use esp_hal_embassy::InterruptExecutor; +use esp_wifi::InitializationError; +use hil_test as _; +use static_cell::StaticCell; + +macro_rules! mk_static { + ($t:ty,$val:expr) => {{ + static STATIC_CELL: static_cell::StaticCell<$t> = static_cell::StaticCell::new(); + #[deny(unused_attributes)] + let x = STATIC_CELL.uninit().write(($val)); + x + }}; +} + +#[embassy_executor::task] +async fn try_init( + signal: &'static Signal>, + timer: TIMG0<'static>, + rng: RNG<'static>, + radio_clk: RADIO_CLK<'static>, +) { + let timg0 = TimerGroup::new(timer); + + let init = esp_wifi::init(timg0.timer0, Rng::new(rng), radio_clk); + + match init { + Ok(_) => { + signal.signal(None); + } + Err(err) => { + signal.signal(Some(err)); + } + } +} + +#[cfg(test)] +#[embedded_test::tests(default_timeout = 3, executor = esp_hal_embassy::Executor::new())] +mod tests { + use super::*; + + #[init] + fn init() -> Peripherals { + esp_alloc::heap_allocator!(size: 72 * 1024); + + let config = esp_hal::Config::default().with_cpu_clock(CpuClock::max()); + esp_hal::init(config) + } + + #[test] + fn test_init_fails_cs(peripherals: Peripherals) { + let timg0 = TimerGroup::new(peripherals.TIMG0); + let init = critical_section::with(|_| { + esp_wifi::init( + timg0.timer0, + Rng::new(peripherals.RNG), + peripherals.RADIO_CLK, + ) + }); + + assert!(matches!( + init, + Err(esp_wifi::InitializationError::InterruptsDisabled), + )); + } + + #[test] + fn test_init_fails_interrupt_free(peripherals: Peripherals) { + let timg0 = TimerGroup::new(peripherals.TIMG0); + let init = interrupt_free(|| { + esp_wifi::init( + timg0.timer0, + Rng::new(peripherals.RNG), + peripherals.RADIO_CLK, + ) + }); + + assert!(matches!( + init, + Err(esp_wifi::InitializationError::InterruptsDisabled), + )); + } + + #[test] + async fn test_init_fails_in_interrupt_executor_task(peripherals: Peripherals) { + let sw_ints = SoftwareInterruptControl::new(peripherals.SW_INTERRUPT); + + static EXECUTOR_CORE_0: StaticCell> = StaticCell::new(); + let executor_core0 = InterruptExecutor::new(sw_ints.software_interrupt1); + let executor_core0 = EXECUTOR_CORE_0.init(executor_core0); + + let spawner = executor_core0.start(Priority::Priority1); + + let signal = + mk_static!(Signal>, Signal::new()); + + spawner + .spawn(try_init( + signal, + peripherals.TIMG0, + peripherals.RNG, + peripherals.RADIO_CLK, + )) + .ok(); + + let res = signal.wait().await; + + assert!(matches!( + res, + Some(esp_wifi::InitializationError::InterruptsDisabled), + )); + } +}