From 9e796b4f3be2eb86e867e1360e54c7f6c650607c Mon Sep 17 00:00:00 2001
From: Benjamin Koch <bbbsnowball@gmail.com>
Date: Sat, 13 May 2023 05:18:17 +0200
Subject: [PATCH] try to make zero byte write transfers work

---
 firmware/rust1/src/bin/heizung.rs |  4 +-
 firmware/rust1/src/rp/i2c.rs      | 66 ++++++++++++++++++++++++++-----
 2 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/firmware/rust1/src/bin/heizung.rs b/firmware/rust1/src/bin/heizung.rs
index 0c1d767..42947b7 100644
--- a/firmware/rust1/src/bin/heizung.rs
+++ b/firmware/rust1/src/bin/heizung.rs
@@ -83,9 +83,9 @@ async fn i2c_task(mut i2c: i2c::I2c<'static, I2C0, i2c::Async>) {
             //NOTE It looks like the blocking implementation doesn't accept an empty buffer
             //     but the async one does. Lucky for us because we need that here!
             // -> Actually, it doesn't. It just hangs.
-            //info!("I2C testing: {}", i);
+            info!("I2C testing: {}", i);
             //FIXME Don't test with buffer size 1 because that will actually write, which can cause all kinds of issues!
-            match i2c.write_async(i.into(), [0; 1]).await {
+            match i2c.write_async(i.into(), [0; 0]).await {
                 Result::Ok(()) => {
                     //info!("I2C ok: {}", i);
                     scan_result[i as usize] = ScanResult::Ack
diff --git a/firmware/rust1/src/rp/i2c.rs b/firmware/rust1/src/rp/i2c.rs
index 7804701..48c3e32 100644
--- a/firmware/rust1/src/rp/i2c.rs
+++ b/firmware/rust1/src/rp/i2c.rs
@@ -172,7 +172,7 @@ impl<'d, T: Instance> I2c<'d, T, Async> {
                 .wait_on(
                     |me| {
                         let rxfifo = Self::rx_fifo_len();
-                        if let Err(abort_reason) = me.read_and_clear_abort_reason() {
+                        if let Err(abort_reason) = me.read_and_clear_abort_reason(false) {
                             Poll::Ready(Err(abort_reason))
                         } else if rxfifo >= batch {
                             Poll::Ready(Ok(rxfifo))
@@ -211,7 +211,7 @@ impl<'d, T: Instance> I2c<'d, T, Async> {
             };
         }
 
-        self.wait_stop_det(abort_reason, send_stop).await
+        self.wait_stop_det(abort_reason, send_stop, false).await
     }
 
     async fn write_async_internal(
@@ -222,6 +222,10 @@ impl<'d, T: Instance> I2c<'d, T, Async> {
         let p = T::regs();
 
         let mut bytes = bytes.into_iter().peekable();
+        let mut empty_transfer = true;
+
+        let status = unsafe { p.ic_status().read() };
+        defmt::info!("ic_status: {:?}, {}", status.0, status.mst_activity());
 
         let res = 'xmit: loop {
             let tx_fifo_space = Self::tx_fifo_capacity();
@@ -229,6 +233,7 @@ impl<'d, T: Instance> I2c<'d, T, Async> {
             for _ in 0..tx_fifo_space {
                 if let Some(byte) = bytes.next() {
                     let last = bytes.peek().is_none();
+                    empty_transfer = false;
 
                     unsafe {
                         p.ic_data_cmd().write(|w| {
@@ -237,6 +242,24 @@ impl<'d, T: Instance> I2c<'d, T, Async> {
                             w.set_dat(byte);
                         });
                     }
+                } else if empty_transfer && send_stop {
+                    // This is no good because it will still send one byte before stopping.
+                    //p.ic_data_cmd().write(|w| {
+                    //    w.set_stop(send_stop);
+                    //});
+    
+                    // The Arduino code that we used before resorts to bitbanging in this case:
+                    // https://github.com/earlephilhower/arduino-pico/blob/6e52b72523b11470b2ad2b0578ca1500be238108/libraries/Wire/src/Wire.cpp#L257
+    
+                    // We can abort the transfer to send a STOP but we have to wait for the address
+                    // to be sent. If we set TX_EMPTY_CTRL=1, it should wait until the address has
+                    // been sent.
+                    //FIXME I think we set this in any case.
+                    unsafe {
+                        p.ic_con().modify(|w| {
+                            w.set_tx_empty_ctrl(true);
+                        });
+                    }
                 } else {
                     break 'xmit Ok(());
                 }
@@ -245,7 +268,7 @@ impl<'d, T: Instance> I2c<'d, T, Async> {
             let res = self
                 .wait_on(
                     |me| {
-                        if let abort_reason @ Err(_) = me.read_and_clear_abort_reason() {
+                        if let abort_reason @ Err(_) = me.read_and_clear_abort_reason(false) {
                             Poll::Ready(abort_reason)
                         } else if !Self::tx_fifo_full() {
                             // resume if there's any space free in the tx fifo
@@ -267,12 +290,35 @@ impl<'d, T: Instance> I2c<'d, T, Async> {
                     },
                 )
                 .await;
+            //defmt::info!("got tx empty: {:?}", res);
             if res.is_err() {
                 break res;
             }
+
+            if empty_transfer && send_stop {
+                let status = unsafe { p.ic_status().read() };
+                let abort_reason = unsafe { p.ic_tx_abrt_source().read() };
+                defmt::info!("ic_status: {:?}, {}, {:?}", status.0, status.mst_activity(), abort_reason.0);
+                unsafe {
+                    p.ic_enable().modify(|w| {
+                        w.set_abort(true);
+                    })
+                }
+                let status = unsafe { p.ic_status().read() };
+                let abort_reason = unsafe { p.ic_tx_abrt_source().read() };
+                defmt::info!("ic_status: {:?}, {}, {:?}", status.0, status.mst_activity(), abort_reason.0);
+
+                defmt::info!("wait_stop_det");
+                let x = self.wait_stop_det(res, send_stop, empty_transfer && send_stop && false).await;
+                return match x {
+                    Ok(()) => Ok(()),
+                    Err(Error::Abort(AbortReason::Other(65536))) => Ok(()),  // user abort is expected
+                    Err(e) => Err(e),
+                }
+            }
         };
 
-        self.wait_stop_det(res, send_stop).await
+        self.wait_stop_det(res, send_stop, empty_transfer && send_stop && false).await
     }
 
     /// Helper to wait for a stop bit, for both tx and rx. If we had an abort,
@@ -280,7 +326,7 @@ impl<'d, T: Instance> I2c<'d, T, Async> {
     /// we're expecting it.
     ///
     /// Also handles an abort which arises while processing the tx fifo.
-    async fn wait_stop_det(&mut self, had_abort: Result<(), Error>, do_stop: bool) -> Result<(), Error> {
+    async fn wait_stop_det(&mut self, had_abort: Result<(), Error>, do_stop: bool, user_abort_is_ok: bool) -> Result<(), Error> {
         if had_abort.is_err() || do_stop {
             let p = T::regs();
 
@@ -289,7 +335,7 @@ impl<'d, T: Instance> I2c<'d, T, Async> {
                     |me| unsafe {
                         // We could see an abort while processing fifo backlog,
                         // so handle it here.
-                        let abort = me.read_and_clear_abort_reason();
+                        let abort = me.read_and_clear_abort_reason(user_abort_is_ok);
                         if had_abort.is_ok() && abort.is_err() {
                             Poll::Ready(abort)
                         } else if p.ic_raw_intr_stat().read().stop_det() {
@@ -502,7 +548,7 @@ impl<'d, T: Instance + 'd, M: Mode> I2c<'d, T, M> {
         unsafe { p.ic_rxflr().read().rxflr() }
     }
 
-    fn read_and_clear_abort_reason(&mut self) -> Result<(), Error> {
+    fn read_and_clear_abort_reason(&mut self, user_abort_is_ok: bool) -> Result<(), Error> {
         let p = T::regs();
         unsafe {
             let abort_reason = p.ic_tx_abrt_source().read();
@@ -519,6 +565,8 @@ impl<'d, T: Instance + 'd, M: Mode> I2c<'d, T, M> {
                     AbortReason::NoAcknowledge
                 } else if abort_reason.arb_lost() {
                     AbortReason::ArbitrationLoss
+                } else if user_abort_is_ok && abort_reason.abrt_user_abrt() {
+                    return Ok(())
                 } else {
                     AbortReason::Other(abort_reason.0)
                 };
@@ -554,7 +602,7 @@ impl<'d, T: Instance + 'd, M: Mode> I2c<'d, T, M> {
                 });
 
                 while Self::rx_fifo_len() == 0 {
-                    self.read_and_clear_abort_reason()?;
+                    self.read_and_clear_abort_reason(false)?;
                 }
 
                 *byte = p.ic_data_cmd().read().dat();
@@ -587,7 +635,7 @@ impl<'d, T: Instance + 'd, M: Mode> I2c<'d, T, M> {
                 // TX_EMPTY_CTRL flag was set in i2c_init.
                 while !p.ic_raw_intr_stat().read().tx_empty() {}
 
-                let abort_reason = self.read_and_clear_abort_reason();
+                let abort_reason = self.read_and_clear_abort_reason(false);
 
                 if abort_reason.is_err() || (send_stop && last) {
                     // If the transaction was aborted or if it completed
-- 
GitLab