review comments

esp-hal/src/lcd_cam/lcd/i8080.rs

@@ -77,31 +77,30 @@ use crate::{
         lcd::{i8080::private::TxPins, ClockMode, DelayMode, Phase, Polarity},
         private::calculate_clkm,
         BitOrder,
+        BlockingWithInterrupts,
         ByteOrder,
-        NamesAreHard,
+        InterruptControl,
         Lcd,
         LcdCamInterrupt,
     },
     peripheral::{Peripheral, PeripheralRef},
     peripherals::LCD_CAM,
     Async,
-    Blocking,
     InterruptConfigurable,
-    Mode,
     DEFAULT_INTERRUPT_HANDLER,
 };
 
 /// Represents the I8080 LCD interface.
-pub struct I8080<'d, DM: Mode> {
+pub struct I8080<'d, DM> {
     lcd_cam: PeripheralRef<'d, LCD_CAM>,
     tx_channel: ChannelTx<'d, <LCD_CAM as DmaEligible>::Dma>,
     _phantom: PhantomData<DM>,
 }
 
-impl<'d> I8080<'d, Blocking> {
+impl<'d, DM> I8080<'d, DM> {
     /// Creates a new instance of the I8080 LCD interface.
     pub fn new<P, CH>(
-        lcd: Lcd<'d>,
+        lcd: Lcd<'d, DM>,
         channel: ChannelTx<'d, CH>,
         mut pins: P,
         frequency: HertzU32,
@@ -217,9 +216,13 @@ impl<'d> I8080<'d, Blocking> {
             _phantom: PhantomData,
         }
     }
+}
 
+impl<'d> I8080<'d, BlockingWithInterrupts> {
     /// Convert this blocking driver into an async one.
-    pub fn into_async(self, mut interrupts: NamesAreHard<'d>) -> I8080<'d, Async> {
+    pub fn into_async(self) -> I8080<'d, Async> {
+        let mut interrupts = unsafe { InterruptControl::steal() };
+
         // Disable any active interrupts for safety.
         interrupts.unlisten(EnumSet::all());
 
@@ -236,27 +239,24 @@ impl<'d> I8080<'d, Blocking> {
 
 impl<'d> I8080<'d, Async> {
     /// Convert this async driver into a blocking one.
-    pub fn into_blocking(self) -> (I8080<'d, Blocking>, NamesAreHard<'d>) {
-        let mut interrupts = unsafe { NamesAreHard::steal() };
+    pub fn into_blocking(self) -> I8080<'d, BlockingWithInterrupts> {
+        let mut interrupts = unsafe { InterruptControl::steal() };
 
         // Disable any active interrupts for safety.
         interrupts.unlisten(EnumSet::all());
 
         // "Unset" interrupt handler.
         interrupts.set_interrupt_handler(DEFAULT_INTERRUPT_HANDLER);
 
-        (
-            I8080 {
-                lcd_cam: self.lcd_cam,
-                tx_channel: self.tx_channel,
-                _phantom: PhantomData,
-            },
-            interrupts,
-        )
+        I8080 {
+            lcd_cam: self.lcd_cam,
+            tx_channel: self.tx_channel,
+            _phantom: PhantomData,
+        }
     }
 }
 
-impl<'d, DM: Mode> I8080<'d, DM> {
+impl<'d, DM> I8080<'d, DM> {
     /// Configures the byte order for data transmission.
     pub fn set_byte_order(&mut self, byte_order: ByteOrder) -> &mut Self {
         let is_inverted = byte_order != ByteOrder::default();
@@ -411,20 +411,20 @@ impl<'d, DM: Mode> I8080<'d, DM> {
     }
 }
 
-impl<'d, DM: Mode> core::fmt::Debug for I8080<'d, DM> {
+impl<'d, DM> core::fmt::Debug for I8080<'d, DM> {
     fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
         f.debug_struct("I8080").finish()
     }
 }
 
 /// Represents an ongoing (or potentially finished) transfer using the I8080 LCD
 /// interface
-pub struct I8080Transfer<'d, BUF: DmaTxBuffer, DM: Mode> {
+pub struct I8080Transfer<'d, BUF: DmaTxBuffer, DM> {
     i8080: ManuallyDrop<I8080<'d, DM>>,
     buf_view: ManuallyDrop<BUF::View>,
 }
 
-impl<'d, BUF: DmaTxBuffer, DM: Mode> I8080Transfer<'d, BUF, DM> {
+impl<'d, BUF: DmaTxBuffer, DM> I8080Transfer<'d, BUF, DM> {
     /// Returns true when [Self::wait] will not block.
     pub fn is_done(&self) -> bool {
         self.i8080
@@ -485,15 +485,15 @@ impl<'d, BUF: DmaTxBuffer, DM: Mode> I8080Transfer<'d, BUF, DM> {
     }
 }
 
-impl<'d, BUF: DmaTxBuffer, DM: Mode> Deref for I8080Transfer<'d, BUF, DM> {
+impl<'d, BUF: DmaTxBuffer, DM> Deref for I8080Transfer<'d, BUF, DM> {
     type Target = BUF::View;
 
     fn deref(&self) -> &Self::Target {
         &self.buf_view
     }
 }
 
-impl<'d, BUF: DmaTxBuffer, DM: Mode> DerefMut for I8080Transfer<'d, BUF, DM> {
+impl<'d, BUF: DmaTxBuffer, DM> DerefMut for I8080Transfer<'d, BUF, DM> {
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.buf_view
     }
@@ -514,7 +514,7 @@ impl<'d, BUF: DmaTxBuffer> I8080Transfer<'d, BUF, Async> {
             type Output = ();
 
             fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
-                let interrupts = unsafe { NamesAreHard::steal() };
+                let interrupts = unsafe { InterruptControl::steal() };
                 WAKER.register(cx.waker());
                 if interrupts
                     .pending_interrupts()
@@ -532,7 +532,7 @@ impl<'d, BUF: DmaTxBuffer> I8080Transfer<'d, BUF, Async> {
 
         impl Drop for LcdDoneFuture {
             fn drop(&mut self) {
-                let interrupts = unsafe { NamesAreHard::steal() };
+                let interrupts = unsafe { InterruptControl::steal() };
                 interrupts.unlisten(LcdCamInterrupt::LcdTransDone);
             }
         }
@@ -541,7 +541,7 @@ impl<'d, BUF: DmaTxBuffer> I8080Transfer<'d, BUF, Async> {
     }
 }
 
-impl<'d, BUF: DmaTxBuffer, DM: Mode> Drop for I8080Transfer<'d, BUF, DM> {
+impl<'d, BUF: DmaTxBuffer, DM> Drop for I8080Transfer<'d, BUF, DM> {
     fn drop(&mut self) {
         self.stop_peripherals();
 

esp-hal/src/lcd_cam/lcd/mod.rs

@@ -10,14 +10,17 @@
 //! ## Implementation State
 //! - RGB is not supported yet
 
+use core::marker::PhantomData;
+
 use crate::{peripheral::PeripheralRef, peripherals::LCD_CAM};
 
 pub mod i8080;
 
 /// Represents an LCD interface.
-pub struct Lcd<'d> {
+pub struct Lcd<'d, M> {
     /// The `LCD_CAM` peripheral reference for managing the LCD functionality.
     pub(crate) lcd_cam: PeripheralRef<'d, LCD_CAM>,
+    pub(super) mode: PhantomData<M>,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Default)]

esp-hal/src/lcd_cam/mod.rs

@@ -18,20 +18,19 @@ use crate::{
     peripheral::Peripheral,
     peripherals::LCD_CAM,
     system::{self, PeripheralClockControl},
+    Blocking,
     InterruptConfigurable,
 };
 
 /// Represents a combined LCD and Camera interface.
-pub struct LcdCam<'d> {
+pub struct LcdCam<'d, M = BlockingWithInterrupts> {
     /// The LCD interface.
-    pub lcd: Lcd<'d>,
+    pub lcd: Lcd<'d, M>,
     /// The Camera interface.
     pub cam: Cam<'d>,
-    /// Interrupts
-    pub interrupts: NamesAreHard<'d>,
 }
 
-impl<'d> LcdCam<'d> {
+impl<'d> LcdCam<'d, BlockingWithInterrupts> {
     /// Creates a new `LcdCam` instance.
     pub fn new(lcd_cam: impl Peripheral<P = LCD_CAM> + 'd) -> Self {
         crate::into_ref!(lcd_cam);
@@ -42,13 +41,28 @@ impl<'d> LcdCam<'d> {
         Self {
             lcd: Lcd {
                 lcd_cam: unsafe { lcd_cam.clone_unchecked() },
+                mode: PhantomData,
             },
             cam: Cam {
                 lcd_cam: unsafe { lcd_cam.clone_unchecked() },
             },
-            interrupts: NamesAreHard(PhantomData),
         }
     }
+
+    /// Split out the [InterruptControl] from the driver.
+    pub fn split_interrupts(self) -> (LcdCam<'d, Blocking>, InterruptControl<'d>) {
+        let interrupts = unsafe { InterruptControl::steal() };
+        (
+            LcdCam {
+                lcd: Lcd {
+                    lcd_cam: self.lcd.lcd_cam,
+                    mode: PhantomData,
+                },
+                cam: self.cam,
+            },
+            interrupts,
+        )
+    }
 }
 
 /// Types of interrupts emitted by the LCD_CAM.
@@ -72,9 +86,9 @@ pub enum LcdCamInterrupt {
 }
 
 /// Access to the interrupt bits of the LCD_CAM.
-pub struct NamesAreHard<'d>(PhantomData<&'d ()>);
+pub struct InterruptControl<'d>(PhantomData<&'d ()>);
 
-impl<'d> NamesAreHard<'d> {
+impl<'d> InterruptControl<'d> {
     /// Listen for the given interrupts.
     pub fn listen(&self, interrupts: impl Into<EnumSet<LcdCamInterrupt>>) {
         self.enable_listen(interrupts.into(), true)
@@ -105,7 +119,7 @@ impl<'d> NamesAreHard<'d> {
         let interrupts = interrupts.into();
 
         let lcd_cam = unsafe { esp32s3::LCD_CAM::steal() };
-        lcd_cam.lc_dma_int_clr().modify(|_, w| {
+        lcd_cam.lc_dma_int_clr().write(|w| {
             for interrupt in interrupts {
                 match interrupt {
                     LcdCamInterrupt::CamHsync => w.cam_hs_int_clr().set_bit(),
@@ -201,9 +215,9 @@ impl<'d> NamesAreHard<'d> {
     }
 }
 
-impl<'d> crate::private::Sealed for NamesAreHard {}
+impl<'d> crate::private::Sealed for InterruptControl<'d> {}
 
-impl<'d> InterruptConfigurable for NamesAreHard<'d> {
+impl<'d> InterruptConfigurable for InterruptControl<'d> {
     fn set_interrupt_handler(&mut self, handler: InterruptHandler) {
         unsafe {
             crate::interrupt::bind_interrupt(
@@ -216,6 +230,9 @@ impl<'d> InterruptConfigurable for NamesAreHard<'d> {
     }
 }
 
+/// Same as [Blocking] but with interrupts access.
+pub struct BlockingWithInterrupts;
+
 /// LCD_CAM bit order
 #[derive(Debug, Clone, Copy, PartialEq, Default)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
@@ -243,13 +260,13 @@ pub mod asynch {
     use embassy_sync::waitqueue::AtomicWaker;
     use procmacros::handler;
 
-    use crate::lcd_cam::NamesAreHard;
+    use crate::lcd_cam::InterruptControl;
 
     pub(crate) static WAKER: AtomicWaker = AtomicWaker::new();
 
     #[handler]
     pub(crate) fn interrupt_handler() {
-        let int_access = unsafe { NamesAreHard::steal() };
+        let int_access = unsafe { InterruptControl::steal() };
 
         let active_interrupts = int_access.active_interrupts();
         if !active_interrupts.is_empty() {

examples/src/bin/lcd_i8080.rs

@@ -30,10 +30,10 @@ use esp_hal::{
     gpio::{Input, Io, Level, Output, Pull},
     lcd_cam::{
         lcd::i8080::{Config, TxEightBits, I8080},
+        BlockingWithInterrupts,
         LcdCam,
     },
     prelude::*,
-    Blocking,
 };
 use esp_println::println;
 
@@ -87,20 +87,22 @@ fn main() -> ! {
     // considerations, like how to manage the CS pin, the CD pin, cancellation semantics,
     // 8 vs 16 bit, non-native primitives like Rgb565, Rgb888, etc. This Bus is just provided as
     // an example of how to implement your own.
-    struct Bus<'d> {
-        resources: Option<(I8080<'d, Blocking>, DmaTxBuf)>,
+    struct Bus<'d, M> {
+        resources: Option<(I8080<'d, M>, DmaTxBuf)>,
     }
-    impl<'d> Bus<'d> {
+    impl<'d, M> Bus<'d, M> {
         fn use_resources<T>(
             &mut self,
-            func: impl FnOnce(I8080<'d, Blocking>, DmaTxBuf) -> (T, I8080<'d, Blocking>, DmaTxBuf),
+            func: impl FnOnce(I8080<'d, M>, DmaTxBuf) -> (T, I8080<'d, M>, DmaTxBuf),
         ) -> T {
             let (i8080, buf) = self.resources.take().unwrap();
             let (result, i8080, buf) = func(i8080, buf);
             self.resources = Some((i8080, buf));
             result
         }
+    }
 
+    impl<'d> Bus<'d, BlockingWithInterrupts> {
         pub fn send(&mut self, cmd: u8, data: &[u8]) {
             self.use_resources(|i8080, mut buf| {
                 buf.fill(data);

hil-test/tests/lcd_cam_i8080_async.rs

@@ -59,7 +59,7 @@ mod tests {
             20.MHz(),
             Config::default(),
         )
-        .into_async(ctx.lcd_cam.interrupts);
+        .into_async();
 
         let mut transfer = i8080.send(Command::<u8>::None, 0, ctx.dma_buf).unwrap();
 
@@ -86,7 +86,7 @@ mod tests {
             20.MHz(),
             Config::default(),
         )
-        .into_async(ctx.lcd_cam.interrupts);
+        .into_async();
 
         let mut transfer = i8080.send(Command::<u8>::None, 0, ctx.dma_buf).unwrap();