From ba61a34ad1ba3e136f449c71d0a4f46364774783 Mon Sep 17 00:00:00 2001
From: Christopher Durand <christopher.durand@rwth-aachen.de>
Date: Wed, 25 Jan 2023 17:58:59 +0100
Subject: [PATCH] [stm32] Fix stm32-extended I2C driver NACK handling

Detach transaction after completion of STOP condition when NACK was
received.
---
 .../i2c/stm32-extended/i2c_master.cpp.in      | 49 ++++++++-----------
 1 file changed, 21 insertions(+), 28 deletions(-)

diff --git a/src/modm/platform/i2c/stm32-extended/i2c_master.cpp.in b/src/modm/platform/i2c/stm32-extended/i2c_master.cpp.in
index bf3887a469..cab6749114 100644
--- a/src/modm/platform/i2c/stm32-extended/i2c_master.cpp.in
+++ b/src/modm/platform/i2c/stm32-extended/i2c_master.cpp.in
@@ -124,9 +124,6 @@ namespace
 
 		I2C{{ id }}->CR1 &= ~(I2C_CR1_STOPIE | I2C_CR1_TCIE | I2C_CR1_RXIE | I2C_CR1_TXIE);
 
-		// Always enable the NACK interrupt in case we're trying to write to a slave that's not responding
-		I2C{{ id }}->CR1 |= I2C_CR1_NACKIE;
-
 		if (autoend and (writing.length == 0))
 		{
 			// Transfer is ended by hardware, so wait for Stop condition generated by hardware.
@@ -174,9 +171,6 @@ namespace
 
 		I2C{{ id }}->CR1 &= ~(I2C_CR1_STOPIE | I2C_CR1_TCIE | I2C_CR1_RXIE | I2C_CR1_TXIE);
 
-		// Always enable the NACK interrupt in case we're trying to read from a slave that's not responding
-		I2C{{ id }}->CR1 |= I2C_CR1_NACKIE;
-
 		if (autoend and (reading.length == 0))
 		{
 			// Transfer is ended by hardware, so wait for Stop condition generated by hardware.
@@ -277,12 +271,12 @@ namespace
 		if (sr1 & I2C_ISR_BERR)
 		{
 			DEBUG_STREAM("BUS ERROR");
-			I2C{{ id }}->ICR |= I2C_ICR_BERRCF;
+			I2C{{ id }}->ICR = I2C_ICR_BERRCF;
 			error = modm::I2cMaster::Error::BusCondition;
 		}
 		else if (sr1 & I2C_ISR_ARLO)
 		{	// arbitration lost
-			I2C{{ id }}->ICR |= I2C_ICR_ARLOCF;
+			I2C{{ id }}->ICR = I2C_ICR_ARLOCF;
 			DEBUG_STREAM("ARBITRATION LOST");
 			error = modm::I2cMaster::Error::ArbitrationLost;
 		}
@@ -290,16 +284,16 @@ namespace
 		{
 			// should only occur in unsupported SMBus mode
 			DEBUG_STREAM("UNKNOWN, SMBUS");
-			I2C{{ id }}->ICR |= I2C_ICR_ALERTCF;
-			I2C{{ id }}->ICR |= I2C_ICR_TIMOUTCF;
-			I2C{{ id }}->ICR |= I2C_ICR_PECCF;
+			I2C{{ id }}->ICR = I2C_ICR_ALERTCF;
+			I2C{{ id }}->ICR = I2C_ICR_TIMOUTCF;
+			I2C{{ id }}->ICR = I2C_ICR_PECCF;
 			error = modm::I2cMaster::Error::Unknown;
 		}
 		else if (sr1 & I2C_ISR_OVR)
 		{
 			// should not occur in master mode
 			DEBUG_STREAM("UNKNOWN");
-			I2C{{ id }}->ICR |= I2C_ICR_OVRCF;
+			I2C{{ id }}->ICR = I2C_ICR_OVRCF;
 			error = modm::I2cMaster::Error::Unknown;
 		}
 		else
@@ -345,7 +339,7 @@ MODM_ISR(I2C{{ id }}_EV)
 
 	I2C{{ id }}->CR1 &= ~(I2C_CR1_STOPIE | I2C_CR1_TCIE | I2C_CR1_RXIE | I2C_CR1_TXIE);
 
-	I2C{{ id }}->ICR |= I2C_ICR_STOPCF;
+	I2C{{ id }}->ICR = I2C_ICR_STOPCF;
 
 #if SERIAL_DEBUGGING
 	if (isr & I2C_ISR_BUSY)  { DEBUG_STREAM_N("BUSY " ); } else { DEBUG_STREAM_N("busy " ); }
@@ -388,23 +382,22 @@ MODM_ISR(I2C{{ id }}_EV)
 		}
 	}
 
-	if (isr & I2C_ISR_NACKF)
-	{
-		// acknowledge fail
-		I2C{{ id }}->ICR |= I2C_ICR_NACKCF;
-		DEBUG_STREAM("ACK FAIL");
-		// may also be ADDRESS_NACK
-		error = starting.address ? modm::I2cMaster::Error::AddressNack : modm::I2cMaster::Error::DataNack;
-		if (transaction) {
-			transaction->detaching(modm::I2c::DetachCause::ErrorCondition);
-			transaction = nullptr;
-		}
-	}
-
+	// Stop condition was generated
 	if (isr & I2C_ISR_STOPF)
 	{
-		// Stop condition was generated
-		if (nextOperation == modm::I2c::Operation::Stop)
+		if (isr & I2C_ISR_NACKF)
+		{
+			// acknowledge fail
+			I2C1->ICR = I2C_ICR_NACKCF;
+			DEBUG_STREAM("ACK FAIL");
+			// may also be ADDRESS_NACK
+			error = starting.address ? modm::I2cMaster::Error::AddressNack : modm::I2cMaster::Error::DataNack;
+			if (transaction) {
+				transaction->detaching(modm::I2c::DetachCause::ErrorCondition);
+				transaction = nullptr;
+			}
+		}
+		else if (nextOperation == modm::I2c::Operation::Stop)
 		{
 			if (transaction) {
 				transaction->detaching(modm::I2c::DetachCause::NormalStop);