From 4c34132337f12af079f4c81f5f6e0ff03cf9c728 Mon Sep 17 00:00:00 2001 From: Mathias Date: Wed, 17 Jul 2024 12:20:31 +0200 Subject: [PATCH] Correctly handle modifying LCR register after uart enable --- embassy-rp/src/uart/mod.rs | 73 ++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/embassy-rp/src/uart/mod.rs b/embassy-rp/src/uart/mod.rs index f7c90e97b..f546abe71 100644 --- a/embassy-rp/src/uart/mod.rs +++ b/embassy-rp/src/uart/mod.rs @@ -860,6 +860,56 @@ impl<'d, T: Instance + 'd, M: Mode> Uart<'d, T, M> { }); } + fn lcr_modify(f: impl FnOnce(&mut rp_pac::uart::regs::UartlcrH) -> R) -> R { + let r = T::regs(); + + // Notes from PL011 reference manual: + // + // - Before writing the LCR, if the UART is enabled it needs to be + // disabled and any current TX + RX activity has to be completed + // + // - There is a BUSY flag which waits for the current TX char, but this is + // OR'd with TX FIFO !FULL, so not usable when FIFOs are enabled and + // potentially nonempty + // + // - FIFOs can't be set to disabled whilst a character is in progress + // (else "FIFO integrity is not guaranteed") + // + // Combination of these means there is no general way to halt and poll for + // end of TX character, if FIFOs may be enabled. Either way, there is no + // way to poll for end of RX character. + // + // So, insert a 15 Baud period delay before changing the settings. + // 15 Baud is comfortably higher than start + max data + parity + stop. + // Anything else would require API changes to permit a non-enabled UART + // state after init() where settings can be changed safely. + let clk_base = crate::clocks::clk_peri_freq(); + + let cr = r.uartcr().read(); + if cr.uarten() { + r.uartcr().modify(|w| { + w.set_uarten(false); + w.set_txe(false); + w.set_rxe(false); + }); + + // Note: Maximise precision here. Show working, the compiler will mop this up. + // Create a 16.6 fixed-point fractional division ratio; then scale to 32-bits. + let mut brdiv_ratio = 64 * r.uartibrd().read().0 + r.uartfbrd().read().0; + brdiv_ratio <<= 10; + // 3662 is ~(15 * 244.14) where 244.14 is 16e6 / 2^16 + let scaled_freq = clk_base / 3662; + let wait_time_us = brdiv_ratio / scaled_freq; + embedded_hal_1::delay::DelayNs::delay_us(&mut Delay, wait_time_us); + } + + let res = r.uartlcr_h().modify(f); + + r.uartcr().write_value(cr); + + res + } + /// sets baudrate on runtime pub fn set_baudrate(&mut self, baudrate: u32) { Self::set_baudrate_inner(baudrate); @@ -886,28 +936,7 @@ impl<'d, T: Instance + 'd, M: Mode> Uart<'d, T, M> { r.uartibrd().write_value(pac::uart::regs::Uartibrd(baud_ibrd)); r.uartfbrd().write_value(pac::uart::regs::Uartfbrd(baud_fbrd)); - let cr = r.uartcr().read(); - if cr.uarten() { - r.uartcr().modify(|w| { - w.set_uarten(false); - w.set_txe(false); - w.set_rxe(false); - }); - - // Note: Maximise precision here. Show working, the compiler will mop this up. - // Create a 16.6 fixed-point fractional division ratio; then scale to 32-bits. - let mut brdiv_ratio = 64 * r.uartibrd().read().0 + r.uartfbrd().read().0; - brdiv_ratio <<= 10; - // 3662 is ~(15 * 244.14) where 244.14 is 16e6 / 2^16 - let scaled_freq = clk_base / 3662; - let wait_time_us = brdiv_ratio / scaled_freq; - embedded_hal_1::delay::DelayNs::delay_us(&mut Delay, wait_time_us); - } - // PL011 needs a (dummy) line control register write to latch in the - // divisors. We don't want to actually change LCR contents here. - r.uartlcr_h().modify(|_| {}); - - r.uartcr().write_value(cr); + Self::lcr_modify(|_| {}); } }