nrf: fix stop condition race in i2c

src/machine/machine_nrf.go

@@ -225,6 +225,9 @@ type I2CConfig struct {
 
 // Configure is intended to setup the I2C interface.
 func (i2c *I2C) Configure(config I2CConfig) error {
+
+	i2c.Bus.ENABLE.Set(nrf.TWI_ENABLE_ENABLE_Disabled)
+
 	// Default I2C bus speed is 100 kHz.
 	if config.Frequency == 0 {
 		config.Frequency = TWI_FREQ_100KHZ
@@ -256,9 +259,10 @@ func (i2c *I2C) Configure(config I2CConfig) error {
 		i2c.Bus.FREQUENCY.Set(nrf.TWI_FREQUENCY_FREQUENCY_K100)
 	}
 
-	i2c.Bus.ENABLE.Set(nrf.TWI_ENABLE_ENABLE_Enabled)
 	i2c.setPins(config.SCL, config.SDA)
 
+	i2c.Bus.ENABLE.Set(nrf.TWI_ENABLE_ENABLE_Enabled)
+
 	return nil
 }
 
@@ -266,16 +270,23 @@ func (i2c *I2C) Configure(config I2CConfig) error {
 // It clocks out the given address, writes the bytes in w, reads back len(r)
 // bytes and stores them in r, and generates a stop condition on the bus.
 func (i2c *I2C) Tx(addr uint16, w, r []byte) (err error) {
+
+	// Tricky stop condition.
+	// After reads, the stop condition is generated implicitly with a shortcut.
+	// After writes not followed by reads and in the case of errors, stop must be generated explicitly.
+
 	i2c.Bus.ADDRESS.Set(uint32(addr))
 
 	if len(w) != 0 {
 		i2c.Bus.TASKS_STARTTX.Set(1) // start transmission for writing
 		for _, b := range w {
 			if err = i2c.writeByte(b); err != nil {
-				goto cleanUp
+				i2c.signalStop()
+				return
 			}
 		}
 	}
+
 	if len(r) != 0 {
 		// To trigger suspend task when a byte is received
 		i2c.Bus.SHORTS.Set(nrf.TWI_SHORTS_BB_SUSPEND)
@@ -289,22 +300,25 @@ func (i2c *I2C) Tx(addr uint16, w, r []byte) (err error) {
 				i2c.Bus.TASKS_RESUME.Set(1) // re-start transmission for reading
 			}
 			if r[i], err = i2c.readByte(); err != nil {
-				// goto/break are practically equivalent here,
+				i2c.Bus.SHORTS.Set(nrf.TWI_SHORTS_BB_SUSPEND_Disabled)
-				// but goto makes this more easily understandable for maintenance.
+				i2c.signalStop()
-				goto cleanUp
+				return
 			}
 		}
+		i2c.Bus.SHORTS.Set(nrf.TWI_SHORTS_BB_SUSPEND_Disabled)
 	}
 
-cleanUp:
+	// Stop explicitly when no reads were executed, stoping unconditionally would be a mistake.
+	// It may execute after I2C peripheral has already been stopped by the shortcut in the read block,
+	// so stop task will trigger first thing in a subsequent transaction, hanging it.
+	if len(r) == 0 {
 		i2c.signalStop()
-	i2c.Bus.SHORTS.Set(nrf.TWI_SHORTS_BB_SUSPEND_Disabled)
+	}
+
 	return
 }
 
-// signalStop sends a stop signal when writing or tells the I2C peripheral that
+// signalStop sends a stop signal to the I2C peripheral and waits for confirmation.
-// it must generate a stop condition after the next character is retrieved when
-// reading.
 func (i2c *I2C) signalStop() {
 	i2c.Bus.TASKS_STOP.Set(1)
 	for i2c.Bus.EVENTS_STOPPED.Get() == 0 {
@@ -312,7 +326,7 @@ func (i2c *I2C) signalStop() {
 	i2c.Bus.EVENTS_STOPPED.Set(0)
 }
 
-// writeByte writes a single byte to the I2C bus.
+// writeByte writes a single byte to the I2C bus and waits for confirmation.
 func (i2c *I2C) writeByte(data byte) error {
 	i2c.Bus.TXD.Set(uint32(data))
 	for i2c.Bus.EVENTS_TXDSENT.Get() == 0 {
@@ -325,7 +339,7 @@ func (i2c *I2C) writeByte(data byte) error {
 	return nil
 }
 
-// readByte reads a single byte from the I2C bus.
+// readByte reads a single byte from the I2C bus when it is ready.
 func (i2c *I2C) readByte() (byte, error) {
 	for i2c.Bus.EVENTS_RXDREADY.Get() == 0 {
 		if e := i2c.Bus.EVENTS_ERROR.Get(); e != 0 {