More bootloader and flashloader improvements

flashloader/README.md

@@ -6,6 +6,9 @@ a simple PUS (CCSDS) interface to update the software. It also provides a Python
 called the `image-loader.py` which can be used to upload compiled images to the flashloader
 application to write them to the NVM.
 
+Please note that the both the application and the image loader are tailored towards usage
+with the [bootloader provided by this repository](https://egit.irs.uni-stuttgart.de/rust/va416xx-rs/src/branch/main/bootloader).
+
 The software can quickly be adapted to interface with a real primary on-board software instead of
 the Python script provided here to upload images because it uses a low-level CCSDS based packet
 interface.

flashloader/image-loader.py

@@ -1,6 +1,8 @@
 #!/usr/bin/env python3
+from typing import List
 from spacepackets.ecss.defs import PusService
 from spacepackets.ecss.tm import PusTm
+from tmtccmd.com import ComInterface
 import toml
 import struct
 import logging
@@ -24,16 +26,22 @@ BAUD_RATE = 115200
 BOOTLOADER_START_ADDR = 0x0
 BOOTLOADER_END_ADDR = 0x4000
 BOOTLOADER_CRC_ADDR = 0x3FFC
+BOOTLOADER_MAX_SIZE = BOOTLOADER_END_ADDR - BOOTLOADER_START_ADDR - 4
+
 APP_A_START_ADDR = 0x4000
 APP_A_END_ADDR = 0x22000
 # The actual size of the image which is relevant for CRC calculation.
 APP_A_SIZE_ADDR = 0x21FF8
 APP_A_CRC_ADDR = 0x21FFC
+APP_A_MAX_SIZE = APP_A_END_ADDR - APP_A_START_ADDR - 8
+
 APP_B_START_ADDR = 0x22000
 APP_B_END_ADDR = 0x40000
 # The actual size of the image which is relevant for CRC calculation.
 APP_B_SIZE_ADDR = 0x3FFF8
 APP_B_CRC_ADDR = 0x3FFFC
+APP_B_MAX_SIZE = APP_A_END_ADDR - APP_A_START_ADDR - 8
+
 APP_IMG_SZ = 0x1E000
 
 CHUNK_SIZE = 896
@@ -215,28 +223,60 @@ def main() -> int:
                 )
                 total_size += segment.header.p_filesz
             context_str = None
+
+            # Set context string and perform basic sanity checks.
             if args.target == "bl":
+                if total_size > BOOTLOADER_MAX_SIZE:
+                    _LOGGER.error(
+                        f"provided bootloader app larger than allowed {total_size} bytes"
+                    )
+                    return -1
                 context_str = "Bootloader"
             elif args.target == "a":
+                if total_size > APP_A_MAX_SIZE:
+                    _LOGGER.error(
+                        f"provided App A larger than allowed {total_size} bytes"
+                    )
+                    return -1
                 context_str = "App Slot A"
             elif args.target == "b":
+                if total_size > APP_B_MAX_SIZE:
+                    _LOGGER.error(
+                        f"provided App B larger than allowed {total_size} bytes"
+                    )
+                    return -1
                 context_str = "App Slot B"
             _LOGGER.info(
                 f"Flashing {context_str} with image {file_path} (size {total_size})"
             )
             for idx, segment in enumerate(loadable_segments):
                 _LOGGER.info(
-                    f"Loadable section {idx} {segment.name} with offset {segment.offset:#08x} and size {segment.size}"
+                    f"Loadable section {idx} {segment.name} with offset {segment.offset:#08x} and "
+                    f"size {segment.size}"
                 )
+            result = perform_flashing_algorithm(com_if, loadable_segments, verificator)
+            if result != 0:
+                return result
+            crc_and_app_size_postprocessing(
+                com_if, args.target, total_size, loadable_segments
+            )
+    com_if.close()
+    return 0
+
+
+def perform_flashing_algorithm(
+    com_if: ComInterface,
+    loadable_segments: List[LoadableSegment],
+    verificator: PusVerificator,
+) -> int:
+    # Perform the flashing algorithm.
     for segment in loadable_segments:
         segment_end = segment.offset + segment.size
         current_addr = segment.offset
         pos_in_segment = 0
         while pos_in_segment < segment.size:
             next_chunk_size = min(segment_end - current_addr, CHUNK_SIZE)
