fix asynch uart TX #71

Merged
muellerr merged 1 commits from fix-asynch-uart into main 2026-05-04 19:32:26 +02:00
11 changed files with 98 additions and 63 deletions
@@ -5,10 +5,8 @@
use aarch32_cpu::asm::nop;
use core::panic::PanicInfo;
use embassy_executor::Spawner;
use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex;
use embassy_time::{Duration, Ticker};
use embedded_hal::digital::StatefulOutputPin;
use embedded_hal_async::delay::DelayNs as _;
use embedded_io::Write;
use log::{error, info};
use zynq7000::Peripherals;
@@ -68,17 +66,11 @@ async fn main(spawner: Spawner) -> ! {
uart.write_all(b"-- Zynq 7000 Logging example --\n\r")
.unwrap();
uart.flush().unwrap();
// Small delay to avoid logging glitches when creating an async UART.
embassy_time::Delay.delay_us(500).await;
let (tx, _rx) = uart.split();
let mut logger = TxAsync::new(tx);
static LOG_PIPE: static_cell::ConstStaticCell<
embassy_sync::pipe::Pipe<CriticalSectionRawMutex, 4096>,
> = static_cell::ConstStaticCell::new(embassy_sync::pipe::Pipe::new());
let (log_reader, log_writer) = LOG_PIPE.take().split();
zynq7000_hal::log::asynch::init(log::LevelFilter::Trace, log_writer);
let log_reader = zynq7000_hal::log::asynch::init(log::LevelFilter::Trace).unwrap();
let boot_mode = BootMode::new_from_regs();
info!("Boot mode: {:?}", boot_mode);
@@ -91,7 +83,8 @@ async fn main(spawner: Spawner) -> ! {
loop {
let read_bytes = log_reader.read(&mut log_buf).await;
if read_bytes > 0 {
logger.write(&log_buf[0..read_bytes]).await;
// Unwrap okay, checked that size is larger than 0.
logger.write(&log_buf[0..read_bytes]).unwrap().await;
}
}
}
@@ -92,12 +92,7 @@ async fn main(spawner: Spawner) -> ! {
uart.write_all(b"-- Zynq 7000 Zedboard SPI L3GD20H example --\n\r")
.unwrap();
static LOG_PIPE: static_cell::ConstStaticCell<
embassy_sync::pipe::Pipe<CriticalSectionRawMutex, 4096>,
> = static_cell::ConstStaticCell::new(embassy_sync::pipe::Pipe::new());
let (log_reader, log_writer) = LOG_PIPE.take().split();
zynq7000_hal::log::asynch::init(log::LevelFilter::Trace, log_writer);
let log_reader = zynq7000_hal::log::asynch::init(log::LevelFilter::Trace).unwrap();
let boot_mode = BootMode::new_from_regs();
info!("Boot mode: {:?}", boot_mode);
@@ -180,7 +175,9 @@ pub async fn logger_task(
let mut log_buf: [u8; 2048] = [0; 2048];
loop {
let read_bytes = reader.read(&mut log_buf).await;
tx_async.write(&log_buf[0..read_bytes]).await;
if read_bytes > 0 {
tx_async.write(&log_buf[0..read_bytes]).unwrap().await;
}
}
}
@@ -396,7 +396,7 @@ async fn uart_0_task(uart_tx: zynq7000_hal::uart::Tx) {
let mut idx = 0;
let print_strs = [str0.as_bytes(), str1.as_bytes()];
loop {
tx_async.write(print_strs[idx]).await;
tx_async.write(print_strs[idx]).unwrap().await;
idx += 1;
if idx == 2 {
idx = 0;
+2
View File
@@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Several bugfixes and improvements for GIC module. Some of the registers previously were
completely overwritten instead of only modifying their own bit portions. Also allow targeting
interrupts without clearing other CPU target.
- Do not reset the UART on TX future creation anymore, which lead to glitches and invalid data.
- Robustness improvements for the asynchronous UART TX module.
## Changed
+12 -6
View File
@@ -187,11 +187,15 @@ pub mod asynch {
unsafe impl Send for Logger {}
unsafe impl Sync for Logger {}
static LOGGER_PIPE: Logger = Logger {
static LOGGER: Logger = Logger {
pipe: core::cell::RefCell::new(None),
buf: critical_section::Mutex::new(core::cell::RefCell::new(heapless::String::new())),
};
static PIPE: static_cell::ConstStaticCell<
embassy_sync::pipe::Pipe<CriticalSectionRawMutex, 4096>,
> = static_cell::ConstStaticCell::new(embassy_sync::pipe::Pipe::new());
impl log::Log for Logger {
fn enabled(&self, _metadata: &log::Metadata) -> bool {
true
@@ -226,13 +230,15 @@ pub mod asynch {
pub fn init(
level: LevelFilter,
writer: embassy_sync::pipe::Writer<'static, CriticalSectionRawMutex, 4096>,
) {
//writer: embassy_sync::pipe::Writer<'static, CriticalSectionRawMutex, 4096>,
) -> Option<embassy_sync::pipe::Reader<'static, CriticalSectionRawMutex, 4096>> {
if super::LOGGER_INIT_DONE.swap(true, core::sync::atomic::Ordering::Relaxed) {
return;
return None;
}
LOGGER_PIPE.pipe.borrow_mut().replace(writer);
set_logger(&LOGGER_PIPE).unwrap();
let (reader, writer) = PIPE.take().split();
LOGGER.pipe.borrow_mut().replace(writer);
set_logger(&LOGGER).unwrap();
set_max_level(level); // Adjust as needed
Some(reader)
}
}
+1 -1
View File
@@ -573,7 +573,7 @@ impl Uart {
v
});
// Disable all interrupts.
reg_block.write_idr(InterruptControl::new_with_raw_value(0xFFFF_FFFF));
reg_block.write_interrupt_disable(InterruptControl::new_with_raw_value(0xFFFF_FFFF));
let mode = Mode::builder()
.with_chmode(cfg.chmode)
.with_nbstop(match cfg.stopbits {
+4 -4
View File
@@ -131,7 +131,7 @@ impl Rx {
/// It is recommended to also clear all interrupts immediately after enabling them.
#[inline]
pub fn enable_interrupts(&mut self) {
self.regs.write_ier(
self.regs.write_interrupt_enable(
InterruptControl::builder()
.with_tx_over(false)
.with_tx_near_full(false)
@@ -159,7 +159,7 @@ impl Rx {
reset_rx_timeout: bool,
) -> RxInterruptResult {
let mut result = RxInterruptResult::default();
let imr = self.regs.read_imr();
let imr = self.regs.read_enabled_interrupts();
if !imr.rx_full()
&& !imr.rx_trg()
&& !imr.rx_parity()
@@ -169,7 +169,7 @@ impl Rx {
{
return result;
}
let isr = self.regs.read_isr();
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() {
@@ -215,7 +215,7 @@ impl Rx {
/// This clears all RX related interrupts.
#[inline]
pub fn clear_interrupts(&mut self) {
self.regs.write_isr(
self.regs.write_interrupt_status(
InterruptStatus::builder()
.with_tx_over(false)
.with_tx_near_full(false)
+9 -9
View File
@@ -105,12 +105,12 @@ impl Tx {
/// Enables interrupts relevant for the TX side of the UART except the TX trigger interrupt.
#[inline]
pub fn enable_interrupts(&mut self) {
self.regs.write_ier(
pub fn enable_interrupts(&mut self, tx_trig: bool) {
self.regs.write_interrupt_enable(
InterruptControl::builder()
.with_tx_over(true)
.with_tx_near_full(true)
.with_tx_trig(false)
.with_tx_near_full(false)
.with_tx_trig(tx_trig)
.with_rx_dms(false)
.with_rx_timeout(false)
.with_rx_parity(false)
@@ -128,11 +128,11 @@ impl Tx {
/// Disable interrupts relevant for the TX side of the UART except the TX trigger interrupt.
#[inline]
pub fn disable_interrupts(&mut self) {
self.regs.write_idr(
self.regs.write_interrupt_disable(
InterruptControl::builder()
.with_tx_over(true)
.with_tx_near_full(true)
.with_tx_trig(false)
.with_tx_near_full(false)
.with_tx_trig(true)
.with_rx_dms(false)
.with_rx_timeout(false)
.with_rx_parity(false)
@@ -150,11 +150,11 @@ impl Tx {
/// Clears interrupts relevant for the TX side of the UART except the TX trigger interrupt.
#[inline]
pub fn clear_interrupts(&mut self) {
self.regs.write_isr(
self.regs.write_interrupt_status(
InterruptStatus::builder()
.with_tx_over(true)
.with_tx_near_full(true)
.with_tx_trig(false)
.with_tx_trig(true)
.with_rx_dms(false)
.with_rx_timeout(false)
.with_rx_parity(false)
+55 -20
View File
@@ -1,9 +1,11 @@
//! Asynchronous UART transmitter (TX) implementation.
use core::{cell::RefCell, convert::Infallible, sync::atomic::AtomicBool};
use core::{cell::RefCell, convert::Infallible, marker::PhantomData, sync::atomic::AtomicBool};
use arbitrary_int::u6;
use critical_section::Mutex;
use embassy_sync::waitqueue::AtomicWaker;
use raw_slice::RawBufSlice;
use zynq7000::uart::FifoTrigger;
use crate::uart::{FIFO_DEPTH, Tx, UartId};
@@ -22,15 +24,23 @@ static TX_DONE: [AtomicBool; 2] = [const { AtomicBool::new(false) }; 2];
pub fn on_interrupt_tx(peripheral: UartId) {
let mut tx_with_irq = unsafe { Tx::steal(peripheral) };
let idx = peripheral as usize;
let imr = tx_with_irq.regs().read_imr();
let enabled_irqs = tx_with_irq.regs().read_enabled_interrupts();
// IRQ is not related to TX.
if !imr.tx_over() && !imr.tx_near_full() && !imr.tx_full() && !imr.tx_empty() && !imr.tx_full()
if !enabled_irqs.tx_over()
&& !enabled_irqs.tx_near_full()
&& !enabled_irqs.tx_full()
&& !enabled_irqs.tx_empty()
&& !enabled_irqs.tx_full()
{
return;
}
let isr = tx_with_irq.regs().read_isr();
let unexpected_overrun = isr.tx_over();
let interrupt_status = tx_with_irq.regs().read_interrupt_status();
// Disable interrupts, re-enable them later.
tx_with_irq.disable_interrupts();
// Clear interrupts.
tx_with_irq.clear_interrupts();
let unexpected_overrun = interrupt_status.tx_over();
let mut context = critical_section::with(|cs| {
let context_ref = TX_CONTEXTS[idx].borrow(cs);
*context_ref.borrow()
@@ -41,7 +51,7 @@ pub fn on_interrupt_tx(peripheral: UartId) {
}
let slice_len = context.slice.len().unwrap();
context.tx_overrun = unexpected_overrun;
if (context.progress >= slice_len && isr.tx_empty()) || slice_len == 0 {
if (context.progress >= slice_len && interrupt_status.tx_empty()) || slice_len == 0 {
// Write back updated context structure.
critical_section::with(|cs| {
let context_ref = TX_CONTEXTS[idx].borrow(cs);
@@ -57,6 +67,8 @@ pub fn on_interrupt_tx(peripheral: UartId) {
// Safety: We documented that the user provided slice must outlive the future, so we convert
// the raw pointer back to the slice here.
let slice = unsafe { context.slice.get() }.expect("slice is invalid");
// Pump the FIFO.
while context.progress < slice_len {
if tx_with_irq.regs().read_sr().tx_full() {
break;
@@ -66,14 +78,22 @@ pub fn on_interrupt_tx(peripheral: UartId) {
tx_with_irq.write_fifo_unchecked(slice[context.progress]);
context.progress += 1;
}
let remaining = slice_len - context.progress;
if remaining > FIFO_DEPTH {
tx_with_irq.regs.write_tx_fifo_trigger(
FifoTrigger::builder()
.with_trig(u6::new((FIFO_DEPTH / 2) as u8))
.build(),
);
}
// Write back updated context structure.
critical_section::with(|cs| {
let context_ref = TX_CONTEXTS[idx].borrow(cs);
*context_ref.borrow_mut() = context;
});
// Clear interrupts.
tx_with_irq.clear_interrupts();
tx_with_irq.enable_interrupts(remaining > FIFO_DEPTH);
}
#[derive(Debug, Copy, Clone)]
@@ -95,16 +115,17 @@ impl TxContext {
}
/// Transmission future for UART TX.
pub struct TxFuture {
pub struct TxFuture<'uart> {
id: UartId,
marker: core::marker::PhantomData<&'uart ()>,
}
impl TxFuture {
impl<'uart> TxFuture<'uart> {
/// # Safety
///
/// This function stores the raw pointer of the passed data slice. The user MUST ensure
/// that the slice outlives the data structure.
pub unsafe fn new(tx_with_irq: &mut Tx, data: &[u8]) -> Self {
pub unsafe fn new(tx_with_irq: &'uart mut Tx, data: &[u8]) -> TxFuture<'uart> {
let idx = tx_with_irq.uart_idx() as usize;
TX_DONE[idx].store(false, core::sync::atomic::Ordering::Relaxed);
tx_with_irq.disable_interrupts();
@@ -119,19 +140,29 @@ impl TxFuture {
}
context.progress = init_fill_count; // We fill the FIFO.
});
tx_with_irq.enable(true);
// Apparently, we need to enable the UART before we are able to write something into
// the FIFO.
tx_with_irq.enable(false);
if data.len() > FIFO_DEPTH {
tx_with_irq.regs.write_tx_fifo_trigger(
FifoTrigger::builder()
.with_trig(u6::new((FIFO_DEPTH / 2) as u8))
.build(),
);
}
for data in data.iter().take(init_fill_count) {
tx_with_irq.write_fifo_unchecked(*data);
}
tx_with_irq.enable_interrupts();
tx_with_irq.enable_interrupts(data.len() > FIFO_DEPTH);
Self {
id: tx_with_irq.uart_idx(),
marker: PhantomData,
}
}
}
impl Future for TxFuture {
impl Future for TxFuture<'_> {
type Output = usize;
fn poll(
@@ -151,7 +182,7 @@ impl Future for TxFuture {
}
}
impl Drop for TxFuture {
impl Drop for TxFuture<'_> {
fn drop(&mut self) {
let mut tx = unsafe { Tx::steal(self.id) };
tx.disable_interrupts();
@@ -171,14 +202,15 @@ impl TxAsync {
/// Write a buffer asynchronously.
///
/// Returns [None] if the passed buffer is empty.
///
/// This implementation is not side effect free, and a started future might have already
/// written part of the passed buffer.
pub async fn write(&mut self, buf: &[u8]) -> usize {
pub fn write(&mut self, buf: &[u8]) -> Option<TxFuture<'_>> {
if buf.is_empty() {
return 0;
return None;
}
let fut = unsafe { TxFuture::new(&mut self.tx, buf) };
fut.await
Some(unsafe { TxFuture::new(&mut self.tx, buf) })
}
/// Release the underlying blocking TX driver.
@@ -197,7 +229,10 @@ impl embedded_io_async::Write for TxAsync {
/// This implementation is not side effect free, and a started future might have already
/// written part of the passed buffer.
async fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
Ok(self.write(buf).await)
if buf.is_empty() {
return Ok(0);
}
Ok(self.write(buf).unwrap().await)
}
/// This implementation does not do anything.
+2
View File
@@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
# [unreleased]
- Better names for interrupt registers for UART and SPI
# [v0.2.0] 2026-04-01
- Renamed all register blocks to `Registers` to subblocks to `<Subblock>Registers`.
+5 -5
View File
@@ -221,7 +221,7 @@ pub struct InterruptControl {
rx_trg: bool,
}
#[bitbybit::bitfield(u32)]
#[bitbybit::bitfield(u32, default = 0x0)]
#[derive(Debug)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub struct FifoTrigger {
@@ -330,16 +330,16 @@ pub struct Registers {
mr: Mode,
/// Interrupt enable register
#[mmio(Write)]
ier: InterruptControl,
interrupt_enable: InterruptControl,
/// Interrupt disable register
#[mmio(Write)]
idr: InterruptControl,
interrupt_disable: InterruptControl,
/// Interrupt mask register, showing enabled interrupts.
#[mmio(PureRead)]
imr: InterruptMask,
enabled_interrupts: InterruptMask,
/// Interrupt status register
#[mmio(PureRead, Write)]
isr: InterruptStatus,
interrupt_status: InterruptStatus,
/// Baudgen register
baudgen: Baudgen,
/// RX timeout register