From 377348ca03e6eaa63e2efa279c884b2911a0a2cf Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (42 Technology)" Date: Fri, 14 May 2021 16:16:46 +0100 Subject: [PATCH 1/8] Don't check for txstopped when writing to UART. It seems like endtx and txstopped race, and if we see txstopped first we return an error, causing writeln!(...).unwrap() to panic. This changes makes the UART work reliably on the nRF9160-PDK. --- nrf-hal-common/src/uarte.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/nrf-hal-common/src/uarte.rs b/nrf-hal-common/src/uarte.rs index f9d5bf60..dcde954b 100644 --- a/nrf-hal-common/src/uarte.rs +++ b/nrf-hal-common/src/uarte.rs @@ -135,14 +135,8 @@ where unsafe { w.bits(1) }); // Wait for transmission to end. - let mut endtx; - let mut txstopped; - loop { - endtx = self.0.events_endtx.read().bits() != 0; - txstopped = self.0.events_txstopped.read().bits() != 0; - if endtx || txstopped { - break; - } + while self.0.events_endtx.read().bits() == 0 { + // TODO: Do something here which uses less power. Like `wfi`. } // Conservative compiler fence to prevent optimizations that do not @@ -150,16 +144,17 @@ where // after all possible DMA actions have completed. compiler_fence(SeqCst); - if txstopped { - return Err(Error::Transmit); - } - // Lower power consumption by disabling the transmitter once we're // finished. self.0.tasks_stoptx.write(|w| // `1` is a valid value to write to task registers. unsafe { w.bits(1) }); + // Wait for transmitter to stop. + while self.0.events_txstopped.read().bits() == 0 { + // Spin + } + Ok(()) } From 6e034b6dc7cb10b8cf4c396d71d82de36794dbe9 Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (42 Technology)" Date: Fri, 14 May 2021 16:53:25 +0100 Subject: [PATCH 2/8] Apply the UARTE workaround. See https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197 --- nrf-hal-common/src/uarte.rs | 64 ++++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 4 deletions(-) diff --git a/nrf-hal-common/src/uarte.rs b/nrf-hal-common/src/uarte.rs index dcde954b..905f0598 100644 --- a/nrf-hal-common/src/uarte.rs +++ b/nrf-hal-common/src/uarte.rs @@ -73,9 +73,6 @@ where } }); - // Enable UARTE instance. - uarte.enable.write(|w| w.enable().enabled()); - // Configure. let hardware_flow_control = pins.rts.is_some() && pins.cts.is_some(); uarte @@ -84,8 +81,67 @@ where // Configure frequency. uarte.baudrate.write(|w| w.baudrate().variant(baudrate)); + + let mut u = Uarte(uarte); + + #[cfg(any(feature = "9160", feature = "5340"))] + u.apply_workaround_for_enable_anomaly(); + + // Enable UARTE instance. + u.0.enable.write(|w| w.enable().enabled()); + + u + } + + #[cfg(any(feature = "9160", feature = "5340"))] + fn apply_workaround_for_enable_anomaly(&mut self) + { + // Apply workaround for anomalies: + // - nRF9160 - anomaly 23 + // - nRF5340 - anomaly 44 + let rxenable_reg: *const u32 = ((self.0.deref() as *const _ as usize) + 0x564) as *const u32; + let txenable_reg: *const u32 = ((self.0.deref() as *const _ as usize) + 0x568) as *const u32; + + // NB Safety: This is taken from Nordic's driver - + // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197 + if unsafe { core::ptr::read_volatile(txenable_reg) } == 1 { + self.0.tasks_stoprx.write(|w| unsafe { w.bits(1) }); + } + + // NB Safety: This is taken from Nordic's driver - + // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197 + if unsafe { core::ptr::read_volatile(rxenable_reg) } == 1 { + self.0.enable.write(|w| w.enable().enabled()); + self.0.tasks_stoprx.write(|w| unsafe { w.bits(1) }); + + + let mut workaround_succeded = false; + // The UARTE is able to receive up to four bytes after the STOPRX task has been triggered. + // On lowest supported baud rate (1200 baud), with parity bit and two stop bits configured + // (resulting in 12 bits per data byte sent), this may take up to 40 ms. + for _ in 0..40000 { + // NB Safety: This is taken from Nordic's driver - + // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197 + if unsafe { core::ptr::read_volatile(rxenable_reg) } == 0 { + workaround_succeded = true; + break; + } + else + { + // Need to sleep for 1us here + } + } - Uarte(uarte) + if !workaround_succeded + { + panic!("Failed to apply workaround for UART"); + } + + let errors = self.0.errorsrc.read().bits(); + // NB Safety: safe to write back the bits we just read to clear them + self.0.errorsrc.write(|w| unsafe { w.bits(errors) }); + self.0.enable.write(|w| w.enable().disabled()); + } } /// Write via UARTE. From bdd35edbdd9c97a0885cf3e337c129f20c78f3c6 Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (42 Technology)" Date: Fri, 14 May 2021 17:06:28 +0100 Subject: [PATCH 3/8] Disable UART if we find it already enabled. --- nrf-hal-common/src/uarte.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/nrf-hal-common/src/uarte.rs b/nrf-hal-common/src/uarte.rs index 905f0598..73d63791 100644 --- a/nrf-hal-common/src/uarte.rs +++ b/nrf-hal-common/src/uarte.rs @@ -43,6 +43,16 @@ where T: Instance, { pub fn new(uarte: T, mut pins: Pins, parity: Parity, baudrate: Baudrate) -> Self { + // Is the UART already on? It might be if you had a bootloader + if uarte.enable.read().bits() != 0 { + while uarte.events_txstopped.read().bits() == 0 { + // Spin + } + + // Disable UARTE instance + uarte.enable.write(|w| w.enable().disabled()); + } + // Select pins uarte.psel.rxd.write(|w| { unsafe { w.bits(pins.rxd.psel_bits()) }; From 5502284be5f0938d6a8e0d7b40436d57daf753fa Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (42 Technology)" Date: Fri, 14 May 2021 17:13:47 +0100 Subject: [PATCH 4/8] Fix warnings on non-anomalous platforms. --- nrf-hal-common/src/uarte.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nrf-hal-common/src/uarte.rs b/nrf-hal-common/src/uarte.rs index 73d63791..23e2a80e 100644 --- a/nrf-hal-common/src/uarte.rs +++ b/nrf-hal-common/src/uarte.rs @@ -94,7 +94,6 @@ where let mut u = Uarte(uarte); - #[cfg(any(feature = "9160", feature = "5340"))] u.apply_workaround_for_enable_anomaly(); // Enable UARTE instance. @@ -103,6 +102,12 @@ where u } + #[cfg(not(any(feature = "9160", feature = "5340")))] + fn apply_workaround_for_enable_anomaly(&mut self) + { + // Do nothing + } + #[cfg(any(feature = "9160", feature = "5340"))] fn apply_workaround_for_enable_anomaly(&mut self) { From de05901f6cff4fb3c7cec0b5470677f8a842a945 Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (42 Technology)" Date: Fri, 14 May 2021 17:43:35 +0100 Subject: [PATCH 5/8] We no longer use this event so don't need to reset it. --- nrf-hal-common/src/uarte.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/nrf-hal-common/src/uarte.rs b/nrf-hal-common/src/uarte.rs index 23e2a80e..e4ad1d50 100644 --- a/nrf-hal-common/src/uarte.rs +++ b/nrf-hal-common/src/uarte.rs @@ -180,7 +180,6 @@ where // Reset the events. self.0.events_endtx.reset(); - self.0.events_txstopped.reset(); // Set up the DMA write. self.0.txd.ptr.write(|w| From 1facd186ba91ee8676422bf5027720a3159e510c Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (42 Technology)" Date: Fri, 14 May 2021 17:46:04 +0100 Subject: [PATCH 6/8] Stop the right task in the anomaly workaround. --- nrf-hal-common/src/uarte.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nrf-hal-common/src/uarte.rs b/nrf-hal-common/src/uarte.rs index e4ad1d50..4244bd22 100644 --- a/nrf-hal-common/src/uarte.rs +++ b/nrf-hal-common/src/uarte.rs @@ -120,7 +120,7 @@ where // NB Safety: This is taken from Nordic's driver - // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197 if unsafe { core::ptr::read_volatile(txenable_reg) } == 1 { - self.0.tasks_stoprx.write(|w| unsafe { w.bits(1) }); + self.0.tasks_stoptx.write(|w| unsafe { w.bits(1) }); } // NB Safety: This is taken from Nordic's driver - From bd0f7d8e9daba2c4b4aa37aabac30a478f6b3506 Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (42 Technology)" Date: Fri, 14 May 2021 17:50:43 +0100 Subject: [PATCH 7/8] Stop TX before we wait for TX to stop. --- nrf-hal-common/src/uarte.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/nrf-hal-common/src/uarte.rs b/nrf-hal-common/src/uarte.rs index 4244bd22..a697004e 100644 --- a/nrf-hal-common/src/uarte.rs +++ b/nrf-hal-common/src/uarte.rs @@ -45,6 +45,7 @@ where pub fn new(uarte: T, mut pins: Pins, parity: Parity, baudrate: Baudrate) -> Self { // Is the UART already on? It might be if you had a bootloader if uarte.enable.read().bits() != 0 { + uarte.tasks_stoptx.write(|w| unsafe { w.bits(1) }); while uarte.events_txstopped.read().bits() == 0 { // Spin } From 60a3c0990458ba57e7a43f1d8dca7f38fac8713a Mon Sep 17 00:00:00 2001 From: "Jonathan Pallant (42 Technology)" Date: Fri, 14 May 2021 17:55:05 +0100 Subject: [PATCH 8/8] Move the event resets closer to where we check the events. --- nrf-hal-common/src/uarte.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/nrf-hal-common/src/uarte.rs b/nrf-hal-common/src/uarte.rs index a697004e..5aa26477 100644 --- a/nrf-hal-common/src/uarte.rs +++ b/nrf-hal-common/src/uarte.rs @@ -179,9 +179,7 @@ where // before any DMA action has started. compiler_fence(SeqCst); - // Reset the events. - self.0.events_endtx.reset(); - + // Set up the DMA write. self.0.txd.ptr.write(|w| // We're giving the register a pointer to the stack. Since we're @@ -200,11 +198,15 @@ where // values. unsafe { w.maxcnt().bits(tx_buffer.len() as _) }); + // Reset the event + self.0.events_endtx.reset(); + // Start UARTE Transmit transaction. self.0.tasks_starttx.write(|w| // `1` is a valid value to write to task registers. unsafe { w.bits(1) }); + // Wait for transmission to end. while self.0.events_endtx.read().bits() == 0 { // TODO: Do something here which uses less power. Like `wfi`. @@ -215,6 +217,9 @@ where // after all possible DMA actions have completed. compiler_fence(SeqCst); + // Reset the event + self.0.events_txstopped.reset(); + // Lower power consumption by disabling the transmitter once we're // finished. self.0.tasks_stoptx.write(|w|