-                    data = segment.data[
+            data = segment.data[pos_in_segment : pos_in_segment + next_chunk_size]
-                        pos_in_segment : pos_in_segment + next_chunk_size
-                    ]
             next_packet = pack_memory_write_command(current_addr, data)
             _LOGGER.info(
                 f"Sending memory write command for address {current_addr:#08x} and data with "
@@ -246,7 +286,11 @@ def main() -> int:
             com_if.send(next_packet.pack())
             current_addr += next_chunk_size
             pos_in_segment += next_chunk_size
+            start_time = time.time()
             while True:
+                if time.time() - start_time > 1.0:
+                    _LOGGER.error("Timeout while waiting for reply")
+                    return -1
                 data_available = com_if.data_available(0.1)
                 done = False
                 if not data_available:
@@ -266,12 +310,29 @@ def main() -> int:
                         and check_result.status.completed == StatusField.SUCCESS
                     ):
                         done = True
-                                # Still keep a small delay
+
-                                # time.sleep(0.05)
+                    # This is an optimized variant, but I think the small delay is not an issue.
+                    """
+                    if (
+                        check_result is not None
+                        and check_result.status.step == StatusField.SUCCESS
+                        and len(check_result.status.step_list) == 1
+                    ):
+                        done = True
+                    """
                 verificator.remove_completed_entries()
                 if done:
                     break
-            if args.target == "bl":
+    return 0
+
+
+def crc_and_app_size_postprocessing(
+    com_if: ComInterface,
+    target: str,
+    total_size: int,
+    loadable_segments: List[LoadableSegment],
+):
+    if target == "bl":
         _LOGGER.info("Blanking the bootloader checksum")
         # Blank the checksum. For the bootloader, the bootloader will calculate the
         # checksum itself on the initial run.
@@ -282,17 +343,15 @@ def main() -> int:
     else:
         crc_addr = None
         size_addr = None
-                if args.target == "a":
+        if target == "a":
             crc_addr = APP_A_CRC_ADDR
             size_addr = APP_A_SIZE_ADDR
-                elif args.target == "b":
+        elif target == "b":
             crc_addr = APP_B_CRC_ADDR
             size_addr = APP_B_SIZE_ADDR
         assert crc_addr is not None
         assert size_addr is not None
-                _LOGGER.info(
+        _LOGGER.info(f"Writing app size {total_size} at address {size_addr:#08x}")
-                    f"Writing app size {total_size} at address {size_addr:#08x}"
-                )
         size_write_packet = pack_memory_write_command(
             size_addr, struct.pack("!I", total_size)
         )
@@ -307,8 +366,6 @@ def main() -> int:
         )
         checksum_write_packet = pack_memory_write_command(crc_addr, checksum)
         com_if.send(checksum_write_packet.pack())
-    com_if.close()
-    return 0
 
 
 def pack_memory_write_command(addr: int, data: bytes) -> PusTc:

flashloader/slot-a-blinky/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2021"
 
 [dependencies]
 cortex-m-rt = "0.7"
+va416xx-hal = { path = "../../va416xx-hal", features = ["va41630"] }
 panic-rtt-target = { version = "0.1.3" }
 rtt-target = { version = "0.5" }
 cortex-m = { version = "0.7", features = ["critical-section-single-core"] }

flashloader/slot-b-blinky/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2021"
 
 [dependencies]
 cortex-m-rt = "0.7"
+va416xx-hal = { path = "../../va416xx-hal", features = ["va41630"] }
 panic-rtt-target = { version = "0.1.3" }
 rtt-target = { version = "0.5" }
 cortex-m = { version = "0.7", features = ["critical-section-single-core"] }

flashloader/src/main.rs

@@ -109,6 +109,7 @@ mod app {
         tc::PusTcReader, tm::PusTmCreator, EcssEnumU8, PusPacket, WritablePusPacket,
     };
     use va416xx_hal::irq_router::enable_and_init_irq_router;
+    use va416xx_hal::uart::IrqContextTimeoutOrMaxSize;
     use va416xx_hal::{
         clock::ClkgenExt,
         edac,
@@ -132,6 +133,7 @@ mod app {
     struct Local {
         uart_rx: uart::RxWithIrq<pac::Uart0>,
         uart_tx: uart::Tx<pac::Uart0>,
+        rx_context: IrqContextTimeoutOrMaxSize,
         rom_spi: Option<pac::Spi3>,
         // We handle all TM in one task.
         tm_cons: DataConsumer<BUF_RB_SIZE_TM, SIZES_RB_SIZE_TM>,
@@ -191,7 +193,8 @@ mod app {
         Mono::start(cx.core.SYST, clocks.sysclk().raw());
         CLOCKS.set(clocks).unwrap();
 
-        rx.read_fixed_len_using_irq(MAX_TC_FRAME_SIZE, true)
+        let mut rx_context = IrqContextTimeoutOrMaxSize::new(MAX_TC_FRAME_SIZE);
+        rx.read_fixed_len_or_timeout_based_using_irq(&mut rx_context)
             .expect("initiating UART RX failed");
         pus_tc_handler::spawn().unwrap();
         pus_tm_tx_handler::spawn().unwrap();
@@ -205,6 +208,7 @@ mod app {
             Local {
                 uart_rx: rx,
                 uart_tx: tx,
+                rx_context,
                 rom_spi: Some(cx.device.spi3),
                 tm_cons: DataConsumer {
                     buf_cons: buf_cons_tm,
@@ -231,20 +235,26 @@ mod app {
         }
     }
 
+    // This is the interrupt handler to read all bytes received on the UART0.
     #[task(
         binds = UART0_RX,
         local = [
             cnt: u32 = 0,
             rx_buf: [u8; MAX_TC_FRAME_SIZE] = [0; MAX_TC_FRAME_SIZE],
+            rx_context,
             uart_rx,
             tc_prod
         ],
     )]
     fn uart_rx_irq(cx: uart_rx_irq::Context) {
-        match cx.local.uart_rx.irq_handler(cx.local.rx_buf) {
+        match cx
+            .local
+            .uart_rx
+            .irq_handler_max_size_or_timeout_based(cx.local.rx_context, cx.local.rx_buf)
+        {
             Ok(result) => {
                 if RX_DEBUGGING {
-                    log::debug!("RX Info: {:?}", cx.local.uart_rx.irq_info());
+                    log::debug!("RX Info: {:?}", cx.local.rx_context);
                     log::debug!("RX Result: {:?}", result);
                 }
                 if result.complete() {
@@ -279,7 +289,7 @@ mod app {
                     // Initiate next transfer.
                     cx.local
                         .uart_rx
-                        .read_fixed_len_using_irq(MAX_TC_FRAME_SIZE, true)
+                        .read_fixed_len_or_timeout_based_using_irq(cx.local.rx_context)
                         .expect("read operation failed");
                 }
                 if result.error() {
@@ -438,7 +448,12 @@ mod app {
                     return;
                 }
                 let data = &app_data[10..10 + data_len as usize];
-                log::info!("writing {} bytes at offset {} to NVM", data_len, offset);
+                log::info!(
+                    target: "TC Handler",
+                    "writing {} bytes at offset {} to NVM",
+                    data_len,
+                    offset
+                );
                 // Safety: We only use this for NVM handling and we only do NVM
                 // handling here.
                 let mut sys_cfg = unsafe { pac::Sysconfig::steal() };
@@ -455,7 +470,9 @@ mod app {
                     .completion_success(cx.local.src_data_buf, started_token, 0, 0, &[])
                     .expect("completion success failed");
                 write_and_send(&tm);
-                log::info!("NVM operation done");
+                log::info!(
+                    target: "TC Handler",
+                    "NVM operation done");
             }
         }
     }

va416xx-hal/src/uart.rs

@@ -198,25 +198,49 @@ impl From<Hertz> for Config {
 // IRQ Definitions
 //==================================================================================================
 
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
-pub struct IrqInfo {
+pub struct IrqContextTimeoutOrMaxSize {
-    rx_len: usize,
     rx_idx: usize,
     mode: IrqReceptionMode,
+    pub max_len: usize,
+}
+
+impl IrqContextTimeoutOrMaxSize {
+    pub fn new(max_len: usize) -> Self {
+        IrqContextTimeoutOrMaxSize {
+            rx_idx: 0,
+            max_len,
+            mode: IrqReceptionMode::Idle,
+        }
+    }
+}
+
+impl IrqContextTimeoutOrMaxSize {
+    pub fn reset(&mut self) {
+        self.rx_idx = 0;
+        self.mode = IrqReceptionMode::Idle;
+    }
 }
 
 /// This struct is used to return the default IRQ handler result to the user
 #[derive(Debug, Default)]
 pub struct IrqResult {
+    pub bytes_read: usize,
+    pub errors: IrqUartError,
+}
+
+/// This struct is used to return the default IRQ handler result to the user
+#[derive(Debug, Default)]
+pub struct IrqResultMaxSizeTimeout {
     complete: bool,
     timeout: bool,
     pub errors: IrqUartError,
     pub bytes_read: usize,
 }
 
-impl IrqResult {
+impl IrqResultMaxSizeTimeout {
     pub fn new() -> Self {
-        IrqResult {
+        IrqResultMaxSizeTimeout {
             complete: false,
             timeout: false,
             errors: IrqUartError::default(),
@@ -224,7 +248,7 @@ impl IrqResult {
         }
     }
 }
-impl IrqResult {
+impl IrqResultMaxSizeTimeout {
     #[inline]
     pub fn error(&self) -> bool {
         if self.errors.overflow || self.errors.parity || self.errors.framing {
@@ -259,7 +283,7 @@ impl IrqResult {
     }
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Copy, Clone)]
 enum IrqReceptionMode {
     Idle,
     Pending,
@@ -287,7 +311,7 @@ pub struct Rx<Uart>(Uart);
 // Serial receiver, using interrupts to offload reading to the hardware.
 pub struct RxWithIrq<Uart> {
     inner: Rx<Uart>,
-    irq_info: IrqInfo,
+    // irq_info: IrqContextTimeoutOrMaxSize,
 }
 
 /// Serial transmitter
@@ -564,18 +588,7 @@ impl<TxPinInst: TxPin<UartInstance>, RxPinInst: RxPin<UartInstance>, UartInstanc
     ) {
         let (inner, pins) = self.downgrade_internal();
         let (tx, rx) = inner.split();
-        (
+        (tx, RxWithIrq { inner: rx }, pins)
-            tx,
-            RxWithIrq {
-                inner: rx,
-                irq_info: IrqInfo {
-                    rx_len: 0,
-                    rx_idx: 0,
-                    mode: IrqReceptionMode::Idle,
-                },
-            },
-            pins,
-        )
     }
 
     delegate::delegate! {
@@ -703,28 +716,32 @@ pub enum IrqError {
 }
 
 impl<Uart: Instance> RxWithIrq<Uart> {
-    /// This initializes a non-blocking read transfer using the IRQ capabilities of the UART
+    /// This function should be called once at initialization time if the regular
-    /// peripheral.
+    /// [Self::irq_handler] is used to read the UART receiver to enable and start the receiver.
+    pub fn start(&mut self) {
+        self.inner.enable();
+        self.enable_rx_irq_sources(true);
+        unsafe { enable_interrupt(Uart::IRQ_RX) };
+    }
+
+    /// This function is used together with the [Self::irq_handler_max_size_or_timeout_based]
+    /// function to read packets with a maximum size or variable sized packets by using the
+    /// receive timeout of the hardware.
     ///
-    /// The only required information is the maximum length for variable sized reception
+    /// This function should be called once at initialization to initiate the context state
-    /// or the expected length for fixed length reception. If variable sized packets are expected,
+    /// and to [Self::start] the receiver. After that, it should be called after each
-    /// the timeout functionality of the IRQ should be enabled as well. After calling this function,
+    /// completed [Self::irq_handler_max_size_or_timeout_based] call to restart the reception
-    /// the [`irq_handler`](Self::irq_handler) function should be called in the user interrupt
+    /// of a packet.
-    /// handler to read the received packets and reinitiate another transfer if desired.
+    pub fn read_fixed_len_or_timeout_based_using_irq(
-    pub fn read_fixed_len_using_irq(
         &mut self,
-        max_len: usize,
+        context: &mut IrqContextTimeoutOrMaxSize,
-        enb_timeout_irq: bool,
     ) -> Result<(), Error> {
-        if self.irq_info.mode != IrqReceptionMode::Idle {
+        if context.mode != IrqReceptionMode::Idle {
             return Err(Error::TransferPending);
         }
-        self.irq_info.mode = IrqReceptionMode::Pending;
+        context.mode = IrqReceptionMode::Pending;
-        self.irq_info.rx_idx = 0;
+        context.rx_idx = 0;
-        self.irq_info.rx_len = max_len;
+        self.start();
-        self.inner.enable();
-        self.enable_rx_irq_sources(enb_timeout_irq);
-        unsafe { enable_interrupt(Uart::IRQ_RX) };
         Ok(())
     }
 
@@ -751,26 +768,89 @@ impl<Uart: Instance> RxWithIrq<Uart> {
     pub fn cancel_transfer(&mut self) {
         self.disable_rx_irq_sources();
         self.inner.clear_fifo();
-        self.irq_info.rx_idx = 0;
-        self.irq_info.rx_len = 0;
     }
 
     pub fn uart(&self) -> &Uart {
         &self.inner.0
     }
 
-    /// Default IRQ handler which can be used to read the packets arriving on the UART peripheral.
+    /// This function should be called in the user provided UART interrupt handler.
+    ///
+    /// It simply empties any bytes in the FIFO into the user provided buffer and returns the
+    /// result of the operation.
+    ///
+    /// This function will not disable the RX interrupts, so you don't need to call any other
+    /// API after calling this function to continue emptying the FIFO.
+    pub fn irq_handler(&mut self, buf: &mut [u8; 16]) -> Result<IrqResult, IrqUartError> {
+        let mut result = IrqResult::default();
+        let mut current_idx = 0;
+
+        let irq_end = self.inner.0.irq_end().read();
+        let enb_status = self.inner.0.enable().read();
+        let rx_enabled = enb_status.rxenable().bit_is_set();
+
+        // Half-Full interrupt. We have a guaranteed amount of data we can read.
+        if irq_end.irq_rx().bit_is_set() {
+            // Determine the number of bytes to read, ensuring we leave 1 byte in the FIFO.
+            // We use this trick/hack because the timeout feature of the peripheral relies on data
+            // being in the RX FIFO. If data continues arriving, another half-full IRQ will fire.
+            // If not, the last byte(s) is/are emptied by the timeout interrupt.
+            let available_bytes = self.inner.0.rxfifoirqtrg().read().bits() as usize;
+
+            // If this interrupt bit is set, the trigger level is available at the very least.
+            // Read everything as fast as possible
+            for _ in 0..available_bytes {
+                buf[current_idx] = (self.inner.0.data().read().bits() & 0xff) as u8;
+                current_idx += 1;
+            }
+        }
+
+        // Timeout, empty the FIFO completely.
+        if irq_end.irq_rx_to().bit_is_set() {
+            let read_result = self.inner.read();
+            // While there is data in the FIFO, write it into the reception buffer
+            while let Some(byte) = self.read_handler(&mut result.errors, &read_result) {
+                buf[current_idx] = byte;
+                current_idx += 1;
+            }
+        }
+
+        // RX transfer not complete, check for RX errors
+        if rx_enabled {
+            self.check_for_errors(&mut result.errors);
+        }
+
+        // Clear the interrupt status bits
+        self.inner
+            .0
+            .irq_clr()
+            .write(|w| unsafe { w.bits(irq_end.bits()) });
+        Ok(result)
+    }
+
+    /// This function should be called in the user provided UART interrupt handler.
+    ///
+    /// This function is used to read packets which either have a maximum size or variable sized
+    /// packet which are bounded by sufficient delays between them, triggering a hardware timeout.
+    ///
+    /// If either the maximum number of packets have been read or a timeout occured, the transfer
+    /// will be deemed completed. The state information of the transfer is tracked in the
+    /// [IrqContextTimeoutOrMaxSize] structure.
     ///
     /// If passed buffer is equal to or larger than the specified maximum length, an
     /// [`Error::BufferTooShort`] will be returned
-    pub fn irq_handler(&mut self, buf: &mut [u8]) -> Result<IrqResult, IrqError> {
+    pub fn irq_handler_max_size_or_timeout_based(
-        if buf.len() < self.irq_info.rx_len {
+        &mut self,
+        context: &mut IrqContextTimeoutOrMaxSize,
+        buf: &mut [u8],
+    ) -> Result<IrqResultMaxSizeTimeout, IrqError> {
+        if buf.len() < context.max_len {
             return Err(IrqError::BufferTooShort {
                 found: buf.len(),
-                expected: self.irq_info.rx_len,
+                expected: context.max_len,
             });
         }
-        let mut res = IrqResult::default();
+        let mut result = IrqResultMaxSizeTimeout::default();
 
         let irq_end = self.inner.0.irq_end().read();
         let enb_status = self.inner.0.enable().read();
@@ -786,14 +866,14 @@ impl<Uart: Instance> RxWithIrq<Uart> {
 
             let bytes_to_read = core::cmp::min(
                 available_bytes.saturating_sub(1),
-                self.irq_info.rx_len - self.irq_info.rx_idx,
+                context.max_len - context.rx_idx,
             );
 
             // If this interrupt bit is set, the trigger level is available at the very least.
             // Read everything as fast as possible
             for _ in 0..bytes_to_read {
-                buf[self.irq_info.rx_idx] = (self.inner.0.data().read().bits() & 0xff) as u8;
+                buf[context.rx_idx] = (self.inner.0.data().read().bits() & 0xff) as u8;
-                self.irq_info.rx_idx += 1;
+                context.rx_idx += 1;
             }
 
             // On high-baudrates, data might be available immediately, and we possible have to
@@ -801,67 +881,28 @@ impl<Uart: Instance> RxWithIrq<Uart> {
             // rely on the hardware firing another IRQ. I have not tried baudrates higher than
             // 115200 so far.
         }
-        let read_handler =
-            |possible_error: &mut IrqUartError, read_res: nb::Result<u8, Error>| -> Option<u8> {
-                match read_res {
-                    Ok(byte) => Some(byte),
-                    Err(nb::Error::WouldBlock) => None,
-                    Err(nb::Error::Other(e)) => {
-                        match e {
-                            Error::Overrun => {
-                                possible_error.overflow = true;
-                            }
-                            Error::FramingError => {
-                                possible_error.framing = true;
-                            }
-                            Error::ParityError => {
-                                possible_error.parity = true;
-                            }
-                            _ => {
-                                possible_error.other = true;
-                            }
-                        }
-                        None
-                    }
-                }
-            };
         // Timeout, empty the FIFO completely.
         if irq_end.irq_rx_to().bit_is_set() {
             // While there is data in the FIFO, write it into the reception buffer
             loop {
-                if self.irq_info.rx_idx == self.irq_info.rx_len {
+                if context.rx_idx == context.max_len {
                     break;
                 }
-                if let Some(byte) = read_handler(&mut res.errors, self.inner.read()) {
+                let read_result = self.inner.read();
-                    buf[self.irq_info.rx_idx] = byte;
+                if let Some(byte) = self.read_handler(&mut result.errors, &read_result) {
-                    self.irq_info.rx_idx += 1;
+                    buf[context.rx_idx] = byte;
+                    context.rx_idx += 1;
                 } else {
                     break;
                 }
             }
-            self.irq_completion_handler(&mut res);
+            self.irq_completion_handler_max_size_timeout(&mut result, context);
-            return Ok(res);
+            return Ok(result);
         }
 
         // RX transfer not complete, check for RX errors
-        if (self.irq_info.rx_idx < self.irq_info.rx_len) && rx_enabled {
+        if (context.rx_idx < context.max_len) && rx_enabled {
-            // Read status register again, might have changed since reading received data
+            self.check_for_errors(&mut result.errors);
-            let rx_status = self.inner.0.rxstatus().read();
-            if rx_status.rxovr().bit_is_set() {
-                res.errors.overflow = true;
-            }
-            if rx_status.rxfrm().bit_is_set() {
-                res.errors.framing = true;
-            }
-            if rx_status.rxpar().bit_is_set() {
-                res.errors.parity = true;
-            }
-
-            // If it is not a timeout, it's an error
-            if res.error() {
-                self.disable_rx_irq_sources();
-                return Err(IrqError::Uart(res.errors));
-            }
         }
 
         // Clear the interrupt status bits
@@ -869,21 +910,62 @@ impl<Uart: Instance> RxWithIrq<Uart> {
             .0
             .irq_clr()
             .write(|w| unsafe { w.bits(irq_end.bits()) });
-        Ok(res)
+        Ok(result)
     }
 
-    fn irq_completion_handler(&mut self, res: &mut IrqResult) {
+    fn read_handler(
+        &self,
+        errors: &mut IrqUartError,
+        read_res: &nb::Result<u8, Error>,
+    ) -> Option<u8> {
+        match read_res {
+            Ok(byte) => Some(*byte),
+            Err(nb::Error::WouldBlock) => None,
+            Err(nb::Error::Other(e)) => {
+                match e {
+                    Error::Overrun => {
+                        errors.overflow = true;
+                    }
+                    Error::FramingError => {
+                        errors.framing = true;
+                    }
+                    Error::ParityError => {
+                        errors.parity = true;
+                    }
+                    _ => {
+                        errors.other = true;
+                    }
+                }
+                None
+            }
+        }
+    }
+
+    fn check_for_errors(&self, errors: &mut IrqUartError) {
+        // Read status register again, might have changed since reading received data
+        let rx_status = self.inner.0.rxstatus().read();
+        if rx_status.rxovr().bit_is_set() {
+            errors.overflow = true;
+        }
+        if rx_status.rxfrm().bit_is_set() {
+            errors.framing = true;
+        }
+        if rx_status.rxpar().bit_is_set() {
+            errors.parity = true;
+        }
+    }
+
+    fn irq_completion_handler_max_size_timeout(
+        &mut self,
+        res: &mut IrqResultMaxSizeTimeout,
+        context: &mut IrqContextTimeoutOrMaxSize,
+    ) {
         self.disable_rx_irq_sources();
         self.inner.disable();
-        res.bytes_read = self.irq_info.rx_idx;
+        res.bytes_read = context.rx_idx;
         res.complete = true;
-        self.irq_info.mode = IrqReceptionMode::Idle;
+        context.mode = IrqReceptionMode::Idle;
-        self.irq_info.rx_idx = 0;
+        context.rx_idx = 0;
-        self.irq_info.rx_len = 0;
-    }
-
-    pub fn irq_info(&self) -> &IrqInfo {
-        &self.irq_info
     }
 
     pub fn release(self) -> Uart {