From fcd971a7d3da852ac218531cadadf5c8477c8e4c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 15 May 2026 14:25:04 +0200 Subject: [PATCH] UART improvements --- .../zedboard/src/bin/uart-non-blocking.rs | 32 ++++++++++++++++--- firmware/zynq7000-hal/CHANGELOG.md | 3 ++ firmware/zynq7000-hal/src/uart/rx.rs | 16 +++++----- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/firmware/examples/zedboard/src/bin/uart-non-blocking.rs b/firmware/examples/zedboard/src/bin/uart-non-blocking.rs index a23c44d..6a0115b 100644 --- a/firmware/examples/zedboard/src/bin/uart-non-blocking.rs +++ b/firmware/examples/zedboard/src/bin/uart-non-blocking.rs @@ -50,6 +50,7 @@ use zynq7000_hal::{ uart::{ClockConfig, Config, Uart}, }; +#[derive(Debug, Copy, Clone, PartialEq)] pub enum UartMode { Uart0ToUartlite, Uart0ToUart16550, @@ -74,6 +75,14 @@ const AXI_UAR16550_BASE_ADDR: u32 = 0x43C0_0000; pub const UARTLITE_PL_INT_ID: usize = 0; pub const UART16550_PL_INT_ID: usize = 1; +pub const UART_SPEED: u32 = 115_200; + +// Other common baud rates to test with: + +// pub const UART_SPEED: u32 = 9600; +// pub const UART_SPEED: u32 = 230_400; +// pub const UART_SPEED: u32 = 912_600; + const RB_SIZE: usize = 512; // These queues are used to send all data received in the UART interrupt handlers to the main @@ -239,15 +248,25 @@ async fn main(spawner: Spawner) -> ! { Output::new_for_emio(gpio_pins.emio.take(9).unwrap(), PinState::Low), Output::new_for_emio(gpio_pins.emio.take(10).unwrap(), PinState::Low), ]); + let mut uart_speed = UART_SPEED; match UART_MODE { UartMode::Uart0ToUartlite => uart_mux.select(UartSel::Uart0ToUartlite), UartMode::Uart0ToUart16550 => uart_mux.select(UartSel::Uart0ToUart16550), UartMode::UartliteToUart16550 => uart_mux.select(UartSel::UartliteToUart16550), } + if (UART_MODE == UartMode::Uart0ToUartlite || UART_MODE == UartMode::UartliteToUart16550) + && uart_speed != 115200 + { + log::warn!("UARTLITE speed is not configurable. Hardcoding UART speed to 115200"); + uart_speed = 115200; + } + let uart0_clk_config = ClockConfig::new_autocalc_with_error(clocks.io_clocks(), uart_speed) + .unwrap() + .0; // UART0 routed through EMIO to PL pins. let uart_0 = - Uart::new_with_emio(dp.uart_0, Config::new_with_clk_config(uart_clk_config)).unwrap(); + Uart::new_with_emio(dp.uart_0, Config::new_with_clk_config(uart0_clk_config)).unwrap(); // Safety: Valid address of AXI UARTLITE. let mut uartlite = unsafe { AxiUartlite::new(AXI_UARTLITE_BASE_ADDR) }; // We need to call this before splitting the structure, because the interrupt signal is @@ -256,10 +275,15 @@ async fn main(spawner: Spawner) -> ! { let (clk_config, error) = axi_uart16550::ClockConfig::new_autocalc_with_error( fugit_03::HertzU32::from_raw(clocks.pl_clocks()[0].to_raw()), - 115200, + uart_speed, ) .unwrap(); - assert!(error < 0.02); + if error > 0.02 { + log::warn!( + "Calculated clock config for AXI UART16550 has error of {} %, which is higher than 2%. This may lead to incorrect baud rate. Consider changing the input clock or the target baud rate.", + (error * 100.0) + ); + } let _uart_16550 = unsafe { AxiUart16550::new( AXI_UAR16550_BASE_ADDR, @@ -289,7 +313,7 @@ async fn main(spawner: Spawner) -> ! { let (uartlite_prod, mut uartlite_cons) = QUEUE_UARTLITE.take().split(); let (uart16550_prod, mut uart16550_cons) = QUEUE_UART16550.take().split(); // Use our helper function to start RX handling. - uart_0_rx.start_interrupt_driven_reception(); + uart_0_rx.start_interrupt_driven_reception(0xFF); // Use our helper function to start RX handling. uart_16550_rx.start_interrupt_driven_reception(); critical_section::with(|cs| { diff --git a/firmware/zynq7000-hal/CHANGELOG.md b/firmware/zynq7000-hal/CHANGELOG.md index 87ca800..8fcc6ad 100644 --- a/firmware/zynq7000-hal/CHANGELOG.md +++ b/firmware/zynq7000-hal/CHANGELOG.md @@ -22,6 +22,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Changed +- Increased reliabily of PS UART interrupt reception, which was proven to be buggy for higher baud + rates: Force user to configure RTO value, encouraging non-zero values, and use a RX FIFO trigger + value of FIFO depth divided by 2 by default. - `devcfg` moved to `pl` module - Added division by zero check in gtc frequency_to_ticks to avoid runtime panic - Increased UART type safety by providing dedicated MIO constructors for UART 0 and UART 1 diff --git a/firmware/zynq7000-hal/src/uart/rx.rs b/firmware/zynq7000-hal/src/uart/rx.rs index 306a408..1212d89 100644 --- a/firmware/zynq7000-hal/src/uart/rx.rs +++ b/firmware/zynq7000-hal/src/uart/rx.rs @@ -114,8 +114,14 @@ impl Rx { /// /// This should be called once at system start-up. After that, you only need to call /// [Self::on_interrupt] in the interrupt handler for the UART peripheral. - pub fn start_interrupt_driven_reception(&mut self) { + /// + /// You can also configure a RX timeout by setting the RX timeout value `rto` which has a unit + /// of bit periods times 4. Setting a value of 0 disables the timeout feature of the hardware, + /// but this is strongly discouraged. + pub fn start_interrupt_driven_reception(&mut self, rto: u8) { self.soft_reset(); + self.set_rx_fifo_trigger_level((FIFO_DEPTH / 2) as u8); + self.set_rx_timeout_value(rto); self.clear_interrupts(); self.enable_interrupts(); } @@ -170,13 +176,7 @@ impl Rx { return result; } let isr = self.regs.read_interrupt_status(); - if isr.rx_full() { - // Read all bytes in the full RX fifo. - for byte in buf.iter_mut() { - *byte = self.read_fifo_unchecked(); - } - result.read_bytes = FIFO_DEPTH; - } else if isr.rx_trg() { + if self.regs.read_interrupt_status().rx_trg() { // It is guaranteed that we can read the FIFO level amount of data let fifo_trigger = self.regs.read_rx_fifo_trigger().trig().as_usize(); (0..fifo_trigger).for_each(|i| { -- 2.43.0