From 5ec8be4af28cf9a01cc64b9558292d2d34a4be52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Tue, 21 Jan 2025 11:31:20 +0100 Subject: [PATCH] Fix LCD_CAM disabling its clocks (#3007) --- esp-hal/CHANGELOG.md | 1 + esp-hal/src/lcd_cam/lcd/dpi.rs | 3 +++ esp-hal/src/lcd_cam/lcd/i8080.rs | 3 +++ esp-hal/src/system.rs | 2 ++ hil-test/tests/lcd_cam_i8080.rs | 4 ++++ hil-test/tests/lcd_cam_i8080_async.rs | 4 ++++ 6 files changed, 17 insertions(+) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index d108a1a57..e70a04c3f 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - `DmaDescriptor` is now `#[repr(C)]` (#2988) +- Fixed an issue that caused LCD_CAM drivers to turn off their clocks unexpectedly (#3007) ### Removed diff --git a/esp-hal/src/lcd_cam/lcd/dpi.rs b/esp-hal/src/lcd_cam/lcd/dpi.rs index 5eed6c856..258b92849 100644 --- a/esp-hal/src/lcd_cam/lcd/dpi.rs +++ b/esp-hal/src/lcd_cam/lcd/dpi.rs @@ -115,6 +115,7 @@ use crate::{ }, peripheral::{Peripheral, PeripheralRef}, peripherals::LCD_CAM, + system::{self, GenericPeripheralGuard}, Blocking, DriverMode, }; @@ -131,6 +132,7 @@ pub enum ConfigError { pub struct Dpi<'d, Dm: DriverMode> { lcd_cam: PeripheralRef<'d, LCD_CAM>, tx_channel: ChannelTx<'d, Blocking, PeripheralTxChannel>, + _guard: GenericPeripheralGuard<{ system::Peripheral::LcdCam as u8 }>, _mode: PhantomData, } @@ -152,6 +154,7 @@ where let mut this = Self { lcd_cam: lcd.lcd_cam, tx_channel, + _guard: lcd._guard, _mode: PhantomData, }; diff --git a/esp-hal/src/lcd_cam/lcd/i8080.rs b/esp-hal/src/lcd_cam/lcd/i8080.rs index cf53cfcad..934c31d4f 100644 --- a/esp-hal/src/lcd_cam/lcd/i8080.rs +++ b/esp-hal/src/lcd_cam/lcd/i8080.rs @@ -79,6 +79,7 @@ use crate::{ }, peripheral::{Peripheral, PeripheralRef}, peripherals::LCD_CAM, + system::{self, GenericPeripheralGuard}, Blocking, DriverMode, }; @@ -95,6 +96,7 @@ pub enum ConfigError { pub struct I8080<'d, Dm: DriverMode> { lcd_cam: PeripheralRef<'d, LCD_CAM>, tx_channel: ChannelTx<'d, Blocking, PeripheralTxChannel>, + _guard: GenericPeripheralGuard<{ system::Peripheral::LcdCam as u8 }>, _mode: PhantomData, } @@ -118,6 +120,7 @@ where let mut this = Self { lcd_cam: lcd.lcd_cam, tx_channel, + _guard: lcd._guard, _mode: PhantomData, }; diff --git a/esp-hal/src/system.rs b/esp-hal/src/system.rs index 2452e9bc8..365733894 100755 --- a/esp-hal/src/system.rs +++ b/esp-hal/src/system.rs @@ -1066,12 +1066,14 @@ impl PeripheralClockControl { if enable { let prev = *ref_count; *ref_count += 1; + trace!("Enable {:?} {} -> {}", peripheral, prev, *ref_count); if prev > 0 { return false; } } else { let prev = *ref_count; *ref_count -= 1; + trace!("Disable {:?} {} -> {}", peripheral, prev, *ref_count); if prev > 1 { return false; } diff --git a/hil-test/tests/lcd_cam_i8080.rs b/hil-test/tests/lcd_cam_i8080.rs index e411c9150..343ddd68c 100644 --- a/hil-test/tests/lcd_cam_i8080.rs +++ b/hil-test/tests/lcd_cam_i8080.rs @@ -147,6 +147,10 @@ mod tests { .with_cs(cs_signal) .with_ctrl_pins(NoPin, NoPin); + // explicitly drop the camera half to see if it disables clocks (unexpectedly, + // I8080 should keep it alive) + core::mem::drop(ctx.lcd_cam.cam); + // This is to make the test values look more intuitive. i8080.set_bit_order(BitOrder::Inverted); diff --git a/hil-test/tests/lcd_cam_i8080_async.rs b/hil-test/tests/lcd_cam_i8080_async.rs index 8dccfb6ef..c396fe064 100644 --- a/hil-test/tests/lcd_cam_i8080_async.rs +++ b/hil-test/tests/lcd_cam_i8080_async.rs @@ -58,6 +58,10 @@ mod tests { }) .unwrap(); + // explicitly drop the camera half to see if it disables clocks (unexpectedly, + // I8080 should keep it alive) + core::mem::drop(ctx.lcd_cam.cam); + let mut transfer = i8080.send(Command::::None, 0, ctx.dma_buf).unwrap(); transfer.wait_for_done().await